Clearly protect against algorithm confusion attack

# Conflicts: # src/test/verifyAppleIdToken.test.ts
stefanprokopdev · Nov 6, 2023 · ef6c0eb · ef6c0eb
1 parent 507d5f5
commit ef6c0eb
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 15 deletions.
diff --git a/src/lib/verifyAppleIdToken.ts b/src/lib/verifyAppleIdToken.ts
@@ -5,7 +5,7 @@ import { VerifyAppleIdTokenParams, VerifyAppleIdTokenResponse } from "./types";
 export const APPLE_BASE_URL = "https://appleid.apple.com";
 export const JWKS_APPLE_URI = "/auth/keys";
 
-export const getApplePublicKey = async (kid: string) => {
+export const getAppleJWK = async (kid: string) => {
   const client = jwksClient({
     cache: true,
     jwksUri: `${APPLE_BASE_URL}${JWKS_APPLE_URI}`,
@@ -18,16 +18,31 @@ export const getApplePublicKey = async (kid: string) => {
       return resolve(result);
     });
   });
-  return key.getPublicKey();
+  return {
+    publicKey: key.getPublicKey(),
+    kid: key.kid,
+    alg: key.alg,
+  };
+};
+
+export const getApplePublicKey = async (kid: string) => {
+  const jwk = await getAppleJWK(kid);
+
+  return jwk.publicKey;
 };
 
 export const verifyToken = async (params: VerifyAppleIdTokenParams) => {
   const decoded = jwt.decode(params.idToken, { complete: true });
-  const { kid, alg } = decoded.header;
+  const { kid, alg: jwtAlg } = decoded.header;
+
+  const { publicKey, alg: jwkAlg } = await getAppleJWK(kid);
+
+  if (jwtAlg !== jwkAlg) {
+    throw new Error(`The alg does not match the jwk configuration - alg: ${jwtAlg} | expected: ${jwkAlg}`);
+  }
 
-  const applePublicKey = await getApplePublicKey(kid);
-  const jwtClaims = jwt.verify(params.idToken, applePublicKey, {
-    algorithms: [alg as jwt.Algorithm],
+  const jwtClaims = jwt.verify(params.idToken, publicKey, {
+    algorithms: [jwkAlg as jwt.Algorithm],
     nonce: params.nonce,
   }) as VerifyAppleIdTokenResponse;
 

diff --git a/src/test/verifyAppleIdToken.test.ts b/src/test/verifyAppleIdToken.test.ts
@@ -1,5 +1,6 @@
 import mockDate from "mockdate";
-import verifyAppleIdToken from "../index";
+import * as jwt from "jsonwebtoken";
+import verifyAppleIdToken, { getApplePublicKey } from "../index";
 import { APPLE_BASE_URL, JWKS_APPLE_URI } from "../lib/verifyAppleIdToken";
 import { EXPIRY_DATE, getJwksMock, getToken } from "./utils/jwksMock";
 
@@ -21,7 +22,7 @@ describe("Verify Apple idToken", () => {
         aud: clientId,
         sub: email,
       },
-      jwksMock,
+      jwksMock
     );
     const claims = await verifyAppleIdToken({ clientId, idToken: token });
     expect(claims.email).toEqual(email);
@@ -36,7 +37,7 @@ describe("Verify Apple idToken", () => {
         aud: secondClientId,
         sub: email,
       },
-      jwksMock,
+      jwksMock
     );
     const claims = await verifyAppleIdToken({
       clientId: [clientId, secondClientId],
@@ -46,7 +47,7 @@ describe("Verify Apple idToken", () => {
     expect(claims.aud).toEqual(secondClientId);
     expect(claims).toMatchSnapshot();
   });
-  it("ISS field is not valid", async () => {
+  it("The `iss` field is not valid", async () => {
     try {
       const idToken = getToken(
         {
@@ -55,15 +56,15 @@ describe("Verify Apple idToken", () => {
           aud: clientId,
           iss: "test.iss",
         },
-        jwksMock,
+        jwksMock
       );
       await verifyAppleIdToken({ clientId, idToken });
     } catch (error) {
       return expect(error.message).toMatch(/The iss does not match the Apple URL/);
     }
     throw new Error("Expected to throw");
   });
-  it("The aud field is not valid", async () => {
+  it("The `aud` field is not valid", async () => {
     try {
       const idToken = getToken(
         {
@@ -72,14 +73,38 @@ describe("Verify Apple idToken", () => {
           aud: clientId,
           sub: "sub",
         },
-        jwksMock,
+        jwksMock
       );
       await verifyAppleIdToken({ idToken, clientId: "test" });
     } catch (error) {
       return expect(error.message).toMatch(/The aud parameter does not include this client/);
     }
     throw new Error("Expected to throw");
   });
+  it("The `header.alg` field is not valid", async () => {
+    try {
+      const idToken = getToken(
+        {
+          email,
+          iss: APPLE_BASE_URL,
+          aud: clientId,
+          sub: email,
+        },
+        jwksMock
+      );
+
+      const decoded = jwt.decode(idToken, { complete: true });
+      const { kid } = decoded.header;
+      const publicKey = await getApplePublicKey(kid);
+
+      const modifiedToken = jwt.sign(decoded.payload, publicKey, { algorithm: "HS256", keyid: kid });
+
+      await verifyAppleIdToken({ idToken: modifiedToken, clientId: "test" });
+    } catch (error) {
+      return expect(error.message).toMatch(/The alg does not match the jwk configuration - alg: HS256 | expected: RS256/);
+    }
+    throw new Error("Expected to throw");
+  });
   it("Token is expired", async () => {
     try {
       const idToken = getToken(
@@ -90,7 +115,7 @@ describe("Verify Apple idToken", () => {
           iss: APPLE_BASE_URL,
           exp: new Date("2019-01-01"),
         },
-        jwksMock,
+        jwksMock
       );
       await verifyAppleIdToken({ idToken, clientId });
     } catch (error) {
@@ -108,7 +133,7 @@ describe("Verify Apple idToken", () => {
           iss: APPLE_BASE_URL,
           nonce: "abc",
         },
-        jwksMock,
+        jwksMock
       );
       await verifyAppleIdToken({ idToken, clientId, nonce: "def" });
     } catch (error) {