diff --git a/.github/workflows/backend-motoko.yml b/.github/workflows/backend-motoko.yml index 68e5e1fd..084978a6 100644 --- a/.github/workflows/backend-motoko.yml +++ b/.github/workflows/backend-motoko.yml @@ -30,6 +30,8 @@ jobs: cd backend/mo/ic_vetkeys mops install mops test + cd ../canisters/ic_vetkeys_manager_canister + make test backend-motoko-tests-darwin: runs-on: macos-15 steps: @@ -45,3 +47,5 @@ jobs: cd backend/mo/ic_vetkeys mops install mops test + cd ../canisters/ic_vetkeys_manager_canister + make test diff --git a/backend/mo/canisters/ic_vetkeys_manager_canister/Makefile b/backend/mo/canisters/ic_vetkeys_manager_canister/Makefile new file mode 100644 index 00000000..6ae7faa1 --- /dev/null +++ b/backend/mo/canisters/ic_vetkeys_manager_canister/Makefile @@ -0,0 +1,13 @@ +PWD:=$(shell pwd) + +.PHONY: compile-wasm +.SILENT: compile-wasm +compile-wasm: + dfx build --check + +# Test the APIs of this canister using the respective Rust canister tests. +# This has the advantage that the tests are consistent (less room for bugs by having only one implementation of the tests) and the checked expected behavior is consistent across Rust and Motoko. +.PHONY: test +.SILENT: test +test: compile-wasm + CUSTOM_WASM_PATH=$(PWD)/.dfx/local/canisters/ic_vetkeys_manager_canister/ic_vetkeys_manager_canister.wasm cargo test -p ic-vetkeys-manager-canister diff --git a/backend/mo/canisters/ic_vetkeys_manager_canister/dfx.json b/backend/mo/canisters/ic_vetkeys_manager_canister/dfx.json new file mode 100644 index 00000000..9c595e2c --- /dev/null +++ b/backend/mo/canisters/ic_vetkeys_manager_canister/dfx.json @@ -0,0 +1,9 @@ +{ + "canisters": { + "ic_vetkeys_manager_canister": { + "main": "src/Main.mo", + "type": "motoko", + "args": "--enhanced-orthogonal-persistence" + } + } +} diff --git a/backend/mo/canisters/ic_vetkeys_manager_canister/src/Main.mo b/backend/mo/canisters/ic_vetkeys_manager_canister/src/Main.mo new file mode 100644 index 00000000..cb8c60a9 --- /dev/null +++ b/backend/mo/canisters/ic_vetkeys_manager_canister/src/Main.mo @@ -0,0 +1,86 @@ +import { KeyManager } "../../../ic_vetkeys/src"; +import Types "../../../ic_vetkeys/src/Types"; +import Principal "mo:base/Principal"; +import Text "mo:base/Text"; +import Blob "mo:base/Blob"; +import Result "mo:base/Result"; +import Array "mo:base/Array"; + +actor { + var keyManager = KeyManager.KeyManager("key_manager", Types.accessRightsOperations()); + /// In this canister, we use the `ByteBuf` type to represent blobs. The reason is that we want to be consistent with the Rust canister implementation. + /// Unfortunately, the `Blob` type cannot be serialized/deserialized in the current Rust implementation efficiently without nesting it in another type. + public type ByteBuf = { inner : Blob }; + + /// The result type compatible with Rust's `Result`. + public type Result = { + #Ok : Ok; + #Err : Err; + }; + + public query (msg) func get_accessible_shared_key_ids() : async [(Principal, ByteBuf)] { + Array.map<(Principal, Blob), (Principal, ByteBuf)>( + keyManager.getAccessibleSharedKeyIds(msg.caller), + func((principal, blob) : (Principal, Blob)) { + (principal, { inner = blob }); + }, + ); + }; + + public query (msg) func get_shared_user_access_for_key( + key_owner : Principal, + key_name : ByteBuf, + ) : async Result<[(Principal, Types.AccessRights)], Text> { + convertResult(keyManager.getSharedUserAccessForKey(msg.caller, (key_owner, key_name.inner))); + }; + + public shared func get_vetkey_verification_key() : async ByteBuf { + let inner = await keyManager.getVetkeyVerificationKey(); + { inner }; + }; + + public shared (msg) func get_encrypted_vetkey( + key_owner : Principal, + key_name : ByteBuf, + transport_key : ByteBuf, + ) : async Result { + let vetkeyBytebuf = await keyManager.getEncryptedVetkey(msg.caller, (key_owner, key_name.inner), transport_key.inner); + switch (vetkeyBytebuf) { + case (#err(e)) { #Err(e) }; + case (#ok(inner)) { #Ok({ inner }) }; + }; + }; + + public query (msg) func get_user_rights( + key_owner : Principal, + key_name : ByteBuf, + user : Principal, + ) : async Result { + convertResult(keyManager.getUserRights(msg.caller, (key_owner, key_name.inner), user)); + }; + + public shared (msg) func set_user_rights( + key_owner : Principal, + key_name : ByteBuf, + user : Principal, + access_rights : Types.AccessRights, + ) : async Result { + convertResult(keyManager.setUserRights(msg.caller, (key_owner, key_name.inner), user, access_rights)); + }; + + public shared (msg) func remove_user( + key_owner : Principal, + key_name : ByteBuf, + user : Principal, + ) : async Result { + convertResult(keyManager.removeUserRights(msg.caller, (key_owner, key_name.inner), user)); + }; + + /// Convert to the result type compatible with Rust's `Result` + private func convertResult(result : Result.Result) : Result { + switch (result) { + case (#err(e)) { #Err(e) }; + case (#ok(o)) { #Ok(o) }; + }; + }; +}; diff --git a/backend/mo/ic_vetkeys/src/key_manager/KeyManager.mo b/backend/mo/ic_vetkeys/src/key_manager/KeyManager.mo index cbb66a76..4299d1d7 100644 --- a/backend/mo/ic_vetkeys/src/key_manager/KeyManager.mo +++ b/backend/mo/ic_vetkeys/src/key_manager/KeyManager.mo @@ -7,6 +7,7 @@ import OrderedMap "mo:base/OrderedMap"; import Result "mo:base/Result"; import Types "../Types"; import Text "mo:base/Text"; +import Nat8 "mo:base/Nat8"; module { public type VetKeyVerificationKey = Blob; @@ -114,8 +115,10 @@ module { switch (ensureUserCanRead(caller, keyId)) { case (#err(msg)) { #err(msg) }; case (#ok(_)) { + let principalBytes = Blob.toArray(Principal.toBlob(keyId.0)); let input = Array.flatten([ - Blob.toArray(Principal.toBlob(keyId.0)), + [Nat8.fromNat(Array.size(principalBytes))], + principalBytes, Blob.toArray(keyId.1), ]); @@ -128,7 +131,7 @@ module { transport_public_key = transportKey; }; - let (reply) = await (actor ("aaaaa-aa") : VetkdSystemApi).vetkd_derive_key(request); + let (reply) = await (with cycles = 26_153_846_153) (actor ("aaaaa-aa") : VetkdSystemApi).vetkd_derive_key(request); #ok(reply.encrypted_key); }; }; diff --git a/backend/mo/ic_vetkeys/src/lib.mo b/backend/mo/ic_vetkeys/src/lib.mo index 3a8db7dd..8c3f5b0a 100644 --- a/backend/mo/ic_vetkeys/src/lib.mo +++ b/backend/mo/ic_vetkeys/src/lib.mo @@ -5,10 +5,8 @@ import Types "Types"; module { public type AccessControlOperations = Types.AccessControlOperations; public type AccessRights = Types.AccessRights; - - public type KeyManager = KeyManagerModule.KeyManager; - public let KeyManager = KeyManagerModule.KeyManager; - public type EncryptedMaps = EncryptedMapsModule.EncryptedMaps; - public let EncryptedMaps = EncryptedMapsModule.EncryptedMaps; public let accessRightsOperations = Types.accessRightsOperations; + + public let KeyManager = KeyManagerModule; + public let EncryptedMaps = EncryptedMapsModule; }; diff --git a/backend/mo/ic_vetkeys/test/EncryptedMaps.test.mo b/backend/mo/ic_vetkeys/test/EncryptedMaps.test.mo index d5111b34..301b3e65 100644 --- a/backend/mo/ic_vetkeys/test/EncryptedMaps.test.mo +++ b/backend/mo/ic_vetkeys/test/EncryptedMaps.test.mo @@ -6,10 +6,10 @@ import Text "mo:base/Text"; import Blob "mo:base/Blob"; import { test } "mo:test"; -type EncryptedMaps = VetKey.EncryptedMaps; +type EncryptedMaps = VetKey.EncryptedMaps.EncryptedMaps; let accessRightsOperations = VetKey.accessRightsOperations(); func newEncryptedMaps() : EncryptedMaps { - EncryptedMaps("encrypted maps", accessRightsOperations); + EncryptedMaps.EncryptedMaps("encrypted maps", accessRightsOperations); }; let p1 = Principal.fromText("2vxsx-fae"); @@ -21,7 +21,7 @@ let mapValue = Text.encodeUtf8("some value"); test( "can remove map values", func() { - let encryptedMaps = VetKey.EncryptedMaps("encrypted maps", accessRightsOperations); + let encryptedMaps = newEncryptedMaps(); let result = encryptedMaps.removeMapValues(p1, (p1, mapName)); switch (result) { case (#ok(keys)) { diff --git a/backend/mo/ic_vetkeys/test/KeyManager.test.mo b/backend/mo/ic_vetkeys/test/KeyManager.test.mo index 13cbec63..4ed2bb42 100644 --- a/backend/mo/ic_vetkeys/test/KeyManager.test.mo +++ b/backend/mo/ic_vetkeys/test/KeyManager.test.mo @@ -9,9 +9,8 @@ let p1 = Principal.fromText("2vxsx-fae"); let p2 = Principal.fromText("aaaaa-aa"); let keyName = Text.encodeUtf8("some key"); -type KeyManager = VetKey.KeyManager; -func newKeyManager() : KeyManager { - VetKey.KeyManager("key manager", accessRightsOperations); +func newKeyManager() : VetKey.KeyManager.KeyManager { + VetKey.KeyManager.KeyManager("key manager", accessRightsOperations); }; test( diff --git a/backend/rs/canisters/ic_vetkeys_encrypted_maps_canister/tests/tests.rs b/backend/rs/canisters/ic_vetkeys_encrypted_maps_canister/tests/tests.rs index 64f3b3f6..e99aacbc 100644 --- a/backend/rs/canisters/ic_vetkeys_encrypted_maps_canister/tests/tests.rs +++ b/backend/rs/canisters/ic_vetkeys_encrypted_maps_canister/tests/tests.rs @@ -106,7 +106,7 @@ fn map_sharing_should_work() { map_owner, map_name.clone(), env.principal_1, - AccessRights::Read, + AccessRights::ReadWriteManage, )) .unwrap(), ) @@ -129,7 +129,7 @@ fn map_sharing_should_work() { encode_args((map_owner, map_name.clone(), env.principal_1)).unwrap(), ) .unwrap(); - assert_eq!(current_rights_shared, Some(AccessRights::Read)); + assert_eq!(current_rights_shared, Some(AccessRights::ReadWriteManage)); let mut get_vetkey = |caller: Principal| -> Vec { let transport_key = random_transport_key(rng); diff --git a/backend/rs/canisters/ic_vetkeys_manager_canister/tests/tests.rs b/backend/rs/canisters/ic_vetkeys_manager_canister/tests/tests.rs index 0cd92a6a..9a5fd729 100644 --- a/backend/rs/canisters/ic_vetkeys_manager_canister/tests/tests.rs +++ b/backend/rs/canisters/ic_vetkeys_manager_canister/tests/tests.rs @@ -103,6 +103,15 @@ fn key_sharing_should_work() { let not_key_owner = env.principal_1; let key_name = random_key_name(rng); + assert_eq!( + env.query::, String>>( + not_key_owner, + "get_user_rights", + encode_args((key_owner, key_name.clone(), not_key_owner)).unwrap(), + ), + Err("unauthorized".to_string()) + ); + let prev_rights = env .update::, String>>( env.principal_0, @@ -111,7 +120,7 @@ fn key_sharing_should_work() { key_owner, key_name.clone(), env.principal_1, - AccessRights::Read, + AccessRights::ReadWriteManage, )) .unwrap(), ) @@ -134,7 +143,7 @@ fn key_sharing_should_work() { encode_args((key_owner, key_name.clone(), not_key_owner)).unwrap(), ) .unwrap(); - assert_eq!(current_rights_shared, Some(AccessRights::Read)); + assert_eq!(current_rights_shared, Some(AccessRights::ReadWriteManage)); let mut get_vetkey = |caller: Principal| -> Vec { let transport_key = random_transport_key(rng); @@ -232,10 +241,13 @@ impl TestEnvironment { } fn load_key_manager_example_canister_wasm() -> Vec { - let wasm_path_string = format!( - "{}/target/wasm32-unknown-unknown/release/ic_vetkeys_manager_canister.wasm", - git_root_dir() - ); + let wasm_path_string = match std::env::var("CUSTOM_WASM_PATH") { + Ok(path) if !path.is_empty() => path, + _ => format!( + "{}/target/wasm32-unknown-unknown/release/ic_vetkeys_manager_canister.wasm", + git_root_dir() + ), + }; let wasm_path = Path::new(&wasm_path_string); let wasm_bytes = std::fs::read(wasm_path).expect( "wasm does not exist - run `cargo build --release --target wasm32-unknown-unknown`", diff --git a/backend/rs/ic_vetkeys/src/encrypted_maps/mod.rs b/backend/rs/ic_vetkeys/src/encrypted_maps/mod.rs index 64579499..10cb23ed 100644 --- a/backend/rs/ic_vetkeys/src/encrypted_maps/mod.rs +++ b/backend/rs/ic_vetkeys/src/encrypted_maps/mod.rs @@ -92,10 +92,7 @@ impl EncryptedMaps { caller: Principal, key_id: KeyId, ) -> Result, String> { - match self.key_manager.get_user_rights(caller, key_id, caller)? { - Some(a) if a.can_write() => Ok(()), - Some(_) | None => Err("unauthorized".to_string()), - }?; + self.key_manager.ensure_user_can_write(caller, key_id)?; let keys: Vec<_> = self .mapkey_vals @@ -117,7 +114,7 @@ impl EncryptedMaps { caller: Principal, key_id: KeyId, ) -> Result, String> { - self.key_manager.get_user_rights(caller, key_id, caller)?; + self.key_manager.ensure_user_can_read(caller, key_id)?; Ok(self .mapkey_vals @@ -134,9 +131,8 @@ impl EncryptedMaps { key_id: KeyId, key: MapKey, ) -> Result, String> { - self.key_manager - .get_user_rights(caller, key_id, caller) - .map(|_| self.mapkey_vals.get(&(key_id, key))) + self.key_manager.ensure_user_can_read(caller, key_id)?; + Ok(self.mapkey_vals.get(&(key_id, key))) } /// Retrieves the non-empty map names owned by the caller. @@ -205,11 +201,7 @@ impl EncryptedMaps { key: MapKey, encrypted_value: EncryptedMapValue, ) -> Result, String> { - match self.key_manager.get_user_rights(caller, key_id, caller)? { - Some(a) if a.can_write() => Ok(()), - Some(_) | None => Err("unauthorized".to_string()), - }?; - + self.key_manager.ensure_user_can_write(caller, key_id)?; Ok(self.mapkey_vals.insert((key_id, key), encrypted_value)) } @@ -220,11 +212,7 @@ impl EncryptedMaps { key_id: KeyId, key: MapKey, ) -> Result, String> { - match self.key_manager.get_user_rights(caller, key_id, caller)? { - Some(a) if a.can_write() => Ok(()), - Some(_) | None => Err("unauthorized".to_string()), - }?; - + self.key_manager.ensure_user_can_write(caller, key_id)?; Ok(self.mapkey_vals.remove(&(key_id, key))) } diff --git a/backend/rs/ic_vetkeys/src/key_manager/mod.rs b/backend/rs/ic_vetkeys/src/key_manager/mod.rs index 29b33d14..b237e3e6 100644 --- a/backend/rs/ic_vetkeys/src/key_manager/mod.rs +++ b/backend/rs/ic_vetkeys/src/key_manager/mod.rs @@ -173,7 +173,7 @@ impl KeyManager { key_id: KeyId, user: Principal, ) -> Result, String> { - self.ensure_user_can_read(caller, key_id)?; + self.ensure_user_can_get_user_rights(caller, key_id)?; Ok(self.ensure_user_can_read(user, key_id).ok()) } @@ -215,7 +215,7 @@ impl KeyManager { /// Ensures that a user has read access to a key before proceeding. /// Returns an error if the user is not authorized. - fn ensure_user_can_read(&self, user: Principal, key_id: KeyId) -> Result { + pub fn ensure_user_can_read(&self, user: Principal, key_id: KeyId) -> Result { let is_owner = user == key_id.0; if is_owner { return Ok(T::owner_rights()); @@ -228,7 +228,24 @@ impl KeyManager { } } - fn ensure_user_can_get_user_rights(&self, user: Principal, key_id: KeyId) -> Result { + pub fn ensure_user_can_write(&self, user: Principal, key_id: KeyId) -> Result { + let is_owner = user == key_id.0; + if is_owner { + return Ok(T::owner_rights()); + } + + let has_shared_access = self.access_control.get(&(user, key_id)); + match has_shared_access { + Some(access_rights) if access_rights.can_write() => Ok(access_rights), + _ => Err("unauthorized".to_string()), + } + } + + pub fn ensure_user_can_get_user_rights( + &self, + user: Principal, + key_id: KeyId, + ) -> Result { let is_owner = user == key_id.0; if is_owner { return Ok(T::owner_rights()); @@ -243,7 +260,11 @@ impl KeyManager { /// Ensures that a user has management access to a key before proceeding. /// Returns an error if the user is not authorized. - fn ensure_user_can_set_user_rights(&self, user: Principal, key_id: KeyId) -> Result { + pub fn ensure_user_can_set_user_rights( + &self, + user: Principal, + key_id: KeyId, + ) -> Result { let is_owner = user == key_id.0; if is_owner { return Ok(T::owner_rights()); diff --git a/backend/rs/ic_vetkeys/src/types.rs b/backend/rs/ic_vetkeys/src/types.rs index e30d5a21..1085c45c 100644 --- a/backend/rs/ic_vetkeys/src/types.rs +++ b/backend/rs/ic_vetkeys/src/types.rs @@ -101,6 +101,9 @@ pub trait AccessControl: fn owner_rights() -> Self; } +/// Efficiently serializable and deserializable byte vector that is `Storable` with `ic_stable_structures`. +/// See, e.g., https://mmapped.blog/posts/01-effective-rust-canisters#serde-bytes for more details regarding why `Vec` does not work out of the box. +/// Also, we cannot use `serde_bytes::ByteBuf` directly because it is not `Storable`. #[derive(CandidType, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Clone, Debug)] pub struct ByteBuf { #[serde(with = "serde_bytes")]