Skip to content

Commit

Permalink
Handle present-but-empty extension data when serializing Authenticato…
Browse files Browse the repository at this point in the history
…rData
  • Loading branch information
jschanck committed Nov 8, 2023
1 parent 593f505 commit b63990d
Showing 1 changed file with 29 additions and 5 deletions.
34 changes: 29 additions & 5 deletions src/ctap2/attestation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,6 @@ impl Serialize for AuthenticatorData {
data.extend([self.flags.bits()]); // (2) "flags", len=1 (u8)
data.extend(self.counter.to_be_bytes()); // (3) "signCount", len=4, 32-bit unsigned big-endian integer.

// TODO(MS): Here flags=AT needs to be set, but this data comes from the security device
// and we (probably?) need to just trust the device to set the right flags
if let Some(cred) = &self.credential_data {
// see https://www.w3.org/TR/webauthn-2/#sctn-attested-credential-data
// Attested Credential Data
Expand All @@ -308,9 +306,12 @@ impl Serialize for AuthenticatorData {
.map_err(|_| SerError::custom("Failed to serialize auth_data"))?,
);
}
// TODO(MS): Here flags=ED needs to be set, but this data comes from the security device
// and we (probably?) need to just trust the device to set the right flags
if self.extensions.has_some() {
// If we have parsed extension data, then we should serialize it even if the authenticator
// failed to set the extension data flag.
// If we don't have parsed extension data, then what we output depends on the flag.
// If the flag is set, we output the empty CBOR map. If it is not set, we output nothing.
if self.extensions.has_some() || self.flags.contains(AuthenticatorDataFlags::EXTENSION_DATA)
{
data.extend(
// (5) "extensions", len=variable
&serde_cbor::to_vec(&self.extensions)
Expand Down Expand Up @@ -1078,6 +1079,29 @@ pub mod test {
);
}

#[test]
fn test_empty_extension_data() {
let mut parsed_auth_data: AuthenticatorData =
from_slice(&SAMPLE_AUTH_DATA_MAKE_CREDENTIAL).unwrap();
assert!(parsed_auth_data
.flags
.contains(AuthenticatorDataFlags::EXTENSION_DATA));

// Remove the extension data but keep the extension data flag set.
parsed_auth_data.extensions = Default::default();
let with_flag = to_vec(&parsed_auth_data).expect("could not serialize auth data");
// The serialized auth data should end with an empty map (CBOR 0xA0).
assert_eq!(with_flag[with_flag.len() - 1], 0xA0);

// Remove the extension data flag.
parsed_auth_data
.flags
.remove(AuthenticatorDataFlags::EXTENSION_DATA);
let without_flag = to_vec(&parsed_auth_data).expect("could not serialize auth data");
// The serialized auth data should be one byte shorter.
assert!(with_flag.len() == without_flag.len() + 1);
}

/// See: https://github.com/mozilla/authenticator-rs/issues/187
#[test]
fn test_aaguid_output() {
Expand Down

0 comments on commit b63990d

Please sign in to comment.