search

kanidm / webauthn-rs

An implementation of webauthn components for Rustlang servers
Mozilla Public License 2.0
488 stars 80 forks source link

Missing enum variant of `AuthenticatorTransport` causes error on android #431

Closed iganev closed 5 months ago

iganev commented 5 months ago

I did this

Implemented roughly 1:1 the actix tutorial (passkey flow), but instead of the wasm module I am using pure JS.

I expected the following

Accepted the fact that my old Yubikey won't work with the passkey flow, got a new Yubikey 5c, all good. Works. Pushed to an actual public webserver with https and expected the code to work with my phone as well, which previously did not, because localhost.

What actually happened

On my Pixel 7 Pro (Android 14 with Firefox 124.2 and Chrome 123) when I try to register a passkey, the browser includes a transport array that contains "cable" for whatever reason... to elaborate, I am trying to test the generic happy path of using fingerprint on the phone.

Version (and git commit)

v0.4.8 from crates.io

Operating System / Version

Pixel 7 Pro (Android 14 with Firefox 124.2 and Chrome 123)

Any other comments

Great job guys. I realize implementing such a sh*t show of a "standard" is no easy task, especially with security in mind.

yaleman commented 5 months ago

This is a known issue with Android clients, noted here - it shouldn't break things though?

Ref: https://github.com/kanidm/webauthn-rs/issues/364 fixed in #367

iganev commented 5 months ago

It errors during deserialization of RegisterPublicKeyCredential and its corresponding AuthenticatorAttestationResponseRaw.

AuthenticatorTransport doesn't offer a default deserialize variant as a catch-all option.

A variant Unkown with #[serde(other)] attribute would prevent this error.

Or even Unknown(String) with #[serde(other)] attribute along with the serde-enum-str crate.

P.S.: I see that exact change is made in 0.5.0-dev but is missing in webauthn-rs-proto 0.4.9 (that I have).

Thanks, I will update.