From 99bf254f3aa0b4749a7802b490efabc29ae691e1 Mon Sep 17 00:00:00 2001 From: "John M. Schanck" Date: Tue, 26 May 2026 13:02:02 -0700 Subject: [PATCH 1/2] Preserve AAGUID in AttestationObject::anonymize --- src/ctap2/attestation.rs | 84 +++++++++++++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 14 deletions(-) diff --git a/src/ctap2/attestation.rs b/src/ctap2/attestation.rs index 7626420d..de68a228 100644 --- a/src/ctap2/attestation.rs +++ b/src/ctap2/attestation.rs @@ -569,12 +569,27 @@ pub struct AttestationObject { } impl AttestationObject { + // Apply the AttestationConveyancePreference "none" transform from + // https://w3c.github.io/webauthn/#sctn-createCredential. If the + // attestation is self attestation (zero AAGUID, "packed" format, no x5c) + // the spec says no action is needed. Otherwise the attestation statement + // is replaced with the empty "none" statement, but the + // attestedCredentialData (including the AAGUID, which identifies the + // make/model of the authenticator but no individual user/device) is left + // intact. pub fn anonymize(&mut self) { - // Remove the attestation statement and the AAGUID from the authenticator data. - self.att_stmt = AttestationStatement::None; - if let Some(credential_data) = self.auth_data.credential_data.as_mut() { - credential_data.aaguid = AAGuid::default(); + if let AttestationStatement::Packed(ref packed) = self.att_stmt { + if packed.attestation_cert.is_empty() + && self + .auth_data + .credential_data + .as_ref() + .is_some_and(|d| d.aaguid == AAGuid::default()) + { + return; + } } + self.att_stmt = AttestationStatement::None; } } @@ -1057,30 +1072,71 @@ pub mod test { #[test] fn test_anonymize_att_obj() { // Anonymize should prevent identifying data in the attestation statement from being - // serialized. + // serialized, but should preserve the AAGUID per the WebAuthn spec's + // AttestationConveyancePreference "none" transform. let mut att_obj = create_attestation_obj(); // This test assumes that the sample attestation object contains identifying information assert_ne!(att_obj.att_stmt, AttestationStatement::None); - assert_ne!( + let original_aaguid = att_obj + .auth_data + .credential_data + .as_ref() + .expect("credential_data should be Some") + .aaguid + .clone(); + assert_ne!(original_aaguid, AAGuid::default()); + + att_obj.anonymize(); + + // Write the attestation object out to bytes and read it back. The result should not + // have an attestation statement, but the AAGUID should be preserved. + let encoded_att_obj = to_vec(&att_obj).expect("could not serialize anonymized att_obj"); + let att_obj: AttestationObject = + from_slice(&encoded_att_obj).expect("could not deserialize anonymized att_obj"); + + assert_eq!(att_obj.att_stmt, AttestationStatement::None); + assert_eq!( att_obj .auth_data .credential_data .as_ref() .expect("credential_data should be Some") .aaguid, - AAGuid::default() + original_aaguid ); + } - att_obj.anonymize(); + #[test] + fn test_anonymize_self_attestation_is_noop() { + // Per the WebAuthn spec, if the attestation is self attestation (zero + // AAGUID, "packed" format, no x5c) the "none" transform is a no-op. + let mut att_obj = create_attestation_obj(); - // Write the attestation object out to bytes and read it back. The result should not - // have an attestation statement, and it should have the default AAGUID. - let encoded_att_obj = to_vec(&att_obj).expect("could not serialize anonymized att_obj"); - let att_obj: AttestationObject = - from_slice(&encoded_att_obj).expect("could not deserialize anonymized att_obj"); + // Convert to a self attestation: clear the AAGUID and drop x5c. + att_obj + .auth_data + .credential_data + .as_mut() + .expect("credential_data should be Some") + .aaguid = AAGuid::default(); + let self_att = AttestationStatement::Packed(AttestationStatementPacked { + alg: COSEAlgorithm::ES256, + sig: Signature(vec![0x01, 0x02, 0x03, 0x04]), + attestation_cert: vec![], + }); + att_obj.att_stmt = self_att; - assert_eq!(att_obj.att_stmt, AttestationStatement::None); + let expected_att_stmt = AttestationStatement::Packed(AttestationStatementPacked { + alg: COSEAlgorithm::ES256, + sig: Signature(vec![0x01, 0x02, 0x03, 0x04]), + attestation_cert: vec![], + }); + + att_obj.anonymize(); + + // The self attestation should be preserved unchanged. + assert_eq!(att_obj.att_stmt, expected_att_stmt); assert_eq!( att_obj .auth_data From a59db1cab78662d4af1fbb9f35f8785bb72bb445 Mon Sep 17 00:00:00 2001 From: "John M. Schanck" Date: Tue, 26 May 2026 14:55:35 -0700 Subject: [PATCH 2/2] Bump rust-toolchain.toml channel to 1.90.0 and fix lints --- .github/workflows/ci.yml | 4 ---- Cargo.toml | 2 +- rust-toolchain.toml | 2 +- src/crypto/mod.rs | 2 +- src/crypto/nss.rs | 4 ++-- src/crypto/openssl.rs | 4 ++-- src/ctap2/commands/make_credentials.rs | 2 +- src/ctap2/mod.rs | 2 +- src/ctap2/utils.rs | 8 ++++---- src/transport/freebsd/device.rs | 2 +- src/transport/linux/monitor.rs | 3 +-- src/transport/mod.rs | 2 +- src/transport/netbsd/device.rs | 2 +- src/transport/openbsd/device.rs | 2 +- src/transport/windows/device.rs | 2 +- src/transport/windows/winapi.rs | 4 ++-- src/util.rs | 2 +- 17 files changed, 22 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b00b6c34..9f54fb09 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,13 +42,9 @@ jobs: - OS: ubuntu-latest TARGET: x86_64-unknown-linux-gnu NATIVE_BUILD: true - # XXX: This version downgrade is required because 1.8.x has too high of an MSRV - # and the lockfile isn't checked in for tests to use, so the latest dependency version - # is always used instead. ADD_INSTALL: | sudo apt-get update sudo apt-get install -y libudev-dev - cargo update -p base64ct --precise 1.6.0 BUILD_OPTIONS: --features crypto_rust --no-default-features # Mac dummy crypto diff --git a/Cargo.toml b/Cargo.toml index 3d3b8f10..ce42664a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,7 +81,7 @@ env_logger = "^0.6" getopts = "^0.2" assert_matches = "1.2" rpassword = "5.0" -flate3 = "1" +flate3 = "~1.1" aes-gcm = "0.10" # Workaround for 'broken' generic-array 0.14.9, see ctap2_discoverable_creds.rs for details generic-array = { version = "1.3", features = ["compat-0_14"] } diff --git a/rust-toolchain.toml b/rust-toolchain.toml index b4b600c0..7022661b 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,5 +1,5 @@ [toolchain] # Current rust version for mozilla-central # https://firefox-source-docs.mozilla.org/writing-rust-code/update-policy.html -channel = "1.82.0" +channel = "1.90.0" components = ["rustfmt", "clippy", "rust-analyzer"] diff --git a/src/crypto/mod.rs b/src/crypto/mod.rs index 8b600ebf..735705c3 100644 --- a/src/crypto/mod.rs +++ b/src/crypto/mod.rs @@ -1132,7 +1132,7 @@ pub struct U2FRegisterAnswer<'a> { } // We will only return MalformedInput here -pub fn parse_u2f_der_certificate(data: &[u8]) -> Result { +pub fn parse_u2f_der_certificate(data: &[u8]) -> Result, CryptoError> { // So we don't panic below, when accessing individual bytes if data.len() < 4 { return Err(CryptoError::MalformedInput); diff --git a/src/crypto/nss.rs b/src/crypto/nss.rs index 7f540601..668b7c94 100644 --- a/src/crypto/nss.rs +++ b/src/crypto/nss.rs @@ -195,7 +195,7 @@ pub fn encrypt_aes_256_cbc_no_pad(key: &[u8], iv: Option<&[u8]>, data: &[u8]) -> _ => return Err(CryptoError::LibraryFailure), }; - if data.len() % AES_BLOCK_SIZE != 0 { + if !data.len().is_multiple_of(AES_BLOCK_SIZE) { return Err(CryptoError::LibraryFailure); } @@ -256,7 +256,7 @@ pub fn decrypt_aes_256_cbc_no_pad(key: &[u8], iv: Option<&[u8]>, data: &[u8]) -> _ => return Err(CryptoError::LibraryFailure), }; - if data.len() % AES_BLOCK_SIZE != 0 { + if !data.len().is_multiple_of(AES_BLOCK_SIZE) { return Err(CryptoError::LibraryFailure); } diff --git a/src/crypto/openssl.rs b/src/crypto/openssl.rs index a440ad9d..6504b31e 100644 --- a/src/crypto/openssl.rs +++ b/src/crypto/openssl.rs @@ -77,7 +77,7 @@ pub fn encrypt_aes_256_cbc_no_pad(key: &[u8], iv: Option<&[u8]>, data: &[u8]) -> encrypter.pad(false); let in_len = data.len(); - if in_len % AES_BLOCK_SIZE != 0 { + if !in_len.is_multiple_of(AES_BLOCK_SIZE) { return Err(CryptoError::LibraryFailure); } @@ -101,7 +101,7 @@ pub fn decrypt_aes_256_cbc_no_pad(key: &[u8], iv: Option<&[u8]>, data: &[u8]) -> encrypter.pad(false); let in_len = data.len(); - if in_len % AES_BLOCK_SIZE != 0 { + if !in_len.is_multiple_of(AES_BLOCK_SIZE) { return Err(CryptoError::LibraryFailure); } diff --git a/src/ctap2/commands/make_credentials.rs b/src/ctap2/commands/make_credentials.rs index 9d6c779e..cce1d570 100644 --- a/src/ctap2/commands/make_credentials.rs +++ b/src/ctap2/commands/make_credentials.rs @@ -398,7 +398,7 @@ impl MakeCredentials { // Note: a CTAP 2.0 authenticator is allowed to create a discoverable credential even // if one was not requested, so there is a case in which we cannot confidently // return `rk=false` here. We omit the response entirely in this case. - let dev_supports_rk = maybe_info.map_or(false, |info| info.options.resident_key); + let dev_supports_rk = maybe_info.is_some_and(|info| info.options.resident_key); let requested_rk = self.options.resident_key.unwrap_or(false); let max_supported_version = maybe_info.map_or(AuthenticatorVersion::U2F_V2, |info| { info.max_supported_version() diff --git a/src/ctap2/mod.rs b/src/ctap2/mod.rs index e8e05cd5..f712ba0d 100644 --- a/src/ctap2/mod.rs +++ b/src/ctap2/mod.rs @@ -476,7 +476,7 @@ pub fn register( // that does not support this extension.)" let dev_supports_cred_protect = dev .get_authenticator_info() - .map_or(false, |info| info.supports_cred_protect()); + .is_some_and(|info| info.supports_cred_protect()); if args.extensions.enforce_credential_protection_policy == Some(true) && args.extensions.credential_protection_policy != Some(CredentialProtectionPolicy::UserVerificationOptional) diff --git a/src/ctap2/utils.rs b/src/ctap2/utils.rs index b5d84c7d..0c5b88c0 100644 --- a/src/ctap2/utils.rs +++ b/src/ctap2/utils.rs @@ -32,8 +32,8 @@ pub fn read_be_u16(data: &mut R) -> Result { } pub fn read_byte(data: &mut R) -> Result { - match data.bytes().next() { - Some(Ok(s)) => Ok(s), - _ => Err(serde_parse_err("u8")), - } + let mut buf = [0; 1]; + data.read_exact(&mut buf) + .map_err(|_| serde_parse_err("u8"))?; + Ok(buf[0]) } diff --git a/src/transport/freebsd/device.rs b/src/transport/freebsd/device.rs index 0e8c7d20..8682aeec 100644 --- a/src/transport/freebsd/device.rs +++ b/src/transport/freebsd/device.rs @@ -169,7 +169,7 @@ impl HIDDevice for Device { } fn get_property(&self, _prop_name: &str) -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "Not implemented")) + Err(io::Error::other("Not implemented")) } fn get_device_info(&self) -> U2FDeviceInfo { diff --git a/src/transport/linux/monitor.rs b/src/transport/linux/monitor.rs index f2bdf6ca..0f1ea1ac 100644 --- a/src/transport/linux/monitor.rs +++ b/src/transport/linux/monitor.rs @@ -187,8 +187,7 @@ pub fn get_property_linux(path: &PathBuf, prop_name: &str) -> io::Result } } - Err(io::Error::new( - io::ErrorKind::Other, + Err(io::Error::other( "Unable to find device", )) } diff --git a/src/transport/mod.rs b/src/transport/mod.rs index 318934ed..4ddafd82 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -187,7 +187,7 @@ where fn block_and_blink(&mut self, keep_alive: &dyn Fn() -> bool) -> BlinkResult { let supports_select_cmd = self.get_protocol() == FidoProtocol::CTAP2 - && self.get_authenticator_info().map_or(false, |i| { + && self.get_authenticator_info().is_some_and(|i| { i.versions.contains(&AuthenticatorVersion::FIDO_2_1) }); let resp = if supports_select_cmd { diff --git a/src/transport/netbsd/device.rs b/src/transport/netbsd/device.rs index 4b426b3b..693f079b 100644 --- a/src/transport/netbsd/device.rs +++ b/src/transport/netbsd/device.rs @@ -171,7 +171,7 @@ impl HIDDevice for Device { } fn get_property(&self, _prop_name: &str) -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "Not implemented")) + Err(io::Error::other("Not implemented")) } fn get_device_info(&self) -> U2FDeviceInfo { diff --git a/src/transport/openbsd/device.rs b/src/transport/openbsd/device.rs index f11ded79..ad27ba27 100644 --- a/src/transport/openbsd/device.rs +++ b/src/transport/openbsd/device.rs @@ -152,7 +152,7 @@ impl HIDDevice for Device { } fn get_property(&self, _prop_name: &str) -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "Not implemented")) + Err(io::Error::other("Not implemented")) } fn get_device_info(&self) -> U2FDeviceInfo { diff --git a/src/transport/windows/device.rs b/src/transport/windows/device.rs index bda5d385..fe59569d 100644 --- a/src/transport/windows/device.rs +++ b/src/transport/windows/device.rs @@ -110,7 +110,7 @@ impl HIDDevice for Device { } fn get_property(&self, _prop_name: &str) -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "Not implemented")) + Err(io::Error::other("Not implemented")) } fn get_device_info(&self) -> U2FDeviceInfo { diff --git a/src/transport/windows/winapi.rs b/src/transport/windows/winapi.rs index 44b44898..a1bee37b 100644 --- a/src/transport/windows/winapi.rs +++ b/src/transport/windows/winapi.rs @@ -64,7 +64,7 @@ extern "system" { } fn from_wide_ptr(ptr: *const u16, len: usize) -> String { - assert!(!ptr.is_null() && len % 2 == 0); + assert!(!ptr.is_null() && len.is_multiple_of(2)); let slice = unsafe { slice::from_raw_parts(ptr, len / 2) }; OsString::from_wide(slice).to_string_lossy().into_owned() } @@ -95,7 +95,7 @@ impl DeviceInfoSet { self.set } - pub fn devices(&self) -> DeviceInfoSetIter { + pub fn devices(&self) -> DeviceInfoSetIter<'_> { DeviceInfoSetIter::new(self) } } diff --git a/src/util.rs b/src/util.rs index d7478250..c51f39b2 100644 --- a/src/util.rs +++ b/src/util.rs @@ -67,7 +67,7 @@ pub fn from_unix_result(rv: T) -> io::Result { } pub fn io_err(msg: &str) -> io::Error { - io::Error::new(io::ErrorKind::Other, msg) + io::Error::other(msg) } #[cfg(test)]