Rename find_constraints() and let it throw instead of returning a combination

5 files changed, 218 insertions, 33 deletions

diff --git a/src/lib/cert/x509/key_constraint.cpp b/src/lib/cert/x509/key_constraint.cpp
index f10105f91..a90af013c 100644
--- a/src/lib/cert/x509/key_constraint.cpp
+++ b/src/lib/cert/x509/key_constraint.cpp
@@ -12,29 +12,35 @@
 namespace Botan {
 
 /*
-* Find the allowable key constraints
+* Make sure the given key constraints are permitted for the given key type
 */
-Key_Constraints find_constraints(const Public_Key& pub_key,
-                                 Key_Constraints limits)
+void verify_cert_constraints_valid_for_key_type(const Public_Key& pub_key,
+                                                      Key_Constraints constraints)
    {
    const std::string name = pub_key.algo_name();
 
-   size_t constraints = 0;
+   size_t permitted = 0;
 
    if(name == "DH" || name == "ECDH")
-      constraints |= KEY_AGREEMENT;
+      {
+      permitted |= KEY_AGREEMENT | ENCIPHER_ONLY | DECIPHER_ONLY;
+      }
 
    if(name == "RSA" || name == "ElGamal")
-      constraints |= KEY_ENCIPHERMENT | DATA_ENCIPHERMENT | ENCIPHER_ONLY | DECIPHER_ONLY;
+      {
+      permitted |= KEY_ENCIPHERMENT | DATA_ENCIPHERMENT;
+      }
 
    if(name == "RSA" || name == "RW" || name == "NR" ||
       name == "DSA" || name == "ECDSA" || name == "ECGDSA" || name == "ECKCDSA")
-      constraints |= DIGITAL_SIGNATURE | NON_REPUDIATION | KEY_CERT_SIGN | CRL_SIGN;
-
-   if(limits)
-      constraints &= limits;
-
-   return Key_Constraints(constraints);
+      {
+      permitted |= DIGITAL_SIGNATURE | NON_REPUDIATION | KEY_CERT_SIGN | CRL_SIGN;
+      }
+
+   if ( ( constraints & permitted ) != constraints )
+      {
+      throw Exception("Constraint not permitted for key type " + name);
+      }
    }
 
 }
diff --git a/src/lib/cert/x509/key_constraint.h b/src/lib/cert/x509/key_constraint.h
index 7ae4e26bd..b67eb7010 100644
--- a/src/lib/cert/x509/key_constraint.h
+++ b/src/lib/cert/x509/key_constraint.h
@@ -33,17 +33,13 @@ enum Key_Constraints {
 class Public_Key;
 
 /**
-* Create the key constraints for a specific public key.
-* @param pub_key the public key from which the basic set of
-* constraints to be placed in the return value is derived
-* @param limits additional limits that will be incorporated into the
-* return value
-* @return combination of key type specific constraints and
-* additional limits
+* Check that key constraints are permitted for a specific public key.
+* @param pub_key the public key on which the constraints shall be enforced on
+* @param constrains the constraints that shall be enforced on the key
+* @throw Exception if the given constraints are not permitted for this key
 */
-
-BOTAN_DLL Key_Constraints find_constraints(const Public_Key& pub_key,
-                                           Key_Constraints limits);
+BOTAN_DLL void verify_cert_constraints_valid_for_key_type(const Public_Key& pub_key,
+                                                                Key_Constraints constraints);
 
 }
 
diff --git a/src/lib/cert/x509/x509_ca.cpp b/src/lib/cert/x509/x509_ca.cpp
index d64ade6cd..58c6676f4 100644
--- a/src/lib/cert/x509/x509_ca.cpp
+++ b/src/lib/cert/x509/x509_ca.cpp
@@ -52,11 +52,14 @@ X509_Certificate X509_CA::sign_request(const PKCS10_Request& req,
    {
    Key_Constraints constraints;
    if(req.is_CA())
+      {
       constraints = Key_Constraints(KEY_CERT_SIGN | CRL_SIGN);
+      }
    else
       {
       std::unique_ptr<Public_Key> key(req.subject_public_key());
-      constraints = find_constraints(*key, req.constraints());
+      verify_cert_constraints_valid_for_key_type(*key, req.constraints());
+      constraints = req.constraints();
       }
 
    Extensions extensions;
@@ -65,7 +68,10 @@ X509_Certificate X509_CA::sign_request(const PKCS10_Request& req,
       new Cert_Extension::Basic_Constraints(req.is_CA(), req.path_limit()),
       true);
 
-   extensions.add(new Cert_Extension::Key_Usage(constraints), true);
+   if(constraints != NO_CONSTRAINTS)
+      {
+      extensions.add(new Cert_Extension::Key_Usage(constraints), true);
+      }
 
    extensions.add(new Cert_Extension::Authority_Key_ID(m_cert.subject_key_id()));
    extensions.add(new Cert_Extension::Subject_Key_ID(req.raw_public_key()));
diff --git a/src/lib/cert/x509/x509self.cpp b/src/lib/cert/x509/x509self.cpp
index 8b9aeda09..102e24f77 100644
--- a/src/lib/cert/x509/x509self.cpp
+++ b/src/lib/cert/x509/x509self.cpp
@@ -55,9 +55,14 @@ X509_Certificate create_self_signed_cert(const X509_Cert_Options& opts,
 
    Key_Constraints constraints;
    if(opts.is_CA)
+      {
       constraints = Key_Constraints(KEY_CERT_SIGN | CRL_SIGN);
+      }
    else
-      constraints = find_constraints(key, opts.constraints);
+      {
+      verify_cert_constraints_valid_for_key_type(key, opts.constraints);
+      constraints = opts.constraints;
+      }
 
    Extensions extensions;
 
@@ -65,7 +70,10 @@ X509_Certificate create_self_signed_cert(const X509_Cert_Options& opts,
       new Cert_Extension::Basic_Constraints(opts.is_CA, opts.path_limit),
       true);
 
-   extensions.add(new Cert_Extension::Key_Usage(constraints), true);
+   if(constraints != NO_CONSTRAINTS)
+      {
+      extensions.add(new Cert_Extension::Key_Usage(constraints), true);
+      }
 
    extensions.add(new Cert_Extension::Subject_Key_ID(pub_key));
 
@@ -99,16 +107,27 @@ PKCS10_Request create_cert_req(const X509_Cert_Options& opts,
 
    const size_t PKCS10_VERSION = 0;
 
+   Key_Constraints constraints;
+   if(opts.is_CA)
+      {
+      constraints = Key_Constraints(KEY_CERT_SIGN | CRL_SIGN);
+      }
+   else
+      {
+      verify_cert_constraints_valid_for_key_type(key, opts.constraints);
+      constraints = opts.constraints;
+      }
+
    Extensions extensions;
 
    extensions.add(
       new Cert_Extension::Basic_Constraints(opts.is_CA, opts.path_limit));
-   extensions.add(
-      new Cert_Extension::Key_Usage(
-         opts.is_CA ? Key_Constraints(KEY_CERT_SIGN | CRL_SIGN) :
-                      find_constraints(key, opts.constraints)
-         )
-      );
+
+   if(constraints != NO_CONSTRAINTS)
+      {
+      extensions.add(
+         new Cert_Extension::Key_Usage(constraints));
+      }
    extensions.add(
       new Cert_Extension::Extended_Key_Usage(opts.ex_constraints));
    extensions.add(
diff --git a/src/tests/unit_x509.cpp b/src/tests/unit_x509.cpp
index 5110c7d23..4d3c63a1b 100644
--- a/src/tests/unit_x509.cpp
+++ b/src/tests/unit_x509.cpp
@@ -78,7 +78,7 @@ Botan::X509_Cert_Options req_opts1(const std::string& algo)
 
    if(algo == "RSA")
       {
-      opts.constraints = Botan::Key_Constraints(Botan::DECIPHER_ONLY);
+      opts.constraints = Botan::Key_Constraints(Botan::KEY_ENCIPHERMENT);
       }
    else if(algo == "DSA" || algo == "ECDSA" || algo == "ECGDSA" || algo == "ECKCDSA")
       {
@@ -472,6 +472,153 @@ Test::Result test_usage(const std::string& sig_algo, const std::string& hash_fn
    return result;
    }
 
+using Botan::Key_Constraints;
+
+/**
+* @brief Some typical key usage scenarios (taken from RFC 5280, sec. 4.2.1.3)
+*/
+struct typical_usage_constraints
+   {
+   // ALL constraints are not typical at all, but we use them for a negative test
+   Key_Constraints all = Key_Constraints(
+                           Key_Constraints::DIGITAL_SIGNATURE | Key_Constraints::NON_REPUDIATION | Key_Constraints::KEY_ENCIPHERMENT |
+                           Key_Constraints::DATA_ENCIPHERMENT | Key_Constraints::KEY_AGREEMENT | Key_Constraints::KEY_CERT_SIGN |
+                           Key_Constraints::CRL_SIGN | Key_Constraints::ENCIPHER_ONLY | Key_Constraints::DECIPHER_ONLY);
+
+   Key_Constraints ca = Key_Constraints(Key_Constraints::KEY_CERT_SIGN);
+   Key_Constraints sign_data = Key_Constraints(Key_Constraints::DIGITAL_SIGNATURE);
+   Key_Constraints non_repudiation = Key_Constraints(Key_Constraints::NON_REPUDIATION | Key_Constraints::DIGITAL_SIGNATURE);
+   Key_Constraints key_encipherment = Key_Constraints(Key_Constraints::KEY_ENCIPHERMENT);
+   Key_Constraints data_encipherment = Key_Constraints(Key_Constraints::DATA_ENCIPHERMENT);
+   Key_Constraints key_agreement = Key_Constraints(Key_Constraints::KEY_AGREEMENT);
+   Key_Constraints key_agreement_encipher_only = Key_Constraints(Key_Constraints::KEY_AGREEMENT | Key_Constraints::ENCIPHER_ONLY);
+   Key_Constraints key_agreement_decipher_only = Key_Constraints(Key_Constraints::KEY_AGREEMENT | Key_Constraints::DECIPHER_ONLY);
+   Key_Constraints crl_sign = Key_Constraints(Key_Constraints::CRL_SIGN);
+   Key_Constraints sign_everything = Key_Constraints(Key_Constraints::DIGITAL_SIGNATURE | Key_Constraints::KEY_CERT_SIGN | Key_Constraints::CRL_SIGN);
+   };
+
+
+Test::Result test_valid_constraints(const std::string& pk_algo)
+   {
+   Test::Result result("X509 Valid Constraints");
+
+   std::unique_ptr<Botan::Private_Key> key(make_a_private_key(pk_algo));
+
+   if(!key)
+      {
+      // Failure because X.509 enabled but requested algorithm is not present
+      result.test_note("Skipping due to missing signature algorithm: " + pk_algo);
+      return result;
+      }
+
+   // should not throw on empty constraints
+   verify_cert_constraints_valid_for_key_type(*key, Key_Constraints(Key_Constraints::NO_CONSTRAINTS));
+
+   // now check some typical usage scenarios for the given key type
+   typical_usage_constraints typical_usage;
+
+   if(pk_algo == "DH" || pk_algo == "ECDH")
+      {
+      // DH and ECDH only for key agreement
+      result.test_throws("all constraints not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.all); });
+      result.test_throws("cert sign not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.ca); });
+      result.test_throws("signature not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.sign_data); });
+      result.test_throws("non repudiation not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.non_repudiation); });
+      result.test_throws("key encipherment not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_encipherment); });
+      result.test_throws("data encipherment not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.data_encipherment); });
+
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.key_agreement);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.key_agreement_encipher_only);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.key_agreement_decipher_only);
+
+      result.test_throws("crl sign not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.crl_sign); });
+      result.test_throws("sign, cert sign, crl sign not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.sign_everything); });
+      }
+   else if(pk_algo == "RSA")
+      {
+      // RSA can do everything except key agreement
+      result.test_throws("all constraints not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.all); });
+
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.ca);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.sign_data);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.non_repudiation);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.key_encipherment);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.data_encipherment);
+
+      result.test_throws("key agreement not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement); });
+      result.test_throws("key agreement, encipher only not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement_encipher_only); });
+      result.test_throws("key agreement, decipher only not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement_decipher_only); });
+
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.crl_sign);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.sign_everything);
+      }
+   else if(pk_algo == "ElGamal")
+      {
+      // only ElGamal encryption is currently implemented
+      result.test_throws("all constraints not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.all); });
+      result.test_throws("cert sign not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.ca); });
+
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.non_repudiation);
+
+      result.test_throws("key encipherment not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_encipherment); });
+      result.test_throws("data encipherment not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.data_encipherment); });
+
+      result.test_throws("key agreement not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement); });
+      result.test_throws("key agreement, encipher only not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement_encipher_only); });
+      result.test_throws("key agreement, decipher only not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement_decipher_only); });
+      result.test_throws("crl sign not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.crl_sign); });
+      result.test_throws("sign, cert sign, crl sign not permitted not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.sign_everything); });
+      }
+   else if(pk_algo == "RW" || pk_algo == "NR" || pk_algo == "DSA" ||
+         pk_algo == "ECDSA" || pk_algo == "ECGDSA" || pk_algo == "ECKCDSA")
+      {
+      // these are signature algorithms only
+      result.test_throws("all constraints not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.all); });
+
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.ca);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.sign_data);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.non_repudiation);
+
+      result.test_throws("key encipherment not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_encipherment); });
+      result.test_throws("data encipherment not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.data_encipherment); });
+      result.test_throws("key agreement not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement); });
+      result.test_throws("key agreement, encipher only not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement_encipher_only); });
+      result.test_throws("key agreement, decipher only not permitted", [&key, &typical_usage]() { verify_cert_constraints_valid_for_key_type(*key,
+            typical_usage.key_agreement_decipher_only); });
+
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.crl_sign);
+      verify_cert_constraints_valid_for_key_type(*key, typical_usage.sign_everything);
+      }
+
+   return result;
+   }
+
 
 class X509_Cert_Unit_Tests : public Test
    {
@@ -491,6 +638,17 @@ class X509_Cert_Unit_Tests : public Test
 
          results.push_back(cert_result);
          results.push_back(usage_result);
+
+         const std::vector<std::string> pk_algos { "DH", "ECDH", "RSA", "ElGamal", "RW", "NR",
+                                                   "DSA", "ECDSA", "ECGDSA", "ECKCDSA" };
+         Test::Result valid_constraints_result("X509 Valid Constraints");
+
+         for(const auto& algo : pk_algos)
+            {
+            valid_constraints_result.merge(test_valid_constraints(algo));
+            }
+
+         results.push_back(valid_constraints_result);
          results.push_back(test_x509_dates());
 
          return results;