fix: make sure we return the chain back in the original order

Sphereon-Opensource · Dec 1, 2024 · 683ddb7 · 683ddb7
1 parent 503768f
commit 683ddb7
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 15 deletions.
diff --git a/packages/x509-utils/__tests__/functions.test.ts b/packages/x509-utils/__tests__/functions.test.ts
@@ -142,7 +142,7 @@ describe('functions: validateX5cCertificateChain', () => {
       'QlHHFydMdgaXAiEA1Ib82mhHIYDziE0DDbHEAXOs98al+7dpo8fPGVGTeKI=\n' +
       '-----END CERTIFICATE-----'
     const result = await validateX509CertificateChain({
-      chain: [sphereonCA, sphereonTest],
+      chain: [sphereonTest, sphereonCA],
       trustAnchors: [sphereonSDJWTCA],
       opts: { trustRootWhenNoAnchors: false } /*, trustedCerts: [sphereonCA]*/,
     })
@@ -175,7 +175,7 @@ describe('functions: validateX5cCertificateChain', () => {
   // TODO disabled as cert expired
   xit('should validate a valid certificate chain without providing a CA as trust anchor, but with trustRoot enabled', async () => {
     const result = await validateX509CertificateChain({
-      chain: [walletPEM, sphereonCA],
+      chain: [walletPEM],
       trustAnchors: [sphereonCA],
       opts: {
         client: {

diff --git a/packages/x509-utils/src/x509/x509-validator.ts b/packages/x509-utils/src/x509/x509-validator.ts
@@ -110,7 +110,7 @@ export const validateX509CertificateChain = async ({
   // We allow 1 reversal. We reverse by default as the implementation expects the root ca first, whilst x5c is the opposite. Reversed becomes true if the impl reverses the chain
   return await validateX509CertificateChainImpl({
     reversed: false,
-    chain: pemOrDerChain.reverse(),
+    chain: [...pemOrDerChain].reverse(),
     trustAnchors,
     verificationTime,
     opts,
@@ -149,8 +149,11 @@ const validateX509CertificateChainImpl = async ({
   }
   defaultCryptoEngine()
 
-  // x5c always starts with the leaf cert at index 0 and then the cas. Our internal pkijs service expects it the other way around
+  // x5c always starts with the leaf cert at index 0 and then the cas. Our internal pkijs service expects it the other way around. Before calling this function the change has been revered
   const chain = await Promise.all(pemOrDerChain.map((raw) => parseCertificate(raw)))
+  const x5cOrdereredChain = reversed ? [...chain] : [...chain].reverse()
+  console.log(`x5c orderered chain (reverse: ${reversed}): ${x5cOrdereredChain.map((cert) => cert.certificateInfo.subject.dn.DN).join(', ')}`)
+
   const trustedCerts = trustedPEMs ? await Promise.all(trustedPEMs.map((raw) => parseCertificate(raw))) : undefined
   const blindlyTrusted =
     (
@@ -166,7 +169,7 @@ const validateX509CertificateChainImpl = async ({
         })
       )
     ).filter((cert): cert is ParsedCertificate => cert !== undefined) ?? []
-  const leafCert = chain[chain.length - 1]
+  const leafCert = x5cOrdereredChain[0]
 
   const chainLength = chain.length
   var foundTrustAnchor: ParsedCertificate | undefined = undefined
@@ -183,7 +186,7 @@ const validateX509CertificateChainImpl = async ({
         detailMessage: `Blindly trusted certificate ${blindlyTrustedCert.certificateInfo.subject.dn.DN} was found in the chain.`,
         trustAnchor: blindlyTrustedCert?.certificateInfo,
         verificationTime,
-        certificateChain: chain.map((cert) => cert.certificateInfo),
+        certificateChain: x5cOrdereredChain.map((cert) => cert.certificateInfo),
         ...(client && { client }),
       }
     }
@@ -192,7 +195,7 @@ const validateX509CertificateChainImpl = async ({
         if (!reversed && !disallowReversedChain) {
           return await validateX509CertificateChainImpl({
             reversed: true,
-            chain: pemOrDerChain.reverse(),
+            chain: [...pemOrDerChain].reverse(),
             opts,
             verificationTime,
             trustAnchors,
@@ -201,7 +204,7 @@ const validateX509CertificateChainImpl = async ({
         return {
           error: true,
           critical: true,
-          certificateChain: chain.map((cert) => cert.certificateInfo),
+          certificateChain: x5cOrdereredChain.map((cert) => cert.certificateInfo),
           message: `Certificate chain validation failed for ${leafCert.certificateInfo.subject.dn.DN}.`,
           detailMessage: `The certificate ${currentCert.certificateInfo.subject.dn.DN} with issuer ${currentCert.x509Certificate.issuer}, is not signed by the previous certificate ${previousCert?.certificateInfo.subject.dn.DN} with subject string ${previousCert?.x509Certificate.subject}.`,
           verificationTime,
@@ -220,7 +223,7 @@ const validateX509CertificateChainImpl = async ({
       if (i == 0 && !reversed && !disallowReversedChain) {
         return await validateX509CertificateChainImpl({
           reversed: true,
-          chain: pemOrDerChain.reverse(),
+          chain: [...pemOrDerChain].reverse(),
           opts,
           verificationTime,
           trustAnchors,
@@ -230,7 +233,7 @@ const validateX509CertificateChainImpl = async ({
         error: true,
         critical: true,
         message: `Certificate chain validation failed for ${leafCert.certificateInfo.subject.dn.DN}.`,
-        certificateChain: chain.map((cert) => cert.certificateInfo),
+        certificateChain: x5cOrdereredChain.map((cert) => cert.certificateInfo),
         detailMessage: `Verification of the certificate ${currentCert.certificateInfo.subject.dn.DN} with issuer ${
           currentCert.x509Certificate.issuer
         } failed. Public key: ${JSON.stringify(currentCert.certificateInfo.publicKeyJWK)}.`,
@@ -246,7 +249,7 @@ const validateX509CertificateChainImpl = async ({
         error: false,
         critical: false,
         message: `Certificate chain succeeded as allow single cert result is allowed: ${leafCert.certificateInfo.subject.dn.DN}.`,
-        certificateChain: chain.map((cert) => cert.certificateInfo),
+        certificateChain: x5cOrdereredChain.map((cert) => cert.certificateInfo),
         trustAnchor: foundTrustAnchor?.certificateInfo,
         verificationTime,
         ...(client && { client }),
@@ -259,7 +262,7 @@ const validateX509CertificateChainImpl = async ({
       error: false,
       critical: false,
       message: `Certificate chain was valid`,
-      certificateChain: chain.map((cert) => cert.certificateInfo),
+      certificateChain: x5cOrdereredChain.map((cert) => cert.certificateInfo),
       detailMessage: `The leaf certificate ${leafCert.certificateInfo.subject.dn.DN} is part of a chain with trust anchor ${foundTrustAnchor?.certificateInfo.subject.dn.DN}.`,
       trustAnchor: foundTrustAnchor?.certificateInfo,
       verificationTime,
@@ -271,9 +274,9 @@ const validateX509CertificateChainImpl = async ({
     error: true,
     critical: true,
     message: `Certificate chain validation failed for ${leafCert.certificateInfo.subject.dn.DN}.`,
-    certificateChain: chain.map((cert) => cert.certificateInfo),
-    detailMessage: `No trust anchor was found in the chain. between ${chain[0].certificateInfo.subject.dn.DN} and ${
-      chain[chain.length - 1].certificateInfo.subject.dn.DN
+    certificateChain: x5cOrdereredChain.map((cert) => cert.certificateInfo),
+    detailMessage: `No trust anchor was found in the chain. between (intermediate) CA ${x5cOrdereredChain[chain.length - 1].certificateInfo.subject.dn.DN} and leaf ${
+        x5cOrdereredChain[0].certificateInfo.subject.dn.DN
     }.`,
     verificationTime,
     ...(client && { client }),