From b46aff94cc9b82b89cfa7608e2347501fc4135db Mon Sep 17 00:00:00 2001 From: Hugo Rosenkranz-Costa Date: Thu, 18 Jan 2024 15:33:06 +0100 Subject: [PATCH] fix: use `mem::take` instead of `RefCell` to refresh `UserSecretKey` --- src/core/mod.rs | 3 +-- src/core/primitives.rs | 32 +++++++++++++++----------------- src/core/serialization.rs | 17 +++++++---------- src/test_utils/mod.rs | 18 +++++++++--------- 4 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/core/mod.rs b/src/core/mod.rs index 5db43f16..6de7a9f1 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1,7 +1,6 @@ //! Implements the core functionalities of `Covercrypt`. use std::{ - cell::RefCell, collections::{HashMap, HashSet}, hash::Hash, ops::Deref, @@ -88,7 +87,7 @@ pub struct MasterSecretKey { pub struct UserSecretKey { a: R25519PrivateKey, b: R25519PrivateKey, - pub(crate) subkeys: RefCell>, + pub(crate) subkeys: RevisionVec, kmac: Option, } diff --git a/src/core/primitives.rs b/src/core/primitives.rs index a0f3ff80..e8b45ba5 100644 --- a/src/core/primitives.rs +++ b/src/core/primitives.rs @@ -2,8 +2,8 @@ //! `bib/Covercrypt.pdf`. use std::{ - cell::RefCell, collections::{HashMap, HashSet, LinkedList}, + mem::take, }; use cosmian_crypto_core::{ @@ -49,7 +49,7 @@ fn compute_user_key_kmac(msk: &MasterSecretKey, usk: &UserSecretKey) -> Option { if let Some(sk_j) = sk_j { @@ -466,9 +466,10 @@ pub fn refresh( ) -> Result<(), Error> { verify_user_key_kmac(msk, usk)?; - let new_subkeys = usk - .subkeys - .take() + // Take ownership of `usk.subkeys` + let old_user_subkeys = take(&mut usk.subkeys); + + usk.subkeys = old_user_subkeys .into_iter() .filter_map(|(coordinate, user_chain)| { msk.subkeys.get(&coordinate).and_then(|msk_chain| { @@ -501,8 +502,6 @@ pub fn refresh( }) .collect::>(); - usk.subkeys.replace(new_subkeys); - // Update user key KMAC usk.kmac = compute_user_key_kmac(msk, usk); @@ -614,7 +613,7 @@ mod tests { assert!(client_secret_subkeys.unwrap().0.is_none()); // The developer now has a hybridized key. - assert_eq!(dev_usk.subkeys.borrow().count_elements(), 1); + assert_eq!(dev_usk.subkeys.count_elements(), 1); for key_encapsulation in &encapsulation.encs { if let KeyEncapsulation::ClassicEncapsulation(_) = key_encapsulation { panic!("Wrong hybridization type"); @@ -747,7 +746,7 @@ mod tests { // refresh the user key refresh(&msk, &mut usk, true)?; // user key kept old access to partition 1 - assert!(!usk.subkeys.borrow().flat_iter().any(|x| { + assert!(!usk.subkeys.flat_iter().any(|x| { x == ( &partition_1, old_msk.subkeys.get_latest(&partition_1).unwrap(), @@ -755,11 +754,10 @@ mod tests { })); assert!(usk .subkeys - .borrow() .flat_iter() .any(|x| { x == (&partition_2, msk.subkeys.get_latest(&partition_2).unwrap(),) })); // user key kept the old hybrid key for partition 3 - assert!(!usk.subkeys.borrow().flat_iter().any(|x| { + assert!(!usk.subkeys.flat_iter().any(|x| { x == ( &partition_3, old_msk.subkeys.get_latest(&partition_3).unwrap(), @@ -775,8 +773,8 @@ mod tests { )?; // refresh the user key refresh(&msk, &mut usk, true)?; - let usk_subkeys = usk.subkeys.borrow(); - let usk_subkeys: Vec<_> = usk_subkeys + let usk_subkeys: Vec<_> = usk + .subkeys .flat_iter() .filter(|(part, _)| *part == &partition_2) .map(|(_, subkey)| subkey) @@ -808,14 +806,14 @@ mod tests { // setup scheme let (msk, _) = setup(&mut rng, partitions_set); // create a user key with access to partition 1 and 2 - let usk = keygen(&mut rng, &msk, &HashSet::from([partition_1, partition_2]))?; + let mut usk = keygen(&mut rng, &msk, &HashSet::from([partition_1, partition_2]))?; assert!(verify_user_key_kmac(&msk, &usk).is_ok()); let bytes = usk.serialize()?; let usk_ = UserSecretKey::deserialize(&bytes)?; assert!(verify_user_key_kmac(&msk, &usk_).is_ok()); - usk.subkeys.borrow_mut().create_chain_with_single_value( + usk.subkeys.create_chain_with_single_value( Partition(b"3".to_vec()), (None, R25519PrivateKey::new(&mut rng)), ); diff --git a/src/core/serialization.rs b/src/core/serialization.rs index 2958f093..0839fd06 100644 --- a/src/core/serialization.rs +++ b/src/core/serialization.rs @@ -1,9 +1,6 @@ //! Implements the serialization methods for the `Covercrypt` objects. -use std::{ - cell::RefCell, - collections::{HashMap, HashSet, LinkedList}, -}; +use std::collections::{HashMap, HashSet, LinkedList}; use cosmian_crypto_core::{ bytes_ser_de::{to_leb128_len, Deserializer, Serializable, Serializer}, @@ -186,9 +183,9 @@ impl Serializable for UserSecretKey { let mut length = 2 * R25519PrivateKey::LENGTH + self.kmac.as_ref().map_or_else(|| 0, |kmac| kmac.len()) // subkeys serialization - + to_leb128_len(self.subkeys.borrow().len()) - + self.subkeys.borrow().count_elements() * R25519PrivateKey::LENGTH; - for (partition, chain) in self.subkeys.borrow().iter() { + + to_leb128_len(self.subkeys.len()) + + self.subkeys.count_elements() * R25519PrivateKey::LENGTH; + for (partition, chain) in self.subkeys.iter() { length += to_leb128_len(partition.len()) + partition.len(); length += to_leb128_len(chain.len()); for (sk_i, _) in chain { @@ -201,8 +198,8 @@ impl Serializable for UserSecretKey { fn write(&self, ser: &mut Serializer) -> Result { let mut n = ser.write_array(&self.a.to_bytes())?; n += ser.write_array(&self.b.to_bytes())?; - n += ser.write_leb128_u64(self.subkeys.borrow().len() as u64)?; - for (partition, chain) in self.subkeys.borrow().iter() { + n += ser.write_leb128_u64(self.subkeys.len() as u64)?; + for (partition, chain) in self.subkeys.iter() { // write chain partition n += ser.write_vec(partition)?; // iterate through all subkeys in the chain @@ -241,7 +238,7 @@ impl Serializable for UserSecretKey { Ok(Self { a, b, - subkeys: RefCell::new(subkeys), + subkeys, kmac, }) } diff --git a/src/test_utils/mod.rs b/src/test_utils/mod.rs index 58e22fad..71894048 100644 --- a/src/test_utils/mod.rs +++ b/src/test_utils/mod.rs @@ -156,26 +156,26 @@ mod tests { // 4 partitions accessed by the user were rekeyed (MKG Protected, Low Secret, // Medium Secret and High Secret) assert_eq!( - usk.subkeys.borrow().count_elements(), - original_usk.subkeys.borrow().count_elements() + 4 + usk.subkeys.count_elements(), + original_usk.subkeys.count_elements() + 4 ); - for x_i in original_usk.subkeys.borrow().flat_iter() { - assert!(usk.subkeys.borrow().flat_iter().any(|x| x == x_i)); + for x_i in original_usk.subkeys.flat_iter() { + assert!(usk.subkeys.flat_iter().any(|x| x == x_i)); } // refresh the user key but do NOT preserve access to old partitions cover_crypt.refresh_user_secret_key(&mut usk, &msk, false)?; // the user should still have access to the same number of partitions assert_eq!( - usk.subkeys.borrow().count_elements(), - original_usk.subkeys.borrow().count_elements() + usk.subkeys.count_elements(), + original_usk.subkeys.count_elements() ); - for x_i in original_usk.subkeys.borrow().flat_iter() { - assert!(!usk.subkeys.borrow().flat_iter().any(|x| x == x_i)); + for x_i in original_usk.subkeys.flat_iter() { + assert!(!usk.subkeys.flat_iter().any(|x| x == x_i)); } // try to modify the user key and refresh let part = Partition::from(vec![1, 6]); - usk.subkeys.borrow_mut().create_chain_with_single_value( + usk.subkeys.create_chain_with_single_value( part.clone(), msk.subkeys.get_latest(&part).unwrap().clone(), );