From d1f0eb5d889c9419d374c02a3ba5f29ebc72b4b8 Mon Sep 17 00:00:00 2001
From: chubtub <43381989+chubtub@users.noreply.github.com>
Date: Fri, 25 Jun 2021 11:39:30 -0400
Subject: [PATCH] Check for an empty truststore during cert path validation.
 Removed the recursion in SupplyChainCredentialValidator.validateCertChain.

---
 .../SupplyChainValidationServiceImpl.java     |  3 ++
 .../SupplyChainCredentialValidator.java       | 53 ++++++++++++-------
 2 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/HIRS_AttestationCA/src/main/java/hirs/attestationca/service/SupplyChainValidationServiceImpl.java b/HIRS_AttestationCA/src/main/java/hirs/attestationca/service/SupplyChainValidationServiceImpl.java
index a80f9800..3c9739f5 100644
--- a/HIRS_AttestationCA/src/main/java/hirs/attestationca/service/SupplyChainValidationServiceImpl.java
+++ b/HIRS_AttestationCA/src/main/java/hirs/attestationca/service/SupplyChainValidationServiceImpl.java
@@ -95,6 +95,7 @@ public class SupplyChainValidationServiceImpl implements SupplyChainValidationSe
     /**
      * Constructor to set just the CertificateManager, so that cert chain validating
      * methods can be called from outside classes.
+     * @param certificateManager    the cert manager
      */
     public SupplyChainValidationServiceImpl(final CertificateManager certificateManager) {
         this.certificateManager = certificateManager;
@@ -446,6 +447,8 @@ public class SupplyChainValidationServiceImpl implements SupplyChainValidationSe
                         LOGGER.error("Error getting X509 cert from manager: " + e.getMessage());
                     } catch (SupplyChainValidatorException e) {
                         LOGGER.error("Error validating cert against keystore: " + e.getMessage());
+                        fwStatus = new AppraisalStatus(FAIL,
+                                "Firmware validation failed: invalid certificate path.");
                     }
                     break;
                 }
diff --git a/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java b/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java
index 1d449147..3bf33049 100644
--- a/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java
+++ b/HIRS_Utils/src/main/java/hirs/validation/SupplyChainCredentialValidator.java
@@ -1249,8 +1249,14 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
      */
     public static String verifyCertificate(final X509AttributeCertificateHolder cert,
             final KeyStore trustStore) throws SupplyChainValidatorException {
-        if (cert == null || trustStore == null) {
-            throw new SupplyChainValidatorException("Certificate or trust store is null");
+        try {
+            if (cert == null || trustStore == null) {
+                throw new SupplyChainValidatorException("Certificate or trust store is null");
+            } else if (trustStore.size() == 0) {
+                throw new SupplyChainValidatorException("Truststore is empty");
+            }
+        } catch (KeyStoreException e) {
+            LOGGER.error("Error accessing trust store: " + e.getMessage());
         }
 
         try {
@@ -1289,9 +1295,16 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
      */
     public static boolean verifyCertificate(final X509Certificate cert,
             final KeyStore trustStore) throws SupplyChainValidatorException {
-        if (cert == null || trustStore == null) {
-            throw new SupplyChainValidatorException("Certificate or trust store is null");
+        try {
+            if (cert == null || trustStore == null) {
+                throw new SupplyChainValidatorException("Certificate or trust store is null");
+            } else if (trustStore.size() == 0) {
+                throw new SupplyChainValidatorException("Truststore is empty");
+            }
+        } catch (KeyStoreException e) {
+            LOGGER.error("Error accessing trust store: " + e.getMessage());
         }
+
         try {
             Set<X509Certificate> trustedCerts = new HashSet<>();
 
@@ -1320,7 +1333,8 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
      *            certificate to validate
      * @param additionalCerts
      *            Set of certs to validate against
-     * @return boolean indicating if the validation was successful
+     * @return String status of the cert chain validation -
+     *  blank if successful, error message otherwise
      * @throws SupplyChainValidatorException tried to validate using null certificates
      */
     public static String validateCertChain(final X509AttributeCertificateHolder cert,
@@ -1341,14 +1355,12 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
             signatureMatchesPublicKey = signatureMatchesPublicKey(cert, trustedCert);
             if (issuerMatchesSubject && signatureMatchesPublicKey) {
                 if (isSelfSigned(trustedCert)) {
+                    foundRootOfCertChain = "";
                     LOGGER.info("CA Root found.");
+                    break;
                 } else {
-                    foundRootOfCertChain = validateCertChain(trustedCert, additionalCerts);
-
-                    if (!foundRootOfCertChain.isEmpty()) {
-                        LOGGER.error("Root of certificate chain not found. Check for CA Cert: "
-                                + cert.getIssuer().getNames()[0]);
-                    }
+                    foundRootOfCertChain = "Intermediate signing cert found. Check for CA Cert: "
+                            + cert.getIssuer().getNames()[0];
                 }
             } else {
                 if (!issuerMatchesSubject) {
@@ -1360,6 +1372,9 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
             }
         }
 
+        if (!foundRootOfCertChain.isEmpty()) {
+            LOGGER.error(foundRootOfCertChain);
+        }
         return foundRootOfCertChain;
     }
 
@@ -1374,7 +1389,8 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
      *            certificate to validate
      * @param additionalCerts
      *            Set of certs to validate against
-     * @return boolean indicating if the validation was successful
+     * @return String status of the cert chain validation -
+     *  blank if successful, error message otherwise
      * @throws SupplyChainValidatorException tried to validate using null certificates
      */
     public static String validateCertChain(final X509Certificate cert,
@@ -1395,14 +1411,12 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
             signatureMatchesPublicKey = signatureMatchesPublicKey(cert, trustedCert);
             if (issuerMatchesSubject && signatureMatchesPublicKey) {
                 if (isSelfSigned(trustedCert)) {
+                    foundRootOfCertChain = "";
                     LOGGER.info("CA Root found.");
+                    break;
                 } else if (!cert.equals(trustedCert)) {
-                    foundRootOfCertChain = validateCertChain(trustedCert, additionalCerts);
-
-                    if (!foundRootOfCertChain.isEmpty()) {
-                        LOGGER.error("Root of certificate chain not found. Check for CA Cert: "
-                                + cert.getIssuerDN().getName());
-                    }
+                    foundRootOfCertChain = "Intermediate signing cert found, check for CA cert "
+                            + cert.getIssuerDN().getName();
                 }
             } else {
                 if (!issuerMatchesSubject) {
@@ -1414,6 +1428,9 @@ public final class SupplyChainCredentialValidator implements CredentialValidator
             }
         }
 
+        if (!foundRootOfCertChain.isEmpty()) {
+            LOGGER.error(foundRootOfCertChain);
+        }
         return foundRootOfCertChain;
     }