Expose a function returning a status code for verifing X509 objects

The versions returning bool just tell us if it could be verified but
don't indicate the problem, everything got binned into "signature error"
during verification. Now in the event that the params were invalid, or
the signature algorithm couldn't be found, report that as a specific error.

See GH #1362

6 files changed, 99 insertions, 59 deletions

diff --git a/src/lib/x509/cert_status.cpp b/src/lib/x509/cert_status.cpp
index 76a102aef..9561cae57 100644
--- a/src/lib/x509/cert_status.cpp
+++ b/src/lib/x509/cert_status.cpp
@@ -91,7 +91,12 @@ const char* to_string(Certificate_Status_Code code)
          return "Signature error";
       case Certificate_Status_Code::CERT_PUBKEY_INVALID:
          return "Certificate public key invalid";
-         // intentionally no default so we are warned
+      case Certificate_Status_Code::SIGNATURE_ALGO_UNKNOWN:
+         return "Certificate signed with unknown/unavailable algorithm";
+      case Certificate_Status_Code::SIGNATURE_ALGO_BAD_PARAMS:
+         return "Certificate signature has invalid parameters";
+
+      // intentionally no default so we are warned if new enum values are added
       }
 
    return nullptr;
diff --git a/src/lib/x509/cert_status.h b/src/lib/x509/cert_status.h
index fb246e71a..0dd9f7b84 100644
--- a/src/lib/x509/cert_status.h
+++ b/src/lib/x509/cert_status.h
@@ -72,6 +72,8 @@ enum class Certificate_Status_Code {
    CRL_BAD_SIGNATURE = 5001,
    SIGNATURE_ERROR = 5002,
    CERT_PUBKEY_INVALID = 5003,
+   SIGNATURE_ALGO_UNKNOWN = 5004,
+   SIGNATURE_ALGO_BAD_PARAMS = 5005
 };
 
 /**
diff --git a/src/lib/x509/x509_obj.cpp b/src/lib/x509/x509_obj.cpp
index 4450df7bb..afc6d3f23 100644
--- a/src/lib/x509/x509_obj.cpp
+++ b/src/lib/x509/x509_obj.cpp
@@ -183,71 +183,85 @@ bool X509_Object::check_signature(const Public_Key* pub_key) const
    return check_signature(*key);
    }
 
-/*
-* Check the signature on an object
-*/
 bool X509_Object::check_signature(const Public_Key& pub_key) const
    {
-   try {
-      std::vector<std::string> sig_info =
-         split_on(OIDS::lookup(m_sig_algo.oid), '/');
+   const Certificate_Status_Code code = verify_signature(pub_key);
+   return (code == Certificate_Status_Code::VERIFIED);
+   }
 
-      if(sig_info.size() != 2 || sig_info[0] != pub_key.algo_name())
-         return false;
+Certificate_Status_Code X509_Object::verify_signature(const Public_Key& pub_key) const
+   {
+   const std::vector<std::string> sig_info =
+      split_on(OIDS::lookup(m_sig_algo.oid), '/');
 
-      std::string padding = sig_info[1];
-      Signature_Format format =
-         (pub_key.message_parts() >= 2) ? DER_SEQUENCE : IEEE_1363;
+   if(sig_info.size() != 2 || sig_info[0] != pub_key.algo_name())
+      return Certificate_Status_Code::SIGNATURE_ALGO_BAD_PARAMS;
 
-      if(padding == "EMSA4")
-         {
-         // "MUST contain RSASSA-PSS-params"
-         if(signature_algorithm().parameters.empty())
-            {
-            return false;
-            }
+   std::string padding = sig_info[1];
+   const Signature_Format format =
+      (pub_key.message_parts() >= 2) ? DER_SEQUENCE : IEEE_1363;
 
-         Pss_params pss_parameter = decode_pss_params(signature_algorithm().parameters);
+   if(padding == "EMSA4")
+      {
+      // "MUST contain RSASSA-PSS-params"
+      if(signature_algorithm().parameters.empty())
+         {
+         return Certificate_Status_Code::SIGNATURE_ALGO_BAD_PARAMS;
+         }
 
-         // hash_algo must be SHA1, SHA2-224, SHA2-256, SHA2-384 or SHA2-512
-         std::string hash_algo = OIDS::lookup(pss_parameter.hash_algo.oid);
-         if(hash_algo != "SHA-160" && hash_algo != "SHA-224" && hash_algo != "SHA-256" && hash_algo != "SHA-384"
-               && hash_algo != "SHA-512")
-            {
-            return false;
-            }
+      Pss_params pss_parameter = decode_pss_params(signature_algorithm().parameters);
 
-         std::string mgf_algo = OIDS::lookup(pss_parameter.mask_gen_algo.oid);
-         if(mgf_algo != "MGF1")
-            {
-            return false;
-            }
+      // hash_algo must be SHA1, SHA2-224, SHA2-256, SHA2-384 or SHA2-512
+      const std::string hash_algo = OIDS::lookup(pss_parameter.hash_algo.oid);
+      if(hash_algo != "SHA-160" &&
+         hash_algo != "SHA-224" &&
+         hash_algo != "SHA-256" &&
+         hash_algo != "SHA-384" &&
+         hash_algo != "SHA-512")
+         {
+         return Certificate_Status_Code::UNTRUSTED_HASH;
+         }
 
-         // For MGF1, it is strongly RECOMMENDED that the underlying hash function be the same as the one identified by hashAlgorithm
-         // Must be SHA1, SHA2-224, SHA2-256, SHA2-384 or SHA2-512
-         if(pss_parameter.mask_gen_hash.oid != pss_parameter.hash_algo.oid)
-            {
-            return false;
-            }
+      const std::string mgf_algo = OIDS::lookup(pss_parameter.mask_gen_algo.oid);
+      if(mgf_algo != "MGF1")
+         {
+         return Certificate_Status_Code::SIGNATURE_ALGO_BAD_PARAMS;
+         }
 
-         if(pss_parameter.trailer_field != 1)
-            {
-            return false;
-            }
+      // For MGF1, it is strongly RECOMMENDED that the underlying hash function be the same as the one identified by hashAlgorithm
+      // Must be SHA1, SHA2-224, SHA2-256, SHA2-384 or SHA2-512
+      if(pss_parameter.mask_gen_hash.oid != pss_parameter.hash_algo.oid)
+         {
+         return Certificate_Status_Code::SIGNATURE_ALGO_BAD_PARAMS;
+         }
 
-         padding += "(" + hash_algo;
-         padding += "," + mgf_algo;
-         padding += "," + std::to_string(pss_parameter.salt_len) +
-                    ")";   // salt_len is actually not used for verification. Length is inferred from the signature
+      if(pss_parameter.trailer_field != 1)
+         {
+         return Certificate_Status_Code::SIGNATURE_ALGO_BAD_PARAMS;
          }
 
+      // salt_len is actually not used for verification. Length is inferred from the signature
+      padding += "(" + hash_algo + "," + mgf_algo + "," + std::to_string(pss_parameter.salt_len) + ")";
+      }
+
+   try
+      {
       PK_Verifier verifier(pub_key, padding, format);
+      const bool valid = verifier.verify_message(tbs_data(), signature());
 
-      return verifier.verify_message(tbs_data(), signature());
+      if(valid)
+         return Certificate_Status_Code::VERIFIED;
+      else
+         return Certificate_Status_Code::SIGNATURE_ERROR;
+      }
+   catch(Algorithm_Not_Found&)
+      {
+      return Certificate_Status_Code::SIGNATURE_ALGO_UNKNOWN;
       }
-   catch(std::exception&)
+   catch(...)
       {
-      return false;
+      // This shouldn't happen, fallback to generic signature error
+      return Certificate_Status_Code::SIGNATURE_ERROR;
       }
    }
 
diff --git a/src/lib/x509/x509_obj.h b/src/lib/x509/x509_obj.h
index c9ff0f761..72ba0f534 100644
--- a/src/lib/x509/x509_obj.h
+++ b/src/lib/x509/x509_obj.h
@@ -10,6 +10,7 @@
 
 #include <botan/asn1_obj.h>
 #include <botan/alg_id.h>
+#include <botan/cert_status.h>
 #include <vector>
 
 namespace Botan {
@@ -59,9 +60,17 @@ class BOTAN_PUBLIC_API(2,0) X509_Object : public ASN1_Object
       * @return signed X509 object
       */
       static std::vector<uint8_t> make_signed(class PK_Signer* signer,
-                                           RandomNumberGenerator& rng,
-                                           const AlgorithmIdentifier& alg_id,
-                                           const secure_vector<uint8_t>& tbs);
+                                              RandomNumberGenerator& rng,
+                                              const AlgorithmIdentifier& alg_id,
+                                              const secure_vector<uint8_t>& tbs);
+
+      /**
+      * Check the signature on this data
+      * @param key the public key purportedly used to sign this data
+      * @return status of the signature - OK if verified or otherwise an indicator of
+      *         the problem preventing verification.
+      */
+      Certificate_Status_Code verify_signature(const Public_Key& key) const;
 
       /**
       * Check the signature on this data
@@ -73,7 +82,8 @@ class BOTAN_PUBLIC_API(2,0) X509_Object : public ASN1_Object
       /**
       * Check the signature on this data
       * @param key the public key purportedly used to sign this data
-      *        the pointer will be deleted after use
+      *        the object will be deleted after use (this should have
+      *        been a std::unique_ptr<Public_Key>)
       * @return true if the signature is valid, otherwise false
       */
       bool check_signature(const Public_Key* key) const;
diff --git a/src/lib/x509/x509cert.cpp b/src/lib/x509/x509cert.cpp
index 1370d52b0..5514f0357 100644
--- a/src/lib/x509/x509cert.cpp
+++ b/src/lib/x509/x509cert.cpp
@@ -257,13 +257,22 @@ std::unique_ptr<X509_Certificate_Data> parse_x509_cert_body(const X509_Object& o
    // Check for self-signed vs self-issued certificates
    if(data->m_subject_dn == data->m_issuer_dn)
       {
+      data->m_self_signed = false;
+
       try
          {
          std::unique_ptr<Public_Key> pub_key(
             X509::load_key(ASN1::put_in_sequence(data->m_subject_public_key_bits)));
-         data->m_self_signed = obj.check_signature(*pub_key);
+
+         Certificate_Status_Code sig_status = obj.verify_signature(*pub_key);
+
+         if(sig_status == Certificate_Status_Code::OK ||
+            sig_status == Certificate_Status_Code::SIGNATURE_ALGO_UNKNOWN)
+            {
+            data->m_self_signed = true;
+            }
          }
-      catch(Decoding_Error&)
+      catch(...)
          {
          // ignore errors here to allow parsing to continue
          }
diff --git a/src/lib/x509/x509path.cpp b/src/lib/x509/x509path.cpp
index 237ac33a5..c2a22e7f4 100644
--- a/src/lib/x509/x509path.cpp
+++ b/src/lib/x509/x509path.cpp
@@ -100,10 +100,10 @@ PKIX::check_chain(const std::vector<std::shared_ptr<const X509_Certificate>>& ce
          }
       else
          {
-         if(subject->check_signature(*issuer_key) == false)
-            {
-            status.insert(Certificate_Status_Code::SIGNATURE_ERROR);
-            }
+         const Certificate_Status_Code sig_status = subject->verify_signature(*issuer_key);
+
+         if(sig_status != Certificate_Status_Code::VERIFIED)
+            status.insert(sig_status);
 
          if(issuer_key->estimated_strength() < min_signature_algo_strength)
             status.insert(Certificate_Status_Code::SIGNATURE_METHOD_TOO_WEAK);