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
}
}
