From 1683e99a6e6a33bd6737240ca6bbc8a9d64b798b Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 17:54:47 -0700 Subject: Replace asciiToBinary with more base64url usage --- packages/server/src/helpers/asciiToBinary.ts | 8 -------- packages/server/src/helpers/decodeClientDataJSON.ts | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) delete mode 100644 packages/server/src/helpers/asciiToBinary.ts (limited to 'packages/server/src') diff --git a/packages/server/src/helpers/asciiToBinary.ts b/packages/server/src/helpers/asciiToBinary.ts deleted file mode 100644 index beb6f1d..0000000 --- a/packages/server/src/helpers/asciiToBinary.ts +++ /dev/null @@ -1,8 +0,0 @@ -/** - * Decode a base64-encoded string to a binary string - * - * @param input Base64-encoded string - */ -export default function asciiToBinary(input: string): string { - return Buffer.from(input, 'base64').toString('binary'); -} diff --git a/packages/server/src/helpers/decodeClientDataJSON.ts b/packages/server/src/helpers/decodeClientDataJSON.ts index c0ebb2b..93c6b10 100644 --- a/packages/server/src/helpers/decodeClientDataJSON.ts +++ b/packages/server/src/helpers/decodeClientDataJSON.ts @@ -1,10 +1,10 @@ -import asciiToBinary from './asciiToBinary'; +import base64url from 'base64url'; /** * Decode an authenticator's base64-encoded clientDataJSON to JSON */ export default function decodeClientDataJSON(data: string): ClientDataJSON { - const toString = asciiToBinary(data); + const toString = base64url.decode(data); const clientData: ClientDataJSON = JSON.parse(toString); // `challenge` will be Base64-encoded here. Decode it for easier comparisons with what is provided -- cgit v1.2.3 From 506d1a5beb91ad5e65b30e10cb056d74bc49298c Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 17:55:58 -0700 Subject: Replace “base64” with “base64url” in some comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/server/src/helpers/decodeAttestationObject.test.ts | 4 ++-- packages/server/src/helpers/decodeClientDataJSON.test.ts | 2 +- packages/server/src/helpers/decodeClientDataJSON.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/helpers/decodeAttestationObject.test.ts b/packages/server/src/helpers/decodeAttestationObject.test.ts index e8eb364..2f88f2a 100644 --- a/packages/server/src/helpers/decodeAttestationObject.test.ts +++ b/packages/server/src/helpers/decodeAttestationObject.test.ts @@ -1,6 +1,6 @@ import decodeAttestationObject from './decodeAttestationObject'; -test('should decode base64-encoded indirect attestationObject', () => { +test('should decode base64url-encoded indirect attestationObject', () => { const decoded = decodeAttestationObject( 'o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVjEAbElFazplpnc037DORGDZNjDq86cN9vm6' + '+APoAM20wtBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQKmPuEwByQJ3e89TccUSrCGDkNWquhevjLLn/' + @@ -13,7 +13,7 @@ test('should decode base64-encoded indirect attestationObject', () => { expect(decoded.authData).toBeDefined(); }); -test('should decode base64-encoded direct attestationObject', () => { +test('should decode base64url-encoded direct attestationObject', () => { const decoded = decodeAttestationObject( 'o2NmbXRoZmlkby11MmZnYXR0U3RtdKJjc2lnWEgwRgIhAK40WxA0t7py7AjEXvwGwTlmqlvrOk' + 's5g9lf+9zXzRiVAiEA3bv60xyXveKDOusYzniD7CDSostCet9PYK7FLdnTdZNjeDVjgVkCwTCCAr0wggGloAMCAQICBCrn' + diff --git a/packages/server/src/helpers/decodeClientDataJSON.test.ts b/packages/server/src/helpers/decodeClientDataJSON.test.ts index 7674ec5..b1a7940 100644 --- a/packages/server/src/helpers/decodeClientDataJSON.test.ts +++ b/packages/server/src/helpers/decodeClientDataJSON.test.ts @@ -1,6 +1,6 @@ import decodeClientDataJSON from './decodeClientDataJSON'; -test('should convert base64-encoded attestation clientDataJSON to JSON', () => { +test('should convert base64url-encoded attestation clientDataJSON to JSON', () => { expect( decodeClientDataJSON( 'eyJjaGFsbGVuZ2UiOiJVMmQ0TjNZME0wOU1jbGRQYjFSNVpFeG5UbG95IiwiY2xpZW50RXh0ZW5zaW9ucyI6e30' + diff --git a/packages/server/src/helpers/decodeClientDataJSON.ts b/packages/server/src/helpers/decodeClientDataJSON.ts index 93c6b10..7c09b0d 100644 --- a/packages/server/src/helpers/decodeClientDataJSON.ts +++ b/packages/server/src/helpers/decodeClientDataJSON.ts @@ -1,13 +1,13 @@ import base64url from 'base64url'; /** - * Decode an authenticator's base64-encoded clientDataJSON to JSON + * Decode an authenticator's base64url-encoded clientDataJSON to JSON */ export default function decodeClientDataJSON(data: string): ClientDataJSON { const toString = base64url.decode(data); const clientData: ClientDataJSON = JSON.parse(toString); - // `challenge` will be Base64-encoded here. Decode it for easier comparisons with what is provided + // `challenge` will be Base64URL-encoded here. Decode it for easier comparisons with what is provided // as the expected value clientData.challenge = Buffer.from(clientData.challenge, 'base64').toString('ascii'); -- cgit v1.2.3 From 328dc36408c2d3d1eac4829bd4cee6a6b573af99 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 17:56:47 -0700 Subject: Use base64url to decode clientDataJSON --- packages/server/src/helpers/decodeClientDataJSON.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/helpers/decodeClientDataJSON.ts b/packages/server/src/helpers/decodeClientDataJSON.ts index 7c09b0d..52bbf4c 100644 --- a/packages/server/src/helpers/decodeClientDataJSON.ts +++ b/packages/server/src/helpers/decodeClientDataJSON.ts @@ -7,9 +7,9 @@ export default function decodeClientDataJSON(data: string): ClientDataJSON { const toString = base64url.decode(data); const clientData: ClientDataJSON = JSON.parse(toString); - // `challenge` will be Base64URL-encoded here. Decode it for easier comparisons with what is provided - // as the expected value - clientData.challenge = Buffer.from(clientData.challenge, 'base64').toString('ascii'); + // `challenge` will be Base64URL-encoded here. Decode it for easier comparisons with what is + // provided as the expected value + clientData.challenge = base64url.decode(clientData.challenge); return clientData; } -- cgit v1.2.3 From cd551162ed03a421c689913e8d53b4c2a227fd22 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 17:58:47 -0700 Subject: Reposition attestation check for type as per spec --- packages/server/src/attestation/verifyAttestationResponse.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index ed4ac5c..07eeae6 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -29,6 +29,11 @@ export default function verifyAttestationResponse( const { type, origin, challenge } = clientDataJSON; + // Make sure we're handling an attestation + if (type !== 'webauthn.create') { + throw new Error(`Unexpected attestation type: ${type}`); + } + if (challenge !== expectedChallenge) { throw new Error( `Unexpected attestation challenge "${challenge}", expected "${expectedChallenge}"`, @@ -40,11 +45,6 @@ export default function verifyAttestationResponse( throw new Error(`Unexpected attestation origin "${origin}", expected "${expectedOrigin}"`); } - // Make sure we're handling an attestation - if (type !== 'webauthn.create') { - throw new Error(`Unexpected attestation type: ${type}`); - } - const { fmt } = attestationObject; /** -- cgit v1.2.3 From b760396cd1ddbdd6ae850d87c2e2b2a0d5c5ea78 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 19:31:28 -0700 Subject: Verify RP ID during attestation --- .../src/attestation/verifyAttestationResponse.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 07eeae6..40a619f 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -4,6 +4,8 @@ import { import decodeAttestationObject, { ATTESTATION_FORMATS } from '../helpers/decodeAttestationObject'; import decodeClientDataJSON from '../helpers/decodeClientDataJSON'; +import parseAuthenticatorData from '../helpers/parseAuthenticatorData'; +import toHash from '../helpers/toHash'; import verifyFIDOU2F from './verifications/verifyFIDOU2F'; import verifyPacked from './verifications/verifyPacked'; @@ -22,9 +24,9 @@ export default function verifyAttestationResponse( credential: AttestationCredentialJSON, expectedChallenge: string, expectedOrigin: string, + expectedRPID: string, ): VerifiedAttestation { const { response } = credential; - const attestationObject = decodeAttestationObject(response.attestationObject); const clientDataJSON = decodeClientDataJSON(response.clientDataJSON); const { type, origin, challenge } = clientDataJSON; @@ -34,6 +36,7 @@ export default function verifyAttestationResponse( throw new Error(`Unexpected attestation type: ${type}`); } + // Ensure the device provided the challenge we gave it if (challenge !== expectedChallenge) { throw new Error( `Unexpected attestation challenge "${challenge}", expected "${expectedChallenge}"`, @@ -45,7 +48,18 @@ export default function verifyAttestationResponse( throw new Error(`Unexpected attestation origin "${origin}", expected "${expectedOrigin}"`); } - const { fmt } = attestationObject; + const attestationObject = decodeAttestationObject(response.attestationObject); + const { fmt, authData, attStmt } = attestationObject; + + const parsedAuthData = parseAuthenticatorData(authData); + const { rpIdHash, flags, COSEPublicKey } = parsedAuthData; + + // Make sure the response's RP ID is ours + const expectedRPIDHash = toHash(Buffer.from(expectedRPID, 'ascii')); + if (!rpIdHash.equals(expectedRPIDHash)) { + throw new Error(`Unexpected RP ID hash`); + } + /** * Verification can only be performed when attestation = 'direct' -- cgit v1.2.3 From 06c19cae4618f04d1d3516eb59855b4650fa914a Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 19:44:25 -0700 Subject: Fix attestation verification tests --- .../attestation/verifyAttestationResponse.test.ts | 47 +++++++++------------- 1 file changed, 18 insertions(+), 29 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifyAttestationResponse.test.ts b/packages/server/src/attestation/verifyAttestationResponse.test.ts index 1e4cc0d..d481138 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.test.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.test.ts @@ -20,17 +20,18 @@ test('should verify FIDO U2F attestation', () => { const verification = verifyAttestationResponse( attestationFIDOU2F, attestationFIDOU2FChallenge, - 'https://clover.millertime.dev:3000', + 'https://dev.dontneeda.pw', + 'dev.dontneeda.pw', ); expect(verification.verified).toEqual(true); expect(verification.authenticatorInfo?.fmt).toEqual('fido-u2f'); expect(verification.authenticatorInfo?.counter).toEqual(0); expect(verification.authenticatorInfo?.base64PublicKey).toEqual( - 'BHVixulLxshxcP5P27-v5Os_yy4EjuSl818NhHFMZBF_XmlS8_3G8qCr0SIP6vqu7Wp9FTfot1kdATgQnLjT-8s', + 'BMiRyw5pUoMhBjCrcQND6lJPaRHA0f-XWcKBb5ZwWk1eFJu6aan4o7epl6qa9n9T-6KsIMvZE2PcTnLj8rN58is', ); expect(verification.authenticatorInfo?.base64CredentialID).toEqual( - 'YVh69pHvWm1Tli1c5KdXM9BOwaAr6AuIEqeo9YGZlc1G-MhKqUvGLACnOWt-RNzeUQxgxq2N4AIKeyKM6Q0QYw', + 'VHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUQ', ); }); @@ -39,6 +40,7 @@ test('should verify Packed (EC2) attestation', () => { attestationPacked, attestationPackedChallenge, 'https://dev.dontneeda.pw', + 'dev.dontneeda.pw', ); expect(verification.verified).toEqual(true); @@ -58,6 +60,7 @@ test('should verify Packed (X5C) attestation', () => { attestationPackedX5C, attestationPackedX5CChallenge, 'https://dev.dontneeda.pw', + 'dev.dontneeda.pw', ); expect(verification.verified).toEqual(true); @@ -76,6 +79,7 @@ test('should verify None attestation', () => { attestationNone, attestationNoneChallenge, 'https://dev.dontneeda.pw', + 'dev.dontneeda.pw', ); expect(verification.verified).toEqual(true); @@ -94,6 +98,7 @@ test('should verify Android SafetyNet attestation', () => { attestationAndroidSafetyNet, attestationAndroidSafetyNetChallenge, 'https://dev.dontneeda.pw', + 'dev.dontneeda.pw', ); expect(verification.verified).toEqual(true); @@ -113,6 +118,7 @@ test('should throw when response challenge is not expected value', () => { attestationNone, 'shouldhavebeenthisvalue', 'https://dev.dontneeda.pw', + 'dev.dontneeda.pw', ); }).toThrow(/attestation challenge/i); }); @@ -123,6 +129,7 @@ test('should throw when response origin is not expected value', () => { attestationNone, attestationNoneChallenge, 'https://different.address', + 'dev.dontneeda.pw', ); }).toThrow(/attestation origin/i); }); @@ -139,7 +146,7 @@ test('should throw when attestation type is not webauthn.create', () => { }); expect(() => { - verifyAttestationResponse(attestationNone, challenge, origin); + verifyAttestationResponse(attestationNone, challenge, origin, 'dev.dontneeda.pw'); }).toThrow(/attestation type/i); }); @@ -156,40 +163,22 @@ test('should throw if an unexpected attestation format is specified', () => { attestationNone, attestationNoneChallenge, 'https://dev.dontneeda.pw', + '', ); }).toThrow(); }); const attestationFIDOU2F = { - id: 'YVh69pHvWm1Tli1c5KdXM9BOwaAr6AuIEqeo9YGZlc1G-MhKqUvGLACnOWt-RNzeUQxgxq2N4AIKeyKM6Q0QYw', - rawId: 'YVh69pHvWm1Tli1c5KdXM9BOwaAr6AuIEqeo9YGZlc1G+MhKqUvGLACnOWt+RNzeUQxgxq2N4AIKeyKM6Q0QYw==', + id: 'VHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUQ', + rawId: 'VHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUQ', response: { - attestationObject: - 'o2NmbXRoZmlkby11MmZnYXR0U3RtdKJjc2lnWEgwRgIhAK40WxA0t7py7AjEXvwGw' + - 'TlmqlvrOks5g9lf+9zXzRiVAiEA3bv60xyXveKDOusYzniD7CDSostCet9PYK7FLdnTdZNjeDVjgVkCwTCCAr0wg' + - 'gGloAMCAQICBCrnYmMwDQYJKoZIhvcNAQELBQAwLjEsMCoGA1UEAxMjWXViaWNvIFUyRiBSb290IENBIFNlcmlhb' + - 'CA0NTcyMDA2MzEwIBcNMTQwODAxMDAwMDAwWhgPMjA1MDA5MDQwMDAwMDBaMG4xCzAJBgNVBAYTAlNFMRIwEAYDV' + - 'QQKDAlZdWJpY28gQUIxIjAgBgNVBAsMGUF1dGhlbnRpY2F0b3IgQXR0ZXN0YXRpb24xJzAlBgNVBAMMHll1Ymljb' + - 'yBVMkYgRUUgU2VyaWFsIDcxOTgwNzA3NTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABCoDhl5gQ9meEf8QqiVUV' + - '4S/Ca+Oax47MhcpIW9VEhqM2RDTmd3HaL3+SnvH49q8YubSRp/1Z1uP+okMynSGnj+jbDBqMCIGCSsGAQQBgsQKA' + - 'gQVMS4zLjYuMS40LjEuNDE0ODIuMS4xMBMGCysGAQQBguUcAgEBBAQDAgQwMCEGCysGAQQBguUcAQEEBBIEEG1Eu' + - 'pv27C5JuTAMj+kgy3MwDAYDVR0TAQH/BAIwADANBgkqhkiG9w0BAQsFAAOCAQEAclfQPNzD4RVphJDW+A75W1MHI' + - '3PZ5kcyYysR3Nx3iuxr1ZJtB+F7nFQweI3jL05HtFh2/4xVIgKb6Th4eVcjMecncBaCinEbOcdP1sEli9Hk2eVm1' + - 'XB5A0faUjXAPw/+QLFCjgXG6ReZ5HVUcWkB7riLsFeJNYitiKrTDXFPLy+sNtVNutcQnFsCerDKuM81TvEAigkIb' + - 'KCGlq8M/NvBg5j83wIxbCYiyV7mIr3RwApHieShzLdJo1S6XydgQjC+/64G5r8C+8AVvNFR3zXXCpio5C3KRIj88' + - 'HEEIYjf6h1fdLfqeIsq+cUUqbq5T+c4nNoZUZCysTB9v5EY4akp+GhhdXRoRGF0YVjEAbElFazplpnc037DORGDZ' + - 'NjDq86cN9vm6+APoAM20wtBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQGFYevaR71ptU5YtXOSnVzPQTsGgK+gLiBKnq' + - 'PWBmZXNRvjISqlLxiwApzlrfkTc3lEMYMatjeACCnsijOkNEGOlAQIDJiABIVggdWLG6UvGyHFw/k/bv6/k6z/LL' + - 'gSO5KXzXw2EcUxkEX8iWCBeaVLz/cbyoKvRIg/q+q7tan0VN+i3WR0BOBCcuNP7yw==', - clientDataJSON: - 'eyJjaGFsbGVuZ2UiOiJVMmQ0TjNZME0wOU1jbGRQYjFSNVpFeG5UbG95IiwiY2xpZW50' + - 'RXh0ZW5zaW9ucyI6e30sImhhc2hBbGdvcml0aG0iOiJTSEEtMjU2Iiwib3JpZ2luIjoiaHR0cHM6Ly9jbG92ZXIu' + - 'bWlsbGVydGltZS5kZXY6MzAwMCIsInR5cGUiOiJ3ZWJhdXRobi5jcmVhdGUifQ==', + attestationObject: 'o2NmbXRoZmlkby11MmZnYXR0U3RtdKJjc2lnWEcwRQIgRYUftNUmhT0VWTZmIgDmrOoP26Pcre-kL3DLnCrXbegCIQCOu_x5gqp-Rej76zeBuXlk8e7J-9WM_i-wZmCIbIgCGmN4NWOBWQLBMIICvTCCAaWgAwIBAgIEKudiYzANBgkqhkiG9w0BAQsFADAuMSwwKgYDVQQDEyNZdWJpY28gVTJGIFJvb3QgQ0EgU2VyaWFsIDQ1NzIwMDYzMTAgFw0xNDA4MDEwMDAwMDBaGA8yMDUwMDkwNDAwMDAwMFowbjELMAkGA1UEBhMCU0UxEjAQBgNVBAoMCVl1YmljbyBBQjEiMCAGA1UECwwZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEnMCUGA1UEAwweWXViaWNvIFUyRiBFRSBTZXJpYWwgNzE5ODA3MDc1MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEKgOGXmBD2Z4R_xCqJVRXhL8Jr45rHjsyFykhb1USGozZENOZ3cdovf5Ke8fj2rxi5tJGn_VnW4_6iQzKdIaeP6NsMGowIgYJKwYBBAGCxAoCBBUxLjMuNi4xLjQuMS40MTQ4Mi4xLjEwEwYLKwYBBAGC5RwCAQEEBAMCBDAwIQYLKwYBBAGC5RwBAQQEEgQQbUS6m_bsLkm5MAyP6SDLczAMBgNVHRMBAf8EAjAAMA0GCSqGSIb3DQEBCwUAA4IBAQByV9A83MPhFWmEkNb4DvlbUwcjc9nmRzJjKxHc3HeK7GvVkm0H4XucVDB4jeMvTke0WHb_jFUiApvpOHh5VyMx5ydwFoKKcRs5x0_WwSWL0eTZ5WbVcHkDR9pSNcA_D_5AsUKOBcbpF5nkdVRxaQHuuIuwV4k1iK2IqtMNcU8vL6w21U261xCcWwJ6sMq4zzVO8QCKCQhsoIaWrwz828GDmPzfAjFsJiLJXuYivdHACkeJ5KHMt0mjVLpfJ2BCML7_rgbmvwL7wBW80VHfNdcKmKjkLcpEiPzwcQQhiN_qHV90t-p4iyr5xRSpurlP5zic2hlRkLKxMH2_kRjhqSn4aGF1dGhEYXRhWMQ93EcQ6cCIsinbqJ1WMiC7Ofcimv9GWwplaxr7mor4oEEAAAAAAAAAAAAAAAAAAAAAAAAAAABAVHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUaUBAgMmIAEhWCDIkcsOaVKDIQYwq3EDQ-pST2kRwNH_l1nCgW-WcFpNXiJYIBSbummp-KO3qZeqmvZ_U_uirCDL2RNj3E5y4_KzefIr', + clientDataJSON: 'eyJjaGFsbGVuZ2UiOiJkRzkwWVd4c2VWVnVhWEYxWlZaaGJIVmxSWFpsY25sQmRIUmxjM1JoZEdsdmJnIiwiY2xpZW50RXh0ZW5zaW9ucyI6e30sImhhc2hBbGdvcml0aG0iOiJTSEEtMjU2Iiwib3JpZ2luIjoiaHR0cHM6Ly9kZXYuZG9udG5lZWRhLnB3IiwidHlwZSI6IndlYmF1dGhuLmNyZWF0ZSJ9' }, getClientExtensionResults: () => ({}), - type: 'webauthn.create', + type: 'public-key' }; -const attestationFIDOU2FChallenge = 'Sgx7v43OLrWOoTydLgNZ2'; +const attestationFIDOU2FChallenge = 'totallyUniqueValueEveryAttestation'; const attestationPacked = { id: '', -- cgit v1.2.3 From 70d28189d5a74827c70cd7016373206abc13b4bb Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 19:46:26 -0700 Subject: Move user presence check higher up --- .../src/attestation/verifications/verifyAndroidSafetyNet.ts | 11 +++-------- .../server/src/attestation/verifications/verifyFIDOU2F.ts | 4 ---- packages/server/src/attestation/verifications/verifyNone.ts | 4 ---- packages/server/src/attestation/verifications/verifyPacked.ts | 4 ---- packages/server/src/attestation/verifyAttestationResponse.ts | 5 +++++ 5 files changed, 8 insertions(+), 20 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts index a5dc89a..3d1397a 100644 --- a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts +++ b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts @@ -18,20 +18,15 @@ export default function verifyAttestationAndroidSafetyNet( ): VerifiedAttestation { const { attStmt, authData, fmt } = attestationObject; const authDataStruct = parseAuthenticatorData(authData); - const { counter, credentialID, COSEPublicKey, flags } = authDataStruct; + const { counter, credentialID, flags } = authDataStruct; - if (!flags.up) { - throw new Error('User was not present for attestation (None)'); + if (!credentialID) { + throw new Error('No credential ID was provided by authenticator (SafetyNet)'); } - if (!COSEPublicKey) { throw new Error('No public key was provided by authenticator (SafetyNet)'); } - if (!credentialID) { - throw new Error('No credential ID was provided by authenticator (SafetyNet)'); - } - if (!attStmt.response) { throw new Error('No response was included in attStmt by authenticator (SafetyNet)'); } diff --git a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts index 5842a3c..313bd25 100644 --- a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts +++ b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts @@ -21,10 +21,6 @@ export default function verifyAttestationFIDOU2F( const authDataStruct = parseAuthenticatorData(authData); const { flags, COSEPublicKey, rpIdHash, credentialID, counter } = authDataStruct; - if (!flags.up) { - throw new Error('User was NOT present during authentication (FIDOU2F)'); - } - if (!COSEPublicKey) { throw new Error('No public key was provided by authenticator (FIDOU2F)'); } diff --git a/packages/server/src/attestation/verifications/verifyNone.ts b/packages/server/src/attestation/verifications/verifyNone.ts index 66fd7da..b390471 100644 --- a/packages/server/src/attestation/verifications/verifyNone.ts +++ b/packages/server/src/attestation/verifications/verifyNone.ts @@ -19,10 +19,6 @@ export default function verifyAttestationNone( const { credentialID, COSEPublicKey, counter, flags } = authDataStruct; - if (!flags.up) { - throw new Error('User was not present for attestation (None)'); - } - if (!COSEPublicKey) { throw new Error('No public key was provided by authenticator (None)'); } diff --git a/packages/server/src/attestation/verifications/verifyPacked.ts b/packages/server/src/attestation/verifications/verifyPacked.ts index 16acdfd..00135ab 100644 --- a/packages/server/src/attestation/verifications/verifyPacked.ts +++ b/packages/server/src/attestation/verifications/verifyPacked.ts @@ -30,10 +30,6 @@ export default function verifyAttestationPacked( const { COSEPublicKey, counter, credentialID, flags } = authDataStruct; - if (!flags.up) { - throw new Error('User was not present for attestation (Packed)'); - } - if (!COSEPublicKey) { throw new Error('No public key was provided by authenticator (Packed)'); } diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 40a619f..909b12b 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -60,6 +60,11 @@ export default function verifyAttestationResponse( throw new Error(`Unexpected RP ID hash`); } + // Make sure someone was physically present + if (!flags.up) { + throw new Error('User not present during assertion'); + } + /** * Verification can only be performed when attestation = 'direct' -- cgit v1.2.3 From 739afe2cf3d85a8abe4944efbcf94a478e822089 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 19:47:21 -0700 Subject: Add support for three more key crypto algos --- .../src/attestation/generateAttestationOptions.ts | 19 +++++++++++++------ .../src/attestation/verifications/verifyPacked.ts | 5 +++++ 2 files changed, 18 insertions(+), 6 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/generateAttestationOptions.ts b/packages/server/src/attestation/generateAttestationOptions.ts index d142740..e845e84 100644 --- a/packages/server/src/attestation/generateAttestationOptions.ts +++ b/packages/server/src/attestation/generateAttestationOptions.ts @@ -18,6 +18,15 @@ type Options = { extensions?: AuthenticationExtensionsClientInputs, }; +// Supported crypto algo identifiers +// See https://w3c.github.io/webauthn/#sctn-alg-identifier +const supportedCOSEAlgorithIdentifiers = [ + -7, + -35, + -36, + -8 +]; + /** * Prepare a value to pass into navigator.credentials.create(...) for authenticator "registration" * @@ -67,12 +76,10 @@ export default function generateAttestationOptions( name: userName, displayName: userDisplayName, }, - pubKeyCredParams: [ - { - alg: -7, - type: 'public-key', - }, - ], + pubKeyCredParams: supportedCOSEAlgorithIdentifiers.map(id => ({ + alg: id, + type: 'public-key', + })), timeout, attestation: attestationType, excludeCredentials: excludedCredentialIDs.map((id) => ({ diff --git a/packages/server/src/attestation/verifications/verifyPacked.ts b/packages/server/src/attestation/verifications/verifyPacked.ts index 00135ab..20e27e1 100644 --- a/packages/server/src/attestation/verifications/verifyPacked.ts +++ b/packages/server/src/attestation/verifications/verifyPacked.ts @@ -190,9 +190,14 @@ const COSERSASCHEME: { [key: string]: SigningSchemeHash } = { }; const COSECRV: { [key: number]: string } = { + // alg: -7 1: 'p256', + // alg: -35 2: 'p384', + // alg: -36 3: 'p521', + // alg: -8 + 6: 'ed25519', }; const COSEALGHASH: { [key: string]: string } = { -- cgit v1.2.3 From 8c05800a045e18092c1a1428d4cafa4c9a12e234 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 19:48:01 -0700 Subject: Add note to COSECRV about origins --- packages/server/src/attestation/verifications/verifyPacked.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifications/verifyPacked.ts b/packages/server/src/attestation/verifications/verifyPacked.ts index 20e27e1..803d327 100644 --- a/packages/server/src/attestation/verifications/verifyPacked.ts +++ b/packages/server/src/attestation/verifications/verifyPacked.ts @@ -189,6 +189,7 @@ const COSERSASCHEME: { [key: string]: SigningSchemeHash } = { '-259': 'pkcs1-sha512', }; +// See https://w3c.github.io/webauthn/#sctn-alg-identifier const COSECRV: { [key: number]: string } = { // alg: -7 1: 'p256', -- cgit v1.2.3 From e5bb01fb2405d2899f155ccf108dc240956a9ed2 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 19:50:10 -0700 Subject: Refine type annotation for supported algo ID’s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/server/src/attestation/generateAttestationOptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/generateAttestationOptions.ts b/packages/server/src/attestation/generateAttestationOptions.ts index e845e84..b80f19d 100644 --- a/packages/server/src/attestation/generateAttestationOptions.ts +++ b/packages/server/src/attestation/generateAttestationOptions.ts @@ -20,7 +20,7 @@ type Options = { // Supported crypto algo identifiers // See https://w3c.github.io/webauthn/#sctn-alg-identifier -const supportedCOSEAlgorithIdentifiers = [ +export const supportedCOSEAlgorithIdentifiers: COSEAlgorithmIdentifier[] = [ -7, -35, -36, -- cgit v1.2.3 From 1fc984faa5222e054b22782471f6282704ad52a8 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 19:58:57 -0700 Subject: Verify public key algorithm is supported --- .../src/attestation/verifyAttestationResponse.ts | 19 +++++++++++++++++++ .../server/src/helpers/decodeCredentialPublicKey.ts | 7 +++++++ 2 files changed, 26 insertions(+) create mode 100644 packages/server/src/helpers/decodeCredentialPublicKey.ts (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 909b12b..f50b9a1 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -6,7 +6,10 @@ import decodeAttestationObject, { ATTESTATION_FORMATS } from '../helpers/decodeA import decodeClientDataJSON from '../helpers/decodeClientDataJSON'; import parseAuthenticatorData from '../helpers/parseAuthenticatorData'; import toHash from '../helpers/toHash'; +import decodeCredentialPublicKey from '../helpers/decodeCredentialPublicKey'; +import { COSEKEYS } from '../helpers/convertCOSEtoPKCS'; +import { supportedCOSEAlgorithIdentifiers } from './generateAttestationOptions'; import verifyFIDOU2F from './verifications/verifyFIDOU2F'; import verifyPacked from './verifications/verifyPacked'; import verifyNone from './verifications/verifyNone'; @@ -65,6 +68,22 @@ export default function verifyAttestationResponse( throw new Error('User not present during assertion'); } + if (!COSEPublicKey) { + throw new Error('No public key was provided by authenticator'); + } + + const decodedPublicKey = decodeCredentialPublicKey(COSEPublicKey); + const alg = decodedPublicKey.get(COSEKEYS.alg); + + if (!alg) { + throw new Error('Credential public key was missing alg'); + } + + // Make sure the key algorithm is one we specified within the attestation options + if (!supportedCOSEAlgorithIdentifiers.includes(alg as number)) { + const supported = supportedCOSEAlgorithIdentifiers.join(', '); + throw new Error(`Unexpected public key alg "${alg}", expected one of "${supported}"`); + } /** * Verification can only be performed when attestation = 'direct' diff --git a/packages/server/src/helpers/decodeCredentialPublicKey.ts b/packages/server/src/helpers/decodeCredentialPublicKey.ts new file mode 100644 index 0000000..a856a72 --- /dev/null +++ b/packages/server/src/helpers/decodeCredentialPublicKey.ts @@ -0,0 +1,7 @@ +import cbor from 'cbor'; + +import { COSEPublicKey } from './convertCOSEtoPKCS'; + +export default function decodeCredentialPublicKey(publicKey: Buffer): COSEPublicKey { + return cbor.decodeFirstSync(publicKey); +} -- cgit v1.2.3 From 20daeb22d277f8c3b5a19f3e9cb0db88ae11b8f6 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 20:10:23 -0700 Subject: Pass parsed authData into verification methods --- .../verifications/verifyAndroidSafetyNet.ts | 7 +++--- .../src/attestation/verifications/verifyFIDOU2F.ts | 9 ++++---- .../src/attestation/verifications/verifyNone.ts | 7 +++--- .../src/attestation/verifications/verifyPacked.ts | 8 +++---- .../src/attestation/verifyAttestationResponse.ts | 26 +++++++++++++++++----- .../server/src/helpers/parseAuthenticatorData.ts | 2 +- 6 files changed, 36 insertions(+), 23 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts index 3d1397a..9ef6bf8 100644 --- a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts +++ b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts @@ -1,13 +1,13 @@ import base64url from 'base64url'; import type { AttestationObject } from '../../helpers/decodeAttestationObject'; +import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; import type { VerifiedAttestation } from '../verifyAttestationResponse'; import toHash from '../../helpers/toHash'; import verifySignature from '../../helpers/verifySignature'; import convertCOSEtoPKCS from '../../helpers/convertCOSEtoPKCS'; import getCertificateInfo from '../../helpers/getCertificateInfo'; -import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; /** * Verify an attestation response with fmt 'android-safetynet' @@ -15,10 +15,11 @@ import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; export default function verifyAttestationAndroidSafetyNet( attestationObject: AttestationObject, base64ClientDataJSON: string, + parsedAuthData: ParsedAuthenticatorData, + COSEPublicKey: Buffer, ): VerifiedAttestation { const { attStmt, authData, fmt } = attestationObject; - const authDataStruct = parseAuthenticatorData(authData); - const { counter, credentialID, flags } = authDataStruct; + const { counter, credentialID, flags } = parsedAuthData; if (!credentialID) { throw new Error('No credential ID was provided by authenticator (SafetyNet)'); diff --git a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts index 313bd25..335c239 100644 --- a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts +++ b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts @@ -1,13 +1,13 @@ import base64url from 'base64url'; import type { AttestationObject } from '../../helpers/decodeAttestationObject'; +import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; import type { VerifiedAttestation } from '../verifyAttestationResponse'; import toHash from '../../helpers/toHash'; import convertCOSEtoPKCS from '../../helpers/convertCOSEtoPKCS'; import convertASN1toPEM from '../../helpers/convertASN1toPEM'; import verifySignature from '../../helpers/verifySignature'; -import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; /** * Verify an attestation response with fmt 'fido-u2f' @@ -15,11 +15,10 @@ import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; export default function verifyAttestationFIDOU2F( attestationObject: AttestationObject, base64ClientDataJSON: string, + parsedAuthData: ParsedAuthenticatorData, ): VerifiedAttestation { - const { fmt, authData, attStmt } = attestationObject; - - const authDataStruct = parseAuthenticatorData(authData); - const { flags, COSEPublicKey, rpIdHash, credentialID, counter } = authDataStruct; + const { fmt, attStmt } = attestationObject; + const { flags, COSEPublicKey, rpIdHash, credentialID, counter } = parsedAuthData; if (!COSEPublicKey) { throw new Error('No public key was provided by authenticator (FIDOU2F)'); diff --git a/packages/server/src/attestation/verifications/verifyNone.ts b/packages/server/src/attestation/verifications/verifyNone.ts index b390471..4ac1988 100644 --- a/packages/server/src/attestation/verifications/verifyNone.ts +++ b/packages/server/src/attestation/verifications/verifyNone.ts @@ -1,10 +1,10 @@ import base64url from 'base64url'; import type { AttestationObject } from '../../helpers/decodeAttestationObject'; +import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; import type { VerifiedAttestation } from '../verifyAttestationResponse'; import convertCOSEtoPKCS from '../../helpers/convertCOSEtoPKCS'; -import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; /** * Verify an attestation response with fmt 'none' @@ -13,11 +13,10 @@ import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; */ export default function verifyAttestationNone( attestationObject: AttestationObject, + parsedAuthData: ParsedAuthenticatorData, ): VerifiedAttestation { const { fmt, authData } = attestationObject; - const authDataStruct = parseAuthenticatorData(authData); - - const { credentialID, COSEPublicKey, counter, flags } = authDataStruct; + const { credentialID, COSEPublicKey, counter, flags } = parsedAuthData; if (!COSEPublicKey) { throw new Error('No public key was provided by authenticator (None)'); diff --git a/packages/server/src/attestation/verifications/verifyPacked.ts b/packages/server/src/attestation/verifications/verifyPacked.ts index 803d327..48764aa 100644 --- a/packages/server/src/attestation/verifications/verifyPacked.ts +++ b/packages/server/src/attestation/verifications/verifyPacked.ts @@ -4,6 +4,7 @@ import elliptic from 'elliptic'; import NodeRSA, { SigningSchemeHash } from 'node-rsa'; import type { AttestationObject } from '../../helpers/decodeAttestationObject'; +import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; import type { VerifiedAttestation } from '../verifyAttestationResponse'; import convertCOSEtoPKCS, { @@ -14,7 +15,6 @@ import toHash from '../../helpers/toHash'; import convertASN1toPEM from '../../helpers/convertASN1toPEM'; import getCertificateInfo from '../../helpers/getCertificateInfo'; import verifySignature from '../../helpers/verifySignature'; -import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; /** * Verify an attestation response with fmt 'packed' @@ -22,13 +22,11 @@ import parseAuthenticatorData from '../../helpers/parseAuthenticatorData'; export default function verifyAttestationPacked( attestationObject: AttestationObject, base64ClientDataJSON: string, + parsedAuthData: ParsedAuthenticatorData, ): VerifiedAttestation { const { fmt, authData, attStmt } = attestationObject; const { sig, x5c } = attStmt; - - const authDataStruct = parseAuthenticatorData(authData); - - const { COSEPublicKey, counter, credentialID, flags } = authDataStruct; + const { COSEPublicKey, counter, credentialID, flags } = parsedAuthData; if (!COSEPublicKey) { throw new Error('No public key was provided by authenticator (Packed)'); diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index f50b9a1..6b54d8a 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -52,7 +52,7 @@ export default function verifyAttestationResponse( } const attestationObject = decodeAttestationObject(response.attestationObject); - const { fmt, authData, attStmt } = attestationObject; + const { fmt, authData } = attestationObject; const parsedAuthData = parseAuthenticatorData(authData); const { rpIdHash, flags, COSEPublicKey } = parsedAuthData; @@ -89,19 +89,35 @@ export default function verifyAttestationResponse( * Verification can only be performed when attestation = 'direct' */ if (fmt === ATTESTATION_FORMATS.FIDO_U2F) { - return verifyFIDOU2F(attestationObject, response.clientDataJSON); + return verifyFIDOU2F( + attestationObject, + response.clientDataJSON, + parsedAuthData, + ); } if (fmt === ATTESTATION_FORMATS.PACKED) { - return verifyPacked(attestationObject, response.clientDataJSON); + return verifyPacked( + attestationObject, + response.clientDataJSON, + parsedAuthData, + ); } if (fmt === ATTESTATION_FORMATS.ANDROID_SAFETYNET) { - return verifyAndroidSafetynet(attestationObject, response.clientDataJSON); + return verifyAndroidSafetynet( + attestationObject, + response.clientDataJSON, + parsedAuthData, + COSEPublicKey, + ); } if (fmt === ATTESTATION_FORMATS.NONE) { - return verifyNone(attestationObject); + return verifyNone( + attestationObject, + parsedAuthData, + ); } throw new Error(`Unsupported Attestation Format: ${fmt}`); diff --git a/packages/server/src/helpers/parseAuthenticatorData.ts b/packages/server/src/helpers/parseAuthenticatorData.ts index 3177dd5..510c228 100644 --- a/packages/server/src/helpers/parseAuthenticatorData.ts +++ b/packages/server/src/helpers/parseAuthenticatorData.ts @@ -56,7 +56,7 @@ export default function parseAuthenticatorData(authData: Buffer): ParsedAuthenti }; } -type ParsedAuthenticatorData = { +export type ParsedAuthenticatorData = { rpIdHash: Buffer; flagsBuf: Buffer; flags: { -- cgit v1.2.3 From 553bd0d9f680942cef668e6a57d9c4a8fcab269b Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 20:20:48 -0700 Subject: Update authData parser to map returns to spec --- .../src/attestation/verifications/verifyAndroidSafetyNet.ts | 7 ++----- .../server/src/attestation/verifications/verifyFIDOU2F.ts | 6 +++--- packages/server/src/attestation/verifications/verifyNone.ts | 8 ++++---- .../server/src/attestation/verifications/verifyPacked.ts | 13 ++++++------- .../server/src/attestation/verifyAttestationResponse.ts | 8 ++++---- packages/server/src/helpers/parseAuthenticatorData.ts | 8 ++++---- 6 files changed, 23 insertions(+), 27 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts index 9ef6bf8..31aa53d 100644 --- a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts +++ b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts @@ -16,7 +16,7 @@ export default function verifyAttestationAndroidSafetyNet( attestationObject: AttestationObject, base64ClientDataJSON: string, parsedAuthData: ParsedAuthenticatorData, - COSEPublicKey: Buffer, + credentialPublicKey: Buffer, ): VerifiedAttestation { const { attStmt, authData, fmt } = attestationObject; const { counter, credentialID, flags } = parsedAuthData; @@ -24,9 +24,6 @@ export default function verifyAttestationAndroidSafetyNet( if (!credentialID) { throw new Error('No credential ID was provided by authenticator (SafetyNet)'); } - if (!COSEPublicKey) { - throw new Error('No public key was provided by authenticator (SafetyNet)'); - } if (!attStmt.response) { throw new Error('No response was included in attStmt by authenticator (SafetyNet)'); @@ -109,7 +106,7 @@ export default function verifyAttestationAndroidSafetyNet( if (toReturn.verified) { toReturn.userVerified = flags.uv; - const publicKey = convertCOSEtoPKCS(COSEPublicKey); + const publicKey = convertCOSEtoPKCS(credentialPublicKey); toReturn.authenticatorInfo = { fmt, diff --git a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts index 335c239..508f167 100644 --- a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts +++ b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts @@ -18,9 +18,9 @@ export default function verifyAttestationFIDOU2F( parsedAuthData: ParsedAuthenticatorData, ): VerifiedAttestation { const { fmt, attStmt } = attestationObject; - const { flags, COSEPublicKey, rpIdHash, credentialID, counter } = parsedAuthData; + const { flags, credentialPublicKey, rpIdHash, credentialID, counter } = parsedAuthData; - if (!COSEPublicKey) { + if (!credentialPublicKey) { throw new Error('No public key was provided by authenticator (FIDOU2F)'); } @@ -30,7 +30,7 @@ export default function verifyAttestationFIDOU2F( const clientDataHash = toHash(base64url.toBuffer(base64ClientDataJSON)); const reservedByte = Buffer.from([0x00]); - const publicKey = convertCOSEtoPKCS(COSEPublicKey); + const publicKey = convertCOSEtoPKCS(credentialPublicKey); const signatureBase = Buffer.concat([ reservedByte, diff --git a/packages/server/src/attestation/verifications/verifyNone.ts b/packages/server/src/attestation/verifications/verifyNone.ts index 4ac1988..f276a83 100644 --- a/packages/server/src/attestation/verifications/verifyNone.ts +++ b/packages/server/src/attestation/verifications/verifyNone.ts @@ -15,10 +15,10 @@ export default function verifyAttestationNone( attestationObject: AttestationObject, parsedAuthData: ParsedAuthenticatorData, ): VerifiedAttestation { - const { fmt, authData } = attestationObject; - const { credentialID, COSEPublicKey, counter, flags } = parsedAuthData; + const { fmt } = attestationObject; + const { credentialID, credentialPublicKey, counter, flags } = parsedAuthData; - if (!COSEPublicKey) { + if (!credentialPublicKey) { throw new Error('No public key was provided by authenticator (None)'); } @@ -26,7 +26,7 @@ export default function verifyAttestationNone( throw new Error('No credential ID was provided by authenticator (None)'); } - const publicKey = convertCOSEtoPKCS(COSEPublicKey); + const publicKey = convertCOSEtoPKCS(credentialPublicKey); const toReturn: VerifiedAttestation = { verified: true, diff --git a/packages/server/src/attestation/verifications/verifyPacked.ts b/packages/server/src/attestation/verifications/verifyPacked.ts index 48764aa..c5f8ec1 100644 --- a/packages/server/src/attestation/verifications/verifyPacked.ts +++ b/packages/server/src/attestation/verifications/verifyPacked.ts @@ -1,5 +1,4 @@ import base64url from 'base64url'; -import cbor from 'cbor'; import elliptic from 'elliptic'; import NodeRSA, { SigningSchemeHash } from 'node-rsa'; @@ -9,12 +8,12 @@ import type { VerifiedAttestation } from '../verifyAttestationResponse'; import convertCOSEtoPKCS, { COSEKEYS, - COSEPublicKey as COSEPublicKeyType } from '../../helpers/convertCOSEtoPKCS'; import toHash from '../../helpers/toHash'; import convertASN1toPEM from '../../helpers/convertASN1toPEM'; import getCertificateInfo from '../../helpers/getCertificateInfo'; import verifySignature from '../../helpers/verifySignature'; +import decodeCredentialPublicKey from '../../helpers/decodeCredentialPublicKey'; /** * Verify an attestation response with fmt 'packed' @@ -26,9 +25,9 @@ export default function verifyAttestationPacked( ): VerifiedAttestation { const { fmt, authData, attStmt } = attestationObject; const { sig, x5c } = attStmt; - const { COSEPublicKey, counter, credentialID, flags } = parsedAuthData; + const { credentialPublicKey, counter, credentialID, flags } = parsedAuthData; - if (!COSEPublicKey) { + if (!credentialPublicKey) { throw new Error('No public key was provided by authenticator (Packed)'); } @@ -48,7 +47,7 @@ export default function verifyAttestationPacked( verified: false, userVerified: flags.uv, }; - const publicKey = convertCOSEtoPKCS(COSEPublicKey); + const publicKey = convertCOSEtoPKCS(credentialPublicKey); if (x5c) { const leafCert = convertASN1toPEM(x5c[0]); @@ -83,7 +82,7 @@ export default function verifyAttestationPacked( toReturn.verified = verifySignature(sig, signatureBase, leafCert); } else { - const cosePublicKey: COSEPublicKeyType = cbor.decodeAllSync(COSEPublicKey)[0]; + const cosePublicKey = decodeCredentialPublicKey(credentialPublicKey); const kty = cosePublicKey.get(COSEKEYS.kty); const alg = cosePublicKey.get(COSEKEYS.alg); @@ -105,7 +104,7 @@ export default function verifyAttestationPacked( throw new Error('COSE public key was missing kty crv (Packed|EC2)'); } - const pkcsPublicKey = convertCOSEtoPKCS(COSEPublicKey); + const pkcsPublicKey = convertCOSEtoPKCS(credentialPublicKey); const signatureBaseHash = toHash(signatureBase, hashAlg); /** diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 6b54d8a..96f659a 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -55,7 +55,7 @@ export default function verifyAttestationResponse( const { fmt, authData } = attestationObject; const parsedAuthData = parseAuthenticatorData(authData); - const { rpIdHash, flags, COSEPublicKey } = parsedAuthData; + const { rpIdHash, flags, credentialPublicKey } = parsedAuthData; // Make sure the response's RP ID is ours const expectedRPIDHash = toHash(Buffer.from(expectedRPID, 'ascii')); @@ -68,11 +68,11 @@ export default function verifyAttestationResponse( throw new Error('User not present during assertion'); } - if (!COSEPublicKey) { + if (!credentialPublicKey) { throw new Error('No public key was provided by authenticator'); } - const decodedPublicKey = decodeCredentialPublicKey(COSEPublicKey); + const decodedPublicKey = decodeCredentialPublicKey(credentialPublicKey); const alg = decodedPublicKey.get(COSEKEYS.alg); if (!alg) { @@ -109,7 +109,7 @@ export default function verifyAttestationResponse( attestationObject, response.clientDataJSON, parsedAuthData, - COSEPublicKey, + credentialPublicKey, ); } diff --git a/packages/server/src/helpers/parseAuthenticatorData.ts b/packages/server/src/helpers/parseAuthenticatorData.ts index 510c228..e177002 100644 --- a/packages/server/src/helpers/parseAuthenticatorData.ts +++ b/packages/server/src/helpers/parseAuthenticatorData.ts @@ -27,7 +27,7 @@ export default function parseAuthenticatorData(authData: Buffer): ParsedAuthenti let aaguid: Buffer | undefined = undefined; let credentialID: Buffer | undefined = undefined; - let COSEPublicKey: Buffer | undefined = undefined; + let credentialPublicKey: Buffer | undefined = undefined; if (flags.at) { aaguid = intBuffer.slice(0, 16); @@ -41,7 +41,7 @@ export default function parseAuthenticatorData(authData: Buffer): ParsedAuthenti credentialID = intBuffer.slice(0, credIDLen); intBuffer = intBuffer.slice(credIDLen); - COSEPublicKey = intBuffer; + credentialPublicKey = intBuffer; } return { @@ -52,7 +52,7 @@ export default function parseAuthenticatorData(authData: Buffer): ParsedAuthenti counterBuf, aaguid, credentialID, - COSEPublicKey, + credentialPublicKey, }; } @@ -70,5 +70,5 @@ export type ParsedAuthenticatorData = { counterBuf: Buffer; aaguid?: Buffer; credentialID?: Buffer; - COSEPublicKey?: Buffer; + credentialPublicKey?: Buffer; }; -- cgit v1.2.3 From 1369b9e3f16dfffe131c0a46030ccbab7b024801 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 23:17:05 -0700 Subject: Split up some types in decodeAttestationObject --- packages/server/src/helpers/decodeAttestationObject.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/helpers/decodeAttestationObject.ts b/packages/server/src/helpers/decodeAttestationObject.ts index 2eb9997..e5accdd 100644 --- a/packages/server/src/helpers/decodeAttestationObject.ts +++ b/packages/server/src/helpers/decodeAttestationObject.ts @@ -23,10 +23,12 @@ export enum ATTESTATION_FORMATS { export type AttestationObject = { fmt: ATTESTATION_FORMATS; - attStmt: { - sig?: Buffer; - x5c?: Buffer[]; - response?: Buffer; - }; + attStmt: AttestationStatement; authData: Buffer; }; + +export type AttestationStatement = { + sig?: Buffer; + x5c?: Buffer[]; + response?: Buffer; +}; -- cgit v1.2.3 From 9fa9f70abed26d414bc7162a601901939398e421 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 23:31:07 -0700 Subject: Simplify verification methods --- .../verifications/verifyAndroidSafetyNet.ts | 46 +++--------- .../src/attestation/verifications/verifyFIDOU2F.ts | 49 +++---------- .../src/attestation/verifications/verifyNone.ts | 43 ----------- .../src/attestation/verifications/verifyPacked.ts | 63 +++++----------- .../src/attestation/verifyAttestationResponse.ts | 85 +++++++++++++--------- 5 files changed, 94 insertions(+), 192 deletions(-) delete mode 100644 packages/server/src/attestation/verifications/verifyNone.ts (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts index 31aa53d..9e0c080 100644 --- a/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts +++ b/packages/server/src/attestation/verifications/verifyAndroidSafetyNet.ts @@ -1,29 +1,22 @@ import base64url from 'base64url'; -import type { AttestationObject } from '../../helpers/decodeAttestationObject'; -import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; -import type { VerifiedAttestation } from '../verifyAttestationResponse'; +import type { AttestationStatement } from '../../helpers/decodeAttestationObject'; import toHash from '../../helpers/toHash'; import verifySignature from '../../helpers/verifySignature'; -import convertCOSEtoPKCS from '../../helpers/convertCOSEtoPKCS'; import getCertificateInfo from '../../helpers/getCertificateInfo'; +type Options = { + attStmt: AttestationStatement; + clientDataHash: Buffer; + authData: Buffer; +}; + /** * Verify an attestation response with fmt 'android-safetynet' */ -export default function verifyAttestationAndroidSafetyNet( - attestationObject: AttestationObject, - base64ClientDataJSON: string, - parsedAuthData: ParsedAuthenticatorData, - credentialPublicKey: Buffer, -): VerifiedAttestation { - const { attStmt, authData, fmt } = attestationObject; - const { counter, credentialID, flags } = parsedAuthData; - - if (!credentialID) { - throw new Error('No credential ID was provided by authenticator (SafetyNet)'); - } +export default function verifyAttestationAndroidSafetyNet(options: Options): boolean { + const { attStmt, clientDataHash, authData } = options; if (!attStmt.response) { throw new Error('No response was included in attStmt by authenticator (SafetyNet)'); @@ -41,7 +34,6 @@ export default function verifyAttestationAndroidSafetyNet( * START Verify PAYLOAD */ const { nonce, ctsProfileMatch } = PAYLOAD; - const clientDataHash = toHash(base64url.toBuffer(base64ClientDataJSON)); const nonceBase = Buffer.concat([authData, clientDataHash]); const nonceBuffer = toHash(nonceBase); @@ -95,28 +87,12 @@ export default function verifyAttestationAndroidSafetyNet( const signatureBaseBuffer = Buffer.from(`${jwtParts[0]}.${jwtParts[1]}`); const signatureBuffer = base64url.toBuffer(SIGNATURE); - const toReturn: VerifiedAttestation = { - verified: verifySignature(signatureBuffer, signatureBaseBuffer, certificate), - userVerified: false, - }; + const verified = verifySignature(signatureBuffer, signatureBaseBuffer, certificate); /** * END Verify Signature */ - if (toReturn.verified) { - toReturn.userVerified = flags.uv; - - const publicKey = convertCOSEtoPKCS(credentialPublicKey); - - toReturn.authenticatorInfo = { - fmt, - counter, - base64PublicKey: base64url.encode(publicKey), - base64CredentialID: base64url.encode(credentialID), - }; - } - - return toReturn; + return verified; } /** diff --git a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts index 508f167..0fd2e74 100644 --- a/packages/server/src/attestation/verifications/verifyFIDOU2F.ts +++ b/packages/server/src/attestation/verifications/verifyFIDOU2F.ts @@ -1,34 +1,23 @@ -import base64url from 'base64url'; +import type { AttestationStatement } from '../../helpers/decodeAttestationObject'; -import type { AttestationObject } from '../../helpers/decodeAttestationObject'; -import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; -import type { VerifiedAttestation } from '../verifyAttestationResponse'; - -import toHash from '../../helpers/toHash'; import convertCOSEtoPKCS from '../../helpers/convertCOSEtoPKCS'; import convertASN1toPEM from '../../helpers/convertASN1toPEM'; import verifySignature from '../../helpers/verifySignature'; +type Options = { + attStmt: AttestationStatement; + clientDataHash: Buffer; + rpIdHash: Buffer; + credentialID: Buffer; + credentialPublicKey: Buffer; +}; + /** * Verify an attestation response with fmt 'fido-u2f' */ -export default function verifyAttestationFIDOU2F( - attestationObject: AttestationObject, - base64ClientDataJSON: string, - parsedAuthData: ParsedAuthenticatorData, -): VerifiedAttestation { - const { fmt, attStmt } = attestationObject; - const { flags, credentialPublicKey, rpIdHash, credentialID, counter } = parsedAuthData; +export default function verifyAttestationFIDOU2F(options: Options): boolean { + const { attStmt, clientDataHash, rpIdHash, credentialID, credentialPublicKey } = options; - if (!credentialPublicKey) { - throw new Error('No public key was provided by authenticator (FIDOU2F)'); - } - - if (!credentialID) { - throw new Error('No credential ID was provided by authenticator (FIDOU2F)'); - } - - const clientDataHash = toHash(base64url.toBuffer(base64ClientDataJSON)); const reservedByte = Buffer.from([0x00]); const publicKey = convertCOSEtoPKCS(credentialPublicKey); @@ -52,19 +41,5 @@ export default function verifyAttestationFIDOU2F( const publicKeyCertPEM = convertASN1toPEM(x5c[0]); - const toReturn: VerifiedAttestation = { - verified: verifySignature(sig, signatureBase, publicKeyCertPEM), - userVerified: flags.uv, - }; - - if (toReturn.verified) { - toReturn.authenticatorInfo = { - fmt, - counter, - base64PublicKey: base64url.encode(publicKey), - base64CredentialID: base64url.encode(credentialID), - }; - } - - return toReturn; + return verifySignature(sig, signatureBase, publicKeyCertPEM); } diff --git a/packages/server/src/attestation/verifications/verifyNone.ts b/packages/server/src/attestation/verifications/verifyNone.ts deleted file mode 100644 index f276a83..0000000 --- a/packages/server/src/attestation/verifications/verifyNone.ts +++ /dev/null @@ -1,43 +0,0 @@ -import base64url from 'base64url'; - -import type { AttestationObject } from '../../helpers/decodeAttestationObject'; -import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; -import type { VerifiedAttestation } from '../verifyAttestationResponse'; - -import convertCOSEtoPKCS from '../../helpers/convertCOSEtoPKCS'; - -/** - * Verify an attestation response with fmt 'none' - * - * This is the weaker of the attestations, so there are only so many checks we can perform - */ -export default function verifyAttestationNone( - attestationObject: AttestationObject, - parsedAuthData: ParsedAuthenticatorData, -): VerifiedAttestation { - const { fmt } = attestationObject; - const { credentialID, credentialPublicKey, counter, flags } = parsedAuthData; - - if (!credentialPublicKey) { - throw new Error('No public key was provided by authenticator (None)'); - } - - if (!credentialID) { - throw new Error('No credential ID was provided by authenticator (None)'); - } - - const publicKey = convertCOSEtoPKCS(credentialPublicKey); - - const toReturn: VerifiedAttestation = { - verified: true, - userVerified: flags.uv, - authenticatorInfo: { - fmt, - counter, - base64PublicKey: base64url.encode(publicKey), - base64CredentialID: base64url.encode(credentialID), - }, - }; - - return toReturn; -} diff --git a/packages/server/src/attestation/verifications/verifyPacked.ts b/packages/server/src/attestation/verifications/verifyPacked.ts index c5f8ec1..45bd57e 100644 --- a/packages/server/src/attestation/verifications/verifyPacked.ts +++ b/packages/server/src/attestation/verifications/verifyPacked.ts @@ -1,53 +1,38 @@ -import base64url from 'base64url'; import elliptic from 'elliptic'; import NodeRSA, { SigningSchemeHash } from 'node-rsa'; -import type { AttestationObject } from '../../helpers/decodeAttestationObject'; -import type { ParsedAuthenticatorData } from '../../helpers/parseAuthenticatorData'; -import type { VerifiedAttestation } from '../verifyAttestationResponse'; +import type { AttestationStatement } from '../../helpers/decodeAttestationObject'; -import convertCOSEtoPKCS, { - COSEKEYS, -} from '../../helpers/convertCOSEtoPKCS'; +import convertCOSEtoPKCS, { COSEKEYS } from '../../helpers/convertCOSEtoPKCS'; import toHash from '../../helpers/toHash'; import convertASN1toPEM from '../../helpers/convertASN1toPEM'; import getCertificateInfo from '../../helpers/getCertificateInfo'; import verifySignature from '../../helpers/verifySignature'; import decodeCredentialPublicKey from '../../helpers/decodeCredentialPublicKey'; +type Options = { + attStmt: AttestationStatement; + clientDataHash: Buffer; + authData: Buffer; + credentialPublicKey: Buffer; +}; + /** * Verify an attestation response with fmt 'packed' */ -export default function verifyAttestationPacked( - attestationObject: AttestationObject, - base64ClientDataJSON: string, - parsedAuthData: ParsedAuthenticatorData, -): VerifiedAttestation { - const { fmt, authData, attStmt } = attestationObject; - const { sig, x5c } = attStmt; - const { credentialPublicKey, counter, credentialID, flags } = parsedAuthData; - - if (!credentialPublicKey) { - throw new Error('No public key was provided by authenticator (Packed)'); - } +export default function verifyAttestationPacked(options: Options): boolean { + const { attStmt, clientDataHash, authData, credentialPublicKey } = options; - if (!credentialID) { - throw new Error('No credential ID was provided by authenticator (Packed)'); - } + const { sig, x5c } = attStmt; if (!sig) { throw new Error('No attestation signature provided in attestation statement (Packed)'); } - const clientDataHash = toHash(base64url.toBuffer(base64ClientDataJSON)); - const signatureBase = Buffer.concat([authData, clientDataHash]); - const toReturn: VerifiedAttestation = { - verified: false, - userVerified: flags.uv, - }; - const publicKey = convertCOSEtoPKCS(credentialPublicKey); + let verified = false; + const pkcsPublicKey = convertCOSEtoPKCS(credentialPublicKey); if (x5c) { const leafCert = convertASN1toPEM(x5c[0]); @@ -80,7 +65,7 @@ export default function verifyAttestationPacked( throw new Error('Batch certificate version was not `3` (ASN.1 value of 2) (Packed|Full'); } - toReturn.verified = verifySignature(sig, signatureBase, leafCert); + verified = verifySignature(sig, signatureBase, leafCert); } else { const cosePublicKey = decodeCredentialPublicKey(credentialPublicKey); @@ -104,7 +89,6 @@ export default function verifyAttestationPacked( throw new Error('COSE public key was missing kty crv (Packed|EC2)'); } - const pkcsPublicKey = convertCOSEtoPKCS(credentialPublicKey); const signatureBaseHash = toHash(signatureBase, hashAlg); /** @@ -119,7 +103,7 @@ export default function verifyAttestationPacked( const ec = new elliptic.ec(COSECRV[crv as number]); const key = ec.keyFromPublic(pkcsPublicKey); - toReturn.verified = key.verify(signatureBaseHash, sig); + verified = key.verify(signatureBaseHash, sig); } else if (kty === COSEKTY.RSA) { const n = cosePublicKey.get(COSEKEYS.n); @@ -140,7 +124,7 @@ export default function verifyAttestationPacked( 'components-public', ); - toReturn.verified = key.verify(signatureBase, sig); + verified = key.verify(signatureBase, sig); } else if (kty === COSEKTY.OKP) { const x = cosePublicKey.get(COSEKEYS.x); @@ -154,20 +138,11 @@ export default function verifyAttestationPacked( key.keyFromPublic(x as Buffer); // TODO: is `publicKey` right here? - toReturn.verified = key.verify(signatureBaseHash, sig, publicKey); + verified = key.verify(signatureBaseHash, sig, pkcsPublicKey); } } - if (toReturn.verified) { - toReturn.authenticatorInfo = { - fmt, - counter, - base64PublicKey: base64url.encode(publicKey), - base64CredentialID: base64url.encode(credentialID), - }; - } - - return toReturn; + return verified; } enum COSEKTY { diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 96f659a..309c865 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -1,18 +1,16 @@ -import { - AttestationCredentialJSON, -} from '@simplewebauthn/typescript-types'; +import base64url from 'base64url'; +import { AttestationCredentialJSON } from '@simplewebauthn/typescript-types'; import decodeAttestationObject, { ATTESTATION_FORMATS } from '../helpers/decodeAttestationObject'; import decodeClientDataJSON from '../helpers/decodeClientDataJSON'; import parseAuthenticatorData from '../helpers/parseAuthenticatorData'; import toHash from '../helpers/toHash'; import decodeCredentialPublicKey from '../helpers/decodeCredentialPublicKey'; -import { COSEKEYS } from '../helpers/convertCOSEtoPKCS'; +import convertCOSEtoPKCS, { COSEKEYS } from '../helpers/convertCOSEtoPKCS'; import { supportedCOSEAlgorithIdentifiers } from './generateAttestationOptions'; import verifyFIDOU2F from './verifications/verifyFIDOU2F'; import verifyPacked from './verifications/verifyPacked'; -import verifyNone from './verifications/verifyNone'; import verifyAndroidSafetynet from './verifications/verifyAndroidSafetyNet'; /** @@ -52,10 +50,10 @@ export default function verifyAttestationResponse( } const attestationObject = decodeAttestationObject(response.attestationObject); - const { fmt, authData } = attestationObject; + const { fmt, authData, attStmt } = attestationObject; const parsedAuthData = parseAuthenticatorData(authData); - const { rpIdHash, flags, credentialPublicKey } = parsedAuthData; + const { rpIdHash, flags, credentialID, counter, credentialPublicKey } = parsedAuthData; // Make sure the response's RP ID is ours const expectedRPIDHash = toHash(Buffer.from(expectedRPID, 'ascii')); @@ -68,6 +66,10 @@ export default function verifyAttestationResponse( throw new Error('User not present during assertion'); } + if (!credentialID) { + throw new Error('No credential ID was provided by authenticator'); + } + if (!credentialPublicKey) { throw new Error('No public key was provided by authenticator'); } @@ -85,42 +87,59 @@ export default function verifyAttestationResponse( throw new Error(`Unexpected public key alg "${alg}", expected one of "${supported}"`); } + const clientDataHash = toHash(base64url.toBuffer(response.clientDataJSON)); + /** * Verification can only be performed when attestation = 'direct' */ + let verified = false; if (fmt === ATTESTATION_FORMATS.FIDO_U2F) { - return verifyFIDOU2F( - attestationObject, - response.clientDataJSON, - parsedAuthData, - ); + verified = verifyFIDOU2F({ + attStmt, + clientDataHash, + credentialID, + credentialPublicKey, + rpIdHash, + }); + } else if (fmt === ATTESTATION_FORMATS.PACKED) { + verified = verifyPacked({ + attStmt, + authData, + clientDataHash, + credentialPublicKey, + }); + } else if (fmt === ATTESTATION_FORMATS.ANDROID_SAFETYNET) { + verified = verifyAndroidSafetynet({ + attStmt, + authData, + clientDataHash, + }); + } else if (fmt === ATTESTATION_FORMATS.NONE) { + // This is the weaker of the attestations, so there's nothing else to really check + verified = true; + } else { + throw new Error(`Unsupported Attestation Format: ${fmt}`); } - if (fmt === ATTESTATION_FORMATS.PACKED) { - return verifyPacked( - attestationObject, - response.clientDataJSON, - parsedAuthData, - ); - } + const toReturn: VerifiedAttestation = { + verified, + userVerified: flags.uv, + }; - if (fmt === ATTESTATION_FORMATS.ANDROID_SAFETYNET) { - return verifyAndroidSafetynet( - attestationObject, - response.clientDataJSON, - parsedAuthData, - credentialPublicKey, - ); - } + if (toReturn.verified) { + toReturn.userVerified = flags.uv; - if (fmt === ATTESTATION_FORMATS.NONE) { - return verifyNone( - attestationObject, - parsedAuthData, - ); + const publicKey = convertCOSEtoPKCS(credentialPublicKey); + + toReturn.authenticatorInfo = { + fmt, + counter, + base64PublicKey: base64url.encode(publicKey), + base64CredentialID: base64url.encode(credentialID), + }; } - throw new Error(`Unsupported Attestation Format: ${fmt}`); + return toReturn; } /** -- cgit v1.2.3 From a418886826e4d4e55201f204a263a7686f905b81 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 23:40:50 -0700 Subject: Update generateAttestationOptions test --- .../attestation/generateAttestationOptions.test.ts | 24 +++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/generateAttestationOptions.test.ts b/packages/server/src/attestation/generateAttestationOptions.test.ts index 2b83fa9..112586a 100644 --- a/packages/server/src/attestation/generateAttestationOptions.test.ts +++ b/packages/server/src/attestation/generateAttestationOptions.test.ts @@ -35,6 +35,18 @@ test('should generate credential request options suitable for sending via JSON', alg: -7, type: 'public-key', }, + { + alg: -35, + type: 'public-key', + }, + { + alg: -36, + type: 'public-key', + }, + { + alg: -8, + type: 'public-key', + }, ], timeout, attestation: attestationType, @@ -52,11 +64,13 @@ test('should map excluded credential IDs if specified', () => { excludedCredentialIDs: ['someIDhere'], }); - expect(options.excludeCredentials).toEqual([{ - id: 'someIDhere', - type: 'public-key', - transports: ['usb', 'ble', 'nfc', 'internal'], - }]); + expect(options.excludeCredentials).toEqual([ + { + id: 'someIDhere', + type: 'public-key', + transports: ['usb', 'ble', 'nfc', 'internal'], + }, + ]); }); test('defaults to 60 seconds if no timeout is specified', () => { -- cgit v1.2.3 From e00d8217ddd4e149d2a8ebfd6a153c57d1c5fa61 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 23:43:23 -0700 Subject: Run linting over server package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Husky wasn’t running, so these weren’t completed at commit --- .../src/assertion/generateAssertionOptions.ts | 12 ++++---- .../src/assertion/verifyAssertionResponse.test.ts | 4 +-- .../src/assertion/verifyAssertionResponse.ts | 5 +--- .../src/attestation/generateAttestationOptions.ts | 33 +++++++++------------- .../attestation/verifyAttestationResponse.test.ts | 8 ++++-- 5 files changed, 27 insertions(+), 35 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/assertion/generateAssertionOptions.ts b/packages/server/src/assertion/generateAssertionOptions.ts index 6645e2e..d11677a 100644 --- a/packages/server/src/assertion/generateAssertionOptions.ts +++ b/packages/server/src/assertion/generateAssertionOptions.ts @@ -4,12 +4,12 @@ import type { } from '@simplewebauthn/typescript-types'; type Options = { - challenge: string, - allowedCredentialIDs: Base64URLString[], - suggestedTransports?: AuthenticatorTransport[], - timeout?: number, - userVerification?: UserVerificationRequirement, - extensions?: AuthenticationExtensionsClientInputs, + challenge: string; + allowedCredentialIDs: Base64URLString[]; + suggestedTransports?: AuthenticatorTransport[]; + timeout?: number; + userVerification?: UserVerificationRequirement; + extensions?: AuthenticationExtensionsClientInputs; }; /** diff --git a/packages/server/src/assertion/verifyAssertionResponse.test.ts b/packages/server/src/assertion/verifyAssertionResponse.test.ts index 9da06ce..0677252 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.test.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.test.ts @@ -47,9 +47,7 @@ test('should return authenticator info after verification', () => { ); expect(verification.authenticatorInfo.counter).toEqual(144); - expect(verification.authenticatorInfo.base64CredentialID).toEqual( - authenticator.credentialID, - ); + expect(verification.authenticatorInfo.base64CredentialID).toEqual(authenticator.credentialID); }); test('should throw when response challenge is not expected value', () => { diff --git a/packages/server/src/assertion/verifyAssertionResponse.ts b/packages/server/src/assertion/verifyAssertionResponse.ts index 7d13271..7b8f45a 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.ts @@ -1,8 +1,5 @@ import base64url from 'base64url'; -import { - AssertionCredentialJSON, - AuthenticatorDevice, -} from '@simplewebauthn/typescript-types'; +import { AssertionCredentialJSON, AuthenticatorDevice } from '@simplewebauthn/typescript-types'; import decodeClientDataJSON from '../helpers/decodeClientDataJSON'; import toHash from '../helpers/toHash'; diff --git a/packages/server/src/attestation/generateAttestationOptions.ts b/packages/server/src/attestation/generateAttestationOptions.ts index b80f19d..39d6378 100644 --- a/packages/server/src/attestation/generateAttestationOptions.ts +++ b/packages/server/src/attestation/generateAttestationOptions.ts @@ -4,28 +4,23 @@ import type { } from '@simplewebauthn/typescript-types'; type Options = { - serviceName: string, - rpID: string, - challenge: string, - userID: string, - userName: string, - userDisplayName?: string, - timeout?: number, - attestationType?: AttestationConveyancePreference, - excludedCredentialIDs?: Base64URLString[], - suggestedTransports?: AuthenticatorTransport[], - authenticatorSelection?: AuthenticatorSelectionCriteria, - extensions?: AuthenticationExtensionsClientInputs, + serviceName: string; + rpID: string; + challenge: string; + userID: string; + userName: string; + userDisplayName?: string; + timeout?: number; + attestationType?: AttestationConveyancePreference; + excludedCredentialIDs?: Base64URLString[]; + suggestedTransports?: AuthenticatorTransport[]; + authenticatorSelection?: AuthenticatorSelectionCriteria; + extensions?: AuthenticationExtensionsClientInputs; }; // Supported crypto algo identifiers // See https://w3c.github.io/webauthn/#sctn-alg-identifier -export const supportedCOSEAlgorithIdentifiers: COSEAlgorithmIdentifier[] = [ - -7, - -35, - -36, - -8 -]; +export const supportedCOSEAlgorithIdentifiers: COSEAlgorithmIdentifier[] = [-7, -35, -36, -8]; /** * Prepare a value to pass into navigator.credentials.create(...) for authenticator "registration" @@ -82,7 +77,7 @@ export default function generateAttestationOptions( })), timeout, attestation: attestationType, - excludeCredentials: excludedCredentialIDs.map((id) => ({ + excludeCredentials: excludedCredentialIDs.map(id => ({ id, type: 'public-key', transports: suggestedTransports, diff --git a/packages/server/src/attestation/verifyAttestationResponse.test.ts b/packages/server/src/attestation/verifyAttestationResponse.test.ts index d481138..1d6908f 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.test.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.test.ts @@ -172,11 +172,13 @@ const attestationFIDOU2F = { id: 'VHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUQ', rawId: 'VHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUQ', response: { - attestationObject: 'o2NmbXRoZmlkby11MmZnYXR0U3RtdKJjc2lnWEcwRQIgRYUftNUmhT0VWTZmIgDmrOoP26Pcre-kL3DLnCrXbegCIQCOu_x5gqp-Rej76zeBuXlk8e7J-9WM_i-wZmCIbIgCGmN4NWOBWQLBMIICvTCCAaWgAwIBAgIEKudiYzANBgkqhkiG9w0BAQsFADAuMSwwKgYDVQQDEyNZdWJpY28gVTJGIFJvb3QgQ0EgU2VyaWFsIDQ1NzIwMDYzMTAgFw0xNDA4MDEwMDAwMDBaGA8yMDUwMDkwNDAwMDAwMFowbjELMAkGA1UEBhMCU0UxEjAQBgNVBAoMCVl1YmljbyBBQjEiMCAGA1UECwwZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEnMCUGA1UEAwweWXViaWNvIFUyRiBFRSBTZXJpYWwgNzE5ODA3MDc1MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEKgOGXmBD2Z4R_xCqJVRXhL8Jr45rHjsyFykhb1USGozZENOZ3cdovf5Ke8fj2rxi5tJGn_VnW4_6iQzKdIaeP6NsMGowIgYJKwYBBAGCxAoCBBUxLjMuNi4xLjQuMS40MTQ4Mi4xLjEwEwYLKwYBBAGC5RwCAQEEBAMCBDAwIQYLKwYBBAGC5RwBAQQEEgQQbUS6m_bsLkm5MAyP6SDLczAMBgNVHRMBAf8EAjAAMA0GCSqGSIb3DQEBCwUAA4IBAQByV9A83MPhFWmEkNb4DvlbUwcjc9nmRzJjKxHc3HeK7GvVkm0H4XucVDB4jeMvTke0WHb_jFUiApvpOHh5VyMx5ydwFoKKcRs5x0_WwSWL0eTZ5WbVcHkDR9pSNcA_D_5AsUKOBcbpF5nkdVRxaQHuuIuwV4k1iK2IqtMNcU8vL6w21U261xCcWwJ6sMq4zzVO8QCKCQhsoIaWrwz828GDmPzfAjFsJiLJXuYivdHACkeJ5KHMt0mjVLpfJ2BCML7_rgbmvwL7wBW80VHfNdcKmKjkLcpEiPzwcQQhiN_qHV90t-p4iyr5xRSpurlP5zic2hlRkLKxMH2_kRjhqSn4aGF1dGhEYXRhWMQ93EcQ6cCIsinbqJ1WMiC7Ofcimv9GWwplaxr7mor4oEEAAAAAAAAAAAAAAAAAAAAAAAAAAABAVHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUaUBAgMmIAEhWCDIkcsOaVKDIQYwq3EDQ-pST2kRwNH_l1nCgW-WcFpNXiJYIBSbummp-KO3qZeqmvZ_U_uirCDL2RNj3E5y4_KzefIr', - clientDataJSON: 'eyJjaGFsbGVuZ2UiOiJkRzkwWVd4c2VWVnVhWEYxWlZaaGJIVmxSWFpsY25sQmRIUmxjM1JoZEdsdmJnIiwiY2xpZW50RXh0ZW5zaW9ucyI6e30sImhhc2hBbGdvcml0aG0iOiJTSEEtMjU2Iiwib3JpZ2luIjoiaHR0cHM6Ly9kZXYuZG9udG5lZWRhLnB3IiwidHlwZSI6IndlYmF1dGhuLmNyZWF0ZSJ9' + attestationObject: + 'o2NmbXRoZmlkby11MmZnYXR0U3RtdKJjc2lnWEcwRQIgRYUftNUmhT0VWTZmIgDmrOoP26Pcre-kL3DLnCrXbegCIQCOu_x5gqp-Rej76zeBuXlk8e7J-9WM_i-wZmCIbIgCGmN4NWOBWQLBMIICvTCCAaWgAwIBAgIEKudiYzANBgkqhkiG9w0BAQsFADAuMSwwKgYDVQQDEyNZdWJpY28gVTJGIFJvb3QgQ0EgU2VyaWFsIDQ1NzIwMDYzMTAgFw0xNDA4MDEwMDAwMDBaGA8yMDUwMDkwNDAwMDAwMFowbjELMAkGA1UEBhMCU0UxEjAQBgNVBAoMCVl1YmljbyBBQjEiMCAGA1UECwwZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEnMCUGA1UEAwweWXViaWNvIFUyRiBFRSBTZXJpYWwgNzE5ODA3MDc1MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEKgOGXmBD2Z4R_xCqJVRXhL8Jr45rHjsyFykhb1USGozZENOZ3cdovf5Ke8fj2rxi5tJGn_VnW4_6iQzKdIaeP6NsMGowIgYJKwYBBAGCxAoCBBUxLjMuNi4xLjQuMS40MTQ4Mi4xLjEwEwYLKwYBBAGC5RwCAQEEBAMCBDAwIQYLKwYBBAGC5RwBAQQEEgQQbUS6m_bsLkm5MAyP6SDLczAMBgNVHRMBAf8EAjAAMA0GCSqGSIb3DQEBCwUAA4IBAQByV9A83MPhFWmEkNb4DvlbUwcjc9nmRzJjKxHc3HeK7GvVkm0H4XucVDB4jeMvTke0WHb_jFUiApvpOHh5VyMx5ydwFoKKcRs5x0_WwSWL0eTZ5WbVcHkDR9pSNcA_D_5AsUKOBcbpF5nkdVRxaQHuuIuwV4k1iK2IqtMNcU8vL6w21U261xCcWwJ6sMq4zzVO8QCKCQhsoIaWrwz828GDmPzfAjFsJiLJXuYivdHACkeJ5KHMt0mjVLpfJ2BCML7_rgbmvwL7wBW80VHfNdcKmKjkLcpEiPzwcQQhiN_qHV90t-p4iyr5xRSpurlP5zic2hlRkLKxMH2_kRjhqSn4aGF1dGhEYXRhWMQ93EcQ6cCIsinbqJ1WMiC7Ofcimv9GWwplaxr7mor4oEEAAAAAAAAAAAAAAAAAAAAAAAAAAABAVHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUaUBAgMmIAEhWCDIkcsOaVKDIQYwq3EDQ-pST2kRwNH_l1nCgW-WcFpNXiJYIBSbummp-KO3qZeqmvZ_U_uirCDL2RNj3E5y4_KzefIr', + clientDataJSON: + 'eyJjaGFsbGVuZ2UiOiJkRzkwWVd4c2VWVnVhWEYxWlZaaGJIVmxSWFpsY25sQmRIUmxjM1JoZEdsdmJnIiwiY2xpZW50RXh0ZW5zaW9ucyI6e30sImhhc2hBbGdvcml0aG0iOiJTSEEtMjU2Iiwib3JpZ2luIjoiaHR0cHM6Ly9kZXYuZG9udG5lZWRhLnB3IiwidHlwZSI6IndlYmF1dGhuLmNyZWF0ZSJ9', }, getClientExtensionResults: () => ({}), - type: 'public-key' + type: 'public-key', }; const attestationFIDOU2FChallenge = 'totallyUniqueValueEveryAttestation'; -- cgit v1.2.3 From 682ba019375208d122ca3b5dd1622c093d0ec173 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 5 Jun 2020 23:51:17 -0700 Subject: Add doc for new attestation verification argument --- packages/server/src/attestation/verifyAttestationResponse.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 309c865..5819a12 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -20,6 +20,7 @@ import verifyAndroidSafetynet from './verifications/verifyAndroidSafetyNet'; * @param expectedChallenge The random value provided to generateAttestationOptions for the * authenticator to sign * @param expectedOrigin Expected URL of website attestation should have occurred on + * @param expectedRPID Expect RP ID as it was specified in the attestation options */ export default function verifyAttestationResponse( credential: AttestationCredentialJSON, -- cgit v1.2.3 From 877912b17d0a8fd897c643326ca33f41e4afdd27 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sat, 6 Jun 2020 00:21:59 -0700 Subject: Refactor Attestation verification to args object (BREAKING) --- .../attestation/verifyAttestationResponse.test.ts | 103 +++++++++++---------- .../src/attestation/verifyAttestationResponse.ts | 17 ++-- 2 files changed, 65 insertions(+), 55 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifyAttestationResponse.test.ts b/packages/server/src/attestation/verifyAttestationResponse.test.ts index 1d6908f..cdc2a1a 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.test.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.test.ts @@ -17,12 +17,12 @@ afterEach(() => { }); test('should verify FIDO U2F attestation', () => { - const verification = verifyAttestationResponse( - attestationFIDOU2F, - attestationFIDOU2FChallenge, - 'https://dev.dontneeda.pw', - 'dev.dontneeda.pw', - ); + const verification = verifyAttestationResponse({ + credential: attestationFIDOU2F, + expectedChallenge: attestationFIDOU2FChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); expect(verification.verified).toEqual(true); expect(verification.authenticatorInfo?.fmt).toEqual('fido-u2f'); @@ -36,12 +36,12 @@ test('should verify FIDO U2F attestation', () => { }); test('should verify Packed (EC2) attestation', () => { - const verification = verifyAttestationResponse( - attestationPacked, - attestationPackedChallenge, - 'https://dev.dontneeda.pw', - 'dev.dontneeda.pw', - ); + const verification = verifyAttestationResponse({ + credential: attestationPacked, + expectedChallenge: attestationPackedChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); expect(verification.verified).toEqual(true); expect(verification.authenticatorInfo?.fmt).toEqual('packed'); @@ -56,12 +56,12 @@ test('should verify Packed (EC2) attestation', () => { }); test('should verify Packed (X5C) attestation', () => { - const verification = verifyAttestationResponse( - attestationPackedX5C, - attestationPackedX5CChallenge, - 'https://dev.dontneeda.pw', - 'dev.dontneeda.pw', - ); + const verification = verifyAttestationResponse({ + credential: attestationPackedX5C, + expectedChallenge: attestationPackedX5CChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); expect(verification.verified).toEqual(true); expect(verification.authenticatorInfo?.fmt).toEqual('packed'); @@ -75,12 +75,12 @@ test('should verify Packed (X5C) attestation', () => { }); test('should verify None attestation', () => { - const verification = verifyAttestationResponse( - attestationNone, - attestationNoneChallenge, - 'https://dev.dontneeda.pw', - 'dev.dontneeda.pw', - ); + const verification = verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); expect(verification.verified).toEqual(true); expect(verification.authenticatorInfo?.fmt).toEqual('none'); @@ -94,12 +94,12 @@ test('should verify None attestation', () => { }); test('should verify Android SafetyNet attestation', () => { - const verification = verifyAttestationResponse( - attestationAndroidSafetyNet, - attestationAndroidSafetyNetChallenge, - 'https://dev.dontneeda.pw', - 'dev.dontneeda.pw', - ); + const verification = verifyAttestationResponse({ + credential: attestationAndroidSafetyNet, + expectedChallenge: attestationAndroidSafetyNetChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); expect(verification.verified).toEqual(true); expect(verification.authenticatorInfo?.fmt).toEqual('android-safetynet'); @@ -114,23 +114,23 @@ test('should verify Android SafetyNet attestation', () => { test('should throw when response challenge is not expected value', () => { expect(() => { - verifyAttestationResponse( - attestationNone, - 'shouldhavebeenthisvalue', - 'https://dev.dontneeda.pw', - 'dev.dontneeda.pw', - ); + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: 'shouldhavebeenthisvalue', + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); }).toThrow(/attestation challenge/i); }); test('should throw when response origin is not expected value', () => { expect(() => { - verifyAttestationResponse( - attestationNone, - attestationNoneChallenge, - 'https://different.address', - 'dev.dontneeda.pw', - ); + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://different.address', + expectedRPID: 'dev.dontneeda.pw', + }); }).toThrow(/attestation origin/i); }); @@ -146,7 +146,12 @@ test('should throw when attestation type is not webauthn.create', () => { }); expect(() => { - verifyAttestationResponse(attestationNone, challenge, origin, 'dev.dontneeda.pw'); + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: challenge, + expectedOrigin: origin, + expectedRPID: 'dev.dontneeda.pw', + }); }).toThrow(/attestation type/i); }); @@ -159,12 +164,12 @@ test('should throw if an unexpected attestation format is specified', () => { }); expect(() => { - verifyAttestationResponse( - attestationNone, - attestationNoneChallenge, - 'https://dev.dontneeda.pw', - '', - ); + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: '', + }); }).toThrow(); }); diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 5819a12..889b034 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -13,21 +13,26 @@ import verifyFIDOU2F from './verifications/verifyFIDOU2F'; import verifyPacked from './verifications/verifyPacked'; import verifyAndroidSafetynet from './verifications/verifyAndroidSafetyNet'; +type Options = { + credential: AttestationCredentialJSON; + expectedChallenge: string; + expectedOrigin: string; + expectedRPID: string; +}; + /** * Verify that the user has legitimately completed the registration process * + * **Options:** + * * @param response Authenticator attestation response with base64url-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 * @param expectedRPID Expect RP ID as it was specified in the attestation options */ -export default function verifyAttestationResponse( - credential: AttestationCredentialJSON, - expectedChallenge: string, - expectedOrigin: string, - expectedRPID: string, -): VerifiedAttestation { +export default function verifyAttestationResponse(options: Options): VerifiedAttestation { + const { credential, expectedChallenge, expectedOrigin, expectedRPID } = options; const { response } = credential; const clientDataJSON = decodeClientDataJSON(response.clientDataJSON); -- cgit v1.2.3 From 4cd3b2b1ca8ab2e6426eca03e4f0a3ff6c15f646 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sat, 6 Jun 2020 00:25:44 -0700 Subject: Make verification logic adhere closer to spec (BREAKING) Also converts method arguments into an args object --- .../src/assertion/verifyAssertionResponse.test.ts | 89 +++++++++++++--------- .../src/assertion/verifyAssertionResponse.ts | 54 +++++++------ 2 files changed, 81 insertions(+), 62 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/assertion/verifyAssertionResponse.test.ts b/packages/server/src/assertion/verifyAssertionResponse.test.ts index 0677252..8adecfe 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.test.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.test.ts @@ -2,6 +2,7 @@ import verifyAssertionResponse from './verifyAssertionResponse'; import * as decodeClientDataJSON from '../helpers/decodeClientDataJSON'; import * as parseAuthenticatorData from '../helpers/parseAuthenticatorData'; +import toHash from '../helpers/toHash'; let mockDecodeClientData: jest.SpyInstance; let mockParseAuthData: jest.SpyInstance; @@ -17,34 +18,25 @@ afterEach(() => { }); test('should verify an assertion response', () => { - const verification = verifyAssertionResponse( - assertionResponse, - assertionChallenge, - assertionOrigin, - authenticator, - ); - - expect(verification.verified).toEqual(true); -}); - -test('should verify an assertion response if origin does not start with https', () => { - const verification = verifyAssertionResponse( - assertionResponse, - assertionChallenge, - 'dev.dontneeda.pw', - authenticator, - ); + const verification = verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: assertionOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + }); expect(verification.verified).toEqual(true); }); test('should return authenticator info after verification', () => { - const verification = verifyAssertionResponse( - assertionResponse, - assertionChallenge, - assertionOrigin, - authenticator, - ); + const verification = verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: assertionOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + }); expect(verification.authenticatorInfo.counter).toEqual(144); expect(verification.authenticatorInfo.base64CredentialID).toEqual(authenticator.credentialID); @@ -52,23 +44,25 @@ test('should return authenticator info after verification', () => { test('should throw when response challenge is not expected value', () => { expect(() => { - verifyAssertionResponse( - assertionResponse, - 'shouldhavebeenthisvalue', - 'https://different.address', - authenticator, - ); + verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: 'shouldhavebeenthisvalue', + expectedOrigin: 'https://different.address', + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + }); }).toThrow(/assertion challenge/i); }); test('should throw when response origin is not expected value', () => { expect(() => { - verifyAssertionResponse( - assertionResponse, - assertionChallenge, - 'https://different.address', - authenticator, - ); + verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: 'https://different.address', + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + }); }).toThrow(/assertion origin/i); }); @@ -81,17 +75,30 @@ test('should throw when assertion type is not webauthn.create', () => { }); expect(() => { - verifyAssertionResponse(assertionResponse, assertionChallenge, assertionOrigin, authenticator); + verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: assertionOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + }); }).toThrow(/assertion type/i); }); test('should throw error if user was not present', () => { mockParseAuthData.mockReturnValue({ + rpIdHash: toHash(Buffer.from('dev.dontneeda.pw', 'ascii')), flags: 0, }); expect(() => { - verifyAssertionResponse(assertionResponse, assertionChallenge, assertionOrigin, authenticator); + verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: assertionOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + }); }).toThrow(/not present/i); }); @@ -104,7 +111,13 @@ test('should throw error if previous counter value is not less than in response' }; expect(() => { - verifyAssertionResponse(assertionResponse, assertionChallenge, assertionOrigin, badDevice); + verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: assertionOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: badDevice, + }); }).toThrow(/counter value/i); }); diff --git a/packages/server/src/assertion/verifyAssertionResponse.ts b/packages/server/src/assertion/verifyAssertionResponse.ts index 7b8f45a..93754c7 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.ts @@ -7,27 +7,34 @@ import convertASN1toPEM from '../helpers/convertASN1toPEM'; import verifySignature from '../helpers/verifySignature'; import parseAuthenticatorData from '../helpers/parseAuthenticatorData'; +type Options = { + credential: AssertionCredentialJSON; + expectedChallenge: string; + expectedOrigin: string; + expectedRPID: string; + authenticator: AuthenticatorDevice; +}; + /** * Verify that the user has legitimately completed the login process * + * **Options:** + * * @param response Authenticator assertion response with base64url-encoded values * @param expectedChallenge The random value provided to generateAssertionOptions for the * authenticator to sign * @param expectedOrigin Expected URL of website assertion should have occurred on */ -export default function verifyAssertionResponse( - credential: AssertionCredentialJSON, - expectedChallenge: string, - expectedOrigin: string, - authenticator: AuthenticatorDevice, -): VerifiedAssertion { +export default function verifyAssertionResponse(options: Options): VerifiedAssertion { + const { credential, expectedChallenge, expectedOrigin, expectedRPID, authenticator } = options; const { response } = credential; const clientDataJSON = decodeClientDataJSON(response.clientDataJSON); const { type, origin, challenge } = clientDataJSON; - if (!expectedOrigin.startsWith('https://')) { - expectedOrigin = `https://${expectedOrigin}`; + // Make sure we're handling an assertion + if (type !== 'webauthn.get') { + throw new Error(`Unexpected assertion type: ${type}`); } if (challenge !== expectedChallenge) { @@ -41,20 +48,27 @@ export default function verifyAssertionResponse( throw new Error(`Unexpected assertion origin "${origin}", expected "${expectedOrigin}"`); } - // Make sure we're handling an assertion - if (type !== 'webauthn.get') { - throw new Error(`Unexpected assertion type: ${type}`); - } + const parsedAuthData = parseAuthenticatorData(base64url.toBuffer(response.authenticatorData)); + const { rpIdHash, flags, counter, flagsBuf, counterBuf } = parsedAuthData; - const authDataBuffer = base64url.toBuffer(response.authenticatorData); - const authDataStruct = parseAuthenticatorData(authDataBuffer); - const { flags, counter } = authDataStruct; + // Make sure the response's RP ID is ours + const expectedRPIDHash = toHash(Buffer.from(expectedRPID, 'ascii')); + if (!rpIdHash.equals(expectedRPIDHash)) { + throw new Error(`Unexpected RP ID hash`); + } + // Make sure someone was physically present if (!flags.up) { throw new Error('User not present during assertion'); } - if (counter <= authenticator.counter) { + const clientDataHash = toHash(base64url.toBuffer(response.clientDataJSON)); + const signatureBase = Buffer.concat([rpIdHash, flagsBuf, counterBuf, clientDataHash]); + + const publicKey = convertASN1toPEM(base64url.toBuffer(authenticator.publicKey)); + const signature = base64url.toBuffer(response.signature); + + if ((counter > 0 || authenticator.counter > 0) && counter <= authenticator.counter) { // Error out when the counter in the DB is greater than or equal to the counter in the // dataStruct. It's related to how the authenticator maintains the number of times its been // used for this client. If this happens, then someone's somehow increased the counter @@ -64,14 +78,6 @@ export default function verifyAssertionResponse( ); } - const { rpIdHash, flagsBuf, counterBuf } = authDataStruct; - - const clientDataHash = toHash(base64url.toBuffer(response.clientDataJSON)); - const signatureBase = Buffer.concat([rpIdHash, flagsBuf, counterBuf, clientDataHash]); - - const publicKey = convertASN1toPEM(base64url.toBuffer(authenticator.publicKey)); - const signature = base64url.toBuffer(response.signature); - const toReturn = { verified: verifySignature(signature, signatureBase, publicKey), authenticatorInfo: { -- cgit v1.2.3 From 4dbd0851c04179a53b45eb41b4258fd5b549bac3 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sat, 6 Jun 2020 17:17:47 -0700 Subject: Add unit test for verifying first-time assertion --- .../src/assertion/verifyAssertionResponse.test.ts | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) (limited to 'packages/server/src') diff --git a/packages/server/src/assertion/verifyAssertionResponse.test.ts b/packages/server/src/assertion/verifyAssertionResponse.test.ts index 8adecfe..888afc1 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.test.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.test.ts @@ -121,6 +121,18 @@ test('should throw error if previous counter value is not less than in response' }).toThrow(/counter value/i); }); +test('should not compare counters if both are 0', () => { + const verification = verifyAssertionResponse({ + credential: assertionFirstTimeUsedResponse, + expectedChallenge: assertionFirstTimeUsedChallenge, + expectedOrigin: assertionFirstTimeUsedOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticatorFirstTimeUsed, + }); + + expect(verification.verified).toEqual(true); +}); + const assertionResponse = { id: 'KEbWNCc7NgaYnUyrNeFGX9_3Y-8oJ3KwzjnaiD1d1LVTxR7v3CaKfCz2Vy_g_MHSh7yJ8yL0Pxg6jo_o0hYiew', rawId: '', @@ -147,3 +159,28 @@ const authenticator = { 'KEbWNCc7NgaYnUyrNeFGX9_3Y-8oJ3KwzjnaiD1d1LVTxR7v3CaKfCz2Vy_g_MHSh7yJ8yL0Px' + 'g6jo_o0hYiew', counter: 0, }; + +/** + * Represented a device that's being used on the website for the first time + */ +const assertionFirstTimeUsedResponse = { + id: 'wSisR0_4hlzw3Y1tj4uNwwifIhRa-ZxWJwWbnfror0pVK9qPdBPO5pW3gasPqn6wXHb0LNhXB_IrA1nFoSQJ9A', + rawId: 'wSisR0_4hlzw3Y1tj4uNwwifIhRa-ZxWJwWbnfror0pVK9qPdBPO5pW3gasPqn6wXHb0LNhXB_IrA1nFoSQJ9A', + response: { + authenticatorData: 'PdxHEOnAiLIp26idVjIguzn3Ipr_RlsKZWsa-5qK-KABAAAAAA', + clientDataJSON: + 'eyJjaGFsbGVuZ2UiOiJkRzkwWVd4c2VWVnVhWEYxWlZaaGJIVmxSWFpsY25sQmMzTmxjblJwYjI0IiwiY2xpZW50RXh0ZW5zaW9ucyI6e30sImhhc2hBbGdvcml0aG0iOiJTSEEtMjU2Iiwib3JpZ2luIjoiaHR0cHM6Ly9kZXYuZG9udG5lZWRhLnB3IiwidHlwZSI6IndlYmF1dGhuLmdldCJ9', + signature: + 'MEQCIBu6M-DGzu1O8iocGHEj0UaAZm0HmxTeRIE6-nS3_CPjAiBDsmIzy5sacYwwzgpXqfwRt_2vl5yiQZ_OAqWJQBGVsQ', + }, + type: 'public-key', +}; +const assertionFirstTimeUsedChallenge = 'totallyUniqueValueEveryAssertion'; +const assertionFirstTimeUsedOrigin = 'https://dev.dontneeda.pw'; +const authenticatorFirstTimeUsed = { + publicKey: + 'BGmaxR4mBbukc2QhtW2ldhAAd555r-ljlGQN8MbcTnPP9CyUlE-0AB2fbzZbNgBvJuRa7r6o2jPphOmtyNPR_kY', + credentialID: + 'wSisR0_4hlzw3Y1tj4uNwwifIhRa-ZxWJwWbnfror0pVK9qPdBPO5pW3gasPqn6wXHb0LNhXB_IrA1nFoSQJ9A', + counter: 0, +}; -- cgit v1.2.3 From bf0ba9fe32520a1badde7648e26e9a9f6da8dc4b Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sat, 6 Jun 2020 17:18:10 -0700 Subject: Add assertion verification test for RP ID check --- .../src/assertion/verifyAssertionResponse.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'packages/server/src') diff --git a/packages/server/src/assertion/verifyAssertionResponse.test.ts b/packages/server/src/assertion/verifyAssertionResponse.test.ts index 888afc1..9066f53 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.test.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.test.ts @@ -121,6 +121,23 @@ test('should throw error if previous counter value is not less than in response' }).toThrow(/counter value/i); }); +test('should throw error if assertion RP ID is unexpected value', () => { + mockParseAuthData.mockReturnValue({ + rpIdHash: toHash(Buffer.from('bad.url', 'ascii')), + flags: 0, + }); + + expect(() => { + verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: assertionOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + }); + }).toThrow(/rp id/i); +}); + test('should not compare counters if both are 0', () => { const verification = verifyAssertionResponse({ credential: assertionFirstTimeUsedResponse, -- cgit v1.2.3 From f7157b3aa4d27a148db7b9d28b61d6e99619917a Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sat, 6 Jun 2020 17:18:33 -0700 Subject: Tweak existing assertion test authenticator counter --- packages/server/src/assertion/verifyAssertionResponse.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/server/src') diff --git a/packages/server/src/assertion/verifyAssertionResponse.test.ts b/packages/server/src/assertion/verifyAssertionResponse.test.ts index 9066f53..923e62f 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.test.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.test.ts @@ -174,7 +174,7 @@ const authenticator = { 'BIheFp-u6GvFT2LNGovf3ZrT0iFVBsA_76rRysxRG9A18WGeA6hPmnab0HAViUYVRkwTNcN77QBf_' + 'RR0dv3lIvQ', credentialID: 'KEbWNCc7NgaYnUyrNeFGX9_3Y-8oJ3KwzjnaiD1d1LVTxR7v3CaKfCz2Vy_g_MHSh7yJ8yL0Px' + 'g6jo_o0hYiew', - counter: 0, + counter: 143, }; /** -- cgit v1.2.3 From 9bc56318015a0b8cc11e9222c3c84f8b2119ce17 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sat, 6 Jun 2020 18:52:22 -0700 Subject: Add more tests for attestation verification --- .../attestation/verifyAttestationResponse.test.ts | 145 ++++++++++++++++++++- 1 file changed, 144 insertions(+), 1 deletion(-) (limited to 'packages/server/src') diff --git a/packages/server/src/attestation/verifyAttestationResponse.test.ts b/packages/server/src/attestation/verifyAttestationResponse.test.ts index cdc2a1a..24ec2db 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.test.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.test.ts @@ -2,18 +2,33 @@ import verifyAttestationResponse from './verifyAttestationResponse'; import * as decodeAttestationObject from '../helpers/decodeAttestationObject'; import * as decodeClientDataJSON from '../helpers/decodeClientDataJSON'; +import * as parseAuthenticatorData from '../helpers/parseAuthenticatorData'; +import * as decodeCredentialPublicKey from '../helpers/decodeCredentialPublicKey'; + +import * as verifyFIDOU2F from './verifications/verifyFIDOU2F'; + +import toHash from '../helpers/toHash'; let mockDecodeAttestation: jest.SpyInstance; let mockDecodeClientData: jest.SpyInstance; +let mockParseAuthData: jest.SpyInstance; +let mockDecodePubKey: jest.SpyInstance; +let mockVerifyFIDOU2F: jest.SpyInstance; beforeEach(() => { mockDecodeAttestation = jest.spyOn(decodeAttestationObject, 'default'); mockDecodeClientData = jest.spyOn(decodeClientDataJSON, 'default'); + mockParseAuthData = jest.spyOn(parseAuthenticatorData, 'default'); + mockDecodePubKey = jest.spyOn(decodeCredentialPublicKey, 'default'); + mockVerifyFIDOU2F = jest.spyOn(verifyFIDOU2F, 'default'); }); afterEach(() => { mockDecodeAttestation.mockRestore(); mockDecodeClientData.mockRestore(); + mockParseAuthData.mockRestore(); + mockDecodePubKey.mockRestore(); + mockVerifyFIDOU2F.mockRestore(); }); test('should verify FIDO U2F attestation', () => { @@ -158,11 +173,30 @@ test('should throw when attestation type is not webauthn.create', () => { test('should throw if an unexpected attestation format is specified', () => { const fmt = 'fizzbuzz'; + const realAtteObj = decodeAttestationObject.default(attestationNone.response.attestationObject); + mockDecodeAttestation.mockReturnValue({ + ...realAtteObj, // @ts-ignore 2322 fmt, }); + expect(() => { + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); + }).toThrow(/unsupported attestation format/i); +}); + +test('should throw error if assertion RP ID is unexpected value', () => { + mockParseAuthData.mockReturnValue({ + rpIdHash: toHash(Buffer.from('bad.url', 'ascii')), + flags: 0, + }); + expect(() => { verifyAttestationResponse({ credential: attestationNone, @@ -170,9 +204,118 @@ test('should throw if an unexpected attestation format is specified', () => { expectedOrigin: 'https://dev.dontneeda.pw', expectedRPID: '', }); - }).toThrow(); + }).toThrow(/rp id/i); +}); + +test('should throw error if user was not present', () => { + mockParseAuthData.mockReturnValue({ + rpIdHash: toHash(Buffer.from('dev.dontneeda.pw', 'ascii')), + flags: { + up: false, + }, + }); + + expect(() => { + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); + }).toThrow(/not present/i); +}); + +test('should throw if the authenticator does not give back credential ID', () => { + mockParseAuthData.mockReturnValue({ + rpIdHash: toHash(Buffer.from('dev.dontneeda.pw', 'ascii')), + flags: { + up: true, + }, + credentialID: undefined, + }); + + expect(() => { + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); + }).toThrow(/credential id/i); +}); + +test('should throw if the authenticator does not give back credential public key', () => { + mockParseAuthData.mockReturnValue({ + rpIdHash: toHash(Buffer.from('dev.dontneeda.pw', 'ascii')), + flags: { + up: true, + }, + credentialID: 'aaa', + credentialPublicKey: undefined, + }); + + expect(() => { + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); + }).toThrow(/public key/i); +}); + +test('should throw error if no alg is specified in public key', () => { + mockDecodePubKey.mockReturnValue({ + get: () => undefined, + credentialID: '', + credentialPublicKey: '', + }); + + expect(() => { + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); + }).toThrow(/missing alg/i); +}); + +test('should throw error if unsupported alg is used', () => { + mockDecodePubKey.mockReturnValue({ + get: () => -999, + credentialID: '', + credentialPublicKey: '', + }); + + expect(() => { + verifyAttestationResponse({ + credential: attestationNone, + expectedChallenge: attestationNoneChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); + }).toThrow(/unexpected public key/i); +}); + +test('should not include authenticator info if not verified', () => { + mockVerifyFIDOU2F.mockReturnValue(false); + + const verification = verifyAttestationResponse({ + credential: attestationFIDOU2F, + expectedChallenge: attestationFIDOU2FChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + }); + + expect(verification.verified).toBe(false); + expect(verification.authenticatorInfo).toBeUndefined(); }); +/** + * Various Attestations Below + */ + const attestationFIDOU2F = { id: 'VHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUQ', rawId: 'VHzbxaYaJu2P8m1Y2iHn2gRNHrgK0iYbn9E978L3Qi7Q-chFeicIHwYCRophz5lth2nCgEVKcgWirxlgidgbUQ', -- cgit v1.2.3 From f1c242745df883709d10276fca08eccdecc28f6b Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sun, 7 Jun 2020 14:24:05 -0700 Subject: Enable requiring user verification while verifying --- .../src/assertion/verifyAssertionResponse.test.ts | 30 ++++++++++++++++++++++ .../src/assertion/verifyAssertionResponse.ts | 23 ++++++++++++++--- .../attestation/verifyAttestationResponse.test.ts | 22 ++++++++++++++++ .../src/attestation/verifyAttestationResponse.ts | 20 ++++++++++++--- 4 files changed, 89 insertions(+), 6 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/assertion/verifyAssertionResponse.test.ts b/packages/server/src/assertion/verifyAssertionResponse.test.ts index 923e62f..20b6e0e 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.test.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.test.ts @@ -1,3 +1,4 @@ +import base64url from 'base64url'; import verifyAssertionResponse from './verifyAssertionResponse'; import * as decodeClientDataJSON from '../helpers/decodeClientDataJSON'; @@ -150,6 +151,35 @@ test('should not compare counters if both are 0', () => { expect(verification.verified).toEqual(true); }); +test('should throw an error if user verification is required but user was not verified', () => { + const actualData = parseAuthenticatorData.default( + base64url.toBuffer(assertionResponse.response.authenticatorData), + ); + + mockParseAuthData.mockReturnValue({ + ...actualData, + flags: { + up: true, + uv: false, + }, + }); + + expect(() => { + verifyAssertionResponse({ + credential: assertionResponse, + expectedChallenge: assertionChallenge, + expectedOrigin: assertionOrigin, + expectedRPID: 'dev.dontneeda.pw', + authenticator: authenticator, + requireUserVerification: true, + }); + }).toThrow(/user could not be verified/i); +}); + +/** + * Assertion examples below + */ + const assertionResponse = { id: 'KEbWNCc7NgaYnUyrNeFGX9_3Y-8oJ3KwzjnaiD1d1LVTxR7v3CaKfCz2Vy_g_MHSh7yJ8yL0Pxg6jo_o0hYiew', rawId: '', diff --git a/packages/server/src/assertion/verifyAssertionResponse.ts b/packages/server/src/assertion/verifyAssertionResponse.ts index 93754c7..9dedc2d 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.ts @@ -13,6 +13,7 @@ type Options = { expectedOrigin: string; expectedRPID: string; authenticator: AuthenticatorDevice; + requireUserVerification?: boolean; }; /** @@ -20,13 +21,24 @@ type Options = { * * **Options:** * - * @param response Authenticator assertion response with base64url-encoded values + * @param credential Authenticator credential returned by browser's `startAssertion()` * @param expectedChallenge The random value provided to generateAssertionOptions for the * authenticator to sign - * @param expectedOrigin Expected URL of website assertion should have occurred on + * @param expectedOrigin Website URL that the attestation should have occurred on + * @param expectedRPID RP ID that was specified in the attestation options + * @param authenticator An internal {@link AuthenticatorDevice} matching the credential's ID + * @param requireUserVerification (Optional) Enforce user verification by the authenticator + * (via PIN, fingerprint, etc...) */ export default function verifyAssertionResponse(options: Options): VerifiedAssertion { - const { credential, expectedChallenge, expectedOrigin, expectedRPID, authenticator } = options; + const { + credential, + expectedChallenge, + expectedOrigin, + expectedRPID, + authenticator, + requireUserVerification = false, + } = options; const { response } = credential; const clientDataJSON = decodeClientDataJSON(response.clientDataJSON); @@ -62,6 +74,11 @@ export default function verifyAssertionResponse(options: Options): VerifiedAsser throw new Error('User not present during assertion'); } + // Enforce user verification if specified + if (requireUserVerification && !flags.uv) { + throw new Error('User verification required, but user could not be verified'); + } + const clientDataHash = toHash(base64url.toBuffer(response.clientDataJSON)); const signatureBase = Buffer.concat([rpIdHash, flagsBuf, counterBuf, clientDataHash]); diff --git a/packages/server/src/attestation/verifyAttestationResponse.test.ts b/packages/server/src/attestation/verifyAttestationResponse.test.ts index 24ec2db..b2ff37c 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.test.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.test.ts @@ -1,3 +1,5 @@ +import base64url from 'base64url'; + import verifyAttestationResponse from './verifyAttestationResponse'; import * as decodeAttestationObject from '../helpers/decodeAttestationObject'; @@ -312,6 +314,26 @@ test('should not include authenticator info if not verified', () => { expect(verification.authenticatorInfo).toBeUndefined(); }); +test('should throw an error if user verification is required but user was not verified', () => { + mockParseAuthData.mockReturnValue({ + rpIdHash: toHash(Buffer.from('dev.dontneeda.pw', 'ascii')), + flags: { + up: true, + uv: false, + }, + }); + + expect(() => { + const verification = verifyAttestationResponse({ + credential: attestationFIDOU2F, + expectedChallenge: attestationFIDOU2FChallenge, + expectedOrigin: 'https://dev.dontneeda.pw', + expectedRPID: 'dev.dontneeda.pw', + requireUserVerification: true, + }); + }).toThrow(/user could not be verified/i); +}); + /** * Various Attestations Below */ diff --git a/packages/server/src/attestation/verifyAttestationResponse.ts b/packages/server/src/attestation/verifyAttestationResponse.ts index 889b034..6779244 100644 --- a/packages/server/src/attestation/verifyAttestationResponse.ts +++ b/packages/server/src/attestation/verifyAttestationResponse.ts @@ -18,6 +18,7 @@ type Options = { expectedChallenge: string; expectedOrigin: string; expectedRPID: string; + requireUserVerification?: boolean; }; /** @@ -28,11 +29,19 @@ type Options = { * @param response Authenticator attestation response with base64url-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 - * @param expectedRPID Expect RP ID as it was specified in the attestation options + * @param expectedOrigin Website URL that the attestation should have occurred on + * @param expectedRPID RP ID that was specified in the attestation options + * @param requireUserVerification (Optional) Enforce user verification by the authenticator + * (via PIN, fingerprint, etc...) */ export default function verifyAttestationResponse(options: Options): VerifiedAttestation { - const { credential, expectedChallenge, expectedOrigin, expectedRPID } = options; + const { + credential, + expectedChallenge, + expectedOrigin, + expectedRPID, + requireUserVerification = false, + } = options; const { response } = credential; const clientDataJSON = decodeClientDataJSON(response.clientDataJSON); @@ -72,6 +81,11 @@ export default function verifyAttestationResponse(options: Options): VerifiedAtt throw new Error('User not present during assertion'); } + // Enforce user verification if specified + if (requireUserVerification && !flags.uv) { + throw new Error('User verification required, but user could not be verified'); + } + if (!credentialID) { throw new Error('No credential ID was provided by authenticator'); } -- cgit v1.2.3 From 9b08f52ae1f3a9670e36992db35ac66bf1eb9aa9 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Sun, 7 Jun 2020 14:33:28 -0700 Subject: Simplify signature base concat during assertion --- packages/server/src/assertion/verifyAssertionResponse.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'packages/server/src') diff --git a/packages/server/src/assertion/verifyAssertionResponse.ts b/packages/server/src/assertion/verifyAssertionResponse.ts index 9dedc2d..0029796 100644 --- a/packages/server/src/assertion/verifyAssertionResponse.ts +++ b/packages/server/src/assertion/verifyAssertionResponse.ts @@ -60,8 +60,9 @@ export default function verifyAssertionResponse(options: Options): VerifiedAsser throw new Error(`Unexpected assertion origin "${origin}", expected "${expectedOrigin}"`); } - const parsedAuthData = parseAuthenticatorData(base64url.toBuffer(response.authenticatorData)); - const { rpIdHash, flags, counter, flagsBuf, counterBuf } = parsedAuthData; + const authDataBuffer = base64url.toBuffer(response.authenticatorData); + const parsedAuthData = parseAuthenticatorData(authDataBuffer); + const { rpIdHash, flags, counter } = parsedAuthData; // Make sure the response's RP ID is ours const expectedRPIDHash = toHash(Buffer.from(expectedRPID, 'ascii')); @@ -80,7 +81,7 @@ export default function verifyAssertionResponse(options: Options): VerifiedAsser } const clientDataHash = toHash(base64url.toBuffer(response.clientDataJSON)); - const signatureBase = Buffer.concat([rpIdHash, flagsBuf, counterBuf, clientDataHash]); + const signatureBase = Buffer.concat([authDataBuffer, clientDataHash]); const publicKey = convertASN1toPEM(base64url.toBuffer(authenticator.publicKey)); const signature = base64url.toBuffer(response.signature); -- cgit v1.2.3