Commit 4b65b587 authored by Matthew Braithwaite's avatar Matthew Braithwaite Committed by Commit Bot

CTPolicyEnforcer::CheckCompliance: allow for a testing |Clock|.

This is a followup to b5cccac5, and has the same motivation: to
improve testability of Certificate Transparency enforcement.
Injecting the time of evaluation allows a test to be written that
shows that while a call may return |CT_POLICY_COMPLIES_VIA_SCTS|
today, the same inputs may return |CT_POLICY_BUILD_NOT_TIMELY|
tomorrow.

Change-Id: If4a4519ccaf7b1470477fc4c1978de1d14e58aa8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626016
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: default avatarRyan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664063}
parent 1f54c905
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/numerics/safe_conversions.h" #include "base/numerics/safe_conversions.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/time/default_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/values.h" #include "base/values.h"
#include "base/version.h" #include "base/version.h"
...@@ -109,6 +110,7 @@ ChromeCTPolicyEnforcer::ChromeCTPolicyEnforcer( ...@@ -109,6 +110,7 @@ ChromeCTPolicyEnforcer::ChromeCTPolicyEnforcer(
std::vector<std::string> operated_by_google_logs) std::vector<std::string> operated_by_google_logs)
: disqualified_logs_(disqualified_logs), : disqualified_logs_(disqualified_logs),
operated_by_google_logs_(operated_by_google_logs), operated_by_google_logs_(operated_by_google_logs),
clock_(base::DefaultClock::GetInstance()),
log_list_date_(log_list_date) {} log_list_date_(log_list_date) {}
ChromeCTPolicyEnforcer::~ChromeCTPolicyEnforcer() {} ChromeCTPolicyEnforcer::~ChromeCTPolicyEnforcer() {}
...@@ -163,7 +165,7 @@ bool ChromeCTPolicyEnforcer::IsLogOperatedByGoogle( ...@@ -163,7 +165,7 @@ bool ChromeCTPolicyEnforcer::IsLogOperatedByGoogle(
bool ChromeCTPolicyEnforcer::IsLogDataTimely() const { bool ChromeCTPolicyEnforcer::IsLogDataTimely() const {
// We consider built-in information to be timely for 10 weeks. // We consider built-in information to be timely for 10 weeks.
return (base::Time::Now() - log_list_date_).InDays() < 70 /* 10 weeks */; return (clock_->Now() - log_list_date_).InDays() < 70 /* 10 weeks */;
} }
// Evaluates against the policy specified at // Evaluates against the policy specified at
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/time/clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "net/cert/ct_policy_enforcer.h" #include "net/cert/ct_policy_enforcer.h"
...@@ -42,6 +43,8 @@ class ChromeCTPolicyEnforcer : public net::CTPolicyEnforcer { ...@@ -42,6 +43,8 @@ class ChromeCTPolicyEnforcer : public net::CTPolicyEnforcer {
const net::ct::SCTList& verified_scts, const net::ct::SCTList& verified_scts,
const net::NetLogWithSource& net_log) override; const net::NetLogWithSource& net_log) override;
void SetClockForTesting(const base::Clock* clock) { clock_ = clock; }
private: private:
// Returns true if the log identified by |log_id| (the SHA-256 hash of the // Returns true if the log identified by |log_id| (the SHA-256 hash of the
// log's DER-encoded SPKI) has been disqualified, and sets // log's DER-encoded SPKI) has been disqualified, and sets
...@@ -68,6 +71,8 @@ class ChromeCTPolicyEnforcer : public net::CTPolicyEnforcer { ...@@ -68,6 +71,8 @@ class ChromeCTPolicyEnforcer : public net::CTPolicyEnforcer {
// List of SHA-256(SPKI) for logs operated by Google. // List of SHA-256(SPKI) for logs operated by Google.
std::vector<std::string> operated_by_google_logs_; std::vector<std::string> operated_by_google_logs_;
const base::Clock* clock_;
// The time at which |disqualified_logs_| and |operated_by_google_logs_| were // The time at which |disqualified_logs_| and |operated_by_google_logs_| were
// generated. // generated.
const base::Time log_list_date_; const base::Time log_list_date_;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/build_time.h" #include "base/build_time.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/test/simple_test_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/version.h" #include "base/version.h"
#include "components/certificate_transparency/ct_known_logs.h" #include "components/certificate_transparency/ct_known_logs.h"
...@@ -44,9 +45,10 @@ static_assert(base::size(kGoogleAviatorLogID) - 1 == crypto::kSHA256Length, ...@@ -44,9 +45,10 @@ static_assert(base::size(kGoogleAviatorLogID) - 1 == crypto::kSHA256Length,
class ChromeCTPolicyEnforcerTest : public ::testing::Test { class ChromeCTPolicyEnforcerTest : public ::testing::Test {
public: public:
void SetUp() override { void SetUp() override {
policy_enforcer_.reset( auto enforcer = std::make_unique<ChromeCTPolicyEnforcer>(
new ChromeCTPolicyEnforcer(base::GetBuildTime(), GetDisqualifiedLogs(), base::GetBuildTime(), GetDisqualifiedLogs(), GetLogsOperatedByGoogle());
GetLogsOperatedByGoogle())); enforcer->SetClockForTesting(&clock_);
policy_enforcer_.reset(enforcer.release());
std::string der_test_cert(net::ct::GetDerEncodedX509Cert()); std::string der_test_cert(net::ct::GetDerEncodedX509Cert());
chain_ = X509Certificate::CreateFromBytes(der_test_cert.data(), chain_ = X509Certificate::CreateFromBytes(der_test_cert.data(),
...@@ -54,6 +56,7 @@ class ChromeCTPolicyEnforcerTest : public ::testing::Test { ...@@ -54,6 +56,7 @@ class ChromeCTPolicyEnforcerTest : public ::testing::Test {
ASSERT_TRUE(chain_.get()); ASSERT_TRUE(chain_.get());
google_log_id_ = std::string(kGoogleAviatorLogID, crypto::kSHA256Length); google_log_id_ = std::string(kGoogleAviatorLogID, crypto::kSHA256Length);
non_google_log_id_.assign(crypto::kSHA256Length, 1); non_google_log_id_.assign(crypto::kSHA256Length, 1);
clock_.SetNow(base::Time::Now());
} }
void FillListWithSCTsOfOrigin( void FillListWithSCTsOfOrigin(
...@@ -126,6 +129,7 @@ class ChromeCTPolicyEnforcerTest : public ::testing::Test { ...@@ -126,6 +129,7 @@ class ChromeCTPolicyEnforcerTest : public ::testing::Test {
} }
protected: protected:
base::SimpleTestClock clock_;
std::unique_ptr<net::CTPolicyEnforcer> policy_enforcer_; std::unique_ptr<net::CTPolicyEnforcer> policy_enforcer_;
scoped_refptr<X509Certificate> chain_; scoped_refptr<X509Certificate> chain_;
std::string google_log_id_; std::string google_log_id_;
...@@ -183,6 +187,22 @@ TEST_F(ChromeCTPolicyEnforcerTest, ConformsToCTPolicyWithNonEmbeddedSCTs) { ...@@ -183,6 +187,22 @@ TEST_F(ChromeCTPolicyEnforcerTest, ConformsToCTPolicyWithNonEmbeddedSCTs) {
NetLogWithSource())); NetLogWithSource()));
} }
TEST_F(ChromeCTPolicyEnforcerTest, EnforcementDisabledByBinaryAge) {
SCTList scts;
FillListWithSCTsOfOrigin(SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION,
2, &scts);
EXPECT_EQ(CTPolicyCompliance::CT_POLICY_COMPLIES_VIA_SCTS,
policy_enforcer_->CheckCompliance(chain_.get(), scts,
NetLogWithSource()));
clock_.Advance(base::TimeDelta::FromDays(71));
EXPECT_EQ(CTPolicyCompliance::CT_POLICY_BUILD_NOT_TIMELY,
policy_enforcer_->CheckCompliance(chain_.get(), scts,
NetLogWithSource()));
}
TEST_F(ChromeCTPolicyEnforcerTest, ConformsToCTPolicyWithEmbeddedSCTs) { TEST_F(ChromeCTPolicyEnforcerTest, ConformsToCTPolicyWithEmbeddedSCTs) {
// |chain_| is valid for 10 years - over 121 months - so requires 5 SCTs. // |chain_| is valid for 10 years - over 121 months - so requires 5 SCTs.
SCTList scts; SCTList scts;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment