From baa9c22582614943409c763f8284705250e64748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Thu, 21 Dec 2023 15:43:53 +0100 Subject: [PATCH 1/6] Fix formatting of documentation --- src/backend/data.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/data.rs b/src/backend/data.rs index 5e79feb..0dad80e 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -35,7 +35,7 @@ pub(crate) type Key = ByteArray; /// /// ```rust,compile_fail /// // length depends on hardware -/// let device_ikm: [u8]= values_from_hardware(); +/// let device_ikm: [u8] = values_from_hardware(); /// /// // generated on first power-up, stays constant for the lifetime of the device /// let device_salt: [u8;32] = csprng(); @@ -105,7 +105,7 @@ pub(crate) type Key = ByteArray; /// /// to_presistent_storage(salt, wrapped_key); /// } -/// ```` +/// ``` #[derive(Debug, Deserialize, Serialize)] struct WrappedKeyData { wrapped_key: Key, From f21406ecb50475182deeedb1739ffa71f814c430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Thu, 21 Dec 2023 16:12:05 +0100 Subject: [PATCH 2/6] Add tests for current serialization of PIN data This will ensure that deserialization still works --- Cargo.toml | 2 + src/backend/data.rs | 83 ++++++++++++++++++++++++++++++++++++++++ src/extension/request.rs | 8 +++- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 546e046..0fd63df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,9 @@ trussed = { version = "0.1.0", features = ["serde-extensions"] } [dev-dependencies] quickcheck = { version = "1.0.3", default-features = false } rand_core = { version = "0.6.4", default-features = false, features = ["getrandom"] } +serde_test = "1.0.176" trussed = { version = "0.1.0", features = ["serde-extensions", "virt"] } +serde_cbor = { version = "0.11.2", features = ["std"] } [patch.crates-io] littlefs2 = { git = "https://github.com/Nitrokey/littlefs2", tag = "v0.3.2-nitrokey-2" } diff --git a/src/backend/data.rs b/src/backend/data.rs index 0dad80e..dd8110b 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -106,18 +106,21 @@ pub(crate) type Key = ByteArray; /// to_presistent_storage(salt, wrapped_key); /// } /// ``` +#[cfg_attr(test, derive(PartialEq))] #[derive(Debug, Deserialize, Serialize)] struct WrappedKeyData { wrapped_key: Key, tag: ChaChaTag, } +#[cfg_attr(test, derive(PartialEq))] #[derive(Debug, Deserialize, Serialize)] enum KeyOrHash { Key(WrappedKeyData), Hash(Hash), } +#[cfg_attr(test, derive(PartialEq))] #[derive(Debug, Deserialize, Serialize)] pub(crate) struct PinData { #[serde(skip)] @@ -438,6 +441,7 @@ impl Deref for PinDataMut<'_> { } } +#[cfg_attr(test, derive(PartialEq))] #[derive(Clone, Copy, Debug, Deserialize, Serialize)] struct Retries { max: u8, @@ -580,4 +584,83 @@ mod tests { let serialized = trussed::cbor_serialize_bytes::<_, 1024>(&salt).unwrap(); assert!(serialized.len() <= SALT_LEN + 1, "{}", serialized.len()); } + + #[test] + fn data_serialization() { + use serde_test::{assert_tokens, Token}; + + let data = PinData { + id: PinId::from(0), + retries: Some(Retries { max: 3, left: 1 }), + salt: [0xFE; SALT_LEN].into(), + data: KeyOrHash::Hash([0xFD; HASH_LEN].into()), + }; + + assert_tokens( + &data, + &[ + Token::Struct { + name: "PinData", + // Id is skipped + len: 3, + }, + Token::Str("retries"), + Token::Some, + Token::Struct { + name: "Retries", + len: 2, + }, + Token::Str("max"), + Token::U8(3), + Token::Str("left"), + Token::U8(1), + Token::StructEnd, + Token::Str("salt"), + Token::Bytes(&[0xFE; SALT_LEN]), + Token::Str("data"), + Token::Enum { name: "KeyOrHash" }, + Token::Str("Hash"), + Token::Bytes(&[0xFD; HASH_LEN]), + Token::StructEnd, + ], + ); + + let data = PinData { + id: PinId::from(0), + retries: None, + salt: [0xFE; SALT_LEN].into(), + data: KeyOrHash::Key(WrappedKeyData { + wrapped_key: [0xFC; KEY_LEN].into(), + tag: [0xFB; CHACHA_TAG_LEN].into(), + }), + }; + + assert_tokens( + &data, + &[ + Token::Struct { + name: "PinData", + // Id is skipped + len: 3, + }, + Token::Str("retries"), + Token::None, + Token::Str("salt"), + Token::Bytes(&[0xFE; SALT_LEN]), + Token::Str("data"), + Token::Enum { name: "KeyOrHash" }, + Token::Str("Key"), + Token::Struct { + name: "WrappedKeyData", + len: 2, + }, + Token::Str("wrapped_key"), + Token::Bytes(&[0xFC; KEY_LEN]), + Token::Str("tag"), + Token::Bytes(&[0xFB; CHACHA_TAG_LEN]), + Token::StructEnd, + Token::StructEnd, + ], + ); + } } diff --git a/src/extension/request.rs b/src/extension/request.rs index e46d2b7..12868f1 100644 --- a/src/extension/request.rs +++ b/src/extension/request.rs @@ -53,12 +53,18 @@ impl From for AuthRequest { } } +#[derive(Debug, Deserialize, Serialize)] +pub enum DerivedKeyMechanism { + Chacha8Poly1305, + X25519, +} + #[derive(Debug, Deserialize, Serialize)] pub struct SetPin { pub id: PinId, pub pin: Pin, pub retries: Option, - /// If true, the PIN can be used to wrap/unwrap a PIN key + /// If `Some`, the PIN can be used to wrap/unwrap a PIN key pub derive_key: bool, } From e3e02eebaf0376acce0f826be74b98668863f957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Thu, 21 Dec 2023 17:34:06 +0100 Subject: [PATCH 3/6] Change key (de) serialization to allow using X25519 keys --- src/backend.rs | 18 +++---- src/backend/data.rs | 118 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 104 insertions(+), 32 deletions(-) diff --git a/src/backend.rs b/src/backend.rs index f60f446..71384f6 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -25,9 +25,9 @@ use crate::{ extension::{reply, AuthExtension, AuthReply, AuthRequest}, BACKEND_DIR, }; -use data::{Key, PinData, Salt, KEY_LEN, SALT_LEN}; +use data::{PinData, Salt, CHACHA_KEY_LEN, SALT_LEN}; -use self::data::delete_app_salt; +use self::data::{delete_app_salt, ChachaKey}; /// max accepted length for the hardware initial key material pub const MAX_HW_KEY_LEN: usize = 64; @@ -149,8 +149,8 @@ impl AuthBackend { } } - fn expand(kdf: &Hkdf, client_id: &PathBuf) -> Key { - let mut out = Key::default(); + fn expand(kdf: &Hkdf, client_id: &PathBuf) -> ChachaKey { + let mut out = ChachaKey::default(); #[allow(clippy::expect_used)] kdf.expand(client_id.as_ref().as_bytes(), &mut *out) .expect("Out data is always valid"); @@ -162,7 +162,7 @@ impl AuthBackend { client_id: PathBuf, global_fs: &mut impl Filestore, rng: &mut R, - ) -> Result { + ) -> Result { Ok(match &self.hw_key { HardwareKey::Extracted(okm) => Self::expand(okm, &client_id), HardwareKey::Missing => return Err(Error::MissingHwKey), @@ -183,7 +183,7 @@ impl AuthBackend { global_fs: &mut impl Filestore, ctx: &mut AuthContext, rng: &mut R, - ) -> Result { + ) -> Result { if let Some(app_key) = ctx.application_key { return Ok(app_key); } @@ -197,7 +197,7 @@ impl AuthBackend { /// Per-client context for [`AuthBackend`][] #[derive(Default, Debug)] pub struct AuthContext { - application_key: Option, + application_key: Option, } impl Backend for AuthBackend { @@ -249,7 +249,7 @@ impl ExtensionImpl for AuthBackend { let key_id = keystore.store_key( Location::Volatile, Secrecy::Secret, - Kind::Symmetric(KEY_LEN), + Kind::Symmetric(CHACHA_KEY_LEN), &*k, )?; Ok(reply::GetPinKey { @@ -347,7 +347,7 @@ impl ExtensionImpl for AuthBackend { let key_id = keystore.store_key( Location::Volatile, Secrecy::Secret, - Kind::Symmetric(KEY_LEN), + Kind::Symmetric(CHACHA_KEY_LEN), &*key, )?; Ok(reply::GetApplicationKey { key: key_id }.into()) diff --git a/src/backend/data.rs b/src/backend/data.rs index dd8110b..bcd290f 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -23,12 +23,70 @@ pub(crate) const SIZE: usize = 256; pub(crate) const CHACHA_TAG_LEN: usize = 16; pub(crate) const SALT_LEN: usize = 16; pub(crate) const HASH_LEN: usize = 32; -pub(crate) const KEY_LEN: usize = 32; +pub(crate) const CHACHA_KEY_LEN: usize = 32; +pub(crate) const X25519_KEY_LEN: usize = 32; +const ENCODED_X25519_KEY_LEN: usize = X25519_KEY_LEN + 1; pub(crate) type Salt = ByteArray; pub(crate) type Hash = ByteArray; pub(crate) type ChaChaTag = ByteArray; -pub(crate) type Key = ByteArray; +pub(crate) type ChachaKey = ByteArray; +pub(crate) type X25519Key = ByteArray; + +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum Key { + Chacha20Poly1305(ChachaKey), + X25519(X25519Key), +} + +impl Serialize for Key { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match self { + Key::Chacha20Poly1305(k) => serializer.serialize_bytes(&**k), + Key::X25519(k) => { + let mut encoded = [0; ENCODED_X25519_KEY_LEN]; + encoded[0..X25519_KEY_LEN].copy_from_slice(&**k); + serializer.serialize_bytes(&encoded) + } + } + } +} + +impl<'de> Deserialize<'de> for Key { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + use serde::de::Visitor; + + struct KeyVisitor; + impl<'de> Visitor<'de> for KeyVisitor { + type Value = Key; + fn expecting(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result { + formatter.write_str("A byte array of length 32 or 33") + } + fn visit_bytes(self, v: &[u8]) -> Result + where + E: serde::de::Error, + { + match v.len() { + CHACHA_KEY_LEN => Ok(Key::Chacha20Poly1305(ByteArray::new( + v.try_into().expect("Len was just checked"), + ))), + ENCODED_X25519_KEY_LEN => Ok(Key::X25519(ByteArray::new( + v.try_into().expect("Len was just checked"), + ))), + _ => Err(E::invalid_length(v.len(), &self)), + } + } + } + + deserializer.deserialize_bytes(KeyVisitor) + } +} /// Represent a key wrapped by the pin. /// The key derivation process is as follow (pseudocode): @@ -138,7 +196,7 @@ impl PinData { pin: &Pin, retries: Option, rng: &mut R, - application_key: Option<&Key>, + application_key: Option<&ChachaKey>, ) -> Self where R: CryptoRng + RngCore, @@ -148,7 +206,7 @@ impl PinData { let data = application_key .map(|k| { use chacha20poly1305::{AeadInPlace, KeyInit}; - let mut key = Key::default(); + let mut key = ChachaKey::default(); rng.fill_bytes(&mut *key); let pin_key = derive_key(id, pin, &salt, k); let aead = ChaCha8Poly1305::new((&*pin_key).into()); @@ -162,7 +220,7 @@ impl PinData { .into(); KeyOrHash::Key(WrappedKeyData { - wrapped_key: key, + wrapped_key: Key::Chacha20Poly1305(key), tag: tag.into(), }) }) @@ -180,8 +238,8 @@ impl PinData { pin: &Pin, retries: Option, rng: &mut R, - application_key: &Key, - mut key_to_wrap: Key, + application_key: &ChachaKey, + mut key_to_wrap: ChachaKey, ) -> Self where R: CryptoRng + RngCore, @@ -202,7 +260,7 @@ impl PinData { retries: retries.map(From::from), salt, data: KeyOrHash::Key(WrappedKeyData { - wrapped_key: key_to_wrap, + wrapped_key: Key::Chacha20Poly1305(key_to_wrap), tag: tag.into(), }), } @@ -262,7 +320,7 @@ pub(crate) struct PinDataMut<'a> { enum CheckResult { Validated, - Derived { k: Key, app_key: Key }, + Derived { k: ChachaKey, app_key: ChachaKey }, Failed, } @@ -283,7 +341,7 @@ impl<'a> PinDataMut<'a> { fn check_or_unwrap( &mut self, pin: &Pin, - application_key: impl FnOnce() -> Result, + application_key: impl FnOnce() -> Result, ) -> Result { if self.is_blocked() { return Ok(CheckResult::Failed); @@ -296,7 +354,10 @@ impl<'a> PinDataMut<'a> { CheckResult::Failed } } - KeyOrHash::Key(WrappedKeyData { wrapped_key, tag }) => { + KeyOrHash::Key(WrappedKeyData { + wrapped_key: Key::Chacha20Poly1305(wrapped_key), + tag, + }) => { let app_key = application_key()?; if let Some(k) = self.unwrap_key(pin, &app_key, wrapped_key, &tag) { CheckResult::Derived { k, app_key } @@ -304,6 +365,13 @@ impl<'a> PinDataMut<'a> { CheckResult::Failed } } + KeyOrHash::Key(WrappedKeyData { + wrapped_key: Key::X25519(_x25519), + tag, + }) => { + let _ = tag; + todo!() + } }; if let Some(retries) = &mut self.data.retries { if res.is_success() { @@ -321,7 +389,7 @@ impl<'a> PinDataMut<'a> { pub fn check_pin( &mut self, pin: &Pin, - application_key: impl FnOnce() -> Result, + application_key: impl FnOnce() -> Result, ) -> Result { self.check_or_unwrap(pin, application_key) .map(|res| res.is_success()) @@ -331,10 +399,10 @@ impl<'a> PinDataMut<'a> { fn unwrap_key( &self, pin: &Pin, - application_key: &Key, - mut wrapped_key: Key, + application_key: &ChachaKey, + mut wrapped_key: ChachaKey, tag: &ChaChaTag, - ) -> Option { + ) -> Option { use chacha20poly1305::{AeadInPlace, KeyInit}; let pin_key = derive_key(self.id, pin, &self.salt, application_key); @@ -352,7 +420,11 @@ impl<'a> PinDataMut<'a> { .and(Some(wrapped_key)) } - pub fn get_pin_key(&mut self, pin: &Pin, application_key: &Key) -> Result, Error> { + pub fn get_pin_key( + &mut self, + pin: &Pin, + application_key: &ChachaKey, + ) -> Result, Error> { match self.check_or_unwrap(pin, || Ok(*application_key))? { CheckResult::Validated => Err(Error::BadPinType), CheckResult::Derived { k, .. } => Ok(Some(k)), @@ -378,8 +450,8 @@ impl<'a> PinDataMut<'a> { fn new_wrapping_pin( &mut self, new: &Pin, - mut old_key: Key, - application_key: &Key, + mut old_key: ChachaKey, + application_key: &ChachaKey, rng: &mut R, ) { use chacha20poly1305::{AeadInPlace, KeyInit}; @@ -404,7 +476,7 @@ impl<'a> PinDataMut<'a> { retries: self.retries, salt, data: KeyOrHash::Key(WrappedKeyData { - wrapped_key: old_key, + wrapped_key: Key::Chacha20Poly1305(old_key), tag: tag.into(), }), }; @@ -414,7 +486,7 @@ impl<'a> PinDataMut<'a> { &mut self, old_pin: &Pin, new_pin: &Pin, - application_key: impl FnOnce(&mut R) -> Result, + application_key: impl FnOnce(&mut R) -> Result, rng: &mut R, ) -> Result { match self.check_or_unwrap(old_pin, || application_key(rng))? { @@ -545,7 +617,7 @@ fn load_app_salt(fs: &mut S, location: Location) -> Result| (**b).try_into().map_err(|_| Error::ReadFailed)) } -pub fn expand_app_key(salt: &Salt, application_key: &Key, info: &[u8]) -> Key { +pub fn expand_app_key(salt: &Salt, application_key: &ChachaKey, info: &[u8]) -> ChachaKey { #[allow(clippy::expect_used)] let mut hmac = Hmac::::new_from_slice(&**application_key) .expect("Slice will always be of acceptable size"); @@ -630,7 +702,7 @@ mod tests { retries: None, salt: [0xFE; SALT_LEN].into(), data: KeyOrHash::Key(WrappedKeyData { - wrapped_key: [0xFC; KEY_LEN].into(), + wrapped_key: Key::Chacha20Poly1305([0xFC; CHACHA_KEY_LEN].into()), tag: [0xFB; CHACHA_TAG_LEN].into(), }), }; @@ -655,7 +727,7 @@ mod tests { len: 2, }, Token::Str("wrapped_key"), - Token::Bytes(&[0xFC; KEY_LEN]), + Token::Bytes(&[0xFC; CHACHA_KEY_LEN]), Token::Str("tag"), Token::Bytes(&[0xFB; CHACHA_TAG_LEN]), Token::StructEnd, From f2bee120b40862f323bd2611a4eaa2de01061a4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 22 Dec 2023 12:00:04 +0100 Subject: [PATCH 4/6] Implement X25519 key generation --- Cargo.toml | 1 + src/backend.rs | 14 ++--- src/backend/data.rs | 108 +++++++++++++++++++++++++++++++-------- src/extension.rs | 4 +- src/extension/request.rs | 4 +- 5 files changed, 100 insertions(+), 31 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0fd63df..dc5e556 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ chacha20poly1305 = { version = "0.10.1", default-features = false, features = [" hkdf = "0.12.3" hmac = "0.12.1" rand_core = "0.6.4" +salty = { version = "0.3.0", default-features = false } serde = { version = "1", default-features = false } serde-byte-array = "0.1.2" sha2 = { version = "0.10.6", default-features = false } diff --git a/src/backend.rs b/src/backend.rs index 71384f6..d59d1a3 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -25,7 +25,7 @@ use crate::{ extension::{reply, AuthExtension, AuthReply, AuthRequest}, BACKEND_DIR, }; -use data::{PinData, Salt, CHACHA_KEY_LEN, SALT_LEN}; +use data::{DeriveKey, PinData, Salt, CHACHA_KEY_LEN, SALT_LEN}; use self::data::{delete_app_salt, ChachaKey}; @@ -276,17 +276,19 @@ impl ExtensionImpl for AuthBackend { Ok(reply::ChangePin { success }.into()) } AuthRequest::SetPin(request) => { - let maybe_app_key = if request.derive_key { - Some(self.get_app_key(client_id, global_fs, ctx, rng)?) - } else { - None + let key_derivation = match request.derive_key { + Some(key_type) => Some(DeriveKey { + application_key: self.get_app_key(client_id, global_fs, ctx, rng)?, + key_type, + }), + None => None, }; PinData::new( request.id, &request.pin, request.retries, rng, - maybe_app_key.as_ref(), + key_derivation, ) .save(fs, self.location)?; Ok(reply::SetPin.into()) diff --git a/src/backend/data.rs b/src/backend/data.rs index bcd290f..9560dda 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -17,7 +17,7 @@ use trussed::{ }; use super::Error; -use crate::{Pin, PinId, MAX_PIN_LENGTH}; +use crate::{request::DerivedKeyMechanism, Pin, PinId, MAX_PIN_LENGTH}; pub(crate) const SIZE: usize = 256; pub(crate) const CHACHA_TAG_LEN: usize = 16; @@ -31,7 +31,7 @@ pub(crate) type Salt = ByteArray; pub(crate) type Hash = ByteArray; pub(crate) type ChaChaTag = ByteArray; pub(crate) type ChachaKey = ByteArray; -pub(crate) type X25519Key = ByteArray; +pub(crate) type X25519Key = [u8; X25519_KEY_LEN]; #[derive(Debug, PartialEq, Eq)] pub(crate) enum Key { @@ -48,7 +48,7 @@ impl Serialize for Key { Key::Chacha20Poly1305(k) => serializer.serialize_bytes(&**k), Key::X25519(k) => { let mut encoded = [0; ENCODED_X25519_KEY_LEN]; - encoded[0..X25519_KEY_LEN].copy_from_slice(&**k); + encoded[0..X25519_KEY_LEN].copy_from_slice(&*k); serializer.serialize_bytes(&encoded) } } @@ -76,9 +76,11 @@ impl<'de> Deserialize<'de> for Key { CHACHA_KEY_LEN => Ok(Key::Chacha20Poly1305(ByteArray::new( v.try_into().expect("Len was just checked"), ))), - ENCODED_X25519_KEY_LEN => Ok(Key::X25519(ByteArray::new( - v.try_into().expect("Len was just checked"), - ))), + ENCODED_X25519_KEY_LEN if v[X25519_KEY_LEN] == 0 => Ok(Key::X25519( + v[..X25519_KEY_LEN] + .try_into() + .expect("Len was just checked"), + )), _ => Err(E::invalid_length(v.len(), &self)), } } @@ -188,6 +190,12 @@ pub(crate) struct PinData { data: KeyOrHash, } +/// Information required to derive a key +pub(crate) struct DeriveKey { + pub(crate) application_key: ChachaKey, + pub(crate) key_type: DerivedKeyMechanism, +} + impl PinData { /// An application_key of `None` means that the pin should only be salted/hashed /// `Some` means that it should instead be used to wrap a 32 bytes encryption key @@ -196,35 +204,55 @@ impl PinData { pin: &Pin, retries: Option, rng: &mut R, - application_key: Option<&ChachaKey>, + derive_parameter: Option, ) -> Self where R: CryptoRng + RngCore, { let mut salt = Salt::default(); rng.fill_bytes(salt.as_mut()); - let data = application_key - .map(|k| { - use chacha20poly1305::{AeadInPlace, KeyInit}; + let data = match derive_parameter { + None => KeyOrHash::Hash(hash(id, pin, &salt)), + Some(DeriveKey { + application_key, + key_type: DerivedKeyMechanism::Chacha8Poly1305, + }) => { let mut key = ChachaKey::default(); rng.fill_bytes(&mut *key); - let pin_key = derive_key(id, pin, &salt, k); - let aead = ChaCha8Poly1305::new((&*pin_key).into()); - // The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern - // Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys - let nonce = Default::default(); - #[allow(clippy::expect_used)] - let tag: [u8; CHACHA_TAG_LEN] = aead - .encrypt_in_place_detached(&nonce, &[u8::from(id)], &mut *key) - .expect("Wrapping the key should always work, length are acceptable") - .into(); + let tag = encrypt_pin_data(id, pin, &salt, &application_key, &mut *key, None); KeyOrHash::Key(WrappedKeyData { wrapped_key: Key::Chacha20Poly1305(key), tag: tag.into(), }) - }) - .unwrap_or_else(|| KeyOrHash::Hash(hash(id, pin, &salt))); + } + Some(DeriveKey { + application_key, + key_type: DerivedKeyMechanism::X25519, + }) => { + use salty::agreement::SecretKey; + let mut seed: [u8; 32] = Default::default(); + rng.fill_bytes(&mut seed); + let key = SecretKey::from_seed(&seed); + + let mut key_bytes = key.to_bytes(); + + // X25519 keys have a dedicated AAD to avoid key confusion + let tag = encrypt_pin_data( + id, + pin, + &salt, + &application_key, + &mut key_bytes, + Some([0x00]), + ); + + KeyOrHash::Key(WrappedKeyData { + wrapped_key: Key::X25519(key_bytes), + tag: tag.into(), + }) + } + }; Self { id, retries: retries.map(From::from), @@ -565,6 +593,42 @@ fn derive_key(id: PinId, pin: &Pin, salt: &Salt, application_key: &[u8; 32]) -> tmp.into() } +fn encrypt_pin_data( + id: PinId, + pin: &Pin, + salt: &Salt, + application_key: &[u8; 32], + data: &mut [u8], + aad: Option<[u8; 1]>, +) -> [u8; CHACHA_TAG_LEN] { + use chacha20poly1305::{AeadInPlace, KeyInit}; + let pin_key = derive_key(id, pin, &salt, application_key); + let aead = ChaCha8Poly1305::new((&*pin_key).into()); + // The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern + // Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys + let nonce = Default::default(); + + let sup_1; + let sup_2; + + let aad: &[u8] = match aad { + Some([aad_byte]) => { + sup_1 = [u8::from(id), aad_byte]; + &sup_1 + } + None => { + sup_2 = [u8::from(id)]; + &sup_2 + } + }; + #[allow(clippy::expect_used)] + let tag: [u8; CHACHA_TAG_LEN] = aead + .encrypt_in_place_detached(&nonce, aad, &mut *data) + .expect("Wrapping the key should always work, length are acceptable") + .into(); + tag +} + fn pin_len(pin: &Pin) -> u8 { const _: () = assert!(MAX_PIN_LENGTH <= u8::MAX as usize); pin.len() as u8 diff --git a/src/extension.rs b/src/extension.rs index 416a443..7321f4b 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -14,6 +14,8 @@ use trussed::{ use crate::{Pin, PinId}; +use self::request::DerivedKeyMechanism; + /// A result returned by [`AuthClient`][]. pub type AuthResult<'a, R, C> = ExtensionResult<'a, AuthExtension, R, C>; @@ -116,7 +118,7 @@ pub trait AuthClient: ExtensionClient { id: I, pin: Pin, retries: Option, - derive_key: bool, + derive_key: Option, ) -> AuthResult<'_, reply::SetPin, Self> { self.extension(request::SetPin { id: id.into(), diff --git a/src/extension/request.rs b/src/extension/request.rs index 12868f1..d22779f 100644 --- a/src/extension/request.rs +++ b/src/extension/request.rs @@ -53,7 +53,7 @@ impl From for AuthRequest { } } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, Copy, Clone)] pub enum DerivedKeyMechanism { Chacha8Poly1305, X25519, @@ -65,7 +65,7 @@ pub struct SetPin { pub pin: Pin, pub retries: Option, /// If `Some`, the PIN can be used to wrap/unwrap a PIN key - pub derive_key: bool, + pub derive_key: Option, } impl From for AuthRequest { From 4918709ce5f373ad9b1ab61b59a2d6f8e8232acc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 22 Dec 2023 17:00:05 +0100 Subject: [PATCH 5/6] Implement deriving X25519 keys from pins --- src/backend.rs | 4 +- src/backend/data.rs | 113 +++++++++++++++++++++++++++++++------------- tests/backend.rs | 73 ++++++++++++++++++++-------- 3 files changed, 133 insertions(+), 57 deletions(-) diff --git a/src/backend.rs b/src/backend.rs index d59d1a3..483d068 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -249,8 +249,8 @@ impl ExtensionImpl for AuthBackend { let key_id = keystore.store_key( Location::Volatile, Secrecy::Secret, - Kind::Symmetric(CHACHA_KEY_LEN), - &*k, + k.kind(), + &k.data(), )?; Ok(reply::GetPinKey { result: Some(key_id), diff --git a/src/backend/data.rs b/src/backend/data.rs index 9560dda..b8864eb 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -10,6 +10,7 @@ use serde_byte_array::ByteArray; use sha2::{Digest as _, Sha256}; use subtle::ConstantTimeEq as _; use trussed::{ + key, platform::{CryptoRng, RngCore}, store::filestore::Filestore, types::{Location, PathBuf}, @@ -33,7 +34,7 @@ pub(crate) type ChaChaTag = ByteArray; pub(crate) type ChachaKey = ByteArray; pub(crate) type X25519Key = [u8; X25519_KEY_LEN]; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Clone, Copy)] pub(crate) enum Key { Chacha20Poly1305(ChachaKey), X25519(X25519Key), @@ -90,6 +91,27 @@ impl<'de> Deserialize<'de> for Key { } } +pub(crate) enum DecryptedKey { + Chacha20Poly1305(ChachaKey), + X25519(salty::agreement::SecretKey), +} + +impl DecryptedKey { + pub fn kind(&self) -> key::Kind { + match self { + DecryptedKey::Chacha20Poly1305(_) => key::Kind::Symmetric(CHACHA_KEY_LEN), + DecryptedKey::X25519(_) => key::Kind::X255, + } + } + + pub fn data(&self) -> [u8; 32] { + match self { + DecryptedKey::Chacha20Poly1305(k) => (*k).into(), + DecryptedKey::X25519(k) => k.to_bytes(), + } + } +} + /// Represent a key wrapped by the pin. /// The key derivation process is as follow (pseudocode): /// @@ -348,7 +370,7 @@ pub(crate) struct PinDataMut<'a> { enum CheckResult { Validated, - Derived { k: ChachaKey, app_key: ChachaKey }, + Derived { k: DecryptedKey, app_key: ChachaKey }, Failed, } @@ -382,10 +404,7 @@ impl<'a> PinDataMut<'a> { CheckResult::Failed } } - KeyOrHash::Key(WrappedKeyData { - wrapped_key: Key::Chacha20Poly1305(wrapped_key), - tag, - }) => { + KeyOrHash::Key(WrappedKeyData { wrapped_key, tag }) => { let app_key = application_key()?; if let Some(k) = self.unwrap_key(pin, &app_key, wrapped_key, &tag) { CheckResult::Derived { k, app_key } @@ -393,13 +412,6 @@ impl<'a> PinDataMut<'a> { CheckResult::Failed } } - KeyOrHash::Key(WrappedKeyData { - wrapped_key: Key::X25519(_x25519), - tag, - }) => { - let _ = tag; - todo!() - } }; if let Some(retries) = &mut self.data.retries { if res.is_success() { @@ -428,9 +440,9 @@ impl<'a> PinDataMut<'a> { &self, pin: &Pin, application_key: &ChachaKey, - mut wrapped_key: ChachaKey, + mut wrapped_key: Key, tag: &ChaChaTag, - ) -> Option { + ) -> Option { use chacha20poly1305::{AeadInPlace, KeyInit}; let pin_key = derive_key(self.id, pin, &self.salt, application_key); @@ -438,21 +450,35 @@ impl<'a> PinDataMut<'a> { // The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern // Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys let nonce = Default::default(); - aead.decrypt_in_place_detached( - &nonce, - &[u8::from(self.data.id)], - &mut *wrapped_key, - (&**tag).into(), - ) - .ok() - .and(Some(wrapped_key)) + + let aad_1; + let aad_2; + let (aad, mut key_data): (&[u8], _) = match &mut wrapped_key { + Key::Chacha20Poly1305(k) => { + aad_1 = [u8::from(self.data.id)]; + (&aad_1, **k) + } + Key::X25519(k) => { + aad_2 = [u8::from(self.data.id), 0]; + (&aad_2, *k) + } + }; + + aead.decrypt_in_place_detached(&nonce, aad, &mut key_data, (&**tag).into()) + .ok()?; + match wrapped_key { + Key::Chacha20Poly1305(_) => Some(DecryptedKey::Chacha20Poly1305((key_data).into())), + Key::X25519(_) => Some(DecryptedKey::X25519( + salty::agreement::SecretKey::from_seed(&key_data), + )), + } } pub fn get_pin_key( &mut self, pin: &Pin, application_key: &ChachaKey, - ) -> Result, Error> { + ) -> Result, Error> { match self.check_or_unwrap(pin, || Ok(*application_key))? { CheckResult::Validated => Err(Error::BadPinType), CheckResult::Derived { k, .. } => Ok(Some(k)), @@ -478,7 +504,7 @@ impl<'a> PinDataMut<'a> { fn new_wrapping_pin( &mut self, new: &Pin, - mut old_key: ChachaKey, + old_key: DecryptedKey, application_key: &ChachaKey, rng: &mut R, ) { @@ -493,20 +519,39 @@ impl<'a> PinDataMut<'a> { // Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys let nonce = Default::default(); - #[allow(clippy::expect_used)] - let tag: [u8; CHACHA_TAG_LEN] = aead - .encrypt_in_place_detached(&nonce, &[u8::from(self.id)], &mut *old_key) - .expect("Wrapping the key should always work, length are acceptable") - .into(); + let data = match old_key { + DecryptedKey::Chacha20Poly1305(mut k) => { + #[allow(clippy::expect_used)] + let tag: [u8; CHACHA_TAG_LEN] = aead + .encrypt_in_place_detached(&nonce, &[u8::from(self.id)], &mut *k) + .expect("Wrapping the key should always work, length are acceptable") + .into(); + + KeyOrHash::Key(WrappedKeyData { + wrapped_key: Key::Chacha20Poly1305(k), + tag: tag.into(), + }) + } + DecryptedKey::X25519(k) => { + let mut data = k.to_bytes(); + #[allow(clippy::expect_used)] + let tag: [u8; CHACHA_TAG_LEN] = aead + .encrypt_in_place_detached(&nonce, &[u8::from(self.id), 0], &mut data) + .expect("Wrapping the key should always work, length are acceptable") + .into(); + + KeyOrHash::Key(WrappedKeyData { + wrapped_key: Key::X25519(data), + tag: tag.into(), + }) + } + }; *self.data = PinData { id: self.id, retries: self.retries, salt, - data: KeyOrHash::Key(WrappedKeyData { - wrapped_key: Key::Chacha20Poly1305(old_key), - tag: tag.into(), - }), + data, }; } diff --git a/tests/backend.rs b/tests/backend.rs index dff7afd..46ce412 100644 --- a/tests/backend.rs +++ b/tests/backend.rs @@ -128,7 +128,7 @@ use trussed::{ types::{Bytes, Location, Message, PathBuf}, virt::{self, Ram}, }; -use trussed_auth::{AuthClient as _, PinId, MAX_HW_KEY_LEN}; +use trussed_auth::{request::DerivedKeyMechanism, AuthClient as _, PinId, MAX_HW_KEY_LEN}; use dispatch::{Backend, Dispatch, BACKENDS}; @@ -207,7 +207,7 @@ fn basic() { let reply = syscall!(client.has_pin(Pin::User)); assert!(!reply.has_pin); - syscall!(client.set_pin(Pin::User, pin1.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), None, None)); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); @@ -251,7 +251,12 @@ fn basic_wrapped() { let reply = syscall!(client.has_pin(Pin::User)); assert!(!reply.has_pin); - syscall!(client.set_pin(Pin::User, pin1.clone(), None, true)); + syscall!(client.set_pin( + Pin::User, + pin1.clone(), + None, + Some(DerivedKeyMechanism::Chacha8Poly1305) + )); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); @@ -298,7 +303,12 @@ fn hw_key_wrapped() { let reply = syscall!(client.has_pin(Pin::User)); assert!(!reply.has_pin); - syscall!(client.set_pin(Pin::User, pin1.clone(), None, true)); + syscall!(client.set_pin( + Pin::User, + pin1.clone(), + None, + Some(DerivedKeyMechanism::Chacha8Poly1305) + )); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); @@ -343,12 +353,18 @@ fn missing_hw_key() { let reply = syscall!(client.has_pin(Pin::User)); assert!(!reply.has_pin); - assert!(try_syscall!(client.set_pin(Pin::User, pin1.clone(), None, true)).is_err()); + assert!(try_syscall!(client.set_pin( + Pin::User, + pin1.clone(), + None, + Some(DerivedKeyMechanism::Chacha8Poly1305) + )) + .is_err()); let reply = syscall!(client.has_pin(Pin::User)); assert!(!reply.has_pin); - syscall!(client.set_pin(Pin::User, pin1.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), None, None)); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); @@ -392,7 +408,12 @@ fn pin_key() { let pin1 = Bytes::from_slice(b"12345678").unwrap(); let pin2 = Bytes::from_slice(b"123456").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), Some(3), true)); + syscall!(client.set_pin( + Pin::User, + pin1.clone(), + Some(3), + Some(DerivedKeyMechanism::Chacha8Poly1305) + )); assert!(syscall!(client.get_pin_key(Pin::User, pin2.clone())) .result .is_none()); @@ -441,7 +462,12 @@ fn reset_pin_key() { let pin2 = Bytes::from_slice(b"123456").unwrap(); let pin3 = Bytes::from_slice(b"1234567890").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), Some(3), true)); + syscall!(client.set_pin( + Pin::User, + pin1.clone(), + Some(3), + Some(DerivedKeyMechanism::Chacha8Poly1305) + )); assert!(syscall!(client.get_pin_key(Pin::User, pin2.clone())) .result .is_none()); @@ -489,7 +515,12 @@ fn blocked_pin() { let pin1 = Bytes::from_slice(b"12345678").unwrap(); let pin2 = Bytes::from_slice(b"123456").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), Some(3), false)); + syscall!(client.set_pin( + Pin::User, + pin1.clone(), + Some(3), + Some(DerivedKeyMechanism::Chacha8Poly1305) + )); let reply = syscall!(client.check_pin(Pin::User, pin1.clone())); assert!(reply.success); @@ -510,7 +541,7 @@ fn set_blocked_pin() { let pin1 = Bytes::from_slice(b"12345678").unwrap(); let pin2 = Bytes::from_slice(b"123456").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), Some(1), false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), Some(1), None)); let reply = syscall!(client.check_pin(Pin::User, pin1.clone())); assert!(reply.success); let reply = syscall!(client.check_pin(Pin::User, pin2.clone())); @@ -518,7 +549,7 @@ fn set_blocked_pin() { let reply = syscall!(client.check_pin(Pin::User, pin1)); assert!(!reply.success); - syscall!(client.set_pin(Pin::User, pin2.clone(), Some(1), false)); + syscall!(client.set_pin(Pin::User, pin2.clone(), Some(1), None)); let reply = syscall!(client.check_pin(Pin::User, pin2)); assert!(reply.success); }) @@ -530,7 +561,7 @@ fn empty_pin() { let pin1 = Bytes::new(); let pin2 = Bytes::from_slice(b"123456").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), None, None)); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); let reply = syscall!(client.check_pin(Pin::User, pin1.clone())); @@ -553,7 +584,7 @@ fn max_pin_length() { } }; - syscall!(client.set_pin(Pin::User, pin1.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), None, None)); let reply = syscall!(client.check_pin(Pin::User, pin1)); assert!(reply.success); let reply = syscall!(client.check_pin(Pin::User, pin2)); @@ -568,9 +599,9 @@ fn pin_retries() { let pin2 = Bytes::from_slice(b"123456").unwrap(); let pin3 = Bytes::from_slice(b"654321").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), Some(3), false)); - syscall!(client.set_pin(Pin::Admin, pin2.clone(), Some(5), false)); - syscall!(client.set_pin(Pin::Custom, pin3.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), Some(3), None)); + syscall!(client.set_pin(Pin::Admin, pin2.clone(), Some(5), None)); + syscall!(client.set_pin(Pin::Custom, pin3.clone(), None, None)); let reply = syscall!(client.pin_retries(Pin::User)); assert_eq!(reply.retries, Some(3)); @@ -616,7 +647,7 @@ fn delete_pin() { run(BACKENDS, |client| { let pin = Bytes::from_slice(b"123456").unwrap(); - syscall!(client.set_pin(Pin::User, pin.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin.clone(), None, None)); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); @@ -637,8 +668,8 @@ fn delete_all_pins() { let pin1 = Bytes::from_slice(b"123456").unwrap(); let pin2 = Bytes::from_slice(b"12345678").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), None, false)); - syscall!(client.set_pin(Pin::Admin, pin2.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), None, None)); + syscall!(client.set_pin(Pin::Admin, pin2.clone(), None, None)); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); @@ -726,8 +757,8 @@ fn reset_auth_data() { let pin1 = Bytes::from_slice(b"123456").unwrap(); let pin2 = Bytes::from_slice(b"12345678").unwrap(); - syscall!(client.set_pin(Pin::User, pin1.clone(), None, false)); - syscall!(client.set_pin(Pin::Admin, pin2.clone(), None, false)); + syscall!(client.set_pin(Pin::User, pin1.clone(), None, None)); + syscall!(client.set_pin(Pin::Admin, pin2.clone(), None, None)); let reply = syscall!(client.has_pin(Pin::User)); assert!(reply.has_pin); From a73f23c3d6d6a7182ddcd4857eb4d3840b36f2df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 10 Jan 2024 09:55:42 +0100 Subject: [PATCH 6/6] Use DecryptedKey abstraction --- src/backend.rs | 13 +++-- src/backend/data.rs | 119 ++++++++++++++++++++++---------------------- 2 files changed, 68 insertions(+), 64 deletions(-) diff --git a/src/backend.rs b/src/backend.rs index 483d068..eebfb82 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -7,6 +7,7 @@ use core::fmt; use hkdf::Hkdf; use rand_core::{CryptoRng, RngCore}; +use salty::agreement::SecretKey; use sha2::Sha256; use trussed::{ backend::Backend, @@ -27,7 +28,7 @@ use crate::{ }; use data::{DeriveKey, PinData, Salt, CHACHA_KEY_LEN, SALT_LEN}; -use self::data::{delete_app_salt, ChachaKey}; +use self::data::{delete_app_salt, ChachaKey, DecryptedKey}; /// max accepted length for the hardware initial key material pub const MAX_HW_KEY_LEN: usize = 64; @@ -295,11 +296,15 @@ impl ExtensionImpl for AuthBackend { } AuthRequest::SetPinWithKey(request) => { let app_key = self.get_app_key(client_id, global_fs, ctx, rng)?; - let key_to_wrap = - keystore.load_key(Secrecy::Secret, Some(Kind::Symmetric(32)), &request.key)?; - let key_to_wrap = (&*key_to_wrap.material) + let key_material = keystore.load_key(Secrecy::Secret, None, &request.key)?; + let material_32: [u8; 32] = (&*key_material.material) .try_into() .map_err(|_| Error::ReadFailed)?; + let key_to_wrap = match key_material.kind { + Kind::Symmetric(32) => DecryptedKey::Chacha20Poly1305(material_32.into()), + Kind::X255 => DecryptedKey::X25519(SecretKey::from_seed(&material_32)), + _ => return Err(Error::NotFound)?, + }; PinData::reset_with_key( request.id, &request.pin, diff --git a/src/backend/data.rs b/src/backend/data.rs index b8864eb..dd734f2 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -3,7 +3,7 @@ use core::ops::Deref; -use chacha20poly1305::ChaCha8Poly1305; +use chacha20poly1305::{AeadInPlace, ChaCha8Poly1305, KeyInit}; use hmac::{Hmac, Mac}; use serde::{Deserialize, Serialize}; use serde_byte_array::ByteArray; @@ -32,7 +32,7 @@ pub(crate) type Salt = ByteArray; pub(crate) type Hash = ByteArray; pub(crate) type ChaChaTag = ByteArray; pub(crate) type ChachaKey = ByteArray; -pub(crate) type X25519Key = [u8; X25519_KEY_LEN]; +pub(crate) type X25519Key = ByteArray; #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub(crate) enum Key { @@ -49,7 +49,7 @@ impl Serialize for Key { Key::Chacha20Poly1305(k) => serializer.serialize_bytes(&**k), Key::X25519(k) => { let mut encoded = [0; ENCODED_X25519_KEY_LEN]; - encoded[0..X25519_KEY_LEN].copy_from_slice(&*k); + encoded[0..X25519_KEY_LEN].copy_from_slice(&**k); serializer.serialize_bytes(&encoded) } } @@ -75,9 +75,11 @@ impl<'de> Deserialize<'de> for Key { { match v.len() { CHACHA_KEY_LEN => Ok(Key::Chacha20Poly1305(ByteArray::new( + #[allow(clippy::expect_used)] v.try_into().expect("Len was just checked"), ))), ENCODED_X25519_KEY_LEN if v[X25519_KEY_LEN] == 0 => Ok(Key::X25519( + #[allow(clippy::expect_used)] v[..X25519_KEY_LEN] .try_into() .expect("Len was just checked"), @@ -110,6 +112,47 @@ impl DecryptedKey { DecryptedKey::X25519(k) => k.to_bytes(), } } + + pub fn aad(&self, id: PinId) -> Bytes<2> { + let mut aad = Bytes::new(); + #[allow(clippy::expect_used)] + aad.push(u8::from(id)) + .expect("Capacity is known to be 2 and length is known to be 0"); + match self { + DecryptedKey::Chacha20Poly1305(_) => {} + DecryptedKey::X25519(_) => + { + #[allow(clippy::expect_used)] + aad.push(0x00) + .expect("Capacity is known to be 2 and length is known to be 1") + } + } + aad + } + + pub fn encrypt(&self, id: PinId, pin_key: &ChachaKey) -> (Key, ChaChaTag) { + let aad = self.aad(id); + // The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern + // Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys + + let nonce = Default::default(); + let mut data = self.data(); + + let aead = ChaCha8Poly1305::new((&**pin_key).into()); + + #[allow(clippy::expect_used)] + let tag: [u8; CHACHA_TAG_LEN] = aead + .encrypt_in_place_detached(&nonce, &aad, &mut data) + .expect("Wrapping the key should always work, length are acceptable") + .into(); + + let key = match self { + DecryptedKey::Chacha20Poly1305(_) => Key::Chacha20Poly1305(data.into()), + DecryptedKey::X25519(_) => Key::X25519(data.into()), + }; + + (key, tag.into()) + } } /// Represent a key wrapped by the pin. @@ -270,7 +313,7 @@ impl PinData { ); KeyOrHash::Key(WrappedKeyData { - wrapped_key: Key::X25519(key_bytes), + wrapped_key: Key::X25519(key_bytes.into()), tag: tag.into(), }) } @@ -289,30 +332,21 @@ impl PinData { retries: Option, rng: &mut R, application_key: &ChachaKey, - mut key_to_wrap: ChachaKey, + key_to_wrap: DecryptedKey, ) -> Self where R: CryptoRng + RngCore, { - use chacha20poly1305::{AeadInPlace, KeyInit}; let mut salt = Salt::default(); rng.fill_bytes(salt.as_mut()); let pin_key = derive_key(id, pin, &salt, application_key); - let aead = ChaCha8Poly1305::new((&*pin_key).into()); - let nonce = Default::default(); - #[allow(clippy::expect_used)] - let tag: [u8; CHACHA_TAG_LEN] = aead - .encrypt_in_place_detached(&nonce, &[u8::from(id)], &mut *key_to_wrap) - .expect("Wrapping the key should always work, length are acceptable") - .into(); + + let (wrapped_key, tag) = key_to_wrap.encrypt(id, &pin_key); Self { id, retries: retries.map(From::from), salt, - data: KeyOrHash::Key(WrappedKeyData { - wrapped_key: Key::Chacha20Poly1305(key_to_wrap), - tag: tag.into(), - }), + data: KeyOrHash::Key(WrappedKeyData { wrapped_key, tag }), } } @@ -443,8 +477,6 @@ impl<'a> PinDataMut<'a> { mut wrapped_key: Key, tag: &ChaChaTag, ) -> Option { - use chacha20poly1305::{AeadInPlace, KeyInit}; - let pin_key = derive_key(self.id, pin, &self.salt, application_key); let aead = ChaCha8Poly1305::new((&*pin_key).into()); // The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern @@ -460,7 +492,7 @@ impl<'a> PinDataMut<'a> { } Key::X25519(k) => { aad_2 = [u8::from(self.data.id), 0]; - (&aad_2, *k) + (&aad_2, **k) } }; @@ -508,45 +540,13 @@ impl<'a> PinDataMut<'a> { application_key: &ChachaKey, rng: &mut R, ) { - use chacha20poly1305::{AeadInPlace, KeyInit}; let mut salt = Salt::default(); rng.fill_bytes(&mut *salt); let pin_key = derive_key(self.id, new, &salt, application_key); - let aead = ChaCha8Poly1305::new((&*pin_key).into()); - // The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern - // Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys - let nonce = Default::default(); - - let data = match old_key { - DecryptedKey::Chacha20Poly1305(mut k) => { - #[allow(clippy::expect_used)] - let tag: [u8; CHACHA_TAG_LEN] = aead - .encrypt_in_place_detached(&nonce, &[u8::from(self.id)], &mut *k) - .expect("Wrapping the key should always work, length are acceptable") - .into(); - - KeyOrHash::Key(WrappedKeyData { - wrapped_key: Key::Chacha20Poly1305(k), - tag: tag.into(), - }) - } - DecryptedKey::X25519(k) => { - let mut data = k.to_bytes(); - #[allow(clippy::expect_used)] - let tag: [u8; CHACHA_TAG_LEN] = aead - .encrypt_in_place_detached(&nonce, &[u8::from(self.id), 0], &mut data) - .expect("Wrapping the key should always work, length are acceptable") - .into(); - - KeyOrHash::Key(WrappedKeyData { - wrapped_key: Key::X25519(data), - tag: tag.into(), - }) - } - }; - + let (wrapped_key, tag) = old_key.encrypt(self.id, &pin_key); + let data = KeyOrHash::Key(WrappedKeyData { wrapped_key, tag }); *self.data = PinData { id: self.id, retries: self.retries, @@ -628,8 +628,8 @@ fn hash(id: PinId, pin: &Pin, salt: &Salt) -> Hash { fn derive_key(id: PinId, pin: &Pin, salt: &Salt, application_key: &[u8; 32]) -> Hash { #[allow(clippy::expect_used)] - let mut hmac = Hmac::::new_from_slice(application_key) - .expect("Slice will always be of acceptable size"); + let mut hmac: Hmac = + Mac::new_from_slice(application_key).expect("Hmac is compatible with all key sizes"); hmac.update(&[u8::from(id)]); hmac.update(&[pin_len(pin)]); hmac.update(pin); @@ -646,8 +646,7 @@ fn encrypt_pin_data( data: &mut [u8], aad: Option<[u8; 1]>, ) -> [u8; CHACHA_TAG_LEN] { - use chacha20poly1305::{AeadInPlace, KeyInit}; - let pin_key = derive_key(id, pin, &salt, application_key); + let pin_key = derive_key(id, pin, salt, application_key); let aead = ChaCha8Poly1305::new((&*pin_key).into()); // The pin key is only ever used to once to wrap a key. Nonce reuse is not a concern // Because the salt is also used in the key derivation process, PIN reuse across PINs will still lead to different keys @@ -728,8 +727,8 @@ fn load_app_salt(fs: &mut S, location: Location) -> Result ChachaKey { #[allow(clippy::expect_used)] - let mut hmac = Hmac::::new_from_slice(&**application_key) - .expect("Slice will always be of acceptable size"); + let mut hmac: Hmac = + Mac::new_from_slice(&**application_key).expect("Hmac is compatible with all key sizes"); hmac.update(&**salt); hmac.update(&(info.len() as u64).to_be_bytes()); hmac.update(info);