fix pathLenConstraint validation

Fixes GH #991

The problem with the current implementation of the chain validation code is that is runs from the end certificate to the trust cert.
@securitykernel and me tried to fix the pathLenConstraint validation within this reverse loop but we were not sure if we missed some edge cases.

So we felt safer to use the algorithm listed in RFC 5280 which executes from the top to the bottom. It's probably best to rewrite the code to use the whole algorithm from RFC 5280, i.e. validating the chain from the trust to the end cert.

Additionally, we wrote some tests including the one that raised this issue initially.

2 files changed, 34 insertions, 5 deletions

diff --git a/src/lib/x509/x509cert.cpp b/src/lib/x509/x509cert.cpp
index 512e4aa63..40bdbf477 100644
--- a/src/lib/x509/x509cert.cpp
+++ b/src/lib/x509/x509cert.cpp
@@ -334,7 +334,7 @@ bool X509_Certificate::has_ex_constraint(const std::string& ex_constraint) const
 */
 uint32_t X509_Certificate::path_limit() const
    {
-   return m_subject.get1_uint32("X509v3.BasicConstraints.path_constraint", 0);
+   return m_subject.get1_uint32("X509v3.BasicConstraints.path_constraint", Cert_Extension::NO_CERT_PATH_LIMIT);
    }
 
 /*
diff --git a/src/lib/x509/x509path.cpp b/src/lib/x509/x509path.cpp
index 8521e51a7..a06df2460 100644
--- a/src/lib/x509/x509path.cpp
+++ b/src/lib/x509/x509path.cpp
@@ -76,13 +76,9 @@ PKIX::check_chain(const std::vector<std::shared_ptr<const X509_Certificate>>& ce
          status.insert(Certificate_Status_Code::CERT_HAS_EXPIRED);
 
       // Check issuer constraints
-
       if(!issuer->is_CA_cert() && !self_signed_ee_cert)
          status.insert(Certificate_Status_Code::CA_CERT_NOT_FOR_CERT_ISSUER);
 
-      if(issuer->path_limit() < i)
-         status.insert(Certificate_Status_Code::CERT_CHAIN_TOO_LONG);
-
       std::unique_ptr<Public_Key> issuer_key(issuer->subject_public_key());
 
       if(!issuer_key)
@@ -115,6 +111,39 @@ PKIX::check_chain(const std::vector<std::shared_ptr<const X509_Certificate>>& ce
          }
       }
 
+   // path len check
+   size_t max_path_length = cert_path.size();
+   for(size_t i = cert_path.size() - 1; i > 0 ; --i)
+      {
+      std::set<Certificate_Status_Code>& status = cert_status.at(i);
+      const std::shared_ptr<const X509_Certificate>& subject = cert_path[i];
+
+      /*
+      * If the certificate was not self-issued, verify that max_path_length is
+      * greater than zero and decrement max_path_length by 1.
+      */
+      if(subject->subject_dn() != subject->issuer_dn())
+         {
+         if(max_path_length > 0)
+            {
+            --max_path_length;
+            }
+         else
+            {
+            status.insert(Certificate_Status_Code::CERT_CHAIN_TOO_LONG);
+            }
+         }
+
+      /*
+      * If pathLenConstraint is present in the certificate and is less than max_path_length,
+      * set max_path_length to the value of pathLenConstraint.
+      */
+      if(subject->path_limit() != Cert_Extension::NO_CERT_PATH_LIMIT && subject->path_limit() < max_path_length)
+         {
+         max_path_length = subject->path_limit();
+         }
+      }
+
    return cert_status;
    }