Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cli/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub(crate) fn read_from_file(filename: &str) -> Vec<u8> {
match hex::decode(&buf) {
Ok(decoded) => decoded,
Err(_) => {
// well, it's not hex, so return it raw
// it's not hex, so return it raw
buf
}
}
Expand Down Expand Up @@ -47,7 +47,7 @@ pub(crate) fn read_from_file_or_stdin(filename: &Option<String>) -> Vec<u8> {
match hex::decode(&buf) {
Ok(decoded) => decoded,
Err(_) => {
// well, it's not hex, so return it raw
// it's not hex, so return it raw
buf
}
}
Expand Down Expand Up @@ -98,7 +98,7 @@ pub(crate) fn parse_seed<const SEED_LEN: usize>(bytes: &[u8]) -> Result<KeyMater
}
};

// I think I've checked for all the error conditions, so this shouldn't fail.
// TODO: Verify that all error conditions have been checked
let mut seed = KeyMaterial::<SEED_LEN>::from_bytes_as_type(&seed_bytes, KeyType::Seed).unwrap();

if seed.key_type() == KeyType::Zeroized || seed.security_strength() < SecurityStrength::_256bit
Expand Down
4 changes: 2 additions & 2 deletions cli/src/mldsa_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Yup, this file is as absolutely atrocious mess of duplicate code that could be much improved
//! by using generics or macros. I just, haven't ... yet.
//! Work in progress.
//! TODO: Use generic macros to eliminate duplicated code.

use crate::helpers::{parse_seed, read_from_file, read_from_file_or_stdin, write_bytes_or_hex};
use bouncycastle::core::traits::{Signature, SignaturePrivateKey, SignaturePublicKey};
Expand Down
9 changes: 8 additions & 1 deletion crypto/core/src/key_material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use bouncycastle_utils::{ct, min};
use core::cmp::{Ordering, PartialOrd};
use core::fmt;

/// Sometimes you just need a zero-length dummy key.
/// When it is necessary to get a zero-length dummy key.
pub type KeyMaterial0 = KeyMaterial<0>;

pub type KeyMaterial128 = KeyMaterial<16>;
Expand Down Expand Up @@ -389,16 +389,19 @@ impl<const KEY_LEN: usize> KeyMaterialTrait for KeyMaterial<KEY_LEN> {
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);
}
self.key_type = key_type.clone();
Ok(())
}

fn security_strength(&self) -> SecurityStrength {
self.security_strength.clone()
}
Expand Down Expand Up @@ -453,6 +456,7 @@ impl<const KEY_LEN: usize> KeyMaterialTrait for KeyMaterial<KEY_LEN> {
self.drop_hazardous_operations();
Ok(())
}

/// 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.
///
Expand All @@ -463,10 +467,12 @@ impl<const KEY_LEN: usize> KeyMaterialTrait for KeyMaterial<KEY_LEN> {
fn allow_hazardous_operations(&mut self) {
self.allow_hazardous_operations = true;
}

/// Resets this instance to not be able to perform potentially hazardous operations.
fn drop_hazardous_operations(&mut self) {
self.allow_hazardous_operations = false;
}

/// 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
Expand Down Expand Up @@ -529,6 +535,7 @@ impl<const KEY_LEN: usize> KeyMaterialTrait for KeyMaterial<KEY_LEN> {
self.drop_hazardous_operations();
Ok(())
}

fn is_full_entropy(&self) -> bool {
match self.key_type {
KeyType::BytesFullEntropy
Expand Down
30 changes: 16 additions & 14 deletions crypto/core/tests/key_material_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ mod test_key_material {
_ => panic!("Expected InvalidLength"),
}

// But you can slice it down.
// This can be sliced down.
match KeyMaterial512::from_bytes(&DUMMY_KEY_TOO_LONG[..64]) {
Ok(key) => assert_eq!(key.key_len(), 64),
_ => panic!("Expected InvalidLength"),
Expand All @@ -47,7 +47,7 @@ mod test_key_material {
assert_eq!(key.key_type(), KeyType::Zeroized);
assert_eq!(key.key_len(), 16);

// ... but we can force it.
// however, it can be forced by allowing hazardous operations.
key.allow_hazardous_operations();
key.set_key_type(KeyType::BytesLowEntropy).unwrap();
key.drop_hazardous_operations();
Expand All @@ -59,13 +59,12 @@ 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.
// it can be enabled by allowing hazardous operations first.
let key_bytes = [0u8; 16];
let mut key = KeyMaterial256::new();
key.allow_hazardous_operations();
key.set_bytes_as_type(&key_bytes, KeyType::BytesLowEntropy).unwrap();
assert_eq!(key.key_type(), KeyType::BytesLowEntropy);

// nothing else requires setting hazardous operations.
}

Expand All @@ -89,7 +88,7 @@ mod test_key_material {
assert_eq!(key.mut_ref_to_bytes().unwrap()[..16], [1u8; 16]);
assert_eq!(key.mut_ref_to_bytes().unwrap()[16..], [0u8; 16]);

// and I can set them
// Then they can be set
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]);
Expand Down Expand Up @@ -247,7 +246,7 @@ mod test_key_material {
assert_eq!(key.key_type(), KeyType::BytesLowEntropy);
assert!(!key.is_full_entropy());

// Note: can't use the usual assert_eq!() here because that requires PartialEq, but we're in a no_std context here.
// Note: the usual assert_eq!() can't be used here because that requires PartialEq, but the current context is no_std.
match key.key_type() {
KeyType::BytesLowEntropy => { /* good */ }
_ => panic!("Expected BytesLowEntropy"),
Expand Down Expand Up @@ -343,12 +342,13 @@ mod test_key_material {
}

let zero_key = KeyMaterial256::from_bytes(&[0u8; 19]).unwrap();
// it should have set the key bytes and length, but also set the key type to Zeroized.
// key bytes and length should be set accordingly,
// as well as the key type should be set to Zeroized.
assert_eq!(zero_key.key_type(), KeyType::Zeroized);
assert_eq!(zero_key.key_len(), 19);
assert_eq!(zero_key.ref_to_bytes(), &[0u8; 19]);

// But it's totally fine if you give it non-zero input data.
// It also admits non-zero input data.
let not_zero_key = KeyMaterial256::from_bytes(&[1u8; 19]).unwrap();
assert_eq!(not_zero_key.key_type(), KeyType::BytesLowEntropy);

Expand All @@ -364,10 +364,10 @@ mod test_key_material {
panic!("should have thrown a KeyMaterialError::ActingOnZeroizedKey error.")
}
}
// but it should still have set the key bytes; it's just giving you a friendly warning
// This should still set the key bytes; only giving a friendly warning that the key is zeroized
assert_eq!(zero_key.key_type(), KeyType::Zeroized);

// ... but will allow it if you set .allow_hazardous_operations() first.
// The operation is allowed by setting .allow_hazardous_operations() first.
let mut zero_key = KeyMaterial256::new();
zero_key.allow_hazardous_operations();
zero_key.set_bytes_as_type(&[0u8; 19], KeyType::MACKey).unwrap();
Expand Down Expand Up @@ -402,7 +402,7 @@ mod test_key_material {
_ => panic!("Expected HazardousConversion"),
}

/* Should work if you allow hazardous conversions. */
/* Should work when hazardous conversions are allowed. */
key = KeyMaterial256::from_bytes(&DUMMY_KEY[..32]).unwrap();
key.allow_hazardous_operations();
key.convert_key_type(KeyType::BytesFullEntropy).unwrap();
Expand Down Expand Up @@ -445,7 +445,8 @@ mod test_key_material {
assert_eq!(key1.key_type(), KeyType::MACKey);
assert_eq!(key1.security_strength(), SecurityStrength::_256bit);

// success case: same size using default From impl; only works if the sizes are the same (ie the compiler knows that they are the same type.
// success case: same size using default From impl;
// only works if the sizes are the same (i.e. the compiler knows that they are the same type).
let key2 = KeyMaterial256::from(key1.clone());
assert_eq!(key1.key_len(), key2.key_len());
assert_eq!(key1.key_type(), key2.key_type());
Expand Down Expand Up @@ -490,7 +491,7 @@ mod test_key_material {
_ => panic!("Expected HazardousConversion"),
}

// should work if you allow hazardous conversions.
// should work when hazardous conversions are allowed.
key.allow_hazardous_operations();
key.convert_key_type(KeyType::SymmetricCipherKey).unwrap();
}
Expand Down Expand Up @@ -540,7 +541,7 @@ mod test_key_material {
assert_eq!(key.key_type(), KeyType::BytesFullEntropy);
assert_eq!(key.security_strength(), SecurityStrength::None);

// even if it's long enough, BytesLowEntropy or Zeroized always get ::None
// even if it's long enough, BytesLowEntropy or Zeroized always get ::None as security strength
let key = KeyMaterial512::from_bytes_as_type(DUMMY_KEY, KeyType::BytesLowEntropy).unwrap();
assert_eq!(key.key_type(), KeyType::BytesLowEntropy);
assert_eq!(key.key_len(), 64);
Expand Down Expand Up @@ -683,6 +684,7 @@ mod test_key_material {
b"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F",
)
.unwrap();

let key2 = KeyMaterial256::from_bytes(
b"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F",
)
Expand Down
10 changes: 5 additions & 5 deletions crypto/hkdf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ use bouncycastle_core::traits::XOF;

/*** Constants ***/
// Slightly hacky, but set this to accomodate the underlying hash primitive with the largest output size.
// Would be better to somehow pull that at compile time from H, but I'm not sure how to do that.
// It would be better to pull that at compile time from H, but the implementation does not currently do that.
const HMAC_BLOCK_LEN: usize = 64;

/*** String constants ***/
Expand All @@ -179,7 +179,7 @@ pub type HKDF_SHA256 = HKDF<SHA256>;
pub type HKDF_SHA512 = HKDF<SHA512>;

pub struct HKDF<H: Hash + HashAlgParams + Default> {
hmac: Option<HMAC<H>>, // Optional because we can't construct an HMAC until they give us a key
hmac: Option<HMAC<H>>, // Optional because an HMAC cannot be constructed until a key is provided
// to initialize it with.
// None should correspond to a state of Uninitialized.
entropy: HkdfEntropyTracker<H>,
Expand Down Expand Up @@ -239,7 +239,7 @@ impl<H: Hash + HashAlgParams + Default> HkdfEntropyTracker<H> {
}
}

// Since I don't want this struct to be public, the tests have to go here.
// Because this struct is not public, the tests have to go here.
#[test]
fn test_entropy_tracker() {
let mut entropy = HkdfEntropyTracker::<SHA256>::new();
Expand Down Expand Up @@ -400,7 +400,7 @@ impl<H: Hash + HashAlgParams + Default> HKDF<H> {
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.
// The prk key type must be temporarily changed to MACKey to satisfy HMAC, then restored afterwards.
let prk_as_mac_key =
KeyMaterial::<HMAC_BLOCK_LEN>::from_bytes_as_type(prk.ref_to_bytes(), KeyType::MACKey)?;

Expand Down Expand Up @@ -487,7 +487,7 @@ impl<H: Hash + HashAlgParams + Default> HKDF<H> {
};

// Often HMAC is initialized with a zero salt,
// So we're gonna ignore key strength errors here
// Key strength errors are ignored here.
// This will all be tabulated correctly via entropy.credit_entropy()
self.hmac = Some(HMAC::<H>::new_allow_weak_key(salt)?);

Expand Down
14 changes: 7 additions & 7 deletions crypto/mlkem/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,13 @@
//! | ML-KEM-1024_expanded | 1568 | 10272 | 3168 | 12418 |
//!
//! All values are in bytes. The "in memory" sizes are measured by rust's `std::mem::size_of`.
//! Values in parentheses are the usual sizes in our un-optimized implementation in the \[bouncycastle_mldsa] crate.
//! Values in parentheses are the usual sizes in the un-optimized implementation in the \[bouncycastle_mldsa] crate.
//!
//! # Security
//! All functionality exposed by this crate is considered secure to use.
//! In other words, this crate does not contain any "hazmat" except for the obvious points about
//! handling your private keys properly: if you post your private key to github, or you generate
//! production keys from a weak seed, I can't help you, that's on you.
//! production keys from a weak seed, that use is unsupported
//! It is worth mentioning, however, that if using a [MLKEM::keygen_from_seed], then it is your
//! responsibility to ensure that the seed is cryptographically random and unpredictable.
//! And also that [MLKEM::encaps_internal] requires you to provide the randomness, so the ciphertext
Expand All @@ -133,8 +133,8 @@
//! constructions. That should give this implementation reasonably good resistance to timing and
//! power analysis key extraction attacks, however: A) this is a "best-effort" and not formally verified,
//! and B) the Rust compiler does not guarantee constant-time behaviour no matter how clever your code,
//! so like all Safe Rust code (ie Rust code that does not include inline assembly), we are at the mercy
//! of the Rust compiler's optimizer for whether our bitshift-and-xor code actually remains
//! so like all Safe Rust code (ie Rust code that does not include inline assembly), the Rust compiler's optimizer
//! determines whether the bitshift-and-xor code actually remains
//! constant-time after compilation.

#![no_std]
Expand All @@ -143,13 +143,13 @@
#![allow(incomplete_features)] // needed because currently generic_const_exprs is experimental
#![feature(generic_const_exprs)]
#![feature(adt_const_params)]
// These are because I'm matching variable names exactly against FIPS 204, for example both 'K' and 'k',
// These are because variable names are matched exactly against FIPS 204, for example both 'K' and 'k',
// or 'A' and 'a' are used and have specific meanings.
// But need to tell the rust linter to not care.
#![allow(non_snake_case)]
#![allow(non_upper_case_globals)]
// so I can use private traits to hide internal stuff that needs to be generic within the
// MLKEM implementation, but I don't want accessed from outside, such as FIPS-internal functions.
// so private traits can hide internal items that need to be generic within the
// MLKEM implementation, but should not be accessed from outside, such as FIPS-internal functions.
#![allow(private_bounds)]

// imports needed just for docs
Expand Down
11 changes: 6 additions & 5 deletions src/bench_mldsa_mem_usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
//!
//! > ms_print massif.out.835000
//!
//! or, shoved all into one line:
//! alternatively, as a one line command:
//!
//! > clear; clear; valgrind --tool=massif --heap=no --stacks=yes -- target/release/bench_mldsa_mem_usage > /dev/null; ms_print massif.out.*; rm massif.out.*
//!
//! Make sure you build in release mode!
//!
//! Note: I'm using print!() to force the compiler not to optimize away the actual code.
//! I'm printing the important stuff for benchmarking to stderr so that I can pipe the junk to /dev/null
//! (I'm not doing it the other way because /usr/bin/time prints its useful stuff to stderr as well)
//! Note:
//! The code is using print!() to force the compiler not to optimize away the actual code.
//! It is printing important outputs for benchmarking to stderr so that the rest can be mapped to /dev/null
//! (this is because /usr/bin/time prints useful outputs to stderr as well)
//!
//! Main is at the bottom, controls which this was actually run.

Expand All @@ -25,7 +26,7 @@ use bouncycastle_core_interface::traits::{Signature, SignaturePublicKey};
use bouncycastle_hex as hex;
use bouncycastle_mldsa::MLDSA44PublicKey;

/// This exists so I can use /usr/bin/time to measure the base memory footprint of the cargo bench harness
/// This exists so that /usr/bin/time can be used to measure the base memory footprint of the cargo bench harness
fn bench_do_nothing() {
eprintln!("DoNothing");

Expand Down