diff options
4 files changed, 72 insertions, 11 deletions
diff --git a/packages/server/src/assertion/verifyAssertionResponse.test.ts b/packages/server/src/assertion/verifyAssertionResponse.test.ts index f7ad2e8..35e510e 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.test.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.test.ts @@ -19,6 +19,7 @@ afterEach(() => { test('should verify an assertion response', () => { const verification = verifyAssertionResponse( assertionResponse, + assertionChallenge, assertionOrigin, authenticator, ); @@ -29,6 +30,7 @@ test('should verify an assertion response', () => { test('should verify an assertion response if origin does not start with https', () => { const verification = verifyAssertionResponse( assertionResponse, + assertionChallenge, 'dev.dontneeda.pw', authenticator, ); @@ -39,6 +41,7 @@ test('should verify an assertion response if origin does not start with https', test('should return authenticator info after verification', () => { const verification = verifyAssertionResponse( assertionResponse, + assertionChallenge, assertionOrigin, authenticator, ); @@ -47,14 +50,26 @@ test('should return authenticator info after verification', () => { expect(verification.authenticatorInfo.base64CredentialID).toEqual(authenticator.base64CredentialID); }); +test('should throw when response challenge is not expected value', () => { + expect(() => { + verifyAssertionResponse( + assertionResponse, + 'shouldhavebeenthisvalue', + 'https://different.address', + authenticator, + ); + }).toThrow(/assertion challenge/i); +}); + test('should throw when response origin is not expected value', () => { expect(() => { verifyAssertionResponse( assertionResponse, + assertionChallenge, 'https://different.address', authenticator, ); - }).toThrow(); + }).toThrow(/assertion origin/i); }); test('should throw when assertion type is not webauthn.create', () => { @@ -62,15 +77,17 @@ test('should throw when assertion type is not webauthn.create', () => { mockDecodeClientData.mockReturnValue({ origin: assertionOrigin, type: 'webauthn.badtype', + challenge: assertionChallenge, }); expect(() => { verifyAssertionResponse( assertionResponse, + assertionChallenge, assertionOrigin, authenticator, ); - }).toThrow(); + }).toThrow(/assertion type/i); }); test('should throw error if user was not present', () => { @@ -81,10 +98,11 @@ test('should throw error if user was not present', () => { expect(() => { verifyAssertionResponse( assertionResponse, + assertionChallenge, assertionOrigin, authenticator, ); - }).toThrow(); + }).toThrow(/not present/i); }); test('should throw error if previous counter value is not less than in response', () => { @@ -98,10 +116,11 @@ test('should throw error if previous counter value is not less than in response' expect(() => { verifyAssertionResponse( assertionResponse, + assertionChallenge, assertionOrigin, badDevice, ); - }).toThrow(); + }).toThrow(/counter value/i); }); const assertionResponse = { @@ -114,6 +133,7 @@ const assertionResponse = { base64Signature: 'MEUCIQDYXBOpCWSWq2Ll4558GJKD2RoWg958lvJSB_GdeokxogIgWuEVQ7ee6AswQY0OsuQ6y8Ks6' + 'jhd45bDx92wjXKs900=' }; +const assertionChallenge = 'dG90YWxseVVuaXF1ZVZhbHVlRXZlcnlUaW1l'; const assertionOrigin = 'https://dev.dontneeda.pw'; const authenticator = { diff --git a/packages/server/src/assertion/verifyAssertionResponse.ts b/packages/server/src/assertion/verifyAssertionResponse.ts index f5ae7f2..91deaf6 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.ts @@ -16,22 +16,31 @@ import parseAuthenticatorData from '@helpers/parseAuthenticatorData'; * Verify that the user has legitimately completed the login process * * @param response Authenticator attestation response with base64-encoded values + * @param expectedChallenge The random value provided to generateAttestationOptions for the + * authenticator to sign * @param expectedOrigin Expected URL of website attestation should have occurred on */ export default function verifyAssertionResponse( response: AuthenticatorAssertionResponseJSON, + expectedChallenge: string, expectedOrigin: string, authenticator: AuthenticatorDevice, ): VerifiedAssertion { const { base64AuthenticatorData, base64ClientDataJSON, base64Signature } = response; const clientDataJSON = decodeClientDataJSON(base64ClientDataJSON); - const { type, origin } = clientDataJSON; + const { type, origin, challenge } = clientDataJSON; if (!expectedOrigin.startsWith('https://')) { expectedOrigin = `https://${expectedOrigin}`; } + if (challenge !== expectedChallenge) { + throw new Error( + `Unexpected assertion challenge "${challenge}", expected "${expectedChallenge}"` + ); + } + // Check that the origin is our site if (origin !== expectedOrigin) { throw new Error(`Unexpected assertion origin "${origin}", expected "${expectedOrigin}"`); @@ -47,7 +56,7 @@ export default function verifyAssertionResponse( const { flags, counter } = authDataStruct; if (!(flags.up)) { - throw new Error('User was NOT present during assertion!'); + throw new Error('User not present during assertion'); } if (counter <= authenticator.counter) { diff --git a/packages/server/src/attestation/verifyAttestationResponse.test.ts b/packages/server/src/attestation/verifyAttestationResponse.test.ts index 485e0e3..3e1a761 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.test.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.test.ts @@ -19,6 +19,7 @@ afterEach(() => { test('should verify FIDO U2F attestation', () => { const verification = verifyAttestationResponse( attestationFIDOU2F, + 'U2d4N3Y0M09McldPb1R5ZExnTloy', 'https://clover.millertime.dev:3000', ); @@ -36,6 +37,7 @@ test('should verify FIDO U2F attestation', () => { test('should verify Packed (EC2) attestation', () => { const verification = verifyAttestationResponse( attestationPacked, + 'czZQSWJCblBQbnJHTlNCeE5kdERyVDdVclZZSks5SE0', 'https://dev.dontneeda.pw' ) @@ -54,6 +56,7 @@ test('should verify Packed (EC2) attestation', () => { test ('should verify Packed (X5C) attestation', () => { const verification = verifyAttestationResponse( attestationPackedX5C, + 'dG90YWxseVVuaXF1ZVZhbHVlRXZlcnlUaW1l', 'https://dev.dontneeda.pw', ); @@ -71,6 +74,7 @@ test ('should verify Packed (X5C) attestation', () => { test('should verify None attestation', () => { const verification = verifyAttestationResponse( attestationNone, + 'aEVjY1BXdXppUDAwSDBwNWd4aDJfdTVfUEM0TmVZZ2Q', 'https://dev.dontneeda.pw' ); @@ -88,6 +92,7 @@ test('should verify None attestation', () => { test('should verify Android SafetyNet attestation', () => { const verification = verifyAttestationResponse( attestationAndroidSafetyNet, + 'X3ZWUG9FNDJEaC13azNidkhtYWt0aVZ2RVlDLUx3Qlg', 'https://dev.dontneeda.pw' ); @@ -102,27 +107,44 @@ test('should verify Android SafetyNet attestation', () => { ); }); +test('should throw when response challenge is not expected value', () => { + expect(() => { + verifyAttestationResponse( + attestationNone, + 'shouldhavebeenthisvalue', + 'https://dev.dontneeda.pw', + ); + }).toThrow(/attestation challenge/i); +}); + test('should throw when response origin is not expected value', () => { expect(() => { verifyAttestationResponse( attestationNone, + 'aEVjY1BXdXppUDAwSDBwNWd4aDJfdTVfUEM0TmVZZ2Q', 'https://different.address' ); - }).toThrow(); + }).toThrow(/attestation origin/i); }); test('should throw when attestation type is not webauthn.create', () => { const origin = 'https://dev.dontneeda.pw'; + const challenge = 'aEVjY1BXdXppUDAwSDBwNWd4aDJfdTVfUEM0TmVZZ2Q'; // @ts-ignore 2345 - mockDecodeClientData.mockReturnValue({ origin, type: 'webauthn.badtype' }); + mockDecodeClientData.mockReturnValue({ + origin, + type: 'webauthn.badtype', + challenge: 'aEVjY1BXdXppUDAwSDBwNWd4aDJfdTVfUEM0TmVZZ2Q', + }); expect(() => { verifyAttestationResponse( attestationNone, + challenge, origin, ); - }).toThrow(); + }).toThrow(/attestation type/i); }); test('should throw if an unexpected attestation format is specified', () => { @@ -136,6 +158,7 @@ test('should throw if an unexpected attestation format is specified', () => { expect(() => { verifyAttestationResponse( attestationNone, + 'aEVjY1BXdXppUDAwSDBwNWd4aDJfdTVfUEM0TmVZZ2Q', 'https://dev.dontneeda.pw', ); }).toThrow(); diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index f302771..41708fc 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -11,21 +11,30 @@ import verifyAndroidSafetynet from './verifications/verifyAndroidSafetyNet'; * Verify that the user has legitimately completed the registration process * * @param response Authenticator attestation response with base64-encoded values + * @param expectedChallenge The random value provided to generateAttestationOptions for the + * authenticator to sign * @param expectedOrigin Expected URL of website attestation should have occurred on */ export default function verifyAttestationResponse( response: AuthenticatorAttestationResponseJSON, + expectedChallenge: string, expectedOrigin: string, ): VerifiedAttestation { const { base64AttestationObject, base64ClientDataJSON } = response; const attestationObject = decodeAttestationObject(base64AttestationObject); const clientDataJSON = decodeClientDataJSON(base64ClientDataJSON); - const { type, origin } = clientDataJSON; + const { type, origin, challenge } = clientDataJSON; + + if (challenge !== expectedChallenge) { + throw new Error( + `Unexpected attestation challenge "${challenge}", expected "${expectedChallenge}"` + ); + } // Check that the origin is our site if (origin !== expectedOrigin) { - throw new Error(`Unexpected attestation origin: ${origin}`); + throw new Error(`Unexpected attestation origin "${origin}", expected "${expectedOrigin}"`); } // Make sure we're handling an attestation |