From 86403fa9f2e7db9ee4fca56fb3f837d91d8c0a1a Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 2 Dec 2024 12:01:06 +0100 Subject: [PATCH 1/3] Add test case for credential ID stability This patch adds a test case that ensures that the calculated credential ID for a credential that was created using the old (unstripped) format is the same as the one generated originally. Otherwise, the platform or the RP could reject assertions because of a changed credential ID. --- Cargo.toml | 1 + src/credential.rs | 111 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 54f3841..1956f37 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,6 +68,7 @@ littlefs2 = "0.5.0" log = "0.4.21" p256 = { version = "0.13.2", features = ["ecdh"] } rand = "0.8.4" +rand_chacha = "0.3" sha2 = "0.10" serde_test = "1.0.176" trussed = { version = "0.1", features = ["virt"] } diff --git a/src/credential.rs b/src/credential.rs index e297201..0279356 100644 --- a/src/credential.rs +++ b/src/credential.rs @@ -601,9 +601,17 @@ impl From<&FullCredential> for StrippedCredential { #[cfg(test)] mod test { use super::*; + use hex_literal::hex; + use littlefs2_core::path; + use rand::SeedableRng as _; + use rand_chacha::ChaCha8Rng; use trussed::{ client::{Chacha8Poly1305, Sha256}, + key::{Kind, Secrecy}, + store::keystore::{ClientKeystore, Keystore as _}, types::Location, + virt::{self, Ram}, + Platform as _, }; fn credential_data() -> CredentialData { @@ -630,6 +638,30 @@ mod test { } } + fn old_credential_data() -> CredentialData { + CredentialData { + rp: LocalPublicKeyCredentialRpEntity { + id: String::from("John Doe"), + name: None, + }, + user: LocalPublicKeyCredentialUserEntity { + id: Bytes::from_slice(&[1, 2, 3]).unwrap(), + icon: None, + name: None, + display_name: None, + }, + creation_time: 123, + use_counter: false, + algorithm: -7, + key: Key::WrappedKey(Bytes::from_slice(&[1, 2, 3]).unwrap()), + hmac_secret: Some(false), + cred_protect: None, + use_short_id: None, + large_blob_key: None, + third_party_payment: None, + } + } + fn random_byte_array() -> ByteArray { use rand::{rngs::OsRng, RngCore}; let mut bytes = [0; N]; @@ -733,6 +765,85 @@ mod test { assert_eq!(credential_data, deserialized); } + #[test] + fn old_credential_id() { + // generated with v0.1.1-nitrokey.4 (NK3 firmware version v1.4.0) + const OLD_ID: &[u8] = &hex!("A300583A71AEF80C4DA56033D66EB3266E9ACB8D84923D13F89BCBCE9FF30D8CD77ED968A436CA3D39C49999EC0F69A289CB2A65A08ABF251DEB21BB4B56014C00000000000000000000000002504DF499ABDAE80F5615C870985B74A799"); + const SERIALIZED_DATA: &[u8] = &hex!( + "A700A1626964684A6F686E20446F6501A16269644301020302187B03F404260582014301020306F4" + ); + const SERIALIZED_CREDENTIAL: &[u8] = &hex!("A3000201A700A1626964684A6F686E20446F6501A16269644301020302187B03F404260582014301020306F4024C000000000000000000000000"); + + virt::with_platform(Ram::default(), |mut platform| { + let kek = [0; 44]; + let client_id = path!("fido"); + let kek = { + let rng = ChaCha8Rng::from_rng(platform.rng()).unwrap(); + let mut keystore = ClientKeystore::new(client_id.into(), rng, platform.store()); + keystore + .store_key( + Location::Internal, + Secrecy::Secret, + Kind::Symmetric32Nonce(12), + &kek, + ) + .unwrap() + }; + platform.run_client(client_id.as_str(), |mut client| { + let data = old_credential_data(); + let rp_id_hash = syscall!(client.hash_sha256(data.rp.id.as_ref())).hash; + let credential_id = CredentialId(Bytes::from_slice(OLD_ID).unwrap()); + let encrypted_serialized = + EncryptedSerializedCredential::try_from(credential_id).unwrap(); + let serialized = syscall!(client.decrypt_chacha8poly1305( + kek, + &encrypted_serialized.0.ciphertext, + &rp_id_hash, + &encrypted_serialized.0.nonce, + &encrypted_serialized.0.tag, + )) + .plaintext + .unwrap(); + + let full = FullCredential::deserialize(&serialized).unwrap(); + assert_eq!( + full, + FullCredential { + ctap: CtapVersion::Fido21Pre, + data, + nonce: [0; 12].into(), + } + ); + + let stripped_credential = full.strip(); + + let serialized_data: Bytes<1024> = + trussed::cbor_serialize_bytes(&stripped_credential.data).unwrap(); + assert_eq!( + delog::hexstr!(&serialized_data).to_string(), + delog::hexstr!(SERIALIZED_DATA).to_string() + ); + + let serialized_credential: Bytes<1024> = + trussed::cbor_serialize_bytes(&stripped_credential).unwrap(); + assert_eq!( + delog::hexstr!(&serialized_credential).to_string(), + delog::hexstr!(SERIALIZED_CREDENTIAL).to_string() + ); + + let credential = Credential::Full(full); + let id = credential + .id(&mut client, kek, rp_id_hash.as_ref().try_into().unwrap()) + .unwrap() + .0; + assert_eq!( + delog::hexstr!(&id).to_string(), + delog::hexstr!(OLD_ID).to_string() + ); + }); + }); + } + #[test] fn credential_ids() { trussed::virt::with_ram_client("fido", |mut client| { From 5c3aa0b8af762f697ea5648c9fe44b3f69dea5cc Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 2 Dec 2024 14:17:58 +0100 Subject: [PATCH 2/3] Keep old credential ID for existing credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In #59, we changed the format for serialized credentials to use shorter field names for the RP and user entities. This has an unintended side effect: For non-discoverable credentials that were generated with older crate versions, the stripped data embedded into the credential ID includes the RP and user. If we change their serialization format, we also change these credential IDs. We already supported deserializing both formats using a serde alias. This patch introduces helper enums that deserialize both formats using a custom Deserialize implementation and keep track of the used format. This format is then also used for serialization (using serde’s untagged mechanism that is not available for deserialization in no-std contexts). https://github.com/Nitrokey/fido-authenticator/pull/59 Fixes: https://github.com/Nitrokey/fido-authenticator/issues/111 --- src/credential.rs | 784 ++++++++++++++++++----------- src/ctap2.rs | 14 +- src/ctap2/credential_management.rs | 22 +- 3 files changed, 520 insertions(+), 300 deletions(-) diff --git a/src/credential.rs b/src/credential.rs index 0279356..747269e 100644 --- a/src/credential.rs +++ b/src/credential.rs @@ -10,7 +10,10 @@ pub(crate) use ctap_types::{ // authenticator::{ctap1, ctap2, Error, Request, Response}, ctap2::credential_management::CredentialProtectionPolicy, sizes::*, - webauthn::{PublicKeyCredentialDescriptor, PublicKeyCredentialDescriptorRef}, + webauthn::{ + PublicKeyCredentialDescriptor, PublicKeyCredentialDescriptorRef, + PublicKeyCredentialRpEntity, PublicKeyCredentialUserEntity, + }, Bytes, String, }; @@ -205,31 +208,245 @@ impl Credential { } } -/// Copy of [`ctap_types::webauthn::PublicKeyCredentialUserEntity`] but with shorter field names serialization -#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq, Eq)] +fn deserialize_bytes( + s: &[u8], +) -> core::result::Result, E> { + Bytes::from_slice(s).map_err(|_| E::invalid_length(s.len(), &"a fixed-size sequence of bytes")) +} + +fn deserialize_str( + s: &str, +) -> core::result::Result, E> { + Ok(s.into()) +} + +#[derive(Clone, Debug, PartialEq, serde::Serialize)] +#[serde(untagged)] +pub enum Rp { + Long(PublicKeyCredentialRpEntity), + Short(LocalPublicKeyCredentialRpEntity), +} + +impl Rp { + pub fn id(&self) -> &str { + match self { + Self::Long(rp) => &rp.id, + Self::Short(rp) => &rp.id, + } + } + + pub fn strip(&mut self) { + let name = match self { + Self::Long(rp) => &mut rp.name, + Self::Short(rp) => &mut rp.name, + }; + *name = None; + } +} + +impl<'de> serde::Deserialize<'de> for Rp { + fn deserialize(deserializer: D) -> core::result::Result + where + D: serde::Deserializer<'de>, + { + use serde::de::Error as _; + + #[derive(serde::Deserialize)] + struct R<'a> { + i: Option<&'a str>, + id: Option<&'a str>, + n: Option<&'a str>, + name: Option<&'a str>, + } + + let r = R::deserialize(deserializer)?; + + if r.i.is_some() && r.id.is_some() { + return Err(D::Error::duplicate_field("i")); + } + + if let Some(i) = r.i { + // short format + if r.name.is_some() { + return Err(D::Error::unknown_field("name", &["i", "n"])); + } + + Ok(Self::Short(LocalPublicKeyCredentialRpEntity { + id: deserialize_str(i)?, + name: r.n.map(deserialize_str).transpose()?, + })) + } else if let Some(id) = r.id { + // long format + if r.n.is_some() { + return Err(D::Error::unknown_field("n", &["id", "name"])); + } + + Ok(Self::Long(PublicKeyCredentialRpEntity { + id: deserialize_str(id)?, + name: r.name.map(deserialize_str).transpose()?, + icon: None, + })) + } else { + // ID is missing + Err(D::Error::missing_field("i")) + } + } +} + +impl From for PublicKeyCredentialRpEntity { + fn from(rp: Rp) -> PublicKeyCredentialRpEntity { + match rp { + Rp::Short(rp) => rp.into(), + Rp::Long(rp) => rp, + } + } +} + +/// Copy of [`ctap_types::webauthn::PublicKeyCredentialRpEntity`] but with shorter field names serialization +#[derive(serde::Serialize, Debug, Clone, PartialEq, Eq)] pub struct LocalPublicKeyCredentialRpEntity { - #[serde(rename = "i", alias = "id")] + #[serde(rename = "i")] pub id: String<256>, // Compared to the ctap_types type, we can skip the truncate, // since we know we only even deal with the correct length - #[serde(skip_serializing_if = "Option::is_none", rename = "n", alias = "name")] + #[serde(skip_serializing_if = "Option::is_none", rename = "n")] pub name: Option>, // Icon is ignored } + +#[derive(Clone, Debug, PartialEq, serde::Serialize)] +#[serde(untagged)] +pub enum User { + Long(PublicKeyCredentialUserEntity), + Short(LocalPublicKeyCredentialUserEntity), +} + +impl User { + pub fn id(&self) -> &Bytes<64> { + match self { + Self::Long(user) => &user.id, + Self::Short(user) => &user.id, + } + } + + pub fn strip(&mut self) { + let (icon, name, display_name) = match self { + Self::Long(rp) => (&mut rp.icon, &mut rp.name, &mut rp.display_name), + Self::Short(rp) => (&mut rp.icon, &mut rp.name, &mut rp.display_name), + }; + *icon = None; + *name = None; + *display_name = None; + } + + pub fn set_name(&mut self, name: Option>) { + let field = match self { + Self::Long(rp) => &mut rp.name, + Self::Short(rp) => &mut rp.name, + }; + *field = name; + } + + pub fn set_display_name(&mut self, display_name: Option>) { + let field = match self { + Self::Long(rp) => &mut rp.display_name, + Self::Short(rp) => &mut rp.display_name, + }; + *field = display_name; + } +} + +impl<'de> serde::Deserialize<'de> for User { + fn deserialize(deserializer: D) -> core::result::Result + where + D: serde::Deserializer<'de>, + { + use serde::de::Error as _; + + #[derive(serde::Deserialize)] + struct U<'a> { + i: Option<&'a [u8]>, + id: Option<&'a [u8]>, + #[serde(rename = "I")] + ii: Option<&'a str>, + icon: Option<&'a str>, + n: Option<&'a str>, + name: Option<&'a str>, + d: Option<&'a str>, + #[serde(rename = "displayName")] + display_name: Option<&'a str>, + } + + let u = U::deserialize(deserializer)?; + + if u.i.is_some() && u.id.is_some() { + return Err(D::Error::duplicate_field("i")); + } + + if let Some(i) = u.i { + // short format + let fields = &["i", "I", "n", "d"]; + if u.icon.is_some() { + return Err(D::Error::unknown_field("icon", fields)); + } + if u.name.is_some() { + return Err(D::Error::unknown_field("name", fields)); + } + if u.display_name.is_some() { + return Err(D::Error::unknown_field("display_name", fields)); + } + + Ok(Self::Short(LocalPublicKeyCredentialUserEntity { + id: deserialize_bytes(i)?, + icon: u.ii.map(deserialize_str).transpose()?, + name: u.n.map(deserialize_str).transpose()?, + display_name: u.d.map(deserialize_str).transpose()?, + })) + } else if let Some(id) = u.id { + // long format + let fields = &["id", "icon", "name", "display_name"]; + if u.ii.is_some() { + return Err(D::Error::unknown_field("ii", fields)); + } + if u.n.is_some() { + return Err(D::Error::unknown_field("n", fields)); + } + if u.d.is_some() { + return Err(D::Error::unknown_field("d", fields)); + } + + Ok(Self::Long(PublicKeyCredentialUserEntity { + id: deserialize_bytes(id)?, + icon: u.icon.map(deserialize_str).transpose()?, + name: u.name.map(deserialize_str).transpose()?, + display_name: u.display_name.map(deserialize_str).transpose()?, + })) + } else { + // ID is missing + Err(D::Error::missing_field("i")) + } + } +} + +impl From for PublicKeyCredentialUserEntity { + fn from(user: User) -> PublicKeyCredentialUserEntity { + match user { + User::Short(user) => user.into(), + User::Long(user) => user, + } + } +} + /// Copy of [`ctap_types::webauthn::PublicKeyCredentialUserEntity`] but with with shorter field names serialization -#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq, Eq)] +#[derive(serde::Serialize, Debug, Clone, PartialEq, Eq)] pub struct LocalPublicKeyCredentialUserEntity { - #[serde(rename = "i", alias = "id")] + #[serde(rename = "i")] pub id: Bytes<64>, - #[serde(skip_serializing_if = "Option::is_none", rename = "I", alias = "icon")] + #[serde(skip_serializing_if = "Option::is_none", rename = "I")] pub icon: Option>, - #[serde(skip_serializing_if = "Option::is_none", rename = "n", alias = "name")] + #[serde(skip_serializing_if = "Option::is_none", rename = "n")] pub name: Option>, - #[serde( - skip_serializing_if = "Option::is_none", - rename = "d", - alias = "display_name" - )] + #[serde(skip_serializing_if = "Option::is_none", rename = "d")] pub display_name: Option>, } @@ -300,9 +517,9 @@ impl From )] pub struct CredentialData { // id, name, url - pub rp: LocalPublicKeyCredentialRpEntity, + pub rp: Rp, // id, icon, name, display_name - pub user: LocalPublicKeyCredentialUserEntity, + pub user: User, // can be just a counter, need to be able to determine "latest" pub creation_time: u32, @@ -436,8 +653,8 @@ impl FullCredential { ) -> Self { info!("credential for algorithm {}", algorithm); let data = CredentialData { - rp: rp.clone().into(), - user: user.clone().into(), + rp: Rp::Short(rp.clone().into()), + user: User::Short(user.clone().into()), creation_time: timestamp, use_counter: true, @@ -481,7 +698,7 @@ impl FullCredential { let rp_id_hash: [u8; 32] = if let Some(hash) = rp_id_hash { *hash } else { - syscall!(trussed.hash_sha256(self.rp.id.as_ref())) + syscall!(trussed.hash_sha256(self.rp.id().as_ref())) .hash .as_slice() .try_into() @@ -521,17 +738,8 @@ impl FullCredential { fn strip(&self) -> Self { info_now!(":: stripping ID"); let mut stripped = self.clone(); - let data = &mut stripped.data; - - data.rp.name = None; - - data.user.icon = None; - data.user.name = None; - data.user.display_name = None; - - // data.hmac_secret = None; - // data.cred_protect = None; - + stripped.data.rp.strip(); + stripped.data.user.strip(); stripped } } @@ -605,6 +813,7 @@ mod test { use littlefs2_core::path; use rand::SeedableRng as _; use rand_chacha::ChaCha8Rng; + use serde_test::{assert_de_tokens, assert_ser_tokens, Token}; use trussed::{ client::{Chacha8Poly1305, Sha256}, key::{Kind, Secrecy}, @@ -616,16 +825,16 @@ mod test { fn credential_data() -> CredentialData { CredentialData { - rp: LocalPublicKeyCredentialRpEntity { + rp: Rp::Short(LocalPublicKeyCredentialRpEntity { id: String::from("John Doe"), name: None, - }, - user: LocalPublicKeyCredentialUserEntity { + }), + user: User::Short(LocalPublicKeyCredentialUserEntity { id: Bytes::from_slice(&[1, 2, 3]).unwrap(), icon: None, name: None, display_name: None, - }, + }), creation_time: 123, use_counter: false, algorithm: -7, @@ -640,16 +849,17 @@ mod test { fn old_credential_data() -> CredentialData { CredentialData { - rp: LocalPublicKeyCredentialRpEntity { + rp: Rp::Long(PublicKeyCredentialRpEntity { id: String::from("John Doe"), name: None, - }, - user: LocalPublicKeyCredentialUserEntity { + icon: None, + }), + user: User::Short(LocalPublicKeyCredentialUserEntity { id: Bytes::from_slice(&[1, 2, 3]).unwrap(), icon: None, name: None, display_name: None, - }, + }), creation_time: 123, use_counter: false, algorithm: -7, @@ -726,16 +936,16 @@ mod test { fn random_credential_data() -> CredentialData { CredentialData { - rp: LocalPublicKeyCredentialRpEntity { + rp: Rp::Short(LocalPublicKeyCredentialRpEntity { id: random_string(), name: maybe_random_string(), - }, - user: LocalPublicKeyCredentialUserEntity { + }), + user: User::Short(LocalPublicKeyCredentialUserEntity { id: random_bytes(), //Bytes::from_slice(&[1,2,3]).unwrap(), icon: maybe_random_string(), name: maybe_random_string(), display_name: maybe_random_string(), - }, + }), creation_time: 123, use_counter: false, algorithm: -7, @@ -791,7 +1001,7 @@ mod test { }; platform.run_client(client_id.as_str(), |mut client| { let data = old_credential_data(); - let rp_id_hash = syscall!(client.hash_sha256(data.rp.id.as_ref())).hash; + let rp_id_hash = syscall!(client.hash_sha256(data.rp.id().as_ref())).hash; let credential_id = CredentialId(Bytes::from_slice(OLD_ID).unwrap()); let encrypted_serialized = EncryptedSerializedCredential::try_from(credential_id).unwrap(); @@ -855,7 +1065,7 @@ mod test { data, nonce, }; - let rp_id_hash = syscall!(client.hash_sha256(full_credential.rp.id.as_ref())) + let rp_id_hash = syscall!(client.hash_sha256(full_credential.rp.id().as_ref())) .hash .as_slice() .try_into() @@ -920,265 +1130,268 @@ mod test { }); } - #[test] - fn local_derive_rp_name_none() { - use serde_test::{assert_de_tokens, assert_tokens, Token}; - let rp_id = LocalPublicKeyCredentialRpEntity { - id: "Testing rp id".into(), - name: None, + struct RpValues { + id: &'static str, + name: Option<&'static str>, + } + + impl RpValues { + fn test(&self) { + RpType::SHORT.test(&self.short(), self); + RpType::LONG.test(&self.long(), self); + } + + fn short(&self) -> Rp { + Rp::Short(LocalPublicKeyCredentialRpEntity { + id: self.id.into(), + name: self.name.map(From::from), + }) + } + + fn long(&self) -> Rp { + Rp::Long(PublicKeyCredentialRpEntity { + id: self.id.into(), + name: self.name.map(From::from), + icon: None, + }) + } + } + + struct RpType { + s: &'static str, + id: &'static str, + name: &'static str, + } + + impl RpType { + const SHORT: Self = Self { + s: "LocalPublicKeyCredentialRpEntity", + id: "i", + name: "n", }; - assert_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialRpEntity", - len: 1, - }, - Token::Str("i"), - Token::Str("Testing rp id"), - Token::StructEnd, - ], - ); - assert_de_tokens( - &rp_id, - &[ - Token::Map { len: Some(1) }, - Token::Str("id"), - Token::Str("Testing rp id"), - Token::MapEnd, - ], - ); + const LONG: Self = Self { + s: "PublicKeyCredentialRpEntity", + id: "id", + name: "name", + }; + + fn test(&self, item: &Rp, values: &RpValues) { + let mut len = 1; + if values.name.is_some() { + len += 1; + } + + let mut ser_tokens = vec![Token::Struct { name: self.s, len }]; + ser_tokens.push(Token::Str(self.id)); + ser_tokens.push(Token::Str(values.id)); + if let Some(name) = values.name { + ser_tokens.push(Token::Str(self.name)); + ser_tokens.push(Token::Some); + ser_tokens.push(Token::Str(name)); + } + ser_tokens.push(Token::StructEnd); + assert_ser_tokens(item, &ser_tokens); + + let mut de_tokens = vec![Token::Map { len: Some(len) }]; + de_tokens.push(Token::Str(self.id)); + de_tokens.push(Token::Some); + de_tokens.push(Token::BorrowedStr(values.id)); + if let Some(name) = values.name { + de_tokens.push(Token::Str(self.name)); + de_tokens.push(Token::Some); + de_tokens.push(Token::BorrowedStr(name)); + } + de_tokens.push(Token::MapEnd); + assert_de_tokens(item, &de_tokens); + } } #[test] - fn local_derive_rp_name_some() { - use serde_test::{assert_de_tokens, assert_tokens, Token}; - let rp_id = LocalPublicKeyCredentialRpEntity { - id: "Testing rp id".into(), - name: Some("Testing rp name".into()), + fn serde_rp_name_none() { + RpValues { + id: "Testing rp id", + name: None, + } + .test() + } + + #[test] + fn serde_rp_name_some() { + RpValues { + id: "Testing rp id", + name: Some("Testing rp name"), + } + .test() + } + + struct UserValues { + id: &'static [u8], + icon: Option<&'static str>, + name: Option<&'static str>, + display_name: Option<&'static str>, + } + + impl UserValues { + fn test(&self) { + UserType::SHORT.test(&self.short(), self); + UserType::LONG.test(&self.long(), self); + } + + fn short(&self) -> User { + User::Short(LocalPublicKeyCredentialUserEntity { + id: Bytes::from_slice(self.id).unwrap(), + icon: self.icon.map(From::from), + name: self.name.map(From::from), + display_name: self.display_name.map(From::from), + }) + } + + fn long(&self) -> User { + User::Long(PublicKeyCredentialUserEntity { + id: Bytes::from_slice(self.id).unwrap(), + icon: self.icon.map(From::from), + name: self.name.map(From::from), + display_name: self.display_name.map(From::from), + }) + } + } + + struct UserType { + s: &'static str, + id: &'static str, + icon: &'static str, + name: &'static str, + display_name: &'static str, + } + + impl UserType { + const SHORT: Self = Self { + s: "LocalPublicKeyCredentialUserEntity", + id: "i", + icon: "I", + name: "n", + display_name: "d", }; - assert_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialRpEntity", - len: 2, - }, - Token::Str("i"), - Token::Str("Testing rp id"), - Token::Str("n"), - Token::Some, - Token::Str("Testing rp name"), - Token::StructEnd, - ], - ); - assert_de_tokens( - &rp_id, - &[ - Token::Map { len: Some(2) }, - Token::Str("id"), - Token::Str("Testing rp id"), - Token::Str("name"), - Token::Some, - Token::Str("Testing rp name"), - Token::MapEnd, - ], - ); + const LONG: Self = Self { + s: "PublicKeyCredentialUserEntity", + id: "id", + icon: "icon", + name: "name", + display_name: "displayName", + }; + + fn test(&self, user: &User, values: &UserValues) { + let mut len = 1; + if values.icon.is_some() { + len += 1; + } + if values.name.is_some() { + len += 1; + } + if values.display_name.is_some() { + len += 1; + } + + let mut ser_tokens = vec![Token::Struct { name: self.s, len }]; + ser_tokens.push(Token::Str(self.id)); + ser_tokens.push(Token::Bytes(values.id)); + if let Some(icon) = values.icon { + ser_tokens.push(Token::Str(self.icon)); + ser_tokens.push(Token::Some); + ser_tokens.push(Token::Str(icon)); + } + if let Some(name) = values.name { + ser_tokens.push(Token::Str(self.name)); + ser_tokens.push(Token::Some); + ser_tokens.push(Token::Str(name)); + } + if let Some(display_name) = values.display_name { + ser_tokens.push(Token::Str(self.display_name)); + ser_tokens.push(Token::Some); + ser_tokens.push(Token::Str(display_name)); + } + ser_tokens.push(Token::StructEnd); + assert_ser_tokens(user, &ser_tokens); + + let mut de_tokens = vec![Token::Map { len: Some(len) }]; + de_tokens.push(Token::Str(self.id)); + de_tokens.push(Token::Some); + de_tokens.push(Token::BorrowedBytes(values.id)); + if let Some(icon) = values.icon { + de_tokens.push(Token::Str(self.icon)); + de_tokens.push(Token::Some); + de_tokens.push(Token::BorrowedStr(icon)); + } + if let Some(name) = values.name { + de_tokens.push(Token::Str(self.name)); + de_tokens.push(Token::Some); + de_tokens.push(Token::BorrowedStr(name)); + } + if let Some(display_name) = values.display_name { + de_tokens.push(Token::Str(self.display_name)); + de_tokens.push(Token::Some); + de_tokens.push(Token::BorrowedStr(display_name)); + } + de_tokens.push(Token::MapEnd); + assert_de_tokens(user, &de_tokens); + } } #[test] - fn local_derive_user() { - use serde_test::{assert_de_tokens, assert_tokens, Token}; - - let rp_id = LocalPublicKeyCredentialUserEntity { - id: Bytes::from_slice(b"Testing user id").unwrap(), - icon: Some("Testing user icon".into()), - name: Some("Testing user name".into()), - display_name: Some("Testing user display_name".into()), - }; - assert_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialUserEntity", - len: 4, - }, - Token::Str("i"), - Token::Bytes(b"Testing user id"), - Token::Str("I"), - Token::Some, - Token::Str("Testing user icon"), - Token::Str("n"), - Token::Some, - Token::Str("Testing user name"), - Token::Str("d"), - Token::Some, - Token::Str("Testing user display_name"), - Token::StructEnd, - ], - ); - assert_de_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialUserEntity", - len: 4, - }, - Token::Str("id"), - Token::Bytes(b"Testing user id"), - Token::Str("icon"), - Token::Some, - Token::Str("Testing user icon"), - Token::Str("name"), - Token::Some, - Token::Str("Testing user name"), - Token::Str("display_name"), - Token::Some, - Token::Str("Testing user display_name"), - Token::StructEnd, - ], - ); + fn serde_user_full() { + UserValues { + id: b"Testing user id", + icon: Some("Testing user icon"), + name: Some("Testing user name"), + display_name: Some("Testing user display_name"), + } + .test(); + } - let rp_id = LocalPublicKeyCredentialUserEntity { - id: Bytes::from_slice(b"Testing user id").unwrap(), + #[test] + fn serde_user_display_name() { + UserValues { + id: b"Testing user id", icon: None, name: None, - display_name: Some("Testing user display_name".into()), - }; - assert_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialUserEntity", - len: 2, - }, - Token::Str("i"), - Token::Bytes(b"Testing user id"), - Token::Str("d"), - Token::Some, - Token::Str("Testing user display_name"), - Token::StructEnd, - ], - ); - assert_de_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialUserEntity", - len: 2, - }, - Token::Str("id"), - Token::Bytes(b"Testing user id"), - Token::Str("display_name"), - Token::Some, - Token::Str("Testing user display_name"), - Token::StructEnd, - ], - ); + display_name: Some("Testing user display_name"), + } + .test(); + } - let rp_id = LocalPublicKeyCredentialUserEntity { - id: Bytes::from_slice(b"Testing user id").unwrap(), - icon: Some("Testing user icon".into()), + #[test] + fn serde_user_icon_display_name() { + UserValues { + id: b"Testing user id", + icon: Some("Testing user icon"), name: None, - display_name: Some("Testing user display_name".into()), - }; - assert_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialUserEntity", - len: 3, - }, - Token::Str("i"), - Token::Bytes(b"Testing user id"), - Token::Str("I"), - Token::Some, - Token::Str("Testing user icon"), - Token::Str("d"), - Token::Some, - Token::Str("Testing user display_name"), - Token::StructEnd, - ], - ); - assert_de_tokens( - &rp_id, - &[ - Token::Map { len: Some(3) }, - Token::Str("id"), - Token::Bytes(b"Testing user id"), - Token::Str("icon"), - Token::Some, - Token::Str("Testing user icon"), - Token::Str("display_name"), - Token::Some, - Token::Str("Testing user display_name"), - Token::MapEnd, - ], - ); + display_name: Some("Testing user display_name"), + } + .test(); + } - let rp_id = LocalPublicKeyCredentialUserEntity { - id: Bytes::from_slice(b"Testing user id").unwrap(), - icon: Some("Testing user icon".into()), + #[test] + fn serde_user_icon() { + UserValues { + id: b"Testing user id", + icon: Some("Testing user icon"), name: None, display_name: None, - }; - assert_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialUserEntity", - len: 2, - }, - Token::Str("i"), - Token::Bytes(b"Testing user id"), - Token::Str("I"), - Token::Some, - Token::Str("Testing user icon"), - Token::StructEnd, - ], - ); - assert_de_tokens( - &rp_id, - &[ - Token::Map { len: Some(2) }, - Token::Str("id"), - Token::Bytes(b"Testing user id"), - Token::Str("icon"), - Token::Some, - Token::Str("Testing user icon"), - Token::MapEnd, - ], - ); + } + .test(); + } - let rp_id = LocalPublicKeyCredentialUserEntity { - id: Bytes::from_slice(b"Testing user id").unwrap(), + #[test] + fn serde_user_empty() { + UserValues { + id: b"Testing user id", icon: None, name: None, display_name: None, - }; - assert_tokens( - &rp_id, - &[ - Token::Struct { - name: "LocalPublicKeyCredentialUserEntity", - len: 1, - }, - Token::Str("i"), - Token::Bytes(b"Testing user id"), - Token::StructEnd, - ], - ); - assert_de_tokens( - &rp_id, - &[ - Token::Map { len: Some(1) }, - Token::Str("id"), - Token::Bytes(b"Testing user id"), - Token::MapEnd, - ], - ); + } + .test(); } // Test credentials that were serialized before the migration to shorter field names for serialization @@ -1199,16 +1412,17 @@ mod test { assert_eq!( credential.data, CredentialData { - rp: LocalPublicKeyCredentialRpEntity { + rp: Rp::Long(PublicKeyCredentialRpEntity { id: "webauthn.io".into(), name: None, - }, - user: LocalPublicKeyCredentialUserEntity { + icon: None, + }), + user: User::Long(PublicKeyCredentialUserEntity { id: Bytes::from_slice(&hex!("6447567A644445")).unwrap(), icon: None, name: Some("test1".into()), display_name: None, - }, + }), creation_time: 0, use_counter: true, algorithm: -7, diff --git a/src/ctap2.rs b/src/ctap2.rs index 1a9b82b..58fa72b 100644 --- a/src/ctap2.rs +++ b/src/ctap2.rs @@ -9,7 +9,9 @@ use ctap_types::{ }, heapless::{String, Vec}, heapless_bytes::Bytes, - sizes, ByteArray, Error, + sizes, + webauthn::PublicKeyCredentialUserEntity, + ByteArray, Error, }; use littlefs2_core::path; use sha2::{Digest as _, Sha256}; @@ -388,7 +390,7 @@ impl Authenticator for crate::Authenti let serialized_credential = credential.serialize()?; // first delete any other RK cred with same RP + UserId if there is one. - self.delete_resident_key_by_user_id(&rp_id_hash, &credential.user.id) + self.delete_resident_key_by_user_id(&rp_id_hash, credential.user.id()) .ok(); let mut key_store_full = self.can_fit(serialized_credential.len()) == Some(false) @@ -1739,8 +1741,8 @@ impl crate::Authenticator { // User with empty IDs are ignored for compatibility if is_rk { if let Credential::Full(credential) = &credential { - if !credential.user.id.is_empty() { - let mut user = credential.user.clone(); + if !credential.user.id().is_empty() { + let mut user: PublicKeyCredentialUserEntity = credential.user.clone().into(); // User identifiable information (name, DisplayName, icon) MUST not // be returned if user verification is not done by the authenticator. // For single account per RP case, authenticator returns "id" field. @@ -1749,7 +1751,7 @@ impl crate::Authenticator { user.name = None; user.display_name = None; } - response.user = Some(user.into()); + response.user = Some(user); } } @@ -1793,7 +1795,7 @@ impl crate::Authenticator { let credential_maybe = FullCredential::deserialize(&credential_data); if let Ok(old_credential) = credential_maybe { - if old_credential.user.id == user_id { + if old_credential.user.id() == user_id { match old_credential.key { credential::Key::ResidentKey(key) => { info_now!(":: deleting resident key"); diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index 99f84e2..c1d99d6 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -174,7 +174,7 @@ where let rp = credential.data.rp; - response.rp_id_hash = Some(ByteArray::new(self.hash(rp.id.as_ref()))); + response.rp_id_hash = Some(ByteArray::new(self.hash(rp.id().as_ref()))); response.rp = Some(rp.into()); } } @@ -251,7 +251,7 @@ where let rp = credential.data.rp; - response.rp_id_hash = Some(ByteArray::new(self.hash(rp.id.as_ref()))); + response.rp_id_hash = Some(ByteArray::new(self.hash(rp.id().as_ref()))); response.rp = Some(rp.into()); // cache state for next call @@ -524,18 +524,22 @@ where // TODO: check remaining space, return KeyStoreFull // the updated user ID must match the stored user ID - if credential.user.id != user.id { + if credential.user.id() != &user.id { error!("updated user ID does not match original user ID"); return Err(Error::InvalidParameter); } // update user name and display name unless the values are not set or empty - credential.data.user.name = user.name.as_ref().filter(|s| !s.is_empty()).cloned(); - credential.data.user.display_name = user - .display_name - .as_ref() - .filter(|s| !s.is_empty()) - .cloned(); + credential + .data + .user + .set_name(user.name.as_ref().filter(|s| !s.is_empty()).cloned()); + credential.data.user.set_display_name( + user.display_name + .as_ref() + .filter(|s| !s.is_empty()) + .cloned(), + ); // write updated credential let serialized = credential.serialize()?; From 63a14793877f49e0bd6a99d28834a0013fdb9d64 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 2 Dec 2024 17:57:31 +0100 Subject: [PATCH 3/3] Remove LocalPublicKeyCredential*Entity structs To simplify the Rp and User types introduced in the last commit and to reduce code duplication, this patch removes the LocalPublicKeyCredential*Entity types. Instead the Rp and User types always store a PublicKeyCredential*Entity together with the serialization format. --- src/credential.rs | 651 +++++++++++++---------------- src/ctap2/credential_management.rs | 17 +- 2 files changed, 287 insertions(+), 381 deletions(-) diff --git a/src/credential.rs b/src/credential.rs index 747269e..cf17cda 100644 --- a/src/credential.rs +++ b/src/credential.rs @@ -220,27 +220,55 @@ fn deserialize_str( Ok(s.into()) } -#[derive(Clone, Debug, PartialEq, serde::Serialize)] -#[serde(untagged)] -pub enum Rp { - Long(PublicKeyCredentialRpEntity), - Short(LocalPublicKeyCredentialRpEntity), +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum SerializationFormat { + Short, + Long, +} + +#[derive(Clone, Debug, PartialEq)] +pub struct Rp { + format: SerializationFormat, + inner: PublicKeyCredentialRpEntity, } impl Rp { - pub fn id(&self) -> &str { - match self { - Self::Long(rp) => &rp.id, - Self::Short(rp) => &rp.id, + fn new(inner: PublicKeyCredentialRpEntity) -> Self { + Self { + format: SerializationFormat::Short, + inner, } } - pub fn strip(&mut self) { - let name = match self { - Self::Long(rp) => &mut rp.name, - Self::Short(rp) => &mut rp.name, - }; - *name = None; + fn raw(&self) -> RawRp<'_> { + let mut raw = RawRp::default(); + match self.format { + SerializationFormat::Short => { + raw.i = Some(&self.inner.id); + raw.n = self.inner.name.as_deref(); + } + SerializationFormat::Long => { + raw.id = Some(&self.inner.id); + raw.name = self.inner.name.as_deref(); + } + } + raw + } + + pub fn id(&self) -> &str { + &self.inner.id + } +} + +impl AsRef for Rp { + fn as_ref(&self) -> &PublicKeyCredentialRpEntity { + &self.inner + } +} + +impl AsMut for Rp { + fn as_mut(&mut self) -> &mut PublicKeyCredentialRpEntity { + &mut self.inner } } @@ -251,108 +279,109 @@ impl<'de> serde::Deserialize<'de> for Rp { { use serde::de::Error as _; - #[derive(serde::Deserialize)] - struct R<'a> { - i: Option<&'a str>, - id: Option<&'a str>, - n: Option<&'a str>, - name: Option<&'a str>, - } - - let r = R::deserialize(deserializer)?; + let r = RawRp::deserialize(deserializer)?; if r.i.is_some() && r.id.is_some() { return Err(D::Error::duplicate_field("i")); } - if let Some(i) = r.i { - // short format + let (format, id, name) = if let Some(i) = r.i { if r.name.is_some() { return Err(D::Error::unknown_field("name", &["i", "n"])); } - - Ok(Self::Short(LocalPublicKeyCredentialRpEntity { - id: deserialize_str(i)?, - name: r.n.map(deserialize_str).transpose()?, - })) + (SerializationFormat::Short, i, r.n) } else if let Some(id) = r.id { - // long format if r.n.is_some() { return Err(D::Error::unknown_field("n", &["id", "name"])); } - - Ok(Self::Long(PublicKeyCredentialRpEntity { - id: deserialize_str(id)?, - name: r.name.map(deserialize_str).transpose()?, - icon: None, - })) + (SerializationFormat::Long, id, r.name) } else { - // ID is missing - Err(D::Error::missing_field("i")) - } + return Err(D::Error::missing_field("i")); + }; + + let inner = PublicKeyCredentialRpEntity { + id: deserialize_str(id)?, + name: name.map(deserialize_str).transpose()?, + icon: None, + }; + Ok(Self { format, inner }) + } +} + +impl serde::Serialize for Rp { + fn serialize( + &self, + serializer: S, + ) -> core::result::Result { + self.raw().serialize(serializer) } } impl From for PublicKeyCredentialRpEntity { fn from(rp: Rp) -> PublicKeyCredentialRpEntity { - match rp { - Rp::Short(rp) => rp.into(), - Rp::Long(rp) => rp, - } + rp.inner } } -/// Copy of [`ctap_types::webauthn::PublicKeyCredentialRpEntity`] but with shorter field names serialization -#[derive(serde::Serialize, Debug, Clone, PartialEq, Eq)] -pub struct LocalPublicKeyCredentialRpEntity { - #[serde(rename = "i")] - pub id: String<256>, - // Compared to the ctap_types type, we can skip the truncate, - // since we know we only even deal with the correct length - #[serde(skip_serializing_if = "Option::is_none", rename = "n")] - pub name: Option>, - // Icon is ignored +#[derive(Default, serde::Deserialize, serde::Serialize)] +struct RawRp<'a> { + #[serde(skip_serializing_if = "Option::is_none")] + i: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + id: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + n: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + name: Option<&'a str>, } -#[derive(Clone, Debug, PartialEq, serde::Serialize)] -#[serde(untagged)] -pub enum User { - Long(PublicKeyCredentialUserEntity), - Short(LocalPublicKeyCredentialUserEntity), +#[derive(Clone, Debug, PartialEq)] +pub struct User { + format: SerializationFormat, + inner: PublicKeyCredentialUserEntity, } impl User { - pub fn id(&self) -> &Bytes<64> { - match self { - Self::Long(user) => &user.id, - Self::Short(user) => &user.id, + fn new(inner: PublicKeyCredentialUserEntity) -> Self { + Self { + format: SerializationFormat::Short, + inner, } } - pub fn strip(&mut self) { - let (icon, name, display_name) = match self { - Self::Long(rp) => (&mut rp.icon, &mut rp.name, &mut rp.display_name), - Self::Short(rp) => (&mut rp.icon, &mut rp.name, &mut rp.display_name), - }; - *icon = None; - *name = None; - *display_name = None; + fn raw(&self) -> RawUser<'_> { + let mut raw = RawUser::default(); + match self.format { + SerializationFormat::Short => { + raw.i = Some(self.inner.id.as_slice().into()); + raw.ii = self.inner.icon.as_deref(); + raw.n = self.inner.name.as_deref(); + raw.d = self.inner.display_name.as_deref(); + } + SerializationFormat::Long => { + raw.id = Some(self.inner.id.as_slice().into()); + raw.icon = self.inner.icon.as_deref(); + raw.name = self.inner.name.as_deref(); + raw.display_name = self.inner.display_name.as_deref(); + } + } + raw } - pub fn set_name(&mut self, name: Option>) { - let field = match self { - Self::Long(rp) => &mut rp.name, - Self::Short(rp) => &mut rp.name, - }; - *field = name; + pub fn id(&self) -> &Bytes<64> { + &self.inner.id } +} - pub fn set_display_name(&mut self, display_name: Option>) { - let field = match self { - Self::Long(rp) => &mut rp.display_name, - Self::Short(rp) => &mut rp.display_name, - }; - *field = display_name; +impl AsRef for User { + fn as_ref(&self) -> &PublicKeyCredentialUserEntity { + &self.inner + } +} + +impl AsMut for User { + fn as_mut(&mut self) -> &mut PublicKeyCredentialUserEntity { + &mut self.inner } } @@ -363,27 +392,13 @@ impl<'de> serde::Deserialize<'de> for User { { use serde::de::Error as _; - #[derive(serde::Deserialize)] - struct U<'a> { - i: Option<&'a [u8]>, - id: Option<&'a [u8]>, - #[serde(rename = "I")] - ii: Option<&'a str>, - icon: Option<&'a str>, - n: Option<&'a str>, - name: Option<&'a str>, - d: Option<&'a str>, - #[serde(rename = "displayName")] - display_name: Option<&'a str>, - } - - let u = U::deserialize(deserializer)?; + let u = RawUser::deserialize(deserializer)?; if u.i.is_some() && u.id.is_some() { return Err(D::Error::duplicate_field("i")); } - if let Some(i) = u.i { + let (format, id, icon, name, display_name) = if let Some(i) = u.i { // short format let fields = &["i", "I", "n", "d"]; if u.icon.is_some() { @@ -396,12 +411,7 @@ impl<'de> serde::Deserialize<'de> for User { return Err(D::Error::unknown_field("display_name", fields)); } - Ok(Self::Short(LocalPublicKeyCredentialUserEntity { - id: deserialize_bytes(i)?, - icon: u.ii.map(deserialize_str).transpose()?, - name: u.n.map(deserialize_str).transpose()?, - display_name: u.d.map(deserialize_str).transpose()?, - })) + (SerializationFormat::Short, i, u.ii, u.n, u.d) } else if let Some(id) = u.id { // long format let fields = &["id", "icon", "name", "display_name"]; @@ -415,100 +425,61 @@ impl<'de> serde::Deserialize<'de> for User { return Err(D::Error::unknown_field("d", fields)); } - Ok(Self::Long(PublicKeyCredentialUserEntity { - id: deserialize_bytes(id)?, - icon: u.icon.map(deserialize_str).transpose()?, - name: u.name.map(deserialize_str).transpose()?, - display_name: u.display_name.map(deserialize_str).transpose()?, - })) + ( + SerializationFormat::Long, + id, + u.icon, + u.name, + u.display_name, + ) } else { // ID is missing - Err(D::Error::missing_field("i")) - } - } -} - -impl From for PublicKeyCredentialUserEntity { - fn from(user: User) -> PublicKeyCredentialUserEntity { - match user { - User::Short(user) => user.into(), - User::Long(user) => user, - } - } -} - -/// Copy of [`ctap_types::webauthn::PublicKeyCredentialUserEntity`] but with with shorter field names serialization -#[derive(serde::Serialize, Debug, Clone, PartialEq, Eq)] -pub struct LocalPublicKeyCredentialUserEntity { - #[serde(rename = "i")] - pub id: Bytes<64>, - #[serde(skip_serializing_if = "Option::is_none", rename = "I")] - pub icon: Option>, - #[serde(skip_serializing_if = "Option::is_none", rename = "n")] - pub name: Option>, - #[serde(skip_serializing_if = "Option::is_none", rename = "d")] - pub display_name: Option>, -} - -impl From for LocalPublicKeyCredentialRpEntity { - fn from(value: ctap_types::webauthn::PublicKeyCredentialRpEntity) -> Self { - let ctap_types::webauthn::PublicKeyCredentialRpEntity { id, name, icon } = value; - let _icon = icon; + return Err(D::Error::missing_field("i")); + }; - Self { id, name } + let inner = PublicKeyCredentialUserEntity { + id: deserialize_bytes(id)?, + icon: icon.map(deserialize_str).transpose()?, + name: name.map(deserialize_str).transpose()?, + display_name: display_name.map(deserialize_str).transpose()?, + }; + Ok(Self { format, inner }) } } -impl From for ctap_types::webauthn::PublicKeyCredentialRpEntity { - fn from(value: LocalPublicKeyCredentialRpEntity) -> Self { - let LocalPublicKeyCredentialRpEntity { id, name } = value; - - Self { - id, - name, - icon: None, - } +impl serde::Serialize for User { + fn serialize( + &self, + serializer: S, + ) -> core::result::Result { + self.raw().serialize(serializer) } } -impl From - for LocalPublicKeyCredentialUserEntity -{ - fn from(value: ctap_types::webauthn::PublicKeyCredentialUserEntity) -> Self { - let ctap_types::webauthn::PublicKeyCredentialUserEntity { - id, - icon, - name, - display_name, - } = value; - - Self { - id, - icon, - name, - display_name, - } +impl From for PublicKeyCredentialUserEntity { + fn from(user: User) -> PublicKeyCredentialUserEntity { + user.inner } } -impl From - for ctap_types::webauthn::PublicKeyCredentialUserEntity -{ - fn from(value: LocalPublicKeyCredentialUserEntity) -> Self { - let LocalPublicKeyCredentialUserEntity { - id, - icon, - name, - display_name, - } = value; - - Self { - id, - icon, - name, - display_name, - } - } +#[derive(Default, serde::Deserialize, serde::Serialize)] +struct RawUser<'a> { + #[serde(skip_serializing_if = "Option::is_none")] + i: Option<&'a serde_bytes::Bytes>, + #[serde(skip_serializing_if = "Option::is_none")] + id: Option<&'a serde_bytes::Bytes>, + #[serde(skip_serializing_if = "Option::is_none", rename = "I")] + ii: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + icon: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + n: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + name: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + d: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none", rename = "displayName")] + display_name: Option<&'a str>, } /// The main content of a `FullCredential`. @@ -653,8 +624,8 @@ impl FullCredential { ) -> Self { info!("credential for algorithm {}", algorithm); let data = CredentialData { - rp: Rp::Short(rp.clone().into()), - user: User::Short(user.clone().into()), + rp: Rp::new(rp.clone()), + user: User::new(user.clone()), creation_time: timestamp, use_counter: true, @@ -738,8 +709,12 @@ impl FullCredential { fn strip(&self) -> Self { info_now!(":: stripping ID"); let mut stripped = self.clone(); - stripped.data.rp.strip(); - stripped.data.user.strip(); + let rp = stripped.data.rp.as_mut(); + rp.name = None; + let user = stripped.data.user.as_mut(); + user.icon = None; + user.name = None; + user.display_name = None; stripped } } @@ -813,7 +788,7 @@ mod test { use littlefs2_core::path; use rand::SeedableRng as _; use rand_chacha::ChaCha8Rng; - use serde_test::{assert_de_tokens, assert_ser_tokens, Token}; + use serde_test::{assert_de_tokens, assert_tokens, Token}; use trussed::{ client::{Chacha8Poly1305, Sha256}, key::{Kind, Secrecy}, @@ -825,11 +800,12 @@ mod test { fn credential_data() -> CredentialData { CredentialData { - rp: Rp::Short(LocalPublicKeyCredentialRpEntity { + rp: Rp::new(PublicKeyCredentialRpEntity { id: String::from("John Doe"), name: None, + icon: None, }), - user: User::Short(LocalPublicKeyCredentialUserEntity { + user: User::new(PublicKeyCredentialUserEntity { id: Bytes::from_slice(&[1, 2, 3]).unwrap(), icon: None, name: None, @@ -849,17 +825,23 @@ mod test { fn old_credential_data() -> CredentialData { CredentialData { - rp: Rp::Long(PublicKeyCredentialRpEntity { - id: String::from("John Doe"), - name: None, - icon: None, - }), - user: User::Short(LocalPublicKeyCredentialUserEntity { - id: Bytes::from_slice(&[1, 2, 3]).unwrap(), - icon: None, - name: None, - display_name: None, - }), + rp: Rp { + format: SerializationFormat::Long, + inner: PublicKeyCredentialRpEntity { + id: String::from("John Doe"), + name: None, + icon: None, + }, + }, + user: User { + format: SerializationFormat::Long, + inner: PublicKeyCredentialUserEntity { + id: Bytes::from_slice(&[1, 2, 3]).unwrap(), + icon: None, + name: None, + display_name: None, + }, + }, creation_time: 123, use_counter: false, algorithm: -7, @@ -936,11 +918,12 @@ mod test { fn random_credential_data() -> CredentialData { CredentialData { - rp: Rp::Short(LocalPublicKeyCredentialRpEntity { + rp: Rp::new(PublicKeyCredentialRpEntity { id: random_string(), name: maybe_random_string(), + icon: None, }), - user: User::Short(LocalPublicKeyCredentialUserEntity { + user: User::new(PublicKeyCredentialUserEntity { id: random_bytes(), //Bytes::from_slice(&[1,2,3]).unwrap(), icon: maybe_random_string(), name: maybe_random_string(), @@ -1130,6 +1113,30 @@ mod test { }); } + fn test_serde(item: &T, name: &'static str, fields: &[(&'static str, Token)]) + where + for<'a> T: core::fmt::Debug + PartialEq + serde::Deserialize<'a> + serde::Serialize, + { + let len = fields.len(); + + let mut struct_tokens = vec![Token::Struct { name, len }]; + let mut map_tokens = vec![Token::Map { len: Some(len) }]; + for (key, value) in fields { + struct_tokens.push(Token::Str(key)); + struct_tokens.push(Token::Some); + struct_tokens.push(*value); + + map_tokens.push(Token::Str(key)); + map_tokens.push(Token::Some); + map_tokens.push(*value); + } + struct_tokens.push(Token::StructEnd); + map_tokens.push(Token::MapEnd); + + assert_tokens(item, &struct_tokens); + assert_de_tokens(item, &map_tokens); + } + struct RpValues { id: &'static str, name: Option<&'static str>, @@ -1137,73 +1144,35 @@ mod test { impl RpValues { fn test(&self) { - RpType::SHORT.test(&self.short(), self); - RpType::LONG.test(&self.long(), self); + for format in [SerializationFormat::Short, SerializationFormat::Long] { + self.test_format(format); + } } - fn short(&self) -> Rp { - Rp::Short(LocalPublicKeyCredentialRpEntity { - id: self.id.into(), - name: self.name.map(From::from), - }) + fn test_format(&self, format: SerializationFormat) { + let (id_field, name_field) = match format { + SerializationFormat::Short => ("i", "n"), + SerializationFormat::Long => ("id", "name"), + }; + let rp = Rp { + format, + inner: self.inner(), + }; + + let mut fields = vec![(id_field, Token::BorrowedStr(self.id))]; + if let Some(name) = self.name { + fields.push((name_field, Token::BorrowedStr(name))); + } + + test_serde(&rp, "RawRp", &fields); } - fn long(&self) -> Rp { - Rp::Long(PublicKeyCredentialRpEntity { + fn inner(&self) -> PublicKeyCredentialRpEntity { + PublicKeyCredentialRpEntity { id: self.id.into(), name: self.name.map(From::from), icon: None, - }) - } - } - - struct RpType { - s: &'static str, - id: &'static str, - name: &'static str, - } - - impl RpType { - const SHORT: Self = Self { - s: "LocalPublicKeyCredentialRpEntity", - id: "i", - name: "n", - }; - - const LONG: Self = Self { - s: "PublicKeyCredentialRpEntity", - id: "id", - name: "name", - }; - - fn test(&self, item: &Rp, values: &RpValues) { - let mut len = 1; - if values.name.is_some() { - len += 1; } - - let mut ser_tokens = vec![Token::Struct { name: self.s, len }]; - ser_tokens.push(Token::Str(self.id)); - ser_tokens.push(Token::Str(values.id)); - if let Some(name) = values.name { - ser_tokens.push(Token::Str(self.name)); - ser_tokens.push(Token::Some); - ser_tokens.push(Token::Str(name)); - } - ser_tokens.push(Token::StructEnd); - assert_ser_tokens(item, &ser_tokens); - - let mut de_tokens = vec![Token::Map { len: Some(len) }]; - de_tokens.push(Token::Str(self.id)); - de_tokens.push(Token::Some); - de_tokens.push(Token::BorrowedStr(values.id)); - if let Some(name) = values.name { - de_tokens.push(Token::Str(self.name)); - de_tokens.push(Token::Some); - de_tokens.push(Token::BorrowedStr(name)); - } - de_tokens.push(Token::MapEnd); - assert_de_tokens(item, &de_tokens); } } @@ -1234,108 +1203,42 @@ mod test { impl UserValues { fn test(&self) { - UserType::SHORT.test(&self.short(), self); - UserType::LONG.test(&self.long(), self); + for format in [SerializationFormat::Short, SerializationFormat::Long] { + self.test_format(format); + } } - fn short(&self) -> User { - User::Short(LocalPublicKeyCredentialUserEntity { - id: Bytes::from_slice(self.id).unwrap(), - icon: self.icon.map(From::from), - name: self.name.map(From::from), - display_name: self.display_name.map(From::from), - }) + fn test_format(&self, format: SerializationFormat) { + let (id_field, icon_field, name_field, display_name_field) = match format { + SerializationFormat::Short => ("i", "I", "n", "d"), + SerializationFormat::Long => ("id", "icon", "name", "displayName"), + }; + let user = User { + format, + inner: self.inner(), + }; + + let mut fields = vec![(id_field, Token::BorrowedBytes(self.id))]; + if let Some(icon) = self.icon { + fields.push((icon_field, Token::BorrowedStr(icon))); + } + if let Some(name) = self.name { + fields.push((name_field, Token::BorrowedStr(name))); + } + if let Some(display_name) = self.display_name { + fields.push((display_name_field, Token::BorrowedStr(display_name))); + } + + test_serde(&user, "RawUser", &fields); } - fn long(&self) -> User { - User::Long(PublicKeyCredentialUserEntity { + fn inner(&self) -> PublicKeyCredentialUserEntity { + PublicKeyCredentialUserEntity { id: Bytes::from_slice(self.id).unwrap(), icon: self.icon.map(From::from), name: self.name.map(From::from), display_name: self.display_name.map(From::from), - }) - } - } - - struct UserType { - s: &'static str, - id: &'static str, - icon: &'static str, - name: &'static str, - display_name: &'static str, - } - - impl UserType { - const SHORT: Self = Self { - s: "LocalPublicKeyCredentialUserEntity", - id: "i", - icon: "I", - name: "n", - display_name: "d", - }; - - const LONG: Self = Self { - s: "PublicKeyCredentialUserEntity", - id: "id", - icon: "icon", - name: "name", - display_name: "displayName", - }; - - fn test(&self, user: &User, values: &UserValues) { - let mut len = 1; - if values.icon.is_some() { - len += 1; - } - if values.name.is_some() { - len += 1; } - if values.display_name.is_some() { - len += 1; - } - - let mut ser_tokens = vec![Token::Struct { name: self.s, len }]; - ser_tokens.push(Token::Str(self.id)); - ser_tokens.push(Token::Bytes(values.id)); - if let Some(icon) = values.icon { - ser_tokens.push(Token::Str(self.icon)); - ser_tokens.push(Token::Some); - ser_tokens.push(Token::Str(icon)); - } - if let Some(name) = values.name { - ser_tokens.push(Token::Str(self.name)); - ser_tokens.push(Token::Some); - ser_tokens.push(Token::Str(name)); - } - if let Some(display_name) = values.display_name { - ser_tokens.push(Token::Str(self.display_name)); - ser_tokens.push(Token::Some); - ser_tokens.push(Token::Str(display_name)); - } - ser_tokens.push(Token::StructEnd); - assert_ser_tokens(user, &ser_tokens); - - let mut de_tokens = vec![Token::Map { len: Some(len) }]; - de_tokens.push(Token::Str(self.id)); - de_tokens.push(Token::Some); - de_tokens.push(Token::BorrowedBytes(values.id)); - if let Some(icon) = values.icon { - de_tokens.push(Token::Str(self.icon)); - de_tokens.push(Token::Some); - de_tokens.push(Token::BorrowedStr(icon)); - } - if let Some(name) = values.name { - de_tokens.push(Token::Str(self.name)); - de_tokens.push(Token::Some); - de_tokens.push(Token::BorrowedStr(name)); - } - if let Some(display_name) = values.display_name { - de_tokens.push(Token::Str(self.display_name)); - de_tokens.push(Token::Some); - de_tokens.push(Token::BorrowedStr(display_name)); - } - de_tokens.push(Token::MapEnd); - assert_de_tokens(user, &de_tokens); } } @@ -1412,17 +1315,23 @@ mod test { assert_eq!( credential.data, CredentialData { - rp: Rp::Long(PublicKeyCredentialRpEntity { - id: "webauthn.io".into(), - name: None, - icon: None, - }), - user: User::Long(PublicKeyCredentialUserEntity { - id: Bytes::from_slice(&hex!("6447567A644445")).unwrap(), - icon: None, - name: Some("test1".into()), - display_name: None, - }), + rp: Rp { + format: SerializationFormat::Long, + inner: PublicKeyCredentialRpEntity { + id: "webauthn.io".into(), + name: None, + icon: None, + }, + }, + user: User { + format: SerializationFormat::Long, + inner: PublicKeyCredentialUserEntity { + id: Bytes::from_slice(&hex!("6447567A644445")).unwrap(), + icon: None, + name: Some("test1".into()), + display_name: None, + }, + }, creation_time: 0, use_counter: true, algorithm: -7, diff --git a/src/ctap2/credential_management.rs b/src/ctap2/credential_management.rs index c1d99d6..a1791e7 100644 --- a/src/ctap2/credential_management.rs +++ b/src/ctap2/credential_management.rs @@ -530,16 +530,13 @@ where } // update user name and display name unless the values are not set or empty - credential - .data - .user - .set_name(user.name.as_ref().filter(|s| !s.is_empty()).cloned()); - credential.data.user.set_display_name( - user.display_name - .as_ref() - .filter(|s| !s.is_empty()) - .cloned(), - ); + let credential_user = credential.data.user.as_mut(); + credential_user.name = user.name.as_ref().filter(|s| !s.is_empty()).cloned(); + credential_user.display_name = user + .display_name + .as_ref() + .filter(|s| !s.is_empty()) + .cloned(); // write updated credential let serialized = credential.serialize()?;