From a1d6ed0223f69c83d0ef525f7c05301ee6aa87b1 Mon Sep 17 00:00:00 2001 From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> Date: Tue, 11 Apr 2023 14:48:42 +0200 Subject: [PATCH] Makes our CredRandom derivation FIPS compliant (#613) * Makes our CredRandom derivation FIPS compliant This change breaks existing usage of CredRandom. * fixes rust_crypto and HKDF test style --- libraries/crypto/src/hkdf.rs | 80 ++++++++++++++----- libraries/opensk/src/api/crypto/hkdf256.rs | 19 ++++- libraries/opensk/src/api/crypto/mod.rs | 10 +++ .../opensk/src/api/crypto/rust_crypto.rs | 4 +- .../opensk/src/api/crypto/software_crypto.rs | 4 +- libraries/opensk/src/ctap/mod.rs | 9 ++- 6 files changed, 96 insertions(+), 30 deletions(-) diff --git a/libraries/crypto/src/hkdf.rs b/libraries/crypto/src/hkdf.rs index baaa523..2405261 100644 --- a/libraries/crypto/src/hkdf.rs +++ b/libraries/crypto/src/hkdf.rs @@ -17,6 +17,27 @@ use super::Hash256; const HASH_SIZE: usize = 32; +/// Computes the HKDF with the given salt and 256 bit (one block) output. +/// +/// # Arguments +/// +/// * `ikm` - Input keying material +/// * `salt` - Byte string that acts as a key +/// * `info` - Optional context and application specific information +/// +/// This implementation is equivalent to a standard HKD, with `salt` fixed at a length of +/// 32 byte and the output length l as 32. +pub fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8]) -> [u8; HASH_SIZE] +where + H: Hash256, +{ + let prk = hmac_256::(salt, ikm); + // l is implicitly the block size, so we iterate exactly once. + let mut t = info.to_vec(); + t.push(1); + hmac_256::(&prk, t.as_slice()) +} + /// Computes the HKDF with empty salt and 256 bit (one block) output. /// /// # Arguments @@ -31,11 +52,7 @@ where H: Hash256, { // Salt is a zero block here. - let prk = hmac_256::(&[0; HASH_SIZE], ikm); - // l is implicitly the block size, so we iterate exactly once. - let mut t = info.to_vec(); - t.push(1); - hmac_256::(&prk, t.as_slice()) + hkdf_256::(ikm, &[0; HASH_SIZE], info) } #[cfg(test)] @@ -49,32 +66,53 @@ mod test { // Test vectors generated by pycryptodome using: // HKDF(b'0', 32, b'', SHA256, context=b'\x00').hex() let test_okms = [ - hex::decode("f9be72116cb97f41828210289caafeabde1f3dfb9723bf43538ab18f3666783a") - .unwrap(), - hex::decode("f50f964f5b94d62fd1da9356ab8662b0a0f5b8e36e277178b69b6ffecf50cf44") - .unwrap(), - hex::decode("fc8772ceb5592d67442dcb4353cdd28519e82d6e55b4cf664b5685252c2d2998") - .unwrap(), - hex::decode("62831b924839a180f53be5461eeea1b89dc21779f50142b5a54df0f0cc86d61a") - .unwrap(), - hex::decode("6991f00a12946a4e3b8315cdcf0132c2ca508fd17b769f08d1454d92d33733e0") - .unwrap(), - hex::decode("0f9bb7dddd1ec61f91d8c4f5369b5870f9d44c4ceabccca1b83f06fec115e4e3") - .unwrap(), - hex::decode("235367e2ab6cca2aba1a666825458dba6b272a215a2537c05feebe4b80dab709") - .unwrap(), - hex::decode("96e8edad661da48d1a133b38c255d33e05555bc9aa442579dea1cd8d8b8d2aef") - .unwrap(), + "f9be72116cb97f41828210289caafeabde1f3dfb9723bf43538ab18f3666783a", + "f50f964f5b94d62fd1da9356ab8662b0a0f5b8e36e277178b69b6ffecf50cf44", + "fc8772ceb5592d67442dcb4353cdd28519e82d6e55b4cf664b5685252c2d2998", + "62831b924839a180f53be5461eeea1b89dc21779f50142b5a54df0f0cc86d61a", + "6991f00a12946a4e3b8315cdcf0132c2ca508fd17b769f08d1454d92d33733e0", + "0f9bb7dddd1ec61f91d8c4f5369b5870f9d44c4ceabccca1b83f06fec115e4e3", + "235367e2ab6cca2aba1a666825458dba6b272a215a2537c05feebe4b80dab709", + "96e8edad661da48d1a133b38c255d33e05555bc9aa442579dea1cd8d8b8d2aef", ]; for (i, okm) in test_okms.iter().enumerate() { // String of number i. let ikm = i.to_string(); // Byte i. let info = [i as u8]; + let okm = hex::decode(okm).unwrap(); assert_eq!( &hkdf_empty_salt_256::(&ikm.as_bytes(), &info[..]), array_ref!(okm, 0, 32) ); } } + + #[test] + fn test_hkdf_256_sha256_vectors() { + // Test vectors generated as above, but with salt: + let test_okms = [ + "f9be72116cb97f41828210289caafeabde1f3dfb9723bf43538ab18f3666783a", + "a2480a09c7349d76e459f98a8259da40544bfbd2930d357a0f3250ade0acf941", + "3904f7bf3615df9512fc6b1af651ed69b43f7fad424f9c718aaab63f377a36b9", + "a0027dcffb27d356317199c6e65f153a9286ba114aee2d3cf45bdba83cb7c065", + "786d1f89f54668bac443cc6a8887c95d6fbde07702cb4c16d76c452e87c50f79", + "8e9a5bdf362c5aec2c31a742dfebd0b7b56e16ab8408d9d0609a4fad06446875", + "4a35d3d7c80ff4fab65f7e30d6b305fc7bb39ffe905aabedd6593354f86177b6", + "b1121deabd8b4308f3805cda8af991ee976bd8e413bcb6a8dd3fc26ebe2312d2", + ]; + for (i, okm) in test_okms.iter().enumerate() { + // String of number i. + let ikm = i.to_string(); + // Bytestring of byte i. + let salt = [i as u8; 32]; + // Byte i. + let info = [i as u8]; + let okm = hex::decode(okm).unwrap(); + assert_eq!( + &hkdf_256::(&ikm.as_bytes(), &salt, &info[..]), + array_ref!(okm, 0, 32) + ); + } + } } diff --git a/libraries/opensk/src/api/crypto/hkdf256.rs b/libraries/opensk/src/api/crypto/hkdf256.rs index 8461fe1..2378490 100644 --- a/libraries/opensk/src/api/crypto/hkdf256.rs +++ b/libraries/opensk/src/api/crypto/hkdf256.rs @@ -16,11 +16,28 @@ use super::HASH_SIZE; /// HKDF using SHA256. pub trait Hkdf256 { + /// Computes the HKDF with 256 bit (one block) output. + /// + /// # Arguments + /// + /// * `ikm` - Input keying material + /// * `salt` - Byte string that acts as a key + /// * `info` - Optional context and application specific information + /// + /// This implementation is equivalent to a standard HKD, with `salt` fixed at a length of + /// 32 byte and the output length l as 32. + fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8]) -> [u8; HASH_SIZE]; + /// Computes the HKDF with empty salt and 256 bit (one block) output. /// /// # Arguments /// /// * `ikm` - Input keying material /// * `info` - Optional context and application specific information - fn hkdf_empty_salt_256(ikm: &[u8], info: &[u8]) -> [u8; HASH_SIZE]; + /// + /// This implementation is equivalent to a standard HKDF, with `salt` set to the + /// default block of zeros and the output length l as 32. + fn hkdf_empty_salt_256(ikm: &[u8], info: &[u8]) -> [u8; HASH_SIZE] { + Self::hkdf_256(ikm, &[0; HASH_SIZE], info) + } } diff --git a/libraries/opensk/src/api/crypto/mod.rs b/libraries/opensk/src/api/crypto/mod.rs index fc752ac..fc4c6f7 100644 --- a/libraries/opensk/src/api/crypto/mod.rs +++ b/libraries/opensk/src/api/crypto/mod.rs @@ -158,6 +158,16 @@ mod test { assert_eq!(&SoftwareHkdf256::hkdf_empty_salt_256(b"0", &[0]), &okm); } + #[test] + fn test_hkdf_256_matches() { + let ikm = [0x11; 16]; + let salt = [0; 32]; + let info = [0x22; 16]; + let empty_salt_output = SoftwareHkdf256::hkdf_empty_salt_256(&ikm, &info); + let explicit_output = SoftwareHkdf256::hkdf_256(&ikm, &salt, &info); + assert_eq!(empty_salt_output, explicit_output); + } + #[test] fn test_aes_encrypt_decrypt_block() { let mut block = [0x55; AES_BLOCK_SIZE]; diff --git a/libraries/opensk/src/api/crypto/rust_crypto.rs b/libraries/opensk/src/api/crypto/rust_crypto.rs index eaacc54..401db05 100644 --- a/libraries/opensk/src/api/crypto/rust_crypto.rs +++ b/libraries/opensk/src/api/crypto/rust_crypto.rs @@ -257,8 +257,8 @@ impl Hmac256 for SoftwareHmac256 { pub struct SoftwareHkdf256; impl Hkdf256 for SoftwareHkdf256 { - fn hkdf_empty_salt_256(ikm: &[u8], info: &[u8]) -> [u8; HASH_SIZE] { - let hk = hkdf::Hkdf::::new(Some(&[0; HASH_SIZE]), ikm); + fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8]) -> [u8; HASH_SIZE] { + let hk = hkdf::Hkdf::::new(Some(salt), ikm); let mut okm = [0u8; HASH_SIZE]; hk.expand(info, &mut okm).unwrap(); okm diff --git a/libraries/opensk/src/api/crypto/software_crypto.rs b/libraries/opensk/src/api/crypto/software_crypto.rs index 71d4c2d..e11072e 100644 --- a/libraries/opensk/src/api/crypto/software_crypto.rs +++ b/libraries/opensk/src/api/crypto/software_crypto.rs @@ -208,8 +208,8 @@ impl Hmac256 for SoftwareHmac256 { pub struct SoftwareHkdf256; impl Hkdf256 for SoftwareHkdf256 { - fn hkdf_empty_salt_256(ikm: &[u8], info: &[u8]) -> [u8; HASH_SIZE] { - crypto::hkdf::hkdf_empty_salt_256::(ikm, info) + fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8]) -> [u8; HASH_SIZE] { + crypto::hkdf::hkdf_256::(ikm, salt, info) } } diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index 763288f..1cbf160 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -66,14 +66,14 @@ use crate::api::attestation_store::{self, Attestation, AttestationStore}; use crate::api::clock::Clock; use crate::api::connection::{HidConnection, SendOrRecvStatus, UsbEndpoint}; use crate::api::crypto::ecdsa::{SecretKey as _, Signature}; -use crate::api::crypto::hmac256::Hmac256; +use crate::api::crypto::hkdf256::Hkdf256; use crate::api::crypto::sha256::Sha256; use crate::api::customization::Customization; use crate::api::firmware_protection::FirmwareProtection; use crate::api::rng::Rng; use crate::api::upgrade_storage::UpgradeStorage; use crate::api::user_presence::{UserPresence, UserPresenceError}; -use crate::env::{EcdsaSk, Env, Hmac, Sha}; +use crate::env::{EcdsaSk, Env, Hkdf, Sha}; use alloc::boxed::Box; use alloc::string::{String, ToString}; use alloc::vec; @@ -956,9 +956,10 @@ impl CtapState { private_key: &PrivateKey, has_uv: bool, ) -> Result<[u8; 32], Ctap2StatusCode> { - let entropy = private_key.to_bytes(); + let private_key_bytes = private_key.to_bytes(); + let salt = array_ref!(private_key_bytes, 0, 32); let key = storage::cred_random_secret(env, has_uv)?; - Ok(Hmac::::mac(&key, &entropy)) + Ok(Hkdf::::hkdf_256(&key, salt, b"credRandom")) } // Processes the input of a get_assertion operation for a given credential