From cc3e8564b8446ee76bc71838a97ef65a860dbe8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 26 Apr 2023 12:05:05 +0200 Subject: [PATCH] Add `reset_app_keys` and `reset_auth_data` syscalls. - `delete_all_pins` now doesn't affect application keys - `reset_app_keys`: reset all application keys. Getting them again after calling this will not yield the same key - `reset_auth_data` combines `delete_all_pins` and `reset_app_keys` This is a breaking change and applications (trussed-secrets) relying on the old `delete_all_pins` behaviour will need to be fixed. --- Cargo.toml | 2 +- src/backend.rs | 15 ++++++++- src/backend/data.rs | 11 ++++++ src/extension.rs | 14 ++++++++ src/extension/reply.rs | 39 ++++++++++++++++++++++ src/extension/request.rs | 18 ++++++++++ tests/backend.rs | 72 +++++++++++++++++++++++++++++++++++++++- 7 files changed, 168 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 194e27f..aff3f9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,4 +28,4 @@ trussed = { version = "0.1.0", features = ["serde-extensions", "virt"] } [patch.crates-io] littlefs2 = { git = "https://github.com/Nitrokey/littlefs2", tag = "v0.3.2-nitrokey-2" } -trussed = { git = "https://github.com/Nitrokey/trussed.git", tag = "v0.1.0-nitrokey-5" } +trussed = { git = "https://github.com/sosthene-nitrokey/trussed.git", branch = "remove-dir-all-where" } diff --git a/src/backend.rs b/src/backend.rs index 894f567..e092d2c 100644 --- a/src/backend.rs +++ b/src/backend.rs @@ -27,6 +27,8 @@ use crate::{ }; use data::{Key, PinData, Salt, KEY_LEN, SALT_LEN}; +use self::data::delete_app_salt; + /// max accepted length for the hardware initial key material pub const MAX_HW_KEY_LEN: usize = 64; @@ -316,9 +318,20 @@ impl ExtensionImpl for AuthBackend { Ok(reply::DeletePin.into()) } AuthRequest::DeleteAllPins(_) => { + fs.remove_dir_all_where(&PathBuf::new(), self.location, |entry| { + entry.file_name().as_ref().starts_with("pin.") + }) + .map_err(|_| Error::WriteFailed)?; + Ok(reply::DeleteAllPins.into()) + } + AuthRequest::ResetAppKeys(_) => { + delete_app_salt(fs, self.location)?; + Ok(reply::ResetAppKeys {}.into()) + } + AuthRequest::ResetAuthData(_) => { fs.remove_dir_all(&PathBuf::new(), self.location) .map_err(|_| Error::WriteFailed)?; - Ok(reply::DeleteAllPins.into()) + Ok(reply::ResetAuthData.into()) } AuthRequest::PinRetries(request) => { let retries = PinData::load(fs, self.location, request.id)?.retries_left(); diff --git a/src/backend/data.rs b/src/backend/data.rs index 9cf6943..5e79feb 100644 --- a/src/backend/data.rs +++ b/src/backend/data.rs @@ -512,6 +512,17 @@ pub(crate) fn get_app_salt( } } +pub(crate) fn delete_app_salt( + fs: &mut S, + location: Location, +) -> Result<(), trussed::Error> { + if fs.exists(&app_salt_path(), location) { + fs.remove_file(&app_salt_path(), location) + } else { + Ok(()) + } +} + fn create_app_salt( fs: &mut S, rng: &mut R, diff --git a/src/extension.rs b/src/extension.rs index 059088e..416a443 100644 --- a/src/extension.rs +++ b/src/extension.rs @@ -42,6 +42,8 @@ pub enum AuthRequest { DeletePin(request::DeletePin), DeleteAllPins(request::DeleteAllPins), PinRetries(request::PinRetries), + ResetAppKeys(request::ResetAppKeys), + ResetAuthData(request::ResetAuthData), } #[derive(Debug, Deserialize, Serialize)] @@ -57,6 +59,8 @@ pub enum AuthReply { DeletePin(reply::DeletePin), DeleteAllPins(reply::DeleteAllPins), PinRetries(reply::PinRetries), + ResetAppKeys(reply::ResetAppKeys), + ResetAuthData(reply::ResetAuthData), } /// Provides access to the [`AuthExtension`][]. @@ -180,6 +184,16 @@ pub trait AuthClient: ExtensionClient { ) -> AuthResult<'_, reply::GetApplicationKey, Self> { self.extension(request::GetApplicationKey { info }) } + + /// Delete all application keys + fn reset_app_keys(&mut self) -> AuthResult<'_, reply::ResetAppKeys, Self> { + self.extension(request::ResetAppKeys {}) + } + + /// Combines [`reset_app_keys`][AuthClient::reset_app_keys] and [`delete_all_pins`](AuthClient::delete_all_pins) + fn reset_auth_data(&mut self) -> AuthResult<'_, reply::ResetAuthData, Self> { + self.extension(request::ResetAuthData {}) + } } impl> AuthClient for C {} diff --git a/src/extension/reply.rs b/src/extension/reply.rs index 7fbd32b..e2f878d 100644 --- a/src/extension/reply.rs +++ b/src/extension/reply.rs @@ -226,3 +226,42 @@ impl TryFrom for PinRetries { } } } +#[derive(Debug, Deserialize, Serialize)] +pub struct ResetAppKeys; + +impl From for AuthReply { + fn from(reply: ResetAppKeys) -> Self { + Self::ResetAppKeys(reply) + } +} + +impl TryFrom for ResetAppKeys { + type Error = Error; + + fn try_from(reply: AuthReply) -> Result { + match reply { + AuthReply::ResetAppKeys(reply) => Ok(reply), + _ => Err(Error::InternalError), + } + } +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct ResetAuthData; + +impl From for AuthReply { + fn from(reply: ResetAuthData) -> Self { + Self::ResetAuthData(reply) + } +} + +impl TryFrom for ResetAuthData { + type Error = Error; + + fn try_from(reply: AuthReply) -> Result { + match reply { + AuthReply::ResetAuthData(reply) => Ok(reply), + _ => Err(Error::InternalError), + } + } +} diff --git a/src/extension/request.rs b/src/extension/request.rs index 923eebe..e46d2b7 100644 --- a/src/extension/request.rs +++ b/src/extension/request.rs @@ -126,3 +126,21 @@ impl From for AuthRequest { Self::PinRetries(request) } } + +#[derive(Debug, Deserialize, Serialize)] +pub struct ResetAppKeys; + +impl From for AuthRequest { + fn from(request: ResetAppKeys) -> Self { + Self::ResetAppKeys(request) + } +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct ResetAuthData; + +impl From for AuthRequest { + fn from(request: ResetAuthData) -> Self { + Self::ResetAuthData(request) + } +} diff --git a/tests/backend.rs b/tests/backend.rs index 5b7f4f6..dff7afd 100644 --- a/tests/backend.rs +++ b/tests/backend.rs @@ -649,6 +649,12 @@ fn delete_all_pins() { ) .is_err()); + syscall!(client.reset_app_keys()); + let reply = syscall!(client.has_pin(Pin::User)); + assert!(reply.has_pin); + let reply = syscall!(client.has_pin(Pin::Admin)); + assert!(reply.has_pin); + syscall!(client.delete_all_pins()); let reply = syscall!(client.has_pin(Pin::User)); @@ -664,7 +670,7 @@ fn delete_all_pins() { } #[test] -fn application_key() { +fn reset_application_key() { run(BACKENDS, |client| { let info1 = Message::from_slice(b"test1").unwrap(); let info2 = Message::from_slice(b"test2").unwrap(); @@ -682,6 +688,14 @@ fn application_key() { syscall!(client.delete_all_pins()); + let app_key1_after_delete = syscall!(client.get_application_key(info1.clone())).key; + let mac1_after_delete = + syscall!(client.sign_hmacsha256(app_key1_after_delete, b"Some data")).signature; + // Same info leads to same key + assert_eq!(mac1, mac1_after_delete); + + syscall!(client.reset_app_keys()); + // After deletion same info leads to different keys let app_key1_after_delete = syscall!(client.get_application_key(info1)).key; let mac1_after_delete = @@ -689,3 +703,59 @@ fn application_key() { assert_ne!(mac1, mac1_after_delete); }) } + +#[test] +fn reset_auth_data() { + run(BACKENDS, |client| { + /* ------- APP KEYS ------- */ + let info1 = Message::from_slice(b"test1").unwrap(); + let info2 = Message::from_slice(b"test2").unwrap(); + let app_key1 = syscall!(client.get_application_key(info1.clone())).key; + let app_key2 = syscall!(client.get_application_key(info2)).key; + let mac1 = syscall!(client.sign_hmacsha256(app_key1, b"Some data")).signature; + let mac2 = syscall!(client.sign_hmacsha256(app_key2, b"Some data")).signature; + // Different info leads to different keys + assert_ne!(mac1, mac2); + + let app_key1_again = syscall!(client.get_application_key(info1.clone())).key; + let mac1_again = syscall!(client.sign_hmacsha256(app_key1_again, b"Some data")).signature; + // Same info leads to same key + assert_eq!(mac1, mac1_again); + + /* ------- 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)); + + let reply = syscall!(client.has_pin(Pin::User)); + assert!(reply.has_pin); + let reply = syscall!(client.has_pin(Pin::Admin)); + assert!(reply.has_pin); + assert!(try_syscall!( + client.read_file(Location::Internal, PathBuf::from("/backend-auth/pin.00")) + ) + .is_err()); + + syscall!(client.reset_auth_data()); + + /* ------- APP KEYS ------- */ + // After deletion same info leads to different keys + let app_key1_after_delete = syscall!(client.get_application_key(info1)).key; + let mac1_after_delete = + syscall!(client.sign_hmacsha256(app_key1_after_delete, b"Some data")).signature; + assert_ne!(mac1, mac1_after_delete); + + /* ------- PINS ------- */ + let reply = syscall!(client.has_pin(Pin::User)); + assert!(!reply.has_pin); + let reply = syscall!(client.has_pin(Pin::Admin)); + assert!(!reply.has_pin); + + let result = try_syscall!(client.check_pin(Pin::User, pin1)); + assert!(result.is_err()); + let result = try_syscall!(client.check_pin(Pin::Admin, pin2)); + assert!(result.is_err()); + }) +}