Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Token2 T2F2-NFC-Dual does not work with PinProtocol 2 #269

Closed
micolous opened this issue Feb 2, 2023 · 5 comments · Fixed by #286
Closed

Token2 T2F2-NFC-Dual does not work with PinProtocol 2 #269

micolous opened this issue Feb 2, 2023 · 5 comments · Fixed by #286

Comments

@micolous
Copy link
Collaborator

micolous commented Feb 2, 2023

I did this

Using Token2 T2F2-NFC-Dual:

  1. Reset device to factory default settings (using key_manager)
  2. Set new PIN (using key_manager)
  3. Run authenticate example with ctap backend

Repeated above test with key connected via NFC and via USB.

I expected the following

Prompted for PIN, and can register credential successfully using first available Pin Protocol (2).

What actually happened

Prompted for PIN, got Ctap error: Ctap(Ctap2PinAuthInvalid)

Version (and git commit)

0ed816f

Operating System / Version

Win10, Linux

Any other comments

Device reports:

GetInfoResponse {
  versions: {"FIDO_2_0", "FIDO_2_1", "FIDO_2_1_PRE", "U2F_V2"},
  extensions: Some(["credBlob", "credProtect", "hmac-secret", "largeBlobKey", "minPinLength"]),
  aaguid: Some(ab32f0c6-2239-afbb-c470-d2ef4e254db7),
  options: Some({"alwaysUv": false, "authnrCfg": true, "clientPin": true, "credMgmt": true, "credentialMgmtPreview": true, "largeBlobs": true, "makeCredUvNotRqd": true, "pinUvAuthToken": true, "plat": false, "rk": true, "setMinPINLength": true, "up": true}),
  max_msg_size: Some(1536),
  pin_protocols: Some([2, 1]),
  max_cred_count_in_list: Some(8),
  max_cred_id_len: Some(96),
  transports: Some(["usb", "nfc"]),
  algorithms: Some(Array([Map({Text("alg"): Integer(-7), Text("type"): Text("public-key")})])),
  min_pin_length: Some(4)
}

Interestingly the MDS reports different data (only FIDO 2.1-PRE and PinProtocol 1):

aaguid: ab32f0c6-2239-afbb-c470-d2ef4e254db7
last update: 2019-12-18
description: TOKEN2 FIDO2 Security Key
authenticator_version: 2
authentication_algorithms:
  Secp256r1EcdsaSha256Raw
public_key_alg_and_encodings:
  Cose
user_verification_details:
-- OR
  AND
  PresenceInternal
-- OR
  AND
  PasscodeInternal
key_protection:
  Hardware
  SecureElement
is_key_restricted: true
is_fresh_user_verification_required: true
supported_extensions:
  hmac-secret - false -
authenticator_get_info: AuthenticatorGetInfo { versions: [U2fV2, Fido2_0, Fido2_1Pre], extensions: ["credProtect", "hmac-secret"], aaguid: ab32f0c6-2239-afbb-c470-d2ef4e254db7, options: {"clientPin": false, "credentialMgmtPreview": true, "plat": false, "rk": true, "up": true}, max_msg_size: Some(2048), pin_uv_auth_protocols: [1], max_credential_count_in_list: Some(8), max_credential_id_length: Some(96), transports: ["usb"], algorithms: [Object {"alg": Number(-7), "type": String("public-key")}], max_serialized_large_blob_array: None, force_pin_change: None, min_pin_length: None, firmware_version: None, max_cred_blob_length: None, max_rpids_for_set_min_pin_length: None, preferred_platform_uv_attempts: None, uv_modality: None, certifications: {}, remaining_discoverable_credentials: None, vendor_prototype_config_commands: [] }
status_reports:
  FidoCertified { effective_date: Some("2019-12-18"), authenticator_version: None, certification_descriptor: Some("TOKEN2 T2F2-ALU"), certificate_number: Some("FIDO20020191203001"), certification_policy_version: Some("1.0.0"), certification_requirements_version: Some("1.0.0"), url: None }

There are also other devices which appear to use the same AAGUID (Token2 T2F2-Bio).

If I modify ctap20.rs to only try to authenticate with PinProtocol 1, it works correctly. The key also continues to work when setting or changing a PIN in PinProtocol 2 mode.

I was also able to authenticate correctly using a Yubikey Bio on PinProtocol 2, so I suspect this is a bug with the Token2 key itself.

Windows WebAuthn API appears to use PinProtocol 1 only, so can't really compare that 😞

The Token2 T2F2 which I also got to test appears to be a rebadged Feitian ePass FIDO2 (833b721a-ff5f-4d00-bb2e-bdda3ec01e29) which does not support PinProtocol 2 (and doesn't claim to -- so that's fine)

@Firstyear
Copy link
Member

There are also other devices which appear to use the same AAGUID (Token2 T2F2-Bio).

That's bad, there is meant to be a unique AAGUID for each model.

The MDS being outdated is also bad here because it seems the certified version was in 2019, and the device has clearly had firmware updates.

@micolous
Copy link
Collaborator Author

micolous commented Feb 3, 2023

Unfortunately working around this means we're going to have to build a "quirks mode" to not use features on certain keys.

There are other strange bugs with its NFC implementation which suggest that it's not parsing some commands correctly either. I haven't yet attempted to make a reliable reproduction.

I haven't attempted to contact the vendor yet.

@micolous
Copy link
Collaborator Author

I've sent an email to the vendor today notifying them of the issue and included protocol traces, so will see what they say.

@Firstyear
Copy link
Member

Even if they update the firmware, we'll still potentially need quirks to support devices that haven't received the update.

@micolous
Copy link
Collaborator Author

micolous commented Mar 1, 2023

Vendor identified the bug in webauthn-authenticator-rs: it always truncated the authenticate() return value to 16 bytes, when it should only do that for PIN Protocol 1.

Incidentally, as this code worked on Yubikey Bio, that means those keys have a bug...

micolous added a commit to micolous/webauthn-rs that referenced this issue Mar 1, 2023
It looks like Token2 implemented this correctly, but Yubikey did not.
Firstyear pushed a commit that referenced this issue Mar 1, 2023
It looks like Token2 implemented this correctly, but Yubikey did not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants