WordPress / wporg-two-factor

2FA for WordPress.org accounts
38 stars 8 forks source link

Firefox + Touch ID on Mac #314

Closed dd32 closed 2 months ago

dd32 commented 2 months ago

Firefox on Mac has added iCloud keychain support, which allows it to be used for Touch ID based keys, but it appears that we don't fully support it.

This is likely something that needs fixing in the upstream webauthn plugin.

See https://meta.trac.wordpress.org/ticket/7765 for an example of a real world case.

dd32 commented 2 months ago

It turns out that my knowledge was outdated; Firefox has added Touch ID support - but rather, it should be referred to as iCloud keychain support.

As posted by kallookoo on Trac:

I understand what you say and I will follow the issue on github, as it is really related to the plugin, but I clarify that firefox does support Touch ID, as I personally use it on GitHub and of course on apple's own website.

As I commented on the meta ticket, but this time in table form, for Firefox 130:

Service Login with existing key Create a new iCloud key Login with new iCloud key
WordPress.org
GitHub

So Firefox has added iCloud support, which means it should be supportable here, I'm not sure why the existing iCloud keys aren't compatible, and why we can create but not login using them.. I'm assuming / guessing, that this might be due to the storage specification, I have a feeling we're expecting it to be a hardware-key only, not a keychain key?

dd32 commented 2 months ago

Warning: This comment is a brain dump, I'm trying to put my thoughts together :)

Turns out this is a WebAuthN Level 1 vs Level 2 issue, the plugin we're using uses a library which is a Level 1 implementation, and Firefox w/ iCloud appears to require a Level 2 implementation.

In the WebAuthN Level 1 spec for registering a new credential there's no requirement to store the transports provided by the credential, in the case of iCloud this is an internal transport (it's "internal to the host device"). In the Level 2 spec for registering a new credential there's the recommended suggestion that the server should store the list of provided transports and provide those to the browser in future, as a hint for where that credential may be sourced from.

This might be a firefox implementation bug, as if the transports key of the offered PublicKeyCredential is not set ([ie. it's optional](https://www.w3.org/TR/webauthn-2/#sctn-verifying-assertion:~:text=If%20options.allowCredentials%20is%20present%2C%20the%20transports%20member%20of%20each%20item%20SHOULD%20be%20set%20to%20the%20value%20returned%20by%20credential.response.getTransports()%20when%20the%20corresponding%20credential%20was%20registered.)), or none of it's values found the key, the browser MAY look in other locations (ie. USB -> NFC). It appears that Firefox is simply not looking at internal when the key is empty/not-set.. Anyway.. I'm not 100% if this is a Firefox bug or it's expected. I'll need to create a standalone test to verify. WebAuthN w/ iCloud was added in Firefox 122, about a year ago, there appear to be bunch of open issues, but I'm not seeing one that covers this specific aspect.

I went down the path of doing as the Level2 spec said, before I realised the 1v2 and ended up with https://github.com/dd32/wp-two-factor-provider-webauthn/pull/1, today I've taken a different route of a minimal patch that appears to resolve it, by just hinting to browsers to look in all current supported places: https://github.com/dd32/wp-two-factor-provider-webauthn/pull/2 - It appears to allow cross-browser iCloud Touch ID & Yubikeys still work in both Chrome and Safari.

@sjinks how do you feel about PR2 above?

For comparison:

Service Login with existing key Create a new iCloud key Login with new iCloud key
WordPress.org
GitHub
WordPress.org w/ PR1
WordPress.org w/ PR2
sjinks commented 2 months ago

LGTM. I will merge and release a new version to test it.

dd32 commented 2 months ago

@sjinks if it helps, if you can merge to plugins.svn trunk we can test it live on WordPress.org before you push it out as a versioned release, I don't expect this will cause auth issues, but I haven't widely tested it other than cross-browser on Mac.. :)

sjinks commented 2 months ago

I have also tested it with my keys, don't see any regressions :-)

sjinks commented 2 months ago

2.5.0 is live

dd32 commented 2 months ago

Thanks!

kallookoo commented 2 months ago

I just tested it and it works perfectly, thank you