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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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 bf0b9dc94d0d193a34b2614fa5f5fe81a468cc8e Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Thu, 23 Jan 2025 16:56:55 -0500 Subject: [PATCH 18/19] Address review comments --- .../crypto_lib/bls12_381/vetkd/tests/tests.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 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 21dee15f977..b680f2407a3 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 @@ -384,8 +384,7 @@ fn test_protocol_execution() { // 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) { + if !shares.iter().map(|(i, _eks)| *i).any(|x| x == idx) { break idx as usize; } }; @@ -408,7 +407,7 @@ fn test_protocol_execution() { 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) { + if shares.iter().map(|(i, _eks)| *i).any(|x| x == idx) { break idx as usize; } }; @@ -431,16 +430,25 @@ fn test_protocol_execution() { shares.append(&mut random_subset(rng, &node_info_wrong_did, 4)); shares.shuffle(rng); - let combined = proto.combine_valid(&shares).expect("Combination worked"); + let combined = proto.combine_valid(&shares); + assert!(combined.is_ok(), "Combination succeeded"); let k = setup .transport_sk - .decrypt(&combined, &proto.derived_pk, &proto.did) + .decrypt( + &combined.expect("Already checked"), + &proto.derived_pk, + &proto.did, + ) .expect("Decryption failed"); assert_eq!(k, vetkey); } + // Here check that if we add a random duplicate (valid) share to the + // list, combine_valid succeeds iff the duplicated share does not + // appear in the first `threshold` many shares, since we only look + // at that many for rec_threshold in threshold..nodes { let mut shares = random_subset(rng, &node_info, rec_threshold); From 23ddf08affa5017356ae65830d40b67fa5a6410a Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 24 Jan 2025 10:20:33 -0500 Subject: [PATCH 19/19] Fix assert message --- rs/crypto/internal/crypto_lib/bls12_381/vetkd/tests/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b680f2407a3..0f36058d6c0 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 @@ -431,7 +431,7 @@ fn test_protocol_execution() { shares.shuffle(rng); let combined = proto.combine_valid(&shares); - assert!(combined.is_ok(), "Combination succeeded"); + assert!(combined.is_ok(), "Combination unexpectedly failed"); let k = setup .transport_sk