From 1e62718f41ed75af0c43d178e6aa2f691aa70a63 Mon Sep 17 00:00:00 2001 From: Franz-Stefan Preiss Date: Fri, 29 Nov 2024 10:56:54 +0000 Subject: [PATCH 01/22] feat(crypto): CRP-2648 split EncryptedKey::combine --- .../crypto_lib/bls12_381/vetkd/src/lib.rs | 86 ++++++++++++++----- 1 file changed, 64 insertions(+), 22 deletions(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs index f4328259004..a87f1ebe832 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs @@ -275,9 +275,10 @@ pub enum EncryptedKeyCombinationError { DuplicateNodeIndex, /// There were insufficient shares to perform combination InsufficientShares, - /// Some of the key shares are invalid; the Vec contains the list of - /// node indexes whose shares were malformed - InvalidKeyShares(Vec), + /// Not enough valid shares + InsufficientValidKeyShares, + /// Some of the key shares are invalid + InvalidShares, } #[derive(Clone, Eq, PartialEq, Debug)] @@ -292,14 +293,11 @@ impl EncryptedKey { /// The length of the serialized encoding of this type pub const BYTES: usize = 2 * G1Affine::BYTES + G2Affine::BYTES; - /// Combine several shares into an encrypted key - pub fn combine( - nodes: &[(NodeIndex, G2Affine, EncryptedKeyShare)], + /// Combine, unchecked. + /// The returned key may be invalid. + fn combine_unchecked( + nodes: &[(NodeIndex, EncryptedKeyShare)], reconstruction_threshold: usize, - master_pk: &G2Affine, - tpk: &TransportPublicKey, - derivation_path: &DerivationPath, - did: &[u8], ) -> Result { if nodes.len() < reconstruction_threshold { return Err(EncryptedKeyCombinationError::InsufficientShares); @@ -309,30 +307,74 @@ impl EncryptedKey { .map_err(|_| EncryptedKeyCombinationError::DuplicateNodeIndex)?; let c1 = l - .interpolate_g1(&nodes.iter().map(|i| &i.2.c1).collect::>()) + .interpolate_g1(&nodes.iter().map(|i| &i.1.c1).collect::>()) .expect("Number of nodes and shares guaranteed equal"); let c2 = l - .interpolate_g2(&nodes.iter().map(|i| &i.2.c2).collect::>()) + .interpolate_g2(&nodes.iter().map(|i| &i.1.c2).collect::>()) .expect("Number of nodes and shares guaranteed equal"); let c3 = l - .interpolate_g1(&nodes.iter().map(|i| &i.2.c3).collect::>()) + .interpolate_g1(&nodes.iter().map(|i| &i.1.c3).collect::>()) .expect("Number of nodes and shares guaranteed equal"); - let c = Self { c1, c2, c3 }; + Ok(Self { c1, c2, c3 }) + } + /// Combines all the given shares into an encrypted key. + /// The returned key is guaranteed to be valid. + /// Returns the combined key, if it is valid. + /// Does not take the nodes individual public keys as input. + pub fn combine_all( + nodes: &[(NodeIndex, EncryptedKeyShare)], + reconstruction_threshold: usize, + master_pk: &G2Affine, + tpk: &TransportPublicKey, + derivation_path: &DerivationPath, + did: &[u8], + ) -> Result { + let c = Self::combine_unchecked(nodes, reconstruction_threshold)?; if !c.is_valid(master_pk, derivation_path, did, tpk) { - // Detect and return the invalid share id(s) - let mut invalid = vec![]; + return Err(EncryptedKeyCombinationError::InvalidShares); + } + Ok(c) + } - for (node_id, node_pk, node_eks) in nodes { - if !node_eks.is_valid(master_pk, node_pk, derivation_path, did, tpk) { - invalid.push(*node_id); - } - } + /// Filters the valid shares from the given ones, and combines them into a valid key, if possible. + /// The returned key is guaranteed to be valid. + /// Returns an error if not sufficient shares are given or if not sufficient valid shares are given. + /// Takes also the node's individual public keys as input, which means the individual public keys + /// must be available: calculating them is comparatively expensive. Note that combine_all does not + /// take the individual public keys as input. + pub fn combine_valid_shares( + nodes: &[(NodeIndex, G2Affine, EncryptedKeyShare)], + reconstruction_threshold: usize, + master_pk: &G2Affine, + tpk: &TransportPublicKey, + derivation_path: &DerivationPath, + did: &[u8], + ) -> Result { + if nodes.len() < reconstruction_threshold { + return Err(EncryptedKeyCombinationError::InsufficientShares); + } - return Err(EncryptedKeyCombinationError::InvalidKeyShares(invalid)); + let valid_shares: Vec<_> = nodes + .iter() + .filter(|(_node_index, node_pk, node_eks)| { + node_eks.is_valid(master_pk, node_pk, derivation_path, did, tpk) + }) + .take(reconstruction_threshold) + .cloned() // TODO: can we avoid cloning somehow? + .map(|(node_index, _node_pk, node_eks)| (node_index, node_eks)) + .collect(); + + if valid_shares.len() < reconstruction_threshold { + return Err(EncryptedKeyCombinationError::InsufficientValidKeyShares); } + // TODO: If combining sufficient valid shares does not guarantee a valid key, we + // have to use combine_all (which includes the validity check) instead of + // combine_unchecked here (or add an explicit validity check here). + let c = Self::combine_unchecked(&valid_shares, reconstruction_threshold)?; + Ok(c) } From ab79fd6d41dcdd0dcbce535cc5e9eae3cb305936 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 13 Dec 2024 13:38:28 -0500 Subject: [PATCH 02/22] Fix and improve tests --- Cargo.lock | 1 + .../crypto_lib/bls12_381/type/src/lib.rs | 12 + .../crypto_lib/bls12_381/vetkd/BUILD.bazel | 1 + .../crypto_lib/bls12_381/vetkd/Cargo.toml | 1 + .../crypto_lib/bls12_381/vetkd/src/lib.rs | 72 ++-- .../crypto_lib/bls12_381/vetkd/tests/tests.rs | 356 ++++++++++++++++-- 6 files changed, 375 insertions(+), 68 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0acdfac6009..eebc6853103 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7236,6 +7236,7 @@ dependencies = [ "ic-crypto-test-utils-reproducible-rng", "ic-sha3 1.0.0", "rand 0.8.5", + "rand_chacha 0.3.1", "zeroize", ] diff --git a/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs index 71d7f707f29..99e49f17f88 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs @@ -1908,6 +1908,18 @@ declare_muln_vartime_affine_impl_for!(G1Projective, G1Affine); impl_debug_using_serialize_for!(G1Affine); impl_debug_using_serialize_for!(G1Projective); +impl G1Affine { + /// See draft-irtf-cfrg-bls-signature-01 §4.2.2 for details on BLS augmented signatures + pub fn augmented_hash(pk: &G2Affine, data: &[u8]) -> Self { + let domain_sep = b"BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_AUG_"; + + let mut signature_input = vec![]; + signature_input.extend_from_slice(&pk.serialize()); + signature_input.extend_from_slice(data); + Self::hash(domain_sep, &signature_input) + } +} + define_affine_and_projective_types!(G2Affine, G2Projective, 96); declare_addsub_ops_for!(G2Projective); declare_mixed_addition_ops_for!(G2Projective, G2Affine); diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel index 0bcfb5262b5..41fd0f1a7cd 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel @@ -17,6 +17,7 @@ DEV_DEPENDENCIES = [ # Keep sorted. "//rs/crypto/test_utils/reproducible_rng", "@crate_index//:hex", + "@crate_index//:rand_chacha", ] MACRO_DEV_DEPENDENCIES = [] diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml index 5394d57cc9f..6d31482c955 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml @@ -16,6 +16,7 @@ zeroize = { workspace = true } criterion = { workspace = true } hex = { workspace = true } ic-crypto-test-utils-reproducible-rng = { path = "../../../../test_utils/reproducible_rng" } +rand_chacha = { workspace = true } [[bench]] name = "vetkd" diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs index a87f1ebe832..556dd19174e 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs @@ -5,7 +5,9 @@ #![forbid(unsafe_code)] #![forbid(missing_docs)] -pub use ic_crypto_internal_bls12_381_type::*; +use ic_crypto_internal_bls12_381_type::{ + G1Affine, G1Projective, G2Affine, G2Prepared, Gt, LagrangeCoefficients, Scalar, +}; use rand::{CryptoRng, RngCore}; use zeroize::{Zeroize, ZeroizeOnDrop}; @@ -94,7 +96,7 @@ impl TransportSecretKey { dpk: &DerivedPublicKey, did: &[u8], ) -> Option { - let msg = augmented_hash_to_g1(&dpk.pt, did); + let msg = G1Affine::augmented_hash(&dpk.pt, did); let k = G1Affine::from(G1Projective::from(&ek.c3) - &ek.c1 * self.secret()); @@ -206,6 +208,11 @@ impl DerivedPublicKey { self.pt.serialize() } + /// Return the derived point in G2 + pub fn point(&self) -> &G2Affine { + &self.pt + } + /// Deserialize a previously serialized derived public key pub fn deserialize(bytes: &[u8]) -> Result { let pt = G2Affine::deserialize(&bytes) @@ -214,16 +221,6 @@ impl DerivedPublicKey { } } -/// See draft-irtf-cfrg-bls-signature-01 §4.2.2 for details on BLS augmented signatures -fn augmented_hash_to_g1(pk: &G2Affine, data: &[u8]) -> G1Affine { - let domain_sep = b"BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_AUG_"; - - let mut signature_input = vec![]; - signature_input.extend_from_slice(&pk.serialize()); - signature_input.extend_from_slice(data); - G1Affine::hash(domain_sep, &signature_input) -} - /// Check the validity of an encrypted key or encrypted key share /// /// The only difference in handling is when checking a share, the @@ -320,7 +317,8 @@ impl EncryptedKey { } /// Combines all the given shares into an encrypted key. - /// The returned key is guaranteed to be valid. + /// + /// If the result is Ok(), the returned key is guaranteed to be valid. /// Returns the combined key, if it is valid. /// Does not take the nodes individual public keys as input. pub fn combine_all( @@ -332,10 +330,11 @@ impl EncryptedKey { did: &[u8], ) -> Result { let c = Self::combine_unchecked(nodes, reconstruction_threshold)?; - if !c.is_valid(master_pk, derivation_path, did, tpk) { - return Err(EncryptedKeyCombinationError::InvalidShares); + if c.is_valid(master_pk, derivation_path, did, tpk) { + Ok(c) + } else { + Err(EncryptedKeyCombinationError::InvalidShares) } - Ok(c) } /// Filters the valid shares from the given ones, and combines them into a valid key, if possible. @@ -356,26 +355,35 @@ impl EncryptedKey { return Err(EncryptedKeyCombinationError::InsufficientShares); } - let valid_shares: Vec<_> = nodes - .iter() - .filter(|(_node_index, node_pk, node_eks)| { - node_eks.is_valid(master_pk, node_pk, derivation_path, did, tpk) - }) - .take(reconstruction_threshold) - .cloned() // TODO: can we avoid cloning somehow? - .map(|(node_index, _node_pk, node_eks)| (node_index, node_eks)) - .collect(); + // Take the first reconstruction_threshold shares which pass validity check + let mut valid_shares = Vec::with_capacity(reconstruction_threshold); + + for (node_index, node_pk, node_eks) in nodes.iter() { + if node_eks.is_valid(master_pk, node_pk, derivation_path, did, tpk) { + valid_shares.push((*node_index, node_eks.clone())); + + if valid_shares.len() >= reconstruction_threshold { + break; + } + } + } if valid_shares.len() < reconstruction_threshold { return Err(EncryptedKeyCombinationError::InsufficientValidKeyShares); } - // TODO: If combining sufficient valid shares does not guarantee a valid key, we - // have to use combine_all (which includes the validity check) instead of - // combine_unchecked here (or add an explicit validity check here). let c = Self::combine_unchecked(&valid_shares, reconstruction_threshold)?; - Ok(c) + // If sufficient shares are available, and all were valid (which we already checked) + // then the resulting signature should always be valid as well. + // + // This check is mostly to catch the case where the reconstruction_threshold was + // somehow incorrect. + if c.is_valid(master_pk, derivation_path, did, tpk) { + Ok(c) + } else { + Err(EncryptedKeyCombinationError::InvalidShares) + } } /// Check if this encrypted key is valid with respect to the provided derivation path @@ -387,7 +395,7 @@ impl EncryptedKey { tpk: &TransportPublicKey, ) -> bool { let dpk = DerivedPublicKey::compute_derived_key(master_pk, derivation_path); - let msg = augmented_hash_to_g1(&dpk.pt, did); + let msg = G1Affine::augmented_hash(&dpk.pt, did); check_validity(&self.c1, &self.c2, &self.c3, tpk, &dpk.pt, &msg) } @@ -460,7 +468,7 @@ impl EncryptedKeyShare { let r = Scalar::random(rng); - let msg = augmented_hash_to_g1(&dpk, did); + let msg = G1Affine::augmented_hash(&dpk, did); let c1 = G1Affine::from(G1Affine::generator() * &r); let c2 = G2Affine::from(G2Affine::generator() * &r); @@ -481,7 +489,7 @@ impl EncryptedKeyShare { let dpki = DerivedPublicKey::compute_derived_key(master_pki, derivation_path); let dpk = DerivedPublicKey::compute_derived_key(master_pk, derivation_path); - let msg = augmented_hash_to_g1(&dpk.pt, did); + let msg = G1Affine::augmented_hash(&dpk.pt, did); check_validity(&self.c1, &self.c2, &self.c3, tpk, &dpki.pt, &msg) } diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs index a387224cafb..4a0ff3523df 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs @@ -1,60 +1,344 @@ -use ic_crypto_internal_bls12_381_type::Polynomial; +use ic_crypto_internal_bls12_381_type::{ + verify_bls_signature, G1Affine, G2Affine, Polynomial, Scalar, +}; use ic_crypto_internal_bls12_381_vetkd::*; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; +use rand::{prelude::SliceRandom, CryptoRng, Rng, RngCore, SeedableRng}; #[test] -fn should_encrypted_key_share_be_functional() { - let derivation_path = DerivationPath::new(b"canister-id", &[b"1", b"2"]); - let did = b"message"; +fn transport_key_gen_is_stable() { + let mut rng = rand_chacha::ChaCha20Rng::seed_from_u64(0); + let tsk = TransportSecretKey::generate(&mut rng); - let rng = &mut reproducible_rng(); + assert_eq!( + hex::encode(tsk.serialize()), + "32f7f581d6de3c06a822fd6e7e8265fbc00f8401696a5bdc34f5a6d2ff3f922f" + ); - let nodes = 31; - let threshold = 11; + assert_eq!(hex::encode(tsk.public_key().serialize()), + "ac9524f219f1f958f261c87577e49dbbf2182c4060d51f84b8a0efac33c3c7ba9cd0bc9f41edf434d6c8a8fe20077e50"); +} + +fn random_derivation_path(rng: &mut R) -> DerivationPath { + let canister_id = rng.gen::<[u8; 32]>(); + + let num_extra = rng.gen::() % 16; + + let extra_paths = { + let mut ep = vec![]; + for _ in 0..num_extra { + let path_len = rng.gen::() % 24; + + let mut path = vec![0u8; path_len]; + rng.fill_bytes(&mut path); + ep.push(path); + } + ep + }; + + DerivationPath::new(&canister_id, &extra_paths) +} - let poly = Polynomial::random(threshold + 1, rng); +fn shake256(bytes: &[u8]) -> [u8; 32] { + let mut output = [0u8; 32]; + let mut shake = ic_sha3::Shake256::new(); + shake.update(bytes); + let mut xof_reader = shake.finalize_xof(); + xof_reader.read(&mut output); + output +} + +#[test] +fn encrypted_key_share_creation_is_stable() { + let mut rng = rand_chacha::ChaCha20Rng::seed_from_u64(1); + + let tpk = TransportSecretKey::generate(&mut rng).public_key(); + + const NODES: usize = 13; + let threshold = 3; - let tsk = TransportSecretKey::generate(rng); - let tpk = tsk.public_key(); - //let (tpk, tsk) = transport_keygen(rng); + let poly = Polynomial::random(threshold, &mut rng); let master_sk = poly.coeff(0); let master_pk = G2Affine::from(G2Affine::generator() * master_sk); - let dpk = DerivedPublicKey::compute_derived_key(&master_pk, &derivation_path); + let derivation_path = random_derivation_path(&mut rng); + let did = rng.gen::<[u8; 28]>(); - let mut node_info = Vec::with_capacity(nodes); + const EXPECTED_EKS_HASH: [&str; NODES] = [ + "cb3075ecd2c5cd946dcfaf8486031cf7bbca656092aada97644307c598af5073", + "fbfaa432bd0fc9c60b456742368034d35d5fefceae1cdd027c27147ecc7f012c", + "7d6bd8ef201443fe5b570b62d244fc8192819f53783a1ae81e3fe747a793decd", + "12f894d3ddce41e5fa0cb1cd05e77d50ed214248cba809a9fe6fccf5515f5c13", + "d187e7bf4defab92ce9c82c692a9b3c3de65994cd4bf422438cbc515a8a48501", + "9aa47ad2dcf06a6a308152d935698684799714c772dfaf2b6654e6642a11937b", + "d96c4f897ba7e7ac657d363171b324adbad2faf13ef395bd0efa28a60273583d", + "fc3b2d0eae4c7d96212058d815d48701267a673c20197a08f28762301043405d", + "85ba21972c3a7717922d6bc734217d5dae12be7df42ffb93604e0ae7228ac9ed", + "3f7b6c16d35118519c92d31e074633f00fe30b6e4b522cda47d81088dc4011bb", + "658383824de7771db64a87b884ffbefe92567baeb851229785d9f3717c91bdda", + "e7a69b641278a5a8084938d1c03de2850a6639a882713a190fe0417f678e118e", + "d59283b0a48181d93c6e1a22e5ff9130df68a01bcadf96a25d37a2a092cb1a1a", + ]; - for node in 0..nodes { + for node in 0..NODES { let node_sk = poly.evaluate_at(&Scalar::from_node_index(node as u32)); - let node_pk = G2Affine::from(G2Affine::generator() * &node_sk); + let eks = + EncryptedKeyShare::create(&mut rng, &master_pk, &node_sk, &tpk, &derivation_path, &did); + assert_eq!( + hex::encode(shake256(&eks.serialize())), + EXPECTED_EKS_HASH[node] + ); + } +} + +struct VetkdTestProtocolSetup { + nodes: usize, + threshold: usize, + transport_sk: TransportSecretKey, + transport_pk: TransportPublicKey, + master_pk: G2Affine, + node_key_material: Vec<(G2Affine, Scalar)>, +} + +impl VetkdTestProtocolSetup { + fn new(rng: &mut R, nodes: usize, threshold: usize) -> Self { + let transport_sk = TransportSecretKey::generate(rng); + let transport_pk = transport_sk.public_key(); - let eks = EncryptedKeyShare::create(rng, &master_pk, &node_sk, &tpk, &derivation_path, did); + // In production this is done using a DKG... + let poly = Polynomial::random(threshold, rng); - assert!(eks.is_valid(&master_pk, &node_pk, &derivation_path, did, &tpk)); + let master_sk = poly.coeff(0); + let master_pk = G2Affine::from(G2Affine::generator() * master_sk); - // check that EKS serialization round trips: - let eks_bytes = eks.serialize(); - let eks2 = EncryptedKeyShare::deserialize(eks_bytes).unwrap(); - assert_eq!(eks, eks2); + let mut node_key_material = Vec::with_capacity(nodes); - node_info.push((node as u32, node_pk, eks)); + for node in 0..nodes { + let node_sk = poly.evaluate_at(&Scalar::from_node_index(node as u32)); + let node_pk = G2Affine::from(G2Affine::generator() * &node_sk); + node_key_material.push((node_pk, node_sk)); + } + + Self { + nodes, + threshold, + transport_sk, + transport_pk, + master_pk, + node_key_material, + } + } +} + +struct VetkdTestProtocolExecution<'a> { + setup: &'a VetkdTestProtocolSetup, + did: Vec, + derivation_path: DerivationPath, + derived_pk: DerivedPublicKey, +} + +impl<'a> VetkdTestProtocolExecution<'a> { + fn new( + rng: &mut R, + setup: &'a VetkdTestProtocolSetup, + ) -> VetkdTestProtocolExecution<'a> { + let did = rng.gen::<[u8; 32]>().to_vec(); + let derivation_path = random_derivation_path(rng); + + let derived_pk = DerivedPublicKey::compute_derived_key(&setup.master_pk, &derivation_path); + + Self { + setup, + did, + derivation_path, + derived_pk, + } } - let ek = EncryptedKey::combine( - &node_info, - threshold, - &master_pk, - &tpk, - &derivation_path, - did, - ) - .unwrap(); + fn create_encrypted_key_shares( + &self, + rng: &mut R, + did: Option<&[u8]>, + ) -> Vec<(u32, G2Affine, EncryptedKeyShare)> { + let mut node_info = Vec::with_capacity(self.setup.nodes); + + let did = did.unwrap_or(&self.did); + + for (node_idx, (node_pk, node_sk)) in self.setup.node_key_material.iter().enumerate() { + let eks = EncryptedKeyShare::create( + rng, + &self.setup.master_pk, + &node_sk, + &self.setup.transport_pk, + &self.derivation_path, + did, + ); - let _k = tsk.decrypt(&ek, &dpk, did).unwrap(); + assert!(eks.is_valid( + &self.setup.master_pk, + &node_pk, + &self.derivation_path, + did, + &self.setup.transport_pk + )); + + // check that EKS serialization round trips: + let eks_bytes = eks.serialize(); + let eks2 = EncryptedKeyShare::deserialize(eks_bytes).unwrap(); + assert_eq!(eks, eks2); + + node_info.push((node_idx as u32, node_pk.clone(), eks.clone())); + } + + node_info + } + + fn combine_all( + &self, + node_eks: &[(u32, EncryptedKeyShare)], + ) -> Result { + EncryptedKey::combine_all( + node_eks, + self.setup.threshold, + &self.setup.master_pk, + &self.setup.transport_pk, + &self.derivation_path, + &self.did, + ) + } + + fn combine_valid( + &self, + node_info: &[(u32, G2Affine, EncryptedKeyShare)], + ) -> Result { + EncryptedKey::combine_valid_shares( + &node_info, + self.setup.threshold, + &self.setup.master_pk, + &self.setup.transport_pk, + &self.derivation_path, + &self.did, + ) + } +} - let derived_key = tsk - .decrypt_and_hash(&ek, &dpk, did, 32, b"aes-256-gcm-siv") - .unwrap(); - assert_eq!(derived_key.len(), 32); +fn random_subset(rng: &mut R, items: &Vec, include: usize) -> Vec { + assert!(include <= items.len()); + + if items.len() == include { + return items.clone(); + } + + let mut result = Vec::with_capacity(include); + + let mut taken = vec![false; items.len()]; + + while result.len() != include { + loop { + let idx = rng.gen::() % items.len(); + if taken[idx] == false { + result.push(items[idx].clone()); + taken[idx] = true; + break; + } + } + } + + assert_eq!(result.len(), include); + result +} + +#[test] +fn test_protocol_execution() { + let rng = &mut reproducible_rng(); + + let nodes = 31; + let threshold = 11; + + let setup = VetkdTestProtocolSetup::new(rng, nodes, threshold); + let proto = VetkdTestProtocolExecution::new(rng, &setup); + + let node_info = proto.create_encrypted_key_shares(rng, None); + + let node_eks = node_info + .iter() + .map(|(idx, _pk, eks)| (*idx, eks.clone())) + .collect::>(); + + let mut keys_recovered = vec![]; + + // Check that recovery works with sufficient shares, and fails without sufficient shares + for rec_threshold in 1..nodes { + if let Ok(ek) = proto.combine_all(&random_subset(rng, &node_eks, rec_threshold)) { + assert!( + rec_threshold >= threshold, + "Recovery only works with sufficient quorum" + ); + + let k = setup + .transport_sk + .decrypt(&ek, &proto.derived_pk, &proto.did) + .expect("Decryption failed"); + + keys_recovered.push(k); + } else { + assert!( + rec_threshold < threshold, + "Recovery fails with insufficient quorum" + ); + } + } + + // Now check that each recovered vetkey is the same: + let vetkey = keys_recovered[0].clone(); + + for k in &keys_recovered { + assert_eq!(*k, vetkey); + } + + // Check that the vetkey output is a valid BLS signature + let msg = G1Affine::augmented_hash(&proto.derived_pk.point(), &proto.did); + + assert!(verify_bls_signature( + &vetkey, + proto.derived_pk.point(), + &msg + )); + + // Check that if we introduce incorrect shares then combine_all will fail + + let other_did = rng.gen::<[u8; 24]>(); + let node_info_wrong_did = proto.create_encrypted_key_shares(rng, Some(&other_did)); + + let node_eks_wrong_did = node_info_wrong_did + .iter() + .map(|(idx, _pk, eks)| (*idx, eks.clone())) + .collect::>(); + + // With combine_all even if we provide sufficiently many valid shares + // if any one share is invalid then combination will fail + for rec_threshold in 2..nodes { + let mut shares = random_subset(rng, &node_eks, rec_threshold - 1); + shares.append(&mut random_subset(rng, &node_eks_wrong_did, 1)); + shares.shuffle(rng); + assert!(proto.combine_all(&shares).is_err()); + } + + // With combine_valid_shares OTOH we detect and reject the shares + + for rec_threshold in threshold..nodes { + let mut shares = random_subset(rng, &node_info, rec_threshold); + shares.append(&mut random_subset(rng, &node_info_wrong_did, 4)); + shares.shuffle(rng); + + let combined = proto.combine_valid(&shares).expect("Combination worked"); + + let k = setup + .transport_sk + .decrypt(&combined, &proto.derived_pk, &proto.did) + .expect("Decryption failed"); + + assert_eq!(k, vetkey); + } } From 46d205afe39851d6359a6958bdc91628e1de651c Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 20 Dec 2024 15:25:53 -0500 Subject: [PATCH 03/22] Fix export of types --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs index 556dd19174e..a78b026cc26 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs @@ -5,9 +5,11 @@ #![forbid(unsafe_code)] #![forbid(missing_docs)] +pub use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine}; use ic_crypto_internal_bls12_381_type::{ - G1Affine, G1Projective, G2Affine, G2Prepared, Gt, LagrangeCoefficients, Scalar, + G1Projective, G2Prepared, Gt, LagrangeCoefficients, Scalar, }; + use rand::{CryptoRng, RngCore}; use zeroize::{Zeroize, ZeroizeOnDrop}; From 7f8758ddb5bfaf19a4af0dea95aa1eda736534ae Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 20 Dec 2024 15:49:39 -0500 Subject: [PATCH 04/22] Fix benchmark and clippys --- .../bls12_381/vetkd/benches/vetkd.rs | 32 +++++++++++-------- .../crypto_lib/bls12_381/vetkd/tests/tests.rs | 23 ++++++------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs index 795cb43f261..d771cdf93e0 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs @@ -1,4 +1,5 @@ use criterion::*; +use ic_crypto_internal_bls12_381_type::{Polynomial, Scalar}; use ic_crypto_internal_bls12_381_vetkd::*; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; use rand::Rng; @@ -124,22 +125,25 @@ fn vetkd_bench(c: &mut Criterion) { node_info.push((node as u32, node_pk, eks)); } - group.bench_function(format!("EncryptedKey::combine (n={})", nodes), |b| { - b.iter(|| { - EncryptedKey::combine( - &node_info, - threshold, - &master_pk, - &tpk, - &derivation_path, - &did, - ) - .unwrap() - }) - }); + group.bench_function( + format!("EncryptedKey::combine_valid_shares (n={})", nodes), + |b| { + b.iter(|| { + EncryptedKey::combine_valid_shares( + &node_info, + threshold, + &master_pk, + &tpk, + &derivation_path, + &did, + ) + .unwrap() + }) + }, + ); if threshold == 9 { - let ek = EncryptedKey::combine( + let ek = EncryptedKey::combine_valid_shares( &node_info, threshold, &master_pk, diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs index 4a0ff3523df..cd12790fb47 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs @@ -81,14 +81,11 @@ fn encrypted_key_share_creation_is_stable() { "d59283b0a48181d93c6e1a22e5ff9130df68a01bcadf96a25d37a2a092cb1a1a", ]; - for node in 0..NODES { - let node_sk = poly.evaluate_at(&Scalar::from_node_index(node as u32)); + for (node_idx, expected_eks_hash) in EXPECTED_EKS_HASH.iter().enumerate() { + let node_sk = poly.evaluate_at(&Scalar::from_node_index(node_idx as u32)); let eks = EncryptedKeyShare::create(&mut rng, &master_pk, &node_sk, &tpk, &derivation_path, &did); - assert_eq!( - hex::encode(shake256(&eks.serialize())), - EXPECTED_EKS_HASH[node] - ); + assert_eq!(hex::encode(shake256(&eks.serialize())), *expected_eks_hash,); } } @@ -169,7 +166,7 @@ impl<'a> VetkdTestProtocolExecution<'a> { let eks = EncryptedKeyShare::create( rng, &self.setup.master_pk, - &node_sk, + node_sk, &self.setup.transport_pk, &self.derivation_path, did, @@ -177,7 +174,7 @@ impl<'a> VetkdTestProtocolExecution<'a> { assert!(eks.is_valid( &self.setup.master_pk, - &node_pk, + node_pk, &self.derivation_path, did, &self.setup.transport_pk @@ -213,7 +210,7 @@ impl<'a> VetkdTestProtocolExecution<'a> { node_info: &[(u32, G2Affine, EncryptedKeyShare)], ) -> Result { EncryptedKey::combine_valid_shares( - &node_info, + node_info, self.setup.threshold, &self.setup.master_pk, &self.setup.transport_pk, @@ -223,11 +220,11 @@ impl<'a> VetkdTestProtocolExecution<'a> { } } -fn random_subset(rng: &mut R, items: &Vec, include: usize) -> Vec { +fn random_subset(rng: &mut R, items: &[T], include: usize) -> Vec { assert!(include <= items.len()); if items.len() == include { - return items.clone(); + return items.to_owned(); } let mut result = Vec::with_capacity(include); @@ -237,7 +234,7 @@ fn random_subset(rng: &mut R, items: &Vec, include: u while result.len() != include { loop { let idx = rng.gen::() % items.len(); - if taken[idx] == false { + if !taken[idx] { result.push(items[idx].clone()); taken[idx] = true; break; @@ -298,7 +295,7 @@ fn test_protocol_execution() { } // Check that the vetkey output is a valid BLS signature - let msg = G1Affine::augmented_hash(&proto.derived_pk.point(), &proto.did); + let msg = G1Affine::augmented_hash(proto.derived_pk.point(), &proto.did); assert!(verify_bls_signature( &vetkey, From 41736a99f13786f838ff11f95a811e0e11fd8fdd Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 20 Dec 2024 17:05:41 -0500 Subject: [PATCH 05/22] Fix exports for CSP --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs index a78b026cc26..dc0ef745f35 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs @@ -5,9 +5,9 @@ #![forbid(unsafe_code)] #![forbid(missing_docs)] -pub use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine}; +pub use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine, Scalar, PairingInvalidPoint}; use ic_crypto_internal_bls12_381_type::{ - G1Projective, G2Prepared, Gt, LagrangeCoefficients, Scalar, + G1Projective, G2Prepared, Gt, LagrangeCoefficients, }; use rand::{CryptoRng, RngCore}; From c5b9a87010f779d793398e795bef1a0172c67e95 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 20 Dec 2024 17:10:14 -0500 Subject: [PATCH 06/22] fmt --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs index dc0ef745f35..3e7ece00435 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs @@ -5,10 +5,8 @@ #![forbid(unsafe_code)] #![forbid(missing_docs)] -pub use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine, Scalar, PairingInvalidPoint}; -use ic_crypto_internal_bls12_381_type::{ - G1Projective, G2Prepared, Gt, LagrangeCoefficients, -}; +pub use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine, PairingInvalidPoint, Scalar}; +use ic_crypto_internal_bls12_381_type::{G1Projective, G2Prepared, Gt, LagrangeCoefficients}; use rand::{CryptoRng, RngCore}; use zeroize::{Zeroize, ZeroizeOnDrop}; From 05714940af6b01504b8a99aa3ad826cbd0ec8879 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 20 Dec 2024 17:11:23 -0500 Subject: [PATCH 07/22] Appease bazel --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel index 41fd0f1a7cd..d68a3c3092d 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel @@ -15,6 +15,7 @@ MACRO_DEPENDENCIES = [] DEV_DEPENDENCIES = [ # Keep sorted. + "//rs/crypto/internal/crypto_lib/bls12_381/type", "//rs/crypto/test_utils/reproducible_rng", "@crate_index//:hex", "@crate_index//:rand_chacha", From 51698d1bf17b73c915784828798bcdc9a51a87a7 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Mon, 6 Jan 2025 14:47:04 -0500 Subject: [PATCH 08/22] Alas --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel index d68a3c3092d..41fd0f1a7cd 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel @@ -15,7 +15,6 @@ MACRO_DEPENDENCIES = [] DEV_DEPENDENCIES = [ # Keep sorted. - "//rs/crypto/internal/crypto_lib/bls12_381/type", "//rs/crypto/test_utils/reproducible_rng", "@crate_index//:hex", "@crate_index//:rand_chacha", From 45a6066f103bdbd3e0558f52d4cb869ac5aba516 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Mon, 6 Jan 2025 15:01:28 -0500 Subject: [PATCH 09/22] bench --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel index 41fd0f1a7cd..1a44978e51a 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel @@ -48,6 +48,7 @@ rust_bench( deps = [ # Keep sorted. ":vetkd", + "//rs/crypto/internal/crypto_lib/bls12_381/type", "//rs/crypto/test_utils/reproducible_rng", "@crate_index//:criterion", "@crate_index//:rand", From 43140b0cca4af500d9561da9704f3f5d60342a98 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Tue, 14 Jan 2025 15:53:25 -0500 Subject: [PATCH 10/22] Address review comments --- Cargo.lock | 1 - .../crypto_lib/bls12_381/type/src/lib.rs | 2 +- .../crypto_lib/bls12_381/type/tests/tests.rs | 40 ++++ .../crypto_lib/bls12_381/vetkd/BUILD.bazel | 1 - .../crypto_lib/bls12_381/vetkd/Cargo.toml | 1 - .../crypto_lib/bls12_381/vetkd/src/lib.rs | 140 +++---------- .../crypto_lib/bls12_381/vetkd/src/ro.rs | 6 - .../crypto_lib/bls12_381/vetkd/tests/tests.rs | 188 +++++++++++++++--- 8 files changed, 231 insertions(+), 148 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4228a75536c..5ee12a4a4c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7240,7 +7240,6 @@ dependencies = [ "ic-sha3 1.0.0", "rand 0.8.5", "rand_chacha 0.3.1", - "zeroize", ] [[package]] diff --git a/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs index 99e49f17f88..ee788b95a54 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/type/src/lib.rs @@ -1909,7 +1909,7 @@ impl_debug_using_serialize_for!(G1Affine); impl_debug_using_serialize_for!(G1Projective); impl G1Affine { - /// See draft-irtf-cfrg-bls-signature-01 §4.2.2 for details on BLS augmented signatures + /// See draft-irtf-cfrg-bls-signature-05 §4.2.2 for details on BLS augmented signatures pub fn augmented_hash(pk: &G2Affine, data: &[u8]) -> Self { let domain_sep = b"BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_AUG_"; diff --git a/rs/crypto/internal/crypto_lib/bls12_381/type/tests/tests.rs b/rs/crypto/internal/crypto_lib/bls12_381/type/tests/tests.rs index 78f9036f374..4cc6aceafb8 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/type/tests/tests.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/type/tests/tests.rs @@ -883,6 +883,46 @@ fn test_scalar_muln() { } } +#[test] +fn test_g1_augmented_hash_test_vectors() { + /* + * No source for test vectors for the G1 augmented hash could be located. These + * values were generated by zkcrypto/bls12_381 + */ + let test_vectors = [ + ("c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "ce900db33cb4f6bc3db0b3ce0c462e78", "816c73d8098a07f0584d2e5a9a13abd98fe0536b86e921bebaa953577a03300453764e88b6383e442e10a7fed0de37f0"), + + ("864ed23497b7c6bf4b95f981f6a8ebc5de6e303ad90e1dad71a1a8b7676912bfeaa7a3b61eeabf2a5d728ec298c268fb18ab9f32c0ee43d99bc4901eb8d3815cd1a8cef6591931dd706f212691bba1ad3490e0fa2e7c593d978e920566486066", "a7eed036705827efec713b65533b9b10", "9900f2cd31147dda4400ba954849eba5785730d62010b38a0ebaabf93281f49d8b24eb839493b9563ed8a422e2de3337"), + + ("8ac83f419f09283c558cb9630ceeb887de1c646cb249ae9e5a5777699127d9044b37a8a34de665b98aa6ff3954cefc270f15e4e7fc38f2c317b726326c01a7fd679c6bcfa14bf0575b971e14705ef3d874281360586d86fb5ced64a568eb7c6c", "8be4fa4a80dcaa0621cfe00face73486", "ade977d871dcb7ff69ec2a154ff9b0aff750ea3872057bf0721ae0c3fd7a086e5086793dd72256d993acc630c5dba61d"), + + ("a43ac20c81f7c3a88a49913bc9dc11c736088af78715a438f1e72d6a006f2a90761c0b5f0cb9ae1e5828d6e7abde5b7a058f75435777ea0bdf0d52e9c03b11f6a0683351cd9b8d28e8657b3fb2ac1f24087bada88b7c5168a0a1468d52cf5842", "6bdf244fded974ef5833df52b70db200", "a19250e0e4fe6b1ef839c3f3edab385f56e9d991566f3bc19a250df09b087b695e39d59c05dc5a58921afe17e57823cb"), + + ("8d73306394bbc5646a195507ac6424ea852a8f8e5928ab8dd108d6d9fdecce9e0b22204209fd16ccefde1d6f1b73ca1718950972a2b74f2a0aac0ee4d9d72dd2102726ba047405d0377dc8f714e563f99e37a9ee3b31263b002793fc206e5771", "b1f88617c75b527152977ac506b8d46f", "a9b3056b47b56613445def1729ea323ec883dd856ac36f2eefb56ad7c40353c19f9f23f0bafaa5a65322c4cc7437e5f2"), + + ("980cf164639279acee275350c22686ab48e2eb6e79eca03ca409b4c160e72530a42eef3668bc8d997b7b103515d902610ff7bf3a1793c3445b24006c863fbdf1713652fbab371a1813037a1c2ab35804ec1973aafd6ee51b4602143e10685bb0", "2d5ba3c198d8728c64ccd660353561fe", "b23869d704e064f8a60870b3aaa2d3a81a2e2f965504aa9d64f60a945e4ed1d872ea9736f202a5029926a80450f9872d"), + + ("832e3b180d3ca7f787327b5d615804b7e883f911ddda48da42e58e45d779f0788d9cd563f4d4b65ed2573b5a50641efe1873bca56f0836c156b3e82787a18881b9f8c7aa130e67b28c2762bbc1c8d19506b91150ecdedbccce8e07da6e74c014", "6a86af2c8226707d25bfa649cb9223ec", "8e775d6c996f1c8f9902d8f19193694f387c6e453eea4ade39f541c484c15f3a8e47f0d2fbd98f1f590aaa04e3dcd41a"), + + ("95f90c37e583863c6e7d8e17f64dda7ecf56b3568c5558362ebdb39b6b9304fabc4ef91efc92e0932abc9d87b44a3fdd08d6f23cc0f547ef35cdf975c5eb2b37c00095b5e818587cd08ae9ee4fe76b6de5fc07a79c046ef68d6fbde8db3631b6", "042259be774dc136482d40f3587103f8", "99c74b44d6e1092173e62358faa7d388cc15d5bcbffa17511d30fb2699b4d2dc58d33ed346fa4fec317da4d51f7cd8d8"), + + ("85fcc79dbcd6ad60b2ceb7d2759fd3a165c1a4cfc9a3813e32eb9a4e13148d3b74cfc18331eb82a0ec82f5947dec5f2d16e517fefedc0123e36f93ca758ff9b23810a692e80ecc40ca97edb76d210fe037339c4fb2e4151ccb9721f91cd124d3", "10d4bd96176d6368c45349277369909c", "a76b58c80a52efcd1c91ece11b13b60b21d533713f689721b4ca6bedb214a124b3619ee41ad73c255d72590e25d44631"), + + ("999986ca521874abf02735b50647f9890cf194214cbaaa6f9f6fb6ac924e397699227c5ac15ce6574d7b989ebaa17fb4103d7403ada1e927fadc413c4e0aa4286e29572261628c708e90c89e8825679ef6c977e5290e83c9b96af7251a4e1c75", "6936ad26ab5ca7c291287827e3388e55", "a420355356cb7d91af6be049123e93212340d601f068ab99d62f568a63255e2f6991d6b4f5a2cafc92669a925ce17783"), + + ("8a02185f6a780cae9264a3cb6e811122faa01c50d9cb2b359cc7bbb8de2527aed3f8048f289de6d4d2d8d7eeafb8d08804be0d1a26e8f4a573d8fc87ebc8475beefbfa4f0221493f80b51e75bcd3c6403a2e872601cc5c33ce1c9938d2264de5", "33f280df9b7638f9b4785524961c0af6", "8abb3a215edabeee131d6ab3e2f6666b39636d177b103e9aea73c33d672cd37959aefe9c88a3fc1aaa179bc63f5ba503") + ]; + + for (g2, data, g1) in &test_vectors { + let g2 = G2Affine::deserialize(&hex::decode(g2).expect("Invalid hex")).expect("Invalid G2"); + let data = hex::decode(data).expect("Invalid hex"); + + let computed_g1 = G1Affine::augmented_hash(&g2, &data); + + assert_eq!(hex::encode(computed_g1.serialize()), *g1); + } +} + #[test] fn test_verify_bls_signature() { let rng = &mut reproducible_rng(); diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel index 1a44978e51a..039aea2f17d 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/BUILD.bazel @@ -8,7 +8,6 @@ DEPENDENCIES = [ "//packages/ic-sha3", "//rs/crypto/internal/crypto_lib/bls12_381/type", "@crate_index//:rand", - "@crate_index//:zeroize", ] MACRO_DEPENDENCIES = [] diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml index 6d31482c955..091fe601ccb 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/Cargo.toml @@ -10,7 +10,6 @@ documentation.workspace = true ic-crypto-internal-bls12-381-type = { path = "../type" } ic-sha3 = { path = "../../../../../../packages/ic-sha3" } rand = { workspace = true } -zeroize = { workspace = true } [dev-dependencies] criterion = { workspace = true } diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs index 3e7ece00435..3e2da411648 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs @@ -6,10 +6,9 @@ #![forbid(missing_docs)] pub use ic_crypto_internal_bls12_381_type::{G1Affine, G2Affine, PairingInvalidPoint, Scalar}; -use ic_crypto_internal_bls12_381_type::{G1Projective, G2Prepared, Gt, LagrangeCoefficients}; +use ic_crypto_internal_bls12_381_type::{G2Prepared, Gt, LagrangeCoefficients}; use rand::{CryptoRng, RngCore}; -use zeroize::{Zeroize, ZeroizeOnDrop}; mod ro; @@ -42,107 +41,7 @@ impl DerivationPath { } } -#[derive(Copy, Clone, Debug)] -/// Deserialization of a transport secret key failed -pub enum TransportSecretKeyDeserializationError { - /// Error indicating the key was not a valid scalar - InvalidSecretKey, -} - -#[derive(Clone, Zeroize, ZeroizeOnDrop)] -/// Secret key of the transport key pair -pub struct TransportSecretKey { - secret_key: Scalar, -} - -impl TransportSecretKey { - /// The length of the serialized encoding of this type - pub const BYTES: usize = Scalar::BYTES; - - /// Create a new transport secret key - pub fn generate(rng: &mut R) -> Self { - let secret_key = Scalar::random(rng); - Self { secret_key } - } - - /// Serialize the transport secret key to a bytestring - pub fn serialize(&self) -> [u8; Self::BYTES] { - self.secret_key.serialize() - } - - /// Deserialize a previously serialized transport secret key - pub fn deserialize(bytes: &[u8]) -> Result { - let secret_key = Scalar::deserialize(&bytes) - .map_err(|_| TransportSecretKeyDeserializationError::InvalidSecretKey)?; - Ok(Self { secret_key }) - } - - /// Return the public key associated with this secret key - pub fn public_key(&self) -> TransportPublicKey { - let public_key = G1Affine::generator() * &self.secret_key; - TransportPublicKey::new(public_key.to_affine()) - } - - fn secret(&self) -> &Scalar { - &self.secret_key - } - - /// Decrypt an encrypted key - /// - /// Returns None if decryption failed - pub fn decrypt( - &self, - ek: &EncryptedKey, - dpk: &DerivedPublicKey, - did: &[u8], - ) -> Option { - let msg = G1Affine::augmented_hash(&dpk.pt, did); - - let k = G1Affine::from(G1Projective::from(&ek.c3) - &ek.c1 * self.secret()); - - let dpk_prep = G2Prepared::from(&dpk.pt); - let k_is_valid_sig = - Gt::multipairing(&[(&k, G2Prepared::neg_generator()), (&msg, &dpk_prep)]).is_identity(); - - if k_is_valid_sig { - Some(k) - } else { - None - } - } - - /// Decrypt an encrypted key, and hash it to a symmetric key - /// - /// Returns None if decryption failed - /// - /// The output length can be arbitrary and is specified by the caller - /// - /// The `symmetric_key_associated_data` field should include information about - /// the protocol and cipher that this key will be used for - pub fn decrypt_and_hash( - &self, - ek: &EncryptedKey, - dpk: &DerivedPublicKey, - did: &[u8], - symmetric_key_bytes: usize, - symmetric_key_associated_data: &[u8], - ) -> Option> { - match self.decrypt(ek, dpk, did) { - None => None, - Some(k) => { - let mut ro = ro::RandomOracle::new(&format!( - "ic-crypto-vetkd-bls12-381-create-secret-key-{}-bytes", - symmetric_key_bytes - )); - ro.update_bin(symmetric_key_associated_data); - ro.update_bin(&k.serialize()); - Some(ro.finalize_to_vec(symmetric_key_bytes)) - } - } - } -} - -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] /// Error indicating that deserializing a transport public key failed pub enum TransportPublicKeyDeserializationError { /// Error indicating the public key was not a valid elliptic curve point @@ -159,10 +58,6 @@ impl TransportPublicKey { /// The length of the serialized encoding of this type pub const BYTES: usize = G1Affine::BYTES; - fn new(public_key: G1Affine) -> Self { - Self { public_key } - } - /// Serialize this public key to a bytestring pub fn serialize(&self) -> [u8; Self::BYTES] { self.public_key.serialize() @@ -180,7 +75,7 @@ impl TransportPublicKey { } } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] /// Error indicating that deserializing a derived public key failed pub enum DerivedPublicKeyDeserializationError { /// The public point was not a valid encoding @@ -258,14 +153,14 @@ fn check_validity( true } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] /// Error indicating that deserializing an encrypted key failed pub enum EncryptedKeyDeserializationError { /// Error indicating one or more of the points was invalid InvalidEncryptedKey, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] /// Error indicating that combining shares into an encrypted key failed pub enum EncryptedKeyCombinationError { /// Two shares had the same node index @@ -276,6 +171,8 @@ pub enum EncryptedKeyCombinationError { InsufficientValidKeyShares, /// Some of the key shares are invalid InvalidShares, + /// The reconstruction threshold was invalid + ReconstructionFailed, } #[derive(Clone, Eq, PartialEq, Debug)] @@ -320,7 +217,7 @@ impl EncryptedKey { /// /// If the result is Ok(), the returned key is guaranteed to be valid. /// Returns the combined key, if it is valid. - /// Does not take the nodes individual public keys as input. + /// Does not take the nodes' individual public keys as input. pub fn combine_all( nodes: &[(NodeIndex, EncryptedKeyShare)], reconstruction_threshold: usize, @@ -340,7 +237,7 @@ impl EncryptedKey { /// Filters the valid shares from the given ones, and combines them into a valid key, if possible. /// The returned key is guaranteed to be valid. /// Returns an error if not sufficient shares are given or if not sufficient valid shares are given. - /// Takes also the node's individual public keys as input, which means the individual public keys + /// Takes also the nodes' individual public keys as input, which means the individual public keys /// must be available: calculating them is comparatively expensive. Note that combine_all does not /// take the individual public keys as input. pub fn combine_valid_shares( @@ -382,7 +279,7 @@ impl EncryptedKey { if c.is_valid(master_pk, derivation_path, did, tpk) { Ok(c) } else { - Err(EncryptedKeyCombinationError::InvalidShares) + Err(EncryptedKeyCombinationError::ReconstructionFailed) } } @@ -431,6 +328,21 @@ impl EncryptedKey { output } + + /// Return the c1 element + pub fn c1(&self) -> &G1Affine { + &self.c1 + } + + /// Return the c2 element + pub fn c2(&self) -> &G2Affine { + &self.c2 + } + + /// Return the c3 element + pub fn c3(&self) -> &G1Affine { + &self.c3 + } } #[derive(Clone, Eq, PartialEq, Debug)] @@ -441,7 +353,7 @@ pub struct EncryptedKeyShare { c3: G1Affine, } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] /// Error indicating that deserialization of an encrypted key share failed pub enum EncryptedKeyShareDeserializationError { /// One or more of the share points were not valid diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/ro.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/ro.rs index 375b2904a8c..96fb4677c4e 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/ro.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/ro.rs @@ -35,12 +35,6 @@ impl RandomOracle { xof.read(output); } - pub(crate) fn finalize_to_vec(self, output_len: usize) -> Vec { - let mut output = vec![0u8; output_len]; - self.finalize(&mut output); - output - } - pub(crate) fn finalize_to_scalar(self) -> Scalar { let mut output = [0u8; 2 * Scalar::BYTES]; self.finalize(&mut output); diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs index cd12790fb47..6c8af537466 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs @@ -1,10 +1,81 @@ use ic_crypto_internal_bls12_381_type::{ - verify_bls_signature, G1Affine, G2Affine, Polynomial, Scalar, + verify_bls_signature, G1Affine, G1Projective, G2Affine, G2Prepared, Gt, Polynomial, Scalar, }; use ic_crypto_internal_bls12_381_vetkd::*; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; use rand::{prelude::SliceRandom, CryptoRng, Rng, RngCore, SeedableRng}; +#[derive(Copy, Clone, Debug)] +/// Deserialization of a transport secret key failed +pub enum TransportSecretKeyDeserializationError { + /// Error indicating the key was not a valid scalar + InvalidSecretKey, +} + +#[derive(Clone)] +/// Secret key of the transport key pair +pub struct TransportSecretKey { + secret_key: Scalar, +} + +impl TransportSecretKey { + /// The length of the serialized encoding of this type + pub const BYTES: usize = Scalar::BYTES; + + /// Create a new transport secret key + pub fn generate(rng: &mut R) -> Self { + let secret_key = Scalar::random(rng); + Self { secret_key } + } + + /// Serialize the transport secret key to a bytestring + pub fn serialize(&self) -> [u8; Self::BYTES] { + self.secret_key.serialize() + } + + /// Deserialize a previously serialized transport secret key + pub fn deserialize(bytes: &[u8]) -> Result { + let secret_key = Scalar::deserialize(&bytes) + .map_err(|_| TransportSecretKeyDeserializationError::InvalidSecretKey)?; + Ok(Self { secret_key }) + } + + /// Return the public key associated with this secret key + pub fn public_key(&self) -> TransportPublicKey { + let public_key = G1Affine::generator() * &self.secret_key; + let pk_bytes = public_key.to_affine().serialize(); + TransportPublicKey::deserialize(&pk_bytes).expect("Invalid public key") + } + + fn secret(&self) -> &Scalar { + &self.secret_key + } + + /// Decrypt an encrypted key + /// + /// Returns None if decryption failed + pub fn decrypt( + &self, + ek: &EncryptedKey, + dpk: &DerivedPublicKey, + did: &[u8], + ) -> Option { + let msg = G1Affine::augmented_hash(dpk.point(), did); + + let k = G1Affine::from(G1Projective::from(ek.c3()) - ek.c1() * self.secret()); + + let dpk_prep = G2Prepared::from(dpk.point()); + let k_is_valid_sig = + Gt::multipairing(&[(&k, G2Prepared::neg_generator()), (&msg, &dpk_prep)]).is_identity(); + + if k_is_valid_sig { + Some(k) + } else { + None + } + } +} + #[test] fn transport_key_gen_is_stable() { let mut rng = rand_chacha::ChaCha20Rng::seed_from_u64(0); @@ -221,28 +292,12 @@ impl<'a> VetkdTestProtocolExecution<'a> { } fn random_subset(rng: &mut R, items: &[T], include: usize) -> Vec { - assert!(include <= items.len()); - - if items.len() == include { - return items.to_owned(); - } - - let mut result = Vec::with_capacity(include); - - let mut taken = vec![false; items.len()]; - - while result.len() != include { - loop { - let idx = rng.gen::() % items.len(); - if !taken[idx] { - result.push(items[idx].clone()); - taken[idx] = true; - break; - } - } - } + use rand::seq::SliceRandom; + assert!(include <= items.len()); + let result: Vec<_> = items.choose_multiple(rng, include).cloned().collect(); assert_eq!(result.len(), include); + result } @@ -273,6 +328,13 @@ fn test_protocol_execution() { "Recovery only works with sufficient quorum" ); + assert!(ek.is_valid( + &setup.master_pk, + &proto.derivation_path, + &proto.did, + &setup.transport_pk + )); + let k = setup .transport_sk .decrypt(&ek, &proto.derived_pk, &proto.did) @@ -306,6 +368,7 @@ fn test_protocol_execution() { // Check that if we introduce incorrect shares then combine_all will fail let other_did = rng.gen::<[u8; 24]>(); + assert_ne!(proto.did, other_did); let node_info_wrong_did = proto.create_encrypted_key_shares(rng, Some(&other_did)); let node_eks_wrong_did = node_info_wrong_did @@ -317,12 +380,51 @@ fn test_protocol_execution() { // if any one share is invalid then combination will fail for rec_threshold in 2..nodes { let mut shares = random_subset(rng, &node_eks, rec_threshold - 1); - shares.append(&mut random_subset(rng, &node_eks_wrong_did, 1)); + + // Avoid using a duplicate index for this test + let random_unused_idx = loop { + let idx = (rng.gen::() % node_eks_wrong_did.len()) as u32; + + if !shares.iter().map(|x| x.0).any(|x| x == idx) { + break idx as usize; + } + }; + + shares.push(node_eks_wrong_did[random_unused_idx].clone()); shares.shuffle(rng); - assert!(proto.combine_all(&shares).is_err()); + + let expected_error = if rec_threshold < threshold { + EncryptedKeyCombinationError::InsufficientShares + } else { + EncryptedKeyCombinationError::InvalidShares + }; + assert_eq!(proto.combine_all(&shares), Err(expected_error)); } - // With combine_valid_shares OTOH we detect and reject the shares + // Check that duplicate node indexes are detected + for rec_threshold in 2..nodes { + let mut shares = random_subset(rng, &node_eks, rec_threshold - 1); + + let random_duplicate_idx = loop { + let idx = (rng.gen::() % node_eks.len()) as u32; + + if shares.iter().map(|x| x.0).any(|x| x == idx) { + break idx as usize; + } + }; + + shares.push(node_eks[random_duplicate_idx].clone()); + shares.shuffle(rng); + + let expected_error = if rec_threshold < threshold { + EncryptedKeyCombinationError::InsufficientShares + } else { + EncryptedKeyCombinationError::DuplicateNodeIndex + }; + assert_eq!(proto.combine_all(&shares), Err(expected_error)); + } + + // With combine_valid_shares OTOH we detect and reject the invalid shares for rec_threshold in threshold..nodes { let mut shares = random_subset(rng, &node_info, rec_threshold); @@ -338,4 +440,42 @@ fn test_protocol_execution() { assert_eq!(k, vetkey); } + + for rec_threshold in threshold..nodes { + let mut shares = random_subset(rng, &node_info, rec_threshold); + + let random_duplicate_idx = loop { + let idx = (rng.gen::() % node_eks.len()) as u32; + + if shares.iter().map(|x| x.0).any(|x| x == idx) { + break idx as usize; + } + }; + + println!("dup {}", random_duplicate_idx); + shares.push(node_info[random_duplicate_idx].clone()); + shares.shuffle(rng); + + let result = proto.combine_valid(&shares); + + if result.is_ok() { + // This can still suceed since we only look at the first threshold shares + // If success, verify that the duplicate appears later in the list + + let indexes = shares + .iter() + .map(|s| s.0) + .enumerate() + .filter(|(_i, s)| *s == random_duplicate_idx as u32) + .map(|s| s.0) + .collect::>(); + assert_eq!(indexes.len(), 2); + assert!(indexes[1] > threshold); + } else { + assert_eq!( + result, + Err(EncryptedKeyCombinationError::DuplicateNodeIndex) + ); + } + } } From 77888330ba4c84952b5587996d375328c1b96017 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Tue, 14 Jan 2025 16:55:43 -0500 Subject: [PATCH 11/22] Fix benchmarks --- .../bls12_381/vetkd/benches/vetkd.rs | 74 +------------------ 1 file changed, 3 insertions(+), 71 deletions(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs index d771cdf93e0..153bce95137 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs @@ -4,63 +4,13 @@ use ic_crypto_internal_bls12_381_vetkd::*; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; use rand::Rng; -fn transport_key_bench(c: &mut Criterion) { - let mut group = c.benchmark_group("crypto_bls12_381_transport_key"); - - let rng = &mut reproducible_rng(); - - group.bench_function("TransportSecretKey::generate", |b| { - b.iter(|| TransportSecretKey::generate(rng)) - }); - - group.bench_function("TransportSecretKey::serialize", |b| { - b.iter_batched( - || TransportSecretKey::generate(rng), - |key| key.serialize(), - BatchSize::SmallInput, - ) - }); - - group.bench_function("TransportSecretKey::deserialize", |b| { - b.iter_batched( - || TransportSecretKey::generate(rng).serialize(), - |key| TransportSecretKey::deserialize(&key), - BatchSize::SmallInput, - ) - }); - - group.bench_function("TransportSecretKey::public_key", |b| { - b.iter_batched( - || TransportSecretKey::generate(rng), - |key| key.public_key(), - BatchSize::SmallInput, - ) - }); - - group.bench_function("TransportPublicKey::serialize", |b| { - b.iter_batched( - || TransportSecretKey::generate(rng).public_key(), - |key| key.serialize(), - BatchSize::SmallInput, - ) - }); - - group.bench_function("TransportPublicKey::deserialize", |b| { - b.iter_batched( - || TransportSecretKey::generate(rng).public_key().serialize(), - |key| TransportPublicKey::deserialize(&key), - BatchSize::SmallInput, - ) - }); -} - fn vetkd_bench(c: &mut Criterion) { let mut group = c.benchmark_group("crypto_bls12_381_vetkd"); let rng = &mut reproducible_rng(); - let tsk = TransportSecretKey::generate(rng); - let tpk = tsk.public_key(); + let tsk = Scalar::random(rng); + let tpk = TransportPublicKey::deserialize(&(G1Affine::generator() * tsk).to_affine().serialize()).unwrap(); let derivation_path = DerivationPath::new(&[1, 2, 3, 4], &[&[1, 2, 3]]); let did = rng.gen::<[u8; 32]>(); @@ -77,8 +27,6 @@ fn vetkd_bench(c: &mut Criterion) { let node_sk = poly.evaluate_at(&Scalar::from_node_index(node_id)); let node_pk = G2Affine::from(G2Affine::generator() * &node_sk); - let dpk = DerivedPublicKey::compute_derived_key(&master_pk, &derivation_path); - if threshold == 9 { group.bench_function("EncryptedKeyShare::create", |b| { b.iter(|| { @@ -141,24 +89,8 @@ fn vetkd_bench(c: &mut Criterion) { }) }, ); - - if threshold == 9 { - let ek = EncryptedKey::combine_valid_shares( - &node_info, - threshold, - &master_pk, - &tpk, - &derivation_path, - &did, - ) - .unwrap(); - - group.bench_function("TransportSecretKey::decrypt", |b| { - b.iter(|| tsk.decrypt(&ek, &dpk, &did).unwrap()) - }); - } } } -criterion_group!(benches, transport_key_bench, vetkd_bench); +criterion_group!(benches, vetkd_bench); criterion_main!(benches); From 1e900a39b9624c690f095fb6f49c074c88861a1e Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Tue, 14 Jan 2025 18:11:54 -0500 Subject: [PATCH 12/22] fmt --- .../internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs index 153bce95137..e9bf527021a 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/benches/vetkd.rs @@ -10,7 +10,9 @@ fn vetkd_bench(c: &mut Criterion) { let rng = &mut reproducible_rng(); let tsk = Scalar::random(rng); - let tpk = TransportPublicKey::deserialize(&(G1Affine::generator() * tsk).to_affine().serialize()).unwrap(); + let tpk = + TransportPublicKey::deserialize(&(G1Affine::generator() * tsk).to_affine().serialize()) + .unwrap(); let derivation_path = DerivationPath::new(&[1, 2, 3, 4], &[&[1, 2, 3]]); let did = rng.gen::<[u8; 32]>(); From 358ff3e6e09c74241cd66af9046008aafa085e25 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Tue, 14 Jan 2025 18:13:05 -0500 Subject: [PATCH 13/22] Remove debug println fix occasional test fail --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs index 6c8af537466..21dee15f977 100644 --- a/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs +++ b/rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs @@ -452,7 +452,6 @@ fn test_protocol_execution() { } }; - println!("dup {}", random_duplicate_idx); shares.push(node_info[random_duplicate_idx].clone()); shares.shuffle(rng); @@ -470,7 +469,7 @@ fn test_protocol_execution() { .map(|s| s.0) .collect::>(); assert_eq!(indexes.len(), 2); - assert!(indexes[1] > threshold); + assert!(indexes[1] >= threshold); } else { assert_eq!( result, From c5cb6bbbff3081b56f2fdee1131d3e090a6ae159 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 15 Jan 2025 10:57:07 -0500 Subject: [PATCH 14/22] Fix CSP tests --- .../src/vault/local_csp_vault/vetkd/tests.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs index ae543665a57..66f08d59f53 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs @@ -3,7 +3,7 @@ use crate::types::CspSecretKey; use crate::vault::api::{VetKdCspVault, VetKdEncryptedKeyShareCreationVaultError}; use crate::{key_id::KeyId, LocalCspVault}; use assert_matches::assert_matches; -use ic_crypto_internal_bls12_381_vetkd::{G2Affine, Scalar, TransportSecretKey}; +use ic_crypto_internal_bls12_381_vetkd::{G2Affine, Scalar, TransportPublicKey}; use ic_crypto_internal_multi_sig_bls12381::types as multi_types; use ic_crypto_internal_threshold_sig_bls12381::types as threshold_types; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; @@ -13,6 +13,12 @@ use ic_types_test_utils::ids::canister_test_id; use rand::{CryptoRng, Rng, SeedableRng}; use rand_chacha::ChaCha20Rng; +fn create_transport_public_key(rng: &mut R) -> TransportPublicKey { + let g2 = G2Affine::hash("ic-crypto-test".as_bytes(), &rng.gen::<[u8; 32]>()); + TransportPublicKey::deserialize(&g2.serialize()) + .expect("Failed to deserialize G2Affine") +} + #[test] fn should_correctly_create_encrypted_vetkd_key_share() { let rng = reproducible_rng(); @@ -50,7 +56,8 @@ fn create_encrypted_vetkd_key_share( ) -> Result { let master_secret_key = Scalar::random(&mut rng); let master_public_key = G2Affine::from(G2Affine::generator() * &master_secret_key); - let encryption_public_key = TransportSecretKey::generate(&mut rng).public_key(); + let encryption_public_key = create_transport_public_key(&mut rng); + let key_id = KeyId::from([123; 32]); let mut node_sks = MockSecretKeyStore::new(); @@ -88,7 +95,7 @@ fn should_fail_to_create_key_share_with_invalid_master_public_key() { let rng = &mut reproducible_rng(); let invalid_master_public_key = b"invalid-master-public-key".to_vec(); - let encryption_public_key = TransportSecretKey::generate(rng).public_key(); + let encryption_public_key = create_transport_public_key(rng); let key_id = KeyId::from([123; 32]); let derivation_path = ExtendedDerivationPath { @@ -153,7 +160,7 @@ fn should_fail_to_create_key_share_if_key_is_missing_in_secret_key_store() { let master_secret_key = Scalar::random(&mut rng); let master_public_key = G2Affine::from(G2Affine::generator() * &master_secret_key); - let encryption_public_key = TransportSecretKey::generate(&mut rng).public_key(); + let encryption_public_key = create_transport_public_key(&mut rng); let key_id = KeyId::from([123; 32]); let mut node_sks = MockSecretKeyStore::new(); @@ -195,7 +202,7 @@ fn should_fail_to_create_key_share_if_key_in_secret_key_store_has_wrong_type() { let master_secret_key = Scalar::random(&mut rng); let master_public_key = G2Affine::from(G2Affine::generator() * &master_secret_key); - let encryption_public_key = TransportSecretKey::generate(&mut rng).public_key(); + let encryption_public_key = create_transport_public_key(&mut rng); let key_id = KeyId::from([123; 32]); let mut node_sks = MockSecretKeyStore::new(); From 0032b10b4a0827d02a941296184606003dd55bbe Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 15 Jan 2025 11:03:06 -0500 Subject: [PATCH 15/22] fixup --- .../src/vault/local_csp_vault/vetkd/tests.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs index 3d1d20ecb8c..730463e5855 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs @@ -3,7 +3,7 @@ use crate::types::CspSecretKey; use crate::vault::api::{VetKdCspVault, VetKdEncryptedKeyShareCreationVaultError}; use crate::{key_id::KeyId, LocalCspVault}; use assert_matches::assert_matches; -use ic_crypto_internal_bls12_381_vetkd::{G2Affine, Scalar, TransportPublicKey}; +use ic_crypto_internal_bls12_381_vetkd::{G2Affine, Scalar}; use ic_crypto_internal_multi_sig_bls12381::types as multi_types; use ic_crypto_internal_threshold_sig_bls12381::types as threshold_types; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; @@ -13,12 +13,6 @@ use ic_types_test_utils::ids::canister_test_id; use rand::{CryptoRng, Rng, SeedableRng}; use rand_chacha::ChaCha20Rng; -fn create_transport_public_key(rng: &mut R) -> TransportPublicKey { - let g2 = G2Affine::hash("ic-crypto-test".as_bytes(), &rng.gen::<[u8; 32]>()); - TransportPublicKey::deserialize(&g2.serialize()) - .expect("Failed to deserialize G2Affine") -} - #[test] fn should_correctly_create_encrypted_vetkd_key_share() { let rng = &mut reproducible_rng(); @@ -138,7 +132,7 @@ impl CreateVetKdKeyShareTestSetup { pub fn new(rng: &mut R) -> Self { let master_secret_key = Scalar::random(rng); let master_public_key = G2Affine::from(G2Affine::generator() * &master_secret_key); - let transport_secret_key = TransportSecretKey::generate(rng); + let transport_public_key = G2Affine::hash("ic-crypto-vetkd-test".as_bytes(), &rng.gen::<[u8; 32]>()); let key_id = KeyId::from([123; 32]); let derivation_path = ExtendedDerivationPath { caller: canister_test_id(234).get(), @@ -150,7 +144,7 @@ impl CreateVetKdKeyShareTestSetup { Self { master_secret_key, master_public_key: master_public_key.serialize().to_vec(), - transport_public_key: transport_secret_key.public_key().serialize().to_vec(), + transport_public_key: transport_public_key.serialize().to_vec(), key_id, derivation_path, derivation_id, From d2f915312772d8e7eaa991412c76219516e16f96 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 15 Jan 2025 13:56:27 -0500 Subject: [PATCH 16/22] fmt --- .../src/vault/local_csp_vault/vetkd/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs index 730463e5855..4ee45b17820 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs @@ -132,7 +132,8 @@ impl CreateVetKdKeyShareTestSetup { pub fn new(rng: &mut R) -> Self { let master_secret_key = Scalar::random(rng); let master_public_key = G2Affine::from(G2Affine::generator() * &master_secret_key); - let transport_public_key = G2Affine::hash("ic-crypto-vetkd-test".as_bytes(), &rng.gen::<[u8; 32]>()); + let transport_public_key = + G2Affine::hash("ic-crypto-vetkd-test".as_bytes(), &rng.gen::<[u8; 32]>()); let key_id = KeyId::from([123; 32]); let derivation_path = ExtendedDerivationPath { caller: canister_test_id(234).get(), From b92d509a0583b1a420b705d7d659cf24ac857158 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 15 Jan 2025 15:35:06 -0500 Subject: [PATCH 17/22] Fix csp tests --- .../src/vault/local_csp_vault/vetkd/tests.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs index 4ee45b17820..fdc86140ec4 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs @@ -3,7 +3,7 @@ use crate::types::CspSecretKey; use crate::vault::api::{VetKdCspVault, VetKdEncryptedKeyShareCreationVaultError}; use crate::{key_id::KeyId, LocalCspVault}; use assert_matches::assert_matches; -use ic_crypto_internal_bls12_381_vetkd::{G2Affine, Scalar}; +use ic_crypto_internal_bls12_381_vetkd::{G1Affine, G2Affine, Scalar}; use ic_crypto_internal_multi_sig_bls12381::types as multi_types; use ic_crypto_internal_threshold_sig_bls12381::types as threshold_types; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; @@ -132,8 +132,7 @@ impl CreateVetKdKeyShareTestSetup { pub fn new(rng: &mut R) -> Self { let master_secret_key = Scalar::random(rng); let master_public_key = G2Affine::from(G2Affine::generator() * &master_secret_key); - let transport_public_key = - G2Affine::hash("ic-crypto-vetkd-test".as_bytes(), &rng.gen::<[u8; 32]>()); + let transport_public_key = G1Affine::from(G1Affine::generator() * Scalar::random(rng)); let key_id = KeyId::from([123; 32]); let derivation_path = ExtendedDerivationPath { caller: canister_test_id(234).get(), From 4f2f9f5e41f63d7f549a785315ec4766a0b1cdb4 Mon Sep 17 00:00:00 2001 From: Franz-Stefan Preiss Date: Wed, 22 Jan 2025 13:53:53 +0000 Subject: [PATCH 18/22] feat(crypto): CRP-2599 implement VetKdProtocol trait for CryptoComponent --- Cargo.lock | 1 + rs/crypto/BUILD.bazel | 1 + rs/crypto/Cargo.toml | 1 + .../crypto_service_provider/src/vault/api.rs | 8 +- .../src/vault/local_csp_vault/vetkd/mod.rs | 41 +- .../src/vault/local_csp_vault/vetkd/tests.rs | 12 +- rs/crypto/src/lib.rs | 1 + rs/crypto/src/sign/mod.rs | 6 +- rs/crypto/src/sign/threshold_sig.rs | 2 +- rs/crypto/src/vetkd/mod.rs | 542 ++++++++++++++++++ rs/interfaces/src/crypto/errors.rs | 4 +- .../v1/crypto_log_entry.proto | 4 + .../src/gen/log/log.crypto_log_entry.v1.rs | 8 + rs/types/types/src/crypto/hash.rs | 9 - rs/types/types/src/crypto/sign.rs | 8 + rs/types/types/src/crypto/vetkd.rs | 37 +- 16 files changed, 632 insertions(+), 53 deletions(-) create mode 100644 rs/crypto/src/vetkd/mod.rs diff --git a/Cargo.lock b/Cargo.lock index aa181f1311b..33231730f6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6909,6 +6909,7 @@ dependencies = [ "ic-crypto-internal-basic-sig-ecdsa-secp256r1", "ic-crypto-internal-basic-sig-ed25519", "ic-crypto-internal-basic-sig-rsa-pkcs1", + "ic-crypto-internal-bls12-381-vetkd", "ic-crypto-internal-csp", "ic-crypto-internal-csp-proptest-utils", "ic-crypto-internal-csp-test-utils", diff --git a/rs/crypto/BUILD.bazel b/rs/crypto/BUILD.bazel index 2c98b370a4c..3cdaa0b0151 100644 --- a/rs/crypto/BUILD.bazel +++ b/rs/crypto/BUILD.bazel @@ -17,6 +17,7 @@ DEPENDENCIES = [ "//rs/crypto/ed25519", "//rs/crypto/interfaces/sig_verification", "//rs/crypto/internal/crypto_lib/basic_sig/ed25519", + "//rs/crypto/internal/crypto_lib/bls12_381/vetkd", "//rs/crypto/internal/crypto_lib/seed", "//rs/crypto/internal/crypto_lib/threshold_sig/bls12_381", "//rs/crypto/internal/crypto_lib/threshold_sig/canister_threshold_sig", diff --git a/rs/crypto/Cargo.toml b/rs/crypto/Cargo.toml index 590ad0001dc..46b7c841deb 100644 --- a/rs/crypto/Cargo.toml +++ b/rs/crypto/Cargo.toml @@ -25,6 +25,7 @@ ic-crypto-internal-logmon = { path = "internal/logmon" } ic-crypto-internal-seed = { path = "internal/crypto_lib/seed" } ic-crypto-internal-threshold-sig-bls12381 = { path = "internal/crypto_lib/threshold_sig/bls12_381" } ic-crypto-internal-threshold-sig-canister-threshold-sig = { path = "internal/crypto_lib/threshold_sig/canister_threshold_sig" } +ic-crypto-internal-bls12-381-vetkd = { path = "internal/crypto_lib/bls12_381/vetkd" } ic-crypto-internal-types = { path = "internal/crypto_lib/types" } ic-crypto-standalone-sig-verifier = { path = "standalone-sig-verifier" } ic-crypto-tls-cert-validation = { path = "node_key_validation/tls_cert_validation" } diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/api.rs b/rs/crypto/internal/crypto_service_provider/src/vault/api.rs index 3b82edf4386..21c46f56354 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/api.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/api.rs @@ -974,10 +974,14 @@ pub trait VetKdCspVault { /// Vault-level error for vetKD key share creation. #[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)] pub enum VetKdEncryptedKeyShareCreationVaultError { - /// If some arguments are invalid - InvalidArgument(String), + /// If the secret key is missing in the key store of if it has the wrong type + SecretKeyMissingOrWrongType(String), /// If a transient internal error occurs, e.g., an RPC error communicating with the remote vault TransientInternalError(String), + /// If the given master public key is invalid + InvalidArgumentMasterPublicKey, + /// If the given encryption public key is invalid + InvalidArgumentEncryptionPublicKey, } /// An error returned by failing to generate a public seed from [`CspVault`]. diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/mod.rs b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/mod.rs index fef59022a17..fd7b4bf88fe 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/mod.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/mod.rs @@ -60,40 +60,35 @@ impl Result { let master_public_key = G2Affine::deserialize(&master_public_key).map_err(|_: PairingInvalidPoint| { - VetKdEncryptedKeyShareCreationVaultError::InvalidArgument(format!( - "invalid master public key: 0x{}", - hex::encode(&master_public_key) - )) + VetKdEncryptedKeyShareCreationVaultError::InvalidArgumentMasterPublicKey })?; let transport_public_key = TransportPublicKey::deserialize(&encryption_public_key) .map_err(|e| match e { TransportPublicKeyDeserializationError::InvalidPublicKey => { - VetKdEncryptedKeyShareCreationVaultError::InvalidArgument(format!( - "invalid encryption public key: 0x{}", - hex::encode(&encryption_public_key) - )) + VetKdEncryptedKeyShareCreationVaultError::InvalidArgumentEncryptionPublicKey } })?; let secret_key_from_store = self.sks_read_lock().get(&key_id).ok_or( - VetKdEncryptedKeyShareCreationVaultError::InvalidArgument(format!( - "missing key with ID {key_id:?}", + VetKdEncryptedKeyShareCreationVaultError::SecretKeyMissingOrWrongType(format!( + "missing key with ID {key_id}" )), )?; - let secret_bls_scalar = if let CspSecretKey::ThresBls12_381(secret_key_bytes) = - &secret_key_from_store - { - // We use the unchecked deserialization here because it is slighly cheaper, but mainly because - // it cannot fail, and the data is anyway trusted as it comes from the secret key store. - Ok(Scalar::deserialize_unchecked( - secret_key_bytes.inner_secret().expose_secret(), - )) - } else { - Err(VetKdEncryptedKeyShareCreationVaultError::InvalidArgument( - format!("wrong secret key type for key with ID {key_id}: expected ThresBls12_381"), - )) - }?; + let secret_bls_scalar = + if let CspSecretKey::ThresBls12_381(secret_key_bytes) = &secret_key_from_store { + // We use the unchecked deserialization here because it is slighly cheaper, but mainly because + // it cannot fail, and the data is anyway trusted as it comes from the secret key store. + Ok(Scalar::deserialize_unchecked( + secret_key_bytes.inner_secret().expose_secret(), + )) + } else { + Err( + VetKdEncryptedKeyShareCreationVaultError::SecretKeyMissingOrWrongType(format!( + "wrong secret key type for key with ID {key_id}: expected ThresBls12_381" + )), + ) + }?; // Create encrypted key share using our library let encrypted_key_share = EncryptedKeyShare::create( diff --git a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs index fdc86140ec4..68108017335 100644 --- a/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs +++ b/rs/crypto/internal/crypto_service_provider/src/vault/local_csp_vault/vetkd/tests.rs @@ -63,8 +63,8 @@ fn should_fail_to_create_key_share_with_invalid_master_public_key() { let result = test_env.create_encrypted_vetkd_key_share(); assert_matches!( - result, Err(VetKdEncryptedKeyShareCreationVaultError::InvalidArgument(error)) - if error.contains("invalid master public key") + result, + Err(VetKdEncryptedKeyShareCreationVaultError::InvalidArgumentMasterPublicKey) ); } @@ -79,8 +79,8 @@ fn should_fail_to_create_key_share_with_invalid_encryption_public_key() { let result = test_env.create_encrypted_vetkd_key_share(); assert_matches!( - result, Err(VetKdEncryptedKeyShareCreationVaultError::InvalidArgument(error)) - if error.contains("invalid encryption public key") + result, + Err(VetKdEncryptedKeyShareCreationVaultError::InvalidArgumentEncryptionPublicKey) ); } @@ -94,7 +94,7 @@ fn should_fail_to_create_key_share_if_key_is_missing_in_secret_key_store() { let result = test_env.create_encrypted_vetkd_key_share(); assert_matches!( - result, Err(VetKdEncryptedKeyShareCreationVaultError::InvalidArgument(error)) + result, Err(VetKdEncryptedKeyShareCreationVaultError::SecretKeyMissingOrWrongType(error)) if error.contains("missing key with ID") ); } @@ -111,7 +111,7 @@ fn should_fail_to_create_key_share_if_key_in_secret_key_store_has_wrong_type() { let result = test_env.create_encrypted_vetkd_key_share(); assert_matches!( - result, Err(VetKdEncryptedKeyShareCreationVaultError::InvalidArgument(error)) + result, Err(VetKdEncryptedKeyShareCreationVaultError::SecretKeyMissingOrWrongType(error)) if error.contains("wrong secret key type") ); } diff --git a/rs/crypto/src/lib.rs b/rs/crypto/src/lib.rs index a720e2ff407..0d917897660 100644 --- a/rs/crypto/src/lib.rs +++ b/rs/crypto/src/lib.rs @@ -16,6 +16,7 @@ mod common; mod keygen; mod sign; mod tls; +mod vetkd; use ic_crypto_internal_csp::vault::api::CspVault; pub use sign::{ diff --git a/rs/crypto/src/sign/mod.rs b/rs/crypto/src/sign/mod.rs index 668e462383e..de4da5f1080 100644 --- a/rs/crypto/src/sign/mod.rs +++ b/rs/crypto/src/sign/mod.rs @@ -1,7 +1,5 @@ use super::*; -use crate::sign::basic_sig::BasicSigVerifierInternal; -use crate::sign::basic_sig::BasicSignerInternal; use crate::sign::multi_sig::MultiSigVerifierInternal; use crate::sign::multi_sig::MultiSignerInternal; use crate::sign::threshold_sig::{ThresholdSigVerifierInternal, ThresholdSignerInternal}; @@ -37,7 +35,9 @@ use ic_types::{NodeId, RegistryVersion, SubnetId}; use std::collections::{BTreeMap, BTreeSet}; use std::convert::TryFrom; -pub use threshold_sig::ThresholdSigDataStoreImpl; +pub(crate) use basic_sig::{BasicSigVerifierInternal, BasicSignerInternal}; +pub(crate) use threshold_sig::lazily_calculated_public_key_from_store; +pub use threshold_sig::{ThresholdSigDataStore, ThresholdSigDataStoreImpl}; mod basic_sig; mod canister_threshold_sig; diff --git a/rs/crypto/src/sign/threshold_sig.rs b/rs/crypto/src/sign/threshold_sig.rs index 0c44c0a3f44..efce632ee54 100644 --- a/rs/crypto/src/sign/threshold_sig.rs +++ b/rs/crypto/src/sign/threshold_sig.rs @@ -192,7 +192,7 @@ impl ThresholdSigVerifierInternal { /// Given that both cases indicate that the implementations of DKG and threshold /// signatures are not aligned and also a caller could not recover from this, we /// panic. -fn lazily_calculated_public_key_from_store( +pub(crate) fn lazily_calculated_public_key_from_store( lockable_threshold_sig_data_store: &LockableThresholdSigDataStore, threshold_sig_csp_client: &C, dkg_id: &NiDkgId, diff --git a/rs/crypto/src/vetkd/mod.rs b/rs/crypto/src/vetkd/mod.rs new file mode 100644 index 00000000000..673edca16d2 --- /dev/null +++ b/rs/crypto/src/vetkd/mod.rs @@ -0,0 +1,542 @@ +use super::get_log_id; +use crate::sign::lazily_calculated_public_key_from_store; +use crate::sign::BasicSigVerifierInternal; +use crate::sign::BasicSignerInternal; +use crate::sign::ThresholdSigDataStore; +use crate::{CryptoComponentImpl, LockableThresholdSigDataStore}; +use ic_crypto_internal_bls12_381_vetkd::DerivationPath; +use ic_crypto_internal_bls12_381_vetkd::EncryptedKey; +use ic_crypto_internal_bls12_381_vetkd::EncryptedKeyCombinationError; +use ic_crypto_internal_bls12_381_vetkd::EncryptedKeyShare; +use ic_crypto_internal_bls12_381_vetkd::EncryptedKeyShareDeserializationError; +use ic_crypto_internal_bls12_381_vetkd::G2Affine; +use ic_crypto_internal_bls12_381_vetkd::NodeIndex; +use ic_crypto_internal_bls12_381_vetkd::PairingInvalidPoint; +use ic_crypto_internal_bls12_381_vetkd::TransportPublicKey; +use ic_crypto_internal_bls12_381_vetkd::TransportPublicKeyDeserializationError; +use ic_crypto_internal_csp::api::CspSigner; +use ic_crypto_internal_csp::api::ThresholdSignatureCspClient; +use ic_crypto_internal_csp::key_id::KeyIdInstantiationError; +use ic_crypto_internal_csp::vault::api::VetKdEncryptedKeyShareCreationVaultError; +use ic_crypto_internal_csp::{key_id::KeyId, vault::api::CspVault, CryptoServiceProvider}; +use ic_crypto_internal_logmon::metrics::{MetricsDomain, MetricsResult, MetricsScope}; +use ic_crypto_internal_types::sign::threshold_sig::public_coefficients::PublicCoefficients; +use ic_crypto_internal_types::sign::threshold_sig::public_key::CspThresholdSigPublicKey; +use ic_interfaces::crypto::VetKdProtocol; +use ic_interfaces_registry::RegistryClient; +use ic_logger::{debug, info, new_logger, ReplicaLogger}; +use ic_types::crypto::threshold_sig::errors::threshold_sig_data_not_found_error::ThresholdSigDataNotFoundError; +use ic_types::crypto::vetkd::{ + VetKdArgs, VetKdEncryptedKey, VetKdEncryptedKeyShare, VetKdKeyShareCombinationError, + VetKdKeyShareCreationError, VetKdKeyShareVerificationError, VetKdKeyVerificationError, +}; +use ic_types::crypto::{BasicSig, BasicSigOf}; +use ic_types::NodeId; +use std::collections::BTreeMap; +use std::fmt; + +impl VetKdProtocol for CryptoComponentImpl { + // TODO(CRP-2639): Adapt VetKdKeyShareCreationError so that clippy exception is no longer needed + #[allow(clippy::result_large_err)] + fn create_encrypted_key_share( + &self, + args: VetKdArgs, + ) -> Result { + let log_id = get_log_id(&self.logger); + let logger = new_logger!(&self.logger; + crypto.log_id => log_id, + crypto.trait_name => "VetKdProtocol", + crypto.method_name => "create_encrypted_key_share", + ); + debug!(logger; + crypto.description => "start", + crypto.vetkd_args => format!("{}", args), + ); + let start_time = self.metrics.now(); + let result = create_encrypted_key_share_internal( + &self.lockable_threshold_sig_data_store, + self.registry_client.as_ref(), + self.vault.as_ref(), + &self.csp, + args, + self.node_id, + ); + self.metrics.observe_duration_seconds( + MetricsDomain::VetKd, + MetricsScope::Full, + "create_encrypted_key_share", + MetricsResult::from(&result), + start_time, + ); + debug!(logger; + crypto.description => "end", + crypto.is_ok => result.is_ok(), + crypto.error => log_err(result.as_ref().err()), + crypto.vetkd_key_share => log_ok_content(&result), + ); + result + } + + fn verify_encrypted_key_share( + &self, + signer: NodeId, + key_share: &VetKdEncryptedKeyShare, + _args: &VetKdArgs, + ) -> Result<(), VetKdKeyShareVerificationError> { + let log_id = get_log_id(&self.logger); + let logger = new_logger!(&self.logger; + crypto.log_id => log_id, + crypto.trait_name => "VetKdProtocol", + crypto.method_name => "verify_encrypted_key_share", + ); + debug!(logger; + crypto.description => "start", + crypto.signer => format!("{}", signer), + crypto.vetkd_key_share => format!("{}", key_share), + ); + let start_time = self.metrics.now(); + let result = verify_encrypted_key_share_internal( + self.registry_client.as_ref(), + &self.csp, + key_share, + signer, + ); + self.metrics.observe_duration_seconds( + MetricsDomain::VetKd, + MetricsScope::Full, + "verify_encrypted_key_share", + MetricsResult::from(&result), + start_time, + ); + debug!(logger; + crypto.description => "end", + crypto.is_ok => result.is_ok(), + crypto.error => log_err(result.as_ref().err()), + ); + result + } + + fn combine_encrypted_key_shares( + &self, + shares: &BTreeMap, + args: &VetKdArgs, + ) -> Result { + let log_id = get_log_id(&self.logger); + let logger = new_logger!(&self.logger; + crypto.log_id => log_id, + crypto.trait_name => "VetKdProtocol", + crypto.method_name => "combine_encrypted_key_shares", + ); + debug!(logger; + crypto.description => "start", + crypto.vetkd_args => format!("{}", args), + crypto.vetkd_key_shares => format!("{:?}", shares), + ); + let start_time = self.metrics.now(); + let result = combine_encrypted_key_shares_internal( + &self.lockable_threshold_sig_data_store, + &self.csp, + &self.logger, + shares, + args, + ); + self.metrics.observe_duration_seconds( + MetricsDomain::VetKd, + MetricsScope::Full, + "combine_encrypted_key_shares", + MetricsResult::from(&result), + start_time, + ); + debug!(logger; + crypto.description => "end", + crypto.is_ok => result.is_ok(), + crypto.error => log_err(result.as_ref().err()), + crypto.vetkd_key => log_ok_content(&result), + ); + result + } + + fn verify_encrypted_key( + &self, + key: &VetKdEncryptedKey, + args: &VetKdArgs, + ) -> Result<(), VetKdKeyVerificationError> { + let log_id = get_log_id(&self.logger); + let logger = new_logger!(&self.logger; + crypto.log_id => log_id, + crypto.trait_name => "VetKdProtocol", + crypto.method_name => "verify_encrypted_key", + ); + debug!(logger; + crypto.description => "start", + crypto.vetkd_args => format!("{}", args), + crypto.vetkd_key => format!("{}", key), + ); + let start_time = self.metrics.now(); + let result = + verify_encrypted_key_internal(&self.lockable_threshold_sig_data_store, key, args); + self.metrics.observe_duration_seconds( + MetricsDomain::VetKd, + MetricsScope::Full, + "verify_encrypted_key", + MetricsResult::from(&result), + start_time, + ); + debug!(logger; + crypto.description => "end", + crypto.is_ok => result.is_ok(), + crypto.error => log_err(result.as_ref().err()), + ); + result + } +} + +// TODO(CRP-2639): Adapt VetKdKeyShareCreationError so that clippy exception is no longer needed +#[allow(clippy::result_large_err)] +fn create_encrypted_key_share_internal( + lockable_threshold_sig_data_store: &LockableThresholdSigDataStore, + registry: &dyn RegistryClient, + vault: &dyn CspVault, + csp_signer: &S, + args: VetKdArgs, + self_node_id: NodeId, +) -> Result { + let pub_coeffs_from_store = lockable_threshold_sig_data_store + .read() + .transcript_data(&args.ni_dkg_id) + .map(|data| data.public_coefficients().clone()) + .ok_or_else(|| { + VetKdKeyShareCreationError::ThresholdSigDataNotFound( + ThresholdSigDataNotFoundError::ThresholdSigDataNotFound { + dkg_id: args.ni_dkg_id.clone(), + }, + ) + })?; + let key_id = KeyId::try_from(&pub_coeffs_from_store).map_err(|e| match e { + KeyIdInstantiationError::InvalidArguments(msg) => { + VetKdKeyShareCreationError::KeyIdInstantiationError(msg) + } + })?; + let master_public_key = match pub_coeffs_from_store { + PublicCoefficients::Bls12_381(pub_coeffs) => pub_coeffs + .coefficients + .iter() + .copied() + .next() + .ok_or_else(|| { + VetKdKeyShareCreationError::InternalError(format!( + "public coefficients for NI-DKG ID {} are empty", + &args.ni_dkg_id + )) + })?, + }; + + let encrypted_key_share = vault + .create_encrypted_vetkd_key_share( + key_id, + master_public_key.as_bytes().to_vec(), + args.encryption_public_key, + args.derivation_path, + args.derivation_id, + ) + .map_err(vetkd_key_share_creation_error_from_vault_error)?; + + let signature = BasicSignerInternal::sign_basic( + csp_signer, + registry, + &encrypted_key_share, + self_node_id, + // TODO(CRP-2670): Store registry version in ThresholdSigDataStore when loading transcript and then use it here instead get_latest_version + // TODO(CRP-2666): Cleanup: Remove registry_version from BasicSigner::sign_basic API + registry.get_latest_version(), + ) + .map_err(VetKdKeyShareCreationError::KeyShareSigningError)?; + + Ok(VetKdEncryptedKeyShare { + encrypted_key_share, + node_signature: signature.get().0, + }) +} + +fn vetkd_key_share_creation_error_from_vault_error( + error: VetKdEncryptedKeyShareCreationVaultError, +) -> VetKdKeyShareCreationError { + match error { + VetKdEncryptedKeyShareCreationVaultError::SecretKeyMissingOrWrongType(error) => { + VetKdKeyShareCreationError::InternalError(format!( + "secret key missing or wrong type: {error}" + )) + } + VetKdEncryptedKeyShareCreationVaultError::InvalidArgumentMasterPublicKey => { + VetKdKeyShareCreationError::InternalError("invalid master public key".to_string()) + } + VetKdEncryptedKeyShareCreationVaultError::InvalidArgumentEncryptionPublicKey => { + VetKdKeyShareCreationError::InvalidArgumentEncryptionPublicKey + } + VetKdEncryptedKeyShareCreationVaultError::TransientInternalError(error) => { + VetKdKeyShareCreationError::TransientInternalError(error) + } + } +} + +fn verify_encrypted_key_share_internal( + registry: &dyn RegistryClient, + csp_signer: &S, + key_share: &VetKdEncryptedKeyShare, + signer: NodeId, +) -> Result<(), VetKdKeyShareVerificationError> { + let signature = BasicSigOf::new(BasicSig(key_share.node_signature.clone())); + BasicSigVerifierInternal::verify_basic_sig( + csp_signer, + registry, + &signature, + &key_share.encrypted_key_share, + signer, + // TODO(CRP-2670): Store registry version in ThresholdSigDataStore when loading transcript and then use it here + registry.get_latest_version(), + ) + .map_err(VetKdKeyShareVerificationError::VerificationError) +} + +fn combine_encrypted_key_shares_internal( + lockable_threshold_sig_data_store: &LockableThresholdSigDataStore, + threshold_sig_csp_client: &C, + logger: &ReplicaLogger, + shares: &BTreeMap, + args: &VetKdArgs, +) -> Result { + let transcript_data_from_store = lockable_threshold_sig_data_store + .read() + .transcript_data(&args.ni_dkg_id) + .cloned() + .ok_or_else(|| { + VetKdKeyShareCombinationError::ThresholdSigDataNotFound( + ThresholdSigDataNotFoundError::ThresholdSigDataNotFound { + dkg_id: args.ni_dkg_id.clone(), + }, + ) + })?; + let pub_coeffs_from_store = match transcript_data_from_store.public_coefficients() { + PublicCoefficients::Bls12_381(pub_coeffs) => &pub_coeffs.coefficients, + }; + let reconstruction_threshold = pub_coeffs_from_store.len(); + let master_public_key = { + let public_key_bytes = pub_coeffs_from_store + .iter() + .copied() + .next() + .ok_or_else(|| { + VetKdKeyShareCombinationError::InternalError(format!( + "failed to determine master public key: public coefficients for NI-DKG ID {} are empty", + &args.ni_dkg_id + )) + })?; + ic_crypto_internal_bls12_381_vetkd::G2Affine::deserialize(&public_key_bytes).map_err( + |_: PairingInvalidPoint| VetKdKeyShareCombinationError::InvalidArgumentMasterPublicKey, + )? + }; + let transport_public_key = TransportPublicKey::deserialize(&args.encryption_public_key) + .map_err(|e| match e { + TransportPublicKeyDeserializationError::InvalidPublicKey => { + VetKdKeyShareCombinationError::InvalidArgumentEncryptionPublicKey + } + })?; + let clib_shares: Vec<(NodeId, NodeIndex, EncryptedKeyShare)> = shares + .iter() + .map(|(&node_id, share)| { + let node_index = transcript_data_from_store.index(node_id).ok_or( + VetKdKeyShareCombinationError::InternalError(format!( + "missing index for node with ID {node_id} in threshold \ + sig data store for NI-DKG ID {}", + args.ni_dkg_id + )), + )?; + let clib_share = share_to_clib_share(share)?; + Ok((node_id, *node_index, clib_share)) + }) + .collect::>()?; + let clib_shares_for_combine_all: Vec<(NodeIndex, EncryptedKeyShare)> = clib_shares + .iter() + .map(|(_node_id, node_index, clib_share)| (*node_index, clib_share.clone())) + .collect(); + let derivation_path = DerivationPath::new( + args.derivation_path.caller.as_slice(), + &args.derivation_path.derivation_path, + ); + + match ic_crypto_internal_bls12_381_vetkd::EncryptedKey::combine_all( + &clib_shares_for_combine_all[..], + reconstruction_threshold, + &master_public_key, + &transport_public_key, + &derivation_path, + &args.derivation_id, + ) { + Ok(encrypted_key) => Ok(encrypted_key), + Err(EncryptedKeyCombinationError::InsufficientShares) => { + Err(VetKdKeyShareCombinationError::UnsatisfiedReconstructionThreshold { + threshold: reconstruction_threshold, + share_count: clib_shares_for_combine_all.len() + }) + } + Err(EncryptedKeyCombinationError::InvalidShares) => { + info!(logger, "EncryptedKey::combine_all failed with InvalidShares, \ + falling back to EncryptedKey::combine_valid_shares" + ); + + let clib_shares_for_combine_valid: Vec<(NodeIndex, G2Affine, EncryptedKeyShare)> = clib_shares + .iter() + .map(|(node_id, node_index, clib_share)| { + let node_public_key = lazily_calculated_public_key_from_store( + lockable_threshold_sig_data_store, + threshold_sig_csp_client, + &args.ni_dkg_id, + *node_id, + ) + .map_err(|e| { + VetKdKeyShareCombinationError::IndividualPublicKeyComputationError(e) + })?; + let node_public_key_g2affine = match node_public_key { + CspThresholdSigPublicKey::ThresBls12_381(public_key_bytes) => { + /////////////////////////////////////////////////////////////// + // TODO: Can/should we use G2Affine::deserialize_unchecked here + G2Affine::deserialize(&public_key_bytes.0) + .map_err(|_: PairingInvalidPoint| VetKdKeyShareCombinationError::InternalError( + format!("individual public key of node with ID {node_id} in threshold sig data store") + )) + } + }?; + Ok((*node_index, node_public_key_g2affine, clib_share.clone())) + }) + .collect::>()?; + + ic_crypto_internal_bls12_381_vetkd::EncryptedKey::combine_valid_shares( + &clib_shares_for_combine_valid[..], + reconstruction_threshold, + &master_public_key, + &transport_public_key, + &derivation_path, + &args.derivation_id, + ) + .map_err(|e| { + VetKdKeyShareCombinationError::CombinationError(format!( + "failed to combine the valid encrypted vetKD key shares: {e:?}" + )) + }) + }, + Err(other_error) => { + Err(VetKdKeyShareCombinationError::CombinationError(format!( + "failed to combine the valid encrypted vetKD key shares: {other_error:?}" + ))) + } + } + .map(|encrypted_key| VetKdEncryptedKey { + encrypted_key: encrypted_key.serialize().to_vec(), + }) +} + +fn share_to_clib_share( + share: &ic_types::crypto::vetkd::VetKdEncryptedKeyShare, +) -> Result { + let encrypted_key_share_192_bytes = { + if share.encrypted_key_share.0.len() != EncryptedKeyShare::BYTES { + return Err(VetKdKeyShareCombinationError::InvalidArgumentEncryptedKeyShare); + } + let mut encrypted_key = [0u8; EncryptedKeyShare::BYTES]; + encrypted_key.copy_from_slice(&share.encrypted_key_share.0); + encrypted_key + }; + ic_crypto_internal_bls12_381_vetkd::EncryptedKeyShare::deserialize( + encrypted_key_share_192_bytes, + ) + .map_err(|e| match e { + EncryptedKeyShareDeserializationError::InvalidEncryptedKeyShare => { + VetKdKeyShareCombinationError::InvalidArgumentEncryptedKeyShare + } + }) +} + +fn verify_encrypted_key_internal( + lockable_threshold_sig_data_store: &LockableThresholdSigDataStore, + key: &VetKdEncryptedKey, + args: &VetKdArgs, +) -> Result<(), VetKdKeyVerificationError> { + let encrypted_key = { + let encrypted_key_192_bytes = { + if key.encrypted_key.len() != EncryptedKey::BYTES { + return Err(VetKdKeyVerificationError::InvalidArgumentEncryptedKey); + } + let mut encrypted_key = [0u8; EncryptedKey::BYTES]; + encrypted_key.copy_from_slice(&key.encrypted_key); + encrypted_key + }; + ic_crypto_internal_bls12_381_vetkd::EncryptedKey::deserialize(encrypted_key_192_bytes) + .map_err(|e| match e { + ic_crypto_internal_bls12_381_vetkd::EncryptedKeyDeserializationError::InvalidEncryptedKey => VetKdKeyVerificationError::InvalidArgumentEncryptedKey, + })? + }; + + let master_public_key = { + let pub_coeffs_from_store = lockable_threshold_sig_data_store + .read() + .transcript_data(&args.ni_dkg_id) + .map(|data| data.public_coefficients().clone()) + .ok_or_else(|| { + VetKdKeyVerificationError::ThresholdSigDataNotFound( + ThresholdSigDataNotFoundError::ThresholdSigDataNotFound { + dkg_id: args.ni_dkg_id.clone(), + }, + ) + })?; + let public_key_bytes = match pub_coeffs_from_store { + PublicCoefficients::Bls12_381(pub_coeffs) => pub_coeffs + .coefficients + .iter() + .copied() + .next() + .ok_or_else(|| { + VetKdKeyVerificationError::InternalError(format!( + "public coefficients for NI-DKG ID {} are empty", + &args.ni_dkg_id + )) + })?, + }; + ic_crypto_internal_bls12_381_vetkd::G2Affine::deserialize(&public_key_bytes).map_err( + |_: PairingInvalidPoint| VetKdKeyVerificationError::InvalidArgumentMasterPublicKey, + )? + }; + + let transport_public_key = TransportPublicKey::deserialize(&args.encryption_public_key) + .map_err(|e| match e { + TransportPublicKeyDeserializationError::InvalidPublicKey => { + VetKdKeyVerificationError::InvalidArgumentEncryptionPublicKey + } + })?; + + match encrypted_key.is_valid( + &master_public_key, + &DerivationPath::new( + args.derivation_path.caller.as_slice(), + &args.derivation_path.derivation_path, + ), + &args.derivation_id, + &transport_public_key, + ) { + true => Ok(()), + false => Err(VetKdKeyVerificationError::VerificationError), + } +} + +fn log_err(error_option: Option<&T>) -> String { + if let Some(error) = error_option { + return format!("{}", error); + } + "none".to_string() +} + +pub fn log_ok_content(result: &Result) -> String { + if let Ok(content) = result { + return format!("{}", content); + } + "none".to_string() +} diff --git a/rs/interfaces/src/crypto/errors.rs b/rs/interfaces/src/crypto/errors.rs index e51872c6094..b187164feb6 100644 --- a/rs/interfaces/src/crypto/errors.rs +++ b/rs/interfaces/src/crypto/errors.rs @@ -432,10 +432,8 @@ impl ErrorReproducibility for VetKdKeyShareVerificationError { // to avoid defaults, which might be error-prone. // Upon addition of any new error this match has to be updated. - // VetKd key share verification does not depend on any local or private - // state and so is inherently replicated. match self { - Self::InvalidSignature => true, + Self::VerificationError(crypto_error) => crypto_error.is_reproducible(), } } } diff --git a/rs/protobuf/def/log/crypto_log_entry/v1/crypto_log_entry.proto b/rs/protobuf/def/log/crypto_log_entry/v1/crypto_log_entry.proto index e71c9f521bc..f9f2738ae0a 100644 --- a/rs/protobuf/def/log/crypto_log_entry/v1/crypto_log_entry.proto +++ b/rs/protobuf/def/log/crypto_log_entry/v1/crypto_log_entry.proto @@ -34,4 +34,8 @@ message CryptoLogEntry { google.protobuf.StringValue signature_shares = 26; google.protobuf.StringValue signature_inputs = 27; google.protobuf.UInt64Value log_id = 28; + google.protobuf.StringValue vetkd_args = 29; + google.protobuf.StringValue vetkd_key_share = 30; + google.protobuf.StringValue vetkd_key_shares = 31; + google.protobuf.StringValue vetkd_key = 32; } diff --git a/rs/protobuf/src/gen/log/log.crypto_log_entry.v1.rs b/rs/protobuf/src/gen/log/log.crypto_log_entry.v1.rs index d43066d9c89..556c14cb8db 100644 --- a/rs/protobuf/src/gen/log/log.crypto_log_entry.v1.rs +++ b/rs/protobuf/src/gen/log/log.crypto_log_entry.v1.rs @@ -85,4 +85,12 @@ pub struct CryptoLogEntry { #[prost(message, optional, tag = "28")] #[serde(skip_serializing_if = "Option::is_none")] pub log_id: ::core::option::Option, + #[prost(message, optional, tag = "29")] + pub vetkd_args: ::core::option::Option<::prost::alloc::string::String>, + #[prost(message, optional, tag = "30")] + pub vetkd_key_share: ::core::option::Option<::prost::alloc::string::String>, + #[prost(message, optional, tag = "31")] + pub vetkd_key_shares: ::core::option::Option<::prost::alloc::string::String>, + #[prost(message, optional, tag = "32")] + pub vetkd_key: ::core::option::Option<::prost::alloc::string::String>, } diff --git a/rs/types/types/src/crypto/hash.rs b/rs/types/types/src/crypto/hash.rs index 0241708c8fa..a2b0e5e8de3 100644 --- a/rs/types/types/src/crypto/hash.rs +++ b/rs/types/types/src/crypto/hash.rs @@ -19,7 +19,6 @@ use crate::consensus::{ use crate::crypto::canister_threshold_sig::idkg::{ IDkgDealing, IDkgDealingSupport, IDkgTranscript, SignedIDkgDealing, }; -use crate::crypto::vetkd::VetKdEncryptedKeyShareContent; use crate::crypto::{CryptoHash, CryptoHashOf, Signed}; use crate::messages::{HttpCanisterUpdate, MessageId, SignedRequestBytes}; use crate::signature::{ @@ -124,8 +123,6 @@ mod private { impl CryptoHashDomainSeal for SchnorrSigShare {} impl CryptoHashDomainSeal for VetKdKeyShare {} - impl CryptoHashDomainSeal for VetKdEncryptedKeyShareContent {} - impl CryptoHashDomainSeal for IDkgComplaintContent {} impl CryptoHashDomainSeal for Signed> {} @@ -393,12 +390,6 @@ impl CryptoHashDomain for VetKdKeyShare { } } -impl CryptoHashDomain for VetKdEncryptedKeyShareContent { - fn domain(&self) -> String { - DomainSeparator::VetKdEncryptedKeyShareContent.to_string() - } -} - impl CryptoHashDomain for IDkgComplaintContent { fn domain(&self) -> String { DomainSeparator::IDkgComplaintContent.to_string() diff --git a/rs/types/types/src/crypto/sign.rs b/rs/types/types/src/crypto/sign.rs index f0567a8296e..df8fe621833 100644 --- a/rs/types/types/src/crypto/sign.rs +++ b/rs/types/types/src/crypto/sign.rs @@ -10,6 +10,7 @@ use crate::consensus::{ NotarizationContent, RandomBeaconContent, RandomTapeContent, }; use crate::crypto::canister_threshold_sig::idkg::{IDkgDealing, SignedIDkgDealing}; +use crate::crypto::vetkd::VetKdEncryptedKeyShareContent; use crate::crypto::SignedBytesWithoutDomainSeparator; use crate::messages::{Delegation, MessageId, QueryResponseHash, WebAuthnEnvelope}; use std::convert::TryFrom; @@ -74,6 +75,7 @@ mod private { impl SignatureDomainSeal for RandomTapeContent {} impl SignatureDomainSeal for SignableMock {} impl SignatureDomainSeal for QueryResponseHash {} + impl SignatureDomainSeal for VetKdEncryptedKeyShareContent {} } impl SignatureDomain for CanisterHttpResponseMetadata { @@ -190,6 +192,12 @@ impl SignatureDomain for QueryResponseHash { } } +impl SignatureDomain for VetKdEncryptedKeyShareContent { + fn domain(&self) -> Vec { + domain_with_prepended_length(DomainSeparator::VetKdEncryptedKeyShareContent.as_str()) + } +} + // Returns a vector of bytes that contains the given domain // prepended with a single byte that holds the length of the domain. // This is the recommended format for non-empty domain separators, diff --git a/rs/types/types/src/crypto/vetkd.rs b/rs/types/types/src/crypto/vetkd.rs index 45032393436..a2dc725ef0d 100644 --- a/rs/types/types/src/crypto/vetkd.rs +++ b/rs/types/types/src/crypto/vetkd.rs @@ -1,9 +1,10 @@ use crate::crypto::impl_display_using_debug; use crate::crypto::threshold_sig::errors::threshold_sig_data_not_found_error::ThresholdSigDataNotFoundError; use crate::crypto::threshold_sig::ni_dkg::NiDkgId; +use crate::crypto::CryptoError; use crate::crypto::ExtendedDerivationPath; use crate::crypto::HexEncoding; -use crate::NodeId; +use crate::crypto::SignedBytesWithoutDomainSeparator; use serde::{Deserialize, Serialize}; use std::fmt; @@ -47,6 +48,12 @@ impl std::fmt::Debug for VetKdEncryptedKeyShareContent { } impl_display_using_debug!(VetKdEncryptedKeyShareContent); +impl SignedBytesWithoutDomainSeparator for VetKdEncryptedKeyShareContent { + fn as_signed_bytes_without_domain_separator(&self) -> Vec { + self.0.clone() + } +} + #[derive(Clone, Eq, PartialEq, Hash, Deserialize, Serialize)] pub struct VetKdEncryptedKeyShare { pub encrypted_key_share: VetKdEncryptedKeyShareContent, @@ -83,25 +90,43 @@ impl_display_using_debug!(VetKdEncryptedKey); #[derive(Clone, Eq, PartialEq, Debug)] pub enum VetKdKeyShareCreationError { ThresholdSigDataNotFound(ThresholdSigDataNotFoundError), - SecretKeyNotFound { dkg_id: NiDkgId, key_id: String }, KeyIdInstantiationError(String), + InternalError(String), + InvalidArgumentEncryptionPublicKey, + KeyShareSigningError(CryptoError), TransientInternalError(String), } impl_display_using_debug!(VetKdKeyShareCreationError); #[derive(Clone, Eq, PartialEq, Debug)] pub enum VetKdKeyShareVerificationError { - InvalidSignature, + VerificationError(CryptoError), } impl_display_using_debug!(VetKdKeyShareVerificationError); #[derive(Clone, Eq, PartialEq, Debug)] pub enum VetKdKeyShareCombinationError { - InvalidShares(Vec), - UnsatisfiedReconstructionThreshold { threshold: u32, share_count: usize }, + ThresholdSigDataNotFound(ThresholdSigDataNotFoundError), + InvalidArgumentMasterPublicKey, + InvalidArgumentEncryptionPublicKey, + InvalidArgumentEncryptedKeyShare, + IndividualPublicKeyComputationError(CryptoError), + CombinationError(String), + InternalError(String), + UnsatisfiedReconstructionThreshold { + threshold: usize, + share_count: usize, + }, } impl_display_using_debug!(VetKdKeyShareCombinationError); #[derive(Clone, Eq, PartialEq, Debug)] -pub enum VetKdKeyVerificationError {} +pub enum VetKdKeyVerificationError { + InvalidArgumentEncryptedKey, + ThresholdSigDataNotFound(ThresholdSigDataNotFoundError), + InternalError(String), + InvalidArgumentMasterPublicKey, + InvalidArgumentEncryptionPublicKey, + VerificationError, +} impl_display_using_debug!(VetKdKeyVerificationError); From de4505ad1ea99531bd26390e6bb1badcbe5484d9 Mon Sep 17 00:00:00 2001 From: Franz-Stefan Preiss Date: Tue, 28 Jan 2025 09:13:56 +0000 Subject: [PATCH 19/22] Use registry version from store --- rs/crypto/src/vetkd/mod.rs | 34 +++++++++++++++++++++++------- rs/interfaces/src/crypto/errors.rs | 6 ++++-- rs/types/types/src/crypto/vetkd.rs | 1 + 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/rs/crypto/src/vetkd/mod.rs b/rs/crypto/src/vetkd/mod.rs index 673edca16d2..730d53aab1c 100644 --- a/rs/crypto/src/vetkd/mod.rs +++ b/rs/crypto/src/vetkd/mod.rs @@ -81,7 +81,7 @@ impl VetKdProtocol for CryptoComponentImpl { &self, signer: NodeId, key_share: &VetKdEncryptedKeyShare, - _args: &VetKdArgs, + args: &VetKdArgs, ) -> Result<(), VetKdKeyShareVerificationError> { let log_id = get_log_id(&self.logger); let logger = new_logger!(&self.logger; @@ -96,10 +96,12 @@ impl VetKdProtocol for CryptoComponentImpl { ); let start_time = self.metrics.now(); let result = verify_encrypted_key_share_internal( + &self.lockable_threshold_sig_data_store, self.registry_client.as_ref(), &self.csp, key_share, signer, + args, ); self.metrics.observe_duration_seconds( MetricsDomain::VetKd, @@ -201,10 +203,14 @@ fn create_encrypted_key_share_internal( args: VetKdArgs, self_node_id: NodeId, ) -> Result { - let pub_coeffs_from_store = lockable_threshold_sig_data_store + let (pub_coeffs_from_store, registry_version_from_store) = lockable_threshold_sig_data_store .read() .transcript_data(&args.ni_dkg_id) - .map(|data| data.public_coefficients().clone()) + .map(|transcript_data| { + let pub_coeffs = transcript_data.public_coefficients().clone(); + let registry_version = transcript_data.registry_version(); + (pub_coeffs, registry_version) + }) .ok_or_else(|| { VetKdKeyShareCreationError::ThresholdSigDataNotFound( ThresholdSigDataNotFoundError::ThresholdSigDataNotFound { @@ -217,7 +223,7 @@ fn create_encrypted_key_share_internal( VetKdKeyShareCreationError::KeyIdInstantiationError(msg) } })?; - let master_public_key = match pub_coeffs_from_store { + let master_public_key = match &pub_coeffs_from_store { PublicCoefficients::Bls12_381(pub_coeffs) => pub_coeffs .coefficients .iter() @@ -246,9 +252,8 @@ fn create_encrypted_key_share_internal( registry, &encrypted_key_share, self_node_id, - // TODO(CRP-2670): Store registry version in ThresholdSigDataStore when loading transcript and then use it here instead get_latest_version // TODO(CRP-2666): Cleanup: Remove registry_version from BasicSigner::sign_basic API - registry.get_latest_version(), + registry_version_from_store, ) .map_err(VetKdKeyShareCreationError::KeyShareSigningError)?; @@ -280,11 +285,25 @@ fn vetkd_key_share_creation_error_from_vault_error( } fn verify_encrypted_key_share_internal( + lockable_threshold_sig_data_store: &LockableThresholdSigDataStore, registry: &dyn RegistryClient, csp_signer: &S, key_share: &VetKdEncryptedKeyShare, signer: NodeId, + args: &VetKdArgs, ) -> Result<(), VetKdKeyShareVerificationError> { + let registry_version_from_store = lockable_threshold_sig_data_store + .read() + .transcript_data(&args.ni_dkg_id) + .map(|transcript_data| transcript_data.registry_version()) + .ok_or_else(|| { + VetKdKeyShareVerificationError::ThresholdSigDataNotFound( + ThresholdSigDataNotFoundError::ThresholdSigDataNotFound { + dkg_id: args.ni_dkg_id.clone(), + }, + ) + })?; + let signature = BasicSigOf::new(BasicSig(key_share.node_signature.clone())); BasicSigVerifierInternal::verify_basic_sig( csp_signer, @@ -292,8 +311,7 @@ fn verify_encrypted_key_share_internal( &signature, &key_share.encrypted_key_share, signer, - // TODO(CRP-2670): Store registry version in ThresholdSigDataStore when loading transcript and then use it here - registry.get_latest_version(), + registry_version_from_store, ) .map_err(VetKdKeyShareVerificationError::VerificationError) } diff --git a/rs/interfaces/src/crypto/errors.rs b/rs/interfaces/src/crypto/errors.rs index b187164feb6..69d52033703 100644 --- a/rs/interfaces/src/crypto/errors.rs +++ b/rs/interfaces/src/crypto/errors.rs @@ -52,8 +52,8 @@ impl ErrorReproducibility for CryptoError { CryptoError::InconsistentAlgorithms { .. } => true, // true, as the set of supported algorithms is stable (bound to code version) CryptoError::AlgorithmNotSupported { .. } => true, - // false, as the result may change if the DKG transcript is reloaded. - CryptoError::ThresholdSigDataNotFound { .. } => false, + // true, as the result will not change upon retrying, unless the correct DKG transcript is loaded. + CryptoError::ThresholdSigDataNotFound { .. } => true, CryptoError::RegistryClient(registry_client_error) => { registry_client_error.is_reproducible() } @@ -434,6 +434,8 @@ impl ErrorReproducibility for VetKdKeyShareVerificationError { match self { Self::VerificationError(crypto_error) => crypto_error.is_reproducible(), + // true, as the result will not change upon retrying, unless the correct DKG transcript is loaded. + Self::ThresholdSigDataNotFound(_) => true, } } } diff --git a/rs/types/types/src/crypto/vetkd.rs b/rs/types/types/src/crypto/vetkd.rs index a2dc725ef0d..c646be47672 100644 --- a/rs/types/types/src/crypto/vetkd.rs +++ b/rs/types/types/src/crypto/vetkd.rs @@ -100,6 +100,7 @@ impl_display_using_debug!(VetKdKeyShareCreationError); #[derive(Clone, Eq, PartialEq, Debug)] pub enum VetKdKeyShareVerificationError { + ThresholdSigDataNotFound(ThresholdSigDataNotFoundError), VerificationError(CryptoError), } impl_display_using_debug!(VetKdKeyShareVerificationError); From 5df202ba34132d85e30ac7bedd1ebc4e496b48fa Mon Sep 17 00:00:00 2001 From: Franz-Stefan Preiss Date: Tue, 28 Jan 2025 09:33:00 +0000 Subject: [PATCH 20/22] test(crypto): CRP-2599 add smoke test for VetKdProtocol impl --- Cargo.lock | 1 + rs/crypto/BUILD.bazel | 1 + rs/crypto/Cargo.toml | 1 + rs/crypto/test_utils/ni-dkg/src/lib.rs | 19 ++- rs/crypto/tests/vetkd.rs | 218 +++++++++++++++++++++++++ 5 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 rs/crypto/tests/vetkd.rs diff --git a/Cargo.lock b/Cargo.lock index e246e6aad34..3a324eb193f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7037,6 +7037,7 @@ dependencies = [ "ic-test-utilities-time", "ic-types", "ic-types-test-utils", + "ic-vetkd-utils", "k256", "maplit", "mockall", diff --git a/rs/crypto/BUILD.bazel b/rs/crypto/BUILD.bazel index 3cdaa0b0151..72317f6bb02 100644 --- a/rs/crypto/BUILD.bazel +++ b/rs/crypto/BUILD.bazel @@ -59,6 +59,7 @@ MACRO_DEPENDENCIES = [ DEV_DEPENDENCIES = [ # Keep sorted. + "//packages/ic-vetkd-utils", "//rs/certification/test-utils", "//rs/crypto/ecdsa_secp256r1", "//rs/crypto/for_verification_only", diff --git a/rs/crypto/Cargo.toml b/rs/crypto/Cargo.toml index 46b7c841deb..ae2d4889ba2 100644 --- a/rs/crypto/Cargo.toml +++ b/rs/crypto/Cargo.toml @@ -41,6 +41,7 @@ ic-protobuf = { path = "../protobuf" } ic-registry-client-helpers = { path = "../registry/helpers" } ic-registry-keys = { path = "../registry/keys" } ic-types = { path = "../types/types" } +ic-vetkd-utils = { path = "../../packages/ic-vetkd-utils" } parking_lot = { workspace = true } rustls = { workspace = true } serde = { workspace = true } diff --git a/rs/crypto/test_utils/ni-dkg/src/lib.rs b/rs/crypto/test_utils/ni-dkg/src/lib.rs index 4cb89c34c68..3b3d82255a6 100644 --- a/rs/crypto/test_utils/ni-dkg/src/lib.rs +++ b/rs/crypto/test_utils/ni-dkg/src/lib.rs @@ -878,7 +878,11 @@ impl NiDkgTestEnvironment { let temp_crypto_builder = TempCryptoComponent::builder() .with_registry(Arc::clone(&self.registry) as Arc<_>) .with_node_id(node_id) - .with_keys(NodeKeysToGenerate::only_dkg_dealing_encryption_key()) + .with_keys(NodeKeysToGenerate { + generate_node_signing_keys: true, + generate_dkg_dealing_encryption_keys: true, + ..NodeKeysToGenerate::none() + }) .with_rng(ChaCha20Rng::from_seed(rng.gen())); let temp_crypto_builder = if use_remote_vault { temp_crypto_builder.with_remote_vault() @@ -891,6 +895,11 @@ impl NiDkgTestEnvironment { .expect("Failed to retrieve node public keys") .dkg_dealing_encryption_public_key .expect("missing dkg_dealing_encryption_pk"); + let node_signing_pubkey = temp_crypto + .current_node_public_keys() + .expect("Failed to retrieve node public keys") + .node_signing_public_key + .expect("missing dkg_dealing_encryption_pk"); self.crypto_components.insert(node_id, temp_crypto); // Insert DKG dealing encryption public key into registry @@ -901,6 +910,14 @@ impl NiDkgTestEnvironment { Some(dkg_dealing_encryption_pubkey), ) .expect("failed to add DKG dealing encryption key to registry"); + // Insert node signing public key into registry + self.registry_data + .add( + &make_crypto_node_key(node_id, KeyPurpose::NodeSigning), + ni_dkg_config.registry_version(), + Some(node_signing_pubkey), + ) + .expect("failed to add node signing public key to registry"); } /// Cleans up nodes whose IDs are no longer in use diff --git a/rs/crypto/tests/vetkd.rs b/rs/crypto/tests/vetkd.rs new file mode 100644 index 00000000000..e86aaee1c91 --- /dev/null +++ b/rs/crypto/tests/vetkd.rs @@ -0,0 +1,218 @@ +use ic_crypto_temp_crypto::CryptoComponentRng; +use ic_crypto_temp_crypto::TempCryptoComponentGeneric; +use ic_crypto_test_utils::crypto_for; +use ic_crypto_test_utils_ni_dkg::{ + run_ni_dkg_and_create_single_transcript, NiDkgTestEnvironment, RandomNiDkgConfig, +}; +use ic_crypto_test_utils_reproducible_rng::reproducible_rng; +use ic_interfaces::crypto::VetKdProtocol; +use ic_interfaces::crypto::{LoadTranscriptResult, NiDkgAlgorithm}; +use ic_types::crypto::canister_threshold_sig::MasterPublicKey; +use ic_types::crypto::threshold_sig::ni_dkg::config::NiDkgConfig; +use ic_types::crypto::threshold_sig::ni_dkg::{NiDkgId, NiDkgTranscript}; +use ic_types::crypto::threshold_sig::ThresholdSigPublicKey; +use ic_types::crypto::vetkd::VetKdArgs; +use ic_types::crypto::vetkd::VetKdEncryptedKey; +use ic_types::crypto::vetkd::VetKdEncryptedKeyShare; +use ic_types::crypto::AlgorithmId; +use ic_types::crypto::ExtendedDerivationPath; +use ic_types::{NodeId, NumberOfNodes}; +use ic_types_test_utils::ids::canister_test_id; +use rand::prelude::*; +use rand_chacha::ChaCha20Rng; +use std::collections::{BTreeMap, BTreeSet}; +use std::convert::TryFrom; + +#[test] +fn should_consistently_derive_the_same_vetkey_given_sufficient_shares() { + let rng = &mut reproducible_rng(); + let subnet_size = rng.gen_range(1..7); + let (config, dkg_id, crypto_components) = setup_with_random_ni_dkg_config(subnet_size, rng); + + let transcript = run_ni_dkg_and_load_transcript_for_receivers(&config, &crypto_components); + + let derivation_path = ExtendedDerivationPath { + caller: canister_test_id(234).get(), + derivation_path: vec![b"some".to_vec(), b"derivation".to_vec(), b"path".to_vec()], + }; + let derived_public_key = ic_crypto_utils_canister_threshold_sig::derive_vetkd_public_key( + &MasterPublicKey { + algorithm_id: AlgorithmId::ThresBls12_381, + public_key: ThresholdSigPublicKey::try_from(&transcript) + .expect("invalid transcript") + .into_bytes() + .to_vec(), + }, + &derivation_path, + ) + .expect("failed to compute derived public key"); + let transport_secret_key = + ic_vetkd_utils::TransportSecretKey::from_seed(rng.gen::<[u8; 32]>().to_vec()) + .expect("failed to create transport secret key"); + let vetkd_args = VetKdArgs { + ni_dkg_id: dkg_id, + derivation_path, + derivation_id: b"some-derivation-id".to_vec(), + encryption_public_key: transport_secret_key.public_key(), + }; + + let mut expected_decrypted_key: Option> = None; + for _ in 1..=3 { + let encrypted_key = create_key_shares_and_verify_and_combine( + KeyShareCreatorsAndCombiner { + creators: n_random_nodes_in( + config.receivers().get(), + config.threshold().get(), + rng, + ), + combiner: random_node_in(config.receivers().get(), rng), + }, + &vetkd_args, + &crypto_components, + ); + + let random_verifier = random_node_in(config.receivers().get(), rng); + assert_eq!( + crypto_for(random_verifier, &crypto_components) + .verify_encrypted_key(&encrypted_key, &vetkd_args), + Ok(()) + ); + + let decrypted_key = transport_secret_key + .decrypt( + &encrypted_key.encrypted_key, + &derived_public_key, + &vetkd_args.derivation_id, + ) + .expect("failed to decrypt vetKey"); + + if let Some(expected_decrypted_key) = &expected_decrypted_key { + assert_eq!(&decrypted_key, expected_decrypted_key); + } else { + expected_decrypted_key = Some(decrypted_key); + } + } +} + +fn setup_with_random_ni_dkg_config( + subnet_size: usize, + rng: &mut R, +) -> ( + NiDkgConfig, + NiDkgId, + BTreeMap>, +) { + let config = RandomNiDkgConfig::builder() + .subnet_size(subnet_size) + .build(rng) + .into_config(); + let dkg_id = config.dkg_id().clone(); + let crypto_components = + NiDkgTestEnvironment::new_for_config_with_remote_vault(&config, rng).crypto_components; + (config, dkg_id, crypto_components) +} + +fn create_key_shares_and_verify_and_combine( + creators_and_combiner: KeyShareCreatorsAndCombiner, + vetkd_args: &VetKdArgs, + crypto_components: &BTreeMap>, +) -> VetKdEncryptedKey { + let key_shares = create_and_verify_key_shares_for_each( + &creators_and_combiner.creators, + vetkd_args, + crypto_components, + ); + crypto_for(creators_and_combiner.combiner, crypto_components) + .combine_encrypted_key_shares(&key_shares, &vetkd_args) + .expect("failed to combine signature shares") +} + +fn create_and_verify_key_shares_for_each( + key_share_creators: &[NodeId], + vetkd_args: &VetKdArgs, + crypto_components: &BTreeMap>, +) -> BTreeMap { + key_share_creators + .iter() + .map(|creator| { + let crypto = crypto_for(*creator, crypto_components); + let key_share = crypto + .create_encrypted_key_share(vetkd_args.clone()) + .unwrap_or_else(|e| { + panic!( + "vetKD encrypted key share creation by node {:?} failed: {}", + creator, e + ) + }); + assert_eq!( + crypto.verify_encrypted_key_share(*creator, &key_share, vetkd_args), + Ok(()) + ); + (*creator, key_share) + }) + .collect() +} + +#[derive(Clone, Debug)] +struct KeyShareCreatorsAndCombiner { + creators: Vec, + combiner: NodeId, +} + +///////////////////////////////////////////////////////////////////////////////// +// The following helper functions where copied from threshold_sigs_with_ni_dkg.rs +///////////////////////////////////////////////////////////////////////////////// + +fn run_ni_dkg_and_load_transcript_for_receivers( + config: &NiDkgConfig, + crypto_components: &BTreeMap>, +) -> NiDkgTranscript { + let transcript = run_ni_dkg_and_create_single_transcript(config, crypto_components); + load_transcript_for_receivers_expecting_status( + config, + &transcript, + crypto_components, + Some(LoadTranscriptResult::SigningKeyAvailable), + ); + transcript +} + +fn load_transcript_for_receivers_expecting_status( + config: &NiDkgConfig, + transcript: &NiDkgTranscript, + crypto_components: &BTreeMap>, + expected_status: Option, +) { + for node_id in config.receivers().get() { + let result = crypto_for(*node_id, crypto_components).load_transcript(transcript); + + if result.is_err() { + panic!( + "failed to load transcript {} for node {}: {}", + transcript, + *node_id, + result.unwrap_err() + ); + } + + if let Some(expected_status) = expected_status { + let result = result.unwrap(); + assert_eq!(result, expected_status); + } + } +} + +fn random_node_in(nodes: &BTreeSet, rng: &mut R) -> NodeId { + *nodes.iter().choose(rng).expect("nodes empty") +} + +fn n_random_nodes_in( + nodes: &BTreeSet, + n: NumberOfNodes, + rng: &mut R, +) -> Vec { + let n_usize = usize::try_from(n.get()).expect("conversion to usize failed"); + let chosen = nodes.iter().copied().choose_multiple(rng, n_usize); + assert_eq!(chosen.len(), n_usize); + chosen +} From c2bd7a6cfeab0d9b4d4e0e8062bef862a5c3bfde Mon Sep 17 00:00:00 2001 From: Franz-Stefan Preiss Date: Tue, 28 Jan 2025 09:40:01 +0000 Subject: [PATCH 21/22] Clippy --- rs/crypto/tests/vetkd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/crypto/tests/vetkd.rs b/rs/crypto/tests/vetkd.rs index e86aaee1c91..600b397b49b 100644 --- a/rs/crypto/tests/vetkd.rs +++ b/rs/crypto/tests/vetkd.rs @@ -123,7 +123,7 @@ fn create_key_shares_and_verify_and_combine( crypto_components, ); crypto_for(creators_and_combiner.combiner, crypto_components) - .combine_encrypted_key_shares(&key_shares, &vetkd_args) + .combine_encrypted_key_shares(&key_shares, vetkd_args) .expect("failed to combine signature shares") } From e82aa0ff9edb58900b2be525993307bad53e19f7 Mon Sep 17 00:00:00 2001 From: Franz-Stefan Preiss Date: Fri, 31 Jan 2025 13:41:21 +0000 Subject: [PATCH 22/22] Revert unintended change --- rs/interfaces/src/crypto/errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/interfaces/src/crypto/errors.rs b/rs/interfaces/src/crypto/errors.rs index 1ab25472121..638b2d33202 100644 --- a/rs/interfaces/src/crypto/errors.rs +++ b/rs/interfaces/src/crypto/errors.rs @@ -52,8 +52,8 @@ impl ErrorReproducibility for CryptoError { CryptoError::InconsistentAlgorithms { .. } => true, // true, as the set of supported algorithms is stable (bound to code version) CryptoError::AlgorithmNotSupported { .. } => true, - // true, as the result will not change upon retrying, unless the correct DKG transcript is loaded. - CryptoError::ThresholdSigDataNotFound { .. } => true, + // false, as the result may change if the DKG transcript is reloaded. + CryptoError::ThresholdSigDataNotFound { .. } => false, CryptoError::RegistryClient(registry_client_error) => { registry_client_error.is_reproducible() }