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

Device registration fails with firefox (works with Chrome) #463

Closed AntonyChiossi closed 1 year ago

AntonyChiossi commented 1 year ago

As requested here: https://github.com/MasterKale/SimpleWebAuthn/issues/440#issuecomment-1756923797

After updating both packages to their latest version, registration fails but with a different error this time. Chrome keeps working.

Here some debugging info:

Working with Chrome:

{
  request: {
     challenge: 'NkUgPU8KfTEuMF-78rbkr81NIF0SoU9hBhkFRffApI0',
     rp: { name: 'REDACTED', id: 'localhost' },
     user: {
       id: 'REDACTED',
       name: 'REDACTED',
       displayName: 'REDACTED'
     },
     pubKeyCredParams: [ [Object], [Object], [Object] ],
     timeout: 60000,
     attestation: 'indirect',
     excludeCredentials: [],
     authenticatorSelection: {
       residentKey: 'preferred',
       userVerification: 'preferred',
       requireResidentKey: false
     },
     extensions: { credProps: true }
   },
   response: {
     id: 'KEthTlOv7trIQ0g9hvZmvAOxVOmhh-UPa7QotW5nNDUNsJXdIviXylbk5HvEhtfx',
     rawId: 'KEthTlOv7trIQ0g9hvZmvAOxVOmhh-UPa7QotW5nNDUNsJXdIviXylbk5HvEhtfx',
     response: {
       attestationObject: 'o2NmbXRmcGFja2VkZ2F0dFN0bXSjY2FsZyZjc2lnWEYwRAIgaDfm9cLN7bE8fIIT2JwNNjpggnHXClnAiBNZ3WYrNBMCICoDf7lc6Y3eX70TNVWyYTy75BdmPPSmN88lcfQ97eAzY3g1Y4FZAt0wggLZMIIBwaADAgECAgkA4RPwzmmndbYwDQYJKoZIhvcNAQELBQAwLjEsMCoGA1UEAxMjWXViaWNvIFUyRiBSb290IENBIFNlcmlhbCA0NTcyMDA2MzEwIBcNMTQwODAxMDAwMDAwWhgPMjA1MDA5MDQwMDAwMDBaMG8xCzAJBgNVBAYTAlNFMRIwEAYDVQQKDAlZdWJpY28gQUIxIjAgBgNVBAsMGUF1dGhlbnRpY2F0b3IgQXR0ZXN0YXRpb24xKDAmBgNVBAMMH1l1YmljbyBVMkYgRUUgU2VyaWFsIDEzMjQzNTY1NzcwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS-EvB5JATjjS2-GSN4diiQ48S8kAj5g26rCBkDfZIcQYjKL6qE6gKM39rgzxD1khkn6i1A40ik2wC_8Z_60--qo4GBMH8wEwYKKwYBBAGCxAoNAQQFBAMFBAMwIgYJKwYBBAGCxAoCBBUxLjMuNi4xLjQuMS40MTQ4Mi4xLjEwEwYLKwYBBAGC5RwCAQEEBAMCBDAwIQYLKwYBBAGC5RwBAQQEEgQQpOn8bUy-R1i4ujdZi7W7qjAMBgNVHRMBAf8EAjAAMA0GCSqGSIb3DQEBCwUAA4IBAQChRXSOMBqLK8u9kS4S3r2Ma7Dz740fXnifG3l8Il57IXCDLH9r3qXAG9-a8cBVTF4MvGOBMK90wB1B8LXlCUM6lRM2sKDYN2c1AxhYB3T4EEmVuxT9dDwumyOOm68aOi2nnGDKtgxelx-71dj-bKvZGN3ToOysx8WKbsnxHnDo1qNOTxyxeEgKKG_AXjxLIAgZHTXaYTrY_h6uGVr96lPzKGD-Im7lVsdWE9Bce7xzRoO62faYWC71xbBiNWU-t53v8QBDi_Ua4zYR0Zm_ULnwRUqHQ4X5MPLzxRw59NBh3JUuVKWC69ByxFqIOlFm56Y_UuLvYo8E3Q_wsPBj1WBraGF1dGhEYXRhWJ9Jlg3liA6MaHQ0Fw9kdmBbj-SuuaKGMseZXPO6gx2XY8UAAAAEpOn8bUy-R1i4ujdZi7W7qgAwKEthTlOv7trIQ0g9hvZmvAOxVOmhh-UPa7QotW5nNDUNsJXdIviXylbk5HvEhtfxpAEBAycgBiFYIChLYU5Tr-7ayENIPYac8k9s8ypXjiqCFkDg_FpuygxpoWtjcmVkUHJvdGVjdAI',
       clientDataJSON: 'eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoiNVFVQml0NjRiZFViQ21faGtFSWN1ZnJkNDY3VVhidjdXa1U3VW5xYlM5OCIsIm9yaWdpbiI6Imh0dHBzOi8vbG9jYWxob3N0OjQyMDAiLCJjcm9zc09yaWdpbiI6ZmFsc2V9',
       transports: [Array],
       publicKeyAlgorithm: -8,
       publicKey: 'MCowBQYDK2VwAyEAKEthTlOv7trIQ0g9hpzyT2zzKleOKoIWQOD8Wm7KDGk',
       authenticatorData: 'SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2PFAAAABKTp_G1MvkdYuLo3WYu1u6oAMChLYU5Tr-7ayENIPYb2ZrwDsVTpoYflD2u0KLVuZzQ1DbCV3SL4l8pW5OR7xIbX8aQBAQMnIAYhWCAoS2FOU6_u2shDSD2GnPJPbPMqV44qghZA4PxabsoMaaFrY3JlZFByb3RlY3QC'
     },
     type: 'public-key',
     clientExtensionResults: { credProps: [Object] },
     authenticatorAttachment: 'cross-platform'
   },
   expectedChallenge: '5QUBit64bdUbCm_hkEIcufrd467UXbv7WkU7UnqbS98',
   expectedOrigin: 'https://localhost:4200',
   expectedRPID: 'localhost',
   verification: {
     verified: true,
     registrationInfo: {
       fmt: 'packed',
       counter: 4,
       aaguid: 'a4e9fc6d-4cbe-4758-b8ba-37598bb5bbaa',
       credentialID: [Uint8Array],
       credentialPublicKey: [Uint8Array],
       credentialType: 'public-key',
       attestationObject: [Uint8Array],
       userVerified: true,
       credentialDeviceType: 'singleDevice',
       credentialBackedUp: false,
       origin: 'https://localhost:4200',
       rpID: 'localhost',
       authenticatorExtensionResults: [Object]
     }
   }
 }

Failing with Firefox

{
  request: {
     challenge: 'OqmoL2yMQdmwcL4jLL85SkcEq1_6cI0InUDUcO1WE1o',
     rp: { name: 'REDACTED', id: 'localhost' },
     user: {
       id: '63d3d082b7610778c308f25c',
       name: 'REDACTED',
       displayName: 'REDACTED'
     },
     pubKeyCredParams: [ [Object], [Object], [Object] ],
     timeout: 60000,
     attestation: 'indirect',
     excludeCredentials: [],
     authenticatorSelection: {
       residentKey: 'preferred',
       userVerification: 'preferred',
       requireResidentKey: false
     },
     extensions: { credProps: true }
   },
   response: {
     id: '2PvlO37W-pdWdMvEnigOU0BlBZ-PtE4OZSZsxPbRN8bodRw378VPVmaur1Oks6u5',
     rawId: '2PvlO37W-pdWdMvEnigOU0BlBZ-PtE4OZSZsxPbRN8bodRw378VPVmaur1Oks6u5',
     response: {
       attestationObject: 'o2NmbXRmcGFja2VkZ2F0dFN0bXSjY2FsZyZjc2lnWEYwRAIgHcyLYtm1V14B1Y90sNTpZGXpi2OV40za0-8xkyYtTeECICkGdaW-oRAJHrkbUuEetcGVgRXxGXHtAbkZt2fTKS2kY3g1Y4FZAt0wggLZMIIBwaADAgECAgkA4RPwzmmndbYwDQYJKoZIhvcNAQELBQAwLjEsMCoGA1UEAxMjWXViaWNvIFUyRiBSb290IENBIFNlcmlhbCA0NTcyMDA2MzEwIBcNMTQwODAxMDAwMDAwWhgPMjA1MDA5MDQwMDAwMDBaMG8xCzAJBgNVBAYTAlNFMRIwEAYDVQQKDAlZdWJpY28gQUIxIjAgBgNVBAsMGUF1dGhlbnRpY2F0b3IgQXR0ZXN0YXRpb24xKDAmBgNVBAMMH1l1YmljbyBVMkYgRUUgU2VyaWFsIDEzMjQzNTY1NzcwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS-EvB5JATjjS2-GSN4diiQ48S8kAj5g26rCBkDfZIcQYjKL6qE6gKM39rgzxD1khkn6i1A40ik2wC_8Z_60--qo4GBMH8wEwYKKwYBBAGCxAoNAQQFBAMFBAMwIgYJKwYBBAGCxAoCBBUxLjMuNi4xLjQuMS40MTQ4Mi4xLjEwEwYLKwYBBAGC5RwCAQEEBAMCBDAwIQYLKwYBBAGC5RwBAQQEEgQQpOn8bUy-R1i4ujdZi7W7qjAMBgNVHRMBAf8EAjAAMA0GCSqGSIb3DQEBCwUAA4IBAQChRXSOMBqLK8u9kS4S3r2Ma7Dz740fXnifG3l8Il57IXCDLH9r3qXAG9-a8cBVTF4MvGOBMK90wB1B8LXlCUM6lRM2sKDYN2c1AxhYB3T4EEmVuxT9dDwumyOOm68aOi2nnGDKtgxelx-71dj-bKvZGN3ToOysx8WKbsnxHnDo1qNOTxyxeEgKKG_AXjxLIAgZHTXaYTrY_h6uGVr96lPzKGD-Im7lVsdWE9Bce7xzRoO62faYWC71xbBiNWU-t53v8QBDi_Ua4zYR0Zm_ULnwRUqHQ4X5MPLzxRw59NBh3JUuVKWC69ByxFqIOlFm56Y_UuLvYo8E3Q_wsPBj1WBraGF1dGhEYXRhWLlJlg3liA6MaHQ0Fw9kdmBbj-SuuaKGMseZXPO6gx2XY0UAAAAEpOn8bUy-R1i4ujdZi7W7qgAw2PvlO37W-pdWdMvEnigOU0BlBZ-PtE4OZSZsxPbRN8bodRw378VPVmaur1Oks6u5owFjT0tQAycgZ0VkMjU1MTkhmCAY2Bj7GOUYOxh-GNYY-hiXGFYYdBjLGMQYnhjCGJYYuRjrGF8Yvhh0GB0YkhhHDhg3GE4YewcYORhPGNAY6A',
       clientDataJSON: 'eyJjaGFsbGVuZ2UiOiJEVHlCRUxQWEx5WHIwZlQ1ZmttaGlzbF9BRU1jMTNld1FUUVMteExDdkNRIiwib3JpZ2luIjoiaHR0cHM6Ly9sb2NhbGhvc3Q6NDIwMCIsInR5cGUiOiJ3ZWJhdXRobi5jcmVhdGUifQ'
     },
     type: 'public-key',
     clientExtensionResults: {}
   },
   expectedChallenge: 'DTyBELPXLyXr0fT5fkmhisl_AEMc13ewQTQS-xLCvCQ',
   expectedOrigin: 'https://localhost:4200',
   expectedRPID: 'localhost',
   verification: { verified: false }
 }

The verification key is the result of the method verifyRegistrationResponse

MasterKale commented 1 year ago

@AntonyChiossi Can you confirm that you're testing this with a security key? I see -8 for an algorithm ID in the Chrome response. And which version of Firefox and which OS again?

Bizarre that the same implementation of SimpleWebAuthn succeeds with responses in Chrome but not in Firefox...

Mar0xy commented 1 year ago

Other users are also running into a similar issue on another software that uses this module for webauthn where it works fine on chrome/chromium stable but not firefox and chrome dev but what is different is in this case it spits out a certificate error on chrome dev.

See the open issue on the other software: https://github.com/misskey-dev/misskey/issues/12115

Snippet of the error

Error: Certificate not good after "Sun Oct 22 2023 00:43:10 GMT+0000 (Coordinated Universal Time)" (Packed|Full)
    at verifyAttestationPacked (file:///node_modules/.pnpm/@simplewebauthn+server@8.3.2/node_modules/@simplewebauthn/server/esm/registration/verifications/verifyAttestationPacked.js:55:19)
    at verifyRegistrationResponse (file:///node_modules/.pnpm/@simplewebauthn+server@8.3.2/node_modules/@simplewebauthn/server/esm/registration/verifyRegistrationResponse.js:164:26)
MasterKale commented 1 year ago

@AntonyChiossi After some experimentation tonight it just dawned on me that the "fix" I added to address #440 is going to break security key responses in those versions of Firefox suffering the malformed CBOR issue because I'm flipping a bit in authData. This breaks signature verification for these bad responses because authData is part of the bytes that get signed over:

https://github.com/MasterKale/SimpleWebAuthn/blob/master/packages/server/src/registration/verifications/verifyAttestationPacked.ts#L47

Unfortunately, even when I experimented with flipping the bit back so that authData isn't changed at all, signature verification of that Firefox response is still broken. Part of me wonders if there's not more fallout related to the malformed authData structure in Firefox. For one, the CBOR-encoded credential public key decodes to { kty: "OKP", crv: "Ed25519" } instead of { kty: 1, crv: 6 } like it does in Chrome. Without diving into Firefox 117 and 118 to see how they constructed authData I can only speculate that there's something else going on in these versions of Firefox that's continuing to break signature verification.

Based on all of this I'm inclined to say that this isn't something I can really address in this library. A "fix" for this issue is likely to be to specify only [-7, -257] for supportedAlgorithmIDs when generating registration options, and remove this option (to go back to the default of [-8, -7, -257]) after Firefox 119 comes out with a proper fix.

MasterKale commented 1 year ago

I'm leaving this issue open for now as I'm going to add in a small change that'll prevent bad CBOR handling from permanently changing authData for the rest of the response verification.

MasterKale commented 1 year ago

I've released @simplewebauthn/server@8.3.3 that restores authData after handling bad CBOR. That should remove anything SimpleWebAuthn might be doing that would cause the failing signature verification with security key responses only in Firefox...

I'll leave this issue open for a bit in case someone (😉) with a bit more expertise around signature verification and CBOR sees something else I'm missing here. Otherwise I'll have to close this out as an issue with Firefox that can't be solved at the library level.

aseigler commented 1 year ago

The 0xa4 to 0xa3 thing is definitely a no-no, you're actually losing all the key material when you do that.

"Good": A401634F4B50032720674564323535313921982018D818FB18E5183B187E18D618FA18971856187418CB18C4189E18C2189618B918EB185F18BE1874181D189218470E1837184E187B071839184F18D018E8

{1: "OKP", 3: -8, -1: "Ed25519", -2: [216, 251, 229, 59, 126, 214, 250, 151, 86, 116, 203, 196, 158, 194, 150, 185, 235, 95, 190, 116, 29, 146, 71, 14, 55, 78, 123, 7, 57, 79, 208, 232]}

Bad:

A301634F4B50032720674564323535313921982018D818FB18E5183B187E18D618FA18971856187418CB18C4189E18C2189618B918EB185F18BE1874181D189218470E1837184E187B071839184F18D018E8

{1: "OKP", 3: -8, -1: "Ed25519"}

That "Good" actually looks pretty hosed to me, when did we decide we could use strings instead of int values? It should look like:

A401010327200621583E18D818FB18E5183B187E18D618FA18971856187418CB18C4189E18C2189618B918EB185F18BE1874181D189218470E1837184E187B071839184F18D018E8

{1: 1, 3: -8, -1: 6, -2: h'18D818FB18E5183B187E18D618FA18971856187418CB18C4189E18C2189618B918EB185F18BE1874181D189218470E1837184E187B071839184F18D018E8'}

I feel like data is getting messed with along the way, which would invalidate the signature of course.

The key used is one of the new FIDO only Yubico Security Keys, the ones that used to be blue but are now black.

MasterKale commented 1 year ago

The 0xa4 to 0xa3 thing is definitely a no-no, you're actually losing all the key material when you do that.

Now I'm resetting the 0xa4 back to the 0xa3 it was before, I had to flip 0xa3 to 0xa4 because the bad CBOR encoding from Firefox 117 and Firefox 118 prevented the credential public key from being parsed. See #440 for more context if you want, so far I've been working to support these bad -8 security key responses.

I added the flip back just now because I realized I was invalidating signatures by permanently changing authData, so SimpleWebAuthn should no longer be responsible for the failing signature validation that's still gone unsolved in this issue.

MasterKale commented 1 year ago

That "Good" actually looks pretty hosed to me, when did we decide we could use strings instead of int values? It should look like:

We didn't, Mozilla goofed and fixed the problem in Firefox 119 https://github.com/mozilla/authenticator-rs/pull/292

aseigler commented 1 year ago

But 0xa3 would be 3 pairs, 0xa4 would be 4 pairs which is what we should see for this...

aseigler commented 1 year ago

That "Good" actually looks pretty hosed to me, when did we decide we could use strings instead of int values? It should look like:

We didn't, Mozilla goofed and fixed the problem in Firefox 119 mozilla/authenticator-rs#292

This does not surprise me at all, this reminded me of https://bugzilla.mozilla.org/show_bug.cgi?id=1759098.

AntonyChiossi commented 1 year ago

Hi @MasterKale, thanks for investigating on this issue. Is it still open in the lastest version right? if so, is there any workaround I can try? Is this a good one?

MasterKale commented 1 year ago

Sorry to leave you hanging @AntonyChiossi, I'd say "yes" to your question about limiting public key credential algorithms to ES256 (-7) and RS256 (-257) in the meantime when calling generateRegistrationOptions() (i.e. setting supportedAlgorithmIDs: [-7, -257].) That should "solve" this problem by sidestepping the use of Ed25519 entirely.

Closing this out as there's not much more to do at the library level.