Fix encoding of subject key identifier

Changed in #884 - we were copying the entire public key as the
public key id. Instead hash it with whatever hash we are using
to sign the certificate.

5 files changed, 27 insertions, 23 deletions

diff --git a/src/lib/x509/x509_ca.cpp b/src/lib/x509/x509_ca.cpp
index 682002111..6569f506b 100644
--- a/src/lib/x509/x509_ca.cpp
+++ b/src/lib/x509/x509_ca.cpp
@@ -75,7 +75,7 @@ X509_Certificate X509_CA::sign_request(const PKCS10_Request& req,
       }
 
    extensions.replace(new Cert_Extension::Authority_Key_ID(m_ca_cert.subject_key_id()));
-   extensions.replace(new Cert_Extension::Subject_Key_ID(req.raw_public_key()));
+   extensions.replace(new Cert_Extension::Subject_Key_ID(req.raw_public_key(), m_hash_fn));
 
    extensions.replace(
       new Cert_Extension::Subject_Alternative_Name(req.subject_alt_name()));
diff --git a/src/lib/x509/x509_ext.cpp b/src/lib/x509/x509_ext.cpp
index a7e3c8f61..1b13d36e1 100644
--- a/src/lib/x509/x509_ext.cpp
+++ b/src/lib/x509/x509_ext.cpp
@@ -594,6 +594,9 @@ void Name_Constraints::validate(const X509_Certificate& subject, const X509_Cert
       if(!subject.is_CA_cert() || !subject.is_critical("X509v3.NameConstraints"))
          cert_status.at(pos).insert(Certificate_Status_Code::NAME_CONSTRAINT_ERROR);
 
+      const bool issuer_name_constraint_critical =
+         issuer.is_critical("X509v3.NameConstraints");
+
       const bool at_self_signed_root = (pos == cert_path.size() - 1);
 
       // Check that all subordinate certs pass the name constraint
@@ -609,16 +612,16 @@ void Name_Constraints::validate(const X509_Certificate& subject, const X509_Cert
             {
             switch(c.base().matches(*cert_path.at(j)))
                {
-            case GeneralName::MatchResult::NotFound:
-            case GeneralName::MatchResult::All:
-               permitted = true;
-               break;
-            case GeneralName::MatchResult::UnknownType:
-               failed = issuer.is_critical("X509v3.NameConstraints");
-               permitted = true;
-               break;
-            default:
-               break;
+               case GeneralName::MatchResult::NotFound:
+               case GeneralName::MatchResult::All:
+                  permitted = true;
+                  break;
+               case GeneralName::MatchResult::UnknownType:
+                  failed = issuer_name_constraint_critical;
+                  permitted = true;
+                  break;
+               default:
+                  break;
                }
             }
 
@@ -626,15 +629,15 @@ void Name_Constraints::validate(const X509_Certificate& subject, const X509_Cert
             {
             switch(c.base().matches(*cert_path.at(j)))
                {
-            case GeneralName::MatchResult::All:
-            case GeneralName::MatchResult::Some:
-               failed = true;
-               break;
-            case GeneralName::MatchResult::UnknownType:
-               failed = issuer.is_critical("X509v3.NameConstraints");
-               break;
-            default:
-               break;
+               case GeneralName::MatchResult::All:
+               case GeneralName::MatchResult::Some:
+                  failed = true;
+                  break;
+               case GeneralName::MatchResult::UnknownType:
+                  failed = issuer_name_constraint_critical;
+                  break;
+               default:
+                  break;
                }
             }
 
diff --git a/src/lib/x509/x509_ext.h b/src/lib/x509/x509_ext.h
index 2cc0115ff..1680bd9dd 100644
--- a/src/lib/x509/x509_ext.h
+++ b/src/lib/x509/x509_ext.h
@@ -336,8 +336,6 @@ class BOTAN_PUBLIC_API(2,0) Subject_Key_ID final : public Certificate_Extension
    public:
       Subject_Key_ID() = default;
 
-      explicit Subject_Key_ID(const std::vector<uint8_t>& k) : m_key_id(k) {}
-
       Subject_Key_ID(const std::vector<uint8_t>& public_key,
                      const std::string& hash_fn);
 
@@ -350,6 +348,8 @@ class BOTAN_PUBLIC_API(2,0) Subject_Key_ID final : public Certificate_Extension
       OID oid_of() const override { return static_oid(); }
 
    private:
+      explicit Subject_Key_ID(const std::vector<uint8_t>& k) : m_key_id(k) {}
+
       std::string oid_name() const override
          { return "X509v3.SubjectKeyIdentifier"; }
 
diff --git a/src/lib/x509/x509cert.cpp b/src/lib/x509/x509cert.cpp
index 6814f54c1..74bd17811 100644
--- a/src/lib/x509/x509cert.cpp
+++ b/src/lib/x509/x509cert.cpp
@@ -771,6 +771,7 @@ std::string X509_Certificate::to_string() const
       }
 
    NameConstraints name_constraints = this->name_constraints();
+
    if(!name_constraints.permitted().empty() ||
          !name_constraints.excluded().empty())
       {
diff --git a/src/lib/x509/x509self.cpp b/src/lib/x509/x509self.cpp
index b8f8fbdc8..ad0e9af94 100644
--- a/src/lib/x509/x509self.cpp
+++ b/src/lib/x509/x509self.cpp
@@ -76,7 +76,7 @@ X509_Certificate create_self_signed_cert(const X509_Cert_Options& opts,
       extensions.add(new Cert_Extension::Key_Usage(constraints), true);
       }
 
-   extensions.add(new Cert_Extension::Subject_Key_ID(pub_key));
+   extensions.add(new Cert_Extension::Subject_Key_ID(pub_key, hash_fn));
 
    extensions.add(
       new Cert_Extension::Subject_Alternative_Name(subject_alt));