From 79f89b85ba19429c2f4974bc012071aa6f542c55 Mon Sep 17 00:00:00 2001 From: Corentin Mors Date: Wed, 19 Jul 2023 15:29:20 +0200 Subject: Add origin in authentication and registration info Closes #414 --- .../server/src/authentication/verifyAuthenticationResponse.test.ts | 3 +++ packages/server/src/authentication/verifyAuthenticationResponse.ts | 3 +++ .../server/src/registration/verifyRegistrationResponse.test.ts | 7 +++++++ packages/server/src/registration/verifyRegistrationResponse.ts | 3 +++ 4 files changed, 16 insertions(+) (limited to 'packages/server/src') diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts index 30eb9d1..5547224 100644 --- a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts +++ b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts @@ -44,6 +44,7 @@ test('should return authenticator info after verification', async () => { expect(verification.authenticationInfo.newCounter).toEqual(144); expect(verification.authenticationInfo.credentialID).toEqual(authenticator.credentialID); + expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); }); test('should throw when response challenge is not expected value', async () => { @@ -224,6 +225,7 @@ test('should support multiple possible origins', async () => { }); expect(verification.verified).toEqual(true); + expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); }); test('should throw an error if origin not in list of expected origins', async () => { @@ -372,6 +374,7 @@ test('should return credential backup info', async () => { expect(verification.authenticationInfo?.credentialDeviceType).toEqual('singleDevice'); expect(verification.authenticationInfo?.credentialBackedUp).toEqual(false); + expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); }); /** diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.ts b/packages/server/src/authentication/verifyAuthenticationResponse.ts index d95bca5..bfc5bf5 100644 --- a/packages/server/src/authentication/verifyAuthenticationResponse.ts +++ b/packages/server/src/authentication/verifyAuthenticationResponse.ts @@ -215,6 +215,7 @@ export async function verifyAuthenticationResponse( credentialDeviceType, credentialBackedUp, authenticatorExtensionResults: extensionsData, + origin: clientDataJSON.origin, }, }; @@ -236,6 +237,7 @@ export async function verifyAuthenticationResponse( * @param authenticationInfo.credentialBackedUp Whether or not the multi-device credential has been * backed up. Always `false` for single-device credentials. **Should be kept in a DB for later * reference!** + * @param authenticationInfo.origin The origin of the website that the authentication occurred on * @param authenticationInfo?.authenticatorExtensionResults The authenticator extensions returned * by the browser */ @@ -247,6 +249,7 @@ export type VerifiedAuthenticationResponse = { userVerified: boolean; credentialDeviceType: CredentialDeviceType; credentialBackedUp: boolean; + origin: string; authenticatorExtensionResults?: AuthenticationExtensionsAuthenticatorOutputs; }; }; diff --git a/packages/server/src/registration/verifyRegistrationResponse.test.ts b/packages/server/src/registration/verifyRegistrationResponse.test.ts index 9fd8a96..2b973e5 100644 --- a/packages/server/src/registration/verifyRegistrationResponse.test.ts +++ b/packages/server/src/registration/verifyRegistrationResponse.test.ts @@ -69,6 +69,7 @@ test('should verify FIDO U2F attestation', async () => { expect(verification.registrationInfo?.attestationObject).toEqual( isoBase64URL.toBuffer(attestationFIDOU2F.response.attestationObject), ); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); }); test('should verify Packed (EC2) attestation', async () => { @@ -140,6 +141,7 @@ test('should verify None attestation', async () => { 'AdKXJEch1aV5Wo7bj7qLHskVY4OoNaj9qu8TPdJ7kSAgUeRxWNngXlcNIGt4gexZGKVGcqZpqqWordXb_he1izY', ), ); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); }); test('should verify None attestation w/RSA public key', async () => { @@ -174,6 +176,7 @@ test('should verify None attestation w/RSA public key', async () => { expect(verification.registrationInfo?.credentialID).toEqual( isoBase64URL.toBuffer('kGXv4RJWLeXRw8Yf3T22K3Gq_GGeDv9OKYmAHLm0Ylo'), ); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); }); test('should throw when response challenge is not expected value', async () => { @@ -415,6 +418,7 @@ test('should validate TPM RSA response (SHA256)', async () => { expect(verification.registrationInfo?.credentialID).toEqual( isoBase64URL.toBuffer('lGkWHPe88VpnNYgVBxzon_MRR9-gmgODveQ16uM_bPM'), ); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); }); test('should validate TPM RSA response (SHA1)', async () => { @@ -450,6 +454,7 @@ test('should validate TPM RSA response (SHA1)', async () => { expect(verification.registrationInfo?.credentialID).toEqual( isoBase64URL.toBuffer('oELnad0f6-g2BtzEn_78iLNoubarlq0xFtOtAMXnflU'), ); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); }); test('should validate Android-Key response', async () => { @@ -485,6 +490,7 @@ test('should validate Android-Key response', async () => { expect(verification.registrationInfo?.credentialID).toEqual( isoBase64URL.toBuffer('PPa1spYTB680cQq5q6qBtFuPLLdG1FQ73EastkT8n0o'), ); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); }); test('should support multiple possible origins', async () => { @@ -496,6 +502,7 @@ test('should support multiple possible origins', async () => { }); expect(verification.verified).toBe(true); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); }); test('should throw an error if origin not in list of expected origins', async () => { diff --git a/packages/server/src/registration/verifyRegistrationResponse.ts b/packages/server/src/registration/verifyRegistrationResponse.ts index 2546813..5a52f6a 100644 --- a/packages/server/src/registration/verifyRegistrationResponse.ts +++ b/packages/server/src/registration/verifyRegistrationResponse.ts @@ -246,6 +246,7 @@ export async function verifyRegistrationResponse( userVerified: flags.uv, credentialDeviceType, credentialBackedUp, + origin: clientDataJSON.origin, authenticatorExtensionResults: extensionsData, }; } @@ -273,6 +274,7 @@ export async function verifyRegistrationResponse( * @param registrationInfo.credentialBackedUp Whether or not the multi-device credential has been * backed up. Always `false` for single-device credentials. **Should be kept in a DB for later * reference!** + * @param registrationInfo.origin The origin of the website that the registration occurred on * @param registrationInfo?.authenticatorExtensionResults The authenticator extensions returned * by the browser */ @@ -289,6 +291,7 @@ export type VerifiedRegistrationResponse = { userVerified: boolean; credentialDeviceType: CredentialDeviceType; credentialBackedUp: boolean; + origin: string; authenticatorExtensionResults?: AuthenticationExtensionsAuthenticatorOutputs; }; }; -- cgit v1.2.3 From 57f58b87892fe01ba62f78be1b9ac219decd854c Mon Sep 17 00:00:00 2001 From: Corentin Mors Date: Thu, 20 Jul 2023 09:30:10 +0200 Subject: Add matched RPID to verify response --- .../verifyAuthenticationResponse.test.ts | 3 +++ .../authentication/verifyAuthenticationResponse.ts | 5 ++++- packages/server/src/helpers/matchExpectedRPID.ts | 13 +++++++++---- .../registration/verifyRegistrationResponse.test.ts | 19 +++++++++++++++++++ .../src/registration/verifyRegistrationResponse.ts | 7 ++++++- 5 files changed, 41 insertions(+), 6 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts index 5547224..c996991 100644 --- a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts +++ b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts @@ -45,6 +45,7 @@ test('should return authenticator info after verification', async () => { expect(verification.authenticationInfo.newCounter).toEqual(144); expect(verification.authenticationInfo.credentialID).toEqual(authenticator.credentialID); expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); + expect(verification.authenticationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should throw when response challenge is not expected value', async () => { @@ -226,6 +227,7 @@ test('should support multiple possible origins', async () => { expect(verification.verified).toEqual(true); expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); + expect(verification.authenticationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should throw an error if origin not in list of expected origins', async () => { @@ -375,6 +377,7 @@ test('should return credential backup info', async () => { expect(verification.authenticationInfo?.credentialDeviceType).toEqual('singleDevice'); expect(verification.authenticationInfo?.credentialBackedUp).toEqual(false); expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); + expect(verification.authenticationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); /** diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.ts b/packages/server/src/authentication/verifyAuthenticationResponse.ts index bfc5bf5..c9f23ca 100644 --- a/packages/server/src/authentication/verifyAuthenticationResponse.ts +++ b/packages/server/src/authentication/verifyAuthenticationResponse.ts @@ -154,7 +154,7 @@ export async function verifyAuthenticationResponse( expectedRPIDs = expectedRPID; } - await matchExpectedRPID(rpIdHash, expectedRPIDs); + const matchedRPID = await matchExpectedRPID(rpIdHash, expectedRPIDs); if (advancedFIDOConfig !== undefined) { const { userVerification: fidoUserVerification } = advancedFIDOConfig; @@ -216,6 +216,7 @@ export async function verifyAuthenticationResponse( credentialBackedUp, authenticatorExtensionResults: extensionsData, origin: clientDataJSON.origin, + rpID: matchedRPID, }, }; @@ -238,6 +239,7 @@ export async function verifyAuthenticationResponse( * backed up. Always `false` for single-device credentials. **Should be kept in a DB for later * reference!** * @param authenticationInfo.origin The origin of the website that the authentication occurred on + * @param authenticationInfo.rpID The RP ID that the authentication occurred on * @param authenticationInfo?.authenticatorExtensionResults The authenticator extensions returned * by the browser */ @@ -250,6 +252,7 @@ export type VerifiedAuthenticationResponse = { credentialDeviceType: CredentialDeviceType; credentialBackedUp: boolean; origin: string; + rpID: string; authenticatorExtensionResults?: AuthenticationExtensionsAuthenticatorOutputs; }; }; diff --git a/packages/server/src/helpers/matchExpectedRPID.ts b/packages/server/src/helpers/matchExpectedRPID.ts index be49fc2..c08c223 100644 --- a/packages/server/src/helpers/matchExpectedRPID.ts +++ b/packages/server/src/helpers/matchExpectedRPID.ts @@ -2,19 +2,22 @@ import { toHash } from './toHash'; import { isoUint8Array } from './iso'; /** - * Go through each expected RP ID and try to find one that matches. Raises an Error if no + * Go through each expected RP ID and try to find one that matches. Returns the unhashed RP ID + * that matched the hash in the response. + * + * Raises an `UnexpectedRPIDHash` error if no match is found */ export async function matchExpectedRPID( rpIDHash: Uint8Array, expectedRPIDs: string[], -): Promise { +): Promise { try { - await Promise.any( + const matchedRPID = await Promise.any( expectedRPIDs.map(expected => { return new Promise((resolve, reject) => { toHash(isoUint8Array.fromASCIIString(expected)).then(expectedRPIDHash => { if (isoUint8Array.areEqual(rpIDHash, expectedRPIDHash)) { - resolve(true); + resolve(expected); } else { reject(); } @@ -22,6 +25,8 @@ export async function matchExpectedRPID( }); }), ); + + return matchedRPID; } catch (err) { const _err = err as Error; diff --git a/packages/server/src/registration/verifyRegistrationResponse.test.ts b/packages/server/src/registration/verifyRegistrationResponse.test.ts index 2b973e5..e6acd2a 100644 --- a/packages/server/src/registration/verifyRegistrationResponse.test.ts +++ b/packages/server/src/registration/verifyRegistrationResponse.test.ts @@ -70,6 +70,7 @@ test('should verify FIDO U2F attestation', async () => { isoBase64URL.toBuffer(attestationFIDOU2F.response.attestationObject), ); expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); + expect(verification.registrationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should verify Packed (EC2) attestation', async () => { @@ -177,6 +178,7 @@ test('should verify None attestation w/RSA public key', async () => { isoBase64URL.toBuffer('kGXv4RJWLeXRw8Yf3T22K3Gq_GGeDv9OKYmAHLm0Ylo'), ); expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); + expect(verification.registrationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should throw when response challenge is not expected value', async () => { @@ -419,6 +421,7 @@ test('should validate TPM RSA response (SHA256)', async () => { isoBase64URL.toBuffer('lGkWHPe88VpnNYgVBxzon_MRR9-gmgODveQ16uM_bPM'), ); expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); + expect(verification.registrationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should validate TPM RSA response (SHA1)', async () => { @@ -455,6 +458,7 @@ test('should validate TPM RSA response (SHA1)', async () => { isoBase64URL.toBuffer('oELnad0f6-g2BtzEn_78iLNoubarlq0xFtOtAMXnflU'), ); expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); + expect(verification.registrationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should validate Android-Key response', async () => { @@ -491,6 +495,7 @@ test('should validate Android-Key response', async () => { isoBase64URL.toBuffer('PPa1spYTB680cQq5q6qBtFuPLLdG1FQ73EastkT8n0o'), ); expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); + expect(verification.registrationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should support multiple possible origins', async () => { @@ -503,6 +508,20 @@ test('should support multiple possible origins', async () => { expect(verification.verified).toBe(true); expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); + expect(verification.registrationInfo?.rpID).toEqual('dev.dontneeda.pw'); +}); + +test('should not set RPID in registrationInfo when not expected', async () => { + const verification = await verifyRegistrationResponse({ + response: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: undefined, + }); + + expect(verification.verified).toBe(true); + expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); + expect(verification.registrationInfo?.rpID).toBeUndefined(); }); test('should throw an error if origin not in list of expected origins', async () => { diff --git a/packages/server/src/registration/verifyRegistrationResponse.ts b/packages/server/src/registration/verifyRegistrationResponse.ts index 5a52f6a..d33cdea 100644 --- a/packages/server/src/registration/verifyRegistrationResponse.ts +++ b/packages/server/src/registration/verifyRegistrationResponse.ts @@ -141,6 +141,7 @@ export async function verifyRegistrationResponse( parsedAuthData; // Make sure the response's RP ID is ours + let matchedRPID: string | undefined; if (expectedRPID) { let expectedRPIDs: string[] = []; if (typeof expectedRPID === 'string') { @@ -149,7 +150,7 @@ export async function verifyRegistrationResponse( expectedRPIDs = expectedRPID; } - await matchExpectedRPID(rpIdHash, expectedRPIDs); + matchedRPID = await matchExpectedRPID(rpIdHash, expectedRPIDs); } // Make sure someone was physically present @@ -247,6 +248,7 @@ export async function verifyRegistrationResponse( credentialDeviceType, credentialBackedUp, origin: clientDataJSON.origin, + rpID: matchedRPID, authenticatorExtensionResults: extensionsData, }; } @@ -275,6 +277,8 @@ export async function verifyRegistrationResponse( * backed up. Always `false` for single-device credentials. **Should be kept in a DB for later * reference!** * @param registrationInfo.origin The origin of the website that the registration occurred on + * @param registrationInfo?.rpID The RP ID that the registration occurred on, if one or more were + * specified in the registration options * @param registrationInfo?.authenticatorExtensionResults The authenticator extensions returned * by the browser */ @@ -292,6 +296,7 @@ export type VerifiedRegistrationResponse = { credentialDeviceType: CredentialDeviceType; credentialBackedUp: boolean; origin: string; + rpID?: string; authenticatorExtensionResults?: AuthenticationExtensionsAuthenticatorOutputs; }; }; -- cgit v1.2.3 From 22ca0cd6dab119a28794dbaa33d650f97ab107a1 Mon Sep 17 00:00:00 2001 From: Corentin Mors Date: Fri, 21 Jul 2023 10:20:15 +0200 Subject: Remove unnecessary tests in verify methods --- .../server/src/authentication/verifyAuthenticationResponse.test.ts | 4 +--- packages/server/src/registration/verifyRegistrationResponse.test.ts | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts index c996991..5a760e4 100644 --- a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts +++ b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts @@ -227,7 +227,6 @@ test('should support multiple possible origins', async () => { expect(verification.verified).toEqual(true); expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); - expect(verification.authenticationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should throw an error if origin not in list of expected origins', async () => { @@ -253,6 +252,7 @@ test('should support multiple possible RP IDs', async () => { }); expect(verification.verified).toEqual(true); + expect(verification.authenticationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); test('should throw an error if RP ID not in list of possible RP IDs', async () => { @@ -376,8 +376,6 @@ test('should return credential backup info', async () => { expect(verification.authenticationInfo?.credentialDeviceType).toEqual('singleDevice'); expect(verification.authenticationInfo?.credentialBackedUp).toEqual(false); - expect(verification.authenticationInfo?.origin).toEqual(assertionOrigin); - expect(verification.authenticationInfo?.rpID).toEqual('dev.dontneeda.pw'); }); /** diff --git a/packages/server/src/registration/verifyRegistrationResponse.test.ts b/packages/server/src/registration/verifyRegistrationResponse.test.ts index e6acd2a..7f89857 100644 --- a/packages/server/src/registration/verifyRegistrationResponse.test.ts +++ b/packages/server/src/registration/verifyRegistrationResponse.test.ts @@ -520,7 +520,6 @@ test('should not set RPID in registrationInfo when not expected', async () => { }); expect(verification.verified).toBe(true); - expect(verification.registrationInfo?.origin).toEqual('https://dev.dontneeda.pw'); expect(verification.registrationInfo?.rpID).toBeUndefined(); }); -- cgit v1.2.3