mozilla/authenticator-rs

examples/ctap2.rs panics with UnsupportedOption on some devices

riastradh opened this issue · 2 comments

When I apply a patch to fix clone_device_as_write_only, and run examples/ctap2.rs with a Yubikey 5 Nano, it immediately crashes with an UnsupportedOption panic before I even have an opportunity to interact with the device:

[2023-04-21T11:01:03Z DEBUG authenticator::ctap2::commands::make_credentials] Serialize MakeCredentials
STATUS: Continuing with device: Vendor: Unknown Vendor, Device: Unknown Device, Interface: 2, Firmware: v5.2.4, Capabilities: 05
[2023-04-21T11:01:03Z DEBUG authenticator::transport] got from Device "/dev/uhid0" status=Cbor: [43]
[2023-04-21T11:01:03Z DEBUG authenticator::ctap2::commands::make_credentials] response status code: UnsupportedOption
[2023-04-21T11:01:03Z WARN  authenticator::statemachine] error happened: Error: Error issuing command: CommandError: Unexpected code: UnsupportedOption (None)
thread '[2023-04-21T11:01:03Z DEBUG authenticator::authenticatorservice] Callback observer is running, cancelling 0 unchosen transports...
main' panicked at 'Registration failed: HIDError(Command(StatusCode(UnsupportedOption, None)))', examples/ctap2.rs:222:23

ctap2.log

I tried a patch to stop serializing the options in MakeCredentials::serialize, and after that it worked. I tried with and without the attached option.patch, and with and without the --fallback flag in #253, with the following results:

device none fb patch patch+fb caps
Yubikey 5C Nano + + + + 05
Yubikey 5 Nano - + + + 05
Yubico Seckey NFC - + + + 05
Yubikey 4 + + + + 01
SoloKeys Somu + + + + 05

The main issue is that it seems the TODO comment left early last year indicates an actual compatibility problem with several widely used devices on the market.

Two side questions:

  1. Is there a way to dump the transaction messages like in libfido2 with FIDO_DEBUG=1? It would be helpful to see exactly what the device objects to (and it would be really nice to able to replay it with tweaks, but I realize that's a tall order).
  2. Does the ctap2 example display somewhere which paths (CTAP1, CTAP2.0, CTAP2.1) are being used for the transaction?

I'm not sure that the TODO is relevant. But I noticed that the AuthenticatorOptions in your log file has user_verification: None and we send user_verification: Some(false). By the spec, that shouldn't be an issue, but the error code you're seeing is consistent with an authenticator that doesn't like the value of user_verification that we're sending.

Could you try this:

diff --git a/src/ctap2/commands/make_credentials.rs b/src/ctap2/commands/make_credentials.rs
index b815e33..ea72c00 100644
--- a/src/ctap2/commands/make_credentials.rs
+++ b/src/ctap2/commands/make_credentials.rs
@@ -278,7 +278,7 @@ impl PinUvAuthCommand for MakeCredentials {
             // Note: This is basically a no-op right now, since we use `get_uv_option() == Some(false)`, to determine if
             //       the RP is discouraging UV. But we may change that part of the API in the future, so better to be
             //       explicit here.
-            self.set_uv_option(Some(false));
+            self.set_uv_option(None);
             true
         }
     }

Is there a way to dump the transaction messages like in libfido2 with FIDO_DEBUG=1?

No, the RUST_LOG=debug option that you already found is the most verbose output we have. You're right it would be useful to have an option to dump raw transactions.

Does the ctap2 example display somewhere which paths (CTAP1, CTAP2.0, CTAP2.1) are being used for the transaction?

When you pass --fallback we use the legacy_register and legacy_sign functions in statemachine.rs. Otherwise we'll make decisions based on the GetInfo response.

I'm not sure that the TODO is relevant. But I noticed that the AuthenticatorOptions in your log file has user_verification: None and we send user_verification: Some(false)

I don't understand? The log file says this:

[2023-04-21T11:01:03Z DEBUG authenticator::transport] sending MakeCredentials ... options: MakeCredentialsOptions { resident_key: None, user_verification: Some(false) ...

That's consistent with what I understand the software sends, by code inspection.

The only place I see user_verification: None is in AuthenticatorOptions (returned by the device in response to the GETINFO command, as I understand it), not in MakeCredentialOptions (sent by authenticator-rs to the device).

By the spec, that shouldn't be an issue, but the error code you're seeing is consistent with an authenticator that doesn't like the value of user_verification that we're sending.

The CTAP2.1 draft spec appears to be somewhat self-inconsistent:

  1. At one point it implies uv: false is permitted and equivalent to omitting any uv option (https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#op-makecred-step-options):

    If the "uv" option is absent, let the "uv" option be treated as being present with the value false. (This is the default)

  2. At another point it says uv: false is forbidden if the authenticator doesn't support built-in user verification (https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-errata-20220621.html#authenticatorMakeCredential (options table)):

    Platforms MUST NOT include the "uv" option parameter if the authenticator does not support built-in user verification.

I have no idea what the secret FIDO certification test suite checks, if any of this. I assume it must not test uv: false because the Yubikey 5 Nano has passed Level 1 certification.

So, it may be a bug for the Yubikey 5 Nano to reject uv: false in a make-credential command, but also a bug for authenticator-rs to send uv: false in a make-credential command. The FIDO Alliance is trying to keep Postel's principle alive, I suppose! (Too many options in the CTAP2 protocol...)

Could you try this:

diff --git a/src/ctap2/commands/make_credentials.rs b/src/ctap2/commands/make_credentials.rs
index b815e33..ea72c00 100644
--- a/src/ctap2/commands/make_credentials.rs
+++ b/src/ctap2/commands/make_credentials.rs
@@ -278,7 +278,7 @@ impl PinUvAuthCommand for MakeCredentials {
             // Note: This is basically a no-op right now, since we use `get_uv_option() == Some(false)`, to determine if
             //       the RP is discouraging UV. But we may change that part of the API in the future, so better to be
             //       explicit here.
-            self.set_uv_option(Some(false));
+            self.set_uv_option(None);
             true
         }
     }

I think this has the same effect as the change I made. It appears to work with a Yubikey 5 Nano (when applied on top of the change in #252 to use dup instead of open for cloning devices).

Does the ctap2 example display somewhere which paths (CTAP1, CTAP2.0, CTAP2.1) are being used for the transaction?

When you pass --fallback we use the legacy_register and legacy_sign functions in statemachine.rs. Otherwise we'll make decisions based on the GetInfo response.

Right, but could the debug log indicate what decision was made?