diff --git a/alpha_0.1.2_release_notes.md b/alpha_0.1.2_release_notes.md index cebd420..c59991b 100644 --- a/alpha_0.1.2_release_notes.md +++ b/alpha_0.1.2_release_notes.md @@ -28,6 +28,8 @@ * Close all open github issues and document them in this file. * After everything is merged, circle back to crucible, and make sure that the harness still works (and maybe remove the nightly build toolchain) +* Search for all the uses of .unwrap() in non-test code and replace each with either a comment or an expect with a + meaningful error string. # 0.1.2 Features / Changelog @@ -38,6 +40,8 @@ * mlkem-lowmemory -- runs in about 1/4th of the usual memory (~ 12 kb of stack) with comparable performance impact. * All public `*_out(.., out: &mut [u8])` functions now begin by zeroizing the entire output buffer with `.fill(0)`, preventing exposure of stale data in oversized output buffers or on early error returns. +* Reworked the way KeyMaterial hazardous operations work; instead of a stateful .allow_hazardous_operations() / + .drop_hazardous_operations(), it now uses a closure-based do_hazardous_operations(). Github issue #39. * Github issues resolved: * #6: https://github.com/bcgit/bc-rust/issues/6, thanks to Q. T. Felix (github: @Quant-TheodoreFelix) * #10: https://github.com/bcgit/bc-rust/issues/10, thanks to Nicola Tuveri (github: @romen) \ No newline at end of file diff --git a/cli/src/helpers.rs b/cli/src/helpers.rs index 62a7741..43993a3 100644 --- a/cli/src/helpers.rs +++ b/cli/src/helpers.rs @@ -1,4 +1,6 @@ -use bouncycastle::core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; +use bouncycastle::core::key_material::{ + KeyMaterial, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle::core::traits::SecurityStrength; use bouncycastle::hex; use std::fs::File; @@ -106,10 +108,11 @@ pub(crate) fn parse_seed(bytes: &[u8]) -> Result::from_bytes(&salt_bytes).unwrap(); // force it just so the CLI behaves properly even with all-zero or zero-length keys - salt_key.allow_hazardous_operations(); - salt_key.convert_key_type(KeyType::MACKey).unwrap(); + do_hazardous_operations(&mut salt_key, |salt_key| salt_key.set_key_type(KeyType::MACKey)) + .unwrap(); ikm_bytes = if ikm.is_some() { hex::decode(ikm.as_ref().unwrap()).unwrap() diff --git a/cli/src/mac_cmd.rs b/cli/src/mac_cmd.rs index 343cb0b..bb7aafc 100644 --- a/cli/src/mac_cmd.rs +++ b/cli/src/mac_cmd.rs @@ -2,7 +2,9 @@ use std::io::{Read, Write}; use std::process::exit; use std::{fs, io}; -use bouncycastle::core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; +use bouncycastle::core::key_material::{ + KeyMaterial512, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle::core::traits::MAC; use bouncycastle::hex; use bouncycastle::hmac::{HMAC_SHA256, HMAC_SHA512}; @@ -34,8 +36,7 @@ pub(crate) fn mac_cmd( exit(-1); } let mut key = KeyMaterial512::from_bytes(&key_bytes).unwrap(); - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::MACKey).unwrap(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::MACKey)).unwrap(); // instantiate the MAC object and call do_mac() match hmac_variant { diff --git a/crypto/core-test-framework/src/kdf.rs b/crypto/core-test-framework/src/kdf.rs index 3f60b49..21d1b4a 100644 --- a/crypto/core-test-framework/src/kdf.rs +++ b/crypto/core-test-framework/src/kdf.rs @@ -30,7 +30,7 @@ impl TestFrameworkKDF { // account for the fact that XOF style KDFs will will the provided buffer. assert!(bytes_written >= expected_output.key_len()); assert_eq!(output.key_len(), bytes_written); - output.truncate(expected_output.key_len()).unwrap(); + output.set_key_len(expected_output.key_len()).unwrap(); // truncates should be infallible assert_eq!(output.key_len(), expected_output.key_len()); assert_eq!(output.ref_to_bytes(), expected_output.ref_to_bytes()); @@ -99,7 +99,7 @@ impl TestFrameworkKDF { let output = kdf.derive_key_from_multiple(keys, additional_input).unwrap(); // This is sortof a hack since the rust language won't easily allow me to make the KeyMaterials the same length if output.key_len() < expected_output.key_len() { - expected_output.truncate(output.key_len()).unwrap(); + expected_output.set_key_len(output.key_len()).unwrap(); // truncates should be infallible } assert_eq!(output.key_len(), expected_output.key_len()); assert_eq!(output.ref_to_bytes(), expected_output.ref_to_bytes()); @@ -112,7 +112,7 @@ impl TestFrameworkKDF { // account for the fact that XOF style KDFs will will the provided buffer. assert!(bytes_written >= expected_output.key_len()); assert_eq!(output.key_len(), bytes_written); - output.truncate(expected_output.key_len()).unwrap(); + output.set_key_len(expected_output.key_len()).unwrap(); // truncates should be infallible assert_eq!(output.key_len(), expected_output.key_len()); assert_eq!(output.ref_to_bytes(), expected_output.ref_to_bytes()); diff --git a/crypto/core-test-framework/src/mac.rs b/crypto/core-test-framework/src/mac.rs index 87c8db5..a87d7f6 100644 --- a/crypto/core-test-framework/src/mac.rs +++ b/crypto/core-test-framework/src/mac.rs @@ -1,6 +1,8 @@ use crate::DUMMY_SEED_512; use bouncycastle_core::errors::{KeyMaterialError, MACError}; -use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; +use bouncycastle_core::key_material::{ + KeyMaterial512, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle_core::traits::MAC; use bouncycastle_core::traits::SecurityStrength; @@ -88,30 +90,32 @@ impl TestFrameworkMAC { let mut low_security_key = KeyMaterial512::from_bytes_as_type(&DUMMY_SEED_512[..64], KeyType::MACKey).unwrap(); - low_security_key.allow_hazardous_operations(); - match M::new_allow_weak_key(key).unwrap().max_security_strength() { - SecurityStrength::None => { - low_security_key.truncate(13).unwrap(); - low_security_key.set_security_strength(SecurityStrength::None).unwrap(); - } - SecurityStrength::_112bit => { - low_security_key.truncate(28).unwrap(); - low_security_key.set_security_strength(SecurityStrength::None).unwrap(); - } - SecurityStrength::_128bit => { - low_security_key.truncate(32).unwrap(); - low_security_key.set_security_strength(SecurityStrength::_112bit).unwrap(); - } - SecurityStrength::_192bit => { - low_security_key.truncate(48).unwrap(); - low_security_key.set_security_strength(SecurityStrength::_128bit).unwrap(); - } - SecurityStrength::_256bit => { - low_security_key.truncate(64).unwrap(); - low_security_key.set_security_strength(SecurityStrength::_192bit).unwrap(); - } - }; - low_security_key.drop_hazardous_operations(); + do_hazardous_operations(&mut low_security_key, |low_security_key| { + match M::new_allow_weak_key(key).unwrap().max_security_strength() { + SecurityStrength::None => { + low_security_key.set_key_len(13).unwrap(); // truncates should be infallible + low_security_key.set_security_strength(SecurityStrength::None).unwrap(); + } + SecurityStrength::_112bit => { + low_security_key.set_key_len(28).unwrap(); // truncate should be infallible + low_security_key.set_security_strength(SecurityStrength::None).unwrap(); + } + SecurityStrength::_128bit => { + low_security_key.set_key_len(32).unwrap(); // truncate should be infallible + low_security_key.set_security_strength(SecurityStrength::_112bit).unwrap(); + } + SecurityStrength::_192bit => { + low_security_key.set_key_len(48).unwrap(); // truncate should be infallible + low_security_key.set_security_strength(SecurityStrength::_128bit).unwrap(); + } + SecurityStrength::_256bit => { + low_security_key.set_key_len(64).unwrap(); // truncate should be infallible + low_security_key.set_security_strength(SecurityStrength::_192bit).unwrap(); + } + }; + Ok(()) + }) + .unwrap(); // init assert!( diff --git a/crypto/core/src/key_material.rs b/crypto/core/src/key_material.rs index 6097413..dd7a8d5 100644 --- a/crypto/core/src/key_material.rs +++ b/crypto/core/src/key_material.rs @@ -2,10 +2,6 @@ //! The main purpose is to hold metadata about the contained key material such as the key type and //! entropy content to prevent accidental misuse security bugs, such as deriving cryptographic keys //! from uninitialized data. -//! -//! This object allows several types of manual-overrides, which typically require setting the [KeyMaterial::allow_hazardous_operations] flag. -//! For example, the raw bytes data can be extracted, or the key forced to a certain type, -//! but well-designed use of the bc-rust.test library should not need to ever set the [KeyMaterial::allow_hazardous_operations] flag. //! The core idea of this wrapper is to keep track of the usage of the key material, including //! the amount of entropy that it is presumed to contain in order to prevent users from accidentally //! using it inappropriately in a way that could lead to security weaknesses. @@ -21,21 +17,38 @@ //! * Keyed KDFs that are given a key of RawFullEntropy or KeyedHashKey a KeyMaterial data of type RawLowEntropy or RawUnknownEntropy will promote it into RawFullEntropy. //! * Symmetric ciphers or asymmetric ciphers such as X25519 or ML-KEM that accept private key seeds will expect KeyMaterial of type AsymmetricPrivateKeySeed. //! -//! However, there is a [KeyMaterial::convert_key_type] for cases where the user has more context knowledge than the library. +//! However, there is a [KeyMaterialTrait::set_key_type] for cases where the user has more context knowledge than the library. //! Some conversions, such as converting a key of type RawLowEntropy into a SymmetricCipherKey, will fail unless -//! the user has explicitly allowed them via calling allow_hazardous_operations() prior to the conversion. +//! run inside of a [do_hazardous_operations] closure, see below. //! -//! Examples of hazardous conversions that require allow_hazardous_operations() to be called first: -//! -//! * Converting a KeyMaterial of type RawLowEntropy or RawUnknownEntropy into RawFullEntropy or any other full-entropy key type. -//! * Converting any algorithm-specific key type into a different algorithm-specific key type, which is considered hazardous since key reuse between different cryptographic algorithms is generally discouraged and can sometimes lead to key leakage. +//! # Security //! //! Additional security features: //! * Zeroizes on destruction. //! * Implementing Display and Debug to print metadata but not key material to prevent accidental logging. //! +//! # Hazardous Operations +//! +//! This object allows several types of manual-overrides, many of which are considered +//! "hazardous operations" since by definition they are allowing you to bypass checks meant to detect +//! conditions that could lead to security vulnerabilities. +//! Consider, for example, that you are reading a symmetric key from somewhere outside the library, +//! maybe from disk or from another process, but maybe you handed in the wrong variable and instead +//! handed in an uninitialized (all-zero) buffer. +//! Since this is a common bug that has catestrophic security implications, the library will normally +//! check for all-zero KeyMoterial objects and throw an error. +//! But there will be cases in which you really do need to use an all-zero key, so you can create +//! one if you do it in hazardous operations mode. +//! +//! Examples of hazardous conversions that are required to be run inside of a do_hazardous_operations() closure: +//! +//! * Converting a KeyMaterial of type RawLowEntropy or RawUnknownEntropy into RawFullEntropy or any other full-entropy key type. +//! * Converting any algorithm-specific key type into a different algorithm-specific key type, which is considered hazardous since key reuse between different cryptographic algorithms is generally discouraged and can sometimes lead to key leakage. +//! //! As with all wrappers of this nature, the intent is to protect the user from making silly mistakes, not to prevent expert users from doing what they need to do. //! It as always possible, for example, to extract the bytes from a KeyMaterial object, manipulate them, and then re-wrap them in a new KeyMaterial object. +//! +//! See [do_hazardous_operations] for documentation and sample code. use crate::errors::KeyMaterialError; use crate::traits::{RNG, Secret, SecurityStrength}; @@ -53,19 +66,20 @@ pub type KeyMaterial512 = KeyMaterial<64>; /// A helper class used across the bc-rust.test library to hold bytes-like key material. /// See [KeyMaterial] for for details, such as constructors. -pub trait KeyMaterialTrait { +#[allow(private_bounds)] +pub trait KeyMaterialTrait: KeyMaterialInternalTrait { /// Loads the provided data into a new KeyMaterial of the specified type. /// This is discouraged unless the caller knows the provenance of the data, such as loading it /// from a cryptographic private key file. /// - /// This behaves differently on all-zero input key depending on whether [KeyMaterialTrait::allow_hazardous_operations] is set: + /// This behaves differently on all-zero input key depending on whether it is run within a [do_hazardous_operations] closure: /// if not set, then it will succeed, setting the key type to [KeyType::Zeroized] and also return a [KeyMaterialError::ActingOnZeroizedKey] /// to indicate that you may want to perform error-handling, which could be manually setting the key type /// if you intend to allow zero keys, or do some other error-handling, like figure out why your RNG is broken. /// Note that even if a [KeyMaterialError::ActingOnZeroizedKey] is returned, the object is still populated and usable. /// For example, you could catch it like this: /// ``` - /// use bouncycastle_core::key_material::{KeyMaterial256, KeyType, KeyMaterialTrait}; + /// use bouncycastle_core::key_material::{KeyMaterial256, KeyType, KeyMaterialTrait, do_hazardous_operations}; /// use bouncycastle_core::key_material::KeyMaterial; /// use bouncycastle_core::errors::KeyMaterialError; /// @@ -76,15 +90,15 @@ pub trait KeyMaterialTrait { /// Err(KeyMaterialError::ActingOnZeroizedKey) => { /// // Either figure out why your passed an all-zero key, /// // or set the key type manually, if that's what you intended. - /// key.allow_hazardous_operations(); - /// key.set_key_type(KeyType::BytesLowEntropy).unwrap(); // probably you should do something more elegant than .unwrap in your code ;) - /// key.drop_hazardous_operations(); + /// do_hazardous_operations(&mut key, |key| { + /// key.set_key_type(KeyType::BytesLowEntropy) + /// }).unwrap(); // probably you should do something more elegant than .unwrap in your code ;) /// }, /// Err(_) => { /* figure out what else went wrong */ }, /// Ok(_) => { /* good */ }, /// } /// ``` - /// On the other hand, if [KeyMaterialTrait::allow_hazardous_operations] is set then it will just do what you asked without complaining. + /// On the other hand, if run inside a [do_hazardous_operations] closure then it will just do what you asked without complaining. /// /// Since this zeroizes and resets the key material, this is considered a dangerous conversion. /// @@ -100,18 +114,19 @@ pub trait KeyMaterialTrait { /// Get a reference to the underlying key material bytes. /// /// By reading the key bytes out of the [KeyMaterialTrait] object, you lose the protections that it offers, - /// however, this does not require [KeyMaterialTrait::allow_hazardous_operations] in the name of API ergonomics: - /// setting [KeyMaterialTrait::allow_hazardous_operations] requires a mutable reference and reading the bytes + /// however, this does not require [do_hazardous_operations] in the name of API ergonomics: + /// setting [do_hazardous_operations] requires a mutable reference and reading the bytes /// is not an operation that should require mutability. - /// TODO -- consider whether this should consume the object fn ref_to_bytes(&self) -> &[u8]; /// Get a mutable reference to the underlying key material bytes so that you can read or write /// to the underlying bytes without needing to create a temporary buffer, especially useful in /// cases where the required size of that buffer may be tricky to figure out at compile-time. - /// This requires [KeyMaterialTrait::allow_hazardous_operations] to be set. - /// When writing directly to the buffer, you are responsible for setting the key_len and key_type afterwards, - /// and you should [KeyMaterialTrait::drop_hazardous_operations]. + /// + /// # 🚨 Hazardous Operation🚨 + /// This function needs to be run within a [do_hazardous_operations] closure. + /// + /// When writing directly to the buffer, you are responsible for setting the key_len and key_type afterward. fn mut_ref_to_bytes(&mut self) -> Result<&mut [u8], KeyMaterialError>; /// The size of the internal buffer; ie the largest key that this instance can hold. @@ -121,12 +136,35 @@ pub trait KeyMaterialTrait { /// Length of the key material in bytes. fn key_len(&self) -> usize; - /// Requires [KeyMaterialTrait::allow_hazardous_operations]. + /// Sets the internal key length without changing the capacity of the KeyMaterial. + /// Primarily intended for truncation if you are provided with a key that is larger than you need, + /// or to extend the length of an undersized KeyMaterial. + /// + /// If truncating, it will automatically downgrade the SecurityStrength accordingly. + /// + /// # 🚨 Hazardous Operation🚨 + /// Using this function to extend the length of a key is always hazardous and needs to be run + /// within a [do_hazardous_operations] closure since this can result + /// in a key containing a large number of zeroes, or containing key material from a previous key + /// held in the same buffer. When extending the length, you take responsibility for the security + /// implications. + /// + /// Truncation (that is, reducing the length) is always safe and does not require a + /// [do_hazardous_operations] closure. fn set_key_len(&mut self, key_len: usize) -> Result<(), KeyMaterialError>; fn key_type(&self) -> KeyType; - /// Requires [KeyMaterialTrait::allow_hazardous_operations]. + /// Sets (or safely converts) the [KeyType] of this KeyMaterial object. + /// Does not perform any operations on the actual key material, other than changing the key_type field. + /// + /// # 🚨 Hazardous Operation🚨 + /// Inside a [do_hazardous_operations] closure this will set the key to any [KeyType]. + /// Outside such a closure, only "safe" conversions are permitted: a [KeyType::BytesFullEntropy] + /// key may be converted to any type, and any type may be converted to itself (a no-op). + /// A hazardous conversion attempted outside a [do_hazardous_operations] closure returns + /// [KeyMaterialError::HazardousOperationNotPermitted], and converting a [KeyType::Zeroized] key + /// returns [KeyMaterialError::ActingOnZeroizedKey]. fn set_key_type(&mut self, key_type: KeyType) -> Result<(), KeyMaterialError>; /// Security Strength, as used here, aligns with NIST SP 800-90A guidance for random number generation, @@ -142,47 +180,32 @@ pub trait KeyMaterialTrait { /// tracked independantly from key length and entropy level / key type. fn security_strength(&self) -> SecurityStrength; - /// Requires [KeyMaterialTrait::allow_hazardous_operations] to raise the security strength, but not to lower it. - /// Throws [KeyMaterialError::HazardousOperationNotPermitted] on a request to raise the security level without - /// [KeyMaterialTrait::allow_hazardous_operations] set. - /// Throws [KeyMaterialError::InvalidLength] on a request to set the security level higher than the current key length. + /// Set the [SecurityStrength] of the KeyMaterial. + /// + /// # 🚨 Hazardous Operation🚨 + /// This function needs to be run within a [do_hazardous_operations] closure to raise the security + /// strength, but not to lower it. + /// + /// Outside of a [do_hazardous_operations] closure it will throw a + /// [KeyMaterialError::HazardousOperationNotPermitted] on a request to raise the security level, and + /// throw a [KeyMaterialError::InvalidLength] on a request to set the security level higher than the current key length. Inside a [do_hazardous_operations] it will do what you asked without complaining. fn set_security_strength(&mut self, strength: SecurityStrength) -> Result<(), KeyMaterialError>; - /// Sets this instance to be able to perform potentially hazardous operations such as - /// casting a KeyMaterial of type RawUnknownEntropy or RawLowEntropy into RawFullEntropy or SymmetricCipherKey, - /// or manually setting the key bytes via [KeyMaterialTrait::mut_ref_to_bytes], which then requires you to be responsible - /// for setting the key_len and key_type afterwards. - /// - /// The purpose of the hazardous operations guard is not to prevent the user from accessing their data, - /// but rather to make the developer think carefully about the operation they are about to perform, - /// and to give static analysis tools an obvious marker that a given KeyMaterial variable warrants - /// further inspection. - fn allow_hazardous_operations(&mut self); - - /// Resets this instance to not be able to perform potentially hazardous operations. - fn drop_hazardous_operations(&mut self); - - /// Sets the key_type of this KeyMaterial object. - /// Does not perform any operations on the actual key material, other than changing the key_type field. - /// If allow_hazardous_operations is true, this method will allow conversion to any KeyType, otherwise - /// checking is performed to ensure that the conversion is "safe". - /// This drops the allow_hazardous_operations flag, so if you need to do multiple hazardous conversions - /// on the same instance, then you'll need to call .allow_hazardous_operations() each time. - fn convert_key_type(&mut self, new_key_type: KeyType) -> Result<(), KeyMaterialError>; - + /// Whether or not the KeyMaterial is one of the full entropy key types. fn is_full_entropy(&self) -> bool; + /// Securely resets the contents to all zeroes. + /// Note that KeyMaterial will automatically zeroize itself when dropped, so it is not necessary + /// to call this method simply because the object is going out of scope, but it provided + /// in case you want to zeroize it early, or before re-using the same instance of KeyMaterial to + /// hold a different key, potentially of a different length. fn zeroize(&mut self); - /// Is simply an alias to [KeyMaterialTrait::set_key_len], however, this does not require [KeyMaterialTrait::allow_hazardous_operations] - /// since truncation is a safe operation. - /// If truncating below the current security strength, the security strength will be lowered accordingly. - fn truncate(&mut self, new_len: usize) -> Result<(), KeyMaterialError>; - /// Adds the other KeyMaterial into this one, assuming there is space. - /// Does not require [KeyMaterialTrait::allow_hazardous_operations]. + /// /// Throws [KeyMaterialError::InvalidLength] if this object does not have enough space to add the other one. + /// /// The resulting [KeyType] and security strength will be the lesser of the two keys. /// In other words, concatenating two 128-bit full entropy keys generated at a 128-bit DRBG security level /// will result in a 256-bit full entropy key still at the 128-bit DRBG security level. @@ -252,15 +275,16 @@ impl KeyMaterial { /// Create a new instance of KeyMaterial containing random bytes from the provided random number generator. pub fn from_rng(rng: &mut impl RNG) -> Result { let mut key = Self::new(); - key.allow_hazardous_operations(); - rng.next_bytes_out(&mut key.mut_ref_to_bytes().unwrap()) - .map_err(|_| KeyMaterialError::GenericError("RNG failed."))?; + do_hazardous_operations(&mut key, |key| { + rng.next_bytes_out(&mut key.mut_ref_to_bytes().unwrap()) + .map_err(|_| KeyMaterialError::GenericError("RNG failed."))?; + Ok(()) + })?; key.key_len = KEY_LEN; key.key_type = KeyType::BytesFullEntropy; key.security_strength = rng.security_strength(); - key.drop_hazardous_operations(); Ok(key) } @@ -319,7 +343,6 @@ impl KeyMaterialTrait for KeyMaterial { key_type: KeyType, ) -> Result<(), KeyMaterialError> { let allowed_hazardous_operations = self.allow_hazardous_operations; - self.allow_hazardous_operations(); if source.len() > KEY_LEN { return Err(KeyMaterialError::InputDataLongerThanKeyCapacity); @@ -335,12 +358,14 @@ impl KeyMaterialTrait for KeyMaterial { self.key_len = source.len(); self.key_type = new_key_type; - if new_key_type <= KeyType::BytesLowEntropy { - self.set_security_strength(SecurityStrength::None)?; - } else { - self.set_security_strength(SecurityStrength::from_bits(source.len() * 8))?; - } - self.drop_hazardous_operations(); + do_hazardous_operations(self, |s| { + if new_key_type <= KeyType::BytesLowEntropy { + s.set_security_strength(SecurityStrength::None)?; + } else { + s.set_security_strength(SecurityStrength::from_bits(source.len() * 8))?; + } + Ok(()) + })?; // return if new_key_type == KeyType::Zeroized { @@ -370,23 +395,79 @@ impl KeyMaterialTrait for KeyMaterial { } fn set_key_len(&mut self, key_len: usize) -> Result<(), KeyMaterialError> { - if !self.allow_hazardous_operations { - return Err(KeyMaterialError::HazardousOperationNotPermitted); - } if key_len > KEY_LEN { return Err(KeyMaterialError::InvalidLength); } - self.key_len = key_len; - Ok(()) + + // are we extending the key length, or truncating? + if key_len <= self.key_len { + // truncation is always allowed (not hazardous) + + self.security_strength = + min(&self.security_strength, &SecurityStrength::from_bits(key_len * 8)).clone(); + + if key_len == 0 { + self.key_type = KeyType::Zeroized; + } + + self.key_len = key_len; + + Ok(()) + } else { + if !self.allow_hazardous_operations { + return Err(KeyMaterialError::HazardousOperationNotPermitted); + } + self.key_len = key_len; + Ok(()) + } } fn key_type(&self) -> KeyType { self.key_type.clone() } fn set_key_type(&mut self, key_type: KeyType) -> Result<(), KeyMaterialError> { - if !self.allow_hazardous_operations { - return Err(KeyMaterialError::HazardousOperationNotPermitted); + if self.allow_hazardous_operations { + // just do it + self.key_type = key_type; + return Ok(()); } - self.key_type = key_type.clone(); + + match self.key_type { + KeyType::Zeroized => { + return Err(KeyMaterialError::ActingOnZeroizedKey); + } + KeyType::BytesFullEntropy => { + // raw full entropy can be safely converted to anything. + self.key_type = key_type; + } + KeyType::BytesLowEntropy => match key_type { + KeyType::BytesLowEntropy => { /* No change */ } + _ => { + return Err(KeyMaterialError::HazardousOperationNotPermitted); + } + }, + KeyType::MACKey => match key_type { + KeyType::MACKey => { /* No change */ } + // Else: Once a KeyMaterial is typed, it should stay that way. + _ => { + return Err(KeyMaterialError::HazardousOperationNotPermitted); + } + }, + KeyType::SymmetricCipherKey => match key_type { + KeyType::SymmetricCipherKey => { /* No change */ } + // Else: Once a KeyMaterial is typed, it should stay that way. + _ => { + return Err(KeyMaterialError::HazardousOperationNotPermitted); + } + }, + KeyType::Seed => match key_type { + KeyType::Seed => { /* No change */ } + // Else: Once a KeyMaterial is typed, it should stay that way. + _ => { + return Err(KeyMaterialError::HazardousOperationNotPermitted); + } + }, + } + Ok(()) } fn security_strength(&self) -> SecurityStrength { @@ -440,69 +521,6 @@ impl KeyMaterialTrait for KeyMaterial { } self.security_strength = strength; - self.drop_hazardous_operations(); - Ok(()) - } - fn allow_hazardous_operations(&mut self) { - self.allow_hazardous_operations = true; - } - fn drop_hazardous_operations(&mut self) { - self.allow_hazardous_operations = false; - } - fn convert_key_type(&mut self, new_key_type: KeyType) -> Result<(), KeyMaterialError> { - if self.allow_hazardous_operations { - // just do it - self.key_type = new_key_type; - return Ok(()); - } - - match self.key_type { - KeyType::Zeroized => { - return Err(KeyMaterialError::ActingOnZeroizedKey); - } - KeyType::BytesFullEntropy => { - // raw full entropy can be safely converted to anything. - self.key_type = new_key_type; - } - KeyType::BytesLowEntropy => { - match new_key_type { - KeyType::BytesLowEntropy => { /* No change */ } - _ => { - return Err(KeyMaterialError::HazardousOperationNotPermitted); - } - } - } - KeyType::MACKey => { - match new_key_type { - KeyType::MACKey => { /* No change */ } - // Else: Once a KeyMaterial is typed, it should stay that way. - _ => { - return Err(KeyMaterialError::HazardousOperationNotPermitted); - } - } - } - KeyType::SymmetricCipherKey => { - match new_key_type { - KeyType::SymmetricCipherKey => { /* No change */ } - // Else: Once a KeyMaterial is typed, it should stay that way. - _ => { - return Err(KeyMaterialError::HazardousOperationNotPermitted); - } - } - } - KeyType::Seed => { - match new_key_type { - KeyType::Seed => { /* No change */ } - // Else: Once a KeyMaterial is typed, it should stay that way. - _ => { - return Err(KeyMaterialError::HazardousOperationNotPermitted); - } - } - } - } - - // each call to allow_hazardous_operations() is only good for one conversion. - self.drop_hazardous_operations(); Ok(()) } fn is_full_entropy(&self) -> bool { @@ -521,22 +539,6 @@ impl KeyMaterialTrait for KeyMaterial { self.key_type = KeyType::Zeroized; } - fn truncate(&mut self, new_len: usize) -> Result<(), KeyMaterialError> { - if new_len > self.key_len { - return Err(KeyMaterialError::InvalidLength); - } - - self.security_strength = - min(&self.security_strength, &SecurityStrength::from_bits(new_len * 8)).clone(); - - if new_len == 0 { - self.key_type = KeyType::Zeroized; - } - - self.key_len = new_len; - Ok(()) - } - fn concatenate(&mut self, other: &dyn KeyMaterialTrait) -> Result { let new_key_len = self.key_len() + other.key_len(); if self.key_len() + other.key_len() > KEY_LEN { @@ -630,3 +632,149 @@ impl Drop for KeyMaterial { self.zeroize() } } + +/* Hazardous Operations Runner */ + +/// Internal-use trait holding the low-level hazardous-operations guard toggle. +/// +/// These methods are deliberately split out of [KeyMaterialTrait] into a private trait so that +/// they are not accessible from outside this module. +/// +/// This is a supertrait of [KeyMaterialTrait], so anything that implements [KeyMaterialTrait] +/// also implements this. [KeyMaterialTrait] therefore stays dyn-compatible (both methods here are +/// object-safe), which matters because `Box` is used widely as a return type. +trait KeyMaterialInternalTrait { + /// Whether this instance is currently allowed to perform potentially hazardous operations. + fn allows_hazardous_operations(&self) -> bool; + /// Sets this instance to be able to perform potentially hazardous operations such as + /// casting a KeyMaterial of type RawUnknownEntropy or RawLowEntropy into RawFullEntropy or SymmetricCipherKey, + /// or manually setting the key bytes via [KeyMaterialTrait::mut_ref_to_bytes], which then requires you to be responsible + /// for setting the key_len and key_type afterwards. + /// + /// The purpose of the hazardous operations guard is not to prevent the user from accessing their data, + /// but rather to make the developer think carefully about the operation they are about to perform, + /// and to give static analysis tools an obvious marker that a given KeyMaterial variable warrants + /// further inspection. + /// + /// Prefer the scoped [KeyMaterial::do_hazardous_operations] wrapper, which calls this and + /// [KeyMaterialInternalTrait::drop_hazardous_operations] for you so the guard can't be left set. + fn allow_hazardous_operations(&mut self); + + /// Resets this instance to not be able to perform potentially hazardous operations. + fn drop_hazardous_operations(&mut self); +} + +impl KeyMaterialInternalTrait for KeyMaterial { + fn allows_hazardous_operations(&self) -> bool { + self.allow_hazardous_operations + } + fn allow_hazardous_operations(&mut self) { + self.allow_hazardous_operations = true; + } + fn drop_hazardous_operations(&mut self) { + self.allow_hazardous_operations = false; + } +} + +/// Runs the provided closure within which hazardous operations are allowed. +/// All hazardous operations will return a [KeyMaterialError::HazardousOperationNotPermitted] +/// if used outside of this closure. +/// +/// Example usage: +/// +/// ```rust +/// use bouncycastle_core::key_material::{KeyType, KeyMaterial256, KeyMaterialTrait, do_hazardous_operations}; +/// use bouncycastle_core::traits::SecurityStrength; +/// +/// // Let's create an all-zero key +/// let mut key = KeyMaterial256::default(); +/// +/// // Let's set a key of all zeroes, which the library would normally force to be +/// // [KeyType::Zeroized], but we want to force it to [KeyType::Seed], which is considered a +/// // hazardous operation. +/// do_hazardous_operations(&mut key, |key| { +/// key.set_bytes_as_type(&[8u8; 32], KeyType::Seed) +/// // note that the closure is required to return Result<(), KeyMaterialError>, +/// // so we can chain [KeyMaterial::set_bytes_as_type], otherwise we would need +/// // to end with Ok(()). +/// }).unwrap(); +/// +/// assert_eq!(key.key_len(), 32); +/// assert_eq!(key.key_type(), KeyType::Seed); +/// ``` +/// +/// ```rust +/// use bouncycastle_core::key_material::{KeyType, KeyMaterial256, KeyMaterialTrait, do_hazardous_operations}; +/// use bouncycastle_core::traits::SecurityStrength; +/// +/// // Let's create an all-zero key +/// let mut key = KeyMaterial256::default(); +/// assert_eq!(key.key_type(), KeyType::Zeroized); +/// assert_eq!(key.security_strength(), SecurityStrength::None); +/// +/// // Now we want to tell the library that this all-zero key +/// // is to be used as a 32-byte [KeyType::Seed] at the 256-bit security strength, +/// // which the library will not allow you to do outside of the hazerdous operations closure. +/// do_hazardous_operations(&mut key, |key| { +/// key.set_key_len(32)?; +/// key.set_key_type(KeyType::Seed)?; +/// key.set_security_strength(SecurityStrength::_256bit)?; +/// Ok(()) +/// }).unwrap(); +/// +/// assert_eq!(key.key_type(), KeyType::Seed); +/// assert_eq!(key.security_strength(), SecurityStrength::_256bit); +/// ``` +/// +/// Another common usage of hazardous operations is to get a direct mutable reference to the +/// underlying KeyMaterial byte buffer; for example if you want to copy in key bytes from somewhere else. +/// +/// ```rust +/// use bouncycastle_core::key_material::{KeyType, KeyMaterial512, KeyMaterialTrait, do_hazardous_operations}; +/// use bouncycastle_core::traits::SecurityStrength; +/// +/// // In this example, we initialize a KeyMateriol512 (64 bytes) with only 32 bytes of input. +/// let mut key = KeyMaterial512::from_bytes_as_type( +/// &[1u8; 32], +/// KeyType::BytesFullEntropy +/// ).unwrap(); +/// assert_eq!(key.key_len(), 32); +/// +/// // Now we want to expand the length to 64 bytes and copy in an additional 32 bytes of key data, +/// // using [KeyMaterial::mut_ref_to_bytes]. +/// let additional_bytes = [2u8; 32]; +/// do_hazardous_operations(&mut key, |key| { +/// key.set_key_len(64)?; +/// key.mut_ref_to_bytes()?[32..].copy_from_slice(&additional_bytes); +/// Ok(()) +/// }).unwrap(); +/// +/// assert_eq!(key.key_len(), 64); +/// // Reading the key bytes via [KeyMateriol::ref_to_bytes] is not a hazardous operation. +/// assert_eq!(key.ref_to_bytes()[..32], [1u8; 32]); +/// assert_eq!(key.ref_to_bytes()[32..], [2u8; 32]); +/// ``` +/// +// Dev note: This is a free function rather than a method on [KeyMaterialTrait] because it is +// generic over the closure type, which would make the trait non-dyn-compatible; the trait is used +// as `&dyn KeyMaterialTrait` elsewhere (e.g. [KeyMaterialTrait::concatenate], [KeyMaterialTrait::equals]). +// The toggle itself lives on the module-private [KeyMaterialInternalTrait], so external crates cannot +// flip the guard by hand and must go through this scoped wrapper (hence `#[allow(private_bounds)]`). +#[allow(private_bounds)] +pub fn do_hazardous_operations(key: &mut KEY, f: F) -> Result<(), KeyMaterialError> +where + KEY: KeyMaterialTrait + ?Sized, + F: FnOnce(&mut KEY) -> Result<(), KeyMaterialError>, +{ + let allows = key.allows_hazardous_operations(); + + key.allow_hazardous_operations(); + let ret = f(key); + + // to allow nested closures, if this key instance allowed + // before entering, then leave it. + if !allows { + key.drop_hazardous_operations(); + } + ret +} diff --git a/crypto/core/src/traits.rs b/crypto/core/src/traits.rs index 5247bbd..595fc06 100644 --- a/crypto/core/src/traits.rs +++ b/crypto/core/src/traits.rs @@ -129,8 +129,8 @@ pub trait KDF: Default { /// /// Output length: this function will behave differently depending on the underlying hash primitive; /// some, such as SHA2 or SHA3 will produce a fixed-length output, while others, such as SHAKE or HKDF, - /// will fill the provided KeyMaterial to capacity and require you to truncate it afterwards - /// using [KeyMaterialTrait::truncate]. + /// will fill the provided KeyMaterial to capacity and require you to truncate it afterward + /// using [KeyMaterialTrait::set_key_len]. fn derive_key_out( self, key: &impl KeyMaterialTrait, @@ -169,8 +169,8 @@ pub trait KDF: Default { /// /// Output length: this function will behave differently depending on the underlying hash primitive; /// some, such as SHA2 or SHA3 will produce a fixed-length output, while others, such as SHAKE or HKDF, - /// will fill the provided KeyMaterial to capacity and require you to truncate it afterwards - /// by using [KeyMaterialTrait::truncate]. + /// will fill the provided KeyMaterial to capacity and require you to truncate it afterward + /// by using [KeyMaterialTrait::set_key_len]. fn derive_key_from_multiple_out( self, keys: &[&impl KeyMaterialTrait], diff --git a/crypto/core/tests/key_material_tests.rs b/crypto/core/tests/key_material_tests.rs index 95d2b6b..2875214 100644 --- a/crypto/core/tests/key_material_tests.rs +++ b/crypto/core/tests/key_material_tests.rs @@ -3,7 +3,7 @@ mod test_key_material { use bouncycastle_core::errors::KeyMaterialError; use bouncycastle_core::key_material::{ KeyMaterial, KeyMaterial0, KeyMaterial128, KeyMaterial256, KeyMaterial512, - KeyMaterialTrait, KeyType, + KeyMaterialTrait, KeyType, do_hazardous_operations, }; use bouncycastle_core::traits::SecurityStrength; @@ -47,10 +47,12 @@ mod test_key_material { assert_eq!(key.key_type(), KeyType::Zeroized); assert_eq!(key.key_len(), 16); - // ... but we can force it. - key.allow_hazardous_operations(); - key.set_key_type(KeyType::BytesLowEntropy).unwrap(); - key.drop_hazardous_operations(); + // but it'll allow it within tho do_hazardous closure. + do_hazardous_operations(&mut key, |key| { + key.set_key_type(KeyType::BytesLowEntropy)?; + Ok(()) + }) + .unwrap(); } Err(_) => { panic!("should have thrown a KeyMaterialError::ActingOnZeroizedKey error.") @@ -59,18 +61,22 @@ mod test_key_material { assert_eq!(key.key_type(), KeyType::BytesLowEntropy); assert_eq!(key.security_strength(), SecurityStrength::None); - // but it'll allow it if you allow hazardous operations first. + // but it'll allow it within tho do_hazardous closure. let key_bytes = [0u8; 16]; let mut key = KeyMaterial256::new(); - key.allow_hazardous_operations(); - key.set_bytes_as_type(&key_bytes, KeyType::BytesLowEntropy).unwrap(); + do_hazardous_operations(&mut key, |key| { + key.set_bytes_as_type(&key_bytes, KeyType::BytesLowEntropy)?; + Ok(()) + }) + .unwrap(); assert_eq!(key.key_type(), KeyType::BytesLowEntropy); - key.drop_hazardous_operations(); + // nothing else requires setting hazardous operations. } #[test] fn test_refs() { + // by hand — this test exercises guard state-change semantics, so review carefully. let mut key = KeyMaterial256::from_bytes(&[1u8; 16]).unwrap(); assert_eq!(key.capacity(), 32); assert_eq!(key.ref_to_bytes(), &[1u8; 16]); // note: this is also testing that even though the internal buffer is larger than 16 bytes, it slices it down to length. @@ -84,17 +90,26 @@ mod test_key_material { panic!("getting a mut ref should require setting hazardous operations.") } } - key.allow_hazardous_operations(); - assert_eq!(key.mut_ref_to_bytes().unwrap().len(), 32); - assert_eq!(key.mut_ref_to_bytes().unwrap()[..16], [1u8; 16]); - assert_eq!(key.mut_ref_to_bytes().unwrap()[16..], [0u8; 16]); + + // check that we can read from the mut_ref_to_bytes + // (which is an odd way to use it, but legal) + do_hazardous_operations(&mut key, |key| { + assert_eq!(key.mut_ref_to_bytes()?.len(), 32); + assert_eq!(key.mut_ref_to_bytes()?[..16], [1u8; 16]); + assert_eq!(key.mut_ref_to_bytes()?[16..], [0u8; 16]); + Ok(()) + }) + .unwrap(); // and I can set them - key.mut_ref_to_bytes().unwrap().copy_from_slice(&[2u8; 32]); - key.set_key_len(32).unwrap(); + do_hazardous_operations(&mut key, |key| { + key.mut_ref_to_bytes().unwrap().copy_from_slice(&[2u8; 32]); + key.set_key_len(32) + }) + .unwrap(); + assert_eq!(key.ref_to_bytes(), &[2u8; 32]); assert_eq!(key.key_len(), 32); - key.drop_hazardous_operations(); } #[test] @@ -178,9 +193,11 @@ mod test_key_material { // Sanity check: the backing buffer actually holds non-zero key material before it is wiped. // Without this, the post-zeroize assertion below could pass vacuously. - key.allow_hazardous_operations(); - assert!(key.mut_ref_to_bytes().unwrap().iter().any(|&b| b != 0)); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| { + assert!(key.mut_ref_to_bytes().unwrap().iter().any(|&b| b != 0)); + Ok(()) + }) + .unwrap(); key.zeroize(); let key_len = key.key_len(); @@ -191,37 +208,33 @@ mod test_key_material { // Full capacity must be inspected to confirm the previously-set bytes were // actually overwritten with zeros. // Note: key_len is now 0, so ref_to_bytes() returns an empty slice. - key.allow_hazardous_operations(); - let full_buf = key.mut_ref_to_bytes().unwrap(); - assert_eq!(full_buf.len(), capacity); - assert!(full_buf.iter().all(|&b| b == 0)); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| { + let full_buf = key.mut_ref_to_bytes().unwrap(); + assert_eq!(full_buf.len(), capacity); + assert!(full_buf.iter().all(|&b| b == 0)); + Ok(()) + }) + .unwrap(); } #[test] - fn test_truncate() { + fn test_truncation() { let mut key = KeyMaterial512::from_bytes(&DUMMY_KEY[..64]).unwrap(); assert_eq!(key.key_len(), 64); // This should be no change - match key.truncate(64) { + match key.set_key_len(64) { Ok(()) => { /* good */ } _ => panic!("Expected Ok(())"), } assert_eq!(key.key_len(), 64); - key.truncate(32).unwrap(); + key.set_key_len(32).unwrap(); assert_eq!(key.key_len(), 32); - match key.truncate(64) { - Err(KeyMaterialError::InvalidLength) => { /* good */ } - _ => panic!("Expected InvalidLength"), - } - - key.truncate(16).unwrap(); + key.set_key_len(16).unwrap(); assert_eq!(key.key_len(), 16); - key.allow_hazardous_operations(); let key_len = key.key_len(); let mut buf = vec![0u8; key_len]; buf.copy_from_slice(key.ref_to_bytes()); @@ -231,7 +244,7 @@ mod test_key_material { // Test truncating a key to zero length let mut key_zero = KeyMaterial512::from_bytes(&DUMMY_KEY[..64]).unwrap(); - key_zero.truncate(0).unwrap(); + key_zero.set_key_len(0).unwrap(); assert_eq!(key_zero.key_len(), 0); assert_eq!(key_zero.key_type(), KeyType::Zeroized); @@ -239,19 +252,18 @@ mod test_key_material { let mut key = KeyMaterial512::from_bytes_as_type(&[1u8; 64], KeyType::BytesFullEntropy).unwrap(); assert_eq!(key.security_strength(), SecurityStrength::_256bit); - key.truncate(16).unwrap(); + key.set_key_len(16).unwrap(); assert_eq!(key.security_strength(), SecurityStrength::_128bit); - key.truncate(14).unwrap(); + key.set_key_len(14).unwrap(); assert_eq!(key.security_strength(), SecurityStrength::_112bit); - key.truncate(11).unwrap(); + key.set_key_len(11).unwrap(); assert_eq!(key.security_strength(), SecurityStrength::None); // truncate should not raise the security level let mut key = KeyMaterial512::from_bytes_as_type(&[1u8; 64], KeyType::BytesFullEntropy).unwrap(); key.set_security_strength(SecurityStrength::_112bit).unwrap(); - key.drop_hazardous_operations(); - key.truncate(64).unwrap(); + key.set_key_len(64).unwrap(); assert_eq!(key.security_strength(), SecurityStrength::_112bit); } @@ -268,31 +280,32 @@ mod test_key_material { } // This should fail. - match key.convert_key_type(KeyType::BytesFullEntropy) { + match key.set_key_type(KeyType::BytesFullEntropy) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::BytesFullEntropy).unwrap(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::BytesFullEntropy)) + .unwrap(); assert_eq!(key.key_type(), KeyType::BytesFullEntropy); assert!(key.is_full_entropy()); - key.drop_hazardous_operations(); - match key.convert_key_type(KeyType::SymmetricCipherKey) { + // Now we can convert BytesFullEntropy -> SymmetricCipherKey outside of a hazop block + match key.set_key_type(KeyType::SymmetricCipherKey) { Ok(()) => { /* good */ } _ => panic!("Expected Ok(())"), } - match key.convert_key_type(KeyType::BytesFullEntropy) { + match key.set_key_type(KeyType::BytesFullEntropy) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::BytesFullEntropy).unwrap(); - key.drop_hazardous_operations(); - match key.convert_key_type(KeyType::Seed) { + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::BytesFullEntropy)) + .unwrap(); + + // Now we can convert BytesFullEntropy -> Seed outside of a hazop block + match key.set_key_type(KeyType::Seed) { Ok(()) => { /* good */ } _ => panic!("Expected Ok(())"), } @@ -300,34 +313,27 @@ mod test_key_material { // each KeyType can convert to itself let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::BytesLowEntropy)) + .unwrap(); key.set_key_type(KeyType::BytesLowEntropy).unwrap(); - key.drop_hazardous_operations(); - key.convert_key_type(KeyType::BytesLowEntropy).unwrap(); let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::BytesFullEntropy)) + .unwrap(); key.set_key_type(KeyType::BytesFullEntropy).unwrap(); - key.drop_hazardous_operations(); - key.convert_key_type(KeyType::BytesFullEntropy).unwrap(); let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::MACKey)).unwrap(); key.set_key_type(KeyType::MACKey).unwrap(); - key.drop_hazardous_operations(); - key.convert_key_type(KeyType::MACKey).unwrap(); let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::SymmetricCipherKey)) + .unwrap(); key.set_key_type(KeyType::SymmetricCipherKey).unwrap(); - key.drop_hazardous_operations(); - key.convert_key_type(KeyType::SymmetricCipherKey).unwrap(); let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::Seed)).unwrap(); key.set_key_type(KeyType::Seed).unwrap(); - key.drop_hazardous_operations(); - key.convert_key_type(KeyType::Seed).unwrap(); } #[test] @@ -336,23 +342,23 @@ mod test_key_material { assert_eq!(zeroized_key.key_type(), KeyType::Zeroized); /* All conversions should fail. */ - match zeroized_key.convert_key_type(KeyType::BytesLowEntropy) { + match zeroized_key.set_key_type(KeyType::BytesLowEntropy) { Err(KeyMaterialError::ActingOnZeroizedKey) => { /* good */ } _ => panic!("Expected ActingOnZeroizedKey"), } - match zeroized_key.convert_key_type(KeyType::BytesFullEntropy) { + match zeroized_key.set_key_type(KeyType::BytesFullEntropy) { Err(KeyMaterialError::ActingOnZeroizedKey) => { /* good */ } _ => panic!("Expected ActingOnZeroizedKey"), } - match zeroized_key.convert_key_type(KeyType::MACKey) { + match zeroized_key.set_key_type(KeyType::MACKey) { Err(KeyMaterialError::ActingOnZeroizedKey) => { /* good */ } _ => panic!("Expected ActingOnZeroizedKey"), } - match zeroized_key.convert_key_type(KeyType::Seed) { + match zeroized_key.set_key_type(KeyType::Seed) { Err(KeyMaterialError::ActingOnZeroizedKey) => { /* good */ } _ => panic!("Expected ActingOnZeroizedKey"), } - match zeroized_key.convert_key_type(KeyType::SymmetricCipherKey) { + match zeroized_key.set_key_type(KeyType::SymmetricCipherKey) { Err(KeyMaterialError::ActingOnZeroizedKey) => { /* good */ } _ => panic!("Expected ActingOnZeroizedKey"), } @@ -382,11 +388,12 @@ mod test_key_material { // but it should still have set the key bytes; it's just giving you a friendly warning assert_eq!(zero_key.key_type(), KeyType::Zeroized); - // ... but will allow it if you set .allow_hazardous_operations() first. + // ... but will allow it inside a hazop closure let mut zero_key = KeyMaterial256::new(); - zero_key.allow_hazardous_operations(); - zero_key.set_bytes_as_type(&[0u8; 19], KeyType::MACKey).unwrap(); - zero_key.drop_hazardous_operations(); + do_hazardous_operations(&mut zero_key, |key| { + key.set_bytes_as_type(&[0u8; 19], KeyType::MACKey) + }) + .unwrap(); assert_eq!(zero_key.key_type(), KeyType::MACKey); } @@ -400,43 +407,37 @@ mod test_key_material { // ... none /* All the hazardous conversions should fail. */ - match key.convert_key_type(KeyType::BytesFullEntropy) { + match key.set_key_type(KeyType::BytesFullEntropy) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } - match key.convert_key_type(KeyType::MACKey) { + match key.set_key_type(KeyType::MACKey) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } - match key.convert_key_type(KeyType::SymmetricCipherKey) { + match key.set_key_type(KeyType::SymmetricCipherKey) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } - match key.convert_key_type(KeyType::Seed) { + match key.set_key_type(KeyType::Seed) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } /* Should work if you allow hazardous conversions. */ key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::BytesFullEntropy).unwrap(); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::BytesFullEntropy)) + .unwrap(); key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::MACKey).unwrap(); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::MACKey)).unwrap(); key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::SymmetricCipherKey).unwrap(); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::SymmetricCipherKey)) + .unwrap(); key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::Seed).unwrap(); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::Seed)).unwrap(); } #[test] @@ -488,31 +489,28 @@ mod test_key_material { /// Not exhaustive, cargo mutants will probably not be satisfied. fn test_hazardous_conversions_cast_types() { let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::MACKey).unwrap(); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::MACKey)).unwrap(); // converting to self should work (idempotency) - key.convert_key_type(KeyType::MACKey).unwrap(); + key.set_key_type(KeyType::MACKey).unwrap(); /* All the hazardous conversions should fail. */ - match key.convert_key_type(KeyType::BytesFullEntropy) { + match key.set_key_type(KeyType::BytesFullEntropy) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } - match key.convert_key_type(KeyType::SymmetricCipherKey) { + match key.set_key_type(KeyType::SymmetricCipherKey) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } - match key.convert_key_type(KeyType::Seed) { + match key.set_key_type(KeyType::Seed) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected HazardousConversion"), } // should work if you allow hazardous conversions. - key.allow_hazardous_operations(); - key.convert_key_type(KeyType::SymmetricCipherKey).unwrap(); - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| key.set_key_type(KeyType::SymmetricCipherKey)) + .unwrap(); } #[test] @@ -572,56 +570,83 @@ mod test_key_material { assert_eq!(key.security_strength(), SecurityStrength::None); // test set_security_strength() - // Can't increase the security level without setting .allow_hazardous_operations() first. + // Can't increase the security level outside of a hazop block first. let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); assert_eq!(key.key_type(), KeyType::BytesLowEntropy); match key.set_security_strength(SecurityStrength::_128bit) { Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } _ => panic!("Expected KeyMaterialError::HazardousOperationNotPermitted"), } - key.allow_hazardous_operations(); - match key.set_security_strength(SecurityStrength::_128bit) { - Err(KeyMaterialError::SecurityStrength(_)) => { /* good */ } - _ => panic!("Expected KeyMaterialError::SecurityStrength"), - } - - // So let's set it to a full-entropy type - key.set_key_type(KeyType::BytesFullEntropy).unwrap(); - // now it should work - key.set_security_strength(SecurityStrength::_128bit).unwrap(); + do_hazardous_operations(&mut key, |key| { + match key.set_security_strength(SecurityStrength::_128bit) { + Err(KeyMaterialError::SecurityStrength(_)) => { + /* good */ + Ok(()) + } + _ => panic!("Expected KeyMaterialError::SecurityStrength"), + } + }) + .unwrap(); + + // Even in a hazops block, you can't set a BytesLowEntropy to any Security Strength other than None + do_hazardous_operations(&mut key, |key| { + match key.set_security_strength(SecurityStrength::_128bit) { + Err(KeyMaterialError::SecurityStrength(_)) => { + /* good */ + Ok(()) + } + _ => panic!("Expected KeyMaterialError::SecurityStrength"), + } + }) + .unwrap(); + // But it'll work if you set it to a full entropy type + do_hazardous_operations(&mut key, |key| { + key.set_key_type(KeyType::BytesFullEntropy).unwrap(); + key.set_security_strength(SecurityStrength::_128bit) + }) + .unwrap(); + assert_eq!(key.key_type(), KeyType::BytesFullEntropy); assert_eq!(key.security_strength(), SecurityStrength::_128bit); - key.drop_hazardous_operations(); // BytesLowEntropy keys cannot have a security strength other than None. // success let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); assert_eq!(key.key_type(), KeyType::BytesLowEntropy); - // setting to ::None should work .. even without setting .allow_hazardous_operations() + // setting to ::None should work .. even outside of a hazop block key.set_security_strength(SecurityStrength::None).unwrap(); // but to ::_128bit should fail - key.allow_hazardous_operations(); - match key.set_security_strength(SecurityStrength::_128bit) { - Err(KeyMaterialError::SecurityStrength(_)) => { /* good */ } - _ => panic!("Expected KeyMaterialError::SecurityStrength"), - } - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| { + match key.set_security_strength(SecurityStrength::_128bit) { + Err(KeyMaterialError::SecurityStrength(_)) => { + /* good */ + Ok(()) + } + _ => panic!("Expected KeyMaterialError::SecurityStrength"), + } + }) + .unwrap(); // Zeroized keys cannot have a security strength other than None. // success let mut key = KeyMaterial256::new(); - key.allow_hazardous_operations(); - key.set_key_len(32).unwrap(); // still zeroized - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| { + key.set_key_len(32) // still zeroized + }) + .unwrap(); assert_eq!(key.key_type(), KeyType::Zeroized); - // setting to ::None should work .. even without setting .allow_hazardous_operations() + // setting to ::None should work, even outside of a hazop block key.set_security_strength(SecurityStrength::None).unwrap(); - // but to ::_128bit should fail - key.allow_hazardous_operations(); - match key.set_security_strength(SecurityStrength::_128bit) { - Err(KeyMaterialError::SecurityStrength(_)) => { /* good */ } - _ => panic!("Expected KeyMaterialError::SecurityStrength"), - } - key.drop_hazardous_operations(); + // but to ::_128bit should fail, even in a hazop block + do_hazardous_operations(&mut key, |key| { + match key.set_security_strength(SecurityStrength::_128bit) { + Err(KeyMaterialError::SecurityStrength(_)) => { + /* good */ + Ok(()) + } + _ => panic!("Expected KeyMaterialError::SecurityStrength"), + } + }) + .unwrap(); } #[test] @@ -638,9 +663,11 @@ mod test_key_material { assert_eq!(key1.ref_to_bytes()[16..], [2u8; 16]); let mut zeroized_key = KeyMaterial256::default(); - zeroized_key.allow_hazardous_operations(); - zeroized_key.set_key_len(8).unwrap(); - zeroized_key.drop_hazardous_operations(); + do_hazardous_operations(&mut zeroized_key, |zeroized_key| { + zeroized_key.set_key_len(8).unwrap(); + Ok(()) + }) + .unwrap(); assert_eq!(zeroized_key.key_type(), KeyType::Zeroized); assert_eq!(zeroized_key.key_len(), 8); zeroized_key.concatenate(&key2).unwrap(); @@ -652,9 +679,11 @@ mod test_key_material { // This should be symmetric, so test it in the other direction too. let mut zeroized_key = KeyMaterial256::default(); - zeroized_key.allow_hazardous_operations(); - zeroized_key.set_key_len(8).unwrap(); - zeroized_key.drop_hazardous_operations(); + do_hazardous_operations(&mut zeroized_key, |zeroized_key| { + zeroized_key.set_key_len(8).unwrap(); + Ok(()) + }) + .unwrap(); assert_eq!(zeroized_key.key_type(), KeyType::Zeroized); assert_eq!(zeroized_key.key_len(), 8); let mut key2 = KeyMaterial256::from_bytes(&[1u8; 16]).unwrap(); @@ -772,8 +801,11 @@ mod test_key_material { // success case -- zero keys -- different type and different KeyType let key1 = KeyMaterial0::new(); let mut key2 = KeyMaterial256::new(); - key2.allow_hazardous_operations(); - key2.convert_key_type(KeyType::SymmetricCipherKey).unwrap(); + do_hazardous_operations(&mut key2, |key2| { + key2.set_key_type(KeyType::SymmetricCipherKey).unwrap(); + Ok(()) + }) + .unwrap(); assert!(key1.equals(&key2)); assert!(key2.equals(&key1)); @@ -786,8 +818,11 @@ mod test_key_material { // success case -- non-zero keys -- different type and different KeyType let key1 = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); let mut key2 = KeyMaterial512::from_bytes(&DUMMY_KEY[..32]).unwrap(); - key2.allow_hazardous_operations(); - key2.convert_key_type(KeyType::SymmetricCipherKey).unwrap(); + do_hazardous_operations(&mut key2, |key2| { + key2.set_key_type(KeyType::SymmetricCipherKey).unwrap(); + Ok(()) + }) + .unwrap(); assert!(key1.equals(&key2)); assert!(key2.equals(&key1)); @@ -841,4 +876,67 @@ mod test_key_material { } } } + + #[test] + /// Pins the error behaviour of [do_hazardous_operations]: + /// 1. an `Err` returned from the closure propagates out verbatim (not swallowed or remapped), + /// 2. a real `KeyMaterialError` raised by a guarded op inside the closure propagates via `?`, + /// 3. the "flattening" idiom the KDF/RNG crates rely on — collapsing a *foreign* error type + /// into a `KeyMaterialError` with `map_err` inside the closure — surfaces as that + /// `KeyMaterialError`, and + /// 4. the guard is still cleared on the error path (an erroring closure does not leave the + /// instance stuck in hazardous mode). + fn test_hazardous_ops_error_handling() { + let mut key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap(); + + // 1. An explicit Err returned from the closure propagates out unchanged. + let result = + do_hazardous_operations(&mut key, |_key| Err(KeyMaterialError::GenericError("boom"))); + match result { + Err(KeyMaterialError::GenericError("boom")) => { /* good */ } + _ => panic!("the closure's error should propagate out verbatim"), + } + + // 2. A real KeyMaterialError raised by a guarded op inside the closure propagates via `?`. + // Raising to _256bit requires >= 32 bytes, but this key is only 16, so it fails. + let mut short = + KeyMaterial256::from_bytes_as_type(&DUMMY_KEY[..16], KeyType::BytesFullEntropy) + .unwrap(); + let result = do_hazardous_operations(&mut short, |k| { + k.set_security_strength(SecurityStrength::_256bit)?; + Ok(()) + }); + match result { + Err(KeyMaterialError::SecurityStrength(_)) => { /* good */ } + _ => panic!("the SecurityStrength error should propagate from inside the closure"), + } + + // 3. The "flattening" idiom: a foreign error type is collapsed into a KeyMaterialError via + // map_err inside the closure (exactly how hkdf/rng flatten MACError/RNGError), so it + // surfaces as that KeyMaterialError. Outside the closure, the caller would then convert + // the KeyMaterialError back to its own error type via `?` / `From`. + #[derive(Debug)] + struct ForeignError; + fn foreign_op() -> Result<(), ForeignError> { + Err(ForeignError) + } + + let result = do_hazardous_operations(&mut key, |_k| { + foreign_op().map_err(|_| KeyMaterialError::GenericError("flattened"))?; + Ok(()) + }); + match result { + Err(KeyMaterialError::GenericError("flattened")) => { /* good */ } + _ => panic!("the foreign error should be flattened into a KeyMaterialError"), + } + + // 4. The guard is cleared even when the closure returns Err: a guarded op afterwards, outside + // any closure, must still be rejected. `key` is BytesLowEntropy and was never mutated by + // the erroring closures above, so converting it to Seed is a hazardous op that requires + // the guard -- which proves the guard was restored to "off". + match key.set_key_type(KeyType::Seed) { + Err(KeyMaterialError::HazardousOperationNotPermitted) => { /* good */ } + _ => panic!("the guard should have been cleared after the erroring closures"), + } + } } diff --git a/crypto/hkdf/src/lib.rs b/crypto/hkdf/src/lib.rs index 76b47f0..45cf1c3 100644 --- a/crypto/hkdf/src/lib.rs +++ b/crypto/hkdf/src/lib.rs @@ -147,7 +147,8 @@ #![forbid(unsafe_code)] -use bouncycastle_core::errors::{KDFError, MACError}; +use bouncycastle_core::errors::{KDFError, KeyMaterialError, MACError}; +use bouncycastle_core::key_material; use bouncycastle_core::key_material::{ KeyMaterial, KeyMaterial0, KeyMaterial512, KeyMaterialTrait, KeyType, }; @@ -156,7 +157,6 @@ use bouncycastle_hmac::HMAC; use bouncycastle_sha2::{SHA256, SHA512}; use bouncycastle_utils::{max, min}; use std::marker::PhantomData; - // Imports needed only for docs #[allow(unused_imports)] use bouncycastle_core::traits::XOF; @@ -396,8 +396,6 @@ impl HKDF { let N = L.div_ceil(hash_len) as u8; let mut bytes_written: usize = 0; - okm.allow_hazardous_operations(); - let out: &mut [u8] = okm.mut_ref_to_bytes()?; // Could potentially speed this up by unrolling T(0) and T(1) // We're gonna have to kludge the prk key type to MACKey to make HMAC happy, but we'll set it back to the original value afterwards. @@ -408,19 +406,27 @@ impl HKDF { let mut T = [0u8; HMAC_BLOCK_LEN]; let mut t_len: usize = 0; let mut i = 1u8; - while i < N { - // todo: might need this to be new_allow_weak_key() - let mut hmac = HMAC::::new(&prk_as_mac_key)?; - hmac.do_update(&T[..t_len]); - hmac.do_update(info); - hmac.do_update(&[i]); - - t_len = hmac.do_final_out(&mut T)?; - debug_assert_eq!(t_len, hash_len); // this will be true for every iteration after T(0) / T(1) - out[bytes_written..bytes_written + t_len].copy_from_slice(&T[..t_len]); - bytes_written += t_len; - i += 1; - } + + key_material::do_hazardous_operations(okm, |okm| { + let out = okm.mut_ref_to_bytes()?; + while i < N { + // todo: might need this to be new_allow_weak_key() + let mut hmac = HMAC::::new(&prk_as_mac_key) + .map_err(|_| KeyMaterialError::GenericError("HMAC initialization failed"))?; + hmac.do_update(&T[..t_len]); + hmac.do_update(info); + hmac.do_update(&[i]); + + t_len = hmac + .do_final_out(&mut T) + .map_err(|_| KeyMaterialError::GenericError("HMAC finalization failed"))?; + debug_assert_eq!(t_len, hash_len); // this will be true for every iteration after T(0) / T(1) + out[bytes_written..bytes_written + t_len].copy_from_slice(&T[..t_len]); + bytes_written += t_len; + i += 1; + } + Ok(()) + })?; // On the last iteration, we don't take all of the output. let remaining = L - bytes_written; @@ -432,26 +438,32 @@ impl HKDF { t_len = hmac.do_final_out(&mut T[..remaining])?; debug_assert_eq!(t_len, remaining); // this will be true for every iteration after T(0) / T(1) - out[bytes_written..bytes_written + t_len].copy_from_slice(&T[..t_len]); + + key_material::do_hazardous_operations(okm, |okm| { + let out = okm.mut_ref_to_bytes()?; + out[bytes_written..bytes_written + t_len].copy_from_slice(&T[..t_len]); + Ok(()) + })?; bytes_written += t_len; // set the KeyType of the output // since we've done some computation, the result will not actually be zeroized, even if all input key material was zeroized. - if prk.key_type() == KeyType::Zeroized { - okm.set_key_type(KeyType::BytesLowEntropy)?; - } else { - okm.set_key_type(prk.key_type().clone())?; - } - okm.set_key_len(bytes_written)?; - if okm.key_type() <= KeyType::BytesLowEntropy { - okm.set_security_strength(SecurityStrength::None)?; - } else { - okm.set_security_strength( - min(&SecurityStrength::from_bytes(okm.key_len()), &entropy.security_strength) - .clone(), - )?; - } - okm.drop_hazardous_operations(); + key_material::do_hazardous_operations(okm, |okm| { + if prk.key_type() == KeyType::Zeroized { + okm.set_key_type(KeyType::BytesLowEntropy)?; + } else { + okm.set_key_type(prk.key_type().clone())?; + } + okm.set_key_len(bytes_written)?; + if okm.key_type() <= KeyType::BytesLowEntropy { + okm.set_security_strength(SecurityStrength::None) + } else { + okm.set_security_strength( + min(&SecurityStrength::from_bytes(okm.key_len()), &entropy.security_strength) + .clone(), + ) + } + })?; Ok(bytes_written) } @@ -568,20 +580,27 @@ impl HKDF { let output_key_type = self.entropy.get_output_key_type(); // need to do this above self.hmac.do_final_out, which will consume self. - okm.allow_hazardous_operations(); // doing it here to get mut_ref_to_bytes - let bytes_written = - self.hmac.unwrap().do_final_out(&mut okm.mut_ref_to_bytes().unwrap())?; - okm.set_key_len(bytes_written)?; - okm.set_key_type(output_key_type)?; - if output_key_type <= KeyType::BytesLowEntropy { - okm.set_security_strength(SecurityStrength::None)?; - } else { - okm.set_security_strength( - min(&SecurityStrength::from_bytes(okm.key_len()), &self.entropy.security_strength) + let mut bytes_written = 0; + key_material::do_hazardous_operations(okm, |okm| { + bytes_written = self + .hmac + .unwrap() + .do_final_out(&mut okm.mut_ref_to_bytes()?) + .map_err(|_| KeyMaterialError::GenericError("HMAC do_final_out failed"))?; + okm.set_key_len(bytes_written)?; + okm.set_key_type(output_key_type)?; + if output_key_type <= KeyType::BytesLowEntropy { + okm.set_security_strength(SecurityStrength::None) + } else { + okm.set_security_strength( + min( + &SecurityStrength::from_bytes(okm.key_len()), + &self.entropy.security_strength, + ) .clone(), - )?; - } - okm.drop_hazardous_operations(); + ) + } + })?; Ok(bytes_written) } } @@ -605,7 +624,7 @@ impl KDF for HKDF { ) -> Result, KDFError> { let mut output_key = KeyMaterial512::new(); _ = self.derive_key_out(key, additional_input, &mut output_key)?; - output_key.truncate(H::OUTPUT_LEN)?; + output_key.set_key_len(H::OUTPUT_LEN)?; Ok(Box::new(output_key)) } @@ -643,7 +662,7 @@ impl KDF for HKDF { ) -> Result, KDFError> { let mut output_key = KeyMaterial512::new(); _ = self.derive_key_from_multiple_out(keys, additional_input, &mut output_key)?; - output_key.truncate(*min(&output_key.key_len(), &H::OUTPUT_LEN))?; + output_key.set_key_len(*min(&output_key.key_len(), &H::OUTPUT_LEN))?; Ok(Box::new(output_key)) } @@ -676,13 +695,16 @@ impl KDF for HKDF { let bytes_written = HKDF::::expand_out(&prk, additional_input, output_key.capacity(), output_key)?; - output_key.allow_hazardous_operations(); - output_key.set_key_type(entropy.get_output_key_type())?; - output_key.set_security_strength( - min(&SecurityStrength::from_bytes(output_key.key_len()), &entropy.security_strength) + key_material::do_hazardous_operations(output_key, |output_key| { + output_key.set_key_type(entropy.get_output_key_type())?; + output_key.set_security_strength( + min( + &SecurityStrength::from_bytes(output_key.key_len()), + &entropy.security_strength, + ) .clone(), - )?; - output_key.drop_hazardous_operations(); + ) + })?; Ok(bytes_written) } diff --git a/crypto/hkdf/tests/hkdf_tests.rs b/crypto/hkdf/tests/hkdf_tests.rs index 1e13d37..4d598fa 100644 --- a/crypto/hkdf/tests/hkdf_tests.rs +++ b/crypto/hkdf/tests/hkdf_tests.rs @@ -1,6 +1,7 @@ #[cfg(test)] mod hkdf_tests { use bouncycastle_core::errors::{KDFError, KeyMaterialError, MACError}; + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{ KeyMaterial, KeyMaterial0, KeyMaterial128, KeyMaterial256, KeyMaterial512, KeyMaterialTrait, KeyType, @@ -528,20 +529,18 @@ mod hkdf_tests { /*** First with the one-shot APIs ::extract() and ::expand_out(). ***/ let mut ikm_key = KeyMaterial::<100>::new(); - ikm_key.allow_hazardous_operations(); - let r = ikm_key.set_bytes_as_type(&hex::decode(ikm).unwrap(), KeyType::BytesFullEntropy); // just for testing, ignore the error about zeroized keys - if r.is_err() { - ikm_key.allow_hazardous_operations(); - ikm_key.set_key_type(KeyType::MACKey).unwrap(); - } + key_material::do_hazardous_operations(&mut ikm_key, |ikm_key| { + // just for testing, ignore the error about zeroized keys + ikm_key.set_bytes_as_type(&hex::decode(ikm).unwrap(), KeyType::BytesFullEntropy) + }) + .unwrap(); let mut salt_key = KeyMaterial::<100>::new(); - salt_key.allow_hazardous_operations(); - let r = salt_key.set_bytes_as_type(&hex::decode(salt).unwrap(), KeyType::MACKey); // just for testing, ignore the error about zeroized keys - if r.is_err() { - salt_key.allow_hazardous_operations(); - ikm_key.set_key_type(KeyType::MACKey).unwrap(); - } + key_material::do_hazardous_operations(&mut salt_key, |salt_key| { + // just for testing, ignore the error about zeroized keys + salt_key.set_bytes_as_type(&hex::decode(salt).unwrap(), KeyType::MACKey) + }) + .unwrap(); let info = hex::decode(info).unwrap(); let mut prk_key = HKDF_SHA256::extract(&salt_key, &ikm_key).unwrap(); @@ -550,12 +549,14 @@ mod hkdf_tests { // Some of the RFC5896 test vectors have input keys that are too short to meet the entropy seeding rules. // So, just for testing, we'll bump this up to full entropy, regardless of what entropy HKDF::extract() // thinks it should be based on the inputs. - prk_key.allow_hazardous_operations(); - prk_key.set_key_type(KeyType::MACKey).unwrap(); + key_material::do_hazardous_operations(&mut prk_key, |prk_key| { + prk_key.set_key_type(KeyType::MACKey) + }) + .unwrap(); let mut okm_key = KeyMaterial::<100>::new(); _ = HKDF_SHA256::expand_out(&prk_key, &info, L, &mut okm_key).unwrap(); - okm_key.truncate(L).unwrap(); + okm_key.set_key_len(L).unwrap(); assert_eq!(okm_key.ref_to_bytes().len(), L); assert_eq!(okm_key.ref_to_bytes(), hex::decode(okm).unwrap()); @@ -683,8 +684,9 @@ mod hkdf_tests { // SP800-56Cr2 tcId 1 let mut salt = KeyMaterial::<128>::new(); // have to do it this way for it to accept a zeroized key - salt.allow_hazardous_operations(); - salt.set_bytes_as_type(&hex::decode("00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").unwrap(), KeyType::MACKey).unwrap(); + key_material::do_hazardous_operations(&mut salt, |salt|{ + salt.set_bytes_as_type(&hex::decode("00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").unwrap(), KeyType::MACKey) + }).unwrap(); // ikm = z||t // let ikm = KeyMaterial_internal::<128>::from_bytes_as_type(&hex::decode("589410408990A227518017C37997BE2F770AF54063E7393B2AA5463196136D18F365733139EB74CDD6B7268F41D33DB6").unwrap(), KeyType::MACKey).unwrap(); @@ -719,7 +721,7 @@ mod hkdf_tests { let output_key = hkdf.derive_key(&ikm, &additional_input).unwrap(); // kdf.derive_key is a one-step that doesn't expand, so need to truncate the expected key to match. let mut expected_key_truncated = expected_key.clone(); - expected_key_truncated.truncate(output_key.key_len()).unwrap(); + expected_key_truncated.set_key_len(output_key.key_len()).unwrap(); assert_eq!(output_key.ref_to_bytes(), expected_key_truncated.ref_to_bytes()); testframework diff --git a/crypto/hmac/tests/hmac_tests.rs b/crypto/hmac/tests/hmac_tests.rs index a3d8e8c..862bfa1 100644 --- a/crypto/hmac/tests/hmac_tests.rs +++ b/crypto/hmac/tests/hmac_tests.rs @@ -1,6 +1,7 @@ #[cfg(test)] mod hmac_tests { use bouncycastle_core::errors::{KeyMaterialError, MACError}; + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{ KeyMaterial, KeyMaterial256, KeyMaterial512, KeyMaterialTrait, KeyType, }; @@ -16,8 +17,10 @@ mod hmac_tests { fn simple_tests() { // Simple test with zero-length key let mut zero_length_key = KeyMaterial256::default(); - zero_length_key.allow_hazardous_operations(); - zero_length_key.convert_key_type(KeyType::MACKey).unwrap(); + key_material::do_hazardous_operations(&mut zero_length_key, |zero_length_key| { + zero_length_key.set_key_type(KeyType::MACKey) + }) + .unwrap(); assert_eq!(zero_length_key.key_len(), 0); assert_eq!(zero_length_key.key_type(), KeyType::MACKey); @@ -152,8 +155,8 @@ mod hmac_tests { HMAC_SHA256::new_allow_weak_key(&zero_key).unwrap(); // non-zero len key of all-zero bytes - zero_key.allow_hazardous_operations(); - zero_key.set_key_len(32).unwrap(); + key_material::do_hazardous_operations(&mut zero_key, |zero_key| zero_key.set_key_len(32)) + .unwrap(); HMAC_SHA256::new_allow_weak_key(&zero_key).unwrap(); // but we don't allow zero-len keys that are not Zeroized or MACKey @@ -270,8 +273,10 @@ mod hmac_tests { fn hmac_sha224() { let test_framework = TestFrameworkMAC::new(); let mut zero_length_key = KeyMaterial256::default(); - zero_length_key.allow_hazardous_operations(); - zero_length_key.convert_key_type(KeyType::MACKey).unwrap(); + key_material::do_hazardous_operations(&mut zero_length_key, |zero_length_key| { + zero_length_key.set_key_type(KeyType::MACKey) + }) + .unwrap(); assert_eq!(zero_length_key.key_len(), 0); assert_eq!(zero_length_key.key_type(), KeyType::MACKey); @@ -346,8 +351,10 @@ mod hmac_tests { // test with zero-length key let test_framework = TestFrameworkMAC::new(); let mut zero_length_key = KeyMaterial256::default(); - zero_length_key.allow_hazardous_operations(); - zero_length_key.convert_key_type(KeyType::MACKey).unwrap(); + key_material::do_hazardous_operations(&mut zero_length_key, |zero_length_key| { + zero_length_key.set_key_type(KeyType::MACKey) + }) + .unwrap(); assert_eq!(zero_length_key.key_len(), 0); assert_eq!(zero_length_key.key_type(), KeyType::MACKey); @@ -423,8 +430,10 @@ mod hmac_tests { // test with zero-length key let test_framework = TestFrameworkMAC::new(); let mut zero_length_key = KeyMaterial256::default(); - zero_length_key.allow_hazardous_operations(); - zero_length_key.convert_key_type(KeyType::MACKey).unwrap(); + key_material::do_hazardous_operations(&mut zero_length_key, |zero_length_key| { + zero_length_key.set_key_type(KeyType::MACKey) + }) + .unwrap(); assert_eq!(zero_length_key.key_len(), 0); assert_eq!(zero_length_key.key_type(), KeyType::MACKey); @@ -498,8 +507,10 @@ mod hmac_tests { // test with zero-length key let test_framework = TestFrameworkMAC::new(); let mut zero_length_key = KeyMaterial256::default(); - zero_length_key.allow_hazardous_operations(); - zero_length_key.convert_key_type(KeyType::MACKey).unwrap(); + key_material::do_hazardous_operations(&mut zero_length_key, |zero_length_key| { + zero_length_key.set_key_type(KeyType::MACKey) + }) + .unwrap(); assert_eq!(zero_length_key.key_len(), 0); assert_eq!(zero_length_key.key_type(), KeyType::MACKey); diff --git a/crypto/mldsa-lowmemory/src/mldsa_keys.rs b/crypto/mldsa-lowmemory/src/mldsa_keys.rs index b805d89..d4b1c88 100644 --- a/crypto/mldsa-lowmemory/src/mldsa_keys.rs +++ b/crypto/mldsa-lowmemory/src/mldsa_keys.rs @@ -21,6 +21,7 @@ use crate::mldsa::{ }; use crate::{ML_DSA_44_NAME, ML_DSA_65_NAME, ML_DSA_87_NAME}; use bouncycastle_core::errors::SignatureError; +use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{ Secret, SecurityStrength, SignaturePrivateKey, SignaturePublicKey, XOF, @@ -610,10 +611,10 @@ impl< return Err(SignatureError::DecodingError("Invalid seed length")); } let mut keymat = KeyMaterial::<32>::from_bytes(bytes)?; - keymat.allow_hazardous_operations(); - keymat.set_key_type(KeyType::Seed)?; - keymat.set_security_strength(SecurityStrength::_256bit)?; - keymat.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut keymat, |keymat| { + keymat.set_key_type(KeyType::Seed)?; + keymat.set_security_strength(SecurityStrength::_256bit) + })?; Self::new(&keymat) } diff --git a/crypto/mldsa-lowmemory/tests/bc_test_data.rs b/crypto/mldsa-lowmemory/tests/bc_test_data.rs index fdc5c56..94c4b67 100644 --- a/crypto/mldsa-lowmemory/tests/bc_test_data.rs +++ b/crypto/mldsa-lowmemory/tests/bc_test_data.rs @@ -16,6 +16,7 @@ mod bc_test_data { #[allow(dead_code)] use crate::BustedMuBuilder; use bouncycastle_core::errors::SignatureError; + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{ Hash, SecurityStrength, SignaturePrivateKey, SignaturePublicKey, SignatureVerifier, @@ -167,10 +168,10 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + seed.set_security_strength(SecurityStrength::_256bit) + }); match self.parameter_set.as_str() { "ML-DSA-44" => { @@ -700,10 +701,10 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + seed.set_security_strength(SecurityStrength::_256bit) + }); let (pk, sk) = MLDSA44::keygen_from_seed(&seed).unwrap(); let pk_sized: [u8; MLDSA44_PK_LEN] = @@ -776,10 +777,10 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + seed.set_security_strength(SecurityStrength::_256bit) + }); let (pk, sk) = MLDSA65::keygen_from_seed(&seed).unwrap(); let pk_sized: [u8; MLDSA65_PK_LEN] = @@ -846,10 +847,10 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + seed.set_security_strength(SecurityStrength::_256bit) + }); let (pk, sk) = MLDSA87::keygen_from_seed(&seed).unwrap(); let pk_sized: [u8; MLDSA87_PK_LEN] = diff --git a/crypto/mldsa-lowmemory/tests/mldsa_key_tests.rs b/crypto/mldsa-lowmemory/tests/mldsa_key_tests.rs index 8b7884f..f80978c 100644 --- a/crypto/mldsa-lowmemory/tests/mldsa_key_tests.rs +++ b/crypto/mldsa-lowmemory/tests/mldsa_key_tests.rs @@ -4,6 +4,7 @@ mod mldsa_key_tests { #![allow(unused_imports)] use bouncycastle_core::errors::SignatureError; + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{SecurityStrength, SignaturePrivateKey, SignaturePublicKey}; use bouncycastle_core_test_framework::signature::TestFrameworkSignatureKeys; @@ -96,19 +97,22 @@ mod mldsa_key_tests { // it'll reject a keyen with a seed too weak, and preserve the seed otherwise let mut seed128 = seed.clone(); - seed128.allow_hazardous_operations(); - seed128.set_security_strength(SecurityStrength::_128bit).unwrap(); - seed128.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed128, |seed| { + seed.set_security_strength(SecurityStrength::_128bit) + }) + .unwrap(); let mut seed192 = seed.clone(); - seed192.allow_hazardous_operations(); - seed192.set_security_strength(SecurityStrength::_192bit).unwrap(); - seed192.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed192, |seed| { + seed.set_security_strength(SecurityStrength::_192bit) + }) + .unwrap(); let mut seed256 = seed.clone(); - seed256.allow_hazardous_operations(); - seed256.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed256.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed256, |seed| { + seed.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); // MLDSA44 let (_pk, sk) = MLDSA44::keygen_from_seed(&seed128).unwrap(); diff --git a/crypto/mldsa-lowmemory/tests/mldsa_tests.rs b/crypto/mldsa-lowmemory/tests/mldsa_tests.rs index e45ed76..593b6f9 100644 --- a/crypto/mldsa-lowmemory/tests/mldsa_tests.rs +++ b/crypto/mldsa-lowmemory/tests/mldsa_tests.rs @@ -3,6 +3,7 @@ mod mldsa_tests { use crate::{MLDSA44_KAT1, MLDSA65_KAT1, MLDSA87_KAT1}; use bouncycastle_core::errors::SignatureError; + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{ RNG, SecurityStrength, SignaturePrivateKey, SignaturePublicKey, SignatureVerifier, Signer, @@ -188,8 +189,10 @@ mod mldsa_tests { assert_eq!(derived_pk.encode(), expected_pk_bytes.as_slice()); // success case KeyType: BytesFullEntropy - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::BytesFullEntropy).unwrap(); + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::BytesFullEntropy) + }) + .unwrap(); _ = MLDSA44::keygen_from_seed(&seed).unwrap(); // Failure case: key type != Seed || BytesFullEntropy @@ -446,9 +449,10 @@ mod mldsa_tests { KeyType::Seed, ) .unwrap(); - low_security_seed.allow_hazardous_operations(); - low_security_seed.set_security_strength(SecurityStrength::_192bit).unwrap(); - low_security_seed.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut low_security_seed, |seed| { + seed.set_security_strength(SecurityStrength::_192bit) + }) + .unwrap(); // a 128bit secure seed should be rejected by MLDSA87 MLDSA65::sign_mu_deterministic_from_seed(&low_security_seed, &mu, rnd).unwrap(); @@ -459,9 +463,10 @@ mod mldsa_tests { KeyType::Seed, ) .unwrap(); - low_security_seed.allow_hazardous_operations(); - low_security_seed.set_security_strength(SecurityStrength::_128bit).unwrap(); - low_security_seed.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut low_security_seed, |seed| { + seed.set_security_strength(SecurityStrength::_128bit) + }) + .unwrap(); // a 128bit secure seed should be rejected by MLDSA87 match MLDSA87::sign_mu_deterministic_from_seed(&low_security_seed, &mu, rnd) { Err(SignatureError::KeyGenError(_)) => { /* good */ } diff --git a/crypto/mldsa-lowmemory/tests/wycheproof.rs b/crypto/mldsa-lowmemory/tests/wycheproof.rs index 080efdf..3dcdedc 100644 --- a/crypto/mldsa-lowmemory/tests/wycheproof.rs +++ b/crypto/mldsa-lowmemory/tests/wycheproof.rs @@ -22,6 +22,7 @@ #![allow(dead_code)] use bouncycastle_core::errors::SignatureError; +use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{SecurityStrength, SignaturePublicKey, SignatureVerifier}; use bouncycastle_hex as hex; @@ -268,10 +269,24 @@ impl MLDSASignSeedTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { - Ok(_) => (), + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed)?; + match seed.set_security_strength(SecurityStrength::_256bit) { + Ok(_) => Ok(()), + Err(e) => { + if self.result == "invalid" { + /* good */ + Ok(()) + } else { + panic!("{:?}", e) + } + } + } + }) + .unwrap(); + + let (pk, sk) = match MLDSA44::keygen_from_seed(&seed) { + Ok((pk, sk)) => (pk, sk), Err(e) => { if self.result == "invalid" { /* good */ @@ -280,13 +295,6 @@ impl MLDSASignSeedTestCase { panic!("{:?}", e) } } - } - - let (pk, sk) = match MLDSA44::keygen_from_seed(&seed) { - Ok((pk, sk)) => (pk, sk), - Err(e) => { - panic!("{:?}", e) - } }; let loaded_pk = @@ -358,24 +366,31 @@ impl MLDSASignSeedTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { - Ok(_) => (), - Err(e) => { - if self.result == "invalid" { - /* good */ - return; - } else { - panic!("{:?}", e) + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + match seed.set_security_strength(SecurityStrength::_256bit) { + Ok(_) => Ok(()), + Err(e) => { + if self.result == "invalid" { + /* good */ + Ok(()) + } else { + panic!("{:?}", e) + } } } - } + }) + .unwrap(); let (pk, sk) = match MLDSA65::keygen_from_seed(&seed) { Ok((pk, sk)) => (pk, sk), Err(e) => { - panic!("{:?}", e) + if self.result == "invalid" { + /* good, test passed */ + return; + } else { + panic!("{:?}", e) + } } }; @@ -448,24 +463,31 @@ impl MLDSASignSeedTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { - Ok(_) => (), - Err(e) => { - if self.result == "invalid" { - /* good */ - return; - } else { - panic!("{:?}", e) + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + match seed.set_security_strength(SecurityStrength::_256bit) { + Ok(_) => Ok(()), + Err(e) => { + if self.result == "invalid" { + /* good */ + Ok(()) + } else { + panic!("{:?}", e) + } } } - } + }) + .unwrap(); let (pk, sk) = match MLDSA87::keygen_from_seed(&seed) { Ok((pk, sk)) => (pk, sk), Err(e) => { - panic!("{:?}", e) + if self.result == "invalid" { + /* good, test passed */ + return; + } else { + panic!("{:?}", e) + } } }; diff --git a/crypto/mldsa/tests/bc_test_data.rs b/crypto/mldsa/tests/bc_test_data.rs index a732489..3891229 100644 --- a/crypto/mldsa/tests/bc_test_data.rs +++ b/crypto/mldsa/tests/bc_test_data.rs @@ -12,7 +12,9 @@ use bouncycastle_sha3::SHAKE256; mod bc_test_data { use crate::BustedMuBuilder; use bouncycastle_core::errors::SignatureError; - use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; + use bouncycastle_core::key_material::{ + KeyMaterial256, KeyMaterialTrait, KeyType, do_hazardous_operations, + }; use bouncycastle_core::traits::{ Hash, SecurityStrength, SignaturePrivateKey, SignaturePublicKey, SignatureVerifier, }; @@ -162,10 +164,11 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed)?; + seed.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); match self.parameter_set.as_str() { "ML-DSA-44" => { @@ -706,10 +709,11 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed)?; + seed.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); let (pk, sk) = MLDSA44::keygen_from_seed(&seed).unwrap(); let pk_sized: [u8; MLDSA44_PK_LEN] = @@ -784,10 +788,11 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed)?; + seed.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); let (pk, sk) = MLDSA65::keygen_from_seed(&seed).unwrap(); let pk_sized: [u8; MLDSA65_PK_LEN] = @@ -856,10 +861,11 @@ mod bc_test_data { ) .unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed)?; + seed.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); let (pk, sk) = MLDSA87::keygen_from_seed(&seed).unwrap(); let pk_sized: [u8; MLDSA87_PK_LEN] = diff --git a/crypto/mldsa/tests/mldsa_key_tests.rs b/crypto/mldsa/tests/mldsa_key_tests.rs index 81e2493..0fc5fe1 100644 --- a/crypto/mldsa/tests/mldsa_key_tests.rs +++ b/crypto/mldsa/tests/mldsa_key_tests.rs @@ -1,7 +1,9 @@ #[cfg(test)] mod mldsa_key_tests { use bouncycastle_core::errors::SignatureError; - use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; + use bouncycastle_core::key_material::{ + KeyMaterial256, KeyMaterialTrait, KeyType, do_hazardous_operations, + }; use bouncycastle_core::traits::{SecurityStrength, SignaturePrivateKey, SignaturePublicKey}; use bouncycastle_core_test_framework::signature::TestFrameworkSignatureKeys; use bouncycastle_hex as hex; @@ -140,19 +142,22 @@ mod mldsa_key_tests { // it'll reject a keyen with a seed too weak, and preserve the seed otherwise let mut seed128 = seed.clone(); - seed128.allow_hazardous_operations(); - seed128.set_security_strength(SecurityStrength::_128bit).unwrap(); - seed128.drop_hazardous_operations(); + do_hazardous_operations(&mut seed128, |seed128| { + seed128.set_security_strength(SecurityStrength::_128bit) + }) + .unwrap(); let mut seed192 = seed.clone(); - seed192.allow_hazardous_operations(); - seed192.set_security_strength(SecurityStrength::_192bit).unwrap(); - seed192.drop_hazardous_operations(); + do_hazardous_operations(&mut seed192, |seed192| { + seed192.set_security_strength(SecurityStrength::_192bit) + }) + .unwrap(); let mut seed256 = seed.clone(); - seed256.allow_hazardous_operations(); - seed256.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed256.drop_hazardous_operations(); + do_hazardous_operations(&mut seed256, |seed256| { + seed256.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); // MLDSA44 let (_pk, sk) = MLDSA44::keygen_from_seed(&seed128).unwrap(); diff --git a/crypto/mldsa/tests/mldsa_tests.rs b/crypto/mldsa/tests/mldsa_tests.rs index b87a0a8..242800a 100644 --- a/crypto/mldsa/tests/mldsa_tests.rs +++ b/crypto/mldsa/tests/mldsa_tests.rs @@ -3,7 +3,9 @@ mod mldsa_tests { use crate::{MLDSA44_KAT1, MLDSA65_KAT1, MLDSA87_KAT1}; use bouncycastle_core::errors::SignatureError; - use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; + use bouncycastle_core::key_material::{ + KeyMaterial256, KeyMaterialTrait, KeyType, do_hazardous_operations, + }; use bouncycastle_core::traits::{ RNG, SecurityStrength, SignaturePrivateKey, SignaturePublicKey, SignatureVerifier, Signer, }; @@ -223,8 +225,8 @@ mod mldsa_tests { assert_eq!(derived_pk.encode(), expected_pk_bytes.as_slice()); // success case KeyType: BytesFullEntropy - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::BytesFullEntropy).unwrap(); + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::BytesFullEntropy)) + .unwrap(); _ = MLDSA44::keygen_from_seed(&seed).unwrap(); // Failure case: key type != Seed || BytesFullEntropy @@ -586,9 +588,10 @@ mod mldsa_tests { KeyType::Seed, ) .unwrap(); - low_security_seed.allow_hazardous_operations(); - low_security_seed.set_security_strength(SecurityStrength::_192bit).unwrap(); - low_security_seed.drop_hazardous_operations(); + do_hazardous_operations(&mut low_security_seed, |low_security_seed| { + low_security_seed.set_security_strength(SecurityStrength::_192bit) + }) + .unwrap(); // a 128bit secure seed should be rejected by MLDSA87 MLDSA65::sign_mu_deterministic_from_seed(&low_security_seed, &mu, rnd).unwrap(); @@ -599,9 +602,10 @@ mod mldsa_tests { KeyType::Seed, ) .unwrap(); - low_security_seed.allow_hazardous_operations(); - low_security_seed.set_security_strength(SecurityStrength::_128bit).unwrap(); - low_security_seed.drop_hazardous_operations(); + do_hazardous_operations(&mut low_security_seed, |low_security_seed| { + low_security_seed.set_security_strength(SecurityStrength::_128bit) + }) + .unwrap(); // a 128bit secure seed should be rejected by MLDSA87 match MLDSA87::sign_mu_deterministic_from_seed(&low_security_seed, &mu, rnd) { Err(SignatureError::KeyGenError(_)) => { /* good */ } diff --git a/crypto/mldsa/tests/wycheproof.rs b/crypto/mldsa/tests/wycheproof.rs index abe6842..523be36 100644 --- a/crypto/mldsa/tests/wycheproof.rs +++ b/crypto/mldsa/tests/wycheproof.rs @@ -18,7 +18,9 @@ #![allow(dead_code)] use bouncycastle_core::errors::SignatureError; -use bouncycastle_core::key_material::{KeyMaterial256, KeyMaterialTrait, KeyType}; +use bouncycastle_core::key_material::{ + KeyMaterial256, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle_core::traits::{ SecurityStrength, SignaturePrivateKey, SignaturePublicKey, SignatureVerifier, }; @@ -590,9 +592,10 @@ impl MLDSASignSeedTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::Seed)).unwrap(); + match do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(SecurityStrength::_256bit) + }) { Ok(_) => (), Err(e) => { if self.result == "invalid" { @@ -681,9 +684,10 @@ impl MLDSASignSeedTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::Seed)).unwrap(); + match do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(SecurityStrength::_256bit) + }) { Ok(_) => (), Err(e) => { if self.result == "invalid" { @@ -772,9 +776,10 @@ impl MLDSASignSeedTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::Seed)).unwrap(); + match do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(SecurityStrength::_256bit) + }) { Ok(_) => (), Err(e) => { if self.result == "invalid" { diff --git a/crypto/mlkem-lowmemory/src/mlkem.rs b/crypto/mlkem-lowmemory/src/mlkem.rs index 98eca08..5e63c0b 100644 --- a/crypto/mlkem-lowmemory/src/mlkem.rs +++ b/crypto/mlkem-lowmemory/src/mlkem.rs @@ -13,7 +13,9 @@ use crate::mlkem_keys::{MLKEMPrivateKeyInternalTrait, MLKEMPrivateKeyTrait}; use crate::mlkem_keys::{MLKEMPublicKeyInternalTrait, MLKEMPublicKeyTrait}; use crate::polynomial::Polynomial; use bouncycastle_core::errors::KEMError; -use bouncycastle_core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; +use bouncycastle_core::key_material::{ + KeyMaterial, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle_core::traits::{ Algorithm, Hash, KEMDecapsulator, KEMEncapsulator, RNG, SecurityStrength, XOF, }; @@ -693,9 +695,9 @@ impl< let mut ss_keymaterial = KeyMaterial::::from_bytes_as_type(&ss_bytes, KeyType::BytesFullEntropy)?; - ss_keymaterial.allow_hazardous_operations(); - ss_keymaterial.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize))?; - ss_keymaterial.drop_hazardous_operations(); + do_hazardous_operations(&mut ss_keymaterial, |ss_keymaterial| { + ss_keymaterial.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize)) + })?; Ok((ss_keymaterial, ct)) } @@ -748,9 +750,9 @@ impl< let mut ss_keymaterial = KeyMaterial::::from_bytes_as_type(&ss_bytes, KeyType::BytesFullEntropy)?; - ss_keymaterial.allow_hazardous_operations(); - ss_keymaterial.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize))?; - ss_keymaterial.drop_hazardous_operations(); + do_hazardous_operations(&mut ss_keymaterial, |ss_keymaterial| { + ss_keymaterial.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize)) + })?; Ok(ss_keymaterial) } diff --git a/crypto/mlkem-lowmemory/src/mlkem_keys.rs b/crypto/mlkem-lowmemory/src/mlkem_keys.rs index b9e58f3..4c310ab 100644 --- a/crypto/mlkem-lowmemory/src/mlkem_keys.rs +++ b/crypto/mlkem-lowmemory/src/mlkem_keys.rs @@ -18,7 +18,9 @@ use crate::mlkem::{ use crate::polynomial::Polynomial; use crate::{ML_KEM_512_NAME, ML_KEM_768_NAME, ML_KEM_1024_NAME}; use bouncycastle_core::errors::KEMError; -use bouncycastle_core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; +use bouncycastle_core::key_material::{ + KeyMaterial, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle_core::traits::{Hash, KEMPrivateKey, KEMPublicKey, Secret, SecurityStrength}; use bouncycastle_sha3::SHA3_256; use core::fmt; @@ -385,15 +387,15 @@ impl< tmp[..32].copy_from_slice(&self.seed_d); tmp[32..].copy_from_slice(&self.z); let mut seed = KeyMaterial::<64>::from_bytes_as_type(&tmp, KeyType::Seed).unwrap(); - seed.allow_hazardous_operations(); - seed.set_security_strength(match k { - 2 => SecurityStrength::_128bit, - 3 => SecurityStrength::_192bit, - 4 => SecurityStrength::_256bit, - _ => unreachable!("Invalid mlkem param set"), + do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(match k { + 2 => SecurityStrength::_128bit, + 3 => SecurityStrength::_192bit, + 4 => SecurityStrength::_256bit, + _ => unreachable!("Invalid mlkem param set"), + }) }) .unwrap(); - seed.drop_hazardous_operations(); Some(seed) } @@ -564,10 +566,10 @@ impl< return Err(KEMError::DecodingError("Invalid seed length")); } let mut keymat = KeyMaterial::<64>::from_bytes(bytes)?; - keymat.allow_hazardous_operations(); - keymat.set_key_type(KeyType::Seed)?; - keymat.set_security_strength(SecurityStrength::_256bit)?; - keymat.drop_hazardous_operations(); + do_hazardous_operations(&mut keymat, |keymat| { + keymat.set_key_type(KeyType::Seed)?; + keymat.set_security_strength(SecurityStrength::_256bit) + })?; Self::new(&keymat) } diff --git a/crypto/mlkem-lowmemory/tests/bc_test_data.rs b/crypto/mlkem-lowmemory/tests/bc_test_data.rs index 8379b2e..beb259c 100644 --- a/crypto/mlkem-lowmemory/tests/bc_test_data.rs +++ b/crypto/mlkem-lowmemory/tests/bc_test_data.rs @@ -6,7 +6,9 @@ #[cfg(test)] mod bc_test_data { - use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; + use bouncycastle_core::key_material::{ + KeyMaterial512, KeyMaterialTrait, KeyType, do_hazardous_operations, + }; use bouncycastle_core::traits::{KEMPublicKey, SecurityStrength}; use bouncycastle_hex as hex; use bouncycastle_mlkem_lowmemory::mlkem::{ @@ -157,10 +159,11 @@ mod bc_test_data { let mut seed = KeyMaterial512::from_bytes_as_type(&seed_bytes, KeyType::Seed).unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed)?; + seed.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); match self.parameter_set.as_str() { "ML-KEM-512" => { diff --git a/crypto/mlkem-lowmemory/tests/mlkem_tests.rs b/crypto/mlkem-lowmemory/tests/mlkem_tests.rs index 71cc449..2922f13 100644 --- a/crypto/mlkem-lowmemory/tests/mlkem_tests.rs +++ b/crypto/mlkem-lowmemory/tests/mlkem_tests.rs @@ -2,7 +2,9 @@ #[cfg(test)] mod mlkem_tests { use bouncycastle_core::errors::KEMError; - use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; + use bouncycastle_core::key_material::{ + KeyMaterial512, KeyMaterialTrait, KeyType, do_hazardous_operations, + }; use bouncycastle_core::traits::{ KEMDecapsulator, KEMEncapsulator, KEMPrivateKey, KEMPublicKey, SecurityStrength, XOF, }; @@ -336,8 +338,8 @@ mod mlkem_tests { assert_eq!(derived_pk.encode(), expected_pk_bytes.as_slice()); // success case KeyType: BytesFullEntropy - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::BytesFullEntropy).unwrap(); + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::BytesFullEntropy)) + .unwrap(); _ = MLKEM512::keygen_from_seed(&seed).unwrap(); // Failure case: key type != Seed || BytesFullEntropy diff --git a/crypto/mlkem-lowmemory/tests/wycheproof.rs b/crypto/mlkem-lowmemory/tests/wycheproof.rs index f5570c6..a026b9b 100644 --- a/crypto/mlkem-lowmemory/tests/wycheproof.rs +++ b/crypto/mlkem-lowmemory/tests/wycheproof.rs @@ -24,7 +24,9 @@ #![allow(dead_code)] -use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; +use bouncycastle_core::key_material::{ + KeyMaterial512, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle_core::traits::{KEMDecapsulator, KEMPublicKey, SecurityStrength}; use bouncycastle_hex as hex; use bouncycastle_mlkem_lowmemory::{ @@ -567,9 +569,10 @@ impl MLKEMTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::Seed)).unwrap(); + match do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(SecurityStrength::_256bit) + }) { Ok(_) => (), Err(e) => { if self.result == "invalid" { @@ -631,9 +634,10 @@ impl MLKEMTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::Seed)).unwrap(); + match do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(SecurityStrength::_256bit) + }) { Ok(_) => (), Err(e) => { if self.result == "invalid" { @@ -695,9 +699,10 @@ impl MLKEMTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { + do_hazardous_operations(&mut seed, |seed| seed.set_key_type(KeyType::Seed)).unwrap(); + match do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(SecurityStrength::_256bit) + }) { Ok(_) => (), Err(e) => { if self.result == "invalid" { diff --git a/crypto/mlkem/src/mlkem.rs b/crypto/mlkem/src/mlkem.rs index d370ea4..f988f6f 100644 --- a/crypto/mlkem/src/mlkem.rs +++ b/crypto/mlkem/src/mlkem.rs @@ -140,7 +140,9 @@ use crate::mlkem_keys::{ use crate::mlkem_keys::{MLKEMPrivateKeyInternalTrait, MLKEMPrivateKeyTrait}; use crate::polynomial::Polynomial; use bouncycastle_core::errors::KEMError; -use bouncycastle_core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; +use bouncycastle_core::key_material::{ + KeyMaterial, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle_core::traits::{ Algorithm, Hash, KEMDecapsulator, KEMEncapsulator, RNG, SecurityStrength, XOF, }; @@ -775,9 +777,9 @@ impl< let (ss, ct) = Self::encaps_internal(&pk.ek, Some(&pk.A_hat), m); let mut key = KeyMaterial::::from_bytes_as_type(&ss, KeyType::BytesFullEntropy)?; - key.allow_hazardous_operations(); - key.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize))?; - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| { + key.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize)) + })?; Ok((key, ct)) } @@ -806,9 +808,9 @@ impl< let K = Self::decaps_internal(&sk.dk, Some(&sk.A_hat), ct.try_into().unwrap()); let mut key = KeyMaterial::::from_bytes_as_type(&K, KeyType::BytesFullEntropy)?; - key.allow_hazardous_operations(); - key.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize))?; - key.drop_hazardous_operations(); + do_hazardous_operations(&mut key, |key| { + key.set_security_strength(SecurityStrength::from_bits(LAMBDA as usize)) + })?; Ok(key) } diff --git a/crypto/mlkem/src/mlkem_keys.rs b/crypto/mlkem/src/mlkem_keys.rs index 327878c..10fc00b 100644 --- a/crypto/mlkem/src/mlkem_keys.rs +++ b/crypto/mlkem/src/mlkem_keys.rs @@ -6,6 +6,7 @@ use crate::mlkem::{MLKEM768_PK_LEN, MLKEM768_SK_LEN, MLKEM768_k}; use crate::mlkem::{MLKEM1024_PK_LEN, MLKEM1024_SK_LEN, MLKEM1024_k}; use crate::{ML_KEM_512_NAME, ML_KEM_768_NAME, ML_KEM_1024_NAME}; use bouncycastle_core::errors::KEMError; +use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{Hash, KEMPrivateKey, KEMPublicKey, Secret, SecurityStrength}; use bouncycastle_sha3::SHA3_256; @@ -469,15 +470,17 @@ impl< tmp[..32].copy_from_slice(&self.seed_d.unwrap()); tmp[32..].copy_from_slice(&self.z); let mut seed = KeyMaterial::<64>::from_bytes_as_type(&tmp, KeyType::Seed).unwrap(); - seed.allow_hazardous_operations(); - seed.set_security_strength(match k { - 2 => SecurityStrength::_128bit, - 3 => SecurityStrength::_192bit, - 4 => SecurityStrength::_256bit, - _ => unreachable!("Invalid mlkem param set"), + + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_security_strength(match k { + 2 => SecurityStrength::_128bit, + 3 => SecurityStrength::_192bit, + 4 => SecurityStrength::_256bit, + _ => unreachable!("Invalid mlkem param set"), + }) }) .unwrap(); - seed.drop_hazardous_operations(); + Some(seed) } } diff --git a/crypto/mlkem/tests/bc_test_data.rs b/crypto/mlkem/tests/bc_test_data.rs index ef40434..bfb0a64 100644 --- a/crypto/mlkem/tests/bc_test_data.rs +++ b/crypto/mlkem/tests/bc_test_data.rs @@ -4,6 +4,7 @@ #[cfg(test)] mod bc_test_data { + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{ KEMDecapsulator, KEMPrivateKey, KEMPublicKey, SecurityStrength, @@ -155,10 +156,11 @@ mod bc_test_data { let mut seed = KeyMaterial512::from_bytes_as_type(&seed_bytes, KeyType::Seed).unwrap(); // for the purposes of the test cases, accept an all-zero seed - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); - seed.drop_hazardous_operations(); + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed)?; + seed.set_security_strength(SecurityStrength::_256bit) + }) + .unwrap(); match self.parameter_set.as_str() { "ML-KEM-512" => { diff --git a/crypto/mlkem/tests/mlkem_tests.rs b/crypto/mlkem/tests/mlkem_tests.rs index d5aa274..1a9da27 100644 --- a/crypto/mlkem/tests/mlkem_tests.rs +++ b/crypto/mlkem/tests/mlkem_tests.rs @@ -2,6 +2,7 @@ #[cfg(test)] mod mlkem_tests { use bouncycastle_core::errors::KEMError; + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{ KEMDecapsulator, KEMEncapsulator, KEMPrivateKey, KEMPublicKey, SecurityStrength, XOF, @@ -322,8 +323,11 @@ mod mlkem_tests { assert_eq!(derived_pk.encode(), expected_pk_bytes.as_slice()); // success case KeyType: BytesFullEntropy - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::BytesFullEntropy).unwrap(); + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::BytesFullEntropy) + }) + .unwrap(); + _ = MLKEM512::keygen_from_seed(&seed).unwrap(); // Failure case: key type != Seed || BytesFullEntropy diff --git a/crypto/mlkem/tests/wycheproof.rs b/crypto/mlkem/tests/wycheproof.rs index a141e63..cb50268 100644 --- a/crypto/mlkem/tests/wycheproof.rs +++ b/crypto/mlkem/tests/wycheproof.rs @@ -20,6 +20,7 @@ #![allow(dead_code)] +use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{KEMDecapsulator, KEMPrivateKey, KEMPublicKey, SecurityStrength}; use bouncycastle_hex as hex; @@ -717,19 +718,21 @@ impl MLKEMTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { - Ok(_) => (), - Err(e) => { - if self.result == "invalid" { - /* good */ - return; - } else { - panic!("{:?}", e) + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + match seed.set_security_strength(SecurityStrength::_256bit) { + Ok(_) => Ok(()), + Err(e) => { + if self.result == "invalid" { + /* good */ + Ok(()) + } else { + panic!("{:?}", e) + } } } - } + }) + .unwrap(); let (ek, dk) = match MLKEM512::keygen_from_seed(&seed) { Ok((ek, dk)) => (ek, dk), @@ -781,19 +784,21 @@ impl MLKEMTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { - Ok(_) => (), - Err(e) => { - if self.result == "invalid" { - /* good */ - return; - } else { - panic!("{:?}", e) + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + match seed.set_security_strength(SecurityStrength::_256bit) { + Ok(_) => Ok(()), + Err(e) => { + if self.result == "invalid" { + /* good */ + Ok(()) + } else { + panic!("{:?}", e) + } } } - } + }) + .unwrap(); let (ek, dk) = match MLKEM768::keygen_from_seed(&seed) { Ok((ek, dk)) => (ek, dk), @@ -845,19 +850,21 @@ impl MLKEMTestCase { } }; // allow an all-zero seed for testing - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match seed.set_security_strength(SecurityStrength::_256bit) { - Ok(_) => (), - Err(e) => { - if self.result == "invalid" { - /* good */ - return; - } else { - panic!("{:?}", e) + key_material::do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + match seed.set_security_strength(SecurityStrength::_256bit) { + Ok(_) => Ok(()), + Err(e) => { + if self.result == "invalid" { + /* good */ + Ok(()) + } else { + panic!("{:?}", e) + } } } - } + }) + .unwrap(); let (ek, dk) = match MLKEM1024::keygen_from_seed(&seed) { Ok((ek, dk)) => (ek, dk), diff --git a/crypto/rng/src/hash_drbg80090a.rs b/crypto/rng/src/hash_drbg80090a.rs index 75b4177..2d51432 100644 --- a/crypto/rng/src/hash_drbg80090a.rs +++ b/crypto/rng/src/hash_drbg80090a.rs @@ -6,7 +6,9 @@ use crate::Sp80090ADrbg; use bouncycastle_core::errors::{KeyMaterialError, RNGError}; -use bouncycastle_core::key_material::{KeyMaterial512, KeyMaterialTrait, KeyType}; +use bouncycastle_core::key_material::{ + KeyMaterial512, KeyMaterialTrait, KeyType, do_hazardous_operations, +}; use bouncycastle_core::traits::{Hash, HashAlgParams, RNG, SecurityStrength}; use bouncycastle_sha2::{SHA256, SHA512}; use bouncycastle_utils::min; @@ -141,20 +143,23 @@ impl HashDRBG80090A { /// Creates a new instance using the local OS RNG as a source of seed entropy. pub fn new_from_os() -> Self { let mut seed = KeyMaterial512::new(); - seed.allow_hazardous_operations(); - seed.set_key_type(KeyType::Seed).unwrap(); - match H::HASH { - SupportedHash::SHA256 => { - getrandom::fill(&mut seed.mut_ref_to_bytes().unwrap()[..32]).unwrap(); - seed.set_key_len(32).unwrap(); - seed.set_security_strength(SecurityStrength::_128bit).unwrap(); - } - SupportedHash::SHA512 => { - getrandom::fill(&mut seed.mut_ref_to_bytes().unwrap()).unwrap(); - seed.set_key_len(64).unwrap(); - seed.set_security_strength(SecurityStrength::_256bit).unwrap(); + do_hazardous_operations(&mut seed, |seed| { + seed.set_key_type(KeyType::Seed).unwrap(); + match H::HASH { + SupportedHash::SHA256 => { + getrandom::fill(&mut seed.mut_ref_to_bytes().unwrap()[..32]).unwrap(); + seed.set_key_len(32).unwrap(); + seed.set_security_strength(SecurityStrength::_128bit).unwrap(); + } + SupportedHash::SHA512 => { + getrandom::fill(&mut seed.mut_ref_to_bytes().unwrap()).unwrap(); + seed.set_key_len(64).unwrap(); + seed.set_security_strength(SecurityStrength::_256bit).unwrap(); + } } - } + Ok(()) + }) + .unwrap(); let mut rng = Self::new_unititialized(); let ss = seed.security_strength().clone(); @@ -462,15 +467,27 @@ impl Sp80090ADrbg for HashDRBG80090A { additional_input: &[u8], out: &mut impl KeyMaterialTrait, ) -> Result { - out.allow_hazardous_operations(); - let bytes_written = self.generate_out(additional_input, out.mut_ref_to_bytes().unwrap())?; - - out.set_key_len(bytes_written)?; - out.set_key_type(KeyType::BytesFullEntropy)?; - let new_security_strength = - min(&self.admin_info.strength, &SecurityStrength::from_bits(bytes_written * 8)).clone(); - out.set_security_strength(new_security_strength)?; - out.drop_hazardous_operations(); + let mut ret: Result = Ok(0); + do_hazardous_operations(out, |out| { + let out_ref = out.mut_ref_to_bytes()?; + ret = self.generate_out(additional_input, out_ref); + Ok(()) + })?; + + let bytes_written = match ret { + Err(e) => return Err(e), + Ok(bytes_written) => bytes_written, + }; + + do_hazardous_operations(out, |out| { + out.set_key_len(bytes_written)?; + out.set_key_type(KeyType::BytesFullEntropy)?; + let new_security_strength = + min(&self.admin_info.strength, &SecurityStrength::from_bits(bytes_written * 8)) + .clone(); + out.set_security_strength(new_security_strength)?; + Ok(()) + })?; Ok(bytes_written) } } diff --git a/crypto/rng/tests/hash_drbg80090a_tests.rs b/crypto/rng/tests/hash_drbg80090a_tests.rs index e4fd534..d019e40 100644 --- a/crypto/rng/tests/hash_drbg80090a_tests.rs +++ b/crypto/rng/tests/hash_drbg80090a_tests.rs @@ -286,7 +286,8 @@ mod tests { let mut out = KeyMaterial256::new(); match rng.generate_keymaterial_out(&[], &mut out) { Err(RNGError::Uninitialized) => { /*good*/ } - _ => panic!("Expected Uninitialized error"), + Err(e) => panic!("{:?}", e), + Ok(_) => panic!("Expected Uninitialized error"), } // Skipping tests for max lengths of seeds and personalization strings, because they're in the gigabyte range and that'll blow up the test suite. diff --git a/crypto/sha3/src/sha3.rs b/crypto/sha3/src/sha3.rs index cb22998..c28dab6 100644 --- a/crypto/sha3/src/sha3.rs +++ b/crypto/sha3/src/sha3.rs @@ -1,6 +1,7 @@ use crate::SHA3Params; use crate::keccak::KeccakDigest; use bouncycastle_core::errors::{HashError, KDFError}; +use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{Hash, KDF, SecurityStrength}; use bouncycastle_utils::{max, min}; @@ -81,20 +82,34 @@ impl SHA3 { let mut key_type = self.kdf_key_type.clone(); let output_security_strength = self.kdf_security_strength.clone(); - output_key.allow_hazardous_operations(); - let bytes_written = self.do_final_out(output_key.mut_ref_to_bytes()?); - output_key.set_key_len(bytes_written)?; - - // since we've done some computation, the result will not actually be zeroized, even if all input key material was zeroized. + let mut bytes_written: usize = 0; + key_material::do_hazardous_operations(output_key, |output_key| { + bytes_written = self.do_final_out(output_key.mut_ref_to_bytes()?); + output_key.set_key_len(bytes_written)?; + Ok(()) + }) + .expect( + "both mut_ref_to_bytes() and set_key_len() should be infallible within a hazop block", + ); + + // since we've done some computation, the result will not actually be zeroized, + // even if all input key material was zeroized. if key_type == KeyType::Zeroized { key_type = KeyType::BytesLowEntropy; } - output_key.set_key_type(key_type)?; - output_key.set_security_strength( - min(&output_security_strength, &SecurityStrength::from_bits(bytes_written * 8)).clone(), - )?; - output_key.drop_hazardous_operations(); - output_key.truncate(min(&output_key.key_len(), &PARAMS::OUTPUT_LEN).clone())?; + key_material::do_hazardous_operations(&mut *output_key, |output_key| { + output_key.set_key_type(key_type)?; + output_key.set_security_strength( + min(&output_security_strength, &SecurityStrength::from_bits(bytes_written * 8)).clone(), + ) + }) + .expect( + "both set_key_type() and set_security_strength() should be infallible within a hazop block", + ); + + output_key + .set_key_len(min(&output_key.key_len(), &PARAMS::OUTPUT_LEN).clone()) + .expect("should be infallible to truncate key length"); Ok(bytes_written) } } diff --git a/crypto/sha3/src/shake.rs b/crypto/sha3/src/shake.rs index 59f3bac..c57ee91 100644 --- a/crypto/sha3/src/shake.rs +++ b/crypto/sha3/src/shake.rs @@ -1,6 +1,7 @@ use crate::SHAKEParams; -use crate::keccak::KeccakDigest; +use crate::keccak::{KeccakDigest, KeccakSize}; use bouncycastle_core::errors::{HashError, KDFError}; +use bouncycastle_core::key_material; use bouncycastle_core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType}; use bouncycastle_core::traits::{Algorithm, KDF, SecurityStrength, XOF}; use bouncycastle_utils::{max, min}; @@ -87,8 +88,13 @@ impl SHAKE { let mut output_key = KeyMaterial::<64>::new(); self.derive_key_out_final_internal(additional_input, &mut output_key)?; + // truncate // 128 => 32, 256 => 64 - output_key.truncate(2 * (PARAMS::SIZE as usize) / 8)?; + match PARAMS::SIZE { + KeccakSize::_128 => output_key.set_key_len(32).expect("truncate should be infallible"), + KeccakSize::_256 => output_key.set_key_len(64).expect("truncate should be infallible"), + _ => unreachable!(), + } Ok(Box::new(output_key)) } @@ -109,25 +115,25 @@ impl SHAKE { self.absorb(additional_input); - // let mut buf = [0u8; 64]; - output_key.allow_hazardous_operations(); - let bytes_written = self.squeeze_out( - output_key - .mut_ref_to_bytes() - .expect("We just set .allow_hazardous_operations(), so this should be fine."), - ); - output_key.set_key_len(bytes_written)?; + let mut bytes_written: usize = 0; + key_material::do_hazardous_operations(output_key, |output_key| { + bytes_written = self.squeeze_out( + output_key.mut_ref_to_bytes().expect("Infallible within do_hazardous_operations"), + ); + output_key.set_key_len(bytes_written) + })?; // since we've done some computation, the result will not actually be zeroized, even if all input key material was zeroized. if self.kdf_key_type == KeyType::Zeroized { self.kdf_key_type = KeyType::BytesLowEntropy; } - output_key.set_key_type(self.kdf_key_type)?; - output_key.set_security_strength( - min(&self.kdf_security_strength, &SecurityStrength::from_bits(bytes_written * 8)) - .clone(), - )?; - output_key.drop_hazardous_operations(); + key_material::do_hazardous_operations(output_key, |output_key| { + output_key.set_key_type(self.kdf_key_type)?; + output_key.set_security_strength( + min(&self.kdf_security_strength, &SecurityStrength::from_bits(bytes_written * 8)) + .clone(), + ) + })?; Ok(bytes_written) } } diff --git a/crypto/sha3/tests/sha3_tests.rs b/crypto/sha3/tests/sha3_tests.rs index 5fb5cae..0065726 100644 --- a/crypto/sha3/tests/sha3_tests.rs +++ b/crypto/sha3/tests/sha3_tests.rs @@ -1,6 +1,7 @@ #[cfg(test)] mod sha3_tests { use super::sha3_test_helpers::*; + use bouncycastle_core::key_material; use bouncycastle_core::key_material::{ KeyMaterial, KeyMaterial256, KeyMaterial512, KeyMaterialTrait, KeyType, }; @@ -344,7 +345,7 @@ mod sha3_tests { let input_seed = KeyMaterial256::from_bytes(&DUMMY_SEED_512[..32]).expect("Error happened"); let mut output_seed = SHA3_256::new().derive_key(&input_seed, b"nytimes.com").expect("Error happened"); - match output_seed.convert_key_type(KeyType::MACKey) { + match output_seed.set_key_type(KeyType::MACKey) { Ok(_) => { panic!( "Should have failed to convert key type because the input was BytesLowEntropy" @@ -358,10 +359,10 @@ mod sha3_tests { let mut output_seed = SHA3_256::new() .derive_key(&input_seed, b"some addtional input to the KDF") .expect("Error happened"); - output_seed.allow_hazardous_operations(); - output_seed.convert_key_type(KeyType::MACKey).unwrap(); - output_seed.drop_hazardous_operations(); // not strictly necessary because KeyMaterial - // will automatically drop allow_hazardous_conversions() after performing one. + key_material::do_hazardous_operations(&mut *output_seed, |output_seed| { + output_seed.set_key_type(KeyType::MACKey) + }) + .unwrap(); assert_eq!(output_seed.key_type(), KeyType::MACKey); // This works because we explicitly tag the input data as BytesFullEntropy.