From 0367cbbb1db2390812cf67993bdecbdc1e5eb35b Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 28 Feb 2024 12:10:10 +0100 Subject: [PATCH] ctap2: Implement PIN protocol version 2 --- CHANGELOG.md | 5 +- Cargo.toml | 7 +- src/ctap2.rs | 59 +++++++------ src/ctap2/pin.rs | 220 ++++++++++++++++++++++++++++++++++++----------- src/lib.rs | 5 ++ 5 files changed, 217 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 677d635..860df62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Implement the `largeBlobKey` extension and the `largeBlobs` command ([#38][]) - Fix error type for third invalid PIN entry ([#60][]) - Fix error type for cancelled user presence ([#61][]) -- Extract PIN protocol implementation into separate module ([#62][]) +- PIN protocol changes: + - Extract PIN protocol implementation into separate module ([#62][]) + - Implement PIN protocol 2 ([#63][]) [#26]: https://github.com/solokeys/fido-authenticator/issues/26 [#28]: https://github.com/solokeys/fido-authenticator/issues/28 @@ -31,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#60]: https://github.com/Nitrokey/fido-authenticator/pull/60 [#61]: https://github.com/Nitrokey/fido-authenticator/pull/61 [#62]: https://github.com/Nitrokey/fido-authenticator/pull/62 +[#63]: https://github.com/Nitrokey/fido-authenticator/pull/63 ## [0.1.1] - 2022-08-22 - Fix bug that treated U2F payloads as APDU over APDU in NFC transport @conorpp diff --git a/Cargo.toml b/Cargo.toml index 1270ab7..393b1e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ serde_cbor = { version = "0.11.0", default-features = false } serde-indexed = "0.1.0" sha2 = { version = "0.10", default-features = false } trussed = "0.1" +trussed-hkdf = { version = "0.1.0" } trussed-staging = { version = "0.1.0", default-features = false, optional = true } apdu-dispatch = { version = "0.1", optional = true } @@ -56,11 +57,13 @@ usbd-ctaphid = "0.1.0" features = ["dispatch"] [patch.crates-io] -ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "7d4ad69e64ad308944c012aef5b9cfd7654d9be8" } +# ctap-types = { git = "https://github.com/trussed-dev/ctap-types.git", rev = "f0db7acb87cced69edde1e33f41e1bdf1eee54c0" } +ctap-types = { git = "https://github.com/Nitrokey/ctap-types.git", branch = "pin-protocol-2" } ctaphid-dispatch = { git = "https://github.com/trussed-dev/ctaphid-dispatch.git", rev = "57cb3317878a8593847595319aa03ef17c29ec5b" } apdu-dispatch = { git = "https://github.com/trussed-dev/apdu-dispatch.git", rev = "915fc237103fcecc29d0f0b73391f19abf6576de" } +serde-indexed = { git = "https://github.com/sosthene-nitrokey/serde-indexed.git", rev = "5005d23cb4ee8622e62188ea0f9466146f851f0d" } trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "b1781805a2e33615d2d00b8bec80c0b1f5870ca1" } +trussed-hkdf = { git = "https://github.com/Nitrokey/trussed-hkdf-backend.git", tag = "v0.1.0" } trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging", rev = "3b9594d93f89a5e760fe78fa5a96f125dfdcd470" } -serde-indexed = { git = "https://github.com/sosthene-nitrokey/serde-indexed.git", rev = "5005d23cb4ee8622e62188ea0f9466146f851f0d" } trussed-usbip = { git = "https://github.com/Nitrokey/pc-usbip-runner.git", tag = "v0.0.1-nitrokey.1" } usbd-ctaphid = { git = "https://github.com/Nitrokey/usbd-ctaphid.git", tag = "v0.1.0-nitrokey.2" } diff --git a/src/ctap2.rs b/src/ctap2.rs index 404ce07..7167772 100644 --- a/src/ctap2.rs +++ b/src/ctap2.rs @@ -68,8 +68,10 @@ impl Authenticator for crate::Authenti .unwrap(); } - let mut pin_protocols = Vec::::new(); - pin_protocols.push(1).unwrap(); + let mut pin_protocols = Vec::::new(); + for pin_protocol in self.pin_protocols() { + pin_protocols.push(u8::from(*pin_protocol)).unwrap(); + } let options = ctap2::get_info::CtapOptions { ep: None, @@ -692,16 +694,15 @@ impl Authenticator for crate::Authenti } // 3. generate shared secret - let shared_secret = self - .pin_protocol(pin_protocol) - .shared_secret(platform_kek)?; + let mut pin_protocol = self.pin_protocol(pin_protocol); + let shared_secret = pin_protocol.shared_secret(platform_kek)?; // TODO: there are moar early returns!! // - implement Drop? // - do garbage collection outside of this? // 4. verify pinAuth - shared_secret.verify_pin_auth(&mut self.trussed, new_pin_enc, pin_auth)?; + pin_protocol.verify_pin_auth(&shared_secret, new_pin_enc, pin_auth)?; // 5. decrypt and verify new PIN let new_pin = self.decrypt_pin_check_length(&shared_secret, new_pin_enc)?; @@ -754,9 +755,8 @@ impl Authenticator for crate::Authenti self.state.pin_blocked()?; // 3. generate shared secret - let shared_secret = self - .pin_protocol(pin_protocol) - .shared_secret(platform_kek)?; + let mut pin_protocol_impl = self.pin_protocol(pin_protocol); + let shared_secret = pin_protocol_impl.shared_secret(platform_kek)?; // 4. verify pinAuth let mut data = MediumData::new(); @@ -764,7 +764,7 @@ impl Authenticator for crate::Authenti .map_err(|_| Error::InvalidParameter)?; data.extend_from_slice(pin_hash_enc) .map_err(|_| Error::InvalidParameter)?; - shared_secret.verify_pin_auth(&mut self.trussed, &data, pin_auth)?; + pin_protocol_impl.verify_pin_auth(&shared_secret, &data, pin_auth)?; // 5. decrement retries self.state.decrement_retries(&mut self.trussed)?; @@ -1050,15 +1050,24 @@ impl Authenticator for crate::Authenti // impl Authenticator for crate::Authenticator impl crate::Authenticator { - fn parse_pin_protocol(&self, version: impl Into) -> Result { - match version.into() { - 1 => Ok(PinProtocolVersion::V1), - _ => Err(Error::InvalidParameter), + fn parse_pin_protocol(&self, version: impl TryInto) -> Result { + if let Ok(version) = version.try_into() { + for pin_protocol in self.pin_protocols() { + if u8::from(*pin_protocol) == version { + return Ok(*pin_protocol); + } + } } + Err(Error::InvalidParameter) } + // This is the single source of truth for the supported PIN protocols. fn pin_protocols(&self) -> &'static [PinProtocolVersion] { - &[PinProtocolVersion::V1] + if self.config.pin_protocol_v2 { + &[PinProtocolVersion::V1, PinProtocolVersion::V2] + } else { + &[PinProtocolVersion::V1] + } } fn pin_protocol(&mut self, pin_protocol: PinProtocolVersion) -> PinProtocol<'_, T> { @@ -1181,7 +1190,7 @@ impl crate::Authenticator { &mut self, pin_protocol: PinProtocolVersion, shared_secret: &SharedSecret, - pin_hash_enc: &Bytes<64>, + pin_hash_enc: &Bytes<80>, ) -> Result<()> { let pin_hash = shared_secret .decrypt(&mut self.trussed, pin_hash_enc) @@ -1372,9 +1381,6 @@ impl crate::Authenticator { if self.state.persistent.pin_is_set() { // let mut uv_performed = false; if let Some(pin_auth) = pin_auth { - if pin_auth.len() != 16 { - return Err(Error::InvalidParameter); - } // seems a bit redundant to check here in light of 2. // I guess the CTAP spec writers aren't implementers :D if let Some(pin_protocol) = pin_protocol { @@ -1382,7 +1388,7 @@ impl crate::Authenticator { // success --> set uv = 1 // error --> PinAuthInvalid self.pin_protocol(pin_protocol) - .verify_pin_token(pin_auth, data)?; + .verify_pin_token(data, pin_auth)?; return Ok(true); } else { @@ -1449,16 +1455,17 @@ impl crate::Authenticator { .key; // Verify the auth tag, which uses the same process as the pinAuth - let shared_secret = self - .pin_protocol(pin_protocol) - .shared_secret(&hmac_secret.key_agreement)?; - shared_secret.verify_pin_auth( - &mut self.trussed, + let mut pin_protocol = self.pin_protocol(pin_protocol); + let shared_secret = pin_protocol.shared_secret(&hmac_secret.key_agreement)?; + pin_protocol.verify_pin_auth( + &shared_secret, &hmac_secret.salt_enc, &hmac_secret.salt_auth, )?; - if hmac_secret.salt_enc.len() != 32 && hmac_secret.salt_enc.len() != 64 { + if hmac_secret.salt_enc.len() != 32 + && (hmac_secret.salt_enc.len() != 64 || hmac_secret.salt_enc.len() == 80) + { debug_now!("invalid hmac-secret length"); return Err(Error::InvalidLength); } diff --git a/src/ctap2/pin.rs b/src/ctap2/pin.rs index 55cd4d2..32ae7ea 100644 --- a/src/ctap2/pin.rs +++ b/src/ctap2/pin.rs @@ -2,16 +2,28 @@ use crate::{cbor_serialize_message, TrussedRequirements}; use ctap_types::{cose::EcdhEsHkdf256PublicKey, Error, Result}; use trussed::{ cbor_deserialize, - client::{Aes256Cbc, CryptoClient, HmacSha256, P256}, + client::{CryptoClient, HmacSha256, P256}, syscall, try_syscall, - types::{Bytes, KeyId, KeySerialization, Location, Mechanism, StorageAttributes}, + types::{Bytes, KeyId, KeySerialization, Location, Mechanism, Message, StorageAttributes}, }; +use trussed_hkdf::{KeyOrData, OkmId}; -const PIN_TOKEN_LENGTH: usize = 16; +// PIN protocol 1 supports 16 or 32 bytes, PIN protocol 2 requires 32 bytes. +const PIN_TOKEN_LENGTH: usize = 32; #[derive(Clone, Copy, Debug)] pub enum PinProtocolVersion { V1, + V2, +} + +impl From for u8 { + fn from(version: PinProtocolVersion) -> Self { + match version { + PinProtocolVersion::V1 => 1, + PinProtocolVersion::V2 => 2, + } + } } #[derive(Debug)] @@ -19,10 +31,12 @@ pub struct PinProtocolState { key_agreement_key: KeyId, // only used to delete the old shared secret from VFS when generating a new one. ideally, the // SharedSecret struct would clean up after itself. - shared_secret: Option, + shared_secret: Option, // for protocol version 1 pin_token_v1: KeyId, + // for protocol version 2 + pin_token_v2: KeyId, } impl PinProtocolState { @@ -32,6 +46,7 @@ impl PinProtocolState { key_agreement_key: generate_key_agreement_key(trussed), shared_secret: None, pin_token_v1: generate_pin_token(trussed), + pin_token_v2: generate_pin_token(trussed), } } @@ -39,7 +54,7 @@ impl PinProtocolState { syscall!(trussed.delete(self.pin_token_v1)); syscall!(trussed.delete(self.key_agreement_key)); if let Some(shared_secret) = self.shared_secret { - syscall!(trussed.delete(shared_secret)); + shared_secret.delete(trussed); } } } @@ -67,19 +82,21 @@ impl<'a, T: TrussedRequirements> PinProtocol<'a, T> { fn pin_token(&self) -> KeyId { match self.version { PinProtocolVersion::V1 => self.state.pin_token_v1, + PinProtocolVersion::V2 => self.state.pin_token_v2, } } fn set_pin_token(&mut self, pin_token: KeyId) { match self.version { PinProtocolVersion::V1 => self.state.pin_token_v1 = pin_token, + PinProtocolVersion::V2 => self.state.pin_token_v2 = pin_token, } } pub fn regenerate(&mut self) { syscall!(self.trussed.delete(self.state.key_agreement_key)); if let Some(shared_secret) = self.state.shared_secret.take() { - syscall!(self.trussed.delete(shared_secret)); + shared_secret.delete(self.trussed); } self.state.key_agreement_key = generate_key_agreement_key(self.trussed); } @@ -109,10 +126,35 @@ impl<'a, T: TrussedRequirements> PinProtocol<'a, T> { cose_key } + #[must_use] + fn verify(&mut self, key: KeyId, data: &[u8], signature: &[u8]) -> bool { + let actual_signature = syscall!(self.trussed.sign_hmacsha256(key, data)).signature; + let expected_signature = match self.version { + PinProtocolVersion::V1 => &actual_signature[..16], + PinProtocolVersion::V2 => &actual_signature, + }; + expected_signature == signature + } + // in spec: verify(pinUvAuthToken, ...) pub fn verify_pin_token(&mut self, data: &[u8], signature: &[u8]) -> Result<()> { // TODO: check if pin token is in use - if verify(self.trussed, self.pin_token(), data, signature) { + if self.verify(self.pin_token(), data, signature) { + Ok(()) + } else { + Err(Error::PinAuthInvalid) + } + } + + // in spec: verify(shared secret, ...) + pub fn verify_pin_auth( + &mut self, + shared_secret: &SharedSecret, + data: &[u8], + pin_auth: &[u8], + ) -> Result<()> { + let key_id = shared_secret.hmac_key_id(); + if self.verify(key_id, data, pin_auth) { Ok(()) } else { Err(Error::PinAuthInvalid) @@ -120,11 +162,8 @@ impl<'a, T: TrussedRequirements> PinProtocol<'a, T> { } // in spec: encrypt(..., pinUvAuthToken) - pub fn encrypt_pin_token(&mut self, shared_secret: &SharedSecret) -> Result> { - let token = syscall!(self - .trussed - .wrap_key_aes256cbc(shared_secret.key_id, self.pin_token())) - .wrapped_key; + pub fn encrypt_pin_token(&mut self, shared_secret: &SharedSecret) -> Result> { + let token = shared_secret.wrap(self.trussed, self.pin_token()); Bytes::from_slice(&token).map_err(|_| Error::Other) } @@ -156,74 +195,155 @@ impl<'a, T: TrussedRequirements> PinProtocol<'a, T> { syscall!(self.trussed.delete(peer_key)); let pre_shared_secret = result.ok()?.shared_secret; - if let Some(shared_secret) = self.state.shared_secret { - syscall!(self.trussed.delete(shared_secret)); + if let Some(shared_secret) = self.state.shared_secret.take() { + shared_secret.delete(self.trussed); } let shared_secret = self.kdf(pre_shared_secret); - self.state.shared_secret = Some(shared_secret); syscall!(self.trussed.delete(pre_shared_secret)); - Some(SharedSecret { - key_id: shared_secret, - }) + let shared_secret = shared_secret?; + self.state.shared_secret = Some(shared_secret.clone()); + Some(shared_secret) } - fn kdf(&mut self, input: KeyId) -> KeyId { - syscall!(self.trussed.derive_key( + fn kdf(&mut self, input: KeyId) -> Option { + match self.version { + PinProtocolVersion::V1 => self.kdf_v1(input), + PinProtocolVersion::V2 => self.kdf_v2(input), + } + } + + // PIN protocol 1: derive a single key using SHA-256 + fn kdf_v1(&mut self, input: KeyId) -> Option { + let key_id = syscall!(self.trussed.derive_key( Mechanism::Sha256, input, None, StorageAttributes::new().set_persistence(Location::Volatile) )) - .key + .key; + Some(SharedSecret::V1 { key_id }) + } + + // PIN protocol 2: derive two keys using HKDF-SHA-256 + // In the spec, the keys are concatenated and the relevant part is selected during the key + // operations. For simplicity, we store two separate keys instead. + fn kdf_v2(&mut self, input: KeyId) -> Option { + fn hkdf(trussed: &mut T, okm: OkmId, info: &[u8]) -> Option { + let info = Message::from_slice(info).ok()?; + try_syscall!(trussed.hkdf_expand(okm, info, 32, Location::Volatile)) + .ok() + .map(|reply| reply.key) + } + + // salt: 0x00 * 32 => None + let okm = try_syscall!(self.trussed.hkdf_extract( + KeyOrData::Key(input), + None, + Location::Volatile + )) + .ok()? + .okm; + let hmac_key_id = hkdf(self.trussed, okm, b"CTAP2 HMAC key"); + let aes_key_id = hkdf(self.trussed, okm, b"CTAP2 AES key"); + + syscall!(self.trussed.delete(okm.0)); + + Some(SharedSecret::V2 { + hmac_key_id: hmac_key_id?, + aes_key_id: aes_key_id?, + }) } } -pub struct SharedSecret { - key_id: KeyId, +#[derive(Clone, Debug)] +pub enum SharedSecret { + V1 { + key_id: KeyId, + }, + V2 { + hmac_key_id: KeyId, + aes_key_id: KeyId, + }, } impl SharedSecret { - pub fn verify_pin_auth( - &self, - trussed: &mut T, - data: &[u8], - pin_auth: &Bytes<16>, - ) -> Result<()> { - if verify(trussed, self.key_id, data, pin_auth) { - Ok(()) - } else { - Err(Error::PinAuthInvalid) + fn aes_key_id(&self) -> KeyId { + match self { + Self::V1 { key_id } => *key_id, + Self::V2 { aes_key_id, .. } => *aes_key_id, + } + } + + fn hmac_key_id(&self) -> KeyId { + match self { + Self::V1 { key_id } => *key_id, + Self::V2 { hmac_key_id, .. } => *hmac_key_id, } } + fn generate_iv(&self) -> [u8; 16] { + // TODO: randomly generate IV for PIN protocol 2 + [0; 16] + } + #[must_use] pub fn encrypt(&self, trussed: &mut T, data: &[u8]) -> Bytes<1024> { - syscall!(trussed.encrypt(Mechanism::Aes256Cbc, self.key_id, data, b"", None)).ciphertext + let key_id = self.aes_key_id(); + let iv = self.generate_iv(); + let mut ciphertext = + syscall!(trussed.encrypt(Mechanism::Aes256Cbc, key_id, data, &iv, None)).ciphertext; + if matches!(self, Self::V2 { .. }) { + ciphertext.insert_slice_at(&iv, 0).unwrap(); + } + ciphertext } #[must_use] - pub fn decrypt(&self, trussed: &mut T, data: &[u8]) -> Option> { - decrypt(trussed, self.key_id, data) + fn wrap(&self, trussed: &mut T, key: KeyId) -> Bytes<1024> { + let wrapping_key = self.aes_key_id(); + let iv = self.generate_iv(); + let mut wrapped_key = + syscall!(trussed.wrap_key(Mechanism::Aes256Cbc, wrapping_key, key, &iv)).wrapped_key; + if matches!(self, Self::V2 { .. }) { + wrapped_key.insert_slice_at(&iv, 0).unwrap(); + } + wrapped_key } - pub fn delete(self, trussed: &mut T) { - syscall!(trussed.delete(self.key_id)); + #[must_use] + pub fn decrypt(&self, trussed: &mut T, data: &[u8]) -> Option> { + let key_id = self.aes_key_id(); + let (iv, data) = match self { + Self::V1 { .. } => (Default::default(), data), + Self::V2 { .. } => { + if data.len() < 16 { + return None; + } + data.split_at(16) + } + }; + try_syscall!(trussed.decrypt(Mechanism::Aes256Cbc, key_id, data, iv, b"", b"")) + .ok() + .and_then(|response| response.plaintext) } -} - -#[must_use] -fn verify(trussed: &mut T, key: KeyId, data: &[u8], signature: &[u8]) -> bool { - let actual_signature = syscall!(trussed.sign_hmacsha256(key, data)).signature; - &actual_signature[..16] == signature -} -#[must_use] -fn decrypt(trussed: &mut T, key: KeyId, data: &[u8]) -> Option> { - try_syscall!(trussed.decrypt_aes256cbc(key, data)) - .ok() - .and_then(|response| response.plaintext) + pub fn delete(self, trussed: &mut T) { + match self { + Self::V1 { key_id } => { + syscall!(trussed.delete(key_id)); + } + Self::V2 { + hmac_key_id, + aes_key_id, + } => { + for key_id in [hmac_key_id, aes_key_id] { + syscall!(trussed.delete(key_id)); + } + } + } + } } fn generate_pin_token(trussed: &mut T) -> KeyId { diff --git a/src/lib.rs b/src/lib.rs index 8f46acb..4ca9de4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,7 @@ generate_macros!(); use core::time::Duration; use trussed::{client, syscall, types::Message, Client as TrussedClient}; +use trussed_hkdf::HkdfClient; use ctap_types::heapless_bytes::Bytes; @@ -55,6 +56,7 @@ pub trait TrussedRequirements: + client::Sha256 + client::HmacSha256 + client::Ed255 // + client::Totp + + HkdfClient + ExtensionRequirements { } @@ -67,6 +69,7 @@ impl TrussedRequirements for T where + client::Sha256 + client::HmacSha256 + client::Ed255 // + client::Totp + + HkdfClient + ExtensionRequirements { } @@ -100,6 +103,8 @@ pub struct Config { /// /// If this is `None`, the extension and the command are disabled. pub large_blobs: Option, + /// Enable PIN protocol v2. + pub pin_protocol_v2: bool, } impl Config {