From 4e0bd1d08c1d6d99d423a48f69d5ee29aab849d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Thu, 16 Mar 2023 10:30:03 +0100 Subject: [PATCH 1/5] Add reset_pin_key This syscall allows resetting a pin. Unlike `set_pin`, it takes a key as parameter. This key will be returned by future calls to `get_pin_key`. Unlike `change_pin` this doesn't require knowledge of the current value of the PIN. The goal is to allow resetting a PIN from another source. For example, OpenPGP smartcards need to be able to reset the user pin given an admin pin With this patch, this can be done by using the admin key to wrap the user key. --- src/backend.rs | 18 ++++++++++++++ src/backend/data.rs | 33 +++++++++++++++++++++++++ src/extension.rs | 26 +++++++++++++++++++- src/extension/reply.rs | 20 ++++++++++++++++ src/extension/request.rs | 16 +++++++++++++ tests/backend.rs | 52 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 164 insertions(+), 1 deletion(-) diff --git a/src/backend.rs b/src/backend.rs index 219ce9a..02eb7b6 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -284,6 +284,24 @@ impl ExtensionImpl for AuthBackend { .save(fs, self.location)?; Ok(reply::SetPin.into()) } + AuthRequest::ResetPinKey(request) => { + let app_key = self.get_app_key(client_id, trussed_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) + .try_into() + .map_err(|_| Error::ReadFailed)?; + PinData::reset_given_key( + request.id, + &request.pin, + request.retries, + rng, + &app_key, + key_to_wrap, + ) + .save(fs, self.location)?; + Ok(reply::ResetPinKey.into()) + } AuthRequest::DeletePin(request) => { let path = request.id.path(); if fs.exists(&path, self.location) { diff --git a/src/backend/data.rs b/src/backend/data.rs index 661c0df..4d2f53f 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -171,6 +171,39 @@ impl PinData { } } + pub fn reset_given_key( + id: PinId, + pin: &Pin, + retries: Option, + rng: &mut R, + application_key: &Key, + mut key_to_wrap: Key, + ) -> 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(); + Self { + id, + retries: retries.map(From::from), + salt, + data: KeyOrHash::Key(WrappedKeyData { + wrapped_key: key_to_wrap, + tag: tag.into(), + }), + } + } + pub fn load(fs: &mut S, location: Location, id: PinId) -> Result { let path = id.path(); if !fs.exists(&path, location) { diff --git a/src/extension.rs b/src/extension.rs index 4f4dbb6..f77b645 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -7,7 +7,10 @@ pub mod reply; pub mod request; use serde::{Deserialize, Serialize}; -use trussed::serde_extensions::{Extension, ExtensionClient, ExtensionResult}; +use trussed::{ + serde_extensions::{Extension, ExtensionClient, ExtensionResult}, + types::KeyId, +}; use crate::{Pin, PinId}; @@ -32,6 +35,7 @@ pub enum AuthRequest { CheckPin(request::CheckPin), GetPinKey(request::GetPinKey), SetPin(request::SetPin), + ResetPinKey(request::ResetPinKey), ChangePin(request::ChangePin), DeletePin(request::DeletePin), DeleteAllPins(request::DeleteAllPins), @@ -45,6 +49,7 @@ pub enum AuthReply { CheckPin(reply::CheckPin), GetPinKey(reply::GetPinKey), SetPin(reply::SetPin), + ResetPinKey(reply::ResetPinKey), ChangePin(reply::ChangePin), DeletePin(reply::DeletePin), DeleteAllPins(reply::DeleteAllPins), @@ -114,6 +119,25 @@ pub trait AuthClient: ExtensionClient { }) } + /// Reset a pin. + /// + /// Similar to [`set_pin`](AuthClient::set_pin), but allows the key that the pin will unwrap to be configured. + /// This allows for example backing up the key for a pin, to be able to restore it from another source. + fn reset_set_pin_key>( + &mut self, + id: I, + pin: Pin, + retries: Option, + key: KeyId, + ) -> AuthResult<'_, reply::ResetPinKey, Self> { + self.extension(request::ResetPinKey { + id: id.into(), + pin, + retries, + key, + }) + } + /// Change the given PIN and resets its retry counter. /// /// The key obtained by [`get_pin_key`](AuthClient::get_pin_key) will stay the same diff --git a/src/extension/reply.rs b/src/extension/reply.rs index 0777ca1..e5e5d1b 100644 --- a/src/extension/reply.rs +++ b/src/extension/reply.rs @@ -99,6 +99,26 @@ impl TryFrom for SetPin { } } +#[derive(Debug, Deserialize, Serialize)] +pub struct ResetPinKey; + +impl From for AuthReply { + fn from(reply: ResetPinKey) -> Self { + Self::ResetPinKey(reply) + } +} + +impl TryFrom for ResetPinKey { + type Error = Error; + + fn try_from(reply: AuthReply) -> Result { + match reply { + AuthReply::ResetPinKey(reply) => Ok(reply), + _ => Err(Error::InternalError), + } + } +} + #[derive(Debug, Deserialize, Serialize)] pub struct ChangePin { pub success: bool, diff --git a/src/extension/request.rs b/src/extension/request.rs index 04d8117..d0d0da0 100644 --- a/src/extension/request.rs +++ b/src/extension/request.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 or MIT use serde::{Deserialize, Serialize}; +use trussed::types::KeyId; use super::AuthRequest; use crate::{Pin, PinId}; @@ -56,6 +57,21 @@ impl From for AuthRequest { } } +#[derive(Debug, Deserialize, Serialize)] +pub struct ResetPinKey { + pub id: PinId, + pub pin: Pin, + pub retries: Option, + /// If true, the PIN can be used to wrap/unwrap an application key + pub key: KeyId, +} + +impl From for AuthRequest { + fn from(request: ResetPinKey) -> Self { + Self::ResetPinKey(request) + } +} + #[derive(Debug, Deserialize, Serialize)] pub struct ChangePin { pub id: PinId, diff --git a/tests/backend.rs b/tests/backend.rs index 340e5c2..3be39aa 100644 --- a/tests/backend.rs +++ b/tests/backend.rs @@ -431,6 +431,58 @@ fn pin_key() { ) } +#[test] +fn reset_pin_key() { + run_with_hw_key( + BACKENDS, + Bytes::from_slice(b"Some HW ikm").unwrap(), + |client| { + let pin1 = Bytes::from_slice(b"12345678").unwrap(); + 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)); + assert!(syscall!(client.get_pin_key(Pin::User, pin2.clone())) + .result + .is_none()); + assert_eq!(syscall!(client.pin_retries(Pin::User)).retries, Some(2)); + assert!(!syscall!(client.check_pin(Pin::User, pin2.clone())).success); + assert_eq!(syscall!(client.pin_retries(Pin::User)).retries, Some(1)); + assert!(syscall!(client.check_pin(Pin::User, pin1.clone())).success); + let key = syscall!(client.get_pin_key(Pin::User, pin1)) + .result + .unwrap(); + assert_eq!(syscall!(client.pin_retries(Pin::User)).retries, Some(3)); + let mac = syscall!(client.sign_hmacsha256(key, b"Some data")).signature; + + syscall!(client.reset_set_pin_key(Pin::User, pin3.clone(), Some(3), key)); + + let key2 = syscall!(client.get_pin_key(Pin::User, pin3.clone())) + .result + .unwrap(); + let mac2 = syscall!(client.sign_hmacsha256(key2, b"Some data")).signature; + assert_eq!(mac, mac2); + + assert!(syscall!(client.change_pin(Pin::User, pin3.clone(), pin2.clone())).success); + + let key3 = syscall!(client.get_pin_key(Pin::User, pin2.clone())) + .result + .unwrap(); + let mac3 = syscall!(client.sign_hmacsha256(key3, b"Some data")).signature; + assert_eq!(mac, mac3); + + assert!(!syscall!(client.check_pin(Pin::User, pin3.clone())).success); + assert!(!syscall!(client.check_pin(Pin::User, pin3.clone())).success); + assert!(!syscall!(client.check_pin(Pin::User, pin3)).success); + assert!(!syscall!(client.check_pin(Pin::User, pin2.clone())).success); + assert!(syscall!(client.get_pin_key(Pin::User, pin2)) + .result + .is_none()); + assert_eq!(syscall!(client.pin_retries(Pin::User)).retries, Some(0)); + }, + ) +} + #[test] fn blocked_pin() { run(BACKENDS, |client| { From 98d5e12304857ae3ea29d14e9a9ffca3a9b1b856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 28 Mar 2023 09:19:35 +0200 Subject: [PATCH 2/5] Fix comments --- src/extension/request.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extension/request.rs b/src/extension/request.rs index d0d0da0..8443042 100644 --- a/src/extension/request.rs +++ b/src/extension/request.rs @@ -47,7 +47,7 @@ pub struct SetPin { pub id: PinId, pub pin: Pin, pub retries: Option, - /// If true, the PIN can be used to wrap/unwrap an application key + /// If true, the PIN can be used to wrap/unwrap a PIN key pub derive_key: bool, } @@ -62,7 +62,7 @@ pub struct ResetPinKey { pub id: PinId, pub pin: Pin, pub retries: Option, - /// If true, the PIN can be used to wrap/unwrap an application key + /// This key will be wrapped. It can be obtained again via a `GetPinKey` request pub key: KeyId, } From 17f790533a9af2b566e0b113374ed72076970b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 28 Mar 2023 09:32:30 +0200 Subject: [PATCH 3/5] Improve naming --- src/backend.rs | 6 +++--- src/backend/data.rs | 2 +- src/extension.rs | 10 +++++----- src/extension/reply.rs | 12 ++++++------ src/extension/request.rs | 8 ++++---- tests/backend.rs | 2 +- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/backend.rs b/src/backend.rs index 02eb7b6..d57310e 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -284,14 +284,14 @@ impl ExtensionImpl for AuthBackend { .save(fs, self.location)?; Ok(reply::SetPin.into()) } - AuthRequest::ResetPinKey(request) => { + AuthRequest::SetPinWithKey(request) => { let app_key = self.get_app_key(client_id, trussed_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) .try_into() .map_err(|_| Error::ReadFailed)?; - PinData::reset_given_key( + PinData::reset_with_key( request.id, &request.pin, request.retries, @@ -300,7 +300,7 @@ impl ExtensionImpl for AuthBackend { key_to_wrap, ) .save(fs, self.location)?; - Ok(reply::ResetPinKey.into()) + Ok(reply::SetPinWithKey.into()) } AuthRequest::DeletePin(request) => { let path = request.id.path(); diff --git a/src/backend/data.rs b/src/backend/data.rs index 4d2f53f..100c82d 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -171,7 +171,7 @@ impl PinData { } } - pub fn reset_given_key( + pub fn reset_with_key( id: PinId, pin: &Pin, retries: Option, diff --git a/src/extension.rs b/src/extension.rs index f77b645..5a655db 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -35,7 +35,7 @@ pub enum AuthRequest { CheckPin(request::CheckPin), GetPinKey(request::GetPinKey), SetPin(request::SetPin), - ResetPinKey(request::ResetPinKey), + SetPinWithKey(request::SetPinWithKey), ChangePin(request::ChangePin), DeletePin(request::DeletePin), DeleteAllPins(request::DeleteAllPins), @@ -49,7 +49,7 @@ pub enum AuthReply { CheckPin(reply::CheckPin), GetPinKey(reply::GetPinKey), SetPin(reply::SetPin), - ResetPinKey(reply::ResetPinKey), + SetPinWithKey(reply::SetPinWithKey), ChangePin(reply::ChangePin), DeletePin(reply::DeletePin), DeleteAllPins(reply::DeleteAllPins), @@ -123,14 +123,14 @@ pub trait AuthClient: ExtensionClient { /// /// Similar to [`set_pin`](AuthClient::set_pin), but allows the key that the pin will unwrap to be configured. /// This allows for example backing up the key for a pin, to be able to restore it from another source. - fn reset_set_pin_key>( + fn set_pin_with_key>( &mut self, id: I, pin: Pin, retries: Option, key: KeyId, - ) -> AuthResult<'_, reply::ResetPinKey, Self> { - self.extension(request::ResetPinKey { + ) -> AuthResult<'_, reply::SetPinWithKey, Self> { + self.extension(request::SetPinWithKey { id: id.into(), pin, retries, diff --git a/src/extension/reply.rs b/src/extension/reply.rs index e5e5d1b..ed2129f 100644 --- a/src/extension/reply.rs +++ b/src/extension/reply.rs @@ -100,20 +100,20 @@ impl TryFrom for SetPin { } #[derive(Debug, Deserialize, Serialize)] -pub struct ResetPinKey; +pub struct SetPinWithKey; -impl From for AuthReply { - fn from(reply: ResetPinKey) -> Self { - Self::ResetPinKey(reply) +impl From for AuthReply { + fn from(reply: SetPinWithKey) -> Self { + Self::SetPinWithKey(reply) } } -impl TryFrom for ResetPinKey { +impl TryFrom for SetPinWithKey { type Error = Error; fn try_from(reply: AuthReply) -> Result { match reply { - AuthReply::ResetPinKey(reply) => Ok(reply), + AuthReply::SetPinWithKey(reply) => Ok(reply), _ => Err(Error::InternalError), } } diff --git a/src/extension/request.rs b/src/extension/request.rs index 8443042..300c8b2 100644 --- a/src/extension/request.rs +++ b/src/extension/request.rs @@ -58,7 +58,7 @@ impl From for AuthRequest { } #[derive(Debug, Deserialize, Serialize)] -pub struct ResetPinKey { +pub struct SetPinWithKey { pub id: PinId, pub pin: Pin, pub retries: Option, @@ -66,9 +66,9 @@ pub struct ResetPinKey { pub key: KeyId, } -impl From for AuthRequest { - fn from(request: ResetPinKey) -> Self { - Self::ResetPinKey(request) +impl From for AuthRequest { + fn from(request: SetPinWithKey) -> Self { + Self::SetPinWithKey(request) } } diff --git a/tests/backend.rs b/tests/backend.rs index 3be39aa..c9c06d0 100644 --- a/tests/backend.rs +++ b/tests/backend.rs @@ -455,7 +455,7 @@ fn reset_pin_key() { assert_eq!(syscall!(client.pin_retries(Pin::User)).retries, Some(3)); let mac = syscall!(client.sign_hmacsha256(key, b"Some data")).signature; - syscall!(client.reset_set_pin_key(Pin::User, pin3.clone(), Some(3), key)); + syscall!(client.set_pin_with_key(Pin::User, pin3.clone(), Some(3), key)); let key2 = syscall!(client.get_pin_key(Pin::User, pin3.clone())) .result From 6391ba507d6e6c71856241c327ba0f581bf37d4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 28 Mar 2023 13:53:35 +0200 Subject: [PATCH 4/5] Improve documentation --- src/extension.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/extension.rs b/src/extension.rs index 5a655db..89befcf 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -119,9 +119,10 @@ pub trait AuthClient: ExtensionClient { }) } - /// Reset a pin. + /// Set a pin, resetting it's retry counter and setting the key to be wrapped /// /// Similar to [`set_pin`](AuthClient::set_pin), but allows the key that the pin will unwrap to be configured. + /// Currently only symmetric 256 bit keys are accepted. This method should be used only with keys that were obtained through [`get_pin_key`](AuthClient::get_pin_key) /// This allows for example backing up the key for a pin, to be able to restore it from another source. fn set_pin_with_key>( &mut self, From ce5ba2427cd08f349284e8d41855f1f6ea043c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 28 Mar 2023 15:15:15 +0200 Subject: [PATCH 5/5] Fix typo --- src/extension.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension.rs b/src/extension.rs index 89befcf..eeaf298 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -119,7 +119,7 @@ pub trait AuthClient: ExtensionClient { }) } - /// Set a pin, resetting it's retry counter and setting the key to be wrapped + /// Set a pin, resetting its retry counter and setting the key to be wrapped /// /// Similar to [`set_pin`](AuthClient::set_pin), but allows the key that the pin will unwrap to be configured. /// Currently only symmetric 256 bit keys are accepted. This method should be used only with keys that were obtained through [`get_pin_key`](AuthClient::get_pin_key)