Add verify_errors as public error API Bug: 660, b:323560158 Change-Id: I1154fb848de28fd0417660cce1f99e3d29107840 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66327 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/include/openssl/pki/verify_error.h b/include/openssl/pki/verify_error.h new file mode 100644 index 0000000..34b5dc5 --- /dev/null +++ b/include/openssl/pki/verify_error.h
@@ -0,0 +1,137 @@ +/* Copyright (c) 2024, Google Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ + +#if !defined(OPENSSL_HEADER_BSSL_PKI_VERIFY_ERROR_H_) && defined(__cplusplus) +#define OPENSSL_HEADER_BSSL_PKI_VERIFY_ERROR_H_ + +#include <string> +#include <string_view> + +namespace bssl { + +// VerifyError describes certificate chain validation result. +class OPENSSL_EXPORT VerifyError { + public: + VerifyError() = default; + VerifyError(const VerifyError &other) = default; + VerifyError &operator=(const VerifyError &other) = default; + + // Code is the representation of a single error that we could + // find. + enum class StatusCode { + // PATH_VERIFIED means there were no errors, the certificate chain is valid. + PATH_VERIFIED, + + // CERTIFICATE_INVALID_SIGNATURE means that the certificate's signature + // failed to verify. + CERTIFICATE_INVALID_SIGNATURE, + + // CERTIFICATE_UNSUPPORTED_KEY means that the certificate's key type and/or + // size is not supported. + CERTIFICATE_UNSUPPORTED_KEY, + + // CERTIFICATE_UNSUPPORTED_SIGNATURE ALGORITHM means that the signature + // algorithm is not supported. + CERTIFICATE_UNSUPPORTED_SIGNATURE_ALGORITHM, + + // CERTIFICATE_REVOKED means that the certificate has been revoked. + CERTIFICATE_REVOKED, + + // CERTIFICATE_NO_REVOCATION_MECHANISM means that revocation checking was + // required and no revocation mechanism was given for the certificate + CERTIFICATE_NO_REVOCATION_MECHANISM, + + // CERTIFICATE_UNABLE_TO_CHECK_REVOCATION means that revocation checking was + // required and we were unable to check if the certificate was revoked via + // any revocation mechanism. + CERTIFICATE_UNABLE_TO_CHECK_REVOCATION, + + // CERTIFICATE_EXPIRED means that the validation time is after the + // certificate's |notAfter| timestamp. + CERTIFICATE_EXPIRED, + + // CERTIFICATE_NOT_YET_VALID means that the validation time is before the + // certificate's |notBefore| timestamp. + CERTIFICATE_NOT_YET_VALID, + + // CERTIFICATE_NO_MATCHING_EKU means that the certificate's EKU does not + // allow the certificate to be used for the intended purpose. + CERTIFICATE_NO_MATCHING_EKU, + + // CERTIFICATE_INVALID means that the certificate was structurally + // invalid, or invalid for some different reason than the above. + CERTIFICATE_INVALID, + + // PATH_NOT_FOUND means that no path could be found from the leaf + // certificate to any trust anchor. + PATH_NOT_FOUND, + + // PATH_ITERATION_COUNT_EXCEEDED means that the iteration limit for path + // building was hit and so the search for a valid path terminated early. + PATH_ITERATION_COUNT_EXCEEDED, + + // PATH_DEADLINE_EXCEEDED means that the time limit for path building + // was hit and so the search for a valid path terminated early. + PATH_DEADLINE_EXCEEDED, + + // PATH_DEPTH_LIMIT_REACHED means that path building was not able to find a + // path within the configured depth limit for verification. + PATH_DEPTH_LIMIT_REACHED, + + // PATH_MULTIPLE_ERRORS indicates that there are multiple fatal + // errors present on the certificate chain, so that a single error could + // not be reported. + PATH_MULTIPLE_ERRORS, + + // VERIFICATION_FAILURE means that something is wrong with the returned path + // that is not specific to a single certificate. There are many possible + // reasons for a verification to fail. + VERIFICATION_FAILURE, + }; + + VerifyError(StatusCode code, ptrdiff_t offset, std::string diagnostic); + + // Code returns the indicated error code for the certificate path. + StatusCode Code() const; + + // Index returns the certificate in the chain for which the error first + // occured, starting with 0 for the leaf certificate. Later certificates in + // the chain may also exhibit the same error. If the error is not specific to + // a certificate, -1 is returned. + ptrdiff_t Index() const; + + // DiagnosticString returns a string of diagnostic information related to this + // verification attempt. The string aims to be useful to debugging, but it is + // not stable and may not be processed programmatically or asserted on in + // tests. The string may be empty if no diagnostic information was available. + // + // The DiagnosticString is specifically not guaranteed to be unchanging for + // any given error code, as the diagnostic error message can contain + // information specific to the verification attempt and chain presented, due + // to there being multiple possible ways for, as an example, a certificate to + // be invalid, or that we are unable to build a path to a trust anchor. + // + // Needless to say, one should not attempt to parse the string that is + // returned. + const std::string &DiagnosticString() const; + + private: + ptrdiff_t offset_ = -1; + StatusCode code_ = StatusCode::VERIFICATION_FAILURE; + std::string diagnostic_; +}; + +} // namespace bssl + +#endif // OPENSSL_HEADER_BSSL_PKI_VERIFY_ERROR_H_
diff --git a/pki/cert_errors.cc b/pki/cert_errors.cc index 3f5a77f..f54903e 100644 --- a/pki/cert_errors.cc +++ b/pki/cert_errors.cc
@@ -139,6 +139,10 @@ CertErrors *CertPathErrors::GetOtherErrors() { return &other_errors_; } +const CertErrors *CertPathErrors::GetOtherErrors() const { + return &other_errors_; +} + bool CertPathErrors::ContainsError(CertErrorId id) const { for (const CertErrors &errors : cert_errors_) { if (errors.ContainsError(id)) { @@ -168,6 +172,28 @@ return false; } +std::optional<CertErrorId> CertPathErrors::FindSingleHighSeverityError( + ptrdiff_t &out_depth) const { + std::optional<CertErrorId> id_seen; + for (ptrdiff_t i = -1; i < (ptrdiff_t)cert_errors_.size(); ++i) { + const CertErrors *errors = + (i < 0) ? GetOtherErrors() : GetErrorsForCert(i); + for (const CertError &node : errors->nodes_) { + if (node.severity == CertError::SEVERITY_HIGH) { + if (!id_seen.has_value()) { + id_seen = node.id; + out_depth = i; + } else { + if (id_seen.value() != node.id) { + return {}; + } + } + } + } + } + return id_seen; +} + std::string CertPathErrors::ToDebugString( const ParsedCertificateList &certs) const { std::ostringstream result;
diff --git a/pki/cert_errors.h b/pki/cert_errors.h index da35060..43ea7fc 100644 --- a/pki/cert_errors.h +++ b/pki/cert_errors.h
@@ -57,6 +57,7 @@ namespace bssl { class CertErrorParams; +class CertPathErrors; // CertError represents either an error or a warning. struct OPENSSL_EXPORT CertError { @@ -117,6 +118,7 @@ bool ContainsAnyErrorWithSeverity(CertError::Severity severity) const; private: + friend CertPathErrors; std::vector<CertError> nodes_; }; @@ -144,6 +146,7 @@ // Returns a bucket to put errors that are not associated with a particular // certificate. CertErrors *GetOtherErrors(); + const CertErrors *GetOtherErrors() const; // Returns true if CertPathErrors contains the specified error (of any // severity). @@ -152,6 +155,13 @@ // Returns true if this contains any errors of the given severity level. bool ContainsAnyErrorWithSeverity(CertError::Severity severity) const; + // If the path contains only one unique high severity error, return the + // error id and sets |out_depth| to the depth at which the error was + // first seen. A depth of -1 means the error is not associated with + // a single certificate of the path. + std::optional<CertErrorId> FindSingleHighSeverityError( + ptrdiff_t &out_depth) const; + // Shortcut for ContainsAnyErrorWithSeverity(CertError::SEVERITY_HIGH). bool ContainsHighSeverityErrors() const { return ContainsAnyErrorWithSeverity(CertError::SEVERITY_HIGH);
diff --git a/pki/path_builder.cc b/pki/path_builder.cc index a7fddd3..ef23172 100644 --- a/pki/path_builder.cc +++ b/pki/path_builder.cc
@@ -10,6 +10,7 @@ #include <unordered_set> #include <openssl/base.h> +#include <openssl/pki/verify_error.h> #include <openssl/sha.h> #include "cert_issuer_source.h" @@ -710,6 +711,121 @@ return GetTrustedCert() && !errors.ContainsHighSeverityErrors(); } +VerifyError CertPathBuilderResultPath::GetVerifyError() const { + // Diagnostic string is always "everything" about the path. + std::string diagnostic = errors.ToDebugString(certs); + if (!errors.ContainsHighSeverityErrors()) { + // TODO(bbe3): Having to check this after seems awkward: crbug.com/boringssl/713 + if (GetTrustedCert()) { + return VerifyError(VerifyError::StatusCode::PATH_VERIFIED, 0, + std::move(diagnostic)); + } else { + return VerifyError(VerifyError::StatusCode::VERIFICATION_FAILURE, -1, + std::move(diagnostic)); + } + } + + // Check for the presence of things that amount to Internal errors in the + // verification code. We deliberately prioritize this to not hide it in + // multiple error cases. + if (errors.ContainsError(cert_errors::kInternalError) || + errors.ContainsError(cert_errors::kChainIsEmpty)) { + return VerifyError(VerifyError::StatusCode::VERIFICATION_FAILURE, -1, + std::move(diagnostic)); + } + + // Similarly, for the deadline and limit cases, there will often be other + // errors that we probably do not care about, since path building was + // aborted. Surface these errors instead of having them hidden in the multiple + // error case. + // + // Normally callers should check for these in the path builder result before + // calling this on a single path, but this is here in case they do not and + // these errors are actually present on this path. + if (errors.ContainsError(cert_errors::kDeadlineExceeded)) { + return VerifyError(VerifyError::StatusCode::PATH_DEADLINE_EXCEEDED, -1, + std::move(diagnostic)); + } + if (errors.ContainsError(cert_errors::kIterationLimitExceeded)) { + return VerifyError(VerifyError::StatusCode::PATH_ITERATION_COUNT_EXCEEDED, + -1, std::move(diagnostic)); + } + if (errors.ContainsError(cert_errors::kDepthLimitExceeded)) { + return VerifyError(VerifyError::StatusCode::PATH_DEPTH_LIMIT_REACHED, -1, + std::move(diagnostic)); + } + + // If the chain has multiple high severity errors, indicate that. + ptrdiff_t depth = -1; + std::optional<CertErrorId> single_error = + errors.FindSingleHighSeverityError(depth); + if (!single_error.has_value()) { + return VerifyError(VerifyError::StatusCode::PATH_MULTIPLE_ERRORS, -1, + std::move(diagnostic)); + } + + // Otherwise it has a single error, map it appropriately at the + // depth it first occurs. + if (single_error.value() == cert_errors::kValidityFailedNotAfter) { + return VerifyError(VerifyError::StatusCode::CERTIFICATE_EXPIRED, depth, + std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kValidityFailedNotBefore) { + return VerifyError(VerifyError::StatusCode::CERTIFICATE_NOT_YET_VALID, + depth, std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kDistrustedByTrustStore || + single_error.value() == cert_errors::kCertIsNotTrustAnchor || + single_error.value() == cert_errors::kMaxPathLengthViolated || + single_error.value() == cert_errors::kSubjectDoesNotMatchIssuer || + single_error.value() == cert_errors::kNoIssuersFound) { + return VerifyError(VerifyError::StatusCode::PATH_NOT_FOUND, depth, + std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kVerifySignedDataFailed) { + return VerifyError(VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE, + depth, std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kUnacceptableSignatureAlgorithm) { + return VerifyError( + VerifyError::StatusCode::CERTIFICATE_UNSUPPORTED_SIGNATURE_ALGORITHM, + depth, std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kUnacceptablePublicKey) { + return VerifyError(VerifyError::StatusCode::CERTIFICATE_UNSUPPORTED_KEY, + depth, std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kEkuLacksServerAuth || + single_error.value() == cert_errors::kEkuLacksServerAuthButHasAnyEKU || + single_error.value() == cert_errors::kEkuLacksClientAuth || + single_error.value() == cert_errors::kEkuLacksClientAuthButHasAnyEKU || + single_error.value() == cert_errors::kEkuLacksClientAuthOrServerAuth) { + return VerifyError(VerifyError::StatusCode::CERTIFICATE_NO_MATCHING_EKU, + depth, std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kCertificateRevoked) { + return VerifyError(VerifyError::StatusCode::CERTIFICATE_REVOKED, depth, + std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kNoRevocationMechanism) { + return VerifyError( + VerifyError::StatusCode::CERTIFICATE_NO_REVOCATION_MECHANISM, depth, + std::move(diagnostic)); + } + if (single_error.value() == cert_errors::kUnableToCheckRevocation) { + return VerifyError( + VerifyError::StatusCode::CERTIFICATE_UNABLE_TO_CHECK_REVOCATION, depth, + std::move(diagnostic)); + } + // All other High severity errors map to CERTIFICATE_INVALID if associated + // to a certificate, or VERIFICATION_FAILURE if not associated to a + // certificate. + return VerifyError((depth < 0) ? VerifyError::StatusCode::VERIFICATION_FAILURE + : VerifyError::StatusCode::CERTIFICATE_INVALID, + depth, std::move(diagnostic)); +} + + CertPathBuilder::Result::Result() = default; CertPathBuilder::Result::Result(Result &&) = default; CertPathBuilder::Result::~Result() = default; @@ -730,6 +846,39 @@ return false; } +const VerifyError CertPathBuilder::Result::GetBestPathVerifyError() const { + if (HasValidPath()) { + return GetBestValidPath()->GetVerifyError(); + } + // We can only return one error. Returning the errors corresponding to the + // limits if they they appear on any path will make this error prominent even + // if there are other paths with different or multiple errors. + if (exceeded_iteration_limit) { + return VerifyError( + VerifyError::StatusCode::PATH_ITERATION_COUNT_EXCEEDED, -1, + "Iteration count exceeded, could not find a trusted path."); + } + if (exceeded_deadline) { + return VerifyError(VerifyError::StatusCode::PATH_DEADLINE_EXCEEDED, -1, + "Deadline exceeded. Could not find a trusted path."); + } + if (AnyPathContainsError(cert_errors::kDepthLimitExceeded)) { + return VerifyError(VerifyError::StatusCode::PATH_DEPTH_LIMIT_REACHED, -1, + "Depth limit reached. Could not find a trusted path."); + } + + // If there are no paths to report an error on, this probably indicates + // something is wrong with this path builder result. + if (paths.empty()) { + return VerifyError(VerifyError::StatusCode::VERIFICATION_FAILURE, -1, + "No paths in path builder result."); + } + + // If there are paths, report the VerifyError from the best path. + CertPathBuilderResultPath *path = paths[best_result_index].get(); + return path->GetVerifyError(); +} + const CertPathBuilderResultPath *CertPathBuilder::Result::GetBestValidPath() const { const CertPathBuilderResultPath *result_path = GetBestPathPossiblyInvalid();
diff --git a/pki/path_builder.h b/pki/path_builder.h index 05999b0..fd207f7 100644 --- a/pki/path_builder.h +++ b/pki/path_builder.h
@@ -9,6 +9,7 @@ #include <vector> #include <openssl/base.h> +#include <openssl/pki/verify_error.h> #include "cert_errors.h" #include "input.h" @@ -49,6 +50,9 @@ // to it during certificate verification. bool IsValid() const; + // Public verify error result for this candidate path. + VerifyError GetVerifyError() const; + // Returns the chain's root certificate or nullptr if the chain doesn't // chain to a trust anchor. const ParsedCertificate *GetTrustedCert() const; @@ -139,6 +143,9 @@ // Returns true if any of the attempted paths contain |error_id|. bool AnyPathContainsError(CertErrorId error_id) const; + // Returns the best single error from result, using the best path found. + const VerifyError GetBestPathVerifyError() const; + // Returns the CertPathBuilderResultPath for the best valid path, or nullptr // if there was none. const CertPathBuilderResultPath *GetBestValidPath() const;
diff --git a/pki/path_builder_unittest.cc b/pki/path_builder_unittest.cc index 624c0bc..05d9619 100644 --- a/pki/path_builder_unittest.cc +++ b/pki/path_builder_unittest.cc
@@ -230,6 +230,9 @@ auto result = path_builder.Run(); ASSERT_TRUE(result.HasValidPath()); + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); const auto &path = *result.GetBestValidPath(); ASSERT_EQ(2U, path.certs.size()); EXPECT_EQ(a_by_b_, path.certs[0]); @@ -252,6 +255,9 @@ EXPECT_FALSE(result.HasValidPath()); EXPECT_EQ(1U, result.max_depth_seen); + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } // Test a failed path building when the trust anchor is provided as a @@ -291,6 +297,10 @@ EXPECT_EQ(b_by_c_, path0.certs[0]); EXPECT_EQ(c_by_d_, path0.certs[1]); EXPECT_EQ(d_by_d_, path0.certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::CERTIFICATE_NOT_YET_VALID) + << error.DiagnosticString(); } // Test verifying a certificate that is a trust anchor. @@ -315,6 +325,10 @@ ASSERT_EQ(2U, path.certs.size()); EXPECT_EQ(e_by_e_, path.certs[0]); EXPECT_EQ(e_by_e_, path.certs[1]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // If the target cert is directly issued by a trust anchor, it should verify @@ -335,6 +349,10 @@ ASSERT_EQ(2U, path.certs.size()); EXPECT_EQ(a_by_b_, path.certs[0]); EXPECT_EQ(b_by_f_, path.certs[1]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Test that async cert queries are not made if the path can be successfully @@ -362,6 +380,10 @@ EXPECT_TRUE(result.HasValidPath()); EXPECT_EQ(0, async_certs.num_async_gets()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // If async queries are needed, all async sources will be queried @@ -393,6 +415,10 @@ EXPECT_TRUE(result.HasValidPath()); EXPECT_EQ(1, async_certs1.num_async_gets()); EXPECT_EQ(1, async_certs2.num_async_gets()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Test that PathBuilder does not generate longer paths than necessary if one of @@ -421,6 +447,10 @@ // The result path should be A(B) <- B(C) <- C(D) // not the longer but also valid A(B) <- B(C) <- C(D) <- D(D) EXPECT_EQ(3U, result.GetBestValidPath()->certs.size()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Test that PathBuilder will backtrack and try a different path if the first @@ -471,6 +501,10 @@ EXPECT_EQ(b_by_c_, path.certs[1]); EXPECT_EQ(c_by_d_, path.certs[2]); EXPECT_EQ(d_by_d_, path.certs[3]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Test that if no path to a trust anchor was found, the partial path is @@ -502,6 +536,10 @@ EXPECT_EQ(a_by_b_, result.paths[0]->certs[0]); EXPECT_EQ(b_by_f_, result.paths[0]->certs[1]); EXPECT_EQ(f_by_e_, result.paths[0]->certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } // Test that if two partial paths are returned, the first is marked as the best @@ -547,6 +585,10 @@ EXPECT_EQ(a_by_b_, result.paths[1]->certs[0]); EXPECT_EQ(b_by_c_, result.paths[1]->certs[1]); EXPECT_EQ(c_by_d_, result.paths[1]->certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } // Test that if no valid path is found, and the first invalid path is a partial @@ -598,6 +640,10 @@ EXPECT_EQ(b_by_c_, path.certs[1]); EXPECT_EQ(c_by_d_, path.certs[2]); EXPECT_EQ(d_by_d_, path.certs[3]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } // Test that whichever order CertIssuerSource returns the issuers, the path @@ -639,6 +685,10 @@ EXPECT_EQ(b_by_c_, path.certs[1]); EXPECT_EQ(c_by_d_, path.certs[2]); EXPECT_EQ(d_by_d_, path.certs[3]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } } @@ -682,10 +732,16 @@ EXPECT_EQ(!insufficient_limit, result.HasValidPath()); EXPECT_EQ(insufficient_limit, result.exceeded_iteration_limit); + VerifyError error = result.GetBestPathVerifyError(); if (insufficient_limit) { EXPECT_EQ(2U, result.iteration_count); + ASSERT_EQ(error.Code(), + VerifyError::StatusCode::PATH_ITERATION_COUNT_EXCEEDED) + << error.DiagnosticString(); } else { EXPECT_EQ(3U, result.iteration_count); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } } } @@ -777,6 +833,10 @@ EXPECT_EQ(delegate_.GetMockVerifyCache()->CacheHits(), i * 2); EXPECT_EQ(delegate_.GetMockVerifyCache()->CacheMisses(), 2U); EXPECT_EQ(delegate_.GetMockVerifyCache()->CacheStores(), 2U); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } } @@ -819,6 +879,10 @@ EXPECT_EQ(c_by_d_, result.paths[0]->certs[2]); EXPECT_TRUE( result.paths[0]->errors.ContainsError(cert_errors::kDeadlineExceeded)); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_DEADLINE_EXCEEDED) + << error.DiagnosticString(); } TEST_F(PathBuilderMultiRootTest, TestDepthLimit) { @@ -854,16 +918,21 @@ EXPECT_EQ(!insufficient_limit, result.HasValidPath()); EXPECT_EQ(insufficient_limit, result.AnyPathContainsError(cert_errors::kDepthLimitExceeded)); + VerifyError error = result.GetBestPathVerifyError(); if (insufficient_limit) { EXPECT_EQ(2U, result.max_depth_seen); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_DEPTH_LIMIT_REACHED) + << error.DiagnosticString(); } else { EXPECT_EQ(4U, result.max_depth_seen); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } } } TEST_F(PathBuilderMultiRootTest, TestDepthLimitMultiplePaths) { - // This case tests path building backracking due to reaching the path depth + // This case tests path building backtracking due to reaching the path depth // limit. Given the root and issuer certificates below, there can be two paths // from between the leaf to a trusted root, one has length of 3 and the other // has length of 4. These certificates are specifically chosen because path @@ -914,6 +983,10 @@ EXPECT_EQ(a_by_b_, valid_path->certs[0]); EXPECT_EQ(b_by_c_, valid_path->certs[1]); EXPECT_EQ(c_by_d_, valid_path->certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } TEST_F(PathBuilderMultiRootTest, TestPreCertificate) { @@ -942,6 +1015,10 @@ ASSERT_EQ(1U, result.paths.size()); ASSERT_FALSE(result.paths[0]->IsValid()) << result.paths[0]->errors.ToDebugString(result.paths[0]->certs); + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::CERTIFICATE_INVALID) + << error.DiagnosticString(); + // PreCertificate should be accepted if configured. delegate_.AllowPrecert(); @@ -954,6 +1031,9 @@ ASSERT_EQ(1U, result2.paths.size()); ASSERT_TRUE(result2.paths[0]->IsValid()) << result2.paths[0]->errors.ToDebugString(result.paths[0]->certs); + VerifyError error2 = result2.GetBestPathVerifyError(); + ASSERT_EQ(error2.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error2.DiagnosticString(); } class PathBuilderKeyRolloverTest : public ::testing::Test { @@ -1059,6 +1139,10 @@ EXPECT_EQ(newintermediate_, path0.certs[1]); EXPECT_EQ(newrootrollover_, path0.certs[2]); EXPECT_EQ(oldroot_, path0.certs[3]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Tests that if both old and new roots are trusted it builds a path through @@ -1094,6 +1178,10 @@ // path building. EXPECT_EQ(newintermediate_, path.certs[1]); EXPECT_EQ(newroot_, path.certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // If trust anchor query returned no results, and there are no issuer @@ -1116,6 +1204,10 @@ EXPECT_FALSE(result.paths[0]->IsValid()); ASSERT_EQ(1U, path.certs.size()); EXPECT_EQ(target_, path.certs[0]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } // If a path to a trust anchor could not be found, and the last issuer(s) in @@ -1177,6 +1269,10 @@ EXPECT_EQ(newrootrollover_, path.certs[2]); EXPECT_TRUE(path.errors.ContainsError(cert_errors::kNoIssuersFound)); } + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } // Tests that multiple trust root matches on a single path will be considered. @@ -1219,6 +1315,10 @@ EXPECT_EQ(target_, path.certs[0]); EXPECT_EQ(oldintermediate_, path.certs[1]); EXPECT_EQ(oldroot_, path.certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Tests that the path builder doesn't build longer than necessary paths, @@ -1285,6 +1385,10 @@ EXPECT_EQ(newintermediate_, path2.certs[1]); EXPECT_EQ(newrootrollover_, path2.certs[2]); EXPECT_EQ(oldroot_, path2.certs[3]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Tests that when SetExploreAllPaths is combined with SetIterationLimit the @@ -1343,6 +1447,15 @@ auto result = path_builder.Run(); EXPECT_EQ(expectation.expected_num_paths > 0, result.HasValidPath()); + VerifyError error = result.GetBestPathVerifyError(); + if (expectation.expected_num_paths > 0) { + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); + } else { + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_ITERATION_COUNT_EXCEEDED) + << error.DiagnosticString(); + } + if (expectation.partial_path.empty()) { ASSERT_EQ(expectation.expected_num_paths, result.paths.size()); } else { @@ -1485,12 +1598,16 @@ EXPECT_EQ(newroot_, path3.certs[2]); EXPECT_EQ(3U, result.max_depth_seen); } + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } } // If the target cert is a trust anchor, however is not itself *signed* by a // trust anchor, then it is not considered valid (the SPKI and name of the -// trust anchor matches the SPKI and subject of the targe certificate, but the +// trust anchor matches the SPKI and subject of the target certificate, but the // rest of the certificate cannot be verified). TEST_F(PathBuilderKeyRolloverTest, TestEndEntityIsTrustRoot) { // Trust newintermediate. @@ -1506,6 +1623,10 @@ auto result = path_builder.Run(); EXPECT_FALSE(result.HasValidPath()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } // If target has same Name+SAN+SPKI as a necessary intermediate, test if a path @@ -1534,6 +1655,11 @@ // This could actually be OK, but CertPathBuilder does not build the // newroot <- newrootrollover <- oldroot path. EXPECT_FALSE(result.HasValidPath()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), + VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE) + << error.DiagnosticString(); } // If target has same Name+SAN+SPKI as the trust root, test that a (trivial) @@ -1562,6 +1688,10 @@ ASSERT_EQ(2U, best_result->certs.size()); EXPECT_EQ(newroot_, best_result->certs[0]); EXPECT_EQ(newrootrollover_, best_result->certs[1]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Test that PathBuilder will not try the same path twice if multiple @@ -1628,6 +1758,10 @@ EXPECT_EQ(target_, path1.certs[0]); EXPECT_EQ(newintermediate_, path1.certs[1]); EXPECT_EQ(newroot_, path1.certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Test when PathBuilder is given a cert via CertIssuerSources that has the same @@ -1671,6 +1805,11 @@ // Compare the DER instead of ParsedCertificate pointer, don't care which copy // of newroot was used in the path. EXPECT_EQ(newroot_->der_cert(), path.certs[2]->der_cert()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), + VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE) + << error.DiagnosticString(); } class MockCertIssuerSourceRequest : public CertIssuerSource::Request { @@ -1796,6 +1935,10 @@ EXPECT_EQ(target_, path1.certs[0]); EXPECT_EQ(newintermediate_, path1.certs[1]); EXPECT_EQ(newroot_, path1.certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } // Test that PathBuilder will not try the same path twice if CertIssuerSources @@ -1885,6 +2028,10 @@ EXPECT_EQ(target_, path1.certs[0]); EXPECT_EQ(newintermediate_, path1.certs[1]); EXPECT_EQ(newroot_, path1.certs[2]); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } class PathBuilderSimpleChainTest : public ::testing::Test { @@ -2061,8 +2208,130 @@ ASSERT_TRUE(cert1_errors); EXPECT_TRUE(cert1_errors->ContainsErrorWithSeverity( kWarningFromDelegate, CertError::SEVERITY_WARNING)); + + // The warning should not affect the VerifyError + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); } +TEST_F(PathBuilderCheckPathAfterVerificationTest, TestVerifyErrorMapping) { + struct error_mapping { + CertErrorId internal_error; + VerifyError::StatusCode code; + }; + struct error_mapping AllErrors[] = { + {cert_errors::kInternalError, + VerifyError::StatusCode::VERIFICATION_FAILURE}, + {cert_errors::kValidityFailedNotAfter, + VerifyError::StatusCode::CERTIFICATE_EXPIRED}, + {cert_errors::kValidityFailedNotBefore, + VerifyError::StatusCode::CERTIFICATE_NOT_YET_VALID}, + {cert_errors::kDistrustedByTrustStore, + VerifyError::StatusCode::PATH_NOT_FOUND}, + {cert_errors::kSignatureAlgorithmMismatch, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kChainIsEmpty, + VerifyError::StatusCode::VERIFICATION_FAILURE}, + {cert_errors::kUnconsumedCriticalExtension, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kKeyCertSignBitNotSet, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kMaxPathLengthViolated, + VerifyError::StatusCode::PATH_NOT_FOUND}, + {cert_errors::kBasicConstraintsIndicatesNotCa, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kTargetCertShouldNotBeCa, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kMissingBasicConstraints, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kNotPermittedByNameConstraints, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kTooManyNameConstraintChecks, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kSubjectDoesNotMatchIssuer, + VerifyError::StatusCode::PATH_NOT_FOUND}, + {cert_errors::kVerifySignedDataFailed, + VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE}, + {cert_errors::kSignatureAlgorithmsDifferentEncoding, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kEkuLacksServerAuth, + VerifyError::StatusCode::CERTIFICATE_NO_MATCHING_EKU}, + {cert_errors::kEkuLacksServerAuthButHasAnyEKU, + VerifyError::StatusCode::CERTIFICATE_NO_MATCHING_EKU}, + {cert_errors::kEkuLacksClientAuth, + VerifyError::StatusCode::CERTIFICATE_NO_MATCHING_EKU}, + {cert_errors::kEkuLacksClientAuthButHasAnyEKU, + VerifyError::StatusCode::CERTIFICATE_NO_MATCHING_EKU}, + {cert_errors::kEkuLacksClientAuthOrServerAuth, + VerifyError::StatusCode::CERTIFICATE_NO_MATCHING_EKU}, + {cert_errors::kEkuHasProhibitedOCSPSigning, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kEkuHasProhibitedTimeStamping, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kEkuHasProhibitedCodeSigning, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kEkuNotPresent, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kCertIsNotTrustAnchor, + VerifyError::StatusCode::PATH_NOT_FOUND}, + {cert_errors::kNoValidPolicy, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kPolicyMappingAnyPolicy, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kFailedParsingSpki, + VerifyError::StatusCode::CERTIFICATE_INVALID}, + {cert_errors::kUnacceptableSignatureAlgorithm, + VerifyError::StatusCode::CERTIFICATE_UNSUPPORTED_SIGNATURE_ALGORITHM}, + {cert_errors::kUnacceptablePublicKey, + VerifyError::StatusCode::CERTIFICATE_UNSUPPORTED_KEY}, + {cert_errors::kCertificateRevoked, + VerifyError::StatusCode::CERTIFICATE_REVOKED}, + {cert_errors::kNoRevocationMechanism, + VerifyError::StatusCode::CERTIFICATE_NO_REVOCATION_MECHANISM}, + {cert_errors::kUnableToCheckRevocation, + VerifyError::StatusCode::CERTIFICATE_UNABLE_TO_CHECK_REVOCATION}, + {cert_errors::kNoIssuersFound, VerifyError::StatusCode::PATH_NOT_FOUND}, + {cert_errors::kDeadlineExceeded, + VerifyError::StatusCode::PATH_DEADLINE_EXCEEDED}, + {cert_errors::kIterationLimitExceeded, + VerifyError::StatusCode::PATH_ITERATION_COUNT_EXCEEDED}, + {cert_errors::kDepthLimitExceeded, + VerifyError::StatusCode::PATH_DEPTH_LIMIT_REACHED}, + }; + + for (struct error_mapping mapping : AllErrors) { + AddWarningPathBuilderDelegate delegate; + CertPathBuilder::Result result = RunPathBuilder(nullptr, &delegate); + ASSERT_TRUE(result.HasValidPath()); + + CertErrors *errors = + (CertErrors *)result.GetBestValidPath()->errors.GetErrorsForCert(1); + errors->AddError(mapping.internal_error, nullptr); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), mapping.code) + << error.DiagnosticString(); + } +} + +TEST_F(PathBuilderCheckPathAfterVerificationTest, + TestVerifyErrorMulipleMapping) { + AddWarningPathBuilderDelegate delegate; + CertPathBuilder::Result result = RunPathBuilder(nullptr, &delegate); + ASSERT_TRUE(result.HasValidPath()); + + CertErrors *errors = + (CertErrors *)result.GetBestValidPath()->errors.GetErrorsForCert(1); + errors->AddError(cert_errors::kEkuNotPresent, nullptr); + errors->AddError(cert_errors::kNoValidPolicy, nullptr); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_MULTIPLE_ERRORS) + << error.DiagnosticString(); +} + + DEFINE_CERT_ERROR_ID(kErrorFromDelegate, "Error from delegate"); class AddErrorPathBuilderDelegate : public CertPathBuilderDelegateBase { @@ -2089,8 +2358,47 @@ const CertErrors *cert2_errors = failed_path->errors.GetErrorsForCert(2); ASSERT_TRUE(cert2_errors); EXPECT_TRUE(cert2_errors->ContainsError(kErrorFromDelegate)); + + // The newly defined delegate error should map to CERTIFICATE_INVALID + // since it is associated with a certificate (at index 2) + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::CERTIFICATE_INVALID) + << error.DiagnosticString(); } +class AddOtherErrorPathBuilderDelegate : public CertPathBuilderDelegateBase { + public: + void CheckPathAfterVerification(const CertPathBuilder &path_builder, + CertPathBuilderResultPath *path) override { + path->errors.GetOtherErrors()->AddError(kErrorFromDelegate, nullptr); + } +}; + +TEST_F(PathBuilderCheckPathAfterVerificationTest, AddsErrorToOtherErrors) { + AddOtherErrorPathBuilderDelegate delegate; + CertPathBuilder::Result result = RunPathBuilder(nullptr, &delegate); + + // Verification failed. + ASSERT_FALSE(result.HasValidPath()); + + ASSERT_LT(result.best_result_index, result.paths.size()); + const CertPathBuilderResultPath *failed_path = + result.paths[result.best_result_index].get(); + ASSERT_TRUE(failed_path); + + // An error should have been added to other errors + const CertErrors *other_errors = failed_path->errors.GetOtherErrors(); + ASSERT_TRUE(other_errors); + EXPECT_TRUE(other_errors->ContainsError(kErrorFromDelegate)); + + // The newly defined delegate error should map to VERIFICATION_FAILURE + // since the error is not associated to a certificate. + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::VERIFICATION_FAILURE) + << error.DiagnosticString(); +} + + TEST_F(PathBuilderCheckPathAfterVerificationTest, NoopToAlreadyInvalidPath) { StrictMock<MockPathBuilderDelegate> delegate; // Just verify that the hook is called (on an invalid path). @@ -2099,6 +2407,10 @@ // Run the pathbuilder with certificate at index 1 actively distrusted. CertPathBuilder::Result result = RunPathBuilder(test_.chain[1], &delegate); EXPECT_FALSE(result.HasValidPath()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); } struct DelegateData : public CertPathBuilderDelegateData { @@ -2181,6 +2493,11 @@ CertPathBuilder::Result result = path_builder.Run(); EXPECT_FALSE(result.HasValidPath()); + + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); + ASSERT_EQ(4U, result.paths.size()); // Path builder should have attempted paths using the intermediates in @@ -2523,6 +2840,9 @@ CertPathBuilder::Result result = path_builder.Run(); EXPECT_FALSE(result.HasValidPath()); ASSERT_EQ(3U, result.paths.size()); + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_NOT_FOUND) + << error.DiagnosticString(); // The serial & issuer method is not used in prioritization, so the certs // should have been prioritized based on dates. The test certs have the @@ -2579,6 +2899,9 @@ CertPathBuilder::Result result = path_builder.Run(); EXPECT_TRUE(result.HasValidPath()); + VerifyError error = result.GetBestPathVerifyError(); + ASSERT_EQ(error.Code(), VerifyError::StatusCode::PATH_VERIFIED) + << error.DiagnosticString(); // Path builder should have built paths to both trusted roots. ASSERT_EQ(2U, result.paths.size());
diff --git a/pki/verify_error.cc b/pki/verify_error.cc new file mode 100644 index 0000000..da61236 --- /dev/null +++ b/pki/verify_error.cc
@@ -0,0 +1,16 @@ +#include <openssl/base.h> +#include <openssl/pki/verify_error.h> + +namespace bssl { + +VerifyError::VerifyError(StatusCode code, ptrdiff_t offset, + std::string diagnostic) + : offset_(offset), code_(code), diagnostic_(std::move(diagnostic)) {} + +const std::string &VerifyError::DiagnosticString() const { return diagnostic_; } + +ptrdiff_t VerifyError::Index() const { return offset_; } + +VerifyError::StatusCode VerifyError::Code() const { return code_; } + +} // namespacee bssl
diff --git a/sources.cmake b/sources.cmake index dcdf497..94e1f5f 100644 --- a/sources.cmake +++ b/sources.cmake
@@ -451,6 +451,7 @@ pki/trust_store_in_memory.cc pki/trust_store.cc pki/verify_certificate_chain.cc + pki/verify_error.cc pki/verify_name_match.cc pki/verify_signed_data.cc )