Sync pki to chrome 28e4a1b838b2ffbf9e2151ae5fcfffe5ab0ffac0 While we are at it, add a fillins/log.h and modify import_spec.json to remove the need for patching out LOG and DVLOG (which removes one more patch in the patch stack Change-Id: I3b7b512490ee0e8c465734b2236b31da7d132ec7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61225 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/pki/cert_net_fetcher.h b/pki/cert_net_fetcher.h index d91297b..27341ae 100644 --- a/pki/cert_net_fetcher.h +++ b/pki/cert_net_fetcher.h
@@ -13,6 +13,7 @@ #include <vector> #include <memory> +#include "fillins/log.h" #include "fillins/net_errors.h"
diff --git a/pki/fillins/log.h b/pki/fillins/log.h new file mode 100644 index 0000000..74007ec --- /dev/null +++ b/pki/fillins/log.h
@@ -0,0 +1,24 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef BSSL_FILLINS_LOG_H_ +#define BSSL_FILLINS_LOG_H_ + +#include <iostream> + +// This header defines the logging macros, inherited from chrome. + +// This file is not used in chrome, so check here to make sure we are +// only compiling inside boringssl. +#if !defined(_BORINGSSL_LIBPKI_) +#error "_BORINGSSL_LIBPKI_ is not defined when compiling BoringSSL libpki" +#endif + +#if defined(_BORINGSSL_LIBPKI_VERBOSE_) +#define DVLOG(l) std::cerr +#else +#define DVLOG(l) 0 && std::cerr +#endif // _BORINGSSL_LIBPKI_VERBOSE_ + +#endif // BSSL_FILLINS_LOG_H_
diff --git a/pki/import_spec.json b/pki/import_spec.json index 09578fc..705b6ac 100644 --- a/pki/import_spec.json +++ b/pki/import_spec.json
@@ -66,7 +66,7 @@ {"match": "\"net/third_party/nist-pkits", "replace": "\"testdata/nist-pkits"}, {"match": "^#include \"net/base/net_errors.h\"", - "replace": "#include \"fillins/net_errors.h\"\n"}, + "replace": "#include \"fillins/log.h\"\n#include \"fillins/net_errors.h\"\n"}, {"match": "^#include \"net/test/test_certificate_data.h\"", "replace": "#include \"testdata/test_certificate_data.h\""}, {"match": "^#include \"net/third_party/nist-pkits/pkits_testcases-inl.h\"", @@ -111,6 +111,8 @@ "replace": "#include <optional>"}, {"match": "^#include \"base/containers/contains.h\"", "replace": ""}, + {"match": "LOG(ERROR)", + "replace": "std::cerr"}, {"match": "GURL", "replace": "URL", "include": "webutil/url/url.h"},
diff --git a/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch b/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch index 8dd3fdc..1917779 100644 --- a/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch +++ b/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch
@@ -1,7 +1,7 @@ -From e56d5c6dd965c8064a6a6953848aa7fa20ced918 Mon Sep 17 00:00:00 2001 +From 6218984931c841f8e2939a94d58e7315a967fe68 Mon Sep 17 00:00:00 2001 From: Bob Beck <bbe@google.com> Date: Wed, 17 May 2023 10:37:22 -0600 -Subject: [PATCH 1/3] Simplify the name constraint limit to resemble +Subject: [PATCH 1/2] Simplify the name constraint limit to resemble BoringSSL's Disable the name constraint manynames check for now ---
diff --git a/pki/patches/0002-Conditionalize-the-use-of-DVLOG-LOG-in-path-builder-.patch b/pki/patches/0002-Conditionalize-the-use-of-DVLOG-LOG-in-path-builder-.patch deleted file mode 100644 index 11e6295..0000000 --- a/pki/patches/0002-Conditionalize-the-use-of-DVLOG-LOG-in-path-builder-.patch +++ /dev/null
@@ -1,221 +0,0 @@ -From 8776d775ed86427e37b17ce1ea4728e99cb45275 Mon Sep 17 00:00:00 2001 -From: Bob Beck <bbe@google.com> -Date: Thu, 1 Jun 2023 10:54:40 +0200 -Subject: [PATCH 2/3] Conditionalize the use of DVLOG/LOG in path builder on - DVLOG existing - ---- - net/cert/pki/path_builder.cc | 35 +++++++++++++++++++++ - net/cert/pki/path_builder_pkits_unittest.cc | 2 ++ - 2 files changed, 37 insertions(+) - -diff --git a/net/cert/pki/path_builder.cc b/net/cert/pki/path_builder.cc -index c373c8d4cda99..8b84fca03e962 100644 ---- a/net/cert/pki/path_builder.cc -+++ b/net/cert/pki/path_builder.cc -@@ -37,6 +37,7 @@ std::string FingerPrintParsedCertificate(const net::ParsedCertificate* cert) { - } - - // TODO(mattm): decide how much debug logging to keep. -+// TODO(bbe): perhaps none - currently conditionalizing on DVLOG.. - std::string CertDebugString(const ParsedCertificate* cert) { - RDNSequence subject; - std::string subject_str; -@@ -47,6 +48,7 @@ std::string CertDebugString(const ParsedCertificate* cert) { - return FingerPrintParsedCertificate(cert) + " " + subject_str; - } - -+#if defined(DVLOG) - std::string PathDebugString(const ParsedCertificateList& certs) { - std::string s; - for (const auto& cert : certs) { -@@ -56,6 +58,7 @@ std::string PathDebugString(const ParsedCertificateList& certs) { - } - return s; - } -+#endif - - // This structure describes a certificate and its trust level. Note that |cert| - // may be null to indicate an "empty" entry. -@@ -249,7 +252,9 @@ CertIssuersIter::CertIssuersIter( - cert_issuer_sources_(cert_issuer_sources), - trust_store_(trust_store), - debug_data_(debug_data) { -+#if defined(DVLOG) - DVLOG(2) << "CertIssuersIter created for " << CertDebugString(cert()); -+#endif - } - - void CertIssuersIter::GetNextIssuer(IssuerEntry* out) { -@@ -289,8 +294,10 @@ void CertIssuersIter::GetNextIssuer(IssuerEntry* out) { - if (HasCurrentIssuer()) { - SortRemainingIssuers(); - -+#if defined(DVLOG) - DVLOG(2) << "CertIssuersIter returning issuer " << cur_issuer_ << " of " - << issuers_.size() << " for " << CertDebugString(cert()); -+#endif - // Still have issuers that haven't been returned yet, return the highest - // priority one (head of remaining list). A reference to the returned issuer - // is retained, since |present_issuers_| points to data owned by it. -@@ -298,8 +305,10 @@ void CertIssuersIter::GetNextIssuer(IssuerEntry* out) { - return; - } - -+#if defined(DVLOG) - DVLOG(2) << "CertIssuersIter reached the end of all available issuers for " - << CertDebugString(cert()); -+#endif - // Reached the end of all available issuers. - *out = IssuerEntry(); - } -@@ -331,7 +340,9 @@ void CertIssuersIter::DoAsyncIssuerQuery() { - std::unique_ptr<CertIssuerSource::Request> request; - cert_issuer_source->AsyncGetIssuersOf(cert(), &request); - if (request) { -+#if defined(DVLOG) - DVLOG(1) << "AsyncGetIssuersOf pending for " << CertDebugString(cert()); -+#endif - pending_async_requests_.push_back(std::move(request)); - } - } -@@ -558,16 +569,20 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - cur_path_.Length() >= max_path_building_depth) { - cur_path_.CopyPath(out_certs); - out_errors->GetOtherErrors()->AddError(cert_errors::kDepthLimitExceeded); -+#if defined(DVLOG) - DVLOG(1) << "CertPathIter reached depth limit. Returning partial path " - "and backtracking:\n" - << PathDebugString(*out_certs); -+#endif - cur_path_.Pop(); - return true; - } - - if (!next_issuer_.cert) { - if (cur_path_.Empty()) { -+#if defined(DVLOG) - DVLOG(1) << "CertPathIter exhausted all paths..."; -+#endif - return false; - } - -@@ -587,14 +602,18 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - cur_path_.CopyPath(out_certs); - out_errors->GetErrorsForCert(out_certs->size() - 1) - ->AddError(cert_errors::kNoIssuersFound); -+#if defined(DVLOG) - DVLOG(1) << "CertPathIter returning partial path and backtracking:\n" - << PathDebugString(*out_certs); -+#endif - cur_path_.Pop(); - return true; - } else { - // No more issuers for current chain, go back up and see if there are - // any more for the previous cert. -+#if defined(DVLOG) - DVLOG(1) << "CertPathIter backtracking..."; -+#endif - cur_path_.Pop(); - continue; - } -@@ -610,7 +629,9 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - // unspecified trust. This may allow a successful path to be built to a - // different root (or to the same cert if it's self-signed). - if (cur_path_.Empty()) { -+#if defined(DVLOG) - DVLOG(1) << "Leaf is a trust anchor, considering as UNSPECIFIED"; -+#endif - next_issuer_.trust = CertificateTrust::ForUnspecified(); - } - break; -@@ -619,7 +640,9 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - // unspecified trust. This may allow a successful path to be built to a - // trusted root. - if (!cur_path_.Empty()) { -+#if defined(DVLOG) - DVLOG(1) << "Issuer is a trust leaf, considering as UNSPECIFIED"; -+#endif - next_issuer_.trust = CertificateTrust::ForUnspecified(); - } - break; -@@ -639,8 +662,10 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - !VerifyCertificateIsSelfSigned(*next_issuer_.cert, - delegate->GetVerifyCache(), - /*errors=*/nullptr)) { -+#if defined(DVLOG) - DVLOG(1) << "Leaf is trusted with require_leaf_selfsigned but is " - "not self-signed, considering as UNSPECIFIED"; -+#endif - next_issuer_.trust = CertificateTrust::ForUnspecified(); - } - break; -@@ -660,12 +685,16 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - case CertificateTrustType::TRUSTED_ANCHOR_OR_LEAF: - case CertificateTrustType::TRUSTED_LEAF: { - // If the issuer has a known trust level, can stop building the path. -+#if defined(DVLOG) - DVLOG(2) << "CertPathIter got anchor: " - << CertDebugString(next_issuer_.cert.get()); -+#endif - cur_path_.CopyPath(out_certs); - out_certs->push_back(std::move(next_issuer_.cert)); -+#if defined(DVLOG) - DVLOG(1) << "CertPathIter returning path:\n" - << PathDebugString(*out_certs); -+#endif - *out_last_cert_trust = next_issuer_.trust; - next_issuer_ = IssuerEntry(); - return true; -@@ -674,8 +703,10 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - // Skip this cert if it is already in the chain. - if (cur_path_.IsPresent(next_issuer_.cert.get())) { - cur_path_.back()->increment_skipped_issuer_count(); -+#if defined(DVLOG) - DVLOG(1) << "CertPathIter skipping dupe cert: " - << CertDebugString(next_issuer_.cert.get()); -+#endif - next_issuer_ = IssuerEntry(); - continue; - } -@@ -684,7 +715,9 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs, - std::move(next_issuer_.cert), &cert_issuer_sources_, trust_store_, - debug_data_)); - next_issuer_ = IssuerEntry(); -+#if defined(DVLOG) - DVLOG(1) << "CertPathIter cur_path_ =\n" << cur_path_.PathDebugString(); -+#endif - // Continue descending the tree. - continue; - } -@@ -832,8 +865,10 @@ CertPathBuilder::Result CertPathBuilder::Run() { - &result_path->user_constrained_policy_set, &result_path->errors); - } - -+#if defined(DVLOG) - DVLOG(1) << "CertPathBuilder VerifyCertificateChain errors:\n" - << result_path->errors.ToDebugString(result_path->certs); -+#endif - - // Give the delegate a chance to add errors to the path. - delegate_->CheckPathAfterVerification(*this, result_path.get()); -diff --git a/net/cert/pki/path_builder_pkits_unittest.cc b/net/cert/pki/path_builder_pkits_unittest.cc -index 090cce0cbd52b..c8fd0bd993996 100644 ---- a/net/cert/pki/path_builder_pkits_unittest.cc -+++ b/net/cert/pki/path_builder_pkits_unittest.cc -@@ -232,10 +232,12 @@ class PathBuilderPkitsTestDelegate { - - if (info.should_validate != result.HasValidPath()) { - for (size_t i = 0; i < result.paths.size(); ++i) { -+#if defined(DVLOG) - const net::CertPathBuilderResultPath* result_path = - result.paths[i].get(); - LOG(ERROR) << "path " << i << " errors:\n" - << result_path->errors.ToDebugString(result_path->certs); -+#endif - } - } - --- -2.41.0.162.gfafddb0af9-goog -
diff --git a/pki/patches/0003-disable-path-builder-tests-with-unsupported-dependen.patch b/pki/patches/0002-disable-path-builder-tests-with-unsupported-dependen.patch similarity index 96% rename from pki/patches/0003-disable-path-builder-tests-with-unsupported-dependen.patch rename to pki/patches/0002-disable-path-builder-tests-with-unsupported-dependen.patch index c9b1753..6565afa 100644 --- a/pki/patches/0003-disable-path-builder-tests-with-unsupported-dependen.patch +++ b/pki/patches/0002-disable-path-builder-tests-with-unsupported-dependen.patch
@@ -1,7 +1,7 @@ -From 9a38f227afca29cb3e373ca973d2f68126a0145e Mon Sep 17 00:00:00 2001 +From 0a1d630dbc33f63009aabb4907a1dbeb4c934415 Mon Sep 17 00:00:00 2001 From: Bob Beck <bbe@google.com> Date: Fri, 2 Jun 2023 11:08:50 +0200 -Subject: [PATCH 3/3] disable path builder tests with unsupported dependencies +Subject: [PATCH 2/2] disable path builder tests with unsupported dependencies --- net/cert/pki/path_builder_unittest.cc | 12 ++++++++++++
diff --git a/pki/path_builder.cc b/pki/path_builder.cc index 9624598..9f4b7e9 100644 --- a/pki/path_builder.cc +++ b/pki/path_builder.cc
@@ -9,6 +9,7 @@ #include <set> #include <unordered_set> +#include "fillins/log.h" #include "fillins/net_errors.h" #include "cert_issuer_source.h" @@ -38,7 +39,6 @@ } // TODO(mattm): decide how much debug logging to keep. -// TODO(bbe): perhaps none - currently conditionalizing on DVLOG.. std::string CertDebugString(const ParsedCertificate* cert) { RDNSequence subject; std::string subject_str; @@ -49,7 +49,6 @@ return FingerPrintParsedCertificate(cert) + " " + subject_str; } -#if defined(DVLOG) std::string PathDebugString(const ParsedCertificateList& certs) { std::string s; for (const auto& cert : certs) { @@ -59,7 +58,6 @@ } return s; } -#endif // This structure describes a certificate and its trust level. Note that |cert| // may be null to indicate an "empty" entry. @@ -253,9 +251,7 @@ cert_issuer_sources_(cert_issuer_sources), trust_store_(trust_store), debug_data_(debug_data) { -#if defined(DVLOG) DVLOG(2) << "CertIssuersIter created for " << CertDebugString(cert()); -#endif } void CertIssuersIter::GetNextIssuer(IssuerEntry* out) { @@ -295,10 +291,8 @@ if (HasCurrentIssuer()) { SortRemainingIssuers(); -#if defined(DVLOG) DVLOG(2) << "CertIssuersIter returning issuer " << cur_issuer_ << " of " << issuers_.size() << " for " << CertDebugString(cert()); -#endif // Still have issuers that haven't been returned yet, return the highest // priority one (head of remaining list). A reference to the returned issuer // is retained, since |present_issuers_| points to data owned by it. @@ -306,10 +300,8 @@ return; } -#if defined(DVLOG) DVLOG(2) << "CertIssuersIter reached the end of all available issuers for " << CertDebugString(cert()); -#endif // Reached the end of all available issuers. *out = IssuerEntry(); } @@ -341,9 +333,7 @@ std::unique_ptr<CertIssuerSource::Request> request; cert_issuer_source->AsyncGetIssuersOf(cert(), &request); if (request) { -#if defined(DVLOG) DVLOG(1) << "AsyncGetIssuersOf pending for " << CertDebugString(cert()); -#endif pending_async_requests_.push_back(std::move(request)); } } @@ -570,20 +560,16 @@ cur_path_.Length() >= max_path_building_depth) { cur_path_.CopyPath(out_certs); out_errors->GetOtherErrors()->AddError(cert_errors::kDepthLimitExceeded); -#if defined(DVLOG) DVLOG(1) << "CertPathIter reached depth limit. Returning partial path " "and backtracking:\n" << PathDebugString(*out_certs); -#endif cur_path_.Pop(); return true; } if (!next_issuer_.cert) { if (cur_path_.Empty()) { -#if defined(DVLOG) DVLOG(1) << "CertPathIter exhausted all paths..."; -#endif return false; } @@ -603,18 +589,14 @@ cur_path_.CopyPath(out_certs); out_errors->GetErrorsForCert(out_certs->size() - 1) ->AddError(cert_errors::kNoIssuersFound); -#if defined(DVLOG) DVLOG(1) << "CertPathIter returning partial path and backtracking:\n" << PathDebugString(*out_certs); -#endif cur_path_.Pop(); return true; } else { // No more issuers for current chain, go back up and see if there are // any more for the previous cert. -#if defined(DVLOG) DVLOG(1) << "CertPathIter backtracking..."; -#endif cur_path_.Pop(); continue; } @@ -630,9 +612,7 @@ // unspecified trust. This may allow a successful path to be built to a // different root (or to the same cert if it's self-signed). if (cur_path_.Empty()) { -#if defined(DVLOG) DVLOG(1) << "Leaf is a trust anchor, considering as UNSPECIFIED"; -#endif next_issuer_.trust = CertificateTrust::ForUnspecified(); } break; @@ -641,9 +621,7 @@ // unspecified trust. This may allow a successful path to be built to a // trusted root. if (!cur_path_.Empty()) { -#if defined(DVLOG) DVLOG(1) << "Issuer is a trust leaf, considering as UNSPECIFIED"; -#endif next_issuer_.trust = CertificateTrust::ForUnspecified(); } break; @@ -663,10 +641,8 @@ !VerifyCertificateIsSelfSigned(*next_issuer_.cert, delegate->GetVerifyCache(), /*errors=*/nullptr)) { -#if defined(DVLOG) DVLOG(1) << "Leaf is trusted with require_leaf_selfsigned but is " "not self-signed, considering as UNSPECIFIED"; -#endif next_issuer_.trust = CertificateTrust::ForUnspecified(); } break; @@ -686,16 +662,12 @@ case CertificateTrustType::TRUSTED_ANCHOR_OR_LEAF: case CertificateTrustType::TRUSTED_LEAF: { // If the issuer has a known trust level, can stop building the path. -#if defined(DVLOG) DVLOG(2) << "CertPathIter got anchor: " << CertDebugString(next_issuer_.cert.get()); -#endif cur_path_.CopyPath(out_certs); out_certs->push_back(std::move(next_issuer_.cert)); -#if defined(DVLOG) DVLOG(1) << "CertPathIter returning path:\n" << PathDebugString(*out_certs); -#endif *out_last_cert_trust = next_issuer_.trust; next_issuer_ = IssuerEntry(); return true; @@ -704,10 +676,8 @@ // Skip this cert if it is already in the chain. if (cur_path_.IsPresent(next_issuer_.cert.get())) { cur_path_.back()->increment_skipped_issuer_count(); -#if defined(DVLOG) DVLOG(1) << "CertPathIter skipping dupe cert: " << CertDebugString(next_issuer_.cert.get()); -#endif next_issuer_ = IssuerEntry(); continue; } @@ -716,9 +686,7 @@ std::move(next_issuer_.cert), &cert_issuer_sources_, trust_store_, debug_data_)); next_issuer_ = IssuerEntry(); -#if defined(DVLOG) DVLOG(1) << "CertPathIter cur_path_ =\n" << cur_path_.PathDebugString(); -#endif // Continue descending the tree. continue; } @@ -866,10 +834,8 @@ &result_path->user_constrained_policy_set, &result_path->errors); } -#if defined(DVLOG) DVLOG(1) << "CertPathBuilder VerifyCertificateChain errors:\n" << result_path->errors.ToDebugString(result_path->certs); -#endif // Give the delegate a chance to add errors to the path. delegate_->CheckPathAfterVerification(*this, result_path.get());
diff --git a/pki/path_builder_pkits_unittest.cc b/pki/path_builder_pkits_unittest.cc index a6da30f..b49a34d 100644 --- a/pki/path_builder_pkits_unittest.cc +++ b/pki/path_builder_pkits_unittest.cc
@@ -5,7 +5,9 @@ #include "path_builder.h" #include <cstdint> +#include <iostream> +#include "fillins/log.h" #include "fillins/net_errors.h" #include "cert_issuer_source_static.h" @@ -233,12 +235,10 @@ if (info.should_validate != result.HasValidPath()) { for (size_t i = 0; i < result.paths.size(); ++i) { -#if defined(DVLOG) const CertPathBuilderResultPath* result_path = result.paths[i].get(); - LOG(ERROR) << "path " << i << " errors:\n" + std::cerr << "path " << i << " errors:\n" << result_path->errors.ToDebugString(result_path->certs); -#endif } }