From d793a992d3abcc68a68bd9aaa97ffa707ebd743c Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 29 Jun 2022 11:55:02 +0200 Subject: [PATCH] Add a key store to avoid storing secrets in the store --- src/api/key_store.rs | 138 +++++++++++++++++++++++++++++++++++++ src/api/mod.rs | 1 + src/ctap/crypto_wrapper.rs | 27 +++----- src/ctap/status_code.rs | 7 ++ src/ctap/storage.rs | 57 +-------------- src/ctap/storage/key.rs | 6 +- src/env/mod.rs | 3 + src/env/test/mod.rs | 5 ++ src/env/tock/mod.rs | 5 ++ 9 files changed, 172 insertions(+), 77 deletions(-) create mode 100644 src/api/key_store.rs diff --git a/src/api/key_store.rs b/src/api/key_store.rs new file mode 100644 index 0000000..8d68e58 --- /dev/null +++ b/src/api/key_store.rs @@ -0,0 +1,138 @@ +// Copyright 2022 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. + +use alloc::vec::Vec; +use crypto::ecdsa::SecKey; +use persistent_store::StoreError; +use rng256::Rng256; + +use crate::env::Env; + +/// Provides storage for secret keys. +/// +/// Implementations may use the environment store: [`STORE_KEY`] is reserved for this usage. +pub trait KeyStore { + /// Returns the AES key for key handles encryption. + fn kh_encryption(&mut self) -> Result<[u8; 32], Error>; + + /// Returns the key for key handles authentication. + fn kh_authentication(&mut self) -> Result<[u8; 32], 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>; + + /// Generates a seed to derive an ECDSA private key. + fn generate_ecdsa_seed(&mut self) -> Result<[u8; 32], Error>; + + /// Resets the key store. + fn reset(&mut self) -> Result<(), Error>; +} + +/// Key store errors. +/// +/// They are deliberately indistinguishable from each other to avoid leaking information. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct Error; + +/// Key of the environment store reserved for the key store. +pub const STORE_KEY: usize = 2046; + +impl KeyStore for T { + fn kh_encryption(&mut self) -> Result<[u8; 32], Error> { + Ok(get_master_keys(self)?.encryption) + } + + fn kh_authentication(&mut self) -> Result<[u8; 32], Error> { + Ok(get_master_keys(self)?.authentication) + } + + fn derive_ecdsa(&mut self, seed: &[u8; 32]) -> Result<[u8; 32], Error> { + match SecKey::from_bytes(seed) { + None => Err(Error), + Some(_) => Ok(*seed), + } + } + + fn generate_ecdsa_seed(&mut self) -> Result<[u8; 32], Error> { + let mut seed = [0; 32]; + SecKey::gensk(self.rng()).to_bytes(&mut seed); + Ok(seed) + } + + fn reset(&mut self) -> Result<(), Error> { + Ok(self.store().remove(STORE_KEY)?) + } +} + +/// Wrapper for master keys. +struct MasterKeys { + /// Master encryption key. + encryption: [u8; 32], + + /// Master authentication key. + authentication: [u8; 32], +} + +fn get_master_keys(env: &mut impl Env) -> Result { + let master_keys = match env.store().find(STORE_KEY)? { + Some(x) => x, + 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); + env.store().insert(STORE_KEY, &master_keys)?; + master_keys + } + }; + if master_keys.len() != 64 { + return Err(Error); + } + Ok(MasterKeys { + encryption: *array_ref![master_keys, 0, 32], + authentication: *array_ref![master_keys, 32, 32], + }) +} + +impl From for Error { + fn from(_: StoreError) -> Self { + Error + } +} + +#[test] +fn test_key_store() { + let mut env = crate::env::test::TestEnv::new(); + let key_store = env.key_store(); + + // Master keys are well-defined and stable. + let encryption_key = key_store.kh_encryption().unwrap(); + let authentication_key = key_store.kh_authentication().unwrap(); + assert_eq!(key_store.kh_encryption(), Ok(encryption_key)); + assert_eq!(key_store.kh_authentication(), Ok(authentication_key)); + + // ECDSA seeds are well-defined and stable. + let ecdsa_seed = key_store.generate_ecdsa_seed().unwrap(); + let ecdsa_key = key_store.derive_ecdsa(&ecdsa_seed).unwrap(); + assert_eq!(key_store.derive_ecdsa(&ecdsa_seed), Ok(ecdsa_key)); + + // 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.kh_encryption().unwrap() != encryption_key); + assert!(key_store.kh_authentication().unwrap() != authentication_key); +} diff --git a/src/api/mod.rs b/src/api/mod.rs index 8e409f6..27f2cdb 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -20,5 +20,6 @@ pub mod connection; pub mod customization; pub mod firmware_protection; +pub mod key_store; pub mod upgrade_storage; pub mod user_presence; diff --git a/src/ctap/crypto_wrapper.rs b/src/ctap/crypto_wrapper.rs index dc0af0c..6f03da1 100644 --- a/src/ctap/crypto_wrapper.rs +++ b/src/ctap/crypto_wrapper.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::api::key_store::KeyStore; #[cfg(feature = "ed25519")] use crate::ctap::data_formats::EDDSA_ALGORITHM; use crate::ctap::data_formats::{ @@ -19,7 +20,6 @@ use crate::ctap::data_formats::{ PublicKeyCredentialType, SignatureAlgorithm, ES256_ALGORITHM, }; use crate::ctap::status_code::Ctap2StatusCode; -use crate::ctap::storage; use crate::env::Env; use alloc::string::String; use alloc::vec; @@ -245,8 +245,7 @@ pub fn encrypt_key_handle( private_key: &PrivateKey, application: &[u8; 32], ) -> Result, Ctap2StatusCode> { - let master_keys = storage::master_keys(env)?; - let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); let mut plaintext = [0; 64]; let version = match private_key { @@ -265,7 +264,7 @@ pub fn encrypt_key_handle( let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?; encrypted_id.insert(0, version); - let id_hmac = hmac_256::(&master_keys.hmac, &encrypted_id[..]); + let id_hmac = hmac_256::(&env.key_store().kh_authentication()?, &encrypted_id[..]); encrypted_id.extend(&id_hmac); Ok(encrypted_id) } @@ -289,10 +288,9 @@ pub fn decrypt_credential_source( if credential_id.len() < LEGACY_CREDENTIAL_ID_SIZE { return Ok(None); } - let master_keys = storage::master_keys(env)?; let hmac_message_size = credential_id.len() - 32; if !verify_hmac_256::( - &master_keys.hmac, + &env.key_store().kh_authentication()?, &credential_id[..hmac_message_size], array_ref![credential_id, hmac_message_size, 32], ) { @@ -316,7 +314,7 @@ pub fn decrypt_credential_source( return Ok(None); } - let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); let decrypted_id = aes256_cbc_decrypt(&aes_enc_key, payload, true)?; if rp_id_hash != &decrypted_id[32..] { @@ -548,7 +546,6 @@ mod test { fn test_encrypt_decrypt_credential(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), signature_algorithm); let rp_id_hash = [0x55; 32]; @@ -574,7 +571,6 @@ mod test { #[test] fn test_encrypt_decrypt_bad_version() { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); let rp_id_hash = [0x55; 32]; @@ -582,8 +578,8 @@ mod test { encrypted_id[0] = UNSUPPORTED_CREDENTIAL_ID_VERSION; // Override the HMAC to pass the check. encrypted_id.truncate(&encrypted_id.len() - 32); - let master_keys = storage::master_keys(&mut env).unwrap(); - let id_hmac = hmac_256::(&master_keys.hmac, &encrypted_id[..]); + let hmac_key = env.key_store().kh_authentication().unwrap(); + let id_hmac = hmac_256::(&hmac_key, &encrypted_id[..]); encrypted_id.extend(&id_hmac); assert_eq!( @@ -594,7 +590,6 @@ mod test { fn test_encrypt_decrypt_bad_hmac(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), signature_algorithm); let rp_id_hash = [0x55; 32]; @@ -622,7 +617,6 @@ mod test { fn test_decrypt_credential_missing_blocks(signature_algorithm: SignatureAlgorithm) { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), signature_algorithm); let rp_id_hash = [0x55; 32]; @@ -653,14 +647,13 @@ mod test { private_key: crypto::ecdsa::SecKey, application: &[u8; 32], ) -> Result, Ctap2StatusCode> { - let master_keys = storage::master_keys(env)?; - let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption); + let aes_enc_key = crypto::aes256::EncryptionKey::new(&env.key_store().kh_encryption()?); let mut plaintext = [0; 64]; private_key.to_bytes(array_mut_ref!(plaintext, 0, 32)); plaintext[32..64].copy_from_slice(application); let mut encrypted_id = aes256_cbc_encrypt(env.rng(), &aes_enc_key, &plaintext, true)?; - let id_hmac = hmac_256::(&master_keys.hmac, &encrypted_id[..]); + let id_hmac = hmac_256::(&env.key_store().kh_authentication()?, &encrypted_id[..]); encrypted_id.extend(&id_hmac); Ok(encrypted_id) } @@ -668,7 +661,6 @@ mod test { #[test] fn test_encrypt_decrypt_credential_legacy() { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let ecdsa_key = crypto::ecdsa::SecKey::gensk(env.rng()); let private_key = PrivateKey::from(ecdsa_key.clone()); @@ -684,7 +676,6 @@ mod test { #[test] fn test_encrypt_credential_size() { let mut env = TestEnv::new(); - storage::init(&mut env).ok().unwrap(); let private_key = PrivateKey::new(env.rng(), SignatureAlgorithm::ES256); let rp_id_hash = [0x55; 32]; diff --git a/src/ctap/status_code.rs b/src/ctap/status_code.rs index 8d642d5..c6c93c0 100644 --- a/src/ctap/status_code.rs +++ b/src/ctap/status_code.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::api::key_store; use crate::api::user_presence::UserPresenceError; // CTAP specification (version 20190130) section 6.3 @@ -93,3 +94,9 @@ impl From for Ctap2StatusCode { } } } + +impl From for Ctap2StatusCode { + fn from(_: key_store::Error) -> Self { + Self::CTAP2_ERR_VENDOR_INTERNAL_ERROR + } +} diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index d3b3efc..916d5e6 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -15,6 +15,7 @@ mod key; use crate::api::customization::Customization; +use crate::api::key_store::KeyStore; use crate::ctap::client_pin::PIN_AUTH_LENGTH; use crate::ctap::data_formats::{ extract_array, extract_text_string, CredentialProtectionPolicy, PublicKeyCredentialSource, @@ -33,15 +34,6 @@ use persistent_store::{fragment, StoreUpdate}; use rng256::Rng256; use sk_cbor::cbor_array_vec; -/// Wrapper for master keys. -pub struct MasterKeys { - /// Master encryption key. - pub encryption: [u8; 32], - - /// Master hmac key. - pub hmac: [u8; 32], -} - /// Wrapper for PIN properties. struct PinProperties { /// 16 byte prefix of SHA256 of the currently set PIN. @@ -53,16 +45,6 @@ struct PinProperties { /// Initializes the store by creating missing objects. pub fn init(env: &mut impl Env) -> Result<(), Ctap2StatusCode> { - // Generate and store the master keys if they are missing. - if env.store().find_handle(key::MASTER_KEYS)?.is_none() { - let master_encryption_key = env.rng().gen_uniform_u8x32(); - let master_hmac_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_hmac_key); - env.store().insert(key::MASTER_KEYS, &master_keys)?; - } - // Generate and store the CredRandom secrets if they are missing. if env.store().find_handle(key::CRED_RANDOM_SECRET)?.is_none() { let cred_random_with_uv = env.rng().gen_uniform_u8x32(); @@ -280,21 +262,6 @@ pub fn incr_global_signature_counter( Ok(()) } -/// Returns the master keys. -pub fn master_keys(env: &mut impl Env) -> Result { - let master_keys = env - .store() - .find(key::MASTER_KEYS)? - .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?; - if master_keys.len() != 64 { - return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); - } - Ok(MasterKeys { - encryption: *array_ref![master_keys, 0, 32], - hmac: *array_ref![master_keys, 32, 32], - }) -} - /// Returns the CredRandom secret. pub fn cred_random_secret(env: &mut impl Env, has_uv: bool) -> Result<[u8; 32], Ctap2StatusCode> { let cred_random_secret = env @@ -549,6 +516,7 @@ pub fn set_aaguid( /// In particular persistent entries are not reset. pub fn reset(env: &mut impl Env) -> Result<(), Ctap2StatusCode> { env.store().clear(key::NUM_PERSISTENT_KEYS)?; + env.key_store().reset()?; init(env)?; Ok(()) } @@ -981,27 +949,6 @@ mod test { assert_eq!(no_credential, None); } - #[test] - fn test_master_keys() { - let mut env = TestEnv::new(); - init(&mut env).unwrap(); - - // Master keys stay the same within the same CTAP reset cycle. - let master_keys_1 = master_keys(&mut env).unwrap(); - let master_keys_2 = master_keys(&mut env).unwrap(); - assert_eq!(master_keys_2.encryption, master_keys_1.encryption); - assert_eq!(master_keys_2.hmac, master_keys_1.hmac); - - // Master keys change after reset. This test may fail if the random generator produces the - // same keys. - let master_encryption_key = master_keys_1.encryption.to_vec(); - let master_hmac_key = master_keys_1.hmac.to_vec(); - reset(&mut env).unwrap(); - let master_keys_3 = master_keys(&mut env).unwrap(); - assert!(master_keys_3.encryption != master_encryption_key.as_slice()); - assert!(master_keys_3.hmac != master_hmac_key.as_slice()); - } - #[test] fn test_cred_random_secret() { let mut env = TestEnv::new(); diff --git a/src/ctap/storage/key.rs b/src/ctap/storage/key.rs index ee9a860..45fef2d 100644 --- a/src/ctap/storage/key.rs +++ b/src/ctap/storage/key.rs @@ -125,10 +125,8 @@ make_partition! { /// the length, the following are an array with the hash. PIN_PROPERTIES = 2045; - /// The encryption and hmac keys. - /// - /// This entry is always present. It is generated at startup if absent. - MASTER_KEYS = 2046; + /// Reserved for the key store implementation of the environment. + _RESERVED_KEY_STORE = 2046; /// The global signature counter. /// diff --git a/src/env/mod.rs b/src/env/mod.rs index 01b7f3a..08bb9f3 100644 --- a/src/env/mod.rs +++ b/src/env/mod.rs @@ -15,6 +15,7 @@ use crate::api::connection::HidConnection; use crate::api::customization::Customization; use crate::api::firmware_protection::FirmwareProtection; +use crate::api::key_store::KeyStore; use crate::api::upgrade_storage::UpgradeStorage; use crate::api::user_presence::UserPresence; use persistent_store::{Storage, Store}; @@ -29,6 +30,7 @@ pub trait Env { type Rng: Rng256; type UserPresence: UserPresence; type Storage: Storage; + type KeyStore: KeyStore; type UpgradeStorage: UpgradeStorage; type FirmwareProtection: FirmwareProtection; type Write: core::fmt::Write; @@ -38,6 +40,7 @@ pub trait Env { fn rng(&mut self) -> &mut Self::Rng; fn user_presence(&mut self) -> &mut Self::UserPresence; fn store(&mut self) -> &mut Store; + fn key_store(&mut self) -> &mut Self::KeyStore; /// Returns the upgrade storage instance. /// diff --git a/src/env/test/mod.rs b/src/env/test/mod.rs index d936577..ea5f2b0 100644 --- a/src/env/test/mod.rs +++ b/src/env/test/mod.rs @@ -151,6 +151,7 @@ impl Env for TestEnv { type Rng = TestRng256; type UserPresence = TestUserPresence; type Storage = BufferStorage; + type KeyStore = Self; type UpgradeStorage = BufferUpgradeStorage; type FirmwareProtection = Self; type Write = TestWrite; @@ -169,6 +170,10 @@ impl Env for TestEnv { &mut self.store } + fn key_store(&mut self) -> &mut Self { + self + } + fn upgrade_storage(&mut self) -> Option<&mut Self::UpgradeStorage> { self.upgrade_storage.as_mut() } diff --git a/src/env/tock/mod.rs b/src/env/tock/mod.rs index 9d124ca..e76088f 100644 --- a/src/env/tock/mod.rs +++ b/src/env/tock/mod.rs @@ -197,6 +197,7 @@ impl Env for TockEnv { type Rng = TockRng256; type UserPresence = Self; type Storage = TockStorage; + type KeyStore = Self; type UpgradeStorage = TockUpgradeStorage; type FirmwareProtection = Self; type Write = Console; @@ -215,6 +216,10 @@ impl Env for TockEnv { &mut self.store } + fn key_store(&mut self) -> &mut Self { + self + } + fn upgrade_storage(&mut self) -> Option<&mut Self::UpgradeStorage> { self.upgrade_storage.as_mut() }