Merge pull request #607 from justin-tay/gh606

fix(ndi): fix incorrect key selection for ecdh-es
opengovsg · Oct 9, 2023 · 8bee4d4 · 8bee4d4
2 parents 40485b1 + 7d459f2
commit 8bee4d4
Showing 1 changed file with 58 additions and 22 deletions.
diff --git a/lib/express/oidc/v2-ndi.js b/lib/express/oidc/v2-ndi.js
@@ -59,47 +59,80 @@ const id_token_encryption_alg_values_supported = {
   corpPass: corppass_id_token_encryption_alg_values_supported,
 }
 
-function findEncryptionKey(jwks) {
+function findEcdhEsEncryptionKey(jwks, crv, algs) {
   let encryptionKey = jwks.keys.find(
     (item) =>
       item.use === 'enc' &&
       item.kty === 'EC' &&
-      item.crv === 'P-521' &&
-      (item.alg === 'ECDH-ES+A256KW' || !item.alg),
+      item.crv === crv &&
+      (!item.alg ||
+        (item.alg === 'ECDH-ES+A256KW' &&
+          algs.some((alg) => alg === item.alg))),
   )
   if (encryptionKey) {
-    return { ...encryptionKey, alg: 'ECDH-ES+A256KW' }
+    return {
+      ...encryptionKey,
+      ...(!encryptionKey.alg ? { alg: 'ECDH-ES+A256KW' } : {}),
+    }
+  }
+  encryptionKey = jwks.keys.find(
+    (item) =>
+      item.use === 'enc' &&
+      item.kty === 'EC' &&
+      item.crv === crv &&
+      (!item.alg ||
+        (item.alg === 'ECDH-ES+A192KW' &&
+          algs.some((alg) => alg === item.alg))),
+  )
+  if (encryptionKey) {
+    return {
+      ...encryptionKey,
+      ...(!encryptionKey.alg ? { alg: 'ECDH-ES+A256KW' } : {}),
+    }
+  }
+  encryptionKey = jwks.keys.find(
+    (item) =>
+      item.use === 'enc' &&
+      item.kty === 'EC' &&
+      item.crv === crv &&
+      (!item.alg ||
+        (item.alg === 'ECDH-ES+A128KW' &&
+          algs.some((alg) => alg === item.alg))),
+  )
+  if (encryptionKey) {
+    return {
+      ...encryptionKey,
+      ...(!encryptionKey.alg ? { alg: 'ECDH-ES+A256KW' } : {}),
+    }
+  }
+  return null
+}
+
+function findEncryptionKey(jwks, algs) {
+  let encryptionKey = findEcdhEsEncryptionKey(jwks, 'P-521', algs)
+  if (encryptionKey) {
+    return encryptionKey
   }
   if (!encryptionKey) {
-    encryptionKey = jwks.keys.find(
-      (item) =>
-        item.use === 'enc' &&
-        item.kty === 'EC' &&
-        item.crv === 'P-384' &&
-        (item.alg === 'ECDH-ES+A192KW' || !item.alg),
-    )
+    encryptionKey = findEcdhEsEncryptionKey(jwks, 'P-384', algs)
   }
   if (encryptionKey) {
-    return { ...encryptionKey, alg: 'ECDH-ES+A192KW' }
+    return encryptionKey
   }
   if (!encryptionKey) {
-    encryptionKey = jwks.keys.find(
-      (item) =>
-        item.use === 'enc' &&
-        item.kty === 'EC' &&
-        item.crv === 'P-256' &&
-        (item.alg === 'ECDH-ES+A128KW' || !item.alg),
-    )
+    encryptionKey = findEcdhEsEncryptionKey(jwks, 'P-256', algs)
   }
   if (encryptionKey) {
-    return { ...encryptionKey, alg: 'ECDH-ES+A128KW' }
+    return encryptionKey
   }
   if (!encryptionKey) {
     encryptionKey = jwks.keys.find(
       (item) =>
         item.use === 'enc' &&
         item.kty === 'RSA' &&
-        (item.alg === 'RSA-OAEP-256' || !item.alg),
+        (!item.alg ||
+          (item.alg === 'RSA-OAEP-256' &&
+            algs.some((alg) => alg === item.alg))),
     )
   }
   if (encryptionKey) {
@@ -441,7 +474,10 @@ function config(app, { showLoginPage }) {
           .sign(signingKey)
 
         // Step 4: Encrypt ID token with RP encryption key
-        const rpEncryptionKey = findEncryptionKey(rpKeysetJson)
+        const rpEncryptionKey = findEncryptionKey(
+          rpKeysetJson,
+          id_token_encryption_alg_values_supported[idp],
+        )
         if (!rpEncryptionKey) {
           console.error('No suitable encryption key found', rpKeysetJson.keys)
           return res.status(400).send({