From 89da8225ded5552027d527d72d6f25005509e460 Mon Sep 17 00:00:00 2001
From: Kai Kramer <kaikramer@users.noreply.github.com>
Date: Fri, 1 Sep 2023 19:12:54 +0200
Subject: [PATCH] Improved certificate chain detection (issue #421)

Algorithm now also checks the signature, making it more precise, but
this comes with two disadvantages:
1. It is slower than before, this is mitigated by keeping the
verification operations as low as possible
2. If the certificate should contain an unsupported signature algorithm
then the keystore content cannot be displayed anymore
---
 kse/src/org/kse/crypto/x509/X509CertUtil.java | 31 ++++++++++++++++---
 .../org/kse/gui/dialogs/DViewCertificate.java | 14 +++------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/kse/src/org/kse/crypto/x509/X509CertUtil.java b/kse/src/org/kse/crypto/x509/X509CertUtil.java
index c489300b7..c94c88202 100644
--- a/kse/src/org/kse/crypto/x509/X509CertUtil.java
+++ b/kse/src/org/kse/crypto/x509/X509CertUtil.java
@@ -327,22 +327,45 @@ public static X509Certificate[] orderX509CertChain(X509Certificate[] certs) {
     }
 
     private static X509Certificate findIssuedCert(X509Certificate issuerCert, X509Certificate[] certs) {
-        // Find a certificate issued by the supplied certificate based on  distiguished name
+        // Find a certificate issued by the supplied certificate
         for (X509Certificate cert : certs) {
             if (issuerCert.getSubjectX500Principal().equals(cert.getSubjectX500Principal()) &&
-                issuerCert.getIssuerX500Principal().equals(cert.getIssuerX500Principal())) {
+                issuerCert.getIssuerX500Principal().equals(cert.getIssuerX500Principal()) &&
+                issuerCert.getSerialNumber().equals(cert.getSerialNumber())) {
                 // Checked certificate is issuer - ignore it
                 continue;
             }
 
-            if (issuerCert.getSubjectX500Principal().equals(cert.getIssuerX500Principal())) {
+            if (isIssuedBy(cert, issuerCert)) {
                 return cert;
             }
         }
-
         return null;
     }
 
+    /**
+     * Checks if certificate was issued by the other certificate by checking first the DN and only if the issuer DN
+     * matches the subject DN, then the signature is verified. This avoids the slow verification operation when it is
+     * impossible that the second certificate has signed the first one.
+     *
+     * @param cert The issued certificate
+     * @param issuerCert The possible issuer certificate
+     * @return True, if issuerCert has issued cert, false otherwise
+     */
+    public static boolean isIssuedBy(X509Certificate cert, X509Certificate issuerCert) {
+        if (issuerCert.getSubjectX500Principal().equals(cert.getIssuerX500Principal())) {
+            // possible candidate found, now check if signature matches the issuer key
+            try {
+                if (verifyCertificate(cert, issuerCert)) {
+                    return true;
+                }
+            } catch (CryptoException e) {
+                // wrong certificate, continue
+            }
+        }
+        return false;
+    }
+
     /**
      * X.509 encode a certificate.
      *
diff --git a/kse/src/org/kse/gui/dialogs/DViewCertificate.java b/kse/src/org/kse/gui/dialogs/DViewCertificate.java
index 20e2c6167..fb16b9bc5 100644
--- a/kse/src/org/kse/gui/dialogs/DViewCertificate.java
+++ b/kse/src/org/kse/gui/dialogs/DViewCertificate.java
@@ -442,19 +442,15 @@ private DefaultMutableTreeNode createCertificateNodes(X509Certificate[] certs) {
     }
 
     private DefaultMutableTreeNode findIssuer(X509Certificate cert, DefaultMutableTreeNode node) {
-        // Matches on certificate's distinguished name
-
         // If certificate is self-signed then finding an issuer is irrelevant
         if (cert.getIssuerX500Principal().equals(cert.getSubjectX500Principal())) {
             return null;
         }
 
-        Object nodeObj = node.getUserObject();
-
-        if (nodeObj instanceof X509Certificate) {
-            X509Certificate nodeCert = (X509Certificate) nodeObj;
+        if (node.getUserObject() instanceof X509Certificate) {
+            X509Certificate possibleIssuerCert = (X509Certificate) node.getUserObject();
 
-            if (cert.getIssuerX500Principal().equals(nodeCert.getSubjectX500Principal())) {
+            if (X509CertUtil.isIssuedBy(cert, possibleIssuerCert)) {
                 return node;
             }
         }
@@ -478,8 +474,8 @@ private boolean isIssuerInSet(X509Certificate cert, Set<X509Certificate> certSet
             return false;
         }
 
-        for (X509Certificate certToTest : certSet) {
-            if (cert.getIssuerX500Principal().equals(certToTest.getSubjectX500Principal())) {
+        for (X509Certificate possibleIssuer : certSet) {
+            if (X509CertUtil.isIssuedBy(cert, possibleIssuer)) {
                 return true;
             }
         }