2 files changed, 81 insertions, 62 deletions

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: {