Skip to content

Commit

Permalink
Use log list timestamp to calculate timeliness.
Browse files Browse the repository at this point in the history
This CL changes CT enforcement so the log list included timestamp is
used after a CT log list is updated via component updater, instead of
the build date.

Bug: 1199877
Change-Id: I74165814fa29f7ff5d148b28ab6aad6012fa027a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2946451
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893099}
  • Loading branch information
carlosjoan91 authored and Chromium LUCI CQ committed Jun 16, 2021
1 parent c482676 commit 1518adb
Show file tree
Hide file tree
Showing 17 changed files with 151 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ void PKIMetadataComponentInstallerPolicy::UpdateNetworkServiceOnUI(
log_list_mojo.push_back(std::move(log_ptr));
}

network_service->UpdateCtLogList(std::move(log_list_mojo));
base::Time update_time =
base::Time::UnixEpoch() +
base::TimeDelta::FromSeconds(proto->log_list().timestamp().seconds()) +
base::TimeDelta::FromNanoseconds(proto->log_list().timestamp().nanos());
network_service->UpdateCtLogList(std::move(log_list_mojo), update_time);
#endif // BUILDFLAG(IS_CT_SUPPORTED)
}

Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/net/system_network_context_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,8 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
}
log_list_mojo.push_back(std::move(log_info));
}
network_service->UpdateCtLogList(std::move(log_list_mojo));
network_service->UpdateCtLogList(std::move(log_list_mojo),
base::GetBuildTime());
}
#endif

Expand Down Expand Up @@ -680,7 +681,6 @@ void SystemNetworkContextManager::ConfigureDefaultNetworkContextParams(

if (g_enable_certificate_transparency) {
network_context_params->enforce_chrome_ct_policy = true;
network_context_params->ct_log_update_time = base::GetBuildTime();
}

#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,10 @@ CTPolicyCompliance ChromeCTPolicyEnforcer::CheckCompliance(
}

void ChromeCTPolicyEnforcer::UpdateCTLogList(
base::Time update_time,
std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs,
std::vector<std::string> operated_by_google_logs) {
log_list_date_ = update_time;
disqualified_logs_ = std::move(disqualified_logs);
operated_by_google_logs_ = std::move(operated_by_google_logs);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class ChromeCTPolicyEnforcer : public net::CTPolicyEnforcer {
// a map of log ID to disqualification date. |operated_by_google_logs| is a
// list of log IDs operated by Google
void UpdateCTLogList(
base::Time update_time,
std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs,
std::vector<std::string> operated_by_google_logs);

Expand Down Expand Up @@ -95,7 +96,7 @@ class ChromeCTPolicyEnforcer : public net::CTPolicyEnforcer {

// The time at which |disqualified_logs_| and |operated_by_google_logs_| were
// generated.
const base::Time log_list_date_;
base::Time log_list_date_;
};

} // namespace certificate_transparency
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ TEST_F(ChromeCTPolicyEnforcerTest, UpdateCTLogList) {

std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs;
std::vector<std::string> operated_by_google_logs;
chrome_policy_enforcer->UpdateCTLogList(disqualified_logs,
chrome_policy_enforcer->UpdateCTLogList(base::Time::Now(), disqualified_logs,
operated_by_google_logs);

// The check should fail since the Google Aviator log is no longer in the
Expand All @@ -492,7 +492,7 @@ TEST_F(ChromeCTPolicyEnforcerTest, UpdateCTLogList) {
// Update the list again, this time including all the known operated by Google
// logs.
operated_by_google_logs = certificate_transparency::GetLogsOperatedByGoogle();
chrome_policy_enforcer->UpdateCTLogList(disqualified_logs,
chrome_policy_enforcer->UpdateCTLogList(base::Time::Now(), disqualified_logs,
operated_by_google_logs);

// The check should now succeed.
Expand All @@ -501,6 +501,37 @@ TEST_F(ChromeCTPolicyEnforcerTest, UpdateCTLogList) {
NetLogWithSource()));
}

TEST_F(ChromeCTPolicyEnforcerTest, TimestampUpdates) {
ChromeCTPolicyEnforcer* chrome_policy_enforcer =
static_cast<ChromeCTPolicyEnforcer*>(policy_enforcer_.get());
SCTList scts;
FillListWithSCTsOfOrigin(SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION,
2, &scts);

// Clear the log list and set the last updated time to more than 10 weeks ago.
std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs;
std::vector<std::string> operated_by_google_logs;
chrome_policy_enforcer->UpdateCTLogList(
base::Time::Now() - base::TimeDelta::FromDays(71), disqualified_logs,
operated_by_google_logs);

// The check should return build not timely even though the Google Aviator log
// is no longer in the list, since the last update time is greater than 10
// weeks.
EXPECT_EQ(CTPolicyCompliance::CT_POLICY_BUILD_NOT_TIMELY,
chrome_policy_enforcer->CheckCompliance(chain_.get(), scts,
NetLogWithSource()));

// Update the last update time value again, this time with a recent time.
chrome_policy_enforcer->UpdateCTLogList(base::Time::Now(), disqualified_logs,
operated_by_google_logs);

// The check should now fail
EXPECT_EQ(CTPolicyCompliance::CT_POLICY_NOT_DIVERSE_SCTS,
chrome_policy_enforcer->CheckCompliance(chain_.get(), scts,
NetLogWithSource()));
}

} // namespace

} // namespace certificate_transparency
17 changes: 16 additions & 1 deletion content/shell/browser/shell_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "net/ssl/client_cert_identity.h"
#include "services/device/public/cpp/geolocation/location_system_permission_status.h"
#include "services/network/public/mojom/ct_log_info.mojom.h"
#include "services/network/public/mojom/network_context.mojom.h"
#include "services/network/public/mojom/network_service.mojom.h"
#include "third_party/blink/public/common/features.h"
Expand Down Expand Up @@ -563,7 +564,6 @@ void ShellContentBrowserClient::ConfigureNetworkContextParamsForShell(

if (g_enable_expect_ct_for_testing) {
context_params->enforce_chrome_ct_policy = true;
context_params->ct_log_update_time = base::Time::Now();
context_params->enable_expect_ct_reporting = true;
}
}
Expand Down Expand Up @@ -653,4 +653,19 @@ void ShellContentBrowserClient::SetUpFieldTrials() {
&safe_seed_manager, absl::nullopt);
}

void ShellContentBrowserClient::OnNetworkServiceCreated(
network::mojom::NetworkService* network_service) {
// Explicitly configure Certificate Transparency with no logs, but with
// a fresh enough log update time that policy enforcement will still be
// run. This does not use base::GetBuildTime(), as that may cause certain
// checks to be disabled if too far in the past. Callers that set
// `g_enable_expect_ct_reporting` are expected to simulate CT verification
// using `MockCertVerifier` (otherwise CT validation would fail due to the
// empty log list).
if (g_enable_expect_ct_for_testing) {
network_service->UpdateCtLogList(
std::vector<network::mojom::CTLogInfoPtr>(), base::Time::Now());
}
}

} // namespace content
2 changes: 2 additions & 0 deletions content/shell/browser/shell_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ class ShellContentBrowserClient : public ContentBrowserClient {
void GetHyphenationDictionary(
base::OnceCallback<void(const base::FilePath&)>) override;
bool HasErrorPage(int http_status_code) override;
void OnNetworkServiceCreated(
network::mojom::NetworkService* network_service) override;

void CreateFeatureListAndFieldTrials();

Expand Down
25 changes: 23 additions & 2 deletions net/http/transport_security_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,12 @@ void SetTransportSecurityStateSourceForTesting(
}

TransportSecurityState::TransportSecurityState()
: TransportSecurityState(std::vector<std::string>()) {}
: TransportSecurityState(std::vector<std::string>()) {
// By default the CT log list is treated as last updated at build time (since
// a compiled-in list is used), this is overridden if the list is dynamically
// updated.
ct_log_list_last_update_time_ = base::GetBuildTime();
}

TransportSecurityState::TransportSecurityState(
std::vector<std::string> hsts_host_bypass_list)
Expand Down Expand Up @@ -607,6 +612,10 @@ void TransportSecurityState::SetRequireCTDelegate(RequireCTDelegate* delegate) {
require_ct_delegate_ = delegate;
}

void TransportSecurityState::SetCTLogListUpdateTime(base::Time update_time) {
ct_log_list_last_update_time_ = update_time;
}

void TransportSecurityState::AddHSTSInternal(
const std::string& host,
TransportSecurityState::STSState::UpgradeMode upgrade_mode,
Expand Down Expand Up @@ -773,7 +782,7 @@ bool TransportSecurityState::GetStaticExpectCTState(
ExpectCTState* expect_ct_state) const {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);

if (!IsBuildTimely())
if (!IsCTLogListTimely())
return false;

PreloadResult result;
Expand Down Expand Up @@ -1531,4 +1540,16 @@ bool TransportSecurityState::ExpectCTPruningSorter(
it2->second.last_observed);
}

bool TransportSecurityState::IsCTLogListTimely() const {
// Preloaded Expect-CT is enforced if the CT log list is timely. Note that
// unlike HSTS and HPKP, the date of the preloaded list itself (i.e.
// base::GetBuildTime()) is not directly consulted. Consulting the
// build time would allow sites that have subsequently disabled Expect-CT
// to opt-out. However, because as of June 2021, all unexpired certificates
// are already expected to comply with the policies expressed by Expect-CT,
// there's no need to offer an opt-out.
return (base::Time::Now() - ct_log_list_last_update_time_).InDays() <
70 /* 10 weeks */;
}

} // namespace net
7 changes: 7 additions & 0 deletions net/http/transport_security_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,8 @@ class NET_EXPORT TransportSecurityState {
ct_emergency_disable_ = emergency_disable;
}

void SetCTLogListUpdateTime(base::Time update_time);

// Clears all dynamic data (e.g. HSTS and HPKP data).
//
// Does NOT persist changes using the Delegate, as this function is only
Expand Down Expand Up @@ -691,6 +693,9 @@ class NET_EXPORT TransportSecurityState {
static bool ExpectCTPruningSorter(const ExpectCTStateMap::iterator& it1,
const ExpectCTStateMap::iterator& it2);

// Returns true if the CT log list has been updated in the last 10 weeks.
bool IsCTLogListTimely() const;

// The sets of hosts that have enabled TransportSecurity. |domain| will always
// be empty for a STSState, PKPState, or ExpectCTState in these maps; the
// domain comes from the map keys instead. In addition, |upgrade_mode| in the
Expand Down Expand Up @@ -736,6 +741,8 @@ class NET_EXPORT TransportSecurityState {

bool ct_emergency_disable_ = false;

base::Time ct_log_list_last_update_time_;

THREAD_CHECKER(thread_checker_);

DISALLOW_COPY_AND_ASSIGN(TransportSecurityState);
Expand Down
28 changes: 28 additions & 0 deletions net/http/transport_security_state_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1804,6 +1804,34 @@ TEST_F(TransportSecurityStateTest, CTEmergencyDisable) {
NetworkIsolationKey()));
}

// Tests that the if the CT log list last update time is set, it is used for
// enforcement decisions.
TEST_F(TransportSecurityStateTest, CTTimestampUpdate) {
TransportSecurityState state;
TransportSecurityStateTest::EnableStaticExpectCT(&state);
TransportSecurityState::ExpectCTState expect_ct_state;
// Initially the preloaded host should require CT.
EXPECT_TRUE(
GetExpectCTState(&state, kExpectCTStaticHostname, &expect_ct_state));

// Change the last updated time to a value greater than 10 weeks.
// We use a close value (70 days + 1 hour ago) to ensure rounding behavior is
// working properly.
state.SetCTLogListUpdateTime(
base::Time::Now() -
(base::TimeDelta::FromDays(70) + base::TimeDelta::FromHours(1)));
// CT should no longer be required.
EXPECT_FALSE(
GetExpectCTState(&state, kExpectCTStaticHostname, &expect_ct_state));

// CT should once again be required after the log list is newer than 70 days.
state.SetCTLogListUpdateTime(
base::Time::Now() -
(base::TimeDelta::FromDays(70) - base::TimeDelta::FromHours(1)));
EXPECT_TRUE(
GetExpectCTState(&state, kExpectCTStaticHostname, &expect_ct_state));
}

// Tests that Certificate Transparency is required for Symantec-issued
// certificates, unless the certificate was issued prior to 1 June 2016
// or the issuing CA is permitted as independently operated.
Expand Down
12 changes: 6 additions & 6 deletions services/network/network_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1274,17 +1274,18 @@ void NetworkContext::SetSCTAuditingEnabled(bool enabled) {
}

void NetworkContext::OnCTLogListUpdated(
const std::vector<network::mojom::CTLogInfoPtr>& log_list) {
const std::vector<network::mojom::CTLogInfoPtr>& log_list,
base::Time update_time) {
if (!ct_policy_enforcer_)
return;
std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs;
std::vector<std::string> operated_by_google_logs;
GetCTPolicyConfigForCTLogInfo(log_list, &disqualified_logs,
&operated_by_google_logs);
ct_policy_enforcer_->UpdateCTLogList(std::move(disqualified_logs),
ct_policy_enforcer_->UpdateCTLogList(update_time,
std::move(disqualified_logs),
std::move(operated_by_google_logs));
}

#endif // BUILDFLAG(IS_CT_SUPPORTED)

void NetworkContext::CreateUDPSocket(
Expand Down Expand Up @@ -1995,10 +1996,9 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
&disqualified_logs, &operated_by_google_logs);
auto ct_policy_enforcer =
std::make_unique<certificate_transparency::ChromeCTPolicyEnforcer>(
params_->ct_log_update_time, std::move(disqualified_logs),
std::move(operated_by_google_logs));
network_service_->ct_log_list_update_time(),
std::move(disqualified_logs), std::move(operated_by_google_logs));
ct_policy_enforcer_ = ct_policy_enforcer.get();

builder.set_ct_policy_enforcer(std::move(ct_policy_enforcer));
}

Expand Down
3 changes: 2 additions & 1 deletion services/network/network_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
signed_certificate_timestamps);
void SetSCTAuditingEnabled(bool enabled) override;
void OnCTLogListUpdated(
const std::vector<network::mojom::CTLogInfoPtr>& log_list);
const std::vector<network::mojom::CTLogInfoPtr>& log_list,
base::Time update_time);
bool is_sct_auditing_enabled() { return is_sct_auditing_enabled_; }
#endif // BUILDFLAG(IS_CT_SUPPORTED)
void CreateUDPSocket(
Expand Down
4 changes: 2 additions & 2 deletions services/network/network_context_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6078,12 +6078,12 @@ TEST_F(NetworkContextTest, CertificateTransparencyConfig) {

log_list_mojo.push_back(std::move(log_info));
}
network_service()->UpdateCtLogList(std::move(log_list_mojo));
network_service()->UpdateCtLogList(std::move(log_list_mojo),
base::Time::Now());

// Configure CT params in network context.
mojom::NetworkContextParamsPtr params = CreateContextParams();
params->enforce_chrome_ct_policy = true;
params->ct_log_update_time = base::Time::Now();

std::unique_ptr<NetworkContext> network_context =
CreateContextWithParams(std::move(params));
Expand Down
11 changes: 8 additions & 3 deletions services/network/network_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -730,15 +730,20 @@ void NetworkService::ConfigureSCTAuditing(
sct_auditing_cache_->set_url_loader_factory(std::move(factory));
}

void NetworkService::UpdateCtLogList(
std::vector<mojom::CTLogInfoPtr> log_list) {
void NetworkService::UpdateCtLogList(std::vector<mojom::CTLogInfoPtr> log_list,
base::Time update_time) {
log_list_ = std::move(log_list);
ct_log_list_update_time_ = update_time;

if (base::FeatureList::IsEnabled(
certificate_transparency::features::
kCertificateTransparencyComponentUpdater)) {
ct_log_list_distributor_->OnNewCtConfig(log_list_);
for (auto* context : network_contexts_) {
context->OnCTLogListUpdated(log_list_);
context->OnCTLogListUpdated(log_list_, update_time);
context->url_request_context()
->transport_security_state()
->SetCTLogListUpdateTime(update_time);
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion services/network/network_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
const GURL& reporting_uri,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
mojo::PendingRemote<mojom::URLLoaderFactory> factory) override;
void UpdateCtLogList(std::vector<mojom::CTLogInfoPtr> log_list) override;
void UpdateCtLogList(std::vector<mojom::CTLogInfoPtr> log_list,
base::Time update_time) override;
void SetCtEnforcementEnabled(bool enabled) override;
#endif

Expand Down Expand Up @@ -268,6 +269,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
SCTAuditingCache* sct_auditing_cache() { return sct_auditing_cache_.get(); }

const std::vector<mojom::CTLogInfoPtr>& log_list() const { return log_list_; }

base::Time ct_log_list_update_time() const {
return ct_log_list_update_time_;
}
#endif

mojom::URLLoaderNetworkServiceObserver*
Expand Down Expand Up @@ -374,6 +379,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
std::vector<mojom::CTLogInfoPtr> log_list_;

std::unique_ptr<CtLogListDistributor> ct_log_list_distributor_;

base::Time ct_log_list_update_time_;
#endif

// Map from a renderer process id, to the set of plugin origins embedded by
Expand Down
Loading

0 comments on commit 1518adb

Please sign in to comment.