From 5916b25e374752e8b470a5b813e08e1284524c15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 31 Jul 2024 17:16:16 +0200 Subject: [PATCH 01/18] Associate an X25519 key to the user PIN --- Cargo.toml | 12 ++-- src/lib.rs | 46 +++++++++------ src/state.rs | 164 +++++++++++++++++++++++++++++++++++++++++++++++---- src/virt.rs | 38 +++++++++++- 4 files changed, 225 insertions(+), 35 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 261cc47..4a3a2c2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,10 +33,12 @@ heapless-bytes = "0.3.0" subtle = { version = "2", default-features = false } trussed-rsa-alloc = { version = "0.2.1", features = ["raw"], optional = true } trussed-chunked = "0.1.0" -trussed-staging = { version = "0.3.0", features = ["chunked"], default-features = false, optional = true } +trussed-hpke = "0.1.0" +trussed-wrap-key-to-file = "0.1.0" +trussed-staging = { version = "0.3.0", features = ["chunked", "hpke", "wrap-key-to-file"], default-features = false, optional = true } +littlefs2 = "0.4.0" [dev-dependencies] -littlefs2 = "0.3.2" rand_core = { version = "0.6", features = ["getrandom"] } trussed = { version = "0.1.0", features = ["virt"] } env_logger = "0.9" @@ -78,11 +80,13 @@ log-error = [] [patch.crates-io] trussed = { git = "https://github.com/Nitrokey/trussed" , tag = "v0.1.0-nitrokey.18" } -littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "ebd27e49ca321089d01d8c9b169c4aeb58ceeeca" } +littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "498fee7f7b908cb14e6ac18dca8442e19c89c0a9" } trussed-auth = { git = "https://github.com/trussed-dev/trussed-auth.git", tag = "v0.3.0"} trussed-rsa-alloc = { git = "https://github.com/trussed-dev/trussed-rsa-backend.git", tag = "v0.2.1" } trussed-chunked = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "chunked-v0.1.0" } -trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "v0.3.0" } +trussed-staging = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } +trussed-hpke = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } +trussed-wrap-key-to-file = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } apdu-dispatch = { git = "https://github.com/Nitrokey/apdu-dispatch", tag = "v0.1.2-nitrokey.2" } trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" } usbd-ccid = { git = "https://github.com/Nitrokey/usbd-ccid", tag = "v0.2.0-nitrokey.1" } diff --git a/src/lib.rs b/src/lib.rs index cbf1d53..1ce6c6b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,8 @@ mod tlv; pub use piv_types::{AsymmetricAlgorithms, Pin, Puk}; use trussed_chunked::ChunkedClient; +use trussed_hpke::HpkeClient; +use trussed_wrap_key_to_file::WrapKeyToFileClient; #[cfg(feature = "virt")] pub mod virt; @@ -307,15 +309,13 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { // maybe reserve this for the case VerifyLogin::PivPin? pub fn login(&mut self, login: commands::VerifyLogin) -> Result { if let commands::VerifyLogin::PivPin(pin) = login { - let tmp = self.state.persistent.verify_pin(&pin, self.trussed); - debug!("Verify result: {tmp:?}"); - if tmp { - self.state.volatile.app_security_status.pin_verified = true; - self.state.volatile.app_security_status.pin_just_verified = true; + self.state + .volatile + .verify_pin(&pin, self.trussed, &self.options); + if self.state.volatile.pin_verified() { Ok(()) } else { // should we logout here? - self.state.volatile.app_security_status.pin_verified = false; self.state.volatile.app_security_status.pin_just_verified = false; let remaining = self.state.persistent.remaining_pin_retries(self.trussed); if remaining == 0 { @@ -335,8 +335,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { Verify::Login(login) => self.login(login), Verify::Logout(_) => { - self.state.volatile.app_security_status.pin_verified = false; - self.state.volatile.app_security_status.pin_just_verified = false; + self.state.volatile.clear_pin_verified(self.trussed); Ok(()) } @@ -344,7 +343,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { if key_reference != commands::VerifyKeyReference::ApplicationPin { return Err(Status::FunctionNotSupported); } - if self.state.volatile.app_security_status.pin_verified { + if self.state.volatile.pin_verified() { Ok(()) } else { let retries = self.state.persistent.remaining_pin_retries(self.trussed); @@ -370,8 +369,9 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { { return Err(Status::VerificationFailed); } - self.state.volatile.app_security_status.pin_verified = true; - self.state.volatile.app_security_status.pin_just_verified = true; + self.state + .volatile + .verify_pin(&new_pin, self.trussed, &self.options); Ok(()) } @@ -383,7 +383,6 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { { return Err(Status::VerificationFailed); } - self.state.volatile.app_security_status.puk_verified = true; Ok(()) } @@ -712,14 +711,14 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { } match key_ref.use_security_condition() { SecurityCondition::Pin => { - if !self.state.volatile.app_security_status.pin_verified { + if !self.state.volatile.pin_verified() { warn!("Authenticate challenge without pin validated"); return Err(Status::SecurityStatusNotSatisfied); } } SecurityCondition::PinAlways => { if !just_verified { - warn!("AUTHENTICATE CHALLENGE WITHOUT PIN VALIDATED"); + warn!("authenticate challenge without pin validated"); return Err(Status::SecurityStatusNotSatisfied); } } @@ -1093,10 +1092,23 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { /// Super trait with all trussed extensions required by opcard pub trait Client: - trussed::Client + AuthClient + ChunkedClient + trussed::client::Ed255 + client::Tdes + trussed::Client + + AuthClient + + ChunkedClient + + trussed::client::Ed255 + + client::Tdes + + WrapKeyToFileClient + + HpkeClient { } -impl Client - for C +impl< + C: trussed::Client + + AuthClient + + ChunkedClient + + trussed::client::Ed255 + + client::Tdes + + WrapKeyToFileClient + + HpkeClient, + > Client for C { } diff --git a/src/state.rs b/src/state.rs index 23b9d5b..f7038e9 100644 --- a/src/state.rs +++ b/src/state.rs @@ -5,6 +5,7 @@ use flexiber::EncodableHeapless; use heapless::Vec; use heapless_bytes::Bytes; use iso7816::Status; +use littlefs2::{path, path::Path}; use trussed::{ api::reply::Metadata, config::MAX_MESSAGE_LENGTH, @@ -23,6 +24,13 @@ use crate::{ use crate::{Pin, Puk}; +/// User pin key wrapped by the resetting code key +const PUK_USER_KEY_BACKUP: &Path = path!("puk-user-pin-key.bin"); +/// User asymmetric key private part, wrapped by the PIN key +const USER_PRIVATE_KEY: &Path = path!("user-private-key.bin"); +/// User asymmetric key publick part +const USER_PUBLIC_KEY: &Path = path!("user-public-key.bin"); + pub enum PinPolicy { Never, Once, @@ -262,10 +270,47 @@ pub struct Volatile { pub struct GlobalSecurityStatus {} impl Volatile { + pub fn verify_pin( + &mut self, + value: &Pin, + client: &mut T, + options: &crate::Options, + ) -> Option { + self.clear_pin_verified(client); + let pin = Bytes::from_slice(&value.0).expect("Convertion of static array"); + let pin_key = syscall!(client.get_pin_key(PinType::UserPin, pin)).result?; + let key = syscall!(client.unwrap_key_from_file( + Mechanism::Chacha8Poly1305, + pin_key, + PathBuf::from(USER_PRIVATE_KEY), + options.storage, + Location::Volatile, + USER_PRIVATE_KEY.as_str().as_bytes() + )) + .key + .expect("Failed to unwrap private key"); + syscall!(client.delete(pin_key)); + self.app_security_status.pin_verified = Some(key); + self.app_security_status.pin_just_verified = true; + Some(key) + } + + pub fn pin_verified(&self) -> bool { + self.app_security_status.pin_verified.is_some() + } + + pub fn clear_pin_verified(&mut self, client: &mut impl crate::Client) { + if let Some(key) = self.app_security_status.pin_verified { + syscall!(client.delete(key)); + } + self.app_security_status.pin_verified = None; + self.app_security_status.pin_just_verified = false; + } + pub fn security_valid(&self, condition: SecurityCondition, just_verified: bool) -> bool { use SecurityCondition::*; match condition { - Pin => self.app_security_status.pin_verified, + Pin => self.app_security_status.pin_verified.is_some(), PinAlways => just_verified, Always => true, } @@ -274,7 +319,7 @@ impl Volatile { pub fn read_valid(&self, condition: ReadAccessRule) -> bool { use ReadAccessRule::*; match condition { - Pin | PinOrOcc => self.app_security_status.pin_verified, + Pin | PinOrOcc => self.app_security_status.pin_verified.is_some(), Always => true, } } @@ -298,9 +343,9 @@ impl Volatile { #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct AppSecurityStatus { - pub pin_verified: bool, + /// Contains the decrypted asymetric key + pin_verified: Option, pub pin_just_verified: bool, - pub puk_verified: bool, pub administrator_verified: bool, } @@ -330,13 +375,6 @@ impl Persistent { .unwrap_or(0) } - pub fn verify_pin(&mut self, value: &Pin, client: &mut T) -> bool { - let pin = Bytes::from_slice(&value.0).expect("Convertion of static array"); - try_syscall!(client.check_pin(PinType::UserPin, pin)) - .map(|r| r.success) - .unwrap_or(false) - } - pub fn verify_puk(&mut self, value: &Puk, client: &mut T) -> bool { let puk = Bytes::from_slice(&value.0).expect("Convertion of static array"); try_syscall!(client.check_pin(PinType::Puk, puk)) @@ -488,10 +526,111 @@ impl Persistent { } } + fn ensure_pins_not_init(client: &mut T) -> Result<(), Status> { + // If PINs are already there when initializing, it likely means that the state was corrupted rather than absent. + // In that case, we wait for the user to explicitely factory-reset the device to avoid risking loosing data. + // See https://github.com/Nitrokey/opcard-rs/issues/165 + if syscall!(client.has_pin(PinType::UserPin)).has_pin + || syscall!(client.has_pin(PinType::Puk)).has_pin + { + debug!("Init pins after pins are already there"); + return Err(Status::UnspecifiedNonpersistentExecutionError); + } + Ok(()) + } + + fn init_pins(client: &mut T, options: &crate::Options) -> Result<(), Status> { + let default_pin = + Bytes::from_slice(&Self::DEFAULT_PIN.0).expect("Convertion of static array"); + try_syscall!(client.set_pin( + PinType::UserPin, + default_pin.clone(), + Some(Self::PIN_RETRIES_DEFAULT), + true + )) + .map_err(|_err| { + error!("Failed to set pin"); + Status::UnspecifiedPersistentExecutionError + })?; + let default_puk = + Bytes::from_slice(&Self::DEFAULT_PUK.0).expect("Convertion of static array"); + try_syscall!(client.set_pin( + PinType::Puk, + default_puk.clone(), + Some(Self::PIN_RETRIES_DEFAULT), + true + )) + .map_err(|_err| { + error!("Failed to set puk"); + Status::UnspecifiedPersistentExecutionError + })?; + + let user_key = syscall!(client.get_pin_key(PinType::UserPin, default_pin)) + .result + .expect("PIN was just set"); + let puk_key = syscall!(client.get_pin_key(PinType::Puk, default_puk)) + .result + .expect("PUK was just set"); + + let path = PathBuf::from(PUK_USER_KEY_BACKUP); + syscall!(client.wrap_key_to_file( + Mechanism::Chacha8Poly1305, + puk_key, + user_key, + path, + Location::External, + PUK_USER_KEY_BACKUP.as_str().as_bytes() + )); + + syscall!(client.delete(puk_key)); + + let user_asymetric_key = syscall!(client.generate_key( + Mechanism::X255, + StorageAttributes::new().set_persistence(Location::Volatile) + )) + .key; + + syscall!(client.wrap_key_to_file( + Mechanism::Chacha8Poly1305, + user_key, + user_asymetric_key, + PathBuf::from(USER_PRIVATE_KEY), + options.storage, + USER_PRIVATE_KEY.as_str().as_bytes() + )); + + let user_asymetric_public_key = syscall!(client.derive_key( + Mechanism::X255, + user_asymetric_key, + None, + StorageAttributes::new().set_persistence(options.storage) + )) + .key; + let key = syscall!(client.serialize_key( + Mechanism::X255, + user_asymetric_public_key, + KeySerialization::Raw + )) + .serialized_key; + syscall!(client.write_file( + options.storage, + PathBuf::from(USER_PUBLIC_KEY), + Bytes::from_slice(&*key).unwrap(), + None + )); + + syscall!(client.delete(user_key)); + syscall!(client.clear(user_asymetric_key)); + + Ok(()) + } + pub fn initialize( client: &mut T, options: &crate::Options, ) -> Result { + Self::ensure_pins_not_init(client)?; + info!("initializing PIV state"); let administration = KeyWithAlg { id: syscall!(client.unsafe_inject_key( @@ -553,8 +692,7 @@ impl Persistent { storage: options.storage, }; state.save(client); - state.reset_pin(Self::DEFAULT_PIN, client)?; - state.reset_puk(Self::DEFAULT_PUK, client)?; + Self::init_pins(client, options)?; Ok(state) } diff --git a/src/virt.rs b/src/virt.rs index 864ab45..cc820d8 100644 --- a/src/virt.rs +++ b/src/virt.rs @@ -13,9 +13,11 @@ pub mod dispatch { }; use trussed_auth::{AuthBackend, AuthContext, AuthExtension, MAX_HW_KEY_LEN}; use trussed_chunked::ChunkedExtension; + use trussed_hpke::HpkeExtension; #[cfg(feature = "rsa")] use trussed_rsa_alloc::SoftwareRsa; use trussed_staging::{StagingBackend, StagingContext}; + use trussed_wrap_key_to_file::WrapKeyToFileExtension; /// Backends used by opcard pub const BACKENDS: &[BackendId] = &[ @@ -38,6 +40,8 @@ pub mod dispatch { pub enum Extension { Auth, Chunked, + Hpke, + WrapKeyToFile, } impl From for u8 { @@ -45,6 +49,8 @@ pub mod dispatch { match extension { Extension::Auth => 0, Extension::Chunked => 1, + Extension::Hpke => 2, + Extension::WrapKeyToFile => 3, } } } @@ -56,6 +62,8 @@ pub mod dispatch { match id { 0 => Ok(Extension::Auth), 1 => Ok(Extension::Chunked), + 2 => Ok(Extension::Hpke), + 3 => Ok(Extension::WrapKeyToFile), _ => Err(Error::InternalError), } } @@ -141,7 +149,7 @@ pub mod dispatch { request, resources, ), - Extension::Chunked => Err(Error::RequestNotAvailable), + Extension::Chunked|Extension::Hpke|Extension::WrapKeyToFile => Err(Error::RequestNotAvailable), } Backend::Staging =>match extension { Extension::Chunked => { @@ -153,6 +161,24 @@ pub mod dispatch { resources ) } + Extension::Hpke => { + >::extension_request_serialized( + &mut self.staging, + &mut ctx.core, + &mut ctx.backends.staging, + request, + resources + ) + } + Extension::WrapKeyToFile => { + >::extension_request_serialized( + &mut self.staging, + &mut ctx.core, + &mut ctx.backends.staging, + request, + resources + ) + } Extension::Auth => Err(Error::RequestNotAvailable), } #[cfg(feature = "rsa")] @@ -171,6 +197,16 @@ pub mod dispatch { const ID: Self::Id = Self::Id::Chunked; } + impl ExtensionId for Dispatch { + type Id = Extension; + + const ID: Self::Id = Self::Id::Hpke; + } + impl ExtensionId for Dispatch { + type Id = Extension; + + const ID: Self::Id = Self::Id::WrapKeyToFile; + } } use std::path::PathBuf; From 93c182c1aa07e340749f6e44aacc54efe0261493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 2 Aug 2024 16:15:37 +0200 Subject: [PATCH 02/18] Encrypt protected DOs --- src/lib.rs | 13 +- src/state.rs | 274 +++++++++++++++++++++++++++---------- tests/command_response.ron | 33 +++++ 3 files changed, 237 insertions(+), 83 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1ce6c6b..a7b5ec3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -942,14 +942,10 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { container: Container, mut reply: Reply<'_, R>, ) -> Result { - if !self - .state - .volatile - .read_valid(container.contact_access_rule()) - { - warn!("Unauthorized attempt to access: {:?}", container); - return Err(Status::SecurityStatusNotSatisfied); - } + let read_valid = + self.state + .volatile + .read_valid_key(container, self.trussed, &self.options)?; use state::ContainerStorage; let tag = match container { @@ -970,6 +966,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { self.trussed, self.options.storage, reply.lend(), + read_valid, ); if !res? { diff --git a/src/state.rs b/src/state.rs index f7038e9..3d7be9d 100644 --- a/src/state.rs +++ b/src/state.rs @@ -31,6 +31,9 @@ const USER_PRIVATE_KEY: &Path = path!("user-private-key.bin"); /// User asymmetric key publick part const USER_PUBLIC_KEY: &Path = path!("user-public-key.bin"); +/// Info parameter for the container storage +const HPKE_SEALKEY_CONTAINER_INFO: &[u8] = b"Container Storage"; + pub enum PinPolicy { Never, Once, @@ -269,6 +272,18 @@ pub struct Volatile { #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct GlobalSecurityStatus {} +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum ReadValid { + /// Container can be read in plain text + Plain, + /// Container is stored encrypted but is also not found + EncryptedNotFound, + /// Container can be read with read_encrypted_chunk using the key + /// + /// The key is stored in volatile storage and must be deleted after use + Encrypted(KeyId), +} + impl Volatile { pub fn verify_pin( &mut self, @@ -316,14 +331,42 @@ impl Volatile { } } - pub fn read_valid(&self, condition: ReadAccessRule) -> bool { - use ReadAccessRule::*; - match condition { - Pin | PinOrOcc => self.app_security_status.pin_verified.is_some(), - Always => true, - } - } + pub fn read_valid_key( + &mut self, + container: Container, + client: &mut impl crate::Client, + options: &crate::Options, + ) -> Result { + let pin_key = match ( + container.contact_access_rule(), + self.app_security_status.pin_verified, + ) { + (ReadAccessRule::Pin | ReadAccessRule::PinOrOcc, None) => { + warn!("Unauthorized attempt to access: {:?}", container); + return Err(Status::SecurityStatusNotSatisfied); + } + (ReadAccessRule::Always, _) => { + return Ok(ReadValid::Plain); + } + (ReadAccessRule::Pin | ReadAccessRule::PinOrOcc, Some(key)) => key, + }; + // Here we have the pin key, and we are in the complex case + let data_encryption_sealed_key_path = ContainerStorage(container).path_key(); + + let Ok(unsealed_key) = try_syscall!(client.hpke_open_key_from_file( + pin_key, + data_encryption_sealed_key_path, + options.storage, + Location::Volatile, + Bytes::from_slice(ContainerStorage(container).path_key_str().as_bytes()).unwrap(), + Bytes::from_slice(HPKE_SEALKEY_CONTAINER_INFO).unwrap(), + )) else { + return Ok(ReadValid::EncryptedNotFound); + }; + + Ok(ReadValid::Encrypted(unsealed_key.key)) + } pub fn take_single_challenge(&mut self) -> Option> { match self.command_cache.take() { Some(CommandCache::SingleAuthChallenge(b)) => return Some(b), @@ -756,43 +799,47 @@ fn load_if_exists_streaming( location: Location, path: &PathBuf, mut buffer: Reply<'_, R>, + encryption: Option, ) -> Result { - let mut read_len = 0; - let file_len; - match try_syscall!(client.start_chunked_read(location, path.clone())) { - Ok(r) => { - read_len += r.data.len(); - file_len = r.len; - buffer.append_len(file_len)?; - buffer.expand(&r.data)?; - if !r.data.is_full() { - debug_assert_eq!(read_len, file_len); - return Ok(true); - } + let offset = buffer.len(); + match encryption { + Some(key) => { + try_syscall!(client.start_encrypted_chunked_read(location, path.clone(), key,)) + .map_err(|_err| { + error!("Encrypted {path} couldn't be read: {_err:?}"); + Status::UnspecifiedPersistentExecutionError + })?; } - Err(_err) => match try_syscall!(client.entry_metadata(location, path.clone())) { - Ok(Metadata { metadata: None }) => return Ok(false), - Ok(Metadata { - metadata: Some(_metadata), - }) => { - error!("File {path} exists but couldn't be read: {_metadata:?}, {_err:?}"); - return Err(Status::UnspecifiedPersistentExecutionError); - } - Err(_err) => { - error!("File {path} couldn't be read: {_err:?}"); - return Err(Status::UnspecifiedPersistentExecutionError); + None => match try_syscall!(client.start_chunked_read(location, path.clone())) { + Ok(r) => { + buffer.expand(&r.data)?; + if !r.data.is_full() { + buffer.prepend_len(offset)?; + return Ok(true); + } } + Err(_err) => match try_syscall!(client.entry_metadata(location, path.clone())) { + Ok(Metadata { metadata: None }) => return Ok(false), + Ok(Metadata { + metadata: Some(_metadata), + }) => { + error!("File {path} exists but couldn't be read: {_metadata:?}"); + return Err(Status::UnspecifiedPersistentExecutionError); + } + Err(_err) => { + error!("File {path} couldn't be read: {_err:?}"); + return Err(Status::UnspecifiedPersistentExecutionError); + } + }, }, } loop { match try_syscall!(client.read_file_chunk()) { Ok(r) => { - debug_assert_eq!(r.len, file_len); - read_len += r.data.len(); buffer.expand(&r.data)?; if !r.data.is_full() { - debug_assert_eq!(read_len, file_len); + buffer.prepend_len(offset)?; break; } } @@ -809,47 +856,55 @@ fn load_if_exists_streaming( pub struct ContainerStorage(pub Container); impl ContainerStorage { - fn path(self) -> PathBuf { - PathBuf::from(match self.0 { - Container::CardCapabilityContainer => "CardCapabilityContainer", - Container::CardHolderUniqueIdentifier => "CardHolderUniqueIdentifier", - Container::X509CertificateFor9A => "X509CertificateFor9A", - Container::CardholderFingerprints => "CardholderFingerprints", - Container::SecurityObject => "SecurityObject", - Container::CardholderFacialImage => "CardholderFacialImage", - Container::X509CertificateFor9E => "X509CertificateFor9E", - Container::X509CertificateFor9C => "X509CertificateFor9C", - Container::X509CertificateFor9D => "X509CertificateFor9D", - Container::PrintedInformation => "PrintedInformation", - Container::DiscoveryObject => "DiscoveryObject", - Container::KeyHistoryObject => "KeyHistoryObject", - Container::RetiredCert01 => "RetiredCert01", - Container::RetiredCert02 => "RetiredCert02", - Container::RetiredCert03 => "RetiredCert03", - Container::RetiredCert04 => "RetiredCert04", - Container::RetiredCert05 => "RetiredCert05", - Container::RetiredCert06 => "RetiredCert06", - Container::RetiredCert07 => "RetiredCert07", - Container::RetiredCert08 => "RetiredCert08", - Container::RetiredCert09 => "RetiredCert09", - Container::RetiredCert10 => "RetiredCert10", - Container::RetiredCert11 => "RetiredCert11", - Container::RetiredCert12 => "RetiredCert12", - Container::RetiredCert13 => "RetiredCert13", - Container::RetiredCert14 => "RetiredCert14", - Container::RetiredCert15 => "RetiredCert15", - Container::RetiredCert16 => "RetiredCert16", - Container::RetiredCert17 => "RetiredCert17", - Container::RetiredCert18 => "RetiredCert18", - Container::RetiredCert19 => "RetiredCert19", - Container::RetiredCert20 => "RetiredCert20", - Container::CardholderIrisImages => "CardholderIrisImages", + fn path_key_str(self) -> &'static str { + match self.0 { + Container::CardCapabilityContainer => "CardCapabilityContainer.key", + Container::CardHolderUniqueIdentifier => "CardHolderUniqueIdentifier.key", + Container::X509CertificateFor9A => "X509CertificateFor9A.key", + Container::CardholderFingerprints => "CardholderFingerprints.key", + Container::SecurityObject => "SecurityObject.key", + Container::CardholderFacialImage => "CardholderFacialImage.key", + Container::X509CertificateFor9E => "X509CertificateFor9E.key", + Container::X509CertificateFor9C => "X509CertificateFor9C.key", + Container::X509CertificateFor9D => "X509CertificateFor9D.key", + Container::PrintedInformation => "PrintedInformation.key", + Container::DiscoveryObject => "DiscoveryObject.key", + Container::KeyHistoryObject => "KeyHistoryObject.key", + Container::RetiredCert01 => "RetiredCert01.key", + Container::RetiredCert02 => "RetiredCert02.key", + Container::RetiredCert03 => "RetiredCert03.key", + Container::RetiredCert04 => "RetiredCert04.key", + Container::RetiredCert05 => "RetiredCert05.key", + Container::RetiredCert06 => "RetiredCert06.key", + Container::RetiredCert07 => "RetiredCert07.key", + Container::RetiredCert08 => "RetiredCert08.key", + Container::RetiredCert09 => "RetiredCert09.key", + Container::RetiredCert10 => "RetiredCert10.key", + Container::RetiredCert11 => "RetiredCert11.key", + Container::RetiredCert12 => "RetiredCert12.key", + Container::RetiredCert13 => "RetiredCert13.key", + Container::RetiredCert14 => "RetiredCert14.key", + Container::RetiredCert15 => "RetiredCert15.key", + Container::RetiredCert16 => "RetiredCert16.key", + Container::RetiredCert17 => "RetiredCert17.key", + Container::RetiredCert18 => "RetiredCert18.key", + Container::RetiredCert19 => "RetiredCert19.key", + Container::RetiredCert20 => "RetiredCert20.key", + Container::CardholderIrisImages => "CardholderIrisImages.key", Container::BiometricInformationTemplatesGroupTemplate => { - "BiometricInformationTemplatesGroupTemplate" + "BiometricInformationTemplatesGroupTemplate.key" } - Container::SecureMessagingCertificateSigner => "SecureMessagingCertificateSigner", - Container::PairingCodeReferenceDataContainer => "PairingCodeReferenceDataContainer", - }) + Container::SecureMessagingCertificateSigner => "SecureMessagingCertificateSigner.key", + Container::PairingCodeReferenceDataContainer => "PairingCodeReferenceDataContainer.key", + } + } + + fn path_key(self) -> PathBuf { + PathBuf::from(self.path_key_str()) + } + + fn path(self) -> PathBuf { + PathBuf::from(self.path_key_str().strip_suffix(".key").unwrap()) } fn default(self) -> Option> { @@ -894,8 +949,23 @@ impl ContainerStorage { client: &mut impl crate::Client, storage: Location, mut reply: Reply<'_, R>, + read_valid: ReadValid, ) -> Result { - if load_if_exists_streaming(client, storage, &self.path(), reply.lend())? { + let encryption = match read_valid { + ReadValid::Plain => None, + ReadValid::Encrypted(key) => Some(key), + ReadValid::EncryptedNotFound => { + if let Some(data) = self.default() { + reply.append_len(data.len())?; + reply.expand(&data)?; + return Ok(true); + } else { + return Ok(false); + } + } + }; + + if load_if_exists_streaming(client, storage, &self.path(), reply.lend(), encryption)? { return Ok(true); } @@ -914,9 +984,63 @@ impl ContainerStorage { bytes: &[u8], storage: Location, ) -> Result<(), Status> { - utils::write_all(client, storage, self.path(), bytes, None, None).map_err(|_err| { + match self.0.contact_access_rule() { + ReadAccessRule::Always => { + utils::write_all(client, storage, self.path(), bytes, None, None).map_err(|_err| { + error!("Failed to write data object: {:?}", _err); + Status::UnspecifiedNonpersistentExecutionError + }) + } + ReadAccessRule::PinOrOcc | ReadAccessRule::Pin => { + self.save_encrypted(client, bytes, storage) + } + } + } + + fn save_encrypted( + self, + client: &mut impl crate::Client, + bytes: &[u8], + storage: Location, + ) -> Result<(), Status> { + let user_public_key_data = + syscall!(client.read_file(storage, PathBuf::from(USER_PUBLIC_KEY))).data; + let user_public_key = syscall!(client.deserialize_key( + Mechanism::X255, + &user_public_key_data, + KeySerialization::Raw, + StorageAttributes::new().set_persistence(Location::Volatile) + )) + .key; + let key_to_seal = syscall!(client.generate_secret_key(32, Location::Volatile)).key; + syscall!(client.hpke_seal_key_to_file( + self.path_key(), + storage, + user_public_key, + key_to_seal, + Bytes::from_slice(self.path_key_str().as_bytes()).unwrap(), + Bytes::from_slice(HPKE_SEALKEY_CONTAINER_INFO).unwrap(), + )); + syscall!(client.delete(user_public_key)); + + utils::write_all( + client, + storage, + self.path(), + bytes, + None, + Some(utils::EncryptionData { + key: key_to_seal, + // Nonce can be none since the key is only used to encrypt once + nonce: None, + }), + ) + .map_err(|_err| { error!("Failed to write data object: {:?}", _err); Status::UnspecifiedNonpersistentExecutionError - }) + })?; + + syscall!(client.delete(key_to_seal)); + Ok(()) } } diff --git a/tests/command_response.ron b/tests/command_response.ron index ba4f22f..40a4792 100644 --- a/tests/command_response.ron +++ b/tests/command_response.ron @@ -250,4 +250,37 @@ ) ] ), + IoTest( + name: "Protected DOS", + uuid_config: WithBoth("00112233445566778899AABBCCDDEEFF"), + cmd_resp: [ + GetData( + input: "5C 03 5FC108", + expected_status: SecurityStatusNotSatisfied, + ), + AuthenticateManagement( + key: ( + algorithm: Tdes, + key: "0102030405060708 0102030405060708 0102030405060708" + ) + ), + GetData( + input: "5C 03 5FC108", + expected_status: SecurityStatusNotSatisfied, + ), + VerifyApplicationPin(), + GetData( + input: "5C 03 5FC108", + output: All(), + expected_status: NotFound, + ), + PutData( + input: "5C 03 5FC108 53 10 000102030405060708090A0B0C0D0E0F", + ), + GetData( + input: "5C 03 5FC108", + output: Data("53 10 000102030405060708090A0B0C0D0E0F"), + ), + ], + ), ] From f72154bd637f36796a27f24fb2bc52d50973d540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 5 Aug 2024 16:54:59 +0200 Subject: [PATCH 03/18] Fix warnings --- src/lib.rs | 13 ++++++------- src/state.rs | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a7b5ec3..aab05a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,7 +48,6 @@ use reply::Reply; use state::{AdministrationAlgorithm, CommandCache, KeyWithAlg, LoadedState, State, TouchPolicy}; use crate::container::SecurityCondition; -use crate::tlv::get_do; #[derive(Debug, Clone, PartialEq, Eq)] pub struct Options { @@ -311,7 +310,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { if let commands::VerifyLogin::PivPin(pin) = login { self.state .volatile - .verify_pin(&pin, self.trussed, &self.options); + .verify_pin(&pin, self.trussed, self.options); if self.state.volatile.pin_verified() { Ok(()) } else { @@ -371,7 +370,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { } self.state .volatile - .verify_pin(&new_pin, self.trussed, &self.options); + .verify_pin(&new_pin, self.trussed, self.options); Ok(()) } @@ -945,7 +944,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { let read_valid = self.state .volatile - .read_valid_key(container, self.trussed, &self.options)?; + .read_valid_key(container, self.trussed, self.options)?; use state::ContainerStorage; let tag = match container { @@ -1064,9 +1063,9 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { #[cfg(feature = "rsa")] (AsymmetricAlgorithms::Rsa2048, AsymmetricKeyReference::PivAuthentication) => { use trussed_rsa_alloc::RsaImportFormat; - let p = get_do(&[0x01], data).ok_or(Status::IncorrectDataParameter)?; - let q = get_do(&[0x02], data).ok_or(Status::IncorrectDataParameter)?; - let e = get_do(&[0x03], data).ok_or(Status::IncorrectDataParameter)?; + let p = tlv::get_do(&[0x01], data).ok_or(Status::IncorrectDataParameter)?; + let q = tlv::get_do(&[0x02], data).ok_or(Status::IncorrectDataParameter)?; + let e = tlv::get_do(&[0x03], data).ok_or(Status::IncorrectDataParameter)?; let id = syscall!(self.trussed.unsafe_inject_key( trussed::types::Mechanism::Rsa2048Raw, &RsaImportFormat { e, p, q }.serialize().map_err(|_err| { diff --git a/src/state.rs b/src/state.rs index 3d7be9d..e108c25 100644 --- a/src/state.rs +++ b/src/state.rs @@ -658,7 +658,7 @@ impl Persistent { syscall!(client.write_file( options.storage, PathBuf::from(USER_PUBLIC_KEY), - Bytes::from_slice(&*key).unwrap(), + Bytes::from_slice(&key).unwrap(), None )); From 23394d86e96130d0b3bc102598d8d51780d24acf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 23 Aug 2024 17:54:06 +0200 Subject: [PATCH 04/18] This time for keys It works? First try? --- src/container.rs | 234 ++++++++++++++++++++++++++++++++++-- src/lib.rs | 110 +++++++++-------- src/piv_types.rs | 1 + src/state.rs | 303 +++++++++++++++++++++++++++++++++++------------ 4 files changed, 513 insertions(+), 135 deletions(-) diff --git a/src/container.rs b/src/container.rs index 338a244..f03a6e2 100644 --- a/src/container.rs +++ b/src/container.rs @@ -2,6 +2,8 @@ use core::convert::TryFrom; use hex_literal::hex; +use littlefs2::{path, path::Path}; + macro_rules! enum_subset { ( @@ -77,7 +79,19 @@ macro_rules! enum_subset { } } +macro_rules! impl_transitive_into ( + ($leaf:ident, $($interemediary:ident),+ => $grandfather:ident) => { + impl From<$leaf> for $grandfather { + fn from(value: $leaf) -> $grandfather { + $(let value = $interemediary::from(value);)+ + $grandfather::from(value) + } + } + } +); + pub(crate) use enum_subset; +use trussed::types::Location; /// Security condition for the use of a given key. pub enum SecurityCondition { @@ -87,8 +101,23 @@ pub enum SecurityCondition { Always, } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct RetiredIndex(u8); +/// Security condition for the use of a given key. +pub enum KeySecurityCondition { + Pin(&'static Path), + /// Pin must be checked **just before** + PinAlways(&'static Path), + Always, +} + +impl From for SecurityCondition { + fn from(value: KeySecurityCondition) -> Self { + match value { + KeySecurityCondition::Pin(_) => SecurityCondition::Pin, + KeySecurityCondition::PinAlways(_) => SecurityCondition::PinAlways, + KeySecurityCondition::Always => SecurityCondition::Always, + } + } +} crate::enum_u8! { #[derive(Debug)] @@ -131,26 +160,81 @@ crate::enum_u8! { } impl KeyReference { + #[deprecated] pub fn use_security_condition(self) -> SecurityCondition { + self.use_key_security_condition().into() + } + + pub fn use_key_security_condition(self) -> KeySecurityCondition { match self { Self::SecureMessaging | Self::CardAuthentication | Self::PivCardApplicationAdministration - | Self::KeyManagement => SecurityCondition::Always, - Self::DigitalSignature => SecurityCondition::PinAlways, - _ => SecurityCondition::Pin, + | Self::KeyManagement => KeySecurityCondition::Always, + Self::DigitalSignature => KeySecurityCondition::PinAlways(self.name()), + _ => KeySecurityCondition::Pin(self.name()), + } + } + + pub fn name(self) -> &'static Path { + match self { + Self::GlobalPin => path!("GlobalPin"), + Self::SecureMessaging => path!("SecureMessaging"), + Self::ApplicationPin => path!("ApplicationPin"), + Self::PinUnblockingKey => path!("PinUnblockingKey"), + Self::PrimaryFinger => path!("PrimaryFinger"), + Self::SecondaryFinger => path!("SecondaryFinger"), + Self::PairingCode => path!("PairingCode"), + + Self::PivAuthentication => path!("PivAuthentication"), + Self::PivCardApplicationAdministration => path!("PivCardApplicationAdministration"), + Self::DigitalSignature => path!("DigitalSignature"), + Self::KeyManagement => path!("KeyManagement"), + Self::CardAuthentication => path!("CardAuthentication"), + + Self::Retired01 => path!("Retired01"), + Self::Retired02 => path!("Retired02"), + Self::Retired03 => path!("Retired03"), + Self::Retired04 => path!("Retired04"), + Self::Retired05 => path!("Retired05"), + Self::Retired06 => path!("Retired06"), + Self::Retired07 => path!("Retired07"), + Self::Retired08 => path!("Retired08"), + Self::Retired09 => path!("Retired09"), + Self::Retired10 => path!("Retired10"), + Self::Retired11 => path!("Retired11"), + Self::Retired12 => path!("Retired12"), + Self::Retired13 => path!("Retired13"), + Self::Retired14 => path!("Retired14"), + Self::Retired15 => path!("Retired15"), + Self::Retired16 => path!("Retired16"), + Self::Retired17 => path!("Retired17"), + Self::Retired18 => path!("Retired18"), + Self::Retired19 => path!("Retired19"), + Self::Retired20 => path!("Retired20"), } } } -macro_rules! impl_use_security_condition { - ($($name:ident),*) => { +macro_rules! impl_sub_enum_methods { + ($($name:ident,)*) => { $( impl $name { + #[deprecated] pub fn use_security_condition(self) -> SecurityCondition { let tmp: KeyReference = self.into(); + #[allow(deprecated)] tmp.use_security_condition() } + pub fn use_key_security_condition(self) -> KeySecurityCondition { + let tmp: KeyReference = self.into(); + tmp.use_key_security_condition() + } + + pub fn name(self) -> &'static Path { + let tmp: KeyReference = self.into(); + tmp.name() + } } )* }; @@ -191,7 +275,137 @@ enum_subset! { Retired18, Retired19, Retired20, + } +} + +enum AsymmetricKeyMaybeProtected { + Unprocteced(UnprotectedAsymmetricKeyReference), + Protected(ProtectedAsymmetricKeyReference), +} + +impl AsymmetricKeyReference { + /// Get the location to store a new key (important for keys that are encrypted and should be generated in volatile stoarge) + pub fn storage(self, storage: Location) -> Location { + match self { + Self::CardAuthentication | Self::KeyManagement => storage, + _ => Location::Volatile, + } + } + pub fn encrypted_or_plain(self) -> AsymmetricKeyMaybeProtected { + match self { + Self::PivAuthentication => AsymmetricKeyMaybeProtected::Protected( + ProtectedAsymmetricKeyReference::PivAuthentication, + ), + Self::DigitalSignature => AsymmetricKeyMaybeProtected::Protected( + ProtectedAsymmetricKeyReference::DigitalSignature, + ), + Self::KeyManagement => AsymmetricKeyMaybeProtected::Unprocteced( + UnprotectedAsymmetricKeyReference::KeyManagement, + ), + Self::CardAuthentication => AsymmetricKeyMaybeProtected::Unprocteced( + UnprotectedAsymmetricKeyReference::CardAuthentication, + ), + Self::Retired01 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired01) + } + Self::Retired02 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired02) + } + Self::Retired03 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired03) + } + Self::Retired04 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired04) + } + Self::Retired05 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired05) + } + Self::Retired06 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired06) + } + Self::Retired07 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired07) + } + Self::Retired08 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired08) + } + Self::Retired09 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired09) + } + Self::Retired10 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired10) + } + Self::Retired11 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired11) + } + Self::Retired12 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired12) + } + Self::Retired13 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired13) + } + Self::Retired14 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired14) + } + Self::Retired15 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired15) + } + Self::Retired16 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired16) + } + Self::Retired17 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired17) + } + Self::Retired18 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired18) + } + Self::Retired19 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired19) + } + Self::Retired20 => { + AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired20) + } + } + } +} + +enum_subset! { + #[derive(Debug)] + pub enum UnprotectedAsymmetricKeyReference: AsymmetricKeyReference { + KeyManagement, + CardAuthentication, + } +} + +impl_transitive_into!(UnprotectedAsymmetricKeyReference, AsymmetricKeyReference => KeyReference); +impl_transitive_into!(ProtectedAsymmetricKeyReference, AsymmetricKeyReference => KeyReference); + +enum_subset! { + #[derive(Debug)] + pub enum ProtectedAsymmetricKeyReference: AsymmetricKeyReference { + PivAuthentication, + DigitalSignature, + Retired01, + Retired02, + Retired03, + Retired04, + Retired05, + Retired06, + Retired07, + Retired08, + Retired09, + Retired10, + Retired11, + Retired12, + Retired13, + Retired14, + Retired15, + Retired16, + Retired17, + Retired18, + Retired19, + Retired20, } } @@ -251,12 +465,14 @@ enum_subset! { } } -impl_use_security_condition!( +impl_sub_enum_methods!( AttestKeyReference, AsymmetricKeyReference, + UnprotectedAsymmetricKeyReference, + ProtectedAsymmetricKeyReference, ChangeReferenceKeyReference, VerifyKeyReference, - AuthenticateKeyReference + AuthenticateKeyReference, ); macro_rules! impl_try_from { diff --git a/src/lib.rs b/src/lib.rs index aab05a2..37b2c2d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -697,44 +697,55 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { warn!("Attempt to sign with an incorrect key"); return Err(Status::IncorrectP1OrP2Parameter); }; - let Some(KeyWithAlg { alg, id }) = - self.state.persistent.keys.asymetric_for_reference(key_ref) + + let Some(key) = self + .state + .use_valid_key(key_ref, self.trussed, self.options)? else { - warn!("Attempt to use unset key"); return Err(Status::ConditionsOfUseNotSatisfied); }; - if alg != auth.algorithm { - warn!("Bad algorithm: {:?}", auth.algorithm); - return Err(Status::IncorrectP1OrP2Parameter); - } - match key_ref.use_security_condition() { - SecurityCondition::Pin => { - if !self.state.volatile.pin_verified() { - warn!("Authenticate challenge without pin validated"); - return Err(Status::SecurityStatusNotSatisfied); - } - } - SecurityCondition::PinAlways => { - if !just_verified { - warn!("authenticate challenge without pin validated"); - return Err(Status::SecurityStatusNotSatisfied); - } - } - SecurityCondition::Always => {} - } + // let Some(KeyWithAlg { alg, id }) = + // self.state.persistent.keys.asymetric_for_reference(key_ref) + // else { + // warn!("Attempt to use unset key"); + // return Err(Status::ConditionsOfUseNotSatisfied); + // }; + + // if alg != auth.algorithm { + // warn!("Bad algorithm: {:?}", auth.algorithm); + // return Err(Status::IncorrectP1OrP2Parameter); + // } + // match key_ref.use_key_security_condition() { + // SecurityCondition::Pin => { + // if !self.state.volatile.pin_verified() { + // warn!("Authenticate challenge without pin validated"); + // return Err(Status::SecurityStatusNotSatisfied); + // } + // } + // SecurityCondition::PinAlways => { + // if !just_verified { + // warn!("authenticate challenge without pin validated"); + // return Err(Status::SecurityStatusNotSatisfied); + // } + // } + // SecurityCondition::Always => {} + // } - if alg.sign_len() != message.len() { + if key.alg.sign_len() != message.len() { return Err(Status::IncorrectDataParameter); } let response = syscall!(self.trussed.sign( - alg.sign_mechanism(), - id, + key.alg.sign_mechanism(), + key.key, message, - alg.sign_serialization(), + key.alg.sign_serialization(), )) .signature; + + key.clear(self.trussed); + reply.expand(&[0x7C])?; let offset = reply.len(); { @@ -763,22 +774,19 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { ); Status::IncorrectP1OrP2Parameter })?; - let Some(KeyWithAlg { alg, id }) = self + let Some(key) = self .state - .persistent - .keys - .asymetric_for_reference(key_reference) + .use_valid_key(key_reference, self.trussed, self.options)? else { - warn!("Attempt to use unset key"); return Err(Status::ConditionsOfUseNotSatisfied); }; - if alg != auth.algorithm { + if key.alg != auth.algorithm { warn!("Attempt to exponentiate with incorrect algorithm"); return Err(Status::IncorrectP1OrP2Parameter); } - let Some(mechanism) = alg.ecdh_mechanism() else { + let Some(mechanism) = key.alg.ecdh_mechanism() else { warn!("Attempt to exponentiate with non ECDH algorithm"); return Err(Status::ConditionsOfUseNotSatisfied); }; @@ -801,7 +809,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { .key; let shared_secret = syscall!(self.trussed.agree( mechanism, - id, + key.key, public_key, StorageAttributes::default() .set_persistence(Location::Volatile) @@ -817,6 +825,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { .serialized_key; syscall!(self.trussed.delete(public_key)); syscall!(self.trussed.delete(shared_secret)); + key.clear(self.trussed); reply.expand(&[0x7C])?; let offset = reply.len(); @@ -879,6 +888,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { reference, parsed_mechanism, self.trussed, + self.options.storage, ); let public_key = syscall!(self.trussed.derive_key( @@ -1016,14 +1026,16 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { } fn get_key_history_object(&mut self, mut reply: Reply<'_, R>) -> Result { - let num_keys = self - .state - .persistent - .keys - .retired_keys - .iter() - .filter(|k| k.is_some()) - .count() as u8; + // let num_keys = self + // .state + // .persistent + // .keys + // .retired_keys + // .iter() + // .filter(|k| k.is_some()) + // .count() as u8; + // TODO: fix count + let num_keys: u8 = 0u8; let mut num_certs = 0u8; use state::ContainerStorage; @@ -1046,7 +1058,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { &mut self, algo: AsymmetricAlgorithms, key: AsymmetricKeyReference, - data: &[u8], + #[cfg_attr(not(feature = "rsa"), allow(unused))] data: &[u8], mut _reply: Reply<'_, R>, ) -> Result { if !self @@ -1072,13 +1084,17 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { error!("Failed rsa import serialization: {_err:?}"); Status::UnspecifiedNonpersistentExecutionError })?, - self.options.storage, + AsymmetricKeyReference::PivAuthentication.storage(self.options.storage), KeySerialization::RsaParts )) .key; - self.state - .persistent - .replace_asymmetric_key(key, algo, id, self.trussed); + self.state.persistent.replace_asymmetric_key( + key, + algo, + id, + self.trussed, + self.options.storage, + ); Ok(()) } _ => Err(Status::FunctionNotSupported), diff --git a/src/piv_types.rs b/src/piv_types.rs index ba83894..40a6bb9 100644 --- a/src/piv_types.rs +++ b/src/piv_types.rs @@ -514,6 +514,7 @@ impl CardHolderUniqueIdentifier<'_> { } } +#[allow(unused)] #[derive(Clone, Copy, Encodable, Eq, PartialEq)] #[tlv(application, number = "0x13")] pub struct DiscoveryObject { diff --git a/src/state.rs b/src/state.rs index e108c25..04e86e9 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,11 +1,12 @@ use core::convert::{TryFrom, TryInto}; -use core::mem::replace; +use core::mem::{self, replace}; use flexiber::EncodableHeapless; use heapless::Vec; use heapless_bytes::Bytes; use iso7816::Status; use littlefs2::{path, path::Path}; +use trussed::Client; use trussed::{ api::reply::Metadata, config::MAX_MESSAGE_LENGTH, @@ -14,6 +15,7 @@ use trussed::{ }; use trussed_chunked::utils; +use crate::container::KeySecurityCondition; use crate::piv_types::CardHolderUniqueIdentifier; use crate::reply::Reply; use crate::{constants::*, piv_types::AsymmetricAlgorithms}; @@ -34,6 +36,9 @@ const USER_PUBLIC_KEY: &Path = path!("user-public-key.bin"); /// Info parameter for the container storage const HPKE_SEALKEY_CONTAINER_INFO: &[u8] = b"Container Storage"; +/// Info parameter for the container storage +const HPKE_SEALKEY_REFERENCE_INFO: &[u8] = b"Key Storage"; + pub enum PinPolicy { Never, Once, @@ -84,6 +89,12 @@ pub struct KeyWithAlg { pub alg: A, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum KeyOrEncryptedWithAlg { + Plain(Option>), + Encrypted(Option), +} + macro_rules! generate_into_key_with_alg { ($($name:ident),*) => { $( @@ -103,14 +114,14 @@ generate_into_key_with_alg!(AsymmetricAlgorithms, AdministrationAlgorithm); #[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] pub struct Keys { - // 9a "PIV Authentication Key" (YK: PIV Authentication) - pub authentication: KeyWithAlg, + // // 9a "PIV Authentication Key" (YK: PIV Authentication) + pub authentication_alg: AsymmetricAlgorithms, // 9b "PIV Card Application Administration Key" (YK: PIV Management) pub administration: KeyWithAlg, pub is_admin_default: bool, // 9c "Digital Signature Key" (YK: Digital Signature) #[serde(skip_serializing_if = "Option::is_none")] - pub signature: Option>, + pub signature_alg: Option, // 9d "Key Management Key" (YK: Key Management) #[serde(skip_serializing_if = "Option::is_none")] pub key_management: Option>, @@ -118,40 +129,41 @@ pub struct Keys { #[serde(skip_serializing_if = "Option::is_none")] pub card_authentication: Option>, // 0x82..=0x95 (130-149) - pub retired_keys: [Option>; 20], - // pub secure_messaging + pub retired_keys: [Option; 20], + // TODO secure_messaging } impl Keys { pub fn asymetric_for_reference( &self, key: AsymmetricKeyReference, - ) -> Option> { + ) -> KeyOrEncryptedWithAlg { + use KeyOrEncryptedWithAlg::{Encrypted, Plain}; match key { - AsymmetricKeyReference::PivAuthentication => Some(self.authentication), - AsymmetricKeyReference::DigitalSignature => self.signature, - AsymmetricKeyReference::KeyManagement => self.key_management, - AsymmetricKeyReference::CardAuthentication => self.card_authentication, - AsymmetricKeyReference::Retired01 => self.retired_keys[0], - AsymmetricKeyReference::Retired02 => self.retired_keys[1], - AsymmetricKeyReference::Retired03 => self.retired_keys[2], - AsymmetricKeyReference::Retired04 => self.retired_keys[3], - AsymmetricKeyReference::Retired05 => self.retired_keys[4], - AsymmetricKeyReference::Retired06 => self.retired_keys[5], - AsymmetricKeyReference::Retired07 => self.retired_keys[6], - AsymmetricKeyReference::Retired08 => self.retired_keys[7], - AsymmetricKeyReference::Retired09 => self.retired_keys[8], - AsymmetricKeyReference::Retired10 => self.retired_keys[9], - AsymmetricKeyReference::Retired11 => self.retired_keys[10], - AsymmetricKeyReference::Retired12 => self.retired_keys[11], - AsymmetricKeyReference::Retired13 => self.retired_keys[12], - AsymmetricKeyReference::Retired14 => self.retired_keys[13], - AsymmetricKeyReference::Retired15 => self.retired_keys[14], - AsymmetricKeyReference::Retired16 => self.retired_keys[15], - AsymmetricKeyReference::Retired17 => self.retired_keys[16], - AsymmetricKeyReference::Retired18 => self.retired_keys[17], - AsymmetricKeyReference::Retired19 => self.retired_keys[18], - AsymmetricKeyReference::Retired20 => self.retired_keys[19], + AsymmetricKeyReference::PivAuthentication => Encrypted(Some(self.authentication_alg)), + AsymmetricKeyReference::DigitalSignature => Encrypted(self.signature_alg), + AsymmetricKeyReference::KeyManagement => Plain(self.key_management), + AsymmetricKeyReference::CardAuthentication => Plain(self.card_authentication), + AsymmetricKeyReference::Retired01 => Encrypted(self.retired_keys[0]), + AsymmetricKeyReference::Retired02 => Encrypted(self.retired_keys[1]), + AsymmetricKeyReference::Retired03 => Encrypted(self.retired_keys[2]), + AsymmetricKeyReference::Retired04 => Encrypted(self.retired_keys[3]), + AsymmetricKeyReference::Retired05 => Encrypted(self.retired_keys[4]), + AsymmetricKeyReference::Retired06 => Encrypted(self.retired_keys[5]), + AsymmetricKeyReference::Retired07 => Encrypted(self.retired_keys[6]), + AsymmetricKeyReference::Retired08 => Encrypted(self.retired_keys[7]), + AsymmetricKeyReference::Retired09 => Encrypted(self.retired_keys[8]), + AsymmetricKeyReference::Retired10 => Encrypted(self.retired_keys[9]), + AsymmetricKeyReference::Retired11 => Encrypted(self.retired_keys[10]), + AsymmetricKeyReference::Retired12 => Encrypted(self.retired_keys[11]), + AsymmetricKeyReference::Retired13 => Encrypted(self.retired_keys[12]), + AsymmetricKeyReference::Retired14 => Encrypted(self.retired_keys[13]), + AsymmetricKeyReference::Retired15 => Encrypted(self.retired_keys[14]), + AsymmetricKeyReference::Retired16 => Encrypted(self.retired_keys[15]), + AsymmetricKeyReference::Retired17 => Encrypted(self.retired_keys[16]), + AsymmetricKeyReference::Retired18 => Encrypted(self.retired_keys[17]), + AsymmetricKeyReference::Retired19 => Encrypted(self.retired_keys[18]), + AsymmetricKeyReference::Retired20 => Encrypted(self.retired_keys[19]), } } @@ -159,35 +171,78 @@ impl Keys { &mut self, key: AsymmetricKeyReference, new: KeyWithAlg, - ) -> Option> { - match key { - AsymmetricKeyReference::PivAuthentication => { - Some(replace(&mut self.authentication, new)) + storage: Location, + client: &mut impl crate::Client, + ) -> KeyOrEncryptedWithAlg { + let mut user_public_key = None; + let mut get_user_public_key = || { + let user_public_key_data = + syscall!(client.read_file(storage, PathBuf::from(USER_PUBLIC_KEY))).data; + let key_id = syscall!(client.deserialize_key( + Mechanism::X255, + &user_public_key_data, + KeySerialization::Raw, + StorageAttributes::new().set_persistence(Location::Volatile) + )) + .key; + assert!(user_public_key.is_none()); + user_public_key = Some(key_id); + key_id + }; + + let old = match key { + AsymmetricKeyReference::KeyManagement => { + KeyOrEncryptedWithAlg::Plain(self.key_management.replace(new)) + } + AsymmetricKeyReference::CardAuthentication => { + KeyOrEncryptedWithAlg::Plain(self.card_authentication.replace(new)) + } + _ => { + let user_public_key = get_user_public_key(); + syscall!(client.hpke_seal_key_to_file( + PathBuf::from(key.name()), + storage, + user_public_key, + new.id, + Bytes::from_slice(key.name().as_str().as_bytes()).unwrap(), + Bytes::from_slice(HPKE_SEALKEY_REFERENCE_INFO).unwrap(), + )); + + KeyOrEncryptedWithAlg::Encrypted(match key { + AsymmetricKeyReference::PivAuthentication => { + Some(mem::replace(&mut self.authentication_alg, new.alg)) + } + AsymmetricKeyReference::DigitalSignature => self.signature_alg.replace(new.alg), + AsymmetricKeyReference::Retired01 => self.retired_keys[01].replace(new.alg), + AsymmetricKeyReference::Retired02 => self.retired_keys[02].replace(new.alg), + AsymmetricKeyReference::Retired03 => self.retired_keys[03].replace(new.alg), + AsymmetricKeyReference::Retired04 => self.retired_keys[04].replace(new.alg), + AsymmetricKeyReference::Retired05 => self.retired_keys[05].replace(new.alg), + AsymmetricKeyReference::Retired06 => self.retired_keys[06].replace(new.alg), + AsymmetricKeyReference::Retired07 => self.retired_keys[07].replace(new.alg), + AsymmetricKeyReference::Retired08 => self.retired_keys[08].replace(new.alg), + AsymmetricKeyReference::Retired09 => self.retired_keys[09].replace(new.alg), + AsymmetricKeyReference::Retired10 => self.retired_keys[10].replace(new.alg), + AsymmetricKeyReference::Retired11 => self.retired_keys[11].replace(new.alg), + AsymmetricKeyReference::Retired12 => self.retired_keys[12].replace(new.alg), + AsymmetricKeyReference::Retired13 => self.retired_keys[13].replace(new.alg), + AsymmetricKeyReference::Retired14 => self.retired_keys[14].replace(new.alg), + AsymmetricKeyReference::Retired15 => self.retired_keys[15].replace(new.alg), + AsymmetricKeyReference::Retired16 => self.retired_keys[16].replace(new.alg), + AsymmetricKeyReference::Retired17 => self.retired_keys[17].replace(new.alg), + AsymmetricKeyReference::Retired18 => self.retired_keys[18].replace(new.alg), + AsymmetricKeyReference::Retired19 => self.retired_keys[19].replace(new.alg), + AsymmetricKeyReference::Retired20 => self.retired_keys[20].replace(new.alg), + AsymmetricKeyReference::KeyManagement + | AsymmetricKeyReference::CardAuthentication => unreachable!(), + }) } - AsymmetricKeyReference::DigitalSignature => self.signature.replace(new), - AsymmetricKeyReference::KeyManagement => self.key_management.replace(new), - AsymmetricKeyReference::CardAuthentication => self.card_authentication.replace(new), - AsymmetricKeyReference::Retired01 => self.retired_keys[0].replace(new), - AsymmetricKeyReference::Retired02 => self.retired_keys[1].replace(new), - AsymmetricKeyReference::Retired03 => self.retired_keys[2].replace(new), - AsymmetricKeyReference::Retired04 => self.retired_keys[3].replace(new), - AsymmetricKeyReference::Retired05 => self.retired_keys[4].replace(new), - AsymmetricKeyReference::Retired06 => self.retired_keys[5].replace(new), - AsymmetricKeyReference::Retired07 => self.retired_keys[6].replace(new), - AsymmetricKeyReference::Retired08 => self.retired_keys[7].replace(new), - AsymmetricKeyReference::Retired09 => self.retired_keys[8].replace(new), - AsymmetricKeyReference::Retired10 => self.retired_keys[9].replace(new), - AsymmetricKeyReference::Retired11 => self.retired_keys[10].replace(new), - AsymmetricKeyReference::Retired12 => self.retired_keys[11].replace(new), - AsymmetricKeyReference::Retired13 => self.retired_keys[12].replace(new), - AsymmetricKeyReference::Retired14 => self.retired_keys[13].replace(new), - AsymmetricKeyReference::Retired15 => self.retired_keys[14].replace(new), - AsymmetricKeyReference::Retired16 => self.retired_keys[15].replace(new), - AsymmetricKeyReference::Retired17 => self.retired_keys[16].replace(new), - AsymmetricKeyReference::Retired18 => self.retired_keys[17].replace(new), - AsymmetricKeyReference::Retired19 => self.retired_keys[18].replace(new), - AsymmetricKeyReference::Retired20 => self.retired_keys[19].replace(new), + }; + + if let Some(key_id) = user_public_key { + syscall!(client.delete(key_id)); } + old } } @@ -231,6 +286,81 @@ pub struct LoadedState<'t> { pub persistent: &'t mut Persistent, } +pub struct UseValidKey { + pub key: KeyId, + pub alg: AsymmetricAlgorithms, + pub need_clear: bool, +} + +impl UseValidKey { + pub fn clear(mut self, client: &mut impl Client) { + if self.need_clear { + syscall!(client.clear(self.key)); + } + self.need_clear = false; + } +} + +impl Drop for UseValidKey { + fn drop(&mut self) { + assert!(!self.need_clear, "Memory leak of sensitive data") + } +} + +impl<'t> LoadedState<'t> { + pub fn use_valid_key( + &mut self, + key: AsymmetricKeyReference, + client: &mut impl crate::Client, + options: &crate::Options, + ) -> Result, Status> { + let security_condition = key.use_security_condition(); + match security_condition { + SecurityCondition::PinAlways if self.volatile.app_security_status.pin_just_verified => { + } + SecurityCondition::Pin if self.volatile.app_security_status.pin_verified.is_some() => {} + SecurityCondition::Always => {} + _ => return Err(Status::SecurityStatusNotSatisfied), + }; + + let key_with_alg = self.persistent.keys.asymetric_for_reference(key); + let alg = match key_with_alg { + KeyOrEncryptedWithAlg::Plain(None) => return Ok(None), + KeyOrEncryptedWithAlg::Plain(Some(KeyWithAlg { id, alg })) => { + return Ok(Some(UseValidKey { + key: id, + alg, + need_clear: false, + })) + } + KeyOrEncryptedWithAlg::Encrypted(None) => return Ok(None), + KeyOrEncryptedWithAlg::Encrypted(Some(alg)) => alg, + }; + + let pin_key = self.volatile.app_security_status.pin_verified.unwrap(); + + let unsealed_key = try_syscall!(client.hpke_open_key_from_file( + pin_key, + key.name().into(), + options.storage, + Location::Volatile, + Bytes::from_slice(key.name().as_str().as_bytes()).unwrap(), + Bytes::from_slice(HPKE_SEALKEY_REFERENCE_INFO).unwrap(), + )) + .map_err(|_err| { + error!("Failed to unseal key: {_err:?}"); + Status::UnspecifiedNonpersistentExecutionError + })? + .key; + + Ok(Some(UseValidKey { + key: unsealed_key, + alg, + need_clear: true, + })) + } +} + /// exists only to please serde, which doesn't accept enum variants in `#[serde(default=…)]` fn volatile() -> Location { Location::Volatile @@ -531,9 +661,11 @@ impl Persistent { key: AsymmetricKeyReference, id: KeyId, alg: AsymmetricAlgorithms, - ) -> Option> { + storage: Location, + client: &mut impl crate::Client, + ) -> KeyOrEncryptedWithAlg { self.keys - .set_asymetric_for_reference(key, KeyWithAlg { id, alg }) + .set_asymetric_for_reference(key, KeyWithAlg { id, alg }, storage, client) } pub fn generate_asymmetric_key( @@ -541,16 +673,24 @@ impl Persistent { key: AsymmetricKeyReference, alg: AsymmetricAlgorithms, client: &mut impl crate::Client, + storage: Location, ) -> KeyId { let id = syscall!(client.generate_key( alg.key_mechanism(), - StorageAttributes::default().set_persistence(self.storage) + StorageAttributes::default().set_persistence(key.storage(self.storage)) )) .key; - let old = self.set_asymmetric_key(key, id, alg); + let old = self.set_asymmetric_key(key, id, alg, storage, client); self.save(client); - if let Some(old) = old { - syscall!(client.delete(old.id)); + use KeyOrEncryptedWithAlg::{Encrypted, Plain}; + match old { + Plain(None) => {} + Plain(Some(KeyWithAlg { id, alg: _ })) => { + syscall!(client.delete(id)); + } + Encrypted(_) => { + // Old file was already overwritten + } } id } @@ -561,11 +701,18 @@ impl Persistent { alg: AsymmetricAlgorithms, id: KeyId, client: &mut impl crate::Client, + storage: Location, ) { - let old = self.set_asymmetric_key(key, id, alg); + let old = self.set_asymmetric_key(key, id, alg, storage, client); self.save(client); - if let Some(old) = old { - syscall!(client.delete(old.id)); + use KeyOrEncryptedWithAlg::{Encrypted, Plain}; + match old { + Plain(Some(old)) => { + syscall!(client.delete(old.id)); + } + Plain(None) => {} + // The old file was simply overwritten + Encrypted(_) => {} } } @@ -686,15 +833,7 @@ impl Persistent { alg: YUBICO_DEFAULT_MANAGEMENT_KEY_ALG, }; - let authentication = KeyWithAlg { - id: syscall!(client.generate_key( - Mechanism::P256, - StorageAttributes::new().set_persistence(options.storage) - )) - .key, - alg: AsymmetricAlgorithms::P256, - }; - + let authentication_alg = AsymmetricAlgorithms::P256; let guid = options.uuid.unwrap_or_else(|| { let mut guid: [u8; 16] = syscall!(client.random_bytes(16)) .bytes @@ -720,10 +859,10 @@ impl Persistent { .ok(); let keys = Keys { - authentication, + authentication_alg, administration, is_admin_default: true, - signature: None, + signature_alg: None, key_management: None, card_authentication: None, retired_keys: Default::default(), @@ -734,7 +873,13 @@ impl Persistent { timestamp: 0, storage: options.storage, }; - state.save(client); + state.generate_asymmetric_key( + AsymmetricKeyReference::CardAuthentication, + authentication_alg, + client, + options.storage, + ); + Self::init_pins(client, options)?; Ok(state) } From 0579bd03f80b80db02a2dc50d8d98962fb121f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 26 Aug 2024 11:06:48 +0200 Subject: [PATCH 05/18] Fix warnings --- src/container.rs | 150 +---------------------------------------------- src/lib.rs | 17 +++--- src/state.rs | 47 ++++++++------- 3 files changed, 34 insertions(+), 180 deletions(-) diff --git a/src/container.rs b/src/container.rs index f03a6e2..238b31c 100644 --- a/src/container.rs +++ b/src/container.rs @@ -79,17 +79,6 @@ macro_rules! enum_subset { } } -macro_rules! impl_transitive_into ( - ($leaf:ident, $($interemediary:ident),+ => $grandfather:ident) => { - impl From<$leaf> for $grandfather { - fn from(value: $leaf) -> $grandfather { - $(let value = $interemediary::from(value);)+ - $grandfather::from(value) - } - } - } -); - pub(crate) use enum_subset; use trussed::types::Location; @@ -162,17 +151,13 @@ crate::enum_u8! { impl KeyReference { #[deprecated] pub fn use_security_condition(self) -> SecurityCondition { - self.use_key_security_condition().into() - } - - pub fn use_key_security_condition(self) -> KeySecurityCondition { match self { Self::SecureMessaging | Self::CardAuthentication | Self::PivCardApplicationAdministration - | Self::KeyManagement => KeySecurityCondition::Always, - Self::DigitalSignature => KeySecurityCondition::PinAlways(self.name()), - _ => KeySecurityCondition::Pin(self.name()), + | Self::KeyManagement => SecurityCondition::Always, + Self::DigitalSignature => SecurityCondition::PinAlways, + _ => SecurityCondition::Pin, } } @@ -220,17 +205,11 @@ macro_rules! impl_sub_enum_methods { ($($name:ident,)*) => { $( impl $name { - #[deprecated] pub fn use_security_condition(self) -> SecurityCondition { let tmp: KeyReference = self.into(); #[allow(deprecated)] tmp.use_security_condition() } - pub fn use_key_security_condition(self) -> KeySecurityCondition { - let tmp: KeyReference = self.into(); - tmp.use_key_security_condition() - } - pub fn name(self) -> &'static Path { let tmp: KeyReference = self.into(); tmp.name() @@ -278,11 +257,6 @@ enum_subset! { } } -enum AsymmetricKeyMaybeProtected { - Unprocteced(UnprotectedAsymmetricKeyReference), - Protected(ProtectedAsymmetricKeyReference), -} - impl AsymmetricKeyReference { /// Get the location to store a new key (important for keys that are encrypted and should be generated in volatile stoarge) pub fn storage(self, storage: Location) -> Location { @@ -291,122 +265,6 @@ impl AsymmetricKeyReference { _ => Location::Volatile, } } - - pub fn encrypted_or_plain(self) -> AsymmetricKeyMaybeProtected { - match self { - Self::PivAuthentication => AsymmetricKeyMaybeProtected::Protected( - ProtectedAsymmetricKeyReference::PivAuthentication, - ), - Self::DigitalSignature => AsymmetricKeyMaybeProtected::Protected( - ProtectedAsymmetricKeyReference::DigitalSignature, - ), - Self::KeyManagement => AsymmetricKeyMaybeProtected::Unprocteced( - UnprotectedAsymmetricKeyReference::KeyManagement, - ), - Self::CardAuthentication => AsymmetricKeyMaybeProtected::Unprocteced( - UnprotectedAsymmetricKeyReference::CardAuthentication, - ), - Self::Retired01 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired01) - } - Self::Retired02 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired02) - } - Self::Retired03 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired03) - } - Self::Retired04 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired04) - } - Self::Retired05 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired05) - } - Self::Retired06 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired06) - } - Self::Retired07 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired07) - } - Self::Retired08 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired08) - } - Self::Retired09 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired09) - } - Self::Retired10 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired10) - } - Self::Retired11 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired11) - } - Self::Retired12 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired12) - } - Self::Retired13 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired13) - } - Self::Retired14 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired14) - } - Self::Retired15 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired15) - } - Self::Retired16 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired16) - } - Self::Retired17 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired17) - } - Self::Retired18 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired18) - } - Self::Retired19 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired19) - } - Self::Retired20 => { - AsymmetricKeyMaybeProtected::Protected(ProtectedAsymmetricKeyReference::Retired20) - } - } - } -} - -enum_subset! { - #[derive(Debug)] - pub enum UnprotectedAsymmetricKeyReference: AsymmetricKeyReference { - KeyManagement, - CardAuthentication, - } -} - -impl_transitive_into!(UnprotectedAsymmetricKeyReference, AsymmetricKeyReference => KeyReference); -impl_transitive_into!(ProtectedAsymmetricKeyReference, AsymmetricKeyReference => KeyReference); - -enum_subset! { - #[derive(Debug)] - pub enum ProtectedAsymmetricKeyReference: AsymmetricKeyReference { - PivAuthentication, - DigitalSignature, - Retired01, - Retired02, - Retired03, - Retired04, - Retired05, - Retired06, - Retired07, - Retired08, - Retired09, - Retired10, - Retired11, - Retired12, - Retired13, - Retired14, - Retired15, - Retired16, - Retired17, - Retired18, - Retired19, - Retired20, - } } pub type GenerateKeyReference = AsymmetricKeyReference; @@ -468,8 +326,6 @@ enum_subset! { impl_sub_enum_methods!( AttestKeyReference, AsymmetricKeyReference, - UnprotectedAsymmetricKeyReference, - ProtectedAsymmetricKeyReference, ChangeReferenceKeyReference, VerifyKeyReference, AuthenticateKeyReference, diff --git a/src/lib.rs b/src/lib.rs index 37b2c2d..d9b8e08 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,8 +47,6 @@ pub type Result = iso7816::Result; use reply::Reply; use state::{AdministrationAlgorithm, CommandCache, KeyWithAlg, LoadedState, State, TouchPolicy}; -use crate::container::SecurityCondition; - #[derive(Debug, Clone, PartialEq, Eq)] pub struct Options { storage: Location, @@ -491,7 +489,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { challenge: None, response: Some([]), exponentiation: Some(p), - } => self.key_agreement(auth, p, reply.lend())?, + } => self.key_agreement(auth, p, reply.lend(), just_verified)?, Auth { witness: None, challenge: Some([]), @@ -698,9 +696,9 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { return Err(Status::IncorrectP1OrP2Parameter); }; - let Some(key) = self - .state - .use_valid_key(key_ref, self.trussed, self.options)? + let Some(key) = + self.state + .use_valid_key(key_ref, self.trussed, self.options, just_verified)? else { return Err(Status::ConditionsOfUseNotSatisfied); }; @@ -765,6 +763,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { auth: GeneralAuthenticate, data: &[u8], mut reply: Reply<'_, R>, + just_verified: bool, ) -> Result { info!("Request for exponentiation"); let key_reference = auth.key_reference.try_into().map_err(|_| { @@ -774,9 +773,9 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { ); Status::IncorrectP1OrP2Parameter })?; - let Some(key) = self - .state - .use_valid_key(key_reference, self.trussed, self.options)? + let Some(key) = + self.state + .use_valid_key(key_reference, self.trussed, self.options, just_verified)? else { return Err(Status::ConditionsOfUseNotSatisfied); }; diff --git a/src/state.rs b/src/state.rs index 04e86e9..f309504 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,5 +1,5 @@ use core::convert::{TryFrom, TryInto}; -use core::mem::{self, replace}; +use core::mem; use flexiber::EncodableHeapless; use heapless::Vec; @@ -15,7 +15,6 @@ use trussed::{ }; use trussed_chunked::utils; -use crate::container::KeySecurityCondition; use crate::piv_types::CardHolderUniqueIdentifier; use crate::reply::Reply; use crate::{constants::*, piv_types::AsymmetricAlgorithms}; @@ -213,26 +212,26 @@ impl Keys { Some(mem::replace(&mut self.authentication_alg, new.alg)) } AsymmetricKeyReference::DigitalSignature => self.signature_alg.replace(new.alg), - AsymmetricKeyReference::Retired01 => self.retired_keys[01].replace(new.alg), - AsymmetricKeyReference::Retired02 => self.retired_keys[02].replace(new.alg), - AsymmetricKeyReference::Retired03 => self.retired_keys[03].replace(new.alg), - AsymmetricKeyReference::Retired04 => self.retired_keys[04].replace(new.alg), - AsymmetricKeyReference::Retired05 => self.retired_keys[05].replace(new.alg), - AsymmetricKeyReference::Retired06 => self.retired_keys[06].replace(new.alg), - AsymmetricKeyReference::Retired07 => self.retired_keys[07].replace(new.alg), - AsymmetricKeyReference::Retired08 => self.retired_keys[08].replace(new.alg), - AsymmetricKeyReference::Retired09 => self.retired_keys[09].replace(new.alg), - AsymmetricKeyReference::Retired10 => self.retired_keys[10].replace(new.alg), - AsymmetricKeyReference::Retired11 => self.retired_keys[11].replace(new.alg), - AsymmetricKeyReference::Retired12 => self.retired_keys[12].replace(new.alg), - AsymmetricKeyReference::Retired13 => self.retired_keys[13].replace(new.alg), - AsymmetricKeyReference::Retired14 => self.retired_keys[14].replace(new.alg), - AsymmetricKeyReference::Retired15 => self.retired_keys[15].replace(new.alg), - AsymmetricKeyReference::Retired16 => self.retired_keys[16].replace(new.alg), - AsymmetricKeyReference::Retired17 => self.retired_keys[17].replace(new.alg), - AsymmetricKeyReference::Retired18 => self.retired_keys[18].replace(new.alg), - AsymmetricKeyReference::Retired19 => self.retired_keys[19].replace(new.alg), - AsymmetricKeyReference::Retired20 => self.retired_keys[20].replace(new.alg), + AsymmetricKeyReference::Retired01 => self.retired_keys[0].replace(new.alg), + AsymmetricKeyReference::Retired02 => self.retired_keys[1].replace(new.alg), + AsymmetricKeyReference::Retired03 => self.retired_keys[2].replace(new.alg), + AsymmetricKeyReference::Retired04 => self.retired_keys[3].replace(new.alg), + AsymmetricKeyReference::Retired05 => self.retired_keys[4].replace(new.alg), + AsymmetricKeyReference::Retired06 => self.retired_keys[5].replace(new.alg), + AsymmetricKeyReference::Retired07 => self.retired_keys[6].replace(new.alg), + AsymmetricKeyReference::Retired08 => self.retired_keys[7].replace(new.alg), + AsymmetricKeyReference::Retired09 => self.retired_keys[8].replace(new.alg), + AsymmetricKeyReference::Retired10 => self.retired_keys[9].replace(new.alg), + AsymmetricKeyReference::Retired11 => self.retired_keys[10].replace(new.alg), + AsymmetricKeyReference::Retired12 => self.retired_keys[11].replace(new.alg), + AsymmetricKeyReference::Retired13 => self.retired_keys[12].replace(new.alg), + AsymmetricKeyReference::Retired14 => self.retired_keys[13].replace(new.alg), + AsymmetricKeyReference::Retired15 => self.retired_keys[14].replace(new.alg), + AsymmetricKeyReference::Retired16 => self.retired_keys[15].replace(new.alg), + AsymmetricKeyReference::Retired17 => self.retired_keys[16].replace(new.alg), + AsymmetricKeyReference::Retired18 => self.retired_keys[17].replace(new.alg), + AsymmetricKeyReference::Retired19 => self.retired_keys[18].replace(new.alg), + AsymmetricKeyReference::Retired20 => self.retired_keys[19].replace(new.alg), AsymmetricKeyReference::KeyManagement | AsymmetricKeyReference::CardAuthentication => unreachable!(), }) @@ -313,11 +312,11 @@ impl<'t> LoadedState<'t> { key: AsymmetricKeyReference, client: &mut impl crate::Client, options: &crate::Options, + just_verified: bool, ) -> Result, Status> { let security_condition = key.use_security_condition(); match security_condition { - SecurityCondition::PinAlways if self.volatile.app_security_status.pin_just_verified => { - } + SecurityCondition::PinAlways if just_verified => {} SecurityCondition::Pin if self.volatile.app_security_status.pin_verified.is_some() => {} SecurityCondition::Always => {} _ => return Err(Status::SecurityStatusNotSatisfied), From 1303e3bb8296515ce135e39bbd288eb3f881114e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 26 Aug 2024 11:35:01 +0200 Subject: [PATCH 06/18] Fix memory leak --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index d9b8e08..80f1375 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -941,6 +941,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { } }; syscall!(self.trussed.delete(public_key)); + syscall!(self.trussed.delete(secret_key)); Ok(()) } From bd35b579a1ba1b44e1d8798526373090ad4e4a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 26 Aug 2024 11:57:41 +0200 Subject: [PATCH 07/18] Fix key history object --- src/container.rs | 9 +++++++++ src/lib.rs | 14 ++++---------- src/state.rs | 11 +++++++++++ 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/container.rs b/src/container.rs index 238b31c..d49539e 100644 --- a/src/container.rs +++ b/src/container.rs @@ -76,6 +76,15 @@ macro_rules! enum_subset { } } } + + impl $name { + #[allow(unused)] + pub(crate) fn all() -> &'static [Self] { + &[ + $(Self::$var,)* + ] + } + } } } diff --git a/src/lib.rs b/src/lib.rs index 80f1375..8afbb3d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1026,16 +1026,10 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { } fn get_key_history_object(&mut self, mut reply: Reply<'_, R>) -> Result { - // let num_keys = self - // .state - // .persistent - // .keys - // .retired_keys - // .iter() - // .filter(|k| k.is_some()) - // .count() as u8; - // TODO: fix count - let num_keys: u8 = 0u8; + let num_keys = AsymmetricKeyReference::all() + .iter() + .filter(|k| self.state.key_exists(self.trussed, &self.options, **k)) + .count() as u8; let mut num_certs = 0u8; use state::ContainerStorage; diff --git a/src/state.rs b/src/state.rs index f309504..1201317 100644 --- a/src/state.rs +++ b/src/state.rs @@ -307,6 +307,17 @@ impl Drop for UseValidKey { } impl<'t> LoadedState<'t> { + pub fn key_exists( + &self, + client: &mut impl crate::Client, + options: &crate::Options, + key: AsymmetricKeyReference, + ) -> bool { + syscall!(client.entry_metadata(options.storage, key.name().into())) + .metadata + .is_some() + } + pub fn use_valid_key( &mut self, key: AsymmetricKeyReference, From 424600301ad10538bacc2c2cea030f35d8bed702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 26 Aug 2024 14:59:12 +0200 Subject: [PATCH 08/18] Remove commented-out code --- src/lib.rs | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8afbb3d..ec17682 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -703,33 +703,6 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { return Err(Status::ConditionsOfUseNotSatisfied); }; - // let Some(KeyWithAlg { alg, id }) = - // self.state.persistent.keys.asymetric_for_reference(key_ref) - // else { - // warn!("Attempt to use unset key"); - // return Err(Status::ConditionsOfUseNotSatisfied); - // }; - - // if alg != auth.algorithm { - // warn!("Bad algorithm: {:?}", auth.algorithm); - // return Err(Status::IncorrectP1OrP2Parameter); - // } - // match key_ref.use_key_security_condition() { - // SecurityCondition::Pin => { - // if !self.state.volatile.pin_verified() { - // warn!("Authenticate challenge without pin validated"); - // return Err(Status::SecurityStatusNotSatisfied); - // } - // } - // SecurityCondition::PinAlways => { - // if !just_verified { - // warn!("authenticate challenge without pin validated"); - // return Err(Status::SecurityStatusNotSatisfied); - // } - // } - // SecurityCondition::Always => {} - // } - if key.alg.sign_len() != message.len() { return Err(Status::IncorrectDataParameter); } From 7f1783c9289e2ec20b566d37cd0018603dc89c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 20 Sep 2024 10:25:58 +0200 Subject: [PATCH 09/18] Fix clippy warnings --- .cargo/{config => config.toml} | 0 src/container.rs | 7 ++++--- src/lib.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) rename .cargo/{config => config.toml} (100%) diff --git a/.cargo/config b/.cargo/config.toml similarity index 100% rename from .cargo/config rename to .cargo/config.toml diff --git a/src/container.rs b/src/container.rs index d49539e..f93dfd6 100644 --- a/src/container.rs +++ b/src/container.rs @@ -80,9 +80,10 @@ macro_rules! enum_subset { impl $name { #[allow(unused)] pub(crate) fn all() -> &'static [Self] { - &[ - $(Self::$var,)* - ] + &[$( + $(#[cfg($inner)])? + Self::$var, + )*] } } } diff --git a/src/lib.rs b/src/lib.rs index ec17682..30c1e2e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1001,7 +1001,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { fn get_key_history_object(&mut self, mut reply: Reply<'_, R>) -> Result { let num_keys = AsymmetricKeyReference::all() .iter() - .filter(|k| self.state.key_exists(self.trussed, &self.options, **k)) + .filter(|k| self.state.key_exists(self.trussed, self.options, **k)) .count() as u8; let mut num_certs = 0u8; From d4089daed371cc2fee19c0e857fcaf331206b2d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 20 Sep 2024 10:32:05 +0200 Subject: [PATCH 10/18] Remove unused derp module --- src/derp.rs | 91 ----------------------------------------------------- src/lib.rs | 1 - 2 files changed, 92 deletions(-) delete mode 100644 src/derp.rs diff --git a/src/derp.rs b/src/derp.rs deleted file mode 100644 index bd0af07..0000000 --- a/src/derp.rs +++ /dev/null @@ -1,91 +0,0 @@ -pub use untrusted::{Input, Reader}; - -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub enum Error { - HighTagNumberForm, - LongLengthNotSupported, - NonCanonical, - Read, - UnexpectedEnd, - WrongTag, - WrongValue, -} - -pub type Result = core::result::Result; - -impl From for Error { - fn from(_: untrusted::EndOfInput) -> Error { - Error::UnexpectedEnd - } -} - -/// Return the value of the given tag and apply a decoding function to it. -pub fn nested<'a, F, R, E>( - input: &mut Reader<'a>, - incomplete_end: E, - bad_tag: E, - tag: u8, - decoder: F, -) -> core::result::Result -where - F: FnOnce(&mut untrusted::Reader<'a>) -> core::result::Result, -{ - let inner = expect_tag_and_get_value(input, tag).map_err(|_| bad_tag)?; - inner.read_all(incomplete_end, decoder) -} - -/// Read a tag and return it's value. Errors when the expect and actual tag do not match. -pub fn expect_tag_and_get_value<'a>(input: &mut Reader<'a>, tag: u8) -> Result> { - let (actual_tag, inner) = read_tag_and_get_value(input)?; - if usize::from(tag) != usize::from(actual_tag) { - return Err(Error::WrongTag); - } - Ok(inner) -} - -/// Read a tag and its value. Errors when the expected and actual tag and values do not match. -pub fn expect_tag_and_value(input: &mut Reader, tag: u8, value: &[u8]) -> Result<()> { - let (actual_tag, inner) = read_tag_and_get_value(input)?; - if usize::from(tag) != usize::from(actual_tag) { - return Err(Error::WrongTag); - } - if value != inner.as_slice_less_safe() { - return Err(Error::WrongValue); - } - Ok(()) -} - -/// Read the next tag, and return it and its value. -pub fn read_tag_and_get_value<'a>(input: &mut Reader<'a>) -> Result<(u8, Input<'a>)> { - let tag = input.read_byte()?; - if (tag & 0x1F) == 0x1F { - return Err(Error::HighTagNumberForm); - } - - // If the high order bit of the first byte is set to zero then the length - // is encoded in the seven remaining bits of that byte. Otherwise, those - // seven bits represent the number of bytes used to encode the length. - let length = match input.read_byte()? { - n if (n & 0x80) == 0 => usize::from(n), - 0x81 => { - let second_byte = input.read_byte()?; - if second_byte < 128 { - return Err(Error::NonCanonical); - } - usize::from(second_byte) - } - 0x82 => { - let second_byte = usize::from(input.read_byte()?); - let third_byte = usize::from(input.read_byte()?); - let combined = (second_byte << 8) | third_byte; - if combined < 256 { - return Err(Error::NonCanonical); - } - combined - } - _ => return Err(Error::LongLengthNotSupported), - }; - - let inner = input.read_bytes(length)?; - Ok((tag, inner)) -} diff --git a/src/lib.rs b/src/lib.rs index 30c1e2e..f281f82 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,7 +14,6 @@ use commands::{GeneralAuthenticate, PutData, ResetRetryCounter}; pub mod constants; pub mod container; use container::{AuthenticateKeyReference, Container, GenerateKeyReference, KeyReference}; -pub mod derp; #[cfg(feature = "apdu-dispatch")] mod dispatch; pub mod piv_types; From e48df6274cad08bd9ef45a8a54951552933b36bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 20 Sep 2024 11:42:59 +0200 Subject: [PATCH 11/18] Add test for signature with multiple keys (including unprotected ones) --- src/container.rs | 12 +++++++++--- src/lib.rs | 10 +++++++++- src/state.rs | 16 ++++++++-------- tests/card/mod.rs | 4 +++- tests/pivy.rs | 49 ++++++++++++++++++++++++++++++++++------------- 5 files changed, 65 insertions(+), 26 deletions(-) diff --git a/src/container.rs b/src/container.rs index f93dfd6..6b6c42e 100644 --- a/src/container.rs +++ b/src/container.rs @@ -164,8 +164,7 @@ impl KeyReference { match self { Self::SecureMessaging | Self::CardAuthentication - | Self::PivCardApplicationAdministration - | Self::KeyManagement => SecurityCondition::Always, + | Self::PivCardApplicationAdministration => SecurityCondition::Always, Self::DigitalSignature => SecurityCondition::PinAlways, _ => SecurityCondition::Pin, } @@ -271,10 +270,17 @@ impl AsymmetricKeyReference { /// Get the location to store a new key (important for keys that are encrypted and should be generated in volatile stoarge) pub fn storage(self, storage: Location) -> Location { match self { - Self::CardAuthentication | Self::KeyManagement => storage, + Self::CardAuthentication => storage, _ => Location::Volatile, } } + + pub fn is_encrypted(self) -> bool { + match self { + Self::CardAuthentication => false, + _ => true, + } + } } pub type GenerateKeyReference = AsymmetricKeyReference; diff --git a/src/lib.rs b/src/lib.rs index f281f82..9e1695a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -231,6 +231,12 @@ where .yubico_set_administration_key(data, touch_policy, reply)?; } + YubicoPivExtension::GetMetadata(KeyReference::CardAuthentication) => { + let this = self.load()?; + if this.state.persistent.keys.card_authentication.is_some() { + reply.expand(&[0x02, 0x02, 0x01, 0x00])?; + } + } YubicoPivExtension::GetMetadata(_reference) => { /* TODO */ } YubicoPivExtension::ImportAsymmetricKey(algo, key) => { self.load()?.import_asymmetric_key(algo, key, data, reply)?; @@ -913,7 +919,9 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { } }; syscall!(self.trussed.delete(public_key)); - syscall!(self.trussed.delete(secret_key)); + if reference.is_encrypted() { + syscall!(self.trussed.delete(secret_key)); + } Ok(()) } diff --git a/src/state.rs b/src/state.rs index 1201317..4226a02 100644 --- a/src/state.rs +++ b/src/state.rs @@ -123,7 +123,7 @@ pub struct Keys { pub signature_alg: Option, // 9d "Key Management Key" (YK: Key Management) #[serde(skip_serializing_if = "Option::is_none")] - pub key_management: Option>, + pub key_management_alg: Option, // 9e "Card Authentication Key" (YK: Card Authentication) #[serde(skip_serializing_if = "Option::is_none")] pub card_authentication: Option>, @@ -141,7 +141,7 @@ impl Keys { match key { AsymmetricKeyReference::PivAuthentication => Encrypted(Some(self.authentication_alg)), AsymmetricKeyReference::DigitalSignature => Encrypted(self.signature_alg), - AsymmetricKeyReference::KeyManagement => Plain(self.key_management), + AsymmetricKeyReference::KeyManagement => Encrypted(self.key_management_alg), AsymmetricKeyReference::CardAuthentication => Plain(self.card_authentication), AsymmetricKeyReference::Retired01 => Encrypted(self.retired_keys[0]), AsymmetricKeyReference::Retired02 => Encrypted(self.retired_keys[1]), @@ -190,9 +190,6 @@ impl Keys { }; let old = match key { - AsymmetricKeyReference::KeyManagement => { - KeyOrEncryptedWithAlg::Plain(self.key_management.replace(new)) - } AsymmetricKeyReference::CardAuthentication => { KeyOrEncryptedWithAlg::Plain(self.card_authentication.replace(new)) } @@ -211,6 +208,10 @@ impl Keys { AsymmetricKeyReference::PivAuthentication => { Some(mem::replace(&mut self.authentication_alg, new.alg)) } + AsymmetricKeyReference::KeyManagement => { + self.key_management_alg.replace(new.alg) + } + AsymmetricKeyReference::DigitalSignature => self.signature_alg.replace(new.alg), AsymmetricKeyReference::Retired01 => self.retired_keys[0].replace(new.alg), AsymmetricKeyReference::Retired02 => self.retired_keys[1].replace(new.alg), @@ -232,8 +233,7 @@ impl Keys { AsymmetricKeyReference::Retired18 => self.retired_keys[17].replace(new.alg), AsymmetricKeyReference::Retired19 => self.retired_keys[18].replace(new.alg), AsymmetricKeyReference::Retired20 => self.retired_keys[19].replace(new.alg), - AsymmetricKeyReference::KeyManagement - | AsymmetricKeyReference::CardAuthentication => unreachable!(), + AsymmetricKeyReference::CardAuthentication => unreachable!(), }) } }; @@ -873,7 +873,7 @@ impl Persistent { administration, is_admin_default: true, signature_alg: None, - key_management: None, + key_management_alg: None, card_authentication: None, retired_keys: Default::default(), }; diff --git a/tests/card/mod.rs b/tests/card/mod.rs index 0021789..46af00e 100644 --- a/tests/card/mod.rs +++ b/tests/card/mod.rs @@ -11,7 +11,9 @@ pub const WITH_UUID: Options = Options::new().uuid(Some([0; 16])); pub const WITHOUT_UUID: Options = Options::new(); pub fn with_vsc R, R>(options: Options, f: F) -> R { - let _lock = VSC_MUTEX.lock().unwrap(); + let Ok(_lock) = VSC_MUTEX.lock() else { + panic!("Some other test failed, this test is therefore ignored") + }; let mut vpicc = vpicc::connect().expect("failed to connect to vpcd"); diff --git a/tests/pivy.rs b/tests/pivy.rs index 1ed9323..cd1442d 100644 --- a/tests/pivy.rs +++ b/tests/pivy.rs @@ -86,17 +86,25 @@ fn ecdh() { with_vsc(WITHOUT_UUID, test); } -#[test_log::test] -fn sign() { +fn sign_inner(key: &str, requires_pin: bool) { #[cfg(feature = "rsa")] let test_rsa = || { - let mut p = spawn("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate 9A -a rsa2048 -P 123456").unwrap(); - p.expect(Regex("ssh-rsa (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_9A@[A-F0-9]{20}")).unwrap(); + let mut p = spawn(&format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a rsa2048 -P 123456")).unwrap(); + p.expect(Regex(&format!( + "{}{key}{}", + "ssh-rsa (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", + "@[A-F0-9]{20}" + ))) + .unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); let mut p = Command::new("pivy-tool") - .args(["sign", "9A", "-P", "123456"]) + .args(if requires_pin { + vec!["sign", key, "-P", "123456"] + } else { + vec!["sign", key] + }) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() @@ -109,13 +117,17 @@ fn sign() { }; let test_p256 = || { - let mut p = spawn("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate 9A -a eccp256 -P 123456").unwrap(); - p.expect(Regex("ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_9A@[A-F0-9]{20}")).unwrap(); + let mut p = spawn(&format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")).unwrap(); + p.expect(Regex(&format!("{}{key}{}","ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", "@[A-F0-9]{20}"))).unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); let mut p = Command::new("pivy-tool") - .args(["sign", "9A", "-P", "123456"]) + .args(if requires_pin { + vec!["sign", key, "-P", "123456"] + } else { + vec!["sign", key] + }) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() @@ -141,17 +153,28 @@ fn sign() { }; let test = || { - ( - test_p256(), - #[cfg(feature = "rsa")] - test_rsa(), - ) + test_p256(); + #[cfg(feature = "rsa")] + test_rsa(); }; with_vsc(WITH_UUID, test); with_vsc(WITHOUT_UUID, test); } +#[test_log::test] +fn sign_9a() { + sign_inner("9A", true); +} +#[test_log::test] +fn sign_9d() { + sign_inner("9D", true); +} +#[test_log::test] +fn sign_9e() { + sign_inner("9E", false); +} + const LARGE_CERT: &str = "-----BEGIN CERTIFICATE----- MIIHNTCCBh2gAwIBAgIUBeJLVUnOULY3fhLvjaWOZe/qWfYwDQYJKoZIhvcNAQEL BQAwggIoMQswCQYDVQQGEwJURTGBizCBiAYDVQQIDIGAVEVTVFRFU1RURVNUVEVT From 649aef7900f22a3c89638f0851b5d9d96275ca03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 20 Sep 2024 11:51:51 +0200 Subject: [PATCH 12/18] Add ecdh tests for multiple keys including unprotected ones --- src/container.rs | 5 +---- tests/pivy.rs | 36 +++++++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/container.rs b/src/container.rs index 6b6c42e..89a88aa 100644 --- a/src/container.rs +++ b/src/container.rs @@ -276,10 +276,7 @@ impl AsymmetricKeyReference { } pub fn is_encrypted(self) -> bool { - match self { - Self::CardAuthentication => false, - _ => true, - } + !matches!(self, Self::CardAuthentication) } } diff --git a/tests/pivy.rs b/tests/pivy.rs index cd1442d..3583758 100644 --- a/tests/pivy.rs +++ b/tests/pivy.rs @@ -56,19 +56,24 @@ fn generate() { } } -#[test_log::test] -fn ecdh() { +fn ecdh_inner(key: &str, requires_pin: bool) { let test = || { - let mut p = spawn("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate 9A -a eccp256 -P 123456").unwrap(); - p.expect(Regex( - "ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_9A@[A-F0-9]{20}", - )) + let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")).unwrap(); + p.expect(Regex(&format!( + "{}{key}{}", + "ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", + "@[A-F0-9]{20}", + ))) .unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); let mut p = Command::new("pivy-tool") - .args(["ecdh", "9A", "-P", "123456"]) + .args(if requires_pin { + vec!["sign", key, "-P", "123456"] + } else { + vec!["sign", key] + }) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() @@ -86,10 +91,23 @@ fn ecdh() { with_vsc(WITHOUT_UUID, test); } +#[test_log::test] +fn ecdh_9a() { + ecdh_inner("9A", true); +} +#[test_log::test] +fn ecdh_9d() { + ecdh_inner("9D", true); +} +#[test_log::test] +fn ecdh_9e() { + ecdh_inner("9E", false); +} + fn sign_inner(key: &str, requires_pin: bool) { #[cfg(feature = "rsa")] let test_rsa = || { - let mut p = spawn(&format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a rsa2048 -P 123456")).unwrap(); + let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a rsa2048 -P 123456")).unwrap(); p.expect(Regex(&format!( "{}{key}{}", "ssh-rsa (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", @@ -117,7 +135,7 @@ fn sign_inner(key: &str, requires_pin: bool) { }; let test_p256 = || { - let mut p = spawn(&format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")).unwrap(); + let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")).unwrap(); p.expect(Regex(&format!("{}{key}{}","ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", "@[A-F0-9]{20}"))).unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); From a59e6c855d7bf0991c38433ae72dacc00d3c39ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 23 Sep 2024 11:26:42 +0200 Subject: [PATCH 13/18] Use up to date, rebased hpke extension --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4a3a2c2..b3ecb87 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,7 +85,7 @@ trussed-auth = { git = "https://github.com/trussed-dev/trussed-auth.git", tag = trussed-rsa-alloc = { git = "https://github.com/trussed-dev/trussed-rsa-backend.git", tag = "v0.2.1" } trussed-chunked = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "chunked-v0.1.0" } trussed-staging = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } -trussed-hpke = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } +trussed-hpke = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "f0babe53813e7882cfe5ce749ebe3a65fc143fd7" } trussed-wrap-key-to-file = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } apdu-dispatch = { git = "https://github.com/Nitrokey/apdu-dispatch", tag = "v0.1.2-nitrokey.2" } trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" } From c8f31f16d5cdb7a86097df3519b5febb6a2fcc85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 27 Sep 2024 10:36:11 +0200 Subject: [PATCH 14/18] Properly clear rather than delete keys --- src/lib.rs | 2 +- src/state.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9e1695a..aa58871 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -920,7 +920,7 @@ impl<'a, T: Client> LoadedAuthenticator<'a, T> { }; syscall!(self.trussed.delete(public_key)); if reference.is_encrypted() { - syscall!(self.trussed.delete(secret_key)); + syscall!(self.trussed.clear(secret_key)); } Ok(()) diff --git a/src/state.rs b/src/state.rs index 4226a02..7885d2d 100644 --- a/src/state.rs +++ b/src/state.rs @@ -239,7 +239,7 @@ impl Keys { }; if let Some(key_id) = user_public_key { - syscall!(client.delete(key_id)); + syscall!(client.clear(key_id)); } old } @@ -456,7 +456,7 @@ impl Volatile { pub fn clear_pin_verified(&mut self, client: &mut impl crate::Client) { if let Some(key) = self.app_security_status.pin_verified { - syscall!(client.delete(key)); + syscall!(client.clear(key)); } self.app_security_status.pin_verified = None; self.app_security_status.pin_just_verified = false; @@ -1195,7 +1195,7 @@ impl ContainerStorage { Status::UnspecifiedNonpersistentExecutionError })?; - syscall!(client.delete(key_to_seal)); + syscall!(client.clear(key_to_seal)); Ok(()) } } From 6cfab5ee2793cb147b75f68a41ba4c25eefad4f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 27 Sep 2024 10:36:23 +0200 Subject: [PATCH 15/18] Return error instead of crash when the PIN is wrong --- src/state.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/state.rs b/src/state.rs index 7885d2d..b986bd3 100644 --- a/src/state.rs +++ b/src/state.rs @@ -433,7 +433,9 @@ impl Volatile { ) -> Option { self.clear_pin_verified(client); let pin = Bytes::from_slice(&value.0).expect("Convertion of static array"); - let pin_key = syscall!(client.get_pin_key(PinType::UserPin, pin)).result?; + let pin_key = try_syscall!(client.get_pin_key(PinType::UserPin, pin)) + .ok()? + .result?; let key = syscall!(client.unwrap_key_from_file( Mechanism::Chacha8Poly1305, pin_key, From 82aed8b06b881f8fb26eb5c59f647184c433e715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 30 Sep 2024 16:00:07 +0200 Subject: [PATCH 16/18] Enable test suite to run on a hardware device --- Cargo.toml | 3 ++ Makefile | 8 +++ tests/aa_serial.rs | 16 ++++++ tests/card/mod.rs | 34 +++++++++++++ tests/opensc.rs | 65 ++++++++++++++++++++----- tests/pivy.rs | 118 +++++++++++++++++++++++++++++++++++++-------- 6 files changed, 211 insertions(+), 33 deletions(-) create mode 100644 tests/aa_serial.rs diff --git a/Cargo.toml b/Cargo.toml index b3ecb87..cc088c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ trussed-hpke = "0.1.0" trussed-wrap-key-to-file = "0.1.0" trussed-staging = { version = "0.3.0", features = ["chunked", "hpke", "wrap-key-to-file"], default-features = false, optional = true } littlefs2 = "0.4.0" +cfg-if = "1.0.0" [dev-dependencies] rand_core = { version = "0.6", features = ["getrandom"] } @@ -78,6 +79,8 @@ log-debug = [] log-warn = [] log-error = [] +dangerous-test-real-card = [] + [patch.crates-io] trussed = { git = "https://github.com/Nitrokey/trussed" , tag = "v0.1.0-nitrokey.18" } littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "498fee7f7b908cb14e6ac18dca8442e19c89c0a9" } diff --git a/Makefile b/Makefile index 629d96f..16e81b8 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,11 @@ .NOTPARALLEL: +-include variables.mk + export RUST_LOG ?= info,cargo_tarpaulin=off TEST_FEATURES ?=vpicc,pivy-tests,opensc-tests,rsa +export PIV_DANGEROUS_TEST_CARD_READER ?= Virtual PCD 00 00 +export PIV_DANGEROUS_TEST_CARD_PIV_SERIAL ?= 04 B2 BB FB 54 40 4A E3 9B B8 6A E3 CA 82 9C 24 .PHONY: build-cortex-m4 build-cortex-m4: @@ -11,6 +15,10 @@ build-cortex-m4: test: cargo test --features $(TEST_FEATURES) +.PHONY: dangerous-test-real-card +dangerous-test-real-card: + cargo test --features $(TEST_FEATURES),dangerous-test-real-card + .PHONY: check check: RUSTLFAGS='-Dwarnings' cargo check --all-features --all-targets diff --git a/tests/aa_serial.rs b/tests/aa_serial.rs new file mode 100644 index 0000000..af8e1d7 --- /dev/null +++ b/tests/aa_serial.rs @@ -0,0 +1,16 @@ +#![cfg(feature = "dangerous-test-real-card")] +//! Test the serial number to ensure that tests run on the correct card + +use expectrl::{spawn, Eof}; + +const CARD: &str = env!("PIV_DANGEROUS_TEST_CARD_READER"); +const SERIAL: &str = env!("PIV_DANGEROUS_TEST_CARD_PIV_SERIAL"); + +#[test] +fn test_serial_number() { + let mut p = spawn("piv-tool --serial").unwrap(); + p.expect(&format!("Using reader with a card: {CARD}")) + .unwrap(); + p.expect(&format!("{SERIAL}")).unwrap(); + p.expect(Eof).unwrap(); +} diff --git a/tests/card/mod.rs b/tests/card/mod.rs index 46af00e..db6853c 100644 --- a/tests/card/mod.rs +++ b/tests/card/mod.rs @@ -3,13 +3,18 @@ use piv_authenticator::{virt::with_ram_client, vpicc::VpiccCard, Authenticator, use std::{sync::mpsc, thread::sleep, time::Duration}; use stoppable_thread::spawn; +use std::panic::{catch_unwind, resume_unwind, UnwindSafe}; +use std::process::Command; use std::sync::Mutex; static VSC_MUTEX: Mutex<()> = Mutex::new(()); +#[cfg_attr(feature = "dangerous-test-real-card", expect(unused))] pub const WITH_UUID: Options = Options::new().uuid(Some([0; 16])); +#[cfg_attr(feature = "dangerous-test-real-card", expect(unused))] pub const WITHOUT_UUID: Options = Options::new(); +#[cfg_attr(feature = "dangerous-test-real-card", expect(unused))] pub fn with_vsc R, R>(options: Options, f: F) -> R { let Ok(_lock) = VSC_MUTEX.lock() else { panic!("Some other test failed, this test is therefore ignored") @@ -46,3 +51,32 @@ pub fn with_vsc R, R>(options: Options, f: F) -> R { .expect("failed to run vpicc smartcard"); result } + +#[cfg_attr(not(feature = "dangerous-test-real-card"), expect(unused))] +pub fn with_lock_and_reset R, R: UnwindSafe>(f: F) { + let lock = VSC_MUTEX.lock(); + let res = catch_unwind(f); + let output = Command::new("piv-tool") + .args([ + "-s", + "00:20:00:80:08:0102030405060708", + "-s", + "00:20:00:80:08:0102030405060708", + "-s", + "00:20:00:80:08:0102030405060708", //Locked pin + "-s", + "00:FB:00:00", + ]) + .output() + .unwrap(); + if let Err(err) = res { + resume_unwind(err) + } + + println!("Out: {}", String::from_utf8_lossy(&output.stdout)); + println!("Err: {}", String::from_utf8_lossy(&output.stderr)); + + assert!(output.status.success()); + + drop(lock); +} diff --git a/tests/opensc.rs b/tests/opensc.rs index aa140bd..42a9b30 100644 --- a/tests/opensc.rs +++ b/tests/opensc.rs @@ -4,22 +4,35 @@ mod card; use std::process::Command; -use card::{with_vsc, WITHOUT_UUID, WITH_UUID}; +use card::*; +use cfg_if::cfg_if; use expectrl::{spawn, Eof, WaitStatus}; +const CARD: &str = env!("PIV_DANGEROUS_TEST_CARD_READER"); + +use std::time::Duration; +const EXPECT_TIMEOUT: Option = Some(Duration::from_secs(30)); + #[test_log::test] fn list() { let test = || { let mut p = spawn("piv-tool -n").unwrap(); - p.expect("Using reader with a card: Virtual PCD 00 00") + p.set_expect_timeout(EXPECT_TIMEOUT); + p.expect(&format!("Using reader with a card: {CARD}")) .unwrap(); p.expect("Personal Identity Verification Card").unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))] { + with_vsc(WITHOUT_UUID, test); + with_vsc(WITH_UUID, test); + } else { + with_lock_and_reset(test) + } + } } #[test_log::test] @@ -30,14 +43,21 @@ fn admin_mutual() { .env("PIV_EXT_AUTH_KEY", "tests/default_admin_key") .args(["-A", "M:9B:03"]); let mut p = expectrl::session::Session::spawn(command).unwrap(); - p.expect("Using reader with a card: Virtual PCD 00 00") + p.set_expect_timeout(EXPECT_TIMEOUT); + p.expect(&format!("Using reader with a card: {CARD}")) .unwrap(); // p.expect("Personal Identity Verification Card").unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } } /// Fails because of https://github.com/OpenSC/OpenSC/issues/2658 @@ -50,14 +70,21 @@ fn admin_card() { .env("PIV_EXT_AUTH_KEY", "tests/default_admin_key") .args(["-A", "A:9B:03"]); let mut p = expectrl::session::Session::spawn(command).unwrap(); + p.set_expect_timeout(EXPECT_TIMEOUT); p.expect("Using reader with a card: Virtual PCD 00 00") .unwrap(); p.expect("Personal Identity Verification Card").unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } } #[test_log::test] @@ -74,8 +101,14 @@ fn generate_key() { // // Non zero exit code? // assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 1)); // }); - // with_vsc(WITH_UUID, test); - // with_vsc(WITHOUT_UUID, test); + // cfg_if! { + // if #[cfg(not(feature = "dangerous-test-real-card"))]{ + // with_vsc(WITH_UUID, test); + // with_vsc(WITHOUT_UUID, test); + // } else { + // with_lock_and_reset(test) + // } + // } // let test = || { // let mut command = Command::new("piv-tool"); @@ -89,6 +122,12 @@ fn generate_key() { // // Non zero exit code? // assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 1)); // }; - // with_vsc(WITH_UUID, test); - // with_vsc(WITHOUT_UUID, test); + // cfg_if! { + // if #[cfg(not(feature = "dangerous-test-real-card"))]{ + // with_vsc(WITH_UUID, test); + // with_vsc(WITHOUT_UUID, test); + // } else { + // with_lock_and_reset(test) + // } + // } } diff --git a/tests/pivy.rs b/tests/pivy.rs index 3583758..0e1f21b 100644 --- a/tests/pivy.rs +++ b/tests/pivy.rs @@ -2,19 +2,48 @@ mod card; -use card::{with_vsc, WITHOUT_UUID, WITH_UUID}; +use card::*; +use cfg_if::cfg_if; use expectrl::{spawn, Eof, Regex, WaitStatus}; -use std::io::{Read, Write}; +use std::io::{self, Read, Write}; use std::process::{Command, Stdio}; +use std::time::Duration; + +const CARD: &str = env!("PIV_DANGEROUS_TEST_CARD_READER"); + +const EXPECT_TIMEOUT: Option = Some(Duration::from_secs(30)); + +// #[derive(Default)] +// struct LogWriter(Vec); + +// impl Write for LogWriter { +// fn write(&mut self, buf: &[u8]) -> io::Result { +// self.0.write(buf) +// } + +// fn flush(&mut self) -> io::Result<()> { +// self.0.flush() +// } +// } + +// impl Drop for LogWriter { +// fn drop(&mut self) { +// io::stdout().write_all(&self.0).unwrap(); +// } +// } #[test_log::test] fn list() { let test = || { - let mut p = spawn("pivy-tool list").unwrap(); - p.expect(Regex("card: [0-9A-Z]*")).unwrap(); - p.expect("device: Virtual PCD 00 00").unwrap(); + let mut p = spawn("pivy-tool list") + // .unwrap() + // .with_log(LogWriter(Vec::new())) + .unwrap(); + p.set_expect_timeout(EXPECT_TIMEOUT); + p.expect(Regex("card: [0-9A-Z]{8}")).unwrap(); + p.expect(&format!("device: {CARD}")).unwrap(); p.expect("chuid: ok").unwrap(); p.expect(Regex("guid: [0-9A-Z]*")).unwrap(); p.expect("algos: 3DES AES256 ECCP256 (null) (null)") @@ -22,14 +51,21 @@ fn list() { p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } } #[test_log::test] fn generate() { let test = || { let mut p = spawn("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate 9A -a eccp256 -P 123456").unwrap(); + p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex( "ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_9A@[A-F0-9]{20}", )) @@ -37,13 +73,19 @@ fn generate() { p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); - + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } #[cfg(feature = "rsa")] { let test = || { let mut p = spawn("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate 9A -a rsa2048 -P 123456").unwrap(); + p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex( "ssh-rsa (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_9A@[A-F0-9]{20}", )) @@ -51,14 +93,21 @@ fn generate() { p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } } } fn ecdh_inner(key: &str, requires_pin: bool) { let test = || { let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")).unwrap(); + p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex(&format!( "{}{key}{}", "ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", @@ -87,8 +136,14 @@ fn ecdh_inner(key: &str, requires_pin: bool) { assert_eq!(p.wait().unwrap().code(), Some(0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } } #[test_log::test] @@ -107,7 +162,12 @@ fn ecdh_9e() { fn sign_inner(key: &str, requires_pin: bool) { #[cfg(feature = "rsa")] let test_rsa = || { - let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a rsa2048 -P 123456")).unwrap(); + let mut logger = LogWriter::default(); + let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a rsa2048 -P 123456")) + // .unwrap() + // .with_log(LogWriter(Vec::new())) + .unwrap(); + p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex(&format!( "{}{key}{}", "ssh-rsa (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", @@ -135,7 +195,12 @@ fn sign_inner(key: &str, requires_pin: bool) { }; let test_p256 = || { - let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")).unwrap(); + let mut logger = LogWriter::default(); + let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")) + // .unwrap() + // .with_log(LogWriter(Vec::new())) + .unwrap(); + p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex(&format!("{}{key}{}","ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", "@[A-F0-9]{20}"))).unwrap(); p.expect(Eof).unwrap(); assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); @@ -157,6 +222,7 @@ fn sign_inner(key: &str, requires_pin: bool) { let mut out = Vec::new(); stdout.read_to_end(&mut out).unwrap(); + println!("{out:02x?}"); // Check that the signature is an asn.1 sequence let res: asn1::ParseResult<_> = asn1::parse(&out, |d| { d.read_element::()?.parse(|d| { @@ -176,8 +242,14 @@ fn sign_inner(key: &str, requires_pin: bool) { test_rsa(); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } } #[test_log::test] @@ -262,6 +334,12 @@ fn large_cert() { assert_eq!(&buf, LARGE_CERT); assert_eq!(p.wait().unwrap().code(), Some(0)); }; - with_vsc(WITH_UUID, test); - with_vsc(WITHOUT_UUID, test); + cfg_if! { + if #[cfg(not(feature = "dangerous-test-real-card"))]{ + with_vsc(WITH_UUID, test); + with_vsc(WITHOUT_UUID, test); + } else { + with_lock_and_reset(test) + } + } } From bc4f5283eac08a1c9acbf793cdd9e9a2f2bdad65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 30 Sep 2024 17:21:57 +0200 Subject: [PATCH 17/18] Update expectrl --- Cargo.toml | 2 +- tests/opensc.rs | 19 ++++++++---- tests/pivy.rs | 79 +++++++++++++++++++++++++++++-------------------- 3 files changed, 62 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cc088c9..b4502c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,7 +51,7 @@ ron = "0.8" des = "0.8" aes = "0.8.2" stoppable_thread = "0.2.1" -expectrl = "0.6.0" +expectrl = "0.7.0" iso7816 = { version = "0.1.2", features = ["std"] } # Examples diff --git a/tests/opensc.rs b/tests/opensc.rs index 42a9b30..9008abe 100644 --- a/tests/opensc.rs +++ b/tests/opensc.rs @@ -23,7 +23,10 @@ fn list() { .unwrap(); p.expect("Personal Identity Verification Card").unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); }; cfg_if! { if #[cfg(not(feature = "dangerous-test-real-card"))] { @@ -48,7 +51,10 @@ fn admin_mutual() { .unwrap(); // p.expect("Personal Identity Verification Card").unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); }; cfg_if! { if #[cfg(not(feature = "dangerous-test-real-card"))]{ @@ -75,7 +81,10 @@ fn admin_card() { .unwrap(); p.expect("Personal Identity Verification Card").unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); }; cfg_if! { if #[cfg(not(feature = "dangerous-test-real-card"))]{ @@ -99,7 +108,7 @@ fn generate_key() { // .unwrap(); // p.expect(Eof).unwrap(); // // Non zero exit code? - // assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 1)); + // assert_eq!(p.get_process().wait().unwrap(), WaitStatus::Exited(p.get_process().pid(), 1)); // }); // cfg_if! { // if #[cfg(not(feature = "dangerous-test-real-card"))]{ @@ -120,7 +129,7 @@ fn generate_key() { // .unwrap(); // p.expect(Eof).unwrap(); // // Non zero exit code? - // assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 1)); + // assert_eq!(p.get_process().wait().unwrap(), WaitStatus::Exited(p.get_process().pid(), 1)); // }; // cfg_if! { // if #[cfg(not(feature = "dangerous-test-real-card"))]{ diff --git a/tests/pivy.rs b/tests/pivy.rs index 0e1f21b..7d5ec8c 100644 --- a/tests/pivy.rs +++ b/tests/pivy.rs @@ -15,32 +15,31 @@ const CARD: &str = env!("PIV_DANGEROUS_TEST_CARD_READER"); const EXPECT_TIMEOUT: Option = Some(Duration::from_secs(30)); -// #[derive(Default)] -// struct LogWriter(Vec); +#[derive(Default)] +struct LogWriter(Vec); -// impl Write for LogWriter { -// fn write(&mut self, buf: &[u8]) -> io::Result { -// self.0.write(buf) -// } +impl Write for LogWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.write(buf) + } -// fn flush(&mut self) -> io::Result<()> { -// self.0.flush() -// } -// } + fn flush(&mut self) -> io::Result<()> { + self.0.flush() + } +} -// impl Drop for LogWriter { -// fn drop(&mut self) { -// io::stdout().write_all(&self.0).unwrap(); -// } -// } +impl Drop for LogWriter { + fn drop(&mut self) { + io::stdout().write_all(&self.0).unwrap(); + } +} #[test_log::test] fn list() { let test = || { - let mut p = spawn("pivy-tool list") - // .unwrap() - // .with_log(LogWriter(Vec::new())) - .unwrap(); + let mut logger = LogWriter(Vec::new()); + let p = spawn("pivy-tool list").unwrap(); + let mut p = expectrl::session::log(p, &mut logger).unwrap(); p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex("card: [0-9A-Z]{8}")).unwrap(); p.expect(&format!("device: {CARD}")).unwrap(); @@ -49,7 +48,10 @@ fn list() { p.expect("algos: 3DES AES256 ECCP256 (null) (null)") .unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); }; cfg_if! { if #[cfg(not(feature = "dangerous-test-real-card"))]{ @@ -71,7 +73,10 @@ fn generate() { )) .unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); }; cfg_if! { if #[cfg(not(feature = "dangerous-test-real-card"))]{ @@ -91,7 +96,10 @@ fn generate() { )) .unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); }; cfg_if! { if #[cfg(not(feature = "dangerous-test-real-card"))]{ @@ -115,7 +123,10 @@ fn ecdh_inner(key: &str, requires_pin: bool) { ))) .unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); let mut p = Command::new("pivy-tool") .args(if requires_pin { @@ -162,11 +173,10 @@ fn ecdh_9e() { fn sign_inner(key: &str, requires_pin: bool) { #[cfg(feature = "rsa")] let test_rsa = || { - let mut logger = LogWriter::default(); - let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a rsa2048 -P 123456")) - // .unwrap() - // .with_log(LogWriter(Vec::new())) + let mut logger = LogWriter(Vec::new()); + let p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a rsa2048 -P 123456")) .unwrap(); + let mut p = expectrl::session::log(p, &mut logger).unwrap(); p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex(&format!( "{}{key}{}", @@ -175,7 +185,10 @@ fn sign_inner(key: &str, requires_pin: bool) { ))) .unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); let mut p = Command::new("pivy-tool") .args(if requires_pin { @@ -196,14 +209,16 @@ fn sign_inner(key: &str, requires_pin: bool) { let test_p256 = || { let mut logger = LogWriter::default(); - let mut p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")) - // .unwrap() - // .with_log(LogWriter(Vec::new())) + let p = spawn(format!("pivy-tool -A 3des -K 010203040506070801020304050607080102030405060708 generate {key} -a eccp256 -P 123456")) .unwrap(); + let mut p = expectrl::session::log(p, &mut logger).unwrap(); p.set_expect_timeout(EXPECT_TIMEOUT); p.expect(Regex(&format!("{}{key}{}","ecdsa-sha2-nistp256 (?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)? PIV_slot_", "@[A-F0-9]{20}"))).unwrap(); p.expect(Eof).unwrap(); - assert_eq!(p.wait().unwrap(), WaitStatus::Exited(p.pid(), 0)); + assert_eq!( + p.get_process().wait().unwrap(), + WaitStatus::Exited(p.get_process().pid(), 0) + ); let mut p = Command::new("pivy-tool") .args(if requires_pin { From e67a954cf93a4cbf3c364ca9ba6c612d3cc3d5ff Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Oct 2024 22:03:03 +0200 Subject: [PATCH 18/18] Release v0.3.8 --- CHANGELOG.md | 6 ++++++ Cargo.toml | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b717ec6..ed96d3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [v0.3.8][] (2024-10-17) + +[v0.3.8]: https://github.com/Nitrokey/piv-authenticator/releases/tag/v0.3.8 + +- Encrypt data on external flash ([#57](https://github.com/Nitrokey/piv-authenticator/pull/57)) + ## [v0.3.7][] (2024-04-21) - Bump rsa backend version ([#53][]) diff --git a/Cargo.toml b/Cargo.toml index b4502c0..da0a6dc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "piv-authenticator" -version = "0.3.7" +version = "0.3.8" authors = ["Nicolas Stalder ", "Nitrokey GmbH"] edition = "2021" license = "Apache-2.0 OR MIT" @@ -35,7 +35,7 @@ trussed-rsa-alloc = { version = "0.2.1", features = ["raw"], optional = true } trussed-chunked = "0.1.0" trussed-hpke = "0.1.0" trussed-wrap-key-to-file = "0.1.0" -trussed-staging = { version = "0.3.0", features = ["chunked", "hpke", "wrap-key-to-file"], default-features = false, optional = true } +trussed-staging = { version = "0.3.2", features = ["chunked", "hpke", "wrap-key-to-file"], default-features = false, optional = true } littlefs2 = "0.4.0" cfg-if = "1.0.0" @@ -87,9 +87,9 @@ littlefs2 = { git = "https://github.com/trussed-dev/littlefs2.git", rev = "498fe trussed-auth = { git = "https://github.com/trussed-dev/trussed-auth.git", tag = "v0.3.0"} trussed-rsa-alloc = { git = "https://github.com/trussed-dev/trussed-rsa-backend.git", tag = "v0.2.1" } trussed-chunked = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "chunked-v0.1.0" } -trussed-staging = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } -trussed-hpke = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "f0babe53813e7882cfe5ce749ebe3a65fc143fd7" } -trussed-wrap-key-to-file = { git = "https://github.com/Nitrokey/trussed-staging.git", rev = "40ac4268c0ebbe20b0ef71553a345dd0905326de" } +trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "v0.3.2" } +trussed-hpke = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "hpke-v0.1.0" } +trussed-wrap-key-to-file = { git = "https://github.com/trussed-dev/trussed-staging.git", tag = "wrap-key-to-file-v0.1.0" } apdu-dispatch = { git = "https://github.com/Nitrokey/apdu-dispatch", tag = "v0.1.2-nitrokey.2" } trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" } usbd-ccid = { git = "https://github.com/Nitrokey/usbd-ccid", tag = "v0.2.0-nitrokey.1" }