Fix validation of self-issued certificates in chains

Self-issued certificates are certificates where subject_dn == issuer_dn,
but the signature is from a different key (ca key). Chains with such a
certificate could not be verified, because self-issued certificates
(1) would be taken for a self-signed certificate and
(2) find_issuing_cert() would find the same self-issued certificate
that we want to verify, generating a signature error during signature
verification.

To fix, we now first identify a certificate as self-signed only if
subject_dn == issuer_dn AND if we can verify the cert signature
with it's own key. Verification will bring some extra costs, but we
only do it once, in X509_Certificate's constructor. Second, we make
sure find_issuing_cert() does not return the very same certificate
we want to verify. This should be no problem, since path validation
currently does not seem to support validating a self-signed certificate.

3 files changed, 68 insertions, 2 deletions

diff --git a/src/lib/cert/x509/x509cert.cpp b/src/lib/cert/x509/x509cert.cpp
index 8166e43ef..5765214ae 100644
--- a/src/lib/cert/x509/x509cert.cpp
+++ b/src/lib/cert/x509/x509cert.cpp
@@ -102,7 +102,6 @@ void X509_Certificate::force_decode()
    if(m_sig_algo != sig_algo_inner)
       throw Decoding_Error("Algorithm identifier mismatch");
 
-   m_self_signed = (dn_subject == dn_issuer);
 
    m_subject.add(dn_subject.contents());
    m_issuer.add(dn_issuer.contents());
@@ -145,6 +144,9 @@ void X509_Certificate::force_decode()
    m_subject.add("X509.Certificate.public_key",
                hex_encode(public_key.value));
 
+   std::unique_ptr<Public_Key> pub_key(subject_public_key());
+   m_self_signed = (dn_subject == dn_issuer) && check_signature(*pub_key);
+
    if(m_self_signed && version == 0)
       {
       m_subject.add("X509v3.BasicConstraints.is_ca", 1);
diff --git a/src/lib/cert/x509/x509path.cpp b/src/lib/cert/x509/x509path.cpp
index 436e27d39..c08b11d42 100644
--- a/src/lib/cert/x509/x509path.cpp
+++ b/src/lib/cert/x509/x509path.cpp
@@ -28,7 +28,8 @@ find_issuing_cert(const X509_Certificate& cert,
    const X509_DN issuer_dn = cert.issuer_dn();
    const std::vector<byte> auth_key_id = cert.authority_key_id();
 
-   if(const X509_Certificate* c = end_certs.find_cert(issuer_dn, auth_key_id))
+   const X509_Certificate* c = end_certs.find_cert(issuer_dn, auth_key_id);
+   if(c && *c != cert)
       return c;
 
    for(size_t i = 0; i != certstores.size(); ++i)
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index 8449beba4..4313e1373 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -472,6 +472,66 @@ Test::Result test_usage(const std::string& sig_algo, const std::string& hash_fn
    return result;
    }
 
+Test::Result test_self_issued(const std::string& sig_algo, const std::string& hash_fn = "SHA-256")
+   {
+   using Botan::Key_Constraints;
+
+   Test::Result result("X509 Self Issued");
+
+   // create the CA's key and self-signed cert
+   std::unique_ptr<Botan::Private_Key> ca_key(make_a_private_key(sig_algo));
+
+   if(!ca_key)
+      {
+      // Failure because X.509 enabled but requested signature algorithm is not present
+      result.test_note("Skipping due to missing signature algorithm: " + sig_algo);
+      return result;
+      }
+
+   // create the self-signed cert
+   Botan::X509_Certificate ca_cert =
+        Botan::X509::create_self_signed_cert(ca_opts(),
+                                           *ca_key,
+                                           hash_fn,
+                                           Test::rng());
+
+   /* Create the CA object */
+   Botan::X509_CA ca(ca_cert, *ca_key, hash_fn);
+
+   std::unique_ptr<Botan::Private_Key> user_key(make_a_private_key(sig_algo));
+
+   // create a self-issued certificate, that is, a certificate with subject dn == issuer dn,
+   // but signed by a CA, not signed by it's own private key
+   Botan::X509_Cert_Options opts = ca_opts();
+   opts.constraints = Key_Constraints::DIGITAL_SIGNATURE;
+
+   Botan::PKCS10_Request self_issued_req =
+        Botan::X509::create_cert_req(opts,
+                                      *user_key,
+                                      hash_fn,
+                                      Test::rng());
+
+   Botan::X509_Certificate self_issued_cert =
+      ca.sign_request(self_issued_req, Test::rng(),
+                      from_date(2008, 01, 01),
+                      from_date(2033, 01, 01));
+
+   // check that this chain can can be verified successfully
+   Botan::Certificate_Store_In_Memory trusted(ca.ca_certificate());
+
+   Botan::Path_Validation_Restrictions restrictions;
+
+   Botan::Path_Validation_Result validation_result =
+         Botan::x509_path_validate(self_issued_cert,
+                                   restrictions,
+                                   trusted);
+
+   result.confirm("chain with self-issued cert validates", validation_result.successful_validation());
+
+   return result;
+   }
+
+
 using Botan::Key_Constraints;
 
 /**
@@ -629,15 +689,18 @@ class X509_Cert_Unit_Tests : public Test
          const std::vector<std::string> sig_algos { "RSA", "DSA", "ECDSA", "ECGDSA", "ECKCDSA" };
          Test::Result cert_result("X509 Unit");
          Test::Result usage_result("X509 Usage");
+         Test::Result self_issued_result("X509 Self Issued");
 
          for(const auto& algo : sig_algos)
             {
             cert_result.merge(test_x509_cert(algo));
             usage_result.merge(test_usage(algo));
+            self_issued_result.merge(test_self_issued(algo));
             }
 
          results.push_back(cert_result);
          results.push_back(usage_result);
+         results.push_back(self_issued_result);
 
          const std::vector<std::string> pk_algos { "DH", "ECDH", "RSA", "ElGamal", "RW", "NR",
                                                    "DSA", "ECDSA", "ECGDSA", "ECKCDSA" };