MasterKale / SimpleWebAuthn

WebAuthn, Simplified. A collection of TypeScript-first libraries for simpler WebAuthn integration. Supports modern browsers, Node, Deno, and more.
https://simplewebauthn.dev
MIT License
1.62k stars 137 forks source link

verifyAuthenticationResponse() rejects "OKP" key type generated by Firefox #456

Closed pmalouin closed 1 year ago

pmalouin commented 1 year ago

Describe the issue

While testing the fix applied in https://github.com/MasterKale/SimpleWebAuthn/pull/441, while the registration ceremony now succeeds with Firefox and a security key, I encounter a new error that is thrown by the verifyAuthenticationResponse():

Error: Signature verification with public key of kty OKP is not supported by this method
              at Object.verify (/xxx/node_modules/@simplewebauthn/server/script/helpers/iso/isoCrypto/verify.js:30:11)
              at verifySignature (/xxx/node_modules/@simplewebauthn/server/script/helpers/verifySignature.js:25:76)
              at verifyAuthenticationResponse (/xxx/node_modules/@simplewebauthn/server/script/authentication/verifyAuthenticationResponse.js:162:66)

This seems to be related to the fact that the kty attribute is parsed as a string ("OKP") instead of an integer (1) like this library expects. There was a brief mention of this typing issue in this comment, which also points out that the string-typed crv attribute could be problematic as well.

Reproduction Steps

  1. Perform a successful registration using Firefox 118 and a security key.
  2. From the call to verifyRegistrationResponse(), grab the registrationInfo.credentialPublicKey attribute and persist it for the user.
  3. Try to perform an authentication ceremony using the same credentials/public key. Call verifyAuthenticationResponse() and pass the persisted public key value via the authenticator.credentialPublicKey attribute.
  4. Error is thrown while validating this public key.

Expected behavior

Expecting a successful verification of the credentials with the public key.

Code Samples + WebAuthn Options and Responses

If helpful, this is an example hex-encoded public key that reproduces the issue: 'a401634f4b50032720674564323535313921982018c118e80d18be18cf18f918a418ca18c618b418df18bc1888185c189a1859184b18c418e118d718e61884186418a618401842185a18e3101867182c1824'

Dependencies

SimpleWebAuthn Libraries

▶ npm list --depth=0 | grep @simplewebauthn
├── @simplewebauthn/server@8.2.0

Additional context

MasterKale commented 1 year ago

Oh right, I only fixed the malformed CBOR, I never actually handled the "strings instead of ints" issue. I guess I'll fix that too 😂

(I'm choosing to laugh instead of cry at the reality of library maintenance, "why should I have to sully my perfect code?!")

pmalouin commented 1 year ago

I was reading a bit about COSE, and it appears that the spec indicates that kty can be typed as a tstr OR int:

int -- an unsigned integer or a negative integer.
...
tstr -- a UTF-8 text string (major type 3).

...that seems to suggest that while Firefox is doing things differently than everyone else, its behavior might still be correct with respect to the spec. And yeah, I empathize with the maintenance burden that this represents and appreciate it much ❤️

MasterKale commented 1 year ago

Emil Lundberg at Yubico (and fellow WAWG participant) helped clear this up when I was dealing with this over in the py_webauthn project:

It looks like the RFC8152 is using 'OKP' as a human-friendly (though ironically confusing) alias of the actual value 1. It's also confusing that the type of kty is defined as tstr / int, but there are only integer values defined in the registry, both on the IANA registry page and in Section 13 of RFC8152:

...

As far as I can tell, all actual values in the COSE registry (the "Value" and "Label" columns) seem to be integer typed.

See https://github.com/duo-labs/py_webauthn/issues/160#issuecomment-1679113276 for the full conversation.

pmalouin commented 1 year ago

I just tested with Firefox Beta 119.0b6 and the issues cannot be reproduced. The kty and crv attributes are correctly set to integers and even the if added in https://github.com/MasterKale/SimpleWebAuthn/pull/441 is not necessary anymore.

MasterKale commented 1 year ago

Yep, the issue was fixed in Firefox 118 (https://github.com/mozilla/authenticator-rs/pull/292) so I'm glad to hear the fix stuck in 119 too.

Maybe the solution here is to tell impacted people, "update your Firefox" instead of supporting the bad behavior. I could document it in the Troubleshooting section of the docs, perhaps https://simplewebauthn.dev/docs/packages/server#troubleshooting 🤔

pmalouin commented 1 year ago

I reproduced the issue on Firefox 118 ☝️ (current stable)

To confirm, I tracked that the authenticator-rs PR was packaged in version v0.4.0-alpha.20, which was then included in Firefox here: https://bugzilla.mozilla.org/show_bug.cgi?id=1850025, and that one is assigned to milestone 119.

Firefox stable appears to release on a ~monthly cadence, so I estimate that it would land in stable channel near the end of October. Fwiw, from our side, we can live with the minor disruption until end of Oct.

MasterKale commented 1 year ago

Argh, too optimistic then. I'll add support for the bad values then like I did in py_webauthn.

FYI this won't happen till sometime next week at the earliest, just to set expectations on a fix.

AntonyChiossi commented 1 year ago

@MasterKale I dont know if related but I added additional information in the closed issue, hope it helps ;) https://github.com/MasterKale/SimpleWebAuthn/issues/440#issuecomment-1750974029

MasterKale commented 1 year ago

I'll add support for the bad values then like I did in py_webauthn.

It turns out I punted on this in py_webauthn and never actually fixed this over there either.

Fwiw, from our side, we can live with the minor disruption until end of Oct.

"macOS Firefox users can't use Ed25519 with their security keys for one more month" probably doesn't impact a huge number of users, especially since an RP can reduce their list of supported algorithms to [-7, -257] to fix the problem till Firefox 119 widely rolls out. I'll make this a "documentation update" fix and check back when Firefox 119 hits Stable.

MasterKale commented 1 year ago

I've documented this error over in the docs: https://simplewebauthn.dev/docs/packages/server#signature-verification-with-public-key-of-kty-okp-is-not-supported-error-during-authentication

Marking this as resolved for now. Thanks again @pmalouin for reporting this 🙏