mozilla / authenticator-rs

Rust library to interact with Security Keys, used by Firefox
https://crates.io/crates/authenticator
Mozilla Public License 2.0
273 stars 70 forks source link

`ctap2_discoverable_creds`-example fails for mooltipass #242

Open msirringhaus opened 1 year ago

msirringhaus commented 1 year ago

I did some digging in the meantime and it seems that the mooltipass is not returning the name field, even though it is set when the credential is created (and is stored as I can see it on the screen). There does seem to be a possibility in the spec to not send the name field back when the user is not verified, but I consider that a discussion to be had on the mooltipass side whether this applies or not (I'm assuming it doesn't really apply and the mooltipass should send the name field)

Originally posted by @VincentVanlaer in https://github.com/mozilla/authenticator-rs/issues/237#issuecomment-1488103175

Thanks @VincentVanlaer!

Also note: It is only our example-binary that fails here, which just stupidly unwrap()s on the User name. In Firefox, there is a fallback, so we shouldn't error out there.

But especially for discoverable credentials, it would be very helpful for the mooltipass to return the username, otherwise it's hard for the user to decide as which account he wants to log in (e.g. if you have 2 different accounts on the same webpage). Or does the account selection happen in the device-UI itself?

VincentVanlaer commented 1 year ago

Yes, the screen on the device shows all the accounts for a domain, and allows one to choose the right one with a scrollwheel.

msirringhaus commented 1 year ago

Could you try with the latest nightly build of Firefox (and security.webauthn.ctap2 = true) if the "unknown username"-fallback in Firefox works as expected?

VincentVanlaer commented 1 year ago

Hmm, I tried the latest nightly (2023-03-30), but it still fails on the pin protocol error: Error when determining pinAuth: HIDError(Command(UnsupportedPinProtocol))

Should this build already have the recently merged changes or not yet? In any case, it used to work with my patches, so I assume the fallback path is fine.

jschanck commented 1 year ago

The version in Firefox doesn't have #237 yet.

VincentVanlaer commented 1 year ago

I tried to build firefox nightly (without any changes to the authenticator crate for now as a test) but I'm getting

[INFO  authenticator::statemachine] Statemachine was cancelled. Cancelling transaction now.
[INFO  authenticator::transport::platform::transaction] Transaction was cancelled.

Not sure what's going on there, maybe I'm missing some dependency. I will wait then until the #237 has landed in firefox nightly.

jschanck commented 1 year ago

237 will land in Firefox as part of Bug 1827748. Hopefully in the next day or so.

VincentVanlaer commented 1 year ago

I have gotten around to testing this with the latest patches and the fallback in firefox works well.