Added a closure-based do_hazardous_operations()

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)

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<const SEED_LEN: usize>(bytes: &[u8]) -> Result<KeyMater
         eprintln!(
             "Warning: low entropy seed provided. We'll still process it, but it may be insecure."
         );
-        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();
     }
     Ok(seed)
 }

cli/src/hkdf_cmd.rs

@@ -2,7 +2,9 @@ use std::io::Write;
 use std::process::exit;
 use std::{fs, io};
 
-use bouncycastle::core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType};
+use bouncycastle::core::key_material::{
+    KeyMaterial, KeyMaterialTrait, KeyType, do_hazardous_operations,
+};
 use bouncycastle::hex;
 use bouncycastle::hkdf;
 
@@ -43,8 +45,8 @@ pub(crate) fn hkdf_cmd(
     }
     let mut salt_key = KeyMaterial::<1024>::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()

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 {

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());
 

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!(