From 2d6c0b8cab77383c7dde998715dc7d171c2330c4 Mon Sep 17 00:00:00 2001 From: yihuang Date: Sat, 25 Jul 2020 03:51:34 +0800 Subject: [PATCH] Problem: didn't verify secrets against public key in DirectPath Solution: - verify decreted secret against public key - also verify leaf keypackage against leaf secret --- chain-tx-enclave-next/mls/Cargo.toml | 4 +- chain-tx-enclave-next/mls/src/ciphersuite.rs | 9 +- chain-tx-enclave-next/mls/src/group.rs | 324 +++++++++++++------ chain-tx-enclave-next/mls/src/key.rs | 1 - chain-tx-enclave-next/mls/src/keypackage.rs | 63 +++- chain-tx-enclave-next/mls/src/message.rs | 12 +- chain-tx-enclave-next/mls/src/tree.rs | 191 +++++++---- 7 files changed, 436 insertions(+), 168 deletions(-) diff --git a/chain-tx-enclave-next/mls/Cargo.toml b/chain-tx-enclave-next/mls/Cargo.toml index d7703903a..227757bdf 100644 --- a/chain-tx-enclave-next/mls/Cargo.toml +++ b/chain-tx-enclave-next/mls/Cargo.toml @@ -14,13 +14,13 @@ x509-parser = "0.8.0-beta4" secrecy = "0.7.0" sha2 = "0.9" hkdf = { version = "0.9", features = ["std"] } -hpke = { version = "0.1.8", default-features = false, features = ["p256"] } +hpke = { version = "0.1.8", default-features = false, features = ["p256", "std"] } aead = "0.3" rand = "0.7" chrono="0.4.13" ra-client = { path = "../enclave-ra/ra-client" } subtle = "2.2.3" -chain-util = { path = "../../chain-util", default-features = false } +chain-util = { path = "../../chain-util" } [dev-dependencies] chrono = "0.4" diff --git a/chain-tx-enclave-next/mls/src/ciphersuite.rs b/chain-tx-enclave-next/mls/src/ciphersuite.rs index 8e5059d47..7ae7d1062 100644 --- a/chain-tx-enclave-next/mls/src/ciphersuite.rs +++ b/chain-tx-enclave-next/mls/src/ciphersuite.rs @@ -386,8 +386,7 @@ impl CipherSuite { CipherSuite::MLS10_128_DHKEMP256_AES128GCM_SHA256_P256 => { let encapped_key = EncappedKey::< ::Kex, - >::unmarshal(&ct.kem_output) - .expect("valid encapped key"); + >::unmarshal(&ct.kem_output)?; let mut context = hpke::setup_receiver::< AesGcm128, hpke::kdf::HkdfSha256, @@ -402,11 +401,9 @@ impl CipherSuite { let payload_len = ct.ciphertext.len(); let mut payload = ct.ciphertext[0..payload_len - 16].to_vec(); let tag_bytes = &ct.ciphertext[payload_len - 16..payload_len]; - let tag = AeadTag::::unmarshal(tag_bytes).expect("valid tag"); + let tag = AeadTag::::unmarshal(tag_bytes)?; - context - .open(&mut payload, aad, &tag) - .expect("decryption failed"); + context.open(&mut payload, aad, &tag)?; Ok(payload) } } diff --git a/chain-tx-enclave-next/mls/src/group.rs b/chain-tx-enclave-next/mls/src/group.rs index e6abfc368..147133edf 100644 --- a/chain-tx-enclave-next/mls/src/group.rs +++ b/chain-tx-enclave-next/mls/src/group.rs @@ -1,11 +1,13 @@ +use std::collections::BTreeMap; use std::convert::TryFrom; use crate::ciphersuite::*; use crate::extensions as ext; -use crate::key::IdentityPublicKey; -use crate::keypackage::Timespec; -use crate::keypackage::PROTOCOL_VERSION_MLS10; -use crate::keypackage::{self as kp, KeyPackage, KeyPackageSecret}; +use crate::key::{HPKEPrivateKey, IdentityPrivateKey, IdentityPublicKey}; +use crate::keypackage::{ + self as kp, verify_keypackage_and_secrets, KeyPackage, KeyPackageSecret, Timespec, + PROTOCOL_VERSION_MLS10, +}; use crate::message::*; use crate::secrets::*; use crate::tree::*; @@ -14,7 +16,6 @@ use crate::utils::{ encode_vec_option_u32, encode_vec_u8_u16, encode_vec_u8_u8, read_vec_option_u32, read_vec_u8_u16, read_vec_u8_u8, }; -use hkdf::InvalidLength; use ra_client::AttestedCertVerifier; use rustls::internal::msgs::codec::{self, Codec, Reader}; use secrecy::{ExposeSecret, SecretVec}; @@ -29,8 +30,13 @@ pub struct GroupAux { pub tree_secret: TreeSecret, pub secrets: EpochSecrets, pub kp_secret: KeyPackageSecret, - // record the new secret when waiting for self update commit - pub kp_secret_pending: Option, + + /// record the pending credential secret for self-update proposals. + /// inserted when commit self update proposal, removed when processing the commit. + pub pending_updates: BTreeMap, + /// record the new init private key generate in commit. + /// inserted when commit proposals, removed when processing self commit. + pub pending_commit: BTreeMap, } impl GroupAux { @@ -47,7 +53,8 @@ impl GroupAux { tree_secret: TreeSecret::empty(cs), secrets, kp_secret, - kp_secret_pending: None, + pending_updates: BTreeMap::new(), + pending_commit: BTreeMap::new(), } } @@ -102,14 +109,17 @@ impl GroupAux { key_package: keypackage, })), }; - self.kp_secret_pending = Some(secret); let to_be_signed = MLSPlaintextTBS { context: self.context.clone(), content: content.clone(), } .get_encoding(); let signature = self.kp_secret.credential_private_key.sign(&to_be_signed); - MLSPlaintext { content, signature } + let proposal = MLSPlaintext { content, signature }; + let proposal_id = ProposalId(self.tree.cs.hash(&proposal.get_encoding())); + self.pending_updates + .insert(proposal_id, secret.credential_private_key); + proposal } /// Generate and sign remove proposal @@ -175,6 +185,7 @@ impl GroupAux { content: plain.clone(), } .get_encoding(); + let signature = self.kp_secret.credential_private_key.sign(&to_be_signed); MLSPlaintext { content: plain.clone(), @@ -235,16 +246,17 @@ impl GroupAux { } } + /// Generate commit message for proposals. fn do_commit_proposals( - &mut self, + &self, proposals: &[MLSPlaintext], init_genesis: bool, - ) -> (MLSPlaintext, Welcome) { + ) -> (MLSPlaintext, Welcome, Option) { // split proposals by types let mut add_proposals_ids: Vec = Vec::new(); let mut additions: Vec = Vec::new(); let mut update_proposals_ids: Vec = Vec::new(); - let mut updates: Vec<(LeafSize, Update)> = Vec::new(); + let mut updates: Vec<(LeafSize, Update, ProposalId)> = Vec::new(); let mut remove_proposals_ids: Vec = Vec::new(); let mut removes: Vec = Vec::new(); for p in proposals.iter() { @@ -255,8 +267,12 @@ impl GroupAux { additions.push(add.clone()); } ContentType::Proposal(Proposal::Update(update)) => { - update_proposals_ids.push(proposal_id); - updates.push((LeafSize(p.content.sender.sender), update.clone())); + update_proposals_ids.push(proposal_id.clone()); + updates.push(( + LeafSize(p.content.sender.sender), + update.clone(), + proposal_id, + )); } ContentType::Proposal(Proposal::Remove(remove)) => { remove_proposals_ids.push(proposal_id); @@ -265,37 +281,48 @@ impl GroupAux { _ => panic!("invalid proposal message type"), } } + let mut updated_tree = self.tree.clone(); let positions = updated_tree.update(&additions, &updates, &removes); + let self_update_proposal = updates + .iter() + .find(|(sender, _, _)| *sender == self.tree.my_pos) + .map(|(_, _, proposal_id)| proposal_id); + let credential_private_key = if let Some(proposal_id) = self_update_proposal { + self.pending_updates + .get(&proposal_id) + .expect("self update proposal has set pending secret") + } else { + &self.kp_secret.credential_private_key + }; + // pathRequired = isGenesisInit || haveUpdateOrRemove || haveNoProposalsAtAll let should_populate_path = init_genesis || proposals.is_empty() || !updates.is_empty() || !removes.is_empty(); - let (path, tree_secret) = if should_populate_path { - // update init key - self.kp_secret - .update_init_key(updated_tree.get_my_package_mut()); + let (path, tree_secret, init_private_key) = if should_populate_path { + // new init key + let init_private_key = updated_tree.get_my_package_mut().update_init_key(); // update path secrets - let (path_nodes, parent_hash, tree_secret) = updated_tree.evolve( - &self.context.get_encoding(), - &self.kp_secret.init_private_key.marshal(), - ); + let (path_nodes, parent_hash, tree_secret) = + updated_tree.evolve(&self.context.get_encoding(), &init_private_key.marshal()); let kp = updated_tree.get_my_package_mut(); // update keypackage's parent_hash extension kp.payload.put_extension(&ext::ParentHashExt(parent_hash)); - self.kp_secret.update_signature(kp); + kp.update_signature(credential_private_key); ( Some(DirectPath { leaf_key_package: kp.clone(), nodes: path_nodes, }), tree_secret, + Some(init_private_key), ) } else { - (None, TreeSecret::empty(self.tree.cs)) + (None, TreeSecret::empty(self.tree.cs), None) }; let commit = Commit { @@ -348,17 +375,32 @@ impl GroupAux { interim_transcript_hash, positions, ), + init_private_key, ) } /// commit proposals pub fn commit_proposals(&mut self, proposals: &[MLSPlaintext]) -> (MLSPlaintext, Welcome) { - self.do_commit_proposals(proposals, false) + let (msg, welcome, init_private_key) = self.do_commit_proposals(proposals, false); + if let Some(init_private_key) = init_private_key { + self.pending_commit.insert( + ProposalId(self.tree.cs.hash(&msg.get_encoding())), + init_private_key, + ); + } + (msg, welcome) } /// commit the proposals in genesis pub fn init_commit(&mut self, proposals: &[MLSPlaintext]) -> (MLSPlaintext, Welcome) { - self.do_commit_proposals(proposals, true) + let (msg, welcome, init_private_key) = self.do_commit_proposals(proposals, true); + if let Some(init_private_key) = init_private_key { + self.pending_commit.insert( + ProposalId(self.tree.cs.hash(&msg.get_encoding())), + init_private_key, + ); + } + (msg, welcome) } fn verify_msg_signature( @@ -376,49 +418,6 @@ impl GroupAux { .map_err(ProcessCommitError::MsgSignatureVerifyFailed) } - /// apply proposals and commit - fn apply_commit(&mut self, commit: &CommitContent) -> Result<(), ProcessCommitError> { - let ctx = self.context.get_encoding(); - let init_private_key = &self.kp_secret.init_private_key; - - // check path populating condition - let should_populate_path = - (commit.additions.is_empty() && commit.updates.is_empty() && commit.removes.is_empty()) - || !commit.updates.is_empty() - || !commit.removes.is_empty(); - if should_populate_path && commit.commit.path.is_none() { - return Err(ProcessCommitError::CommitPathNotPopulated); - } - - self.tree - .update(&commit.additions, &commit.updates, &commit.removes); - // "If the path value is populated: Process the path value..." - if let Some(path) = &commit.commit.path { - let leaf_parent_hash = self.tree.merge(commit.sender, &path.nodes); - self.tree_secret.apply_path_secrets( - commit.sender, - &self.tree, - &ctx, - &path.nodes, - init_private_key, - )?; - // Verify that the KeyPackage has a `parent_hash` extension and that its value - // matches the new parent of the sender's leaf node. - let ext = path - .leaf_key_package - .payload - .find_extension::()?; - if !bool::from(ext.0.ct_eq(&leaf_parent_hash)) { - return Err(ProcessCommitError::LeafParentHashDontMatch); - } - - self.tree - .set_package(commit.sender, path.leaf_key_package.clone()); - } - - Ok(()) - } - pub fn process_commit( &mut self, commit: MLSPlaintext, @@ -443,7 +442,87 @@ impl GroupAux { let commit_content = CommitContent::new(self.tree.cs, &commit, proposals) .map_err(|_| ProcessCommitError::CommitError)?; - self.apply_commit(&commit_content)?; + // update credential_private_key for self updating proposal + let self_update_proposal = commit_content + .updates + .iter() + .find(|(sender, _, _)| *sender == self.tree.my_pos); + let credential_private_key = if let Some((_, _, proposal_id)) = self_update_proposal { + // apply the pending credential_private_key + self.pending_updates + .get(proposal_id) + .ok_or(ProcessCommitError::PendingCredentialPrivateKeyNotFound)? + } else { + &self.kp_secret.credential_private_key + }; + + let commit_id = ProposalId(self.tree.cs.hash(&commit.get_encoding())); + let init_private_key = + if commit_content.commit.path.is_some() && commit_content.sender == self.tree.my_pos { + // apply pending `init_private_key` if I'm the committer + self.pending_commit + .get(&commit_id) + .ok_or(ProcessCommitError::PendingInitPrivateKeyNotFound)? + } else { + &self.kp_secret.init_private_key + }; + + // verify the leaf key package in commit path + if let Some(path) = &commit_content.commit.path { + if commit_content.sender == self.tree.my_pos { + verify_keypackage_and_secrets( + &path.leaf_key_package, + init_private_key, + credential_private_key, + ra_verifier, + now, + )?; + } else { + path.leaf_key_package.verify(ra_verifier, now)?; + } + } + + // check path populating condition + let should_populate_path = (commit_content.additions.is_empty() + && commit_content.updates.is_empty() + && commit_content.removes.is_empty()) + || !commit_content.updates.is_empty() + || !commit_content.removes.is_empty(); + if should_populate_path && commit_content.commit.path.is_none() { + return Err(ProcessCommitError::CommitPathNotPopulated); + } + + let mut tree = self.tree.clone(); + tree.update( + &commit_content.additions, + &commit_content.updates, + &commit_content.removes, + ); + // "If the path value is populated: Process the path value..." + let tree_diff = if let Some(path) = &commit_content.commit.path { + let leaf_parent_hash = tree.merge(commit_content.sender, &path.nodes); + let tree_diff = self.tree_secret.apply_path_secrets( + commit_content.sender, + &tree, + &self.context.get_encoding(), + &path.nodes, + &init_private_key, + )?; + // Verify that the KeyPackage has a `parent_hash` extension and that its value + // matches the new parent of the sender's leaf node. + let ext = path + .leaf_key_package + .payload + .find_extension::()?; + if !bool::from(ext.0.ct_eq(&leaf_parent_hash)) { + return Err(ProcessCommitError::LeafParentHashDontMatch); + } + + tree.set_package(commit_content.sender, path.leaf_key_package.clone()); + Some(tree_diff) + } else { + None + }; // "Update the new GroupContext's confirmed and interim transcript hashes using the new Commit." let confirmed_transcript_hash = self.get_init_confirmed_transcript_hash( @@ -460,18 +539,18 @@ impl GroupAux { let updated_group_context = GroupContext { epoch: self.context.epoch + 1, - tree_hash: self.tree.compute_tree_hash(), + tree_hash: tree.compute_tree_hash(), confirmed_transcript_hash, ..self.context.clone() }; - let updated_group_context_hash = self.tree.cs.hash(&updated_group_context.get_encoding()); + let updated_group_context_hash = tree.cs.hash(&updated_group_context.get_encoding()); // "Use the commit_secret, the provisional GroupContext, // and the init secret from the previous epoch to compute the epoch secret and derived secrets for the new epoch." let epoch_secrets = self.secrets.generate_new_epoch_secrets( &self.tree_secret.update_secret, updated_group_context_hash, - self.tree.leaf_len(), + tree.leaf_len(), ); let confirmation_computed = epoch_secrets.compute_confirmation(&updated_group_context.confirmed_transcript_hash); @@ -486,30 +565,31 @@ impl GroupAux { return Err(ProcessCommitError::GroupInfoIntegrityError); } - // update kp_secret for self updating proposal - let self_update_proposal = commit_content - .updates - .iter() - .filter_map(|(sender, update)| { - if *sender == self.tree.my_pos { - Some(&update.key_package) - } else { - None - } - }) - .next(); - if self_update_proposal.is_some() { - // apply the kp_secret_pending - self.kp_secret.credential_private_key = self - .kp_secret_pending - .take() - .ok_or(ProcessCommitError::CommitError)? - .credential_private_key; - } - // "If the above checks are successful, consider the updated GroupContext object as the current state of the group." self.context = updated_group_context; self.secrets = epoch_secrets; + self.tree = tree; + if let Some(diff) = tree_diff { + self.tree_secret.apply_tree_diff(diff); + } + // set pending credential_private_key + if let Some((_, _, proposal_id)) = self_update_proposal { + self.kp_secret.credential_private_key = self + .pending_updates + .remove(proposal_id) + .expect("impossible, checked above"); + } + + // set pending init_private_key + if commit_content.commit.path.is_some() && commit_content.sender == self.tree.my_pos { + self.kp_secret.init_private_key = self + .pending_commit + .remove(&commit_id) + .expect("impossible, checked above"); + } + // clear all the pending secrets after processing commit + self.pending_commit.clear(); + self.pending_updates.clear(); Ok(()) } @@ -660,7 +740,8 @@ impl GroupAux { tree_secret: TreeSecret::empty(cs), kp_secret, secrets, - kp_secret_pending: None, + pending_updates: BTreeMap::new(), + pending_commit: BTreeMap::new(), }; // * "Verify the confirmation MAC in the GroupInfo using the derived confirmation key and the confirmed_transcript_hash from the GroupInfo." let confirmation = group @@ -843,8 +924,14 @@ pub enum ProcessCommitError { MsgSignatureVerifyFailed(ring::error::Unspecified), #[error("commit path is not populated")] CommitPathNotPopulated, + #[error("hkdf error")] + HkdfError(#[from] hkdf::InvalidLength), #[error("hpke error")] - HpkeError(#[from] InvalidLength), + HpkeError(#[from] hpke::HpkeError), + #[error("pending init_private_key not found")] + PendingInitPrivateKeyNotFound, + #[error("pending credential private key not found")] + PendingCredentialPrivateKeyNotFound, } #[derive(thiserror::Error, Debug)] @@ -1010,6 +1097,7 @@ mod test { Update { key_package: to_be_updated.clone(), }, + ProposalId(vec![]), )], &[], ); @@ -1130,4 +1218,46 @@ mod test { None ); } + + #[test] + fn test_invalid_commit() { + // process invalid commit don't end up partial mutated state. + let (member1, member1_secret) = get_fake_keypackage(); + let (member2, member2_secret) = get_fake_keypackage(); + let (member3, _member3_secret) = get_fake_keypackage(); + let ra_verifier = MockVerifier {}; + + // add member2 in genesis + let (mut member1_group, proposals, commit, welcome) = + GroupAux::init_group(member1, member1_secret, &[member2.clone()], &ra_verifier, 0) + .expect("group init"); + + // after commit/welcome get confirmed + member1_group + .process_commit(commit, &proposals, &ra_verifier, 0) + .expect("commit ok"); + let mut member2_group = + GroupAux::init_group_from_welcome(member2, member2_secret, welcome, &ra_verifier, 0) + .expect("group init from welcome"); + + // they should get to the same context + assert_eq!(&member1_group.context, &member2_group.context); + + // mess up keypackage of member2 + member1_group.tree.set_package(LeafSize(1), member3); + + // add member3 + let (commit, _welcome) = member1_group.commit_proposals(&[]); + + // after commit/welcome get confirmed + member1_group + .process_commit(commit.clone(), &[], &ra_verifier, 0) + .expect("commit ok"); + + // decryption error + assert!(matches!( + member2_group.process_commit(commit, &[], &ra_verifier, 0), + Err(ProcessCommitError::HpkeError(_)) + )); + } } diff --git a/chain-tx-enclave-next/mls/src/key.rs b/chain-tx-enclave-next/mls/src/key.rs index 18a500e87..0bd1e5011 100644 --- a/chain-tx-enclave-next/mls/src/key.rs +++ b/chain-tx-enclave-next/mls/src/key.rs @@ -98,7 +98,6 @@ impl Codec for HPKEPublicKey { /// p-256 private key /// used for obtaining the initial sealed secrets (HPKE) -#[derive(Clone)] pub struct HPKEPrivateKey(::PrivateKey); impl HPKEPrivateKey { diff --git a/chain-tx-enclave-next/mls/src/keypackage.rs b/chain-tx-enclave-next/mls/src/keypackage.rs index 0e66de2c4..3beb9923c 100644 --- a/chain-tx-enclave-next/mls/src/keypackage.rs +++ b/chain-tx-enclave-next/mls/src/keypackage.rs @@ -7,6 +7,7 @@ use ra_client::{AttestedCertVerifier, CertVerifyResult, EnclaveCertVerifierError #[cfg(target_env = "sgx")] use ra_enclave::{Certificate, EnclaveRaContext, EnclaveRaContextError}; use rustls::internal::msgs::codec::{self, Codec, Reader}; +use subtle::ConstantTimeEq; #[cfg(target_env = "sgx")] use x509_parser::{parse_x509_der, x509}; @@ -218,6 +219,18 @@ impl KeyPackage { .map_err(Error::SignatureVerifyError)?; Ok(info) } + + /// re-sign payload + pub fn update_signature(&mut self, private_key: &IdentityPrivateKey) { + self.signature = private_key.sign(&self.payload.get_encoding()); + } + + /// re-generate init key + pub fn update_init_key(&mut self) -> HPKEPrivateKey { + let (hpke_secret, hpke_public) = HPKEPrivateKey::generate(); + self.payload.init_key = hpke_public; + hpke_secret + } } pub struct KeyPackageSecret { @@ -275,9 +288,7 @@ impl KeyPackageSecret { /// re-sign payload pub fn update_signature(&self, keypackage: &mut KeyPackage) { - keypackage.signature = self - .credential_private_key - .sign(&keypackage.payload.get_encoding()); + keypackage.update_signature(&self.credential_private_key) } /// re-generate init key @@ -286,6 +297,50 @@ impl KeyPackageSecret { keypackage.payload.init_key = hpke_public; self.init_private_key = hpke_secret; } + + /// Verify key package secret and keypackage + pub fn verify( + &self, + kp: &KeyPackage, + ra_verifier: &impl AttestedCertVerifier, + now: Timespec, + ) -> Result { + verify_keypackage_and_secrets( + kp, + &self.init_private_key, + &self.credential_private_key, + ra_verifier, + now, + ) + } +} + +/// Verify key package secret and keypackage +pub fn verify_keypackage_and_secrets( + kp: &KeyPackage, + init_private_key: &HPKEPrivateKey, + credential_private_key: &IdentityPrivateKey, + ra_verifier: &impl AttestedCertVerifier, + now: Timespec, +) -> Result { + let info = kp.verify(ra_verifier, now)?; + // verify init public key and private key match + if !bool::from( + kp.payload + .init_key + .marshal() + .ct_eq(&init_private_key.public_key().marshal()), + ) { + return Err(Error::KeypackageSecretDontMatch); + } + // verify signature public key and private key match + if !bool::from( + info.public_key + .ct_eq(credential_private_key.public_key_raw()), + ) { + return Err(Error::KeypackageSecretDontMatch); + } + Ok(info) } /// Error type for key package verification. @@ -309,6 +364,8 @@ pub enum Error { CertificateVerifyError(EnclaveCertVerifierError), #[error("unsupported cipher suite: {0}")] UnsupportedCipherSuite(CipherSuite), + #[error("Keypackage public keys don't match private keys")] + KeypackageSecretDontMatch, } #[derive(thiserror::Error, Debug)] diff --git a/chain-tx-enclave-next/mls/src/message.rs b/chain-tx-enclave-next/mls/src/message.rs index 26e25b2e3..902f1e81d 100644 --- a/chain-tx-enclave-next/mls/src/message.rs +++ b/chain-tx-enclave-next/mls/src/message.rs @@ -576,7 +576,7 @@ pub struct CommitContent { pub commit: Commit, pub confirmation: Vec, pub additions: Vec, - pub updates: Vec<(LeafSize, Update)>, + pub updates: Vec<(LeafSize, Update, ProposalId)>, pub removes: Vec, } @@ -629,9 +629,13 @@ impl CommitContent { proposals_ids .get(proposal_id) .and_then(|p| { - p.get_update() - .cloned() - .map(|update| (LeafSize(p.content.sender.sender), update)) + p.get_update().cloned().map(|update| { + ( + LeafSize(p.content.sender.sender), + update, + proposal_id.clone(), + ) + }) }) .ok_or(()) }) diff --git a/chain-tx-enclave-next/mls/src/tree.rs b/chain-tx-enclave-next/mls/src/tree.rs index a8131c01b..6727163b0 100644 --- a/chain-tx-enclave-next/mls/src/tree.rs +++ b/chain-tx-enclave-next/mls/src/tree.rs @@ -13,7 +13,7 @@ use crate::message::DirectPathNode; use crate::message::*; use crate::tree_math::{LeafSize, NodeSize, ParentSize}; use crate::utils::{decode_option, encode_option, encode_vec_u32, read_vec_u32}; -use chain_util::non_empty::NonEmpty; +use chain_util::NonEmpty; use ra_client::AttestedCertVerifier; use rustls::internal::msgs::codec::{self, Codec, Reader}; use secrecy::{ExposeSecret, SecretVec}; @@ -299,6 +299,10 @@ impl TreePublicKey { self.set_package(self.my_pos, kp) } + pub fn get_parent_node(&self, pos: ParentSize) -> Option<&ParentNode> { + self.nodes[pos.node_index()].parent_node() + } + pub fn from_group_info( my_pos: LeafSize, cs: CipherSuite, @@ -399,11 +403,11 @@ impl TreePublicKey { pub fn update( &mut self, adds: &[Add], - updates: &[(LeafSize, Update)], + updates: &[(LeafSize, Update, ProposalId)], removes: &[Remove], ) -> Vec<(NodeSize, KeyPackage)> { // spec: draft-ietf-mls-protocol.md#update - for (sender, update) in updates.iter() { + for (sender, update, _) in updates.iter() { self.set_package(*sender, update.key_package.clone()); for p in NodeSize::from(*sender).direct_path(self.leaf_len()) { self.set_blank(p.into()); @@ -480,13 +484,13 @@ impl TreePublicKey { /// /// update path secrets /// - /// returns: direct path nodes, parent_hash, tree_private_key + /// returns: direct path nodes, parent_hash, tree_secret pub fn evolve( &mut self, ctx: &[u8], leaf_secret: &SecretVec, ) -> (Vec, Vec, TreeSecret) { - let tree_private_key = + let tree_secret = TreeSecret::new(self.cs, self.my_pos.into(), self.leaf_len(), &leaf_secret) .expect("invalid length"); let leaf_len = self.leaf_len(); @@ -498,10 +502,7 @@ impl TreePublicKey { // the root node is the last node of path, // and n won't be the root node because of skip(1) in iterator. let full_path = iter::once(my_node).chain(path.iter().copied().map(NodeSize::from)); - let path_with_secrets = path - .iter() - .copied() - .zip(tree_private_key.path_secrets.iter()); + let path_with_secrets = path.iter().copied().zip(tree_secret.path_secrets.iter()); for (node, (parent, secret)) in full_path.zip(path_with_secrets) { // encrypt the secret to resolution maintained let sibling = node.sibling(leaf_len).expect("not root node"); @@ -538,7 +539,7 @@ impl TreePublicKey { // update parent hash in path let leaf_parent_hash = self.set_parent_hash_path(self.my_pos); - (path_nodes, leaf_parent_hash, tree_private_key) + (path_nodes, leaf_parent_hash, tree_secret) } /// merge parent node public keys from `DirectPath` @@ -570,6 +571,26 @@ impl TreePublicKey { } self.nodes[path[0].node_index()].compute_parent_hash(self.cs) } + + /// Verify the secret match the public key in the parent node + pub(crate) fn verify_node_private_key( + &self, + secret: &SecretVec, + pos: ParentSize, + ) -> Result<(), ()> { + if let Some(node) = self.get_parent_node(pos) { + let private_key = self.cs.derive_private_key(secret); + if bool::from( + private_key + .public_key() + .marshal() + .ct_eq(&node.public_key.marshal()), + ) { + return Ok(()); + } + } + Err(()) + } } #[derive(thiserror::Error, Debug)] @@ -584,6 +605,24 @@ pub enum TreeIntegrityError { ParentHashDontMatch, } +/// Store the path secrets for a leaf node. +/// +/// For example, in this tree: +/// +/// ```plain +/// X +/// X +/// X X X +/// X X X X X +/// X X X X X X X X X X X +/// 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 +/// ``` +/// +/// The direct path of leaf 0 is [1, 3, 7, 15], the secret of the nodes in the direct path is +/// stored in `path_secrets`, and the secrets for the leaf node itself is stored in `kp_secret` in +/// `GroupAux`. +/// +/// The `update_secret` is derived from the secret of the root node. pub struct TreeSecret { /// Not including leaf private key pub path_secrets: Vec>, @@ -642,25 +681,26 @@ impl TreeSecret { ctx: &[u8], path_nodes: &[DirectPathNode], leaf_private_key: &HPKEPrivateKey, - ) -> Option<(ParentSize, SecretVec)> { + ) -> Result)>, ProcessCommitError> { let leaf_len = tree.leaf_len(); let ancestor = ParentSize::common_ancestor(sender, tree.my_pos); match ancestor { None => { // `sender` and `my_pos` are the same leaf node let node_index = NodeSize::from(sender); - Some(( - node_index.parent(leaf_len)?, - tree.cs - .expand_label( + match node_index.parent(leaf_len) { + Some(node) => Ok(Some(( + node, + tree.cs.expand_label( &leaf_private_key.marshal(), vec![], "path", &[], tree.cs.secret_size(), - ) - .expect("invalid length"), - )) + )?, + ))), + None => Ok(None), + } } Some(ancestor) => { // find the path node correspounding to the ancestor @@ -680,21 +720,19 @@ impl TreeSecret { // move one level down from ancestor to get to the node the secret encrypted to, // `None` means it's leaf node under the ancestor. let secret = match pos.checked_sub(1) { - None => tree - .cs - .decrypt(&leaf_private_key, ctx, &path_node.encrypted_path_secret[0]) - .expect("decrypt failed"), - Some(pos) => tree - .cs - .decrypt( - &HPKEPrivateKey::unmarshal(self.path_secrets[pos].expose_secret()) - .expect("invalid secret"), - ctx, - &path_node.encrypted_path_secret[0], - ) - .expect("decrypt failed"), + None => tree.cs.decrypt( + &leaf_private_key, + ctx, + &path_node.encrypted_path_secret[0], + )?, + Some(pos) => tree.cs.decrypt( + &HPKEPrivateKey::unmarshal(self.path_secrets[pos].expose_secret()) + .expect("invalid secret"), + ctx, + &path_node.encrypted_path_secret[0], + )?, }; - Some((ancestor, SecretVec::new(secret))) + Ok(Some((ancestor, SecretVec::new(secret)))) } } } @@ -703,62 +741,105 @@ impl TreeSecret { /// /// find the overlap parent node, decrypt the secret and implant it /// - /// returns (leaf_parent_hash, commit_secret) + /// it don't mutate self directly, but return a `TreeSecretDiff` which can be applied to + /// `TreeSecret` later. pub(crate) fn apply_path_secrets( - &mut self, + &self, sender: LeafSize, tree: &TreePublicKey, ctx: &[u8], path_nodes: &[DirectPathNode], leaf_private_key: &HPKEPrivateKey, - ) -> Result<(), ProcessCommitError> { + ) -> Result { let leaf_len = tree.leaf_len(); // Find overlap and decrypt the path secret - if let Some((overlap, last_secret)) = - self.decrypt_path_secrets(sender, tree, ctx, path_nodes, leaf_private_key) + if let Some((overlap, overlap_secret)) = + self.decrypt_path_secrets(sender, tree, ctx, path_nodes, leaf_private_key)? { - let mut secrets = NonEmpty::from(last_secret); - for _ in NodeSize::from(overlap).direct_path(leaf_len).iter() { - secrets.push(tree.cs.expand_label( - secrets.last(), + let direct_path = NodeSize::from(tree.my_pos).direct_path(leaf_len); + let overlap_pos = direct_path + .iter() + .position(|&p| p == overlap) + .expect("overlap is supposed to be ancestor"); + let overlap_path = &direct_path[overlap_pos + 1..]; + debug_assert_eq!( + overlap_path.iter().copied().collect::>(), + NodeSize::from(overlap).direct_path(leaf_len) + ); + + // verify the new path secrets match public keys + tree.verify_node_private_key(&overlap_secret, overlap) + .map_err(|_| ProcessCommitError::PathSecretPublicKeyDontMatch)?; + // the path secrets above(including) the overlap node + let mut path_secrets = NonEmpty::from(overlap_secret); + for _ in overlap_path.iter() { + path_secrets.push(tree.cs.expand_label( + path_secrets.last(), vec![], "path", &[], tree.cs.secret_size(), )?); } - let pos = NodeSize::from(tree.my_pos) - .direct_path(leaf_len) - .iter() - .position(|&p| p == overlap) - .expect("overlap is supposed to be ancestor"); - self.path_secrets.truncate(pos); - self.path_secrets - .extend(>>::from(secrets)); + assert_eq!(overlap_pos + path_secrets.len().get(), direct_path.len()); + + // verify the new path secrets match public keys + for (secret, &parent) in path_secrets.iter().skip(1).zip(overlap_path) { + tree.verify_node_private_key(secret, parent) + .map_err(|_| ProcessCommitError::PathSecretPublicKeyDontMatch)?; + } // Define `commit_secret` as the value `path_secret[n+1]` derived from the // `path_secret[n]` value assigned to the root node. - self.update_secret = tree.cs.expand_label( - self.path_secrets.last().expect("path secret not empty"), + let update_secret = tree.cs.expand_label( + path_secrets.last(), vec![], "path", &[], tree.cs.secret_size(), - )? + )?; + + Ok(TreeSecretDiff { + overlap_pos: Some(overlap_pos), + path_secrets: path_secrets.into(), + update_secret, + }) } else { - self.update_secret = tree.cs.expand_label( + let update_secret = tree.cs.expand_label( &leaf_private_key.marshal(), vec![], "path", &[], tree.cs.secret_size(), )?; - }; + Ok(TreeSecretDiff { + overlap_pos: None, + path_secrets: vec![], + update_secret, + }) + } + } - Ok(()) + pub(crate) fn apply_tree_diff(&mut self, tree_diff: TreeSecretDiff) { + if let Some(overlap_pos) = tree_diff.overlap_pos { + self.path_secrets.truncate(overlap_pos); + self.path_secrets.extend(tree_diff.path_secrets); + } + self.update_secret = tree_diff.update_secret; } } +/// Represent the diff needs to be applied later to `TreeSecret` +pub struct TreeSecretDiff { + /// the nodes from overlap_pos to root need to be updated + /// + /// None means no common ancestor + pub overlap_pos: Option, + /// the path secrets from(including) overlap_pos to root + pub path_secrets: Vec>, + pub update_secret: SecretVec, +} + /// draft-ietf-mls-protocol.md#tree-hashes fn node_hash(nodes: &[Node], cs: CipherSuite, index: NodeSize, leaf_size: LeafSize) -> Vec { let node = &nodes[index.node_index()];