From 5f7eb3177b4a36c757fada5e2adda319e42ca001 Mon Sep 17 00:00:00 2001 From: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> Date: Wed, 19 Apr 2023 18:02:48 +0200 Subject: [PATCH] Cryptographic Secret type (#615) * Adds a type for cryptographic secrets * default implementations and zeroize documentation * removes whitespace --- Cargo.lock | 41 +++++++++ examples/crypto_bench.rs | 3 +- libraries/crypto/Cargo.toml | 1 + libraries/crypto/src/aes256.rs | 15 +++- libraries/crypto/src/ec/exponent256.rs | 13 ++- libraries/crypto/src/ec/gfp256.rs | 14 +-- libraries/crypto/src/ec/int256.rs | 6 +- libraries/crypto/src/ec/montgomery.rs | 7 +- libraries/crypto/src/ec/point.rs | 33 ++++--- libraries/crypto/src/ecdh.rs | 10 ++- libraries/crypto/src/ecdsa.rs | 30 ++++--- libraries/crypto/src/hkdf.rs | 24 +++-- libraries/crypto/src/hmac.rs | 47 ++++++---- libraries/crypto/src/lib.rs | 27 ++++-- libraries/crypto/src/sha256.rs | 22 +++-- libraries/opensk/Cargo.toml | 1 + libraries/opensk/src/api/attestation_store.rs | 5 +- libraries/opensk/src/api/crypto/ecdh.rs | 2 +- libraries/opensk/src/api/crypto/hkdf256.rs | 6 +- libraries/opensk/src/api/crypto/hmac256.rs | 2 +- libraries/opensk/src/api/crypto/mod.rs | 28 ++++-- .../opensk/src/api/crypto/rust_crypto.rs | 21 ++--- libraries/opensk/src/api/crypto/sha256.rs | 19 +++- .../opensk/src/api/crypto/software_crypto.rs | 21 +++-- libraries/opensk/src/api/key_store.rs | 74 ++++++++------- libraries/opensk/src/ctap/client_pin.rs | 74 +++++++++------ libraries/opensk/src/ctap/credential_id.rs | 46 +++++----- libraries/opensk/src/ctap/crypto_wrapper.rs | 42 +++++---- libraries/opensk/src/ctap/ctap1.rs | 3 +- libraries/opensk/src/ctap/mod.rs | 9 +- libraries/opensk/src/ctap/pin_protocol.rs | 83 ++++++++++------- libraries/opensk/src/ctap/secret.rs | 89 +++++++++++++++++++ libraries/opensk/src/ctap/storage.rs | 5 +- libraries/opensk/src/test_helpers/mod.rs | 3 +- src/env/tock/commands.rs | 7 +- src/env/tock/storage.rs | 3 +- 36 files changed, 582 insertions(+), 254 deletions(-) create mode 100644 libraries/opensk/src/ctap/secret.rs diff --git a/Cargo.lock b/Cargo.lock index 1a63227..3fc41aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -67,6 +67,7 @@ dependencies = [ "serde_json", "subtle", "untrusted", + "zeroize", ] [[package]] @@ -242,6 +243,7 @@ dependencies = [ "sk-cbor", "subtle", "uuid", + "zeroize", ] [[package]] @@ -443,12 +445,30 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "synstructure" +version = "0.12.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f36bdaa60a83aca3921b5259d5400cbf5e90fc51931376a9bd4a0eb79aa7210f" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "unicode-xid", +] + [[package]] name = "unicode-ident" version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dcc811dc4066ac62f84f11307873c4850cb653bfa9b1719cee2bd2204a4bc5dd" +[[package]] +name = "unicode-xid" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" + [[package]] name = "untrusted" version = "0.7.1" @@ -561,3 +581,24 @@ name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + +[[package]] +name = "zeroize" +version = "1.5.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c394b5bd0c6f669e7275d9c20aa90ae064cb22e75a1cad54e1b34088034b149f" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44bf07cb3e50ea2003396695d58bf46bc9887a1f362260446fad6bc4e79bd36c" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "synstructure", +] diff --git a/examples/crypto_bench.rs b/examples/crypto_bench.rs index 5972d5c..a327546 100644 --- a/examples/crypto_bench.rs +++ b/examples/crypto_bench.rs @@ -113,7 +113,8 @@ fn main() { || { let mut sha = sha256::Sha256::new(); sha.update(&contents); - sha.finalize(); + let mut dummy_hash = [0; 32]; + sha.finalize(&mut dummy_hash); }, ); } diff --git a/libraries/crypto/Cargo.toml b/libraries/crypto/Cargo.toml index fa1dd42..989a8de 100644 --- a/libraries/crypto/Cargo.toml +++ b/libraries/crypto/Cargo.toml @@ -20,6 +20,7 @@ serde = { version = "1.0", optional = true, features = ["derive"] } serde_json = { version = "=1.0.69", optional = true } regex = { version = "1", optional = true } rand_core = "0.6.4" +zeroize = { version = "1.5.7", features = ["derive"] } [features] std = ["hex", "ring", "untrusted", "serde", "serde_json", "regex", "rand_core/getrandom"] diff --git a/libraries/crypto/src/aes256.rs b/libraries/crypto/src/aes256.rs index da78810..cb71b1c 100644 --- a/libraries/crypto/src/aes256.rs +++ b/libraries/crypto/src/aes256.rs @@ -12,18 +12,29 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! A portable and naive textbook implementation of AES-256 + use super::util::{xor_block_16, Block16}; use arrayref::{array_mut_ref, array_ref}; +use zeroize::Zeroize; -/** A portable and naive textbook implementation of AES-256 **/ type Word = [u8; 4]; -/** This structure caches the round keys, to avoid re-computing the key schedule for each block. **/ +/// Encryption key for AES256. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Zeroize)] pub struct EncryptionKey { + // This structure caches the round keys, to avoid re-computing the key schedule for each block. enc_round_keys: [Block16; 15], } +/// Decryption key for AES256. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Zeroize)] pub struct DecryptionKey { + // This structure caches the round keys, to avoid re-computing the key schedule for each block. dec_round_keys: [Block16; 15], } diff --git a/libraries/crypto/src/ec/exponent256.rs b/libraries/crypto/src/ec/exponent256.rs index ea703a7..74de7bb 100644 --- a/libraries/crypto/src/ec/exponent256.rs +++ b/libraries/crypto/src/ec/exponent256.rs @@ -16,9 +16,12 @@ use super::int256::{Digit, Int256}; use core::ops::Mul; use rand_core::RngCore; use subtle::{self, Choice, ConditionallySelectable, CtOption}; +use zeroize::Zeroize; -// An exponent on the elliptic curve, that is an element modulo the curve order N. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +/// An exponent on the elliptic curve, that is an element modulo the curve order N. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Zeroize)] // TODO: remove this Default once https://github.com/dalek-cryptography/subtle/issues/63 is // resolved. #[derive(Default)] @@ -90,8 +93,10 @@ impl Mul for &ExponentP256 { } } -// A non-zero exponent on the elliptic curve. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +/// A non-zero exponent on the elliptic curve. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Zeroize)] // TODO: remove this Default once https://github.com/dalek-cryptography/subtle/issues/63 is // resolved. #[derive(Default)] diff --git a/libraries/crypto/src/ec/gfp256.rs b/libraries/crypto/src/ec/gfp256.rs index 0e6179f..4b2675b 100644 --- a/libraries/crypto/src/ec/gfp256.rs +++ b/libraries/crypto/src/ec/gfp256.rs @@ -15,12 +15,16 @@ use super::int256::{Digit, Int256}; use core::ops::Mul; use subtle::Choice; +use zeroize::Zeroize; -// A field element on the elliptic curve, that is an element modulo the prime P. -// This is the format used to serialize coordinates of points on the curve. -// This implements enough methods to validate points and to convert them to/from the Montgomery -// form, which is more convenient to operate on. -#[derive(Clone, Copy, PartialEq, Eq)] +/// A field element on the elliptic curve, that is an element modulo the prime P. +/// +/// This is the format used to serialize coordinates of points on the curve. +/// This implements enough methods to validate points and to convert them to/from the Montgomery +/// form, which is more convenient to operate on. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, PartialEq, Eq, Zeroize)] pub struct GFP256 { int: Int256, } diff --git a/libraries/crypto/src/ec/int256.rs b/libraries/crypto/src/ec/int256.rs index 5455a71..03e93f4 100644 --- a/libraries/crypto/src/ec/int256.rs +++ b/libraries/crypto/src/ec/int256.rs @@ -19,6 +19,7 @@ use byteorder::{BigEndian, ByteOrder}; use core::ops::{Add, AddAssign, Sub, SubAssign}; use rand_core::RngCore; use subtle::{self, Choice, ConditionallySelectable, ConstantTimeEq}; +use zeroize::Zeroize; const BITS_PER_DIGIT: usize = 32; const BYTES_PER_DIGIT: usize = BITS_PER_DIGIT >> 3; @@ -29,7 +30,10 @@ pub type Digit = u32; type DoubleDigit = u64; type SignedDoubleDigit = i64; -#[derive(Clone, Copy, PartialEq, Eq)] +/// Big integer implementation with 256 bits. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, PartialEq, Eq, Zeroize)] // TODO: remove this Default once https://github.com/dalek-cryptography/subtle/issues/63 is // resolved. #[derive(Default)] diff --git a/libraries/crypto/src/ec/montgomery.rs b/libraries/crypto/src/ec/montgomery.rs index c197b52..1434586 100644 --- a/libraries/crypto/src/ec/montgomery.rs +++ b/libraries/crypto/src/ec/montgomery.rs @@ -17,13 +17,16 @@ use super::int256::Int256; use super::precomputed; use core::ops::{Add, Mul, Sub}; use subtle::{Choice, ConditionallySelectable, ConstantTimeEq}; +use zeroize::Zeroize; pub const NLIMBS: usize = 9; pub const BOTTOM_28_BITS: u32 = 0x0fff_ffff; pub const BOTTOM_29_BITS: u32 = 0x1fff_ffff; -/** Field element on the secp256r1 curve, represented in Montgomery form **/ -#[derive(Clone, Copy)] +/// Field element on the secp256r1 curve, represented in Montgomery form. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, Zeroize)] pub struct Montgomery { // The 9 limbs use 28 or 29 bits, alternatively: even limbs use 29 bits, odd limbs use 28 bits. // The Montgomery form stores a field element x as (x * 2^257) mod P. diff --git a/libraries/crypto/src/ec/point.rs b/libraries/crypto/src/ec/point.rs index bc55057..826088a 100644 --- a/libraries/crypto/src/ec/point.rs +++ b/libraries/crypto/src/ec/point.rs @@ -21,11 +21,15 @@ use arrayref::array_mut_ref; use arrayref::array_ref; use core::ops::Add; use subtle::{Choice, ConditionallySelectable, ConstantTimeEq}; +use zeroize::Zeroize; -// A point on the elliptic curve is represented by two field elements. -// The "direct" representation with GFP256 (integer modulo p) is used for serialization of public -// keys. -#[derive(Clone, Copy)] +/// A point on the elliptic curve, represented by two field elements. +/// +/// The "direct" representation with GFP256 (integer modulo p) is used for serialization of public +/// keys. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, Zeroize)] pub struct PointP256 { x: GFP256, y: GFP256, @@ -128,12 +132,15 @@ impl PointP256 { } } -// A point on the elliptic curve in projective form. -// This uses Montgomery representation for field elements. -// This is in projective coordinates, i.e. it represents the point { x: x / z, y: y / z }. -// This representation is more convenient to implement complete formulas for elliptic curve -// arithmetic. -#[derive(Clone, Copy)] +/// A point on the elliptic curve in projective form. +/// +/// This uses Montgomery representation for field elements. +/// This is in projective coordinates, i.e. it represents the point { x: x / z, y: y / z }. +/// This representation is more convenient to implement complete formulas for elliptic curve +/// arithmetic. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, Zeroize)] pub struct PointProjective { x: Montgomery, y: Montgomery, @@ -150,8 +157,10 @@ impl ConditionallySelectable for PointProjective { } } -// Equivalent to PointProjective { x, y, z: 1 } -#[derive(Clone, Copy)] +/// Equivalent to PointProjective { x, y, z: 1 } +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Copy, Zeroize)] pub struct PointAffine { x: Montgomery, y: Montgomery, diff --git a/libraries/crypto/src/ecdh.rs b/libraries/crypto/src/ecdh.rs index 89adf1f..ed5bec0 100644 --- a/libraries/crypto/src/ecdh.rs +++ b/libraries/crypto/src/ecdh.rs @@ -17,14 +17,22 @@ use super::ec::int256; use super::ec::int256::Int256; use super::ec::point::PointP256; use rand_core::RngCore; +use zeroize::Zeroize; pub const NBYTES: usize = int256::NBYTES; +/// A private key for ECDH. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Zeroize)] pub struct SecKey { a: NonZeroExponentP256, } -#[derive(Clone, Debug, PartialEq)] +/// A public key for ECDH. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Debug, PartialEq, Zeroize)] pub struct PubKey { p: PointP256, } diff --git a/libraries/crypto/src/ecdsa.rs b/libraries/crypto/src/ecdsa.rs index 7310824..f4e6621 100644 --- a/libraries/crypto/src/ecdsa.rs +++ b/libraries/crypto/src/ecdsa.rs @@ -16,7 +16,6 @@ use super::ec::exponent256::{ExponentP256, NonZeroExponentP256}; use super::ec::int256; use super::ec::int256::Int256; use super::ec::point::PointP256; -use super::hmac::hmac_256; use super::Hash256; use alloc::vec; use alloc::vec::Vec; @@ -25,20 +24,31 @@ use arrayref::array_mut_ref; use arrayref::{array_ref, mut_array_refs}; use core::marker::PhantomData; use rand_core::RngCore; +use zeroize::Zeroize; pub const NBYTES: usize = int256::NBYTES; -#[derive(Clone, Debug, PartialEq, Eq)] +/// A private key for ECDSA. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Debug, PartialEq, Eq, Zeroize)] pub struct SecKey { k: NonZeroExponentP256, } +/// An ECDSA signature. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Zeroize)] pub struct Signature { r: NonZeroExponentP256, s: NonZeroExponentP256, } -#[derive(Clone)] +/// A public key for ECDSA. +/// +/// Never call zeroize explicitly, to not invalidate any invariants. +#[derive(Clone, Zeroize)] pub struct PubKey { p: PointP256, } @@ -310,15 +320,15 @@ where Int256::to_bin(&sk.k.to_int(), contents_k); Int256::to_bin(&Int256::from_bin(&h1).modd(&Int256::N), contents_h1); - let k = hmac_256::(&k, &contents); - let v = hmac_256::(&k, &v); + let k = H::hmac(&k, &contents); + let v = H::hmac(&k, &v); let (contents_v, marker, _) = mut_array_refs![&mut contents, 32, 1, 64]; contents_v.copy_from_slice(&v); marker[0] = 0x01; - let k = hmac_256::(&k, &contents); - let v = hmac_256::(&k, &v); + let k = H::hmac(&k, &contents); + let v = H::hmac(&k, &v); Rfc6979 { k, @@ -330,14 +340,14 @@ where fn next(&mut self) -> Int256 { // Note: at this step, the logic from RFC 6979 is simplified, because the HMAC produces 256 // bits and we need 256 bits. - let t = hmac_256::(&self.k, &self.v); + let t = H::hmac(&self.k, &self.v); let result = Int256::from_bin(&t); let mut v1 = [0; 33]; v1[..32].copy_from_slice(&self.v); v1[32] = 0x00; - self.k = hmac_256::(&self.k, &v1); - self.v = hmac_256::(&self.k, &self.v); + self.k = H::hmac(&self.k, &v1); + self.v = H::hmac(&self.k, &self.v); result } diff --git a/libraries/crypto/src/hkdf.rs b/libraries/crypto/src/hkdf.rs index 2405261..3ac2ee0 100644 --- a/libraries/crypto/src/hkdf.rs +++ b/libraries/crypto/src/hkdf.rs @@ -27,15 +27,15 @@ const HASH_SIZE: usize = 32; /// /// 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] +pub fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8], okm: &mut [u8; HASH_SIZE]) where H: Hash256, { - let prk = hmac_256::(salt, ikm); + let prk = H::hmac(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()) + hmac_256::(&prk, t.as_slice(), okm); } /// Computes the HKDF with empty salt and 256 bit (one block) output. @@ -47,12 +47,12 @@ where /// /// This implementation is equivalent to the below hkdf, with `salt` set to the /// default block of zeros and the output length l as 32. -pub fn hkdf_empty_salt_256(ikm: &[u8], info: &[u8]) -> [u8; HASH_SIZE] +pub fn hkdf_empty_salt_256(ikm: &[u8], info: &[u8], okm: &mut [u8; HASH_SIZE]) where H: Hash256, { // Salt is a zero block here. - hkdf_256::(ikm, &[0; HASH_SIZE], info) + hkdf_256::(ikm, &[0; HASH_SIZE], info, okm); } #[cfg(test)] @@ -81,10 +81,9 @@ mod test { // 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) - ); + let mut output = [0; HASH_SIZE]; + hkdf_empty_salt_256::(&ikm.as_bytes(), &info[..], &mut output); + assert_eq!(&output, array_ref!(okm, 0, HASH_SIZE)); } } @@ -109,10 +108,9 @@ mod test { // 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) - ); + let mut output = [0; HASH_SIZE]; + hkdf_256::(&ikm.as_bytes(), &salt, &info[..], &mut output); + assert_eq!(&output, array_ref!(okm, 0, HASH_SIZE)); } } } diff --git a/libraries/crypto/src/hmac.rs b/libraries/crypto/src/hmac.rs index 696cf75..36e2ef6 100644 --- a/libraries/crypto/src/hmac.rs +++ b/libraries/crypto/src/hmac.rs @@ -24,7 +24,8 @@ pub fn verify_hmac_256(key: &[u8; KEY_SIZE], contents: &[u8], mac: &[u8; HASH where H: Hash256, { - let expected_mac = hmac_256::(key, contents); + let mut expected_mac = [0; HASH_SIZE]; + hmac_256::(key, contents, &mut expected_mac); bool::from(expected_mac.ct_eq(mac)) } @@ -38,19 +39,23 @@ pub fn verify_hmac_256_first_128bits( where H: Hash256, { - let expected_mac = hmac_256::(key, contents); + let mut expected_mac = [0; HASH_SIZE]; + hmac_256::(key, contents, &mut expected_mac); bool::from(array_ref![expected_mac, 0, 16].ct_eq(pin)) } -pub fn hmac_256(key: &[u8; KEY_SIZE], contents: &[u8]) -> [u8; HASH_SIZE] +pub fn hmac_256(key: &[u8; KEY_SIZE], contents: &[u8], output: &mut [u8; HASH_SIZE]) where H: Hash256, { - H::hmac(key, contents) + H::hmac_mut(key, contents, output) } -pub(crate) fn software_hmac_256(key: &[u8; KEY_SIZE], contents: &[u8]) -> [u8; HASH_SIZE] -where +pub(crate) fn software_hmac_256( + key: &[u8; KEY_SIZE], + contents: &[u8], + output: &mut [u8; HASH_SIZE], +) where H: Hash256, { let mut ipad: [u8; BLOCK_SIZE] = [0x36; BLOCK_SIZE]; @@ -64,13 +69,13 @@ where let mut ihasher = H::new(); ihasher.update(&ipad); ihasher.update(contents); - let ihash = ihasher.finalize(); + let mut ihash = [0; HASH_SIZE]; + ihasher.finalize(&mut ihash); let mut ohasher = H::new(); ohasher.update(&opad); ohasher.update(&ihash); - - ohasher.finalize() + ohasher.finalize(output); } fn xor_pads(ipad: &mut [u8; BLOCK_SIZE], opad: &mut [u8; BLOCK_SIZE], key: &[u8; KEY_SIZE]) { @@ -91,7 +96,8 @@ mod test { for len in 0..128 { let key = [0; KEY_SIZE]; let contents = vec![0; len]; - let mac = hmac_256::(&key, &contents); + let mut mac = [0; HASH_SIZE]; + hmac_256::(&key, &contents, &mut mac); assert!(verify_hmac_256::(&key, &contents, &mac)); } } @@ -102,7 +108,8 @@ mod test { for len in 0..128 { let key = [0; KEY_SIZE]; let contents = vec![0; len]; - let mac = hmac_256::(&key, &contents); + let mut mac = [0; HASH_SIZE]; + hmac_256::(&key, &contents, &mut mac); // Check that invalid MACs don't verify, by changing any byte of the valid MAC. for i in 0..HASH_SIZE { @@ -116,14 +123,21 @@ mod test { #[test] fn test_hmac_sha256_examples() { let key = [0; KEY_SIZE]; + let mut mac = [0; HASH_SIZE]; + hmac_256::(&key, &[], &mut mac); assert_eq!( - hmac_256::(&key, &[]), + mac, hex::decode("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad") .unwrap() .as_slice() ); + hmac_256::( + &key, + b"The quick brown fox jumps over the lazy dog", + &mut mac, + ); assert_eq!( - hmac_256::(&key, b"The quick brown fox jumps over the lazy dog"), + mac, hex::decode("fb011e6154a19b9a4c767373c305275a5a69e8b68b0b4c9200c383dced19a416") .unwrap() .as_slice() @@ -274,11 +288,10 @@ mod test { let mut input = Vec::new(); let key = [b'A'; KEY_SIZE]; + let mut mac = [0; HASH_SIZE]; for i in 0..128 { - assert_eq!( - hmac_256::(&key, &input), - hex::decode(hashes[i] as &[u8]).unwrap().as_slice() - ); + hmac_256::(&key, &input, &mut mac); + assert_eq!(mac, hex::decode(hashes[i] as &[u8]).unwrap().as_slice()); input.push(b'A'); } } diff --git a/libraries/crypto/src/lib.rs b/libraries/crypto/src/lib.rs index 6a8e103..f2b484b 100644 --- a/libraries/crypto/src/lib.rs +++ b/libraries/crypto/src/lib.rs @@ -27,26 +27,39 @@ pub mod hmac; pub mod sha256; pub mod util; -// Trait for hash functions that returns a 256-bit hash. -// The type must be Sized (size known at compile time) so that we can instanciate one on the stack -// in the hash() method. +/// Trait for hash functions that returns a 256-bit hash. +/// +/// When you implement this trait, make sure to implement `hash_mut` and `hmac_mut` first, because +/// the default implementations of `hash` and `hmac` rely on it. pub trait Hash256: Sized { fn new() -> Self; fn update(&mut self, contents: &[u8]); - fn finalize(self) -> [u8; 32]; + fn finalize(self, output: &mut [u8; 32]); fn hash(contents: &[u8]) -> [u8; 32] { + let mut output = [0; 32]; + Self::hash_mut(contents, &mut output); + output + } + + fn hash_mut(contents: &[u8], output: &mut [u8; 32]) { let mut h = Self::new(); h.update(contents); - h.finalize() + h.finalize(output) } fn hmac(key: &[u8; 32], contents: &[u8]) -> [u8; 32] { - hmac::software_hmac_256::(key, contents) + let mut output = [0; 32]; + Self::hmac_mut(key, contents, &mut output); + output + } + + fn hmac_mut(key: &[u8; 32], contents: &[u8], output: &mut [u8; 32]) { + hmac::software_hmac_256::(key, contents, output); } } -// Trait for hash functions that operate on 64-byte input blocks. +/// Trait for hash functions that operate on 64-byte input blocks. pub trait HashBlockSize64Bytes { type State; diff --git a/libraries/crypto/src/sha256.rs b/libraries/crypto/src/sha256.rs index 273cd67..09e32e0 100644 --- a/libraries/crypto/src/sha256.rs +++ b/libraries/crypto/src/sha256.rs @@ -17,6 +17,7 @@ use arrayref::{array_mut_ref, array_ref}; use byteorder::{BigEndian, ByteOrder}; use core::cell::Cell; use core::num::Wrapping; +use zeroize::Zeroize; const BLOCK_SIZE: usize = 64; @@ -32,6 +33,17 @@ pub struct Sha256 { total_len: usize, } +impl Drop for Sha256 { + // TODO derive Zeroize instead when we upgrade the toolchain + fn drop(&mut self) { + for s in self.state.iter_mut() { + s.0.zeroize(); + } + self.block.zeroize(); + self.total_len.zeroize(); + } +} + impl Hash256 for Sha256 { fn new() -> Self { assert!(!BUSY.replace(true)); @@ -72,7 +84,7 @@ impl Hash256 for Sha256 { } } - fn finalize(mut self) -> [u8; 32] { + fn finalize(mut self, output: &mut [u8; 32]) { // Last block and padding. let cursor_in_block = self.total_len % BLOCK_SIZE; self.block[cursor_in_block] = 0x80; @@ -97,12 +109,10 @@ impl Hash256 for Sha256 { Sha256::hash_block(&mut self.state, &self.block); // Encode the state's 32-bit words into bytes, using big-endian. - let mut result: [u8; 32] = [0; 32]; for i in 0..8 { - BigEndian::write_u32(array_mut_ref![result, 4 * i, 4], self.state[i].0); + BigEndian::write_u32(array_mut_ref![output, 4 * i, 4], self.state[i].0); } BUSY.set(false); - result } } @@ -272,7 +282,9 @@ mod test { h.update(&input[..i]); h.update(&input[i..j]); h.update(&input[j..]); - assert_eq!(h.finalize(), hash.as_slice()); + let mut digest = [0; 32]; + h.finalize(&mut digest); + assert_eq!(digest, hash.as_slice()); } } } diff --git a/libraries/opensk/Cargo.toml b/libraries/opensk/Cargo.toml index ba80814..964bae5 100644 --- a/libraries/opensk/Cargo.toml +++ b/libraries/opensk/Cargo.toml @@ -28,6 +28,7 @@ hmac = { version = "0.12.1", optional = true } hkdf = { version = "0.12.3", optional = true } aes = { version = "0.8.2", optional = true } cbc = { version = "0.1.2", optional = true } +zeroize = { version = "1.5.7", features = ["derive"] } [features] debug_ctap = [] diff --git a/libraries/opensk/src/api/attestation_store.rs b/libraries/opensk/src/api/attestation_store.rs index 3f80183..f4cfb89 100644 --- a/libraries/opensk/src/api/attestation_store.rs +++ b/libraries/opensk/src/api/attestation_store.rs @@ -13,6 +13,7 @@ // limitations under the License. use crate::api::crypto::EC_FIELD_SIZE; +use crate::ctap::secret::Secret; use crate::env::Env; use alloc::vec::Vec; use persistent_store::{StoreError, StoreUpdate}; @@ -27,7 +28,7 @@ pub enum Id { #[cfg_attr(feature = "std", derive(Debug, PartialEq, Eq))] pub struct Attestation { /// ECDSA private key (big-endian). - pub private_key: [u8; EC_FIELD_SIZE], + pub private_key: Secret<[u8; EC_FIELD_SIZE]>, pub certificate: Vec, } @@ -69,7 +70,7 @@ pub fn helper_get(env: &mut impl Env) -> Result, Error> { return Err(Error::Internal); } Ok(Some(Attestation { - private_key: *array_ref![private_key, 0, EC_FIELD_SIZE], + private_key: Secret::from_exposed_secret(*array_ref![private_key, 0, EC_FIELD_SIZE]), certificate, })) } diff --git a/libraries/opensk/src/api/crypto/ecdh.rs b/libraries/opensk/src/api/crypto/ecdh.rs index 5e47a51..1c4b74f 100644 --- a/libraries/opensk/src/api/crypto/ecdh.rs +++ b/libraries/opensk/src/api/crypto/ecdh.rs @@ -49,5 +49,5 @@ pub trait PublicKey: Sized { /// ECDH shared secret. pub trait SharedSecret { /// Exports the x component of the point computed by Diffie–Hellman. - fn raw_secret_bytes(&self) -> [u8; EC_FIELD_SIZE]; + fn raw_secret_bytes(&self, secret: &mut [u8; EC_FIELD_SIZE]); } diff --git a/libraries/opensk/src/api/crypto/hkdf256.rs b/libraries/opensk/src/api/crypto/hkdf256.rs index 2378490..ca09fd3 100644 --- a/libraries/opensk/src/api/crypto/hkdf256.rs +++ b/libraries/opensk/src/api/crypto/hkdf256.rs @@ -26,7 +26,7 @@ pub trait Hkdf256 { /// /// 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]; + fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8], okm: &mut [u8; HASH_SIZE]); /// Computes the HKDF with empty salt and 256 bit (one block) output. /// @@ -37,7 +37,7 @@ pub trait Hkdf256 { /// /// 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) + fn hkdf_empty_salt_256(ikm: &[u8], info: &[u8], okm: &mut [u8; HASH_SIZE]) { + Self::hkdf_256(ikm, &[0; HASH_SIZE], info, okm) } } diff --git a/libraries/opensk/src/api/crypto/hmac256.rs b/libraries/opensk/src/api/crypto/hmac256.rs index bc3f5de..7011a67 100644 --- a/libraries/opensk/src/api/crypto/hmac256.rs +++ b/libraries/opensk/src/api/crypto/hmac256.rs @@ -17,7 +17,7 @@ use super::{HASH_SIZE, HMAC_KEY_SIZE, TRUNCATED_HMAC_SIZE}; /// For a given hash function, computes and verifies the HMAC. pub trait Hmac256 { /// Computes the HMAC. - fn mac(key: &[u8; HMAC_KEY_SIZE], data: &[u8]) -> [u8; HASH_SIZE]; + fn mac(key: &[u8; HMAC_KEY_SIZE], data: &[u8], output: &mut [u8; HASH_SIZE]); /// Verifies the HMAC. /// diff --git a/libraries/opensk/src/api/crypto/mod.rs b/libraries/opensk/src/api/crypto/mod.rs index 669a6ed..8a81b36 100644 --- a/libraries/opensk/src/api/crypto/mod.rs +++ b/libraries/opensk/src/api/crypto/mod.rs @@ -84,7 +84,11 @@ mod test { let pub2 = private2.public_key(); let shared1 = private1.diffie_hellman(&pub2); let shared2 = private2.diffie_hellman(&pub1); - assert_eq!(shared1.raw_secret_bytes(), shared2.raw_secret_bytes()); + let mut secret_bytes1 = [0; EC_FIELD_SIZE]; + let mut secret_bytes2 = [0; EC_FIELD_SIZE]; + shared1.raw_secret_bytes(&mut secret_bytes1); + shared2.raw_secret_bytes(&mut secret_bytes2); + assert_eq!(secret_bytes1, secret_bytes2); } #[test] @@ -155,14 +159,20 @@ mod test { let data = [0x55; 16]; let mut hasher = SoftwareSha256::new(); hasher.update(&data); - assert_eq!(SoftwareSha256::digest(&data), hasher.finalize()); + let mut hash_from_finalize = [0; HASH_SIZE]; + hasher.finalize(&mut hash_from_finalize); + assert_eq!(SoftwareSha256::digest(&data), hash_from_finalize); + let mut hash_from_mut = [0; HASH_SIZE]; + SoftwareSha256::digest_mut(&data, &mut hash_from_mut); + assert_eq!(SoftwareSha256::digest(&data), hash_from_mut); } #[test] fn test_hmac256_verifies() { let key = [0xAA; HMAC_KEY_SIZE]; let data = [0x55; 16]; - let mac = SoftwareHmac256::mac(&key, &data); + let mut mac = [0; HASH_SIZE]; + SoftwareHmac256::mac(&key, &data, &mut mac); assert!(SoftwareHmac256::verify(&key, &data, &mac)); let truncated_mac = <&[u8; TRUNCATED_HMAC_SIZE]>::try_from(&mac[..TRUNCATED_HMAC_SIZE]).unwrap(); @@ -175,12 +185,14 @@ mod test { #[test] fn test_hkdf_empty_salt_256_vector() { - let okm = [ + let expected_okm = [ 0xf9, 0xbe, 0x72, 0x11, 0x6c, 0xb9, 0x7f, 0x41, 0x82, 0x82, 0x10, 0x28, 0x9c, 0xaa, 0xfe, 0xab, 0xde, 0x1f, 0x3d, 0xfb, 0x97, 0x23, 0xbf, 0x43, 0x53, 0x8a, 0xb1, 0x8f, 0x36, 0x66, 0x78, 0x3a, ]; - assert_eq!(&SoftwareHkdf256::hkdf_empty_salt_256(b"0", &[0]), &okm); + let mut okm = [0; HASH_SIZE]; + SoftwareHkdf256::hkdf_empty_salt_256(b"0", &[0], &mut okm); + assert_eq!(okm, expected_okm); } #[test] @@ -188,8 +200,10 @@ mod test { 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); + let mut empty_salt_output = [0; HASH_SIZE]; + let mut explicit_output = [0; HASH_SIZE]; + SoftwareHkdf256::hkdf_empty_salt_256(&ikm, &info, &mut empty_salt_output); + SoftwareHkdf256::hkdf_256(&ikm, &salt, &info, &mut explicit_output); assert_eq!(empty_salt_output, explicit_output); } diff --git a/libraries/opensk/src/api/crypto/rust_crypto.rs b/libraries/opensk/src/api/crypto/rust_crypto.rs index 3bd5aef..794d30c 100644 --- a/libraries/opensk/src/api/crypto/rust_crypto.rs +++ b/libraries/opensk/src/api/crypto/rust_crypto.rs @@ -34,6 +34,7 @@ use aes::cipher::{ BlockDecrypt, BlockDecryptMut, BlockEncrypt, BlockEncryptMut, KeyInit, KeyIvInit, }; use core::convert::TryFrom; +use hmac::digest::FixedOutput; use hmac::Mac; use p256::ecdh::EphemeralSecret; use p256::ecdsa::signature::hazmat::PrehashVerifier; @@ -109,10 +110,8 @@ pub struct SoftwareEcdhSharedSecret { } impl ecdh::SharedSecret for SoftwareEcdhSharedSecret { - fn raw_secret_bytes(&self) -> [u8; EC_FIELD_SIZE] { - let mut bytes = [0; EC_FIELD_SIZE]; - bytes.copy_from_slice(self.shared_secret.raw_secret_bytes().as_slice()); - bytes + fn raw_secret_bytes(&self, secret: &mut [u8; EC_FIELD_SIZE]) { + secret.copy_from_slice(self.shared_secret.raw_secret_bytes().as_slice()); } } @@ -235,18 +234,18 @@ impl Sha256 for SoftwareSha256 { } /// Finalizes the hashing process, returns the hash value. - fn finalize(self) -> [u8; HASH_SIZE] { - self.hasher.finalize().into() + fn finalize(self, output: &mut [u8; HASH_SIZE]) { + FixedOutput::finalize_into(self.hasher, output.into()); } } pub struct SoftwareHmac256; impl Hmac256 for SoftwareHmac256 { - fn mac(key: &[u8; HMAC_KEY_SIZE], data: &[u8]) -> [u8; HASH_SIZE] { + fn mac(key: &[u8; HMAC_KEY_SIZE], data: &[u8], output: &mut [u8; HASH_SIZE]) { let mut hmac = as hmac::Mac>::new_from_slice(key).unwrap(); hmac.update(data); - hmac.finalize().into_bytes().into() + hmac.finalize_into(output.into()); } fn verify(key: &[u8; HMAC_KEY_SIZE], data: &[u8], mac: &[u8; HASH_SIZE]) -> bool { @@ -269,11 +268,9 @@ impl Hmac256 for SoftwareHmac256 { pub struct SoftwareHkdf256; impl Hkdf256 for SoftwareHkdf256 { - fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8]) -> [u8; HASH_SIZE] { + fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8], okm: &mut [u8; HASH_SIZE]) { let hk = hkdf::Hkdf::::new(Some(salt), ikm); - let mut okm = [0u8; HASH_SIZE]; - hk.expand(info, &mut okm).unwrap(); - okm + hk.expand(info, okm).unwrap(); } } diff --git a/libraries/opensk/src/api/crypto/sha256.rs b/libraries/opensk/src/api/crypto/sha256.rs index b38140e..719fe31 100644 --- a/libraries/opensk/src/api/crypto/sha256.rs +++ b/libraries/opensk/src/api/crypto/sha256.rs @@ -15,12 +15,25 @@ use super::HASH_SIZE; /// Hashes data using SHA256. +/// +/// When you implement this trait, make sure to implement `digest_mut` first, because the default +/// implementations of `digest` relies on it. pub trait Sha256: Sized { - /// Computes the hash of a given message directly. + /// Computes the hash of a given message as an array. + /// + /// This function does not let you control the memory allocation. It should therefore not be + /// used for secrets that need zeroization. fn digest(data: &[u8]) -> [u8; HASH_SIZE] { + let mut output = [0; HASH_SIZE]; + Self::digest_mut(data, &mut output); + output + } + + /// Computes the hash of a given message directly. + fn digest_mut(data: &[u8], output: &mut [u8; HASH_SIZE]) { let mut hasher = Self::new(); hasher.update(data); - hasher.finalize() + hasher.finalize(output) } /// Create a new object that can be incrementally updated for digesting. @@ -30,5 +43,5 @@ pub trait Sha256: Sized { fn update(&mut self, data: &[u8]); /// Finalizes the hashing process, returns the hash value. - fn finalize(self) -> [u8; HASH_SIZE]; + fn finalize(self, output: &mut [u8; HASH_SIZE]); } diff --git a/libraries/opensk/src/api/crypto/software_crypto.rs b/libraries/opensk/src/api/crypto/software_crypto.rs index 74e8cec..f0cd221 100644 --- a/libraries/opensk/src/api/crypto/software_crypto.rs +++ b/libraries/opensk/src/api/crypto/software_crypto.rs @@ -23,7 +23,11 @@ use crate::api::crypto::{ use crate::api::rng::Rng; use alloc::vec::Vec; use crypto::Hash256; +use zeroize::Zeroize; +/// Cryptography implementation using our own library of primitives. +/// +/// Warning: The used library does not implement zeroization. pub struct SoftwareCrypto; pub struct SoftwareEcdh; pub struct SoftwareEcdsa; @@ -81,13 +85,14 @@ impl ecdh::PublicKey for SoftwareEcdhPublicKey { } } +#[derive(Zeroize)] pub struct SoftwareEcdhSharedSecret { shared_secret: [u8; EC_FIELD_SIZE], } impl ecdh::SharedSecret for SoftwareEcdhSharedSecret { - fn raw_secret_bytes(&self) -> [u8; EC_FIELD_SIZE] { - self.shared_secret + fn raw_secret_bytes(&self, secret: &mut [u8; EC_FIELD_SIZE]) { + secret.copy_from_slice(&self.shared_secret); } } @@ -190,16 +195,16 @@ impl Sha256 for SoftwareSha256 { } /// Finalizes the hashing process, returns the hash value. - fn finalize(self) -> [u8; HASH_SIZE] { - self.hasher.finalize() + fn finalize(self, output: &mut [u8; HASH_SIZE]) { + self.hasher.finalize(output) } } pub struct SoftwareHmac256; impl Hmac256 for SoftwareHmac256 { - fn mac(key: &[u8; HMAC_KEY_SIZE], data: &[u8]) -> [u8; HASH_SIZE] { - crypto::hmac::hmac_256::(key, data) + fn mac(key: &[u8; HMAC_KEY_SIZE], data: &[u8], output: &mut [u8; HASH_SIZE]) { + crypto::hmac::hmac_256::(key, data, output) } fn verify(key: &[u8; HMAC_KEY_SIZE], data: &[u8], mac: &[u8; HASH_SIZE]) -> bool { @@ -218,8 +223,8 @@ impl Hmac256 for SoftwareHmac256 { pub struct SoftwareHkdf256; impl Hkdf256 for SoftwareHkdf256 { - fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8]) -> [u8; HASH_SIZE] { - crypto::hkdf::hkdf_256::(ikm, salt, info) + fn hkdf_256(ikm: &[u8], salt: &[u8; HASH_SIZE], info: &[u8], okm: &mut [u8; HASH_SIZE]) { + crypto::hkdf::hkdf_256::(ikm, salt, info, okm); } } diff --git a/libraries/opensk/src/api/key_store.rs b/libraries/opensk/src/api/key_store.rs index 96b3ae4..3c19f2c 100644 --- a/libraries/opensk/src/api/key_store.rs +++ b/libraries/opensk/src/api/key_store.rs @@ -13,28 +13,29 @@ // limitations under the License. use crate::api::crypto::ecdsa::SecretKey as _; -use crate::api::rng::Rng; +use crate::ctap::secret::Secret; use crate::env::{EcdsaSk, Env}; -use alloc::vec::Vec; +use alloc::vec; use persistent_store::StoreError; +use rand_core::RngCore; /// Provides storage for secret keys. /// /// Implementations may use the environment store: [`STORAGE_KEY`] is reserved for this usage. pub trait KeyStore { /// Returns the AES key for key handles encryption. - fn key_handle_encryption(&mut self) -> Result<[u8; 32], Error>; + fn key_handle_encryption(&mut self) -> Result, Error>; /// Returns the key for key handles authentication. - fn key_handle_authentication(&mut self) -> Result<[u8; 32], Error>; + fn key_handle_authentication(&mut self) -> Result, Error>; /// Derives an ECDSA private key from a seed. /// /// The result is big-endian. - fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result<[u8; 32], Error>; + fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result, Error>; /// Generates a seed to derive an ECDSA private key. - fn generate_ecdsa_seed(&mut self) -> Result<[u8; 32], Error>; + fn generate_ecdsa_seed(&mut self) -> Result, Error>; /// Resets the key store. fn reset(&mut self) -> Result<(), Error>; @@ -53,23 +54,27 @@ pub const STORAGE_KEY: usize = 2046; pub trait Helper: Env {} impl KeyStore for T { - fn key_handle_encryption(&mut self) -> Result<[u8; 32], Error> { - Ok(get_master_keys(self)?.encryption) + fn key_handle_encryption(&mut self) -> Result, Error> { + Ok(get_master_keys(self)?.encryption.clone()) } - fn key_handle_authentication(&mut self) -> Result<[u8; 32], Error> { - Ok(get_master_keys(self)?.authentication) + fn key_handle_authentication(&mut self) -> Result, Error> { + Ok(get_master_keys(self)?.authentication.clone()) } - fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result<[u8; 32], Error> { + fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result, Error> { match EcdsaSk::::from_slice(seed) { None => Err(Error), - Some(_) => Ok(*seed), + Some(_) => { + let mut derived: Secret<[u8; 32]> = Secret::default(); + derived.copy_from_slice(seed); + Ok(derived) + } } } - fn generate_ecdsa_seed(&mut self) -> Result<[u8; 32], Error> { - let mut seed = [0; 32]; + fn generate_ecdsa_seed(&mut self) -> Result, Error> { + let mut seed = Secret::default(); EcdsaSk::::random(self.rng()).to_slice(&mut seed); Ok(seed) } @@ -82,31 +87,30 @@ impl KeyStore for T { /// Wrapper for master keys. struct MasterKeys { /// Master encryption key. - encryption: [u8; 32], + encryption: Secret<[u8; 32]>, /// Master authentication key. - authentication: [u8; 32], + authentication: Secret<[u8; 32]>, } fn get_master_keys(env: &mut impl Env) -> Result { let master_keys = match env.store().find(STORAGE_KEY)? { - Some(x) => x, + Some(x) if x.len() == 64 => x, + Some(_) => return Err(Error), None => { - let master_encryption_key = env.rng().gen_uniform_u8x32(); - let master_authentication_key = env.rng().gen_uniform_u8x32(); - let mut master_keys = Vec::with_capacity(64); - master_keys.extend_from_slice(&master_encryption_key); - master_keys.extend_from_slice(&master_authentication_key); + let mut master_keys = vec![0; 64]; + env.rng().fill_bytes(&mut master_keys); env.store().insert(STORAGE_KEY, &master_keys)?; master_keys } }; - if master_keys.len() != 64 { - return Err(Error); - } + let mut encryption: Secret<[u8; 32]> = Secret::default(); + encryption.copy_from_slice(array_ref![master_keys, 0, 32]); + let mut authentication: Secret<[u8; 32]> = Secret::default(); + authentication.copy_from_slice(array_ref![master_keys, 32, 32]); Ok(MasterKeys { - encryption: *array_ref![master_keys, 0, 32], - authentication: *array_ref![master_keys, 32, 32], + encryption, + authentication, }) } @@ -119,19 +123,20 @@ impl From for Error { #[cfg(test)] mod test { use super::*; + use crate::env::test::TestEnv; #[test] fn test_key_store() { - let mut env = crate::env::test::TestEnv::default(); + let mut env = TestEnv::default(); let key_store = env.key_store(); // Master keys are well-defined and stable. let encryption_key = key_store.key_handle_encryption().unwrap(); let authentication_key = key_store.key_handle_authentication().unwrap(); - assert_eq!(key_store.key_handle_encryption(), Ok(encryption_key)); + assert_eq!(&key_store.key_handle_encryption().unwrap(), &encryption_key); assert_eq!( - key_store.key_handle_authentication(), - Ok(authentication_key) + &key_store.key_handle_authentication().unwrap(), + &authentication_key ); // ECDSA seeds are well-defined and stable. @@ -142,7 +147,10 @@ mod test { // Master keys change after reset. We don't require this for ECDSA seeds because it's not // the case, but it might be better. key_store.reset().unwrap(); - assert!(key_store.key_handle_encryption().unwrap() != encryption_key); - assert!(key_store.key_handle_authentication().unwrap() != authentication_key); + assert_ne!(key_store.key_handle_encryption().unwrap(), encryption_key); + assert_ne!( + key_store.key_handle_authentication().unwrap(), + authentication_key + ); } } diff --git a/libraries/opensk/src/ctap/client_pin.rs b/libraries/opensk/src/ctap/client_pin.rs index 5d76f64..db425a5 100644 --- a/libraries/opensk/src/ctap/client_pin.rs +++ b/libraries/opensk/src/ctap/client_pin.rs @@ -18,6 +18,7 @@ use super::data_formats::{ }; use super::pin_protocol::{verify_pin_uv_auth_token, PinProtocol, SharedSecret}; use super::response::{AuthenticatorClientPinResponse, ResponseData}; +use super::secret::Secret; use super::status_code::Ctap2StatusCode; use super::token_state::PinUvAuthTokenState; #[cfg(test)] @@ -32,6 +33,7 @@ use crate::env::{Env, Hmac, Sha}; use alloc::str; use alloc::string::String; use alloc::vec::Vec; +use arrayref::array_ref; #[cfg(test)] use enum_iterator::IntoEnumIterator; use subtle::ConstantTimeEq; @@ -61,7 +63,7 @@ const PIN_PADDED_LENGTH: usize = 64; fn decrypt_pin( shared_secret: &SharedSecret, new_pin_enc: Vec, -) -> Result, Ctap2StatusCode> { +) -> Result, Ctap2StatusCode> { let decrypted_pin = shared_secret.decrypt(&new_pin_enc)?; if decrypted_pin.len() != PIN_PADDED_LENGTH { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); @@ -69,7 +71,13 @@ fn decrypt_pin( // In CTAP 2.1, the specification changed. The new wording might lead to // different behavior when there are non-zero bytes after zero bytes. // This implementation consistently ignores those degenerate cases. - Ok(decrypted_pin.into_iter().take_while(|&c| c != 0).collect()) + let len = decrypted_pin + .iter() + .position(|&c| c == 0) + .unwrap_or(decrypted_pin.len()); + let mut result = Secret::new(len); + result.copy_from_slice(&decrypted_pin[..len]); + Ok(result) } /// Stores a hash prefix of the new PIN in the persistent storage, if correct. @@ -88,10 +96,14 @@ fn check_and_store_new_pin( if pin_length < min_pin_length || pin.len() == PIN_PADDED_LENGTH { return Err(Ctap2StatusCode::CTAP2_ERR_PIN_POLICY_VIOLATION); } - let mut pin_hash = [0u8; PIN_AUTH_LENGTH]; - pin_hash.copy_from_slice(&Sha::::digest(&pin[..])[..PIN_AUTH_LENGTH]); + let mut pin_hash = Secret::default(); + Sha::::digest_mut(&pin, &mut pin_hash); // The PIN length is always < PIN_PADDED_LENGTH < 256. - storage::set_pin(env, &pin_hash, pin_length as u8)?; + storage::set_pin( + env, + array_ref!(pin_hash, 0, PIN_AUTH_LENGTH), + pin_length as u8, + )?; Ok(()) } @@ -471,10 +483,18 @@ impl ClientPin { if decrypted_salts.len() != 32 && decrypted_salts.len() != 64 { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } - let mut output = Hmac::::mac(cred_random, &decrypted_salts[..32]).to_vec(); + let mut output = Secret::new(decrypted_salts.len()); + Hmac::::mac( + cred_random, + &decrypted_salts[..32], + array_mut_ref![&mut output, 0, 32], + ); if decrypted_salts.len() == 64 { - let mut output2 = Hmac::::mac(cred_random, &decrypted_salts[32..]).to_vec(); - output.append(&mut output2); + Hmac::::mac( + cred_random, + &decrypted_salts[32..], + array_mut_ref![&mut output, 32, 32], + ); } shared_secret.encrypt(env, &output) } @@ -575,6 +595,7 @@ impl ClientPin { mod test { use super::super::pin_protocol::authenticate_pin_uv_auth_token; use super::*; + use crate::api::crypto::HASH_SIZE; use crate::env::test::TestEnv; use crate::env::EcdhSk; use alloc::vec; @@ -681,30 +702,30 @@ mod test { .unwrap(); let ciphertext = shared_secret_v1.encrypt(&mut env, &message).unwrap(); let plaintext = shared_secret_v2.decrypt(&ciphertext).unwrap(); - assert_ne!(&message, &plaintext); + assert_ne!(&message, &*plaintext); let ciphertext = shared_secret_v2.encrypt(&mut env, &message).unwrap(); let plaintext = shared_secret_v1.decrypt(&ciphertext).unwrap(); - assert_ne!(&message, &plaintext); + assert_ne!(&message, &*plaintext); let fake_secret_v1 = pin_protocol_v1 .decapsulate(pin_protocol_v2.get_public_key(), PinUvAuthProtocol::V1) .unwrap(); let ciphertext = shared_secret_v1.encrypt(&mut env, &message).unwrap(); let plaintext = fake_secret_v1.decrypt(&ciphertext).unwrap(); - assert_ne!(&message, &plaintext); + assert_ne!(&message, &*plaintext); let ciphertext = fake_secret_v1.encrypt(&mut env, &message).unwrap(); let plaintext = shared_secret_v1.decrypt(&ciphertext).unwrap(); - assert_ne!(&message, &plaintext); + assert_ne!(&message, &*plaintext); let fake_secret_v2 = pin_protocol_v2 .decapsulate(pin_protocol_v1.get_public_key(), PinUvAuthProtocol::V2) .unwrap(); let ciphertext = shared_secret_v2.encrypt(&mut env, &message).unwrap(); let plaintext = fake_secret_v2.decrypt(&ciphertext).unwrap(); - assert_ne!(&message, &plaintext); + assert_ne!(&message, &*plaintext); let ciphertext = fake_secret_v2.encrypt(&mut env, &message).unwrap(); let plaintext = shared_secret_v2.decrypt(&ciphertext).unwrap(); - assert_ne!(&message, &plaintext); + assert_ne!(&message, &*plaintext); } fn test_helper_verify_pin_hash_enc(pin_uv_auth_protocol: PinUvAuthProtocol) { @@ -964,7 +985,7 @@ mod test { _ => panic!("Invalid response type"), }; assert_eq!( - &shared_secret.decrypt(&encrypted_token).unwrap(), + &*shared_secret.decrypt(&encrypted_token).unwrap(), client_pin .get_pin_protocol(pin_uv_auth_protocol) .get_pin_uv_auth_token() @@ -1058,7 +1079,7 @@ mod test { _ => panic!("Invalid response type"), }; assert_eq!( - &shared_secret.decrypt(&encrypted_token).unwrap(), + &*shared_secret.decrypt(&encrypted_token).unwrap(), client_pin .get_pin_protocol(pin_uv_auth_protocol) .get_pin_uv_auth_token() @@ -1154,14 +1175,14 @@ mod test { let new_pin_enc = encrypt_pin(&shared_secret, b"1234".to_vec()); assert_eq!( - decrypt_pin::(&shared_secret, new_pin_enc), - Ok(b"1234".to_vec()), + &*decrypt_pin::(&shared_secret, new_pin_enc).unwrap(), + b"1234", ); let new_pin_enc = encrypt_pin(&shared_secret, b"123".to_vec()); assert_eq!( - decrypt_pin::(&shared_secret, new_pin_enc), - Ok(b"123".to_vec()), + &*decrypt_pin::(&shared_secret, new_pin_enc).unwrap(), + b"123", ); // Encrypted PIN is too short. @@ -1261,7 +1282,7 @@ mod test { pin_uv_auth_protocol, }; let output = client_pin.process_hmac_secret(&mut env, hmac_secret_input, cred_random); - output.map(|v| shared_secret.decrypt(&v).unwrap()) + output.map(|v| shared_secret.decrypt(&v).unwrap().expose_secret_to_vec()) } fn test_helper_process_hmac_secret_bad_salt_auth(pin_uv_auth_protocol: PinUvAuthProtocol) { @@ -1298,12 +1319,13 @@ mod test { let cred_random = [0xC9; 32]; let salt = vec![0x01; 32]; - let expected_output = Hmac::::mac(&cred_random, &salt); + let mut expected_output = [0; HASH_SIZE]; + Hmac::::mac(&cred_random, &salt, &mut expected_output); let output = get_process_hmac_secret_decrypted_output(pin_uv_auth_protocol, &cred_random, salt) .unwrap(); - assert_eq!(&output, &expected_output); + assert_eq!(&*output, &expected_output); } #[test] @@ -1321,8 +1343,10 @@ mod test { let salt1 = [0x01; 32]; let salt2 = [0x02; 32]; - let expected_output1 = Hmac::::mac(&cred_random, &salt1); - let expected_output2 = Hmac::::mac(&cred_random, &salt2); + let mut expected_output1 = [0; HASH_SIZE]; + let mut expected_output2 = [0; HASH_SIZE]; + Hmac::::mac(&cred_random, &salt1, &mut expected_output1); + Hmac::::mac(&cred_random, &salt2, &mut expected_output2); let mut salt12 = vec![0x00; 64]; salt12[..32].copy_from_slice(&salt1); diff --git a/libraries/opensk/src/ctap/credential_id.rs b/libraries/opensk/src/ctap/credential_id.rs index 90c9da2..49b204f 100644 --- a/libraries/opensk/src/ctap/credential_id.rs +++ b/libraries/opensk/src/ctap/credential_id.rs @@ -20,6 +20,7 @@ use super::status_code::Ctap2StatusCode; use super::{cbor_read, cbor_write}; use crate::api::crypto::aes256::Aes256; use crate::api::crypto::hmac256::Hmac256; +use crate::api::crypto::HASH_SIZE; use crate::api::key_store::KeyStore; use crate::ctap::data_formats::{extract_byte_string, extract_map}; use crate::env::{AesKey, Env, Hmac}; @@ -69,8 +70,8 @@ fn decrypt_legacy_credential_id( env: &mut E, bytes: &[u8], ) -> Result, Ctap2StatusCode> { - let aes_key = AesKey::::new(&env.key_store().key_handle_encryption()?); - let plaintext = aes256_cbc_decrypt::(&aes_key, bytes, true)?; + let aes_key = AesKey::::new(&*env.key_store().key_handle_encryption()?); + let plaintext = aes256_cbc_decrypt::(&aes_key, bytes, true)?.expose_secret_to_vec(); if plaintext.len() != 64 { return Ok(None); } @@ -91,11 +92,11 @@ fn decrypt_cbor_credential_id( env: &mut E, bytes: &[u8], ) -> Result, Ctap2StatusCode> { - let aes_key = AesKey::::new(&env.key_store().key_handle_encryption()?); - let mut plaintext = aes256_cbc_decrypt::(&aes_key, bytes, true)?; - remove_padding(&mut plaintext)?; + let aes_key = AesKey::::new(&*env.key_store().key_handle_encryption()?); + let plaintext = aes256_cbc_decrypt::(&aes_key, bytes, true)?; + let unpadded = remove_padding(&plaintext)?; - let cbor_credential_source = cbor_read(plaintext.as_slice())?; + let cbor_credential_source = cbor_read(unpadded)?; destructure_cbor_map! { let { CredentialSourceField::PrivateKey => private_key, @@ -138,7 +139,7 @@ fn add_padding(data: &mut Vec) -> Result<(), Ctap2StatusCode> { Ok(()) } -fn remove_padding(data: &mut Vec) -> Result<(), Ctap2StatusCode> { +fn remove_padding(data: &[u8]) -> Result<&[u8], Ctap2StatusCode> { if data.len() != MAX_PADDING_LENGTH as usize + 1 { // This is an internal error instead of corrupted credential ID which we should just ignore because // we've already checked that the HMAC matched. @@ -148,13 +149,13 @@ fn remove_padding(data: &mut Vec) -> Result<(), Ctap2StatusCode> { if pad_length == 0 || pad_length > MAX_PADDING_LENGTH { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } - if !data - .drain((data.len() - pad_length as usize)..) - .all(|x| x == pad_length) + if !data[(data.len() - pad_length as usize)..] + .iter() + .all(|x| *x == pad_length) { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } - Ok(()) + Ok(&data[..data.len() - pad_length as usize]) } /// Encrypts the given private key, relying party ID hash, and some other metadata into a credential ID. @@ -171,21 +172,23 @@ pub fn encrypt_to_credential_id( let mut payload = Vec::new(); let cbor = cbor_map_options! { CredentialSourceField::PrivateKey => private_key, - CredentialSourceField::RpIdHash=> rp_id_hash, + CredentialSourceField::RpIdHash => rp_id_hash, CredentialSourceField::CredProtectPolicy => cred_protect_policy, CredentialSourceField::CredBlob => cred_blob, }; cbor_write(cbor, &mut payload)?; add_padding(&mut payload)?; - let aes_key = AesKey::::new(&env.key_store().key_handle_encryption()?); + let aes_key = AesKey::::new(&*env.key_store().key_handle_encryption()?); let encrypted_payload = aes256_cbc_encrypt(env, &aes_key, &payload, true)?; let mut credential_id = encrypted_payload; credential_id.insert(0, CBOR_CREDENTIAL_ID_VERSION); - let id_hmac = Hmac::::mac( - &env.key_store().key_handle_authentication()?, + let mut id_hmac = [0; HASH_SIZE]; + Hmac::::mac( + &*env.key_store().key_handle_authentication()?, &credential_id[..], + &mut id_hmac, ); credential_id.extend(&id_hmac); Ok(credential_id) @@ -218,7 +221,7 @@ pub fn decrypt_credential_id( } let hmac_message_size = credential_id.len() - 32; if !Hmac::::verify( - &env.key_store().key_handle_authentication()?, + &*env.key_store().key_handle_authentication()?, &credential_id[..hmac_message_size], array_ref![credential_id, hmac_message_size, 32], ) { @@ -314,7 +317,8 @@ mod test { // Override the HMAC to pass the check. encrypted_id.truncate(&encrypted_id.len() - 32); let hmac_key = env.key_store().key_handle_authentication().unwrap(); - let id_hmac = Hmac::::mac(&hmac_key, &encrypted_id[..]); + let mut id_hmac = [0; HASH_SIZE]; + Hmac::::mac(&hmac_key, &encrypted_id[..], &mut id_hmac); encrypted_id.extend(&id_hmac); assert_eq!( @@ -384,15 +388,17 @@ mod test { private_key: EcdsaSk, application: &[u8; 32], ) -> Result, Ctap2StatusCode> { - let aes_key = AesKey::::new(&env.key_store().key_handle_encryption()?); + let aes_key = AesKey::::new(&*env.key_store().key_handle_encryption()?); let mut plaintext = [0; 64]; private_key.to_slice(array_mut_ref!(plaintext, 0, 32)); plaintext[32..64].copy_from_slice(application); let mut encrypted_id = aes256_cbc_encrypt(env, &aes_key, &plaintext, true)?; - let id_hmac = Hmac::::mac( - &env.key_store().key_handle_authentication()?, + let mut id_hmac = [0; HASH_SIZE]; + Hmac::::mac( + &*env.key_store().key_handle_authentication()?, &encrypted_id[..], + &mut id_hmac, ); encrypted_id.extend(&id_hmac); Ok(encrypted_id) diff --git a/libraries/opensk/src/ctap/crypto_wrapper.rs b/libraries/opensk/src/ctap/crypto_wrapper.rs index 6ab4689..a641b24 100644 --- a/libraries/opensk/src/ctap/crypto_wrapper.rs +++ b/libraries/opensk/src/ctap/crypto_wrapper.rs @@ -15,14 +15,16 @@ use crate::api::crypto::aes256::Aes256; use crate::api::crypto::ecdsa::{SecretKey as _, Signature}; use crate::api::key_store::KeyStore; -#[cfg(feature = "ed25519")] -use crate::api::rng::Rng; use crate::ctap::data_formats::{extract_array, extract_byte_string, CoseKey, SignatureAlgorithm}; +use crate::ctap::secret::Secret; use crate::ctap::status_code::Ctap2StatusCode; use crate::env::{AesKey, EcdsaSk, Env}; use alloc::vec; use alloc::vec::Vec; use core::convert::TryFrom; +use core::ops::Deref; +#[cfg(feature = "ed25519")] +use core::ops::DerefMut; use rand_core::RngCore; use sk_cbor as cbor; use sk_cbor::{cbor_array, cbor_bytes, cbor_int}; @@ -56,7 +58,7 @@ pub fn aes256_cbc_decrypt( aes_key: &AesKey, ciphertext: &[u8], embeds_iv: bool, -) -> Result, Ctap2StatusCode> { +) -> Result, Ctap2StatusCode> { if ciphertext.len() % 16 != 0 || (embeds_iv && ciphertext.is_empty()) { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } @@ -66,7 +68,8 @@ pub fn aes256_cbc_decrypt( } else { (&[0u8; 16], ciphertext) }; - let mut plaintext = ciphertext.to_vec(); + let mut plaintext = Secret::new(ciphertext.len()); + plaintext.copy_from_slice(ciphertext); aes_key.decrypt_cbc(iv, &mut plaintext); Ok(plaintext) } @@ -78,7 +81,7 @@ pub fn aes256_cbc_decrypt( pub enum PrivateKey { // We store the seed instead of the key since we can't get the seed back from the key. We could // store both if we believe deriving the key is done more than once and costly. - Ecdsa([u8; 32]), + Ecdsa(Secret<[u8; 32]>), #[cfg(feature = "ed25519")] Ed25519(ed25519_compact::SecretKey), } @@ -96,8 +99,9 @@ impl PrivateKey { } #[cfg(feature = "ed25519")] SignatureAlgorithm::Eddsa => { - let bytes = env.rng().gen_uniform_u8x32(); - Self::new_ed25519_from_bytes(&bytes).unwrap() + let mut bytes: Secret<[u8; 32]> = Secret::default(); + env.rng().fill_bytes(bytes.deref_mut()); + Self::new_ed25519_from_bytes(&*bytes).unwrap() } SignatureAlgorithm::Unknown => unreachable!(), } @@ -115,7 +119,9 @@ impl PrivateKey { if bytes.len() != 32 { return None; } - Some(PrivateKey::Ecdsa(*array_ref!(bytes, 0, 32))) + let mut seed: Secret<[u8; 32]> = Secret::default(); + seed.copy_from_slice(bytes); + Some(PrivateKey::Ecdsa(seed)) } #[cfg(feature = "ed25519")] @@ -172,12 +178,14 @@ impl PrivateKey { } /// Writes the key bytes. - pub fn to_bytes(&self) -> Vec { + pub fn to_bytes(&self) -> Secret<[u8]> { + let mut bytes = Secret::new(32); match self { - PrivateKey::Ecdsa(ecdsa_seed) => ecdsa_seed.to_vec(), + PrivateKey::Ecdsa(ecdsa_seed) => bytes.copy_from_slice(ecdsa_seed.deref()), #[cfg(feature = "ed25519")] - PrivateKey::Ed25519(ed25519_key) => ed25519_key.seed().to_vec(), + PrivateKey::Ed25519(ed25519_key) => bytes.copy_from_slice(ed25519_key.seed().deref()), } + bytes } } @@ -190,10 +198,12 @@ fn ecdsa_key_from_seed( } impl From<&PrivateKey> for cbor::Value { + /// Writes a private key into CBOR format. This exposes the cryptographic secret. + // TODO called in encrypt_to_credential_id and PublicKeyCredentialSource, needs zeroization fn from(private_key: &PrivateKey) -> Self { cbor_array![ cbor_int!(private_key.signature_algorithm() as i64), - cbor_bytes!(private_key.to_bytes()), + cbor_bytes!(private_key.to_bytes().expose_secret_to_vec()), ] } } @@ -230,7 +240,7 @@ mod test { let plaintext = vec![0xAA; 64]; let ciphertext = aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, true).unwrap(); let decrypted = aes256_cbc_decrypt::(&aes_key, &ciphertext, true).unwrap(); - assert_eq!(decrypted, plaintext); + assert_eq!(*decrypted, plaintext); } #[test] @@ -240,7 +250,7 @@ mod test { let plaintext = vec![0xAA; 64]; let ciphertext = aes256_cbc_encrypt(&mut env, &aes_key, &plaintext, false).unwrap(); let decrypted = aes256_cbc_decrypt::(&aes_key, &ciphertext, false).unwrap(); - assert_eq!(decrypted, plaintext); + assert_eq!(*decrypted, plaintext); } #[test] @@ -253,7 +263,7 @@ mod test { let mut ciphertext_with_iv = vec![0u8; 16]; ciphertext_with_iv.append(&mut ciphertext_no_iv); let decrypted = aes256_cbc_decrypt::(&aes_key, &ciphertext_with_iv, true).unwrap(); - assert_eq!(decrypted, plaintext); + assert_eq!(*decrypted, plaintext); } #[test] @@ -268,7 +278,7 @@ mod test { expected_plaintext[i] ^= 0xBB; } let decrypted = aes256_cbc_decrypt::(&aes_key, &ciphertext, true).unwrap(); - assert_eq!(decrypted, expected_plaintext); + assert_eq!(*decrypted, expected_plaintext); } #[test] diff --git a/libraries/opensk/src/ctap/ctap1.rs b/libraries/opensk/src/ctap/ctap1.rs index 006cc15..6d598ce 100644 --- a/libraries/opensk/src/ctap/ctap1.rs +++ b/libraries/opensk/src/ctap/ctap1.rs @@ -359,6 +359,7 @@ mod test { use super::*; use crate::api::crypto::sha256::Sha256; use crate::api::customization::Customization; + use crate::ctap::secret::Secret; use crate::ctap::storage; use crate::env::test::TestEnv; use crate::env::Sha; @@ -432,7 +433,7 @@ mod test { assert_eq!(response, Err(Ctap1StatusCode::SW_INTERNAL_EXCEPTION)); let attestation = Attestation { - private_key: [0x41; 32], + private_key: Secret::from_exposed_secret([0x41; 32]), certificate: vec![0x99; 100], }; env.attestation_store() diff --git a/libraries/opensk/src/ctap/mod.rs b/libraries/opensk/src/ctap/mod.rs index 49d1636..8e8f61e 100644 --- a/libraries/opensk/src/ctap/mod.rs +++ b/libraries/opensk/src/ctap/mod.rs @@ -27,6 +27,7 @@ mod large_blobs; pub mod main_hid; mod pin_protocol; pub mod response; +pub mod secret; pub mod status_code; mod storage; mod token_state; @@ -57,6 +58,7 @@ use self::response::{ AuthenticatorGetAssertionResponse, AuthenticatorGetInfoResponse, AuthenticatorMakeCredentialResponse, ResponseData, }; +use self::secret::Secret; use self::status_code::Ctap2StatusCode; #[cfg(feature = "with_ctap1")] use self::u2f_up::U2fUserPresenceState; @@ -66,6 +68,7 @@ use crate::api::connection::{HidConnection, SendOrRecvStatus, UsbEndpoint}; use crate::api::crypto::ecdsa::{SecretKey as _, Signature}; use crate::api::crypto::hkdf256::Hkdf256; use crate::api::crypto::sha256::Sha256; +use crate::api::crypto::HASH_SIZE; use crate::api::customization::Customization; use crate::api::rng::Rng; use crate::api::user_presence::{UserPresence, UserPresenceError}; @@ -950,11 +953,13 @@ impl CtapState { env: &mut E, private_key: &PrivateKey, has_uv: bool, - ) -> Result<[u8; 32], Ctap2StatusCode> { + ) -> Result, Ctap2StatusCode> { 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(Hkdf::::hkdf_256(&key, salt, b"credRandom")) + let mut output = Secret::default(); + Hkdf::::hkdf_256(&key, salt, b"credRandom", &mut output); + Ok(output) } // Processes the input of a get_assertion operation for a given credential diff --git a/libraries/opensk/src/ctap/pin_protocol.rs b/libraries/opensk/src/ctap/pin_protocol.rs index 77fd87c..4048bcd 100644 --- a/libraries/opensk/src/ctap/pin_protocol.rs +++ b/libraries/opensk/src/ctap/pin_protocol.rs @@ -17,20 +17,22 @@ use crate::api::crypto::ecdh::{PublicKey as _, SecretKey as _, SharedSecret as _ use crate::api::crypto::hkdf256::Hkdf256; use crate::api::crypto::hmac256::Hmac256; use crate::api::crypto::sha256::Sha256; -use crate::api::rng::Rng; use crate::ctap::client_pin::PIN_TOKEN_LENGTH; use crate::ctap::crypto_wrapper::{aes256_cbc_decrypt, aes256_cbc_encrypt}; use crate::ctap::data_formats::{CoseKey, PinUvAuthProtocol}; +use crate::ctap::secret::Secret; use crate::ctap::status_code::Ctap2StatusCode; #[cfg(test)] use crate::env::test::TestEnv; use crate::env::{AesKey, EcdhPk, EcdhSk, Env, Hkdf, Hmac, Sha}; use alloc::vec::Vec; +use core::ops::DerefMut; +use rand_core::RngCore; /// Implements common functions between existing PIN protocols for handshakes. pub struct PinProtocol { key_agreement_key: EcdhSk, - pin_uv_auth_token: [u8; PIN_TOKEN_LENGTH], + pin_uv_auth_token: Secret<[u8; PIN_TOKEN_LENGTH]>, } impl PinProtocol { @@ -39,7 +41,8 @@ impl PinProtocol { /// This function implements "initialize" from the specification. pub fn new(env: &mut E) -> Self { let key_agreement_key = EcdhSk::::random(env.rng()); - let pin_uv_auth_token = env.rng().gen_uniform_u8x32(); + let mut pin_uv_auth_token: Secret<[u8; 32]> = Secret::default(); + env.rng().fill_bytes(pin_uv_auth_token.deref_mut()); PinProtocol { key_agreement_key, pin_uv_auth_token, @@ -53,7 +56,7 @@ impl PinProtocol { /// Generates a fresh pinUvAuthToken. pub fn reset_pin_uv_auth_token(&mut self, env: &mut E) { - self.pin_uv_auth_token = env.rng().gen_uniform_u8x32(); + env.rng().fill_bytes(self.pin_uv_auth_token.deref_mut()); } /// Returns the authenticator’s public key as a CoseKey structure. @@ -70,10 +73,10 @@ impl PinProtocol { let (x_bytes, y_bytes) = peer_cose_key.try_into_ecdh_coordinates()?; let pk = EcdhPk::::from_coordinates(&x_bytes, &y_bytes) .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)?; - let handshake = self - .key_agreement_key + let mut handshake = Secret::default(); + self.key_agreement_key .diffie_hellman(&pk) - .raw_secret_bytes(); + .raw_secret_bytes(&mut handshake); Ok(SharedSecret::new(pin_uv_auth_protocol, handshake)) } @@ -90,7 +93,7 @@ impl PinProtocol { ) -> Self { PinProtocol { key_agreement_key, - pin_uv_auth_token, + pin_uv_auth_token: Secret::from_exposed_secret(pin_uv_auth_token), } } } @@ -102,9 +105,11 @@ pub fn authenticate_pin_uv_auth_token( message: &[u8], pin_uv_auth_protocol: PinUvAuthProtocol, ) -> Vec { + let mut mac = [0; 32]; + Hmac::::mac(token, message, &mut mac); match pin_uv_auth_protocol { - PinUvAuthProtocol::V1 => Hmac::::mac(token, message)[..16].to_vec(), - PinUvAuthProtocol::V2 => Hmac::::mac(token, message).to_vec(), + PinUvAuthProtocol::V1 => mac[..16].to_vec(), + PinUvAuthProtocol::V2 => mac.to_vec(), } } @@ -130,7 +135,7 @@ impl SharedSecret { /// Creates a new shared secret for the respective PIN protocol. /// /// This enum wraps all types of shared secrets. - fn new(pin_uv_auth_protocol: PinUvAuthProtocol, handshake: [u8; 32]) -> Self { + fn new(pin_uv_auth_protocol: PinUvAuthProtocol, handshake: Secret<[u8; 32]>) -> Self { match pin_uv_auth_protocol { PinUvAuthProtocol::V1 => SharedSecret::V1(SharedSecretV1::new(handshake)), PinUvAuthProtocol::V2 => SharedSecret::V2(SharedSecretV2::new(handshake)), @@ -146,7 +151,7 @@ impl SharedSecret { } /// Returns the decrypted ciphertext. - pub fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { + pub fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { match self { SharedSecret::V1(s) => s.decrypt(ciphertext), SharedSecret::V2(s) => s.decrypt(ciphertext), @@ -163,6 +168,7 @@ impl SharedSecret { /// Creates a signature that matches verify. #[cfg(test)] + // Does not return a Secret as it is only used in tests. pub fn authenticate(&self, message: &[u8]) -> Vec { match self { SharedSecret::V1(s) => s.authenticate(message), @@ -202,14 +208,15 @@ fn verify_v2( } pub struct SharedSecretV1 { - common_secret: [u8; 32], + common_secret: Secret<[u8; 32]>, aes_key: AesKey, } impl SharedSecretV1 { /// Creates a new shared secret from the handshake result. - fn new(handshake: [u8; 32]) -> Self { - let common_secret = Sha::::digest(&handshake); + fn new(handshake: Secret<[u8; 32]>) -> Self { + let mut common_secret = Secret::default(); + Sha::::digest_mut(&*handshake, &mut common_secret); let aes_key = AesKey::::new(&common_secret); SharedSecretV1 { common_secret, @@ -221,7 +228,7 @@ impl SharedSecretV1 { aes256_cbc_encrypt(env, &self.aes_key, plaintext, false) } - fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { + fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { aes256_cbc_decrypt::(&self.aes_key, ciphertext, false) } @@ -231,22 +238,27 @@ impl SharedSecretV1 { #[cfg(test)] fn authenticate(&self, message: &[u8]) -> Vec { - Hmac::::mac(&self.common_secret, message)[..16].to_vec() + let mut output = [0; 32]; + Hmac::::mac(&self.common_secret, message, &mut output); + output[..16].to_vec() } } pub struct SharedSecretV2 { aes_key: AesKey, - hmac_key: [u8; 32], + hmac_key: Secret<[u8; 32]>, } impl SharedSecretV2 { /// Creates a new shared secret from the handshake result. - fn new(handshake: [u8; 32]) -> Self { - let aes_key = Hkdf::::hkdf_empty_salt_256(&handshake, b"CTAP2 AES key"); + fn new(handshake: Secret<[u8; 32]>) -> Self { + let mut aes_key_bytes = Secret::default(); + Hkdf::::hkdf_empty_salt_256(&*handshake, b"CTAP2 AES key", &mut aes_key_bytes); + let mut hmac_key = Secret::default(); + Hkdf::::hkdf_empty_salt_256(&*handshake, b"CTAP2 HMAC key", &mut hmac_key); SharedSecretV2 { - aes_key: AesKey::::new(&aes_key), - hmac_key: Hkdf::::hkdf_empty_salt_256(&handshake, b"CTAP2 HMAC key"), + aes_key: AesKey::::new(&aes_key_bytes), + hmac_key, } } @@ -254,7 +266,7 @@ impl SharedSecretV2 { aes256_cbc_encrypt(env, &self.aes_key, plaintext, true) } - fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { + fn decrypt(&self, ciphertext: &[u8]) -> Result, Ctap2StatusCode> { aes256_cbc_decrypt::(&self.aes_key, ciphertext, true) } @@ -264,7 +276,9 @@ impl SharedSecretV2 { #[cfg(test)] fn authenticate(&self, message: &[u8]) -> Vec { - Hmac::::mac(&self.hmac_key, message).to_vec() + let mut output = [0; 32]; + Hmac::::mac(&self.hmac_key, message, &mut output); + output.to_vec() } } @@ -296,15 +310,16 @@ mod test { #[test] fn test_shared_secret_v1_encrypt_decrypt() { let mut env = TestEnv::default(); - let shared_secret = SharedSecretV1::::new([0x55; 32]); - let plaintext = vec![0xAA; 64]; + let shared_secret = SharedSecretV1::::new(Secret::from_exposed_secret([0x55; 32])); + let mut plaintext = Secret::new(64); + plaintext.fill(0xAA); let ciphertext = shared_secret.encrypt(&mut env, &plaintext).unwrap(); assert_eq!(shared_secret.decrypt(&ciphertext), Ok(plaintext)); } #[test] fn test_shared_secret_v1_authenticate_verify() { - let shared_secret = SharedSecretV1::::new([0x55; 32]); + let shared_secret = SharedSecretV1::::new(Secret::from_exposed_secret([0x55; 32])); let message = [0xAA; 32]; let signature = shared_secret.authenticate(&message); assert_eq!(shared_secret.verify(&message, &signature), Ok(())); @@ -312,7 +327,7 @@ mod test { #[test] fn test_shared_secret_v1_verify() { - let shared_secret = SharedSecretV1::::new([0x55; 32]); + let shared_secret = SharedSecretV1::::new(Secret::from_exposed_secret([0x55; 32])); let message = [0xAA]; let signature = [ 0x8B, 0x60, 0x15, 0x7D, 0xF3, 0x44, 0x82, 0x2E, 0x54, 0x34, 0x7A, 0x01, 0xFB, 0x02, @@ -332,15 +347,16 @@ mod test { #[test] fn test_shared_secret_v2_encrypt_decrypt() { let mut env = TestEnv::default(); - let shared_secret = SharedSecretV2::::new([0x55; 32]); - let plaintext = vec![0xAA; 64]; + let shared_secret = SharedSecretV2::::new(Secret::from_exposed_secret([0x55; 32])); + let mut plaintext = Secret::new(64); + plaintext.fill(0xAA); let ciphertext = shared_secret.encrypt(&mut env, &plaintext).unwrap(); assert_eq!(shared_secret.decrypt(&ciphertext), Ok(plaintext)); } #[test] fn test_shared_secret_v2_authenticate_verify() { - let shared_secret = SharedSecretV2::::new([0x55; 32]); + let shared_secret = SharedSecretV2::::new(Secret::from_exposed_secret([0x55; 32])); let message = [0xAA; 32]; let signature = shared_secret.authenticate(&message); assert_eq!(shared_secret.verify(&message, &signature), Ok(())); @@ -348,7 +364,7 @@ mod test { #[test] fn test_shared_secret_v2_verify() { - let shared_secret = SharedSecretV2::::new([0x55; 32]); + let shared_secret = SharedSecretV2::::new(Secret::from_exposed_secret([0x55; 32])); let message = [0xAA]; let signature = [ 0xC0, 0x3F, 0x2A, 0x22, 0x5C, 0xC3, 0x4E, 0x05, 0xC1, 0x0E, 0x72, 0x9C, 0x8D, 0xD5, @@ -378,7 +394,8 @@ mod test { let shared_secret2 = pin_protocol2 .decapsulate(pin_protocol1.get_public_key(), protocol) .unwrap(); - let plaintext = vec![0xAA; 64]; + let mut plaintext = Secret::new(64); + plaintext.fill(0xAA); let ciphertext = shared_secret1.encrypt(&mut env, &plaintext).unwrap(); assert_eq!(plaintext, shared_secret2.decrypt(&ciphertext).unwrap()); } diff --git a/libraries/opensk/src/ctap/secret.rs b/libraries/opensk/src/ctap/secret.rs new file mode 100644 index 0000000..9ffd4d8 --- /dev/null +++ b/libraries/opensk/src/ctap/secret.rs @@ -0,0 +1,89 @@ +// Copyright 2022-2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Provides secret handling support. +//! +//! This module provides the `Secret` type to store secrets of type `T` while minimizing the +//! amount of time the secret is present in RAM. This type ensures that: +//! - The secret only lives in one place. It is not implicitly copied (or equivalently moved). +//! - The secret is zeroed out (using the `zeroize` crate) after usage. +//! +//! It is possible to escape those principles: +//! - By using functions containing the sequence of words "expose secret" in their name. +//! - By explicitly cloning or copying the secret. +//! +//! We don't use the secrecy crate because the SecretBox is incorrect and it doesn't provide a +//! mutable version of ExposeSecret. +//! +//! Also note that eventually, we may use some Randomize trait instead of Zeroize, to prevent +//! side-channels. + +use alloc::boxed::Box; +use alloc::vec; +use alloc::vec::Vec; +use core::ops::{Deref, DerefMut}; +use zeroize::Zeroize; + +/// Defines a secret that is zeroized on Drop. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Secret(Box); + +impl Secret { + /// Imports an already exposed secret. + /// + /// This is provided for convenience and should be avoided when possible. The expected usage is + /// to create a default secret, then write its content in place. + pub fn from_exposed_secret(secret: T) -> Self { + Self(Box::new(secret)) + } +} + +impl Secret<[u8]> { + pub fn new(len: usize) -> Self { + Self(vec![0; len].into_boxed_slice()) + } + + /// Extracts the secret as a Vec. + /// + /// This means that the secret won't be zeroed-out on Drop. + pub fn expose_secret_to_vec(mut self) -> Vec { + core::mem::take(&mut self.0).into() + } +} + +impl Default for Secret { + fn default() -> Self { + Secret(Box::default()) + } +} + +impl Drop for Secret { + fn drop(&mut self) { + self.0.zeroize(); + } +} + +impl Deref for Secret { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.0.deref() + } +} + +impl DerefMut for Secret { + fn deref_mut(&mut self) -> &mut Self::Target { + self.0.deref_mut() + } +} diff --git a/libraries/opensk/src/ctap/storage.rs b/libraries/opensk/src/ctap/storage.rs index 507d839..9e991eb 100644 --- a/libraries/opensk/src/ctap/storage.rs +++ b/libraries/opensk/src/ctap/storage.rs @@ -624,6 +624,7 @@ mod test { use crate::ctap::data_formats::{ CredentialProtectionPolicy, PublicKeyCredentialSource, PublicKeyCredentialType, }; + use crate::ctap::secret::Secret; use crate::env::test::TestEnv; fn create_credential_source( @@ -936,7 +937,7 @@ mod test { // Make sure the persistent keys are initialized to dummy values. let dummy_attestation = Attestation { - private_key: [0x41; 32], + private_key: Secret::from_exposed_secret([0x41; 32]), certificate: vec![0xdd; 20], }; env.attestation_store() @@ -1083,7 +1084,7 @@ mod test { let mut env = TestEnv::default(); let dummy_attestation = Attestation { - private_key: [0x41; 32], + private_key: Secret::from_exposed_secret([0x41; 32]), certificate: vec![0xdd; 20], }; env.attestation_store() diff --git a/libraries/opensk/src/test_helpers/mod.rs b/libraries/opensk/src/test_helpers/mod.rs index 799160a..a89bcde 100644 --- a/libraries/opensk/src/test_helpers/mod.rs +++ b/libraries/opensk/src/test_helpers/mod.rs @@ -15,6 +15,7 @@ use crate::api::attestation_store::{self, AttestationStore}; use crate::ctap::command::{AuthenticatorConfigParameters, Command}; use crate::ctap::data_formats::ConfigSubCommand; +use crate::ctap::secret::Secret; use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::{Channel, CtapState}; use crate::env::Env; @@ -28,7 +29,7 @@ pub fn enable_enterprise_attestation( env: &mut E, ) -> Result { let attestation = attestation_store::Attestation { - private_key: [0x41; 32], + private_key: Secret::from_exposed_secret([0x41; 32]), certificate: vec![0xdd; 20], }; env.attestation_store() diff --git a/src/env/tock/commands.rs b/src/env/tock/commands.rs index 7bbe668..10ba7ac 100644 --- a/src/env/tock/commands.rs +++ b/src/env/tock/commands.rs @@ -27,6 +27,7 @@ use opensk::ctap::check_user_presence; use opensk::ctap::data_formats::{ extract_bool, extract_byte_string, extract_map, extract_unsigned, ok_or_missing, }; +use opensk::ctap::secret::Secret; use opensk::ctap::status_code::Ctap2StatusCode; use opensk::ctap::{cbor_read, cbor_write, Channel}; use opensk::env::{Env, Sha}; @@ -110,7 +111,7 @@ fn process_vendor_configure( // to not leak information. if current_attestation.is_none() { let attestation = Attestation { - private_key: data.private_key, + private_key: Secret::from_exposed_secret(data.private_key), certificate: data.certificate, }; env.attestation_store() @@ -491,7 +492,7 @@ mod test { assert_eq!( env.attestation_store().get(&attestation_store::Id::Batch), Ok(Some(Attestation { - private_key: dummy_key, + private_key: Secret::from_exposed_secret(dummy_key), certificate: dummy_cert.to_vec(), })) ); @@ -519,7 +520,7 @@ mod test { assert_eq!( env.attestation_store().get(&attestation_store::Id::Batch), Ok(Some(Attestation { - private_key: dummy_key, + private_key: Secret::from_exposed_secret(dummy_key), certificate: dummy_cert.to_vec(), })) ); diff --git a/src/env/tock/storage.rs b/src/env/tock/storage.rs index e48684d..d1dd57e 100644 --- a/src/env/tock/storage.rs +++ b/src/env/tock/storage.rs @@ -343,7 +343,8 @@ impl TockUpgradeStorage { // The hash implementation handles this in chunks, so no memory issues. hasher.update(partition_slice); } - let computed_hash = hasher.finalize(); + let mut computed_hash = [0; 32]; + hasher.finalize(&mut computed_hash); if &computed_hash != parse_metadata_hash(metadata) { return Err(StorageError::CustomError); }