Skip to content

Commit

Permalink
clean up CT requirement date check
Browse files Browse the repository at this point in the history 
CT enforcement is required for all publicly trusted certificates since
2018-05-01 and no certificates from before that date could be valid
anymore, so we can require it unconditionally now.

Change-Id: I7af456780ba3056a0632a1bb9ee17a12bad1ce7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4903244
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1202928}
  • Loading branch information
@matt-mueller
matt-mueller authored and Chromium LUCI CQ committed Sep 28, 2023
1 parent 03aa8e7 commit 0024fe7
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 90 deletions.
11 changes: 3 additions & 8 deletions components/certificate_transparency/chrome_require_ct_delegate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,9 @@ ChromeRequireCTDelegate::IsCTRequiredForHost(
return CTRequirementLevel::NOT_REQUIRED;
}

// Compute >= 2018-05-01, rather than deal with possible fractional
// seconds.
const base::Time kMay_1_2018 =
base::Time::UnixEpoch() + base::Seconds(1525132800);
if (chain->valid_start() >= kMay_1_2018)
return CTRequirementLevel::REQUIRED;

return CTRequirementLevel::DEFAULT;
// CT is required since 2018-05-01, and no certificate issued before that
// date could be valid anymore, so CT is unconditionally required.
return CTRequirementLevel::REQUIRED;
}

void ChromeRequireCTDelegate::UpdateCTPolicies(
Expand Down
110 changes: 28 additions & 82 deletions components/certificate_transparency/chrome_require_ct_delegate_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ namespace {
class ChromeRequireCTDelegateTest : public ::testing::Test {
public:
void SetUp() override {
// Use a certificate with a notBefore prior to May 2018, so that CT is not
// implicitly required.
cert_ = net::CreateCertificateChainFromFile(
net::GetTestCertsDirectory(), "expired_cert.pem",
net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE);
ASSERT_TRUE(cert_);
hashes_.push_back(net::HashValue(net::SHA256HashValue{0}));

net::HashValue spki_hash;
ASSERT_TRUE(net::x509_util::CalculateSha256SpkiHash(cert_->cert_buffer(),
&spki_hash));
hashes_.push_back(spki_hash);
}

protected:
Expand All @@ -58,13 +60,13 @@ TEST_F(ChromeRequireCTDelegateTest, RegistersPrefs) {
EXPECT_NE(registered_prefs, newly_registered_prefs);
}

TEST_F(ChromeRequireCTDelegateTest, DelegateChecksExcluded) {
TEST_F(ChromeRequireCTDelegateTest, DelegateChecksExcludedHosts) {
using CTRequirementLevel =
net::TransportSecurityState::RequireCTDelegate::CTRequirementLevel;
ChromeRequireCTDelegate delegate;

// No setting should yield the default results.
EXPECT_EQ(CTRequirementLevel::DEFAULT,
EXPECT_EQ(CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));

// Add a excluded host
Expand All @@ -75,13 +77,30 @@ TEST_F(ChromeRequireCTDelegateTest, DelegateChecksExcluded) {
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));
}

TEST_F(ChromeRequireCTDelegateTest, DelegateChecksExcludedSPKIs) {
using CTRequirementLevel =
net::TransportSecurityState::RequireCTDelegate::CTRequirementLevel;
ChromeRequireCTDelegate delegate;

// No setting should yield the default results.
EXPECT_EQ(CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));

// Add a excluded SPKI
delegate.UpdateCTPolicies({}, {hashes_.front().ToString()}, {});

// The new setting should take effect.
EXPECT_EQ(CTRequirementLevel::NOT_REQUIRED,
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));
}

TEST_F(ChromeRequireCTDelegateTest, IgnoresInvalidEntries) {
using CTRequirementLevel =
net::TransportSecurityState::RequireCTDelegate::CTRequirementLevel;
ChromeRequireCTDelegate delegate;

// No setting should yield the default results.
EXPECT_EQ(CTRequirementLevel::DEFAULT,
EXPECT_EQ(CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));

// Now setup invalid state (that is, that fail to be parsable as
Expand All @@ -92,7 +111,7 @@ TEST_F(ChromeRequireCTDelegateTest, IgnoresInvalidEntries) {
{}, {});

// Wildcards are ignored (both * and https://*).
EXPECT_EQ(CTRequirementLevel::DEFAULT,
EXPECT_EQ(CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));
// File URL hosts are ignored.
// TODO(rsleevi): https://crbug.com/841407 - Ensure that file URLs have their
Expand Down Expand Up @@ -255,7 +274,7 @@ TEST_F(ChromeRequireCTDelegateTest, SupportsLegacyCaRestrictions) {
hashes_.push_back(net::HashValue(legacy_spki));

// No setting should yield the default results.
EXPECT_EQ(CTRequirementLevel::DEFAULT,
EXPECT_EQ(CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));

// Setting to a non-legacy CA should not work.
Expand All @@ -264,7 +283,7 @@ TEST_F(ChromeRequireCTDelegateTest, SupportsLegacyCaRestrictions) {

// This setting should have no effect, because the hash for |cert_|
// is not a legacy CA hash.
EXPECT_EQ(CTRequirementLevel::DEFAULT,
EXPECT_EQ(CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));

// Now set to a truly legacy CA, and create a chain that
Expand All @@ -274,79 +293,6 @@ TEST_F(ChromeRequireCTDelegateTest, SupportsLegacyCaRestrictions) {
delegate.IsCTRequiredForHost("google.com", cert_.get(), hashes_));
}

TEST_F(ChromeRequireCTDelegateTest, RequiresCTAfterApril2018) {
using CTRequirementLevel =
net::TransportSecurityState::RequireCTDelegate::CTRequirementLevel;
ChromeRequireCTDelegate delegate;

EXPECT_EQ(CTRequirementLevel::DEFAULT,
delegate.IsCTRequiredForHost("example.com", cert_.get(), hashes_));

scoped_refptr<net::X509Certificate> may_2018 =
net::CreateCertificateChainFromFile(
net::GetTestCertsDirectory(), "may_2018.pem",
net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE);
ASSERT_TRUE(may_2018);

net::HashValueVector new_hashes;
new_hashes.push_back(net::HashValue(
net::X509Certificate::CalculateFingerprint256(may_2018->cert_buffer())));

EXPECT_EQ(
CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("example.com", may_2018.get(), new_hashes));
}

TEST_F(ChromeRequireCTDelegateTest,
PoliciesCheckedBeforeRequiringCTAfterApril2018) {
using CTRequirementLevel =
net::TransportSecurityState::RequireCTDelegate::CTRequirementLevel;
ChromeRequireCTDelegate delegate;

scoped_refptr<net::X509Certificate> may_2018 =
net::CreateCertificateChainFromFile(
net::GetTestCertsDirectory(), "may_2018.pem",
net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE);
ASSERT_TRUE(may_2018);

net::HashValueVector new_hashes;
net::HashValue leaf_hash;
ASSERT_TRUE(net::x509_util::CalculateSha256SpkiHash(may_2018->cert_buffer(),
&leaf_hash));
new_hashes.push_back(std::move(leaf_hash));

EXPECT_EQ(
CTRequirementLevel::REQUIRED,
delegate.IsCTRequiredForHost("example.com", may_2018.get(), new_hashes));

// Check excluding by hostname.
delegate.UpdateCTPolicies({"example.com"}, {}, {});
EXPECT_EQ(
CTRequirementLevel::NOT_REQUIRED,
delegate.IsCTRequiredForHost("example.com", may_2018.get(), new_hashes));

// Check excluding by leaf hash.
delegate.UpdateCTPolicies({}, {new_hashes.front().ToString()}, {});
EXPECT_EQ(
CTRequirementLevel::NOT_REQUIRED,
delegate.IsCTRequiredForHost("example.com", may_2018.get(), new_hashes));

// Check excluding by legacy CA hash.

// The hash of a known legacy CA. See
// //net/cert/root_cert_list_generated.h
net::SHA256HashValue legacy_spki = {{
0x00, 0x6C, 0xB2, 0x26, 0xA7, 0x72, 0xC7, 0x18, 0x2D, 0x77, 0x72,
0x38, 0x3E, 0x37, 0x3F, 0x0F, 0x22, 0x9E, 0x7D, 0xFE, 0x34, 0x44,
0x81, 0x0A, 0x8D, 0x6E, 0x50, 0x90, 0x5D, 0x20, 0xD6, 0x61,
}};
new_hashes.push_back(net::HashValue(legacy_spki));
delegate.UpdateCTPolicies({}, {}, {new_hashes.back().ToString()});
EXPECT_EQ(
CTRequirementLevel::NOT_REQUIRED,
delegate.IsCTRequiredForHost("example.com", may_2018.get(), new_hashes));
}

} // namespace

} // namespace certificate_transparency

0 comments on commit 0024fe7

Please sign in to comment.