aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--doc/relnotes/1_11_9.rst9
-rw-r--r--src/lib/cert/x509/cert_status.h56
-rw-r--r--src/lib/cert/x509/ocsp.cpp2
-rw-r--r--src/lib/cert/x509/x509path.cpp210
-rw-r--r--src/lib/cert/x509/x509path.h22
-rw-r--r--src/tests/nist_x509.cpp6
6 files changed, 165 insertions, 140 deletions
diff --git a/doc/relnotes/1_11_9.rst b/doc/relnotes/1_11_9.rst
index 8d9f17666..de88987eb 100644
--- a/doc/relnotes/1_11_9.rst
+++ b/doc/relnotes/1_11_9.rst
@@ -1,6 +1,15 @@
Version 1.11.9, Not Yet Released
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ * X.509 path validation now returns a set of all errors that occurred
+ during validation, rather than immediately returning the first
+ detected error. This prevents a seemingly innocuous error (such as
+ an expired certificate) from hiding an obviously serious error
+ (such as an invalid signature). The Certificate_Status_Code enum is
+ now ordered by severity, and the most severe error is returned by
+ Path_Validation_Result::result(). The entire set of status codes is
+ available with the new all_statuses call.
+
* Fixed a bug in OCSP response decoding which would cause an error
when attempting to decode responses from some widely used
responders.
diff --git a/src/lib/cert/x509/cert_status.h b/src/lib/cert/x509/cert_status.h
index d343d2e58..3b80253dc 100644
--- a/src/lib/cert/x509/cert_status.h
+++ b/src/lib/cert/x509/cert_status.h
@@ -8,50 +8,46 @@
#ifndef BOTAN_X509_PATH_RESULT_H__
#define BOTAN_X509_PATH_RESULT_H__
-#include <string>
-
namespace Botan {
-enum Certificate_Status_Code {
- VERIFIED,
- UNKNOWN_X509_ERROR,
- CANNOT_ESTABLISH_TRUST,
- CERT_CHAIN_TOO_LONG,
- SIGNATURE_ERROR,
- POLICY_ERROR,
- INVALID_USAGE,
+enum class Certificate_Status_Code {
+ VERIFIED = 0x00000000,
+ OCSP_RESPONSE_GOOD,
+ NO_REVOCATION_DATA,
- SIGNATURE_METHOD_TOO_WEAK,
+ // Local policy failures
+ SIGNATURE_METHOD_TOO_WEAK = 1000,
UNTRUSTED_HASH,
- CERT_MULTIPLE_ISSUERS_FOUND,
-
- CERT_FORMAT_ERROR,
- CERT_ISSUER_NOT_FOUND,
- CERT_NOT_YET_VALID,
+ // Time problems
+ CERT_NOT_YET_VALID = 2000,
CERT_HAS_EXPIRED,
- CERT_IS_REVOKED,
-
- NO_REVOCATION_DATA,
-
- CRL_FORMAT_ERROR,
+ OCSP_NOT_YET_VALID,
+ OCSP_HAS_EXPIRED,
CRL_NOT_YET_VALID,
CRL_HAS_EXPIRED,
- CRL_NOT_FOUND,
- CRL_BAD_SIGNATURE,
+ // Chain generation problems
+ CERT_ISSUER_NOT_FOUND = 3000,
+ CANNOT_ESTABLISH_TRUST,
+
+ // Validation errors
+ POLICY_ERROR = 4000,
+ INVALID_USAGE,
+ CERT_CHAIN_TOO_LONG,
+ CA_CERT_NOT_FOR_CERT_ISSUER,
+
+ // Revocation errors
+ CA_CERT_NOT_FOR_CRL_ISSUER,
OCSP_CERT_NOT_LISTED,
- OCSP_NOT_YET_VALID,
- OCSP_EXPIRED,
OCSP_BAD_STATUS,
- OCSP_RESPONSE_GOOD,
- CA_CERT_CANNOT_SIGN,
- CA_CERT_NOT_FOR_CERT_ISSUER,
- CA_CERT_NOT_FOR_CRL_ISSUER
+ // Hard failures
+ CERT_IS_REVOKED = 5000,
+ CRL_BAD_SIGNATURE,
+ SIGNATURE_ERROR,
};
-
}
#endif
diff --git a/src/lib/cert/x509/ocsp.cpp b/src/lib/cert/x509/ocsp.cpp
index fe14588e9..fab14fc84 100644
--- a/src/lib/cert/x509/ocsp.cpp
+++ b/src/lib/cert/x509/ocsp.cpp
@@ -210,7 +210,7 @@ Certificate_Status_Code Response::status_for(const X509_Certificate& issuer,
return Certificate_Status_Code::OCSP_NOT_YET_VALID;
if(response.next_update().time_is_set() && current_time > response.next_update())
- return Certificate_Status_Code::OCSP_EXPIRED;
+ return Certificate_Status_Code::OCSP_HAS_EXPIRED;
if(response.cert_status() == 0)
return Certificate_Status_Code::OCSP_RESPONSE_GOOD;
diff --git a/src/lib/cert/x509/x509path.cpp b/src/lib/cert/x509/x509path.cpp
index 317c34718..c1b68e72d 100644
--- a/src/lib/cert/x509/x509path.cpp
+++ b/src/lib/cert/x509/x509path.cpp
@@ -13,6 +13,9 @@
#include <botan/oids.h>
#include <algorithm>
#include <chrono>
+#include <vector>
+#include <set>
+
#include <iostream>
namespace Botan {
@@ -69,9 +72,10 @@ const X509_CRL* find_crls_for(const X509_Certificate& cert,
return nullptr;
}
-Certificate_Status_Code check_chain(const std::vector<X509_Certificate>& cert_path,
- const Path_Validation_Restrictions& restrictions,
- const std::vector<Certificate_Store*>& certstores)
+std::vector<std::set<Certificate_Status_Code>>
+check_chain(const std::vector<X509_Certificate>& cert_path,
+ const Path_Validation_Restrictions& restrictions,
+ const std::vector<Certificate_Store*>& certstores)
{
const std::set<std::string>& trusted_hashes = restrictions.trusted_hashes();
@@ -81,8 +85,12 @@ Certificate_Status_Code check_chain(const std::vector<X509_Certificate>& cert_pa
std::vector<std::future<OCSP::Response>> ocsp_responses;
+ std::vector<std::set<Certificate_Status_Code>> cert_status(cert_path.size());
+
for(size_t i = 0; i != cert_path.size(); ++i)
{
+ std::set<Certificate_Status_Code>& status = cert_status.at(i);
+
const bool at_self_signed_root = (i == cert_path.size() - 1);
const X509_Certificate& subject = cert_path[i];
@@ -98,37 +106,42 @@ Certificate_Status_Code check_chain(const std::vector<X509_Certificate>& cert_pa
// Check all certs for valid time range
if(current_time < X509_Time(subject.start_time()))
- return Certificate_Status_Code::CERT_NOT_YET_VALID;
+ status.insert(Certificate_Status_Code::CERT_NOT_YET_VALID);
if(current_time > X509_Time(subject.end_time()))
- return Certificate_Status_Code::CERT_HAS_EXPIRED;
+ status.insert(Certificate_Status_Code::CERT_HAS_EXPIRED);
// Check issuer constraints
// Don't require CA bit set on self-signed end entity cert
if(!issuer.is_CA_cert() && !self_signed_ee_cert)
- return Certificate_Status_Code::CA_CERT_NOT_FOR_CERT_ISSUER;
+ status.insert(Certificate_Status_Code::CA_CERT_NOT_FOR_CERT_ISSUER);
if(issuer.path_limit() < i)
- return Certificate_Status_Code::CERT_CHAIN_TOO_LONG;
+ status.insert(Certificate_Status_Code::CERT_CHAIN_TOO_LONG);
std::unique_ptr<Public_Key> issuer_key(issuer.subject_public_key());
if(subject.check_signature(*issuer_key) == false)
- return Certificate_Status_Code::SIGNATURE_ERROR;
+ status.insert(Certificate_Status_Code::SIGNATURE_ERROR);
if(issuer_key->estimated_strength() < restrictions.minimum_key_strength())
- return Certificate_Status_Code::SIGNATURE_METHOD_TOO_WEAK;
+ status.insert(Certificate_Status_Code::SIGNATURE_METHOD_TOO_WEAK);
+ // Allow untrusted hashes on self-signed roots
if(!trusted_hashes.empty() && !at_self_signed_root)
+ {
if(!trusted_hashes.count(subject.hash_used_for_signature()))
- return Certificate_Status_Code::UNTRUSTED_HASH;
+ status.insert(Certificate_Status_Code::UNTRUSTED_HASH);
+ }
}
for(size_t i = 0; i != cert_path.size() - 1; ++i)
{
- const X509_Certificate& subject = cert_path[i];
- const X509_Certificate& ca = cert_path[i+1];
+ std::set<Certificate_Status_Code>& status = cert_status.at(i);
+
+ const X509_Certificate& subject = cert_path.at(i);
+ const X509_Certificate& ca = cert_path.at(i+1);
if(i < ocsp_responses.size())
{
@@ -136,21 +149,21 @@ Certificate_Status_Code check_chain(const std::vector<X509_Certificate>& cert_pa
{
OCSP::Response ocsp = ocsp_responses[i].get();
- auto status = ocsp.status_for(ca, subject);
+ auto ocsp_status = ocsp.status_for(ca, subject);
+
+ status.insert(ocsp_status);
- if(status == CERT_IS_REVOKED)
- return status;
+ //std::cout << "OCSP status: " << Path_Validation_Result::status_string(ocsp_status) << "\n";
- if(status == OCSP_RESPONSE_GOOD)
- {
- if(i == 0 && !restrictions.ocsp_all_intermediates())
- return status; // return immediately to just OCSP end cert
- else
- continue;
- }
+ // Either way we have a definitive answer, no need to check CRLs
+ if(ocsp_status == Certificate_Status_Code::CERT_IS_REVOKED)
+ return cert_status;
+ else if(ocsp_status == Certificate_Status_Code::OCSP_RESPONSE_GOOD)
+ continue;
}
catch(std::exception& e)
{
+ //std::cout << "OCSP error: " << e.what() << "\n";
}
}
@@ -158,33 +171,32 @@ Certificate_Status_Code check_chain(const std::vector<X509_Certificate>& cert_pa
if(!crl_p)
{
- if(restrictions.require_revocation_information())
- return Certificate_Status_Code::CRL_NOT_FOUND;
+ status.insert(Certificate_Status_Code::NO_REVOCATION_DATA);
continue;
}
const X509_CRL& crl = *crl_p;
if(!ca.allowed_usage(CRL_SIGN))
- return Certificate_Status_Code::CA_CERT_NOT_FOR_CRL_ISSUER;
+ status.insert(Certificate_Status_Code::CA_CERT_NOT_FOR_CRL_ISSUER);
if(current_time < X509_Time(crl.this_update()))
- return Certificate_Status_Code::CRL_NOT_YET_VALID;
+ status.insert(Certificate_Status_Code::CRL_NOT_YET_VALID);
if(current_time > X509_Time(crl.next_update()))
- return Certificate_Status_Code::CRL_HAS_EXPIRED;
+ status.insert(Certificate_Status_Code::CRL_HAS_EXPIRED);
if(crl.check_signature(ca.subject_public_key()) == false)
- return Certificate_Status_Code::CRL_BAD_SIGNATURE;
+ status.insert(Certificate_Status_Code::CRL_BAD_SIGNATURE);
if(crl.is_revoked(subject))
- return Certificate_Status_Code::CERT_IS_REVOKED;
+ status.insert(Certificate_Status_Code::CERT_IS_REVOKED);
}
if(self_signed_ee_cert)
- return Certificate_Status_Code::CANNOT_ESTABLISH_TRUST;
+ cert_status.back().insert(Certificate_Status_Code::CANNOT_ESTABLISH_TRUST);
- return Certificate_Status_Code::VERIFIED;
+ return cert_status;
}
}
@@ -209,15 +221,11 @@ Path_Validation_Result x509_path_validate(
if(!cert)
return Path_Validation_Result(Certificate_Status_Code::CERT_ISSUER_NOT_FOUND);
- if(cert->path_limit() && (cert->path_limit() < cert_path.size()-1))
- return Path_Validation_Result(Certificate_Status_Code::CERT_CHAIN_TOO_LONG);
-
cert_path.push_back(*cert);
}
- Certificate_Status_Code res = check_chain(cert_path, restrictions, certstores);
-
- return Path_Validation_Result(res, std::move(cert_path));
+ return Path_Validation_Result(check_chain(cert_path, restrictions, certstores),
+ std::move(cert_path));
}
Path_Validation_Result x509_path_validate(
@@ -271,6 +279,25 @@ Path_Validation_Restrictions::Path_Validation_Restrictions(bool require_rev,
m_trusted_hashes.insert("SHA-512");
}
+Path_Validation_Result::Path_Validation_Result(std::vector<std::set<Certificate_Status_Code>> status,
+ std::vector<X509_Certificate>&& cert_chain) :
+ m_overall(Certificate_Status_Code::VERIFIED),
+ m_all_status(status),
+ m_cert_path(cert_chain)
+ {
+ // take the "worst" error as overall
+ for(const auto& s : m_all_status)
+ {
+ if(!s.empty())
+ {
+ auto worst = *s.rbegin();
+ // Leave OCSP confirmations on cert-level status only
+ if(worst != Certificate_Status_Code::OCSP_RESPONSE_GOOD)
+ m_overall = worst;
+ }
+ }
+ }
+
const X509_Certificate& Path_Validation_Result::trust_root() const
{
return m_cert_path[m_cert_path.size()-1];
@@ -286,85 +313,74 @@ std::set<std::string> Path_Validation_Result::trusted_hashes() const
bool Path_Validation_Result::successful_validation() const
{
- if(status() == VERIFIED || status() == OCSP_RESPONSE_GOOD)
+ if(result() == Certificate_Status_Code::VERIFIED ||
+ result() == Certificate_Status_Code::OCSP_RESPONSE_GOOD)
return true;
return false;
}
std::string Path_Validation_Result::result_string() const
{
- return status_string(m_status);
+ return status_string(result());
}
-std::string Path_Validation_Result::status_string(Certificate_Status_Code code)
+const char* Path_Validation_Result::status_string(Certificate_Status_Code code)
{
switch(code)
{
- case VERIFIED:
- return "verified";
- case UNKNOWN_X509_ERROR:
- return "unknown error";
- case CANNOT_ESTABLISH_TRUST:
- return "cannot establish trust";
- case CERT_CHAIN_TOO_LONG:
- return "certificate chain too long";
- case SIGNATURE_ERROR:
- return "signature error";
- case SIGNATURE_METHOD_TOO_WEAK:
- return "signature method too weak";
-
- case POLICY_ERROR:
- return "policy error";
- case INVALID_USAGE:
- return "invalid usage";
- case UNTRUSTED_HASH:
- return "untrusted hash function";
-
- case CERT_MULTIPLE_ISSUERS_FOUND:
- return "Multiple certificate issuers found";
- case CERT_FORMAT_ERROR:
- return "Certificate format error";
- case CERT_ISSUER_NOT_FOUND:
- return "Certificate issuer not found";
- case CERT_NOT_YET_VALID:
+ case Certificate_Status_Code::VERIFIED:
+ return "Verified";
+ case Certificate_Status_Code::OCSP_RESPONSE_GOOD:
+ return "OCSP response good";
+ case Certificate_Status_Code::NO_REVOCATION_DATA:
+ return "No revocation data";
+ case Certificate_Status_Code::SIGNATURE_METHOD_TOO_WEAK:
+ return "Signature method too weak";
+ case Certificate_Status_Code::UNTRUSTED_HASH:
+ return "Untrusted hash";
+
+ case Certificate_Status_Code::CERT_NOT_YET_VALID:
return "Certificate is not yet valid";
- case CERT_HAS_EXPIRED:
+ case Certificate_Status_Code::CERT_HAS_EXPIRED:
return "Certificate has expired";
- case CERT_IS_REVOKED:
- return "Certificate is revoked";
- case NO_REVOCATION_DATA:
- return "No revocation data available";
- case CRL_FORMAT_ERROR:
- return "CRL format error";
- case CRL_NOT_YET_VALID:
+ case Certificate_Status_Code::OCSP_NOT_YET_VALID:
+ return "OCSP is not yet valid";
+ case Certificate_Status_Code::OCSP_HAS_EXPIRED:
+ return "OCSP has expired";
+ case Certificate_Status_Code::CRL_NOT_YET_VALID:
return "CRL is not yet valid";
- case CRL_HAS_EXPIRED:
+ case Certificate_Status_Code::CRL_HAS_EXPIRED:
return "CRL has expired";
- case CRL_NOT_FOUND:
- return "CRL not found";
- case CRL_BAD_SIGNATURE:
- return "CRL has invalid signature";
- case CA_CERT_CANNOT_SIGN:
- return "CA certificate cannot sign";
- case CA_CERT_NOT_FOR_CERT_ISSUER:
+
+ case Certificate_Status_Code::CERT_ISSUER_NOT_FOUND:
+ return "Certificate issuer not found";
+ case Certificate_Status_Code::CANNOT_ESTABLISH_TRUST:
+ return "Cannot establish trust";
+
+ case Certificate_Status_Code::POLICY_ERROR:
+ return "Policy error";
+ case Certificate_Status_Code::INVALID_USAGE:
+ return "Invalid usage";
+ case Certificate_Status_Code::CERT_CHAIN_TOO_LONG:
+ return "Certificate chain too long";
+ case Certificate_Status_Code::CA_CERT_NOT_FOR_CERT_ISSUER:
return "CA certificate not allowed to issue certs";
- case CA_CERT_NOT_FOR_CRL_ISSUER:
+ case Certificate_Status_Code::CA_CERT_NOT_FOR_CRL_ISSUER:
return "CA certificate not allowed to issue CRLs";
+ case Certificate_Status_Code::OCSP_CERT_NOT_LISTED:
+ return "OCSP cert not listed";
+ case Certificate_Status_Code::OCSP_BAD_STATUS:
+ return "OCSP bad status";
- case OCSP_CERT_NOT_LISTED:
- return "OCSP response does not included requested cert";
- case OCSP_NOT_YET_VALID:
- return "OCSP response is from the future";
- case OCSP_EXPIRED:
- return "OCSP response is expired";
- case OCSP_BAD_STATUS:
- return "OCSP response had unknown/bad status";
- case OCSP_RESPONSE_GOOD:
- return "OCSP response had good status";
+ case Certificate_Status_Code::CERT_IS_REVOKED:
+ return "Certificate is revoked";
+ case Certificate_Status_Code::CRL_BAD_SIGNATURE:
+ return "CRL bad signature";
+ case Certificate_Status_Code::SIGNATURE_ERROR:
+ return "Signature error";
+ default:
+ return "Unknown error";
}
-
- // default case
- return "Unknown code " + std::to_string(code);
}
}
diff --git a/src/lib/cert/x509/x509path.h b/src/lib/cert/x509/x509path.h
index d6a41a8f8..f7e57759e 100644
--- a/src/lib/cert/x509/x509path.h
+++ b/src/lib/cert/x509/x509path.h
@@ -99,24 +99,27 @@ class BOTAN_DLL Path_Validation_Result
bool successful_validation() const;
/**
- * @return validation result code
+ * @return overall validation result code
*/
- Certificate_Status_Code result() const { return m_status; }
+ Certificate_Status_Code result() const { return m_overall; }
- Certificate_Status_Code status() const { return m_status; }
+ /**
+ * Return a set of status codes for each certificate in the chain
+ */
+ const std::vector<std::set<Certificate_Status_Code>>& all_statuses() const
+ { return m_all_status; }
/**
* @return string representation of the validation result
*/
std::string result_string() const;
- static std::string status_string(Certificate_Status_Code code);
+ static const char* status_string(Certificate_Status_Code code);
- Path_Validation_Result(Certificate_Status_Code status,
- std::vector<X509_Certificate>&& cert_chain) :
- m_status(status), m_cert_path(cert_chain) {}
+ Path_Validation_Result(std::vector<std::set<Certificate_Status_Code>> status,
+ std::vector<X509_Certificate>&& cert_chain);
- Path_Validation_Result(Certificate_Status_Code status) : m_status(status) {}
+ Path_Validation_Result(Certificate_Status_Code status) : m_overall(status) {}
private:
friend Path_Validation_Result x509_path_validate(
@@ -124,7 +127,8 @@ class BOTAN_DLL Path_Validation_Result
const Path_Validation_Restrictions& restrictions,
const std::vector<Certificate_Store*>& certstores);
- Certificate_Status_Code m_status;
+ Certificate_Status_Code m_overall;
+ std::vector<std::set<Certificate_Status_Code>> m_all_status;
std::vector<X509_Certificate> m_cert_path;
};
diff --git a/src/tests/nist_x509.cpp b/src/tests/nist_x509.cpp
index cf37a3ba7..48483d46a 100644
--- a/src/tests/nist_x509.cpp
+++ b/src/tests/nist_x509.cpp
@@ -201,7 +201,7 @@ std::map<size_t, Path_Validation_Result::Code> get_expected()
expected_results[17] = Certificate_Status_Code::VERIFIED;
expected_results[18] = Certificate_Status_Code::VERIFIED;
- expected_results[19] = Certificate_Status_Code::CRL_NOT_FOUND;
+ expected_results[19] = Certificate_Status_Code::NO_REVOCATION_DATA;
expected_results[20] = Certificate_Status_Code::CERT_IS_REVOKED;
expected_results[21] = Certificate_Status_Code::CERT_IS_REVOKED;
@@ -275,8 +275,8 @@ std::map<size_t, Path_Validation_Result::Code> get_expected()
expected_results[64] = Certificate_Status_Code::CRL_BAD_SIGNATURE;
- expected_results[65] = Certificate_Status_Code::CRL_NOT_FOUND;
- expected_results[66] = Certificate_Status_Code::CRL_NOT_FOUND;
+ expected_results[65] = Certificate_Status_Code::NO_REVOCATION_DATA;
+ expected_results[66] = Certificate_Status_Code::NO_REVOCATION_DATA;
expected_results[67] = Certificate_Status_Code::VERIFIED;