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

Wrongly enforces getAssertion credential (0x01) member presence in the authenticator response, which is optional in FIDO2 #319

Closed
xchapron-ledger opened this issue Nov 9, 2023 · 10 comments

Comments

@xchapron-ledger
Copy link

Hello :)

I'm the developer working on https://github.com/LedgerHQ/app-security-key/ which purpose is to allow Ledger devices to be used as a FIDO2 authenticator.

It works well on Firefox on Windows and Linux and that's great thanks for your work!

However, I recently gave it a try on Firefox on MacOS and I'm facing issues.

Regarding my issue, here is my setup:

Using Wireshark I see that:

There are multiple request to get the device info, here is the device answer:
{1: ["U2F_V2", "FIDO_2_0"], 2: ["hmac-secret"], 3: h'58B44D0B0A7CF33AFD48F7153C871352', 4: {"rk": false, "up": true, "uv": true, "clientPin": false}, 5: 1024, 6: [1]}

The registration goes well, here is the request:
{1: h'631FFAA981568D40E27CC364137163C8B3DB56FEC9901E59CA99DD2FB9A7CF3C', 2: {"id": "webauthn.io"}, 3: {"id": h'613278316147786F65577835', "name": "kluhlhyly"}, 4: [{"alg": -7, "type": "public-key"}, {"alg": -257, "type": "public-key"}], 7: {"rk": false, "uv": true}}

And here is the response:
{1: "packed", 2: h'74A6EA9213C99C2F74B22492B320CF40262A94C1A950A0397F29250B60841EF045F1D0C02758B44D0B0A7CF33AFD48F7153C871352004102C0251D20C5DE22DA323FE1601775356BD720ED1920F42EDF757ADAF0B6B5729E73554146CF5F88E9FA47602809E8C33315449BB2EF0F9468219C7D03F0C36C75A5010203262001215820837B015EC6E28999A3E0F446843FC079D70191F9439124B7CBB8C6DBC5C539A5225820D334AF49762F97EE6D6A99C0315A5C6457F91EC805AAD9FF39F37ED9008888DE', 3: {"alg": -7, "sig": h'304402202610FAADEA92042CB240F6157FD68A5619D1C1AE271AABD3E5B54C2403679B2802205730D0CC80077A1773364AA1B5B6C0FC613612F8FF452951887CE9D84EEDCC7B', "x5c": [h'308201EE30820194A00302010202140C20109D50E9A06359A6F103E4835EBBD53B10C9300A06082A8648CE3D0403023043310B3009060355040613024652310F300D060355040A0C064C65646765723123302106035504030C1A4C6564676572204649444F204174746573746174696F6E204341301E170D3232303932363038303630385A170D3332303932333038303630385A3076310B3009060355040613024652310F300D060355040A0C064C656467657231223020060355040B0C1941757468656E74696361746F72204174746573746174696F6E3132303006035504030C294C6564676572204E616E6F2D5350204649444F2032204174746573746174696F6E20426174636820313059301306072A8648CE3D020106082A8648CE3D030107034200044A360D348D0D945AF2BBE199353B675B48937F6809E1C4BCADC1920E821E57EAEFA2F44B3C95C40AA2C90F4BBA3D8C43C30FD8755645333CE5F30F2FA0312ECFA33330313021060B2B0601040182E51C0101040412041058B44D0B0A7CF33AFD48F7153C871352300C0603551D130101FF04023000300A06082A8648CE3D0403020348003045022100D90FC542D5044F06851A27F06F988F18EEECBAB3954DD9F63DB164500371D43B02204D60B7752A20B681A947AC4296286CEB8E0E0B29A439B8E55A38899FEC8F0736']}}

Then I when I launch the authentication, I see a first get assertion request with up=false:
{1: "webauthn.io", 2: h'E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855', 3: [{"id": h'02C0251D20C5DE22DA323FE1601775356BD720ED1920F42EDF757ADAF0B6B5729E73554146CF5F88E9FA47602809E8C33315449BB2EF0F9468219C7D03F0C36C75', "type": "public-key"}], 5: {"up": false}}
The device respond with:
{2: h'74A6EA9213C99C2F74B22492B320CF40262A94C1A950A0397F29250B60841EF000F1D0C02A', 3: h'3045022100A05D8A878B8E93D028918639EC2B3A841D63A7C27BAADCC526B47D106B32D81002202FAE26F139FD699E100848214AB28A5C3F39192E9507D4306E399C5FFC9D4167'}

Then the device receive another request:
{1: h'D0CEE6FC7DBF599A919DB8FB951311269F0EB781F7841C6CC0544AD9DA34154B', 2: {"id": "make.me.blink"}, 3: {"id": h'00', "name": "make.me.blink"}, 4: [{"alg": -7, "type": "public-key"}], 8: h'', 9: 1}
Which is linked to this code: https://github.com/mozilla/authenticator-rs/blob/ctap2-2021/src/ctap2/commands/make_credentials.rs#L560-L592
The device returns an error 0x35 (CTAP2_ERR_PIN_NOT_SET) as the pin is not set, as indicated by the getInfo answer.

The webauthn.io window then expose the following failure message:

The request is not allowed by the user agent or the platform in the current context, possibly because the user denied permission.

So here my questions:

  1. How can I known the version of authenticator-rs included in a Firefox version?
  2. What's the best way to retrieve FIDO2 logs on Firefox? On Chrome there is chrome://device-logs, is there a Firefox equivalent? I tried working with the console log but they lack important information. And can always use Wireshark but it is way more painful.
  3. I have difficulties understanding how the authentication works? Why is there a first request with up=false and uv=false, and then what looks like a dummy request to make the device blink? This seems like really specific to an authenticator model and not following the FIDO spec.
  4. I guess the above could be simplified when the device support up and uv option, so that the client pin is not used for user verification?
  5. I'm not sure to understand why the behavior is different on Windows, MacOS and Linux while on the same Firefox version, is it due to different platform used underneath Firefox?

Thanks!

Xavier

@msirringhaus
Copy link
Contributor

I haven't disassembled the responses from your ledger, so I can't give a full breakdown of what happened. See my answer to your question 2, so we can get a more detailed log.

But one step or rather one question at a time:

  1. Using this tracking-bug https://bugzilla.mozilla.org/show_bug.cgi?id=1828974 you should be able to find which version was going into which milestone.
  2. You can start Firefox from the commandline with RUST_LOG=authenticator=debug to get the logs for auth-rs.
  3. "make.me.blink"-requests are sent in two scenarios: First, if multiple devices are plugged in and the user has to select which to use. For this, we make all devices blink and let the user touch the one that should be used. Second, in cases where a register() call was made, but the credential was already registered previously. The spec mandates that the device has to blink anyways, even if we have the opportunity to determine this without user interaction. Or a sign() call was made, and an allow-list was provided that contained no valid credential. Again, we have to blink anyways and collect a user interaction. If the call succeeds or fails with e.g. 'no pin set' should make no difference here.
  4. Not really relevant, see 3.
  5. There is only one implementation of authenticator-rs, the only OS-dependent part of the code is how to discover and interact with the raw USB-devices, which obviously differs for each OS. The business-logic is the same for all. However on newer Windows-machines "Windows Hello" is used instead of authenticator-rs. And I think the same is true for MacOS. But given that your logs show the "make.me.blink"-request, it's pretty obvious that in your case auth-rs is used. So, I'm also not sure what the difference is. Hopefully more detailed logs will help.

Hope that helps!

@xchapron-ledger
Copy link
Author

Hello @msirringhaus ,
First thanks for your help!

1/

Using this tracking-bug https://bugzilla.mozilla.org/show_bug.cgi?id=1828974 you should be able to find which version was going into which milestone.

If I get it correctly, this shows that v0.4.0-alpha.22 is included in firefox119 while this show that v0.4.0-alpha.23 will be included in firefox120?

2/

You can start Firefox from the commandline with RUST_LOG=authenticator=debug to get the logs for auth-rs.

I gave it a try with both export RUST_LOG=authenticator=debug and then launching Firefox with /Applications/Firefox.app/Contents/MacOS/firefox but I didn't see any additional logging, even in the developer console. I'm sorry I have never debugged anything on Firefox and or Mac, and will therefore require a bit more indication here :/ Maybe I need a Firefox debug build? Or I should look somewhere else for logs?

3/

"make.me.blink"-requests are sent in two scenarios: First, if multiple devices are plugged in and the user has to select which to use. For this, we make all devices blink and let the user touch the one that should be used.

I was wondering how other browser do that, and therefore gave it a try on Google Chrome. It appears it sends the request to every authenticator, and once authenticator respond with a success it cancel the other request. I found that more intuitive and maybe more spec compliant, is there any reason not to do that?

Second, in cases where a register() call was made, but the credential was already registered previously. The spec mandates that the device has to blink anyways, even if we have the opportunity to determine this without user interaction.

I'm not sure to understand which part of the spec you are referring? In the FIDO2.1 spec I found this but this specify the behavior of the authenticator which should be done without any "make.me.blink"-requests. Are you referring to another part of the spec, or another spec maybe?

Or a sign() call was made, and an allow-list was provided that contained no valid credential. Again, we have to blink anyways and collect a user interaction.

In both FIDO2.0 and FIDO2.1 spec I can see requirements for authenticators to collect user presence or user verification before responding to a sign call with an allow-list without valid credential, but again, that should be done on authenticator side, without any "make.me.blink"-requests. Are you referring to another part of the spec, or another spec maybe?

Thanks!

Xavier

@msirringhaus
Copy link
Contributor

Hi,
no problem.

\1. Yes, I think thats correct.

\2. Hm, this is weird. I don't have a Mac, so I can't reproduce this. I just asked somebody else and it worked for them (although, AFAIK for released versions the log-levels are capped at "INFO", and "DEBUG" can't be chosen any more). Maybe the problem is the selector for authenticator=. Please try with RUST_LOG=trace /Applications/Firefox\ Beta.app/Contents/MacOS/firefox, although this will print a lot.
On the other hand, you could also just download this repository and run RUST_LOG=debug cargo run --example ctap2. This will cut out the middle-man (Firefox), and make debugging easier.

\3.1.

It appears it sends the request to every authenticator, and once authenticator respond with a success it cancel the other request.

We have to be careful here, as there are multiple different variations, that all have a different outcome. E.g. Chrome checks, if the device also speaks the old CTAP1 protocol and then downgrades the request to that. This is to get around the PIN-stuff. Then it can send the actual request to all devices instead of selection-requests, followed by the actual request. We currently don't do this (and I'm still not 100% convinced we should be). But Chrome does also use dummy requests to select devices in certain cases, see #155 .

\3.2.

In both FIDO2.0 and FIDO2.1 spec I can see requirements for authenticators to collect user presence or user verification before responding to a sign call with an allow-list without valid credential, but again, that should be done on authenticator side, without any "make.me.blink"-requests.

As mentioned Chrome does this, too: https://github.com/chromium/chromium/blob/6aaa00ad13642255e207e1c4893463f31c5ecf68/device/fido/make_credential_task.cc#L126-L156
and
https://github.com/chromium/chromium/blob/6aaa00ad13642255e207e1c4893463f31c5ecf68/device/fido/get_assertion_task.cc#L366-L381

There may be some constellations were we could use the actual request instead of the dummy to make the device fail, but it doesn't really matter what we send, if all we want to do is collect a user-interaction and fail.

As far as I remember, the spec doesn't explicitly mention this, except for the old CTAP1-compatibility section:

If the excludeList is not empty, the platform MUST send signing request with check-only control byte to the CTAP1/U2F authenticator using each of the credential ids (key handles) in the excludeList. If any of them does not result in an error, that means that this is a known device. Afterwards, the platform MUST still send a dummy registration request (with a dummy appid and invalid challenge) to CTAP1/U2F authenticators that it believes are excluded. This makes it so the user still needs to touch the CTAP1/U2F authenticator before the RP gets told that the token is already registered.

@xchapron-ledger
Copy link
Author

Hello :)

\2. Hm, this is weird. I don't have a Mac, so I can't reproduce this. I just asked somebody else and it worked for them (although, AFAIK for released versions the log-levels are capped at "INFO", and "DEBUG" can't be chosen any more). Maybe the problem is the selector for authenticator=. Please try with RUST_LOG=trace /Applications/Firefox\ Beta.app/Contents/MacOS/firefox, although this will print a lot.

Using Firefox Beta, I managed to get some logs, but it was really noisy, and didn't contains interesting logs as far as I know.

On the other hand, you could also just download this repository and run RUST_LOG=debug cargo run --example ctap2. This will cut out the middle-man (Firefox), and make debugging easier.

I therefore went this path, but what a nightmare. I went from one issue to another during the installation of nss.
I eventually managed to make it works by switching to openssl in the cargo.toml with this diff:

diff --git a/Cargo.toml b/Cargo.toml
index 554ea1c..8e60540 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -14,7 +14,7 @@ travis-ci = { repository = "mozilla/authenticator-rs", branch = "master" }
 maintenance = { status = "actively-developed" }
 
 [features]
-default = ["crypto_nss"]
+default = ["crypto_openssl"]
 binding-recompile = ["bindgen"]
 # Crypto backends
 # NOTE: These are mutually exclusive, but cargo does not support that.
@@ -22,8 +22,8 @@ binding-recompile = ["bindgen"]
 # Default: NSS
 crypto_dummy = []
 crypto_openssl = ["openssl", "openssl-sys"]
-crypto_nss = ["nss-gk-api", "pkcs11-bindings"]
-gecko = ["nss-gk-api/gecko"]
+#crypto_nss = ["nss-gk-api", "pkcs11-bindings"]
+#gecko = ["nss-gk-api/gecko"]
 
 [target.'cfg(target_os = "linux")'.dependencies]
 libudev = "^0.2"
@@ -67,7 +67,7 @@ cfg-if = "1.0"
 # Crypto backends
 openssl-sys = { version = "0.9", optional = true}
 openssl = { version = "0.10", optional = true}
-nss-gk-api = { version = "0.3.0", optional = true }
+#nss-gk-api = { version = "0.3.0", optional = true }
 pkcs11-bindings = { version = "0.1.4", optional = true }
 
 [dev-dependencies]

I've attached the log below, I hope this help understanding the issue.
ledger_authenticator_ctap2_failure.txt

Regarding dummy request, first thanks for the links on Google Chrome code, that's interesting.
I guess I should look at implementing the FIDO2.1 spec, this might improve some behavior as it seems implemented (and even expected) in browser.

There may be some constellations were we could use the actual request instead of the dummy to make the device fail, but it doesn't really matter what we send, if all we want to do is collect a user-interaction and fail.

I still don't get why you want a user-interaction at that point. From my point of view, if the authenticator respond to the request with a failure without requesting UV or UP it's already an issue, as one could then design a rogue client which could then try to get information on known credentials. that's why the authenticator has to do this, and this is specified in both FIDO2 and FIDO2.1 spec:

  • FIDO2: The step 7 (collecting UV or UP) should be done before step 8 (responding if no credential is present): "Collect user consent if required. This step MUST happen before the following steps due to privacy reasons (i.e., authenticator cannot disclose existence of a credential until the user interacted with the device):"
  • FIDO2.1 : UV verification should be done in step 6 before maybe responding in step 7 and credentials can be protected to request UV for access.

Thanks,
Xavier

@xchapron-ledger
Copy link
Author

xchapron-ledger commented Nov 15, 2023

Hello @msirringhaus I found a way to work around the problem.

I think it is because in the getAssertion response, the authenticator doesn't add the credential (0x01) member.
This is allowed by the FIDO2 spec in some situation, which this one is:

Optional PublicKeyCredentialDescriptor structure containing the credential identifier whose private key was used to generate the assertion. May be omitted if the allowList has exactly one Credential.

However, it's seems that your implementation enforces it, I try adding it in my authenticator response, and after that, the ctap2 example run successfully.

Do you think this is something you could change to be compatible with more devices that respect the FIDO2 spec?
I guess this should require some changes around here: https://github.com/mozilla/authenticator-rs/blob/ctap2-2021/src/ctap2/mod.rs#L661-L673

I still don't get why it works on Firefox on Linux and on Windows, but not on Firefox on Mac nor when directly using authenticator-rs (on Mac or Linux), do you know?

Edit: looking at the FIDO2.1 spec, this credential (0x01) member is required, so I guess I will change the authenticator behavior to always include it.

Also, I now see that in such situation, your authenticator code make a first silent getAssertion request (with up=false and uv=false) and then a second requiring user presence and verifiation. So now behaving mainly as I expected it in working cases.

So if it's ok for you, I'm going to change the title of this issue to:
Wrongly enforces getAssertion credential (0x01) member presence in the authenticator response, which is optional in FIDO2

Thanks
Xavier

@msirringhaus
Copy link
Contributor

Ok, this makes no sense to me. We do not enforce user to be present anywhere, as far as I can tell.

What I've noticed, though, is that you do not send the credentialID in your GetAssertion response. This is fine for CTAP2.0 devices (although not allowed anymore for 2.1), but it looks to me that our code may have a bug there, and disregard your token because this is missing.
The only situation the omission is allowed for CTAP2.0 is, if the allowList has length 1. This may explain why it sometimes works, and sometimes doesn't. You could try to add a dummy credential to the allowList in examples/ctap2.rs, and see if it works then?

@xchapron-ledger
Copy link
Author

Ok sorry, I was editing my response. Indeed, the issue is due to the absence of credential (0x01) member in the response.

@xchapron-ledger
Copy link
Author

Here is what I have added in my response:

looking at the FIDO2.1 spec, this credential (0x01) member is required, so I guess I will change the authenticator behavior to always include it.

Also, I now see that in such situation, your authenticator code make a first silent getAssertion request (with up=false and uv=false) and then a second requiring user presence and verifiation. So now behaving mainly as I expected it in working cases.

So if it's ok for you, I'm going to change the title of this issue to: Wrongly enforces getAssertion credential (0x01) member presence in the authenticator response, which is optional in FIDO2

@msirringhaus
Copy link
Contributor

Ah, mid-air collision :) Yes, now it makes sense to me.

The problem is in our pre-flighting code here: https://github.com/mozilla/authenticator-rs/blob/ctap2-2021/src/ctap2/preflight.rs#L168-L171

I'll see, if I can come up with a fix (that doesn't introduce 10 additional bugs).

@xchapron-ledger xchapron-ledger changed the title Authenticator up and uv option ignored Wrongly enforces getAssertion credential (0x01) member presence in the authenticator response, which is optional in FIDO2 Nov 15, 2023
xchapron-ledger pushed a commit to LedgerHQ/app-security-key that referenced this issue Nov 15, 2023
This field is optional in FIDO2 in some cases, but required in FIDO2.1.
Some browser enforces it, see
mozilla/authenticator-rs#319
As not including it is an unnecessary optimisation, we rather always put
it if it causes compatibiity issue.
xchapron-ledger pushed a commit to LedgerHQ/app-security-key that referenced this issue Nov 15, 2023
This field is optional in FIDO2 in some cases, but required in FIDO2.1.
Some browser enforces it, see
mozilla/authenticator-rs#319
As not including it is an unnecessary optimisation, we rather always put
it if it causes compatibiity issue.
xchapron-ledger pushed a commit to LedgerHQ/app-security-key that referenced this issue Nov 17, 2023
This field is optional in FIDO2 in some cases, but required in FIDO2.1.
Some browser enforces it, see
mozilla/authenticator-rs#319
As not including it is an unnecessary optimisation, we rather always put
it if it causes compatibiity issue.
msirringhaus pushed a commit to msirringhaus/authenticator-rs that referenced this issue Nov 17, 2023
…al data

Extend the mock device to be able to skip the low-level byte-by-byte
comparison of incoming and outgoing data, and instead use CTAP requests
and responses directly, for higher-level business-logic testing.
Add tests for preflighting.
@jschanck
Copy link
Collaborator

jschanck commented Dec 1, 2023

Resolved by #320

@jschanck jschanck closed this as completed Dec 1, 2023
msirringhaus pushed a commit to msirringhaus/authenticator-rs that referenced this issue Dec 4, 2023
…al data

Extend the mock device to be able to skip the low-level byte-by-byte
comparison of incoming and outgoing data, and instead use CTAP requests
and responses directly, for higher-level business-logic testing.
Add tests for preflighting.
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

No branches or pull requests

3 participants