Skip to content

Commit

Permalink
cert_verify_tool: plumbing for configurable trust settings
Browse files Browse the repository at this point in the history
Follow up CLs will add flags to configure the trust settings used.
Note that the configurable trust settings here only apply when
testing the builtin verifier. With the platform verifiers all
trust settings will act the same.

For the VerifyUsingCertVerifyProc implementation, switch to always
used ScopedTestRoot for adding roots, instead of using the
additional_trust_anchors parameter of Verify() when
SupportsAdditionalTrustAnchors() is true.

There isn't really any benefit to doing both if we need to use
ScopedTestRoot on some implementations anyway. And ScopedTestRoot now
allows to specify additional trust options, which isn't possible with
additional_trust_anchors. So just make it always use ScopedTestRoot.

Bug: 1408473
Change-Id: I1a4313a591558c95549f5c107058ac6074dce9ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4184950
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: Hubert Chao <hchao@chromium.org>
Reviewed-by: Bob Beck <bbe@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096898}
  • Loading branch information
matt-mueller authored and Chromium LUCI CQ committed Jan 25, 2023
1 parent 6818567 commit 36d0168
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 49 deletions.
48 changes: 33 additions & 15 deletions net/tools/cert_verify_tool/cert_verify_tool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "net/cert/cert_verify_proc_builtin.h"
#include "net/cert/crl_set.h"
#include "net/cert/internal/system_trust_store.h"
#include "net/cert/pki/trust_store.h"
#include "net/cert/x509_util.h"
#include "net/cert_net/cert_net_fetcher_url_request.h"
#include "net/tools/cert_verify_tool/cert_verify_tool_util.h"
Expand Down Expand Up @@ -101,7 +102,8 @@ class CertVerifyImpl {
virtual bool VerifyCert(const CertInput& target_der_cert,
const std::string& hostname,
const std::vector<CertInput>& intermediate_der_certs,
const std::vector<CertInput>& root_der_certs,
const std::vector<CertInputWithTrustSetting>&
der_certs_with_trust_settings,
base::Time verify_time,
net::CRLSet* crl_set,
const base::FilePath& dump_prefix_path) = 0;
Expand All @@ -119,7 +121,8 @@ class CertVerifyImplUsingProc : public CertVerifyImpl {
bool VerifyCert(const CertInput& target_der_cert,
const std::string& hostname,
const std::vector<CertInput>& intermediate_der_certs,
const std::vector<CertInput>& root_der_certs,
const std::vector<CertInputWithTrustSetting>&
der_certs_with_trust_settings,
base::Time verify_time,
net::CRLSet* crl_set,
const base::FilePath& dump_prefix_path) override {
Expand All @@ -140,9 +143,9 @@ class CertVerifyImplUsingProc : public CertVerifyImpl {
.InsertBeforeExtensionASCII("." + GetName());
}

return VerifyUsingCertVerifyProc(proc_.get(), target_der_cert, hostname,
intermediate_der_certs, root_der_certs,
crl_set, dump_path);
return VerifyUsingCertVerifyProc(
proc_.get(), target_der_cert, hostname, intermediate_der_certs,
der_certs_with_trust_settings, crl_set, dump_path);
}

private:
Expand All @@ -164,7 +167,8 @@ class CertVerifyImplUsingPathBuilder : public CertVerifyImpl {
bool VerifyCert(const CertInput& target_der_cert,
const std::string& hostname,
const std::vector<CertInput>& intermediate_der_certs,
const std::vector<CertInput>& root_der_certs,
const std::vector<CertInputWithTrustSetting>&
der_certs_with_trust_settings,
base::Time verify_time,
net::CRLSet* crl_set,
const base::FilePath& dump_prefix_path) override {
Expand All @@ -177,8 +181,9 @@ class CertVerifyImplUsingPathBuilder : public CertVerifyImpl {
}

return VerifyUsingPathBuilder(target_der_cert, intermediate_der_certs,
root_der_certs, verify_time, dump_prefix_path,
cert_net_fetcher_, system_trust_store_.get());
der_certs_with_trust_settings, verify_time,
dump_prefix_path, cert_net_fetcher_,
system_trust_store_.get());
}

private:
Expand Down Expand Up @@ -266,11 +271,13 @@ void PrintInputChain(const CertInput& target,
std::cout << "\n";
}

void PrintAdditionalRoots(const std::vector<CertInput>& root_der_certs) {
void PrintAdditionalRoots(const std::vector<CertInputWithTrustSetting>&
der_certs_with_trust_settings) {
std::cout << "Additional roots:\n";
for (const auto& cert : root_der_certs) {
for (const auto& cert : der_certs_with_trust_settings) {
std::cout << " " << cert.trust.ToDebugString() << ":\n ";
PrintCertHashAndSubject(
net::x509_util::CreateCryptoBuffer(cert.der_cert).get());
net::x509_util::CreateCryptoBuffer(cert.cert_input.der_cert).get());
}
std::cout << "\n";
}
Expand Down Expand Up @@ -413,6 +420,7 @@ int main(int argc, char** argv) {

base::FilePath dump_prefix_path = command_line.GetSwitchValuePath("dump");

std::vector<CertInputWithTrustSetting> der_certs_with_trust_settings;
std::vector<CertInput> root_der_certs;
std::vector<CertInput> intermediate_der_certs;
CertInput target_der_cert;
Expand Down Expand Up @@ -445,9 +453,19 @@ int main(int argc, char** argv) {
intermediate_der_certs.pop_back();
}

for (const auto& cert_input : root_der_certs) {
// TODO(https://crbug.com/1408473): Maybe default to the trust setting that
// would be used for locally added anchors on the current platform?
// TODO(https://crbug.com/1408473): Add flag to configure the trust setting
// used here.
der_certs_with_trust_settings.push_back(
{cert_input, net::CertificateTrust::ForTrustAnchor()});
}

PrintInputChain(target_der_cert, intermediate_der_certs);
if (!root_der_certs.empty())
PrintAdditionalRoots(root_der_certs);
if (!der_certs_with_trust_settings.empty()) {
PrintAdditionalRoots(der_certs_with_trust_settings);
}

// Create a network thread to be used for AIA fetches, and wait for a
// CertNetFetcher to be constructed on that thread.
Expand Down Expand Up @@ -500,8 +518,8 @@ int main(int argc, char** argv) {

std::cout << impls[i]->GetName() << ":\n";
if (!impls[i]->VerifyCert(target_der_cert, hostname, intermediate_der_certs,
root_der_certs, verify_time, crl_set.get(),
dump_prefix_path)) {
der_certs_with_trust_settings, verify_time,
crl_set.get(), dump_prefix_path)) {
all_impls_success = false;
}
}
Expand Down
8 changes: 8 additions & 0 deletions net/tools/cert_verify_tool/cert_verify_tool_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "third_party/boringssl/src/include/openssl/base.h"

#include "base/files/file_path.h"
#include "net/cert/pki/trust_store.h"

namespace base {
class SupportsUserData;
Expand All @@ -35,6 +36,13 @@ struct CertInput {
std::string source_details;
};

// Stores DER certificate bytes as well as a trust setting that should be
// applied to them.
struct CertInputWithTrustSetting {
CertInput cert_input;
net::CertificateTrust trust;
};

// Parses |file_path| as a single DER cert or a PEM certificate list.
bool ReadCertificatesFromFile(const base::FilePath& file_path,
std::vector<CertInput>* certs);
Expand Down
45 changes: 18 additions & 27 deletions net/tools/cert_verify_tool/verify_using_cert_verify_proc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ bool VerifyUsingCertVerifyProc(
const CertInput& target_der_cert,
const std::string& hostname,
const std::vector<CertInput>& intermediate_der_certs,
const std::vector<CertInput>& root_der_certs,
const std::vector<CertInputWithTrustSetting>& der_certs_with_trust_settings,
net::CRLSet* crl_set,
const base::FilePath& dump_path) {
std::vector<base::StringPiece> der_cert_chain;
Expand All @@ -108,43 +108,34 @@ bool VerifyUsingCertVerifyProc(
return false;
}

net::CertificateList x509_additional_trust_anchors;
for (const auto& cert : root_der_certs) {
scoped_refptr<net::X509Certificate> x509_root =
net::X509Certificate::CreateFromBytes(
base::as_bytes(base::make_span(cert.der_cert)));
net::TestRootCerts* test_root_certs = net::TestRootCerts::GetInstance();
CHECK(test_root_certs->IsEmpty());

if (!x509_root)
PrintCertError("ERROR: X509Certificate::CreateFromBytes failed:", cert);
else
x509_additional_trust_anchors.push_back(x509_root);
std::vector<net::ScopedTestRoot> scoped_test_roots;
for (const auto& cert_input_with_trust : der_certs_with_trust_settings) {
scoped_refptr<net::X509Certificate> x509_root =
net::X509Certificate::CreateFromBytes(base::as_bytes(
base::make_span(cert_input_with_trust.cert_input.der_cert)));

if (!x509_root) {
PrintCertError("ERROR: X509Certificate::CreateFromBytes failed:",
cert_input_with_trust.cert_input);
} else {
scoped_test_roots.emplace_back(x509_root.get(),
cert_input_with_trust.trust);
}
}

// TODO(mattm): add command line flags to configure VerifyFlags.
int flags = 0;

// Not all platforms support providing additional trust anchors to the
// verifier. To workaround this, use TestRootCerts to modify the
// system trust store globally.
net::TestRootCerts* test_root_certs = net::TestRootCerts::GetInstance();
CHECK(test_root_certs->IsEmpty());

net::ScopedTestRoot scoped_test_roots;
if (!x509_additional_trust_anchors.empty() &&
!cert_verify_proc->SupportsAdditionalTrustAnchors()) {
std::cerr << "NOTE: Additional trust anchors not supported on this "
"platform. Using TestRootCerts instead.\n";

scoped_test_roots.Reset(x509_additional_trust_anchors);
x509_additional_trust_anchors.clear();
}

// TODO(crbug.com/634484): use a real netlog and print the results?
net::CertVerifyResult result;
int rv = cert_verify_proc->Verify(
x509_target_and_intermediates.get(), hostname,
/*ocsp_response=*/std::string(), /*sct_list=*/std::string(), flags,
crl_set, x509_additional_trust_anchors, &result, net::NetLogWithSource());
crl_set, /*additional_trust_anchors=*/{}, &result,
net::NetLogWithSource());

std::cout << "CertVerifyProc result: " << net::ErrorToShortString(rv) << "\n";
PrintCertVerifyResult(result);
Expand Down
3 changes: 2 additions & 1 deletion net/tools/cert_verify_tool/verify_using_cert_verify_proc.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class CertVerifyResult;
}

struct CertInput;
struct CertInputWithTrustSetting;

void PrintCertVerifyResult(const net::CertVerifyResult& result);

Expand All @@ -31,7 +32,7 @@ bool VerifyUsingCertVerifyProc(
const CertInput& target_der_cert,
const std::string& hostname,
const std::vector<CertInput>& intermediate_der_certs,
const std::vector<CertInput>& root_der_certs,
const std::vector<CertInputWithTrustSetting>& der_certs_with_trust_settings,
net::CRLSet* crl_set,
const base::FilePath& dump_path);

Expand Down
12 changes: 7 additions & 5 deletions net/tools/cert_verify_tool/verify_using_path_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ std::shared_ptr<const net::ParsedCertificate> ParseCertificate(
bool VerifyUsingPathBuilder(
const CertInput& target_der_cert,
const std::vector<CertInput>& intermediate_der_certs,
const std::vector<CertInput>& root_der_certs,
const std::vector<CertInputWithTrustSetting>& der_certs_with_trust_settings,
const base::Time at_time,
const base::FilePath& dump_prefix_path,
scoped_refptr<net::CertNetFetcher> cert_net_fetcher,
Expand All @@ -159,18 +159,20 @@ bool VerifyUsingPathBuilder(
net::der::GeneralizedTime time = ConvertExplodedTime(exploded_time);

net::TrustStoreInMemory additional_roots;
for (const auto& der_cert : root_der_certs) {
for (const auto& cert_input_with_trust : der_certs_with_trust_settings) {
std::shared_ptr<const net::ParsedCertificate> cert =
ParseCertificate(der_cert);
ParseCertificate(cert_input_with_trust.cert_input);
if (cert) {
additional_roots.AddTrustAnchor(std::move(cert));
additional_roots.AddCertificate(std::move(cert),
cert_input_with_trust.trust);
}
}
net::TrustStoreCollection trust_store;
trust_store.AddTrustStore(&additional_roots);
trust_store.AddTrustStore(system_trust_store->GetTrustStore());

if (!system_trust_store->UsesSystemTrustStore() && root_der_certs.empty()) {
if (!system_trust_store->UsesSystemTrustStore() &&
der_certs_with_trust_settings.empty()) {
std::cerr << "NOTE: CertPathBuilder does not currently use OS trust "
"settings (--roots must be specified).\n";
}
Expand Down
3 changes: 2 additions & 1 deletion net/tools/cert_verify_tool/verify_using_path_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class SystemTrustStore;
}

struct CertInput;
struct CertInputWithTrustSetting;

// Verifies |target_der_cert| using CertPathBuilder. Returns true if the
// certificate verified successfully, false if it failed to verify or there was
Expand All @@ -28,7 +29,7 @@ struct CertInput;
bool VerifyUsingPathBuilder(
const CertInput& target_der_cert,
const std::vector<CertInput>& intermediate_der_certs,
const std::vector<CertInput>& root_der_certs,
const std::vector<CertInputWithTrustSetting>& der_certs_with_trust_settings,
const base::Time at_time,
const base::FilePath& dump_prefix_path,
scoped_refptr<net::CertNetFetcher> cert_net_fetcher,
Expand Down

0 comments on commit 36d0168

Please sign in to comment.