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
29 changes: 15 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
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