Split chain creation and checking

2 files changed, 104 insertions, 87 deletions

diff --git a/src/cert/x509/x509path.cpp b/src/cert/x509/x509path.cpp
index c8266c129..3ed57206d 100644
--- a/src/cert/x509/x509path.cpp
+++ b/src/cert/x509/x509path.cpp
@@ -59,118 +59,125 @@ const X509_CRL* find_crls_from(const X509_Certificate& cert,
    return nullptr;
    }
 
-}
-
-Path_Validation_Result x509_path_validate(
-   const std::vector<X509_Certificate>& end_certs,
-   const Path_Validation_Restrictions& restrictions,
-   const std::vector<Certificate_Store*>& certstores)
+Certificate_Status_Code check_chain(const std::vector<X509_Certificate>& cert_path,
+                                    const Path_Validation_Restrictions& restrictions,
+                                    const std::vector<Certificate_Store*>& certstores)
    {
-   if(end_certs.empty())
-      throw std::invalid_argument("x509_path_validate called with no subjects");
-
-   Path_Validation_Result r;
-
-   r.m_cert_path = end_certs;
+   const std::set<std::string>& trusted_hashes = restrictions.trusted_hashes();
 
-   std::vector<X509_Certificate>& cert_path = r.m_cert_path;
+   const bool self_signed_ee_cert = (cert_path.size() == 1);
 
-   const std::set<std::string>& trusted_hashes = restrictions.trusted_hashes();
+   X509_Time current_time(std::chrono::system_clock::now());
 
-   try
+   for(size_t i = 0; i != cert_path.size(); ++i)
       {
-      // iterate until we reach a root or cannot find the issuer
-      while(!cert_path.back().is_self_signed())
-         {
-         cert_path.push_back(
-            find_issuing_cert(cert_path.back(), certstores)
-            );
-         }
+      const X509_Certificate& subject = cert_path[i];
 
-      const bool self_signed_ee_cert = (cert_path.size() == 1);
+      // Check all certs for valid time range
+      if(current_time < X509_Time(subject.start_time()))
+         return Certificate_Status_Code::CERT_NOT_YET_VALID;
 
-      X509_Time current_time(std::chrono::system_clock::now());
+      if(current_time > X509_Time(subject.end_time()))
+         return Certificate_Status_Code::CERT_HAS_EXPIRED;
 
-      for(size_t i = 0; i != cert_path.size(); ++i)
-         {
-         const X509_Certificate& subject = cert_path[i];
+      const bool at_self_signed_root = (i == cert_path.size() - 1);
 
-         // Check all certs for valid time range
-         if(current_time < X509_Time(subject.start_time()))
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CERT_NOT_YET_VALID);
+      const X509_Certificate& issuer =
+         cert_path[at_self_signed_root ? (i) : (i + 1)];
 
-         if(current_time > X509_Time(subject.end_time()))
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CERT_HAS_EXPIRED);
+      // Check issuer constraints
 
-         const bool at_self_signed_root = (i == cert_path.size() - 1);
+      // Don't require CA bit set on self-signed end entity cert
+      if(!issuer.is_CA_cert() && !self_signed_ee_cert)
+         return Certificate_Status_Code::CA_CERT_NOT_FOR_CERT_ISSUER;
 
-         const X509_Certificate& issuer =
-            cert_path[at_self_signed_root ? (i) : (i + 1)];
+      if(issuer.path_limit() < i)
+         return Certificate_Status_Code::CERT_CHAIN_TOO_LONG;
 
-         // Check issuer constraints
+      std::unique_ptr<Public_Key> issuer_key(issuer.subject_public_key());
 
-         // Don't require CA bit set on self-signed end entity cert
-         if(!issuer.is_CA_cert() && !self_signed_ee_cert)
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CA_CERT_NOT_FOR_CERT_ISSUER);
+      if(subject.check_signature(*issuer_key) == false)
+         return Certificate_Status_Code::SIGNATURE_ERROR;
 
-         if(issuer.path_limit() < i)
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CERT_CHAIN_TOO_LONG);
+      if(issuer_key->estimated_strength() < restrictions.minimum_key_strength())
+         return Certificate_Status_Code::SIGNATURE_METHOD_TOO_WEAK;
 
-         std::unique_ptr<Public_Key> issuer_key(issuer.subject_public_key());
+      if(!trusted_hashes.empty() && !at_self_signed_root)
+         if(!trusted_hashes.count(subject.hash_used_for_signature()))
+            return Certificate_Status_Code::UNTRUSTED_HASH;
+      }
 
-         if(subject.check_signature(*issuer_key) == false)
-            throw PKIX_Validation_Failure(Certificate_Status_Code::SIGNATURE_ERROR);
+   for(size_t i = 1; i != cert_path.size(); ++i)
+      {
+      const X509_Certificate& subject = cert_path[i-1];
+      const X509_Certificate& ca = cert_path[i];
 
-         if(issuer_key->estimated_strength() < restrictions.minimum_key_strength())
-            throw PKIX_Validation_Failure(Certificate_Status_Code::SIGNATURE_METHOD_TOO_WEAK);
+      const X509_CRL* crl_p = find_crls_from(ca, certstores);
 
-         if(!trusted_hashes.empty() && !at_self_signed_root)
-            if(!trusted_hashes.count(subject.hash_used_for_signature()))
-               throw PKIX_Validation_Failure(Certificate_Status_Code::UNTRUSTED_HASH);
+      if(!crl_p)
+         {
+         if(restrictions.require_revocation_information())
+            return Certificate_Status_Code::CRL_NOT_FOUND;
+         continue;
          }
 
-      for(size_t i = 1; i != cert_path.size(); ++i)
-         {
-         const X509_Certificate& subject = cert_path[i-1];
-         const X509_Certificate& ca = cert_path[i];
+      const X509_CRL& crl = *crl_p;
 
-         const X509_CRL* crl_p = find_crls_from(ca, certstores);
+      if(!ca.allowed_usage(CRL_SIGN))
+         return Certificate_Status_Code::CA_CERT_NOT_FOR_CRL_ISSUER;
 
-         if(!crl_p)
-            {
-            if(restrictions.require_revocation_information())
-               throw PKIX_Validation_Failure(Certificate_Status_Code::CRL_NOT_FOUND);
-            continue;
-            }
+      if(current_time < X509_Time(crl.this_update()))
+         return Certificate_Status_Code::CRL_NOT_YET_VALID;
 
-         const X509_CRL& crl = *crl_p;
+      if(current_time > X509_Time(crl.next_update()))
+         return Certificate_Status_Code::CRL_HAS_EXPIRED;
 
-         if(!ca.allowed_usage(CRL_SIGN))
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CA_CERT_NOT_FOR_CRL_ISSUER);
+      if(crl.check_signature(ca.subject_public_key()) == false)
+         return Certificate_Status_Code::SIGNATURE_ERROR;
 
-         if(current_time < X509_Time(crl.this_update()))
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CRL_NOT_YET_VALID);
+      if(crl.is_revoked(subject))
+         return Certificate_Status_Code::CERT_IS_REVOKED;
+      }
 
-         if(current_time > X509_Time(crl.next_update()))
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CRL_HAS_EXPIRED);
+   if(self_signed_ee_cert)
+      return Certificate_Status_Code::CANNOT_ESTABLISH_TRUST;
 
-         if(crl.check_signature(ca.subject_public_key()) == false)
-            throw PKIX_Validation_Failure(Certificate_Status_Code::SIGNATURE_ERROR);
+   return Certificate_Status_Code::VERIFIED;
+   }
+
+}
+
+
+Path_Validation_Result x509_path_validate(
+   const std::vector<X509_Certificate>& end_certs,
+   const Path_Validation_Restrictions& restrictions,
+   const std::vector<Certificate_Store*>& certstores)
+   {
+   if(end_certs.empty())
+      throw std::invalid_argument("x509_path_validate called with no subjects");
+
+   std::vector<X509_Certificate> cert_path = end_certs;
 
-         if(crl.is_revoked(subject))
-            throw PKIX_Validation_Failure(Certificate_Status_Code::CERT_IS_REVOKED);
+   try
+      {
+      // iterate until we reach a root or cannot find the issuer
+      while(!cert_path.back().is_self_signed())
+         {
+         cert_path.push_back(
+            find_issuing_cert(cert_path.back(), certstores)
+            );
          }
 
-      r.set_result(self_signed_ee_cert ?
-                   Certificate_Status_Code::CANNOT_ESTABLISH_TRUST :
-                   Certificate_Status_Code::VERIFIED);
+      Certificate_Status_Code res = check_chain(cert_path, restrictions, certstores);
+
+      return Path_Validation_Result(res, std::move(cert_path));
       }
    catch(PKIX_Validation_Failure& e)
       {
-      r.set_result(e.code());
+      return Path_Validation_Result(e.code());
       }
 
-   return r;
+   return Path_Validation_Result(Certificate_Status_Code::UNKNOWN_X509_ERROR);
    }
 
 Path_Validation_Result x509_path_validate(
@@ -235,9 +242,14 @@ std::set<std::string> Path_Validation_Result::trusted_hashes() const
    return hashes;
    }
 
-std::string Path_Validation_Result::result_string() const
+std::string Path_Validation_Result::result_string() constmtn
+   {
+   return status_string(m_status);
+   }
+
+std::string Path_Validation_Result::status_string(Certificate_Status_Code code)
    {
-   switch(m_result)
+   switch(code)
       {
       case VERIFIED:
          return "verified";
@@ -299,7 +311,7 @@ std::string Path_Validation_Result::result_string() const
       }
 
    // default case
-   return "Unknown code " + std::to_string(m_result);
+   return "Unknown code " + std::to_string(code);
    }
 
 }
diff --git a/src/cert/x509/x509path.h b/src/cert/x509/x509path.h
index a3854ebf3..c935daa77 100644
--- a/src/cert/x509/x509path.h
+++ b/src/cert/x509/x509path.h
@@ -86,30 +86,35 @@ class BOTAN_DLL Path_Validation_Result
       /**
       * @return true iff the validation was succesful
       */
-      bool successful_validation() const { return result() == VERIFIED; }
+      bool successful_validation() const { return status() == VERIFIED; }
 
       /**
       * @return validation result code
       */
-      Certificate_Status_Code result() const { return m_result; }
+      Certificate_Status_Code result() const { return m_status; }
+
+      Certificate_Status_Code status() const { return m_status; }
 
       /**
       * @return string representation of the validation result
       */
       std::string result_string() const;
 
-   private:
-      Path_Validation_Result() : m_result(UNKNOWN_X509_ERROR) {}
+      static std::string status_string(Certificate_Status_Code code);
+
+      Path_Validation_Result(Certificate_Status_Code status,
+                             std::vector<X509_Certificate>&& cert_chain) :
+         m_status(status), m_cert_path(cert_chain) {}
+
+      Path_Validation_Result(Certificate_Status_Code status) : m_status(status) {}
 
+   private:
       friend Path_Validation_Result x509_path_validate(
          const std::vector<X509_Certificate>& end_certs,
          const Path_Validation_Restrictions& restrictions,
          const std::vector<Certificate_Store*>& certstores);
 
-      void set_result(Certificate_Status_Code result) { m_result = result; }
-
-      Certificate_Status_Code m_result;
-
+      Certificate_Status_Code m_status;
       std::vector<X509_Certificate> m_cert_path;
    };