Skip to content

Commit

Permalink
Update ChromeCTPolicyEnforcer log list via component
Browse files Browse the repository at this point in the history
Prior to this CL, if CT log list updates via component updater were
enabled, they would update MultiLogCTVerifier (which verifies SCTs),
but not ChromeCTPolicyEnforcer (which checks policy compliance). This
CL adds plumbing so the enforcer is also updated.

Bug: 1199877
Change-Id: I616eb1b4ace2b9c78fb150bdadb8ed681b2b009c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2941844
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#891400}
  • Loading branch information
carlosjoan91 authored and Chromium LUCI CQ committed Jun 10, 2021
1 parent b23db6b commit 34f0425
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ CTPolicyCompliance ChromeCTPolicyEnforcer::CheckCompliance(
return compliance;
}

void ChromeCTPolicyEnforcer::UpdateCTLogList(
std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs,
std::vector<std::string> operated_by_google_logs) {
disqualified_logs_ = std::move(disqualified_logs);
operated_by_google_logs_ = std::move(operated_by_google_logs);
}

bool ChromeCTPolicyEnforcer::IsLogDisqualified(
base::StringPiece log_id,
base::Time* disqualification_date) const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ class ChromeCTPolicyEnforcer : public net::CTPolicyEnforcer {
const net::ct::SCTList& verified_scts,
const net::NetLogWithSource& net_log) override;

// Updates the list of logs used for compliance checks. |disqualified_logs| is
// a map of log ID to disqualification date. |operated_by_google_logs| is a
// list of log IDs operated by Google
void UpdateCTLogList(
std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs,
std::vector<std::string> operated_by_google_logs);

void SetClockForTesting(const base::Clock* clock) { clock_ = clock; }

// TODO(https://crbug.com/999240): These are exposed to allow end-to-end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,36 @@ TEST_F(ChromeCTPolicyEnforcerTest,
}
}

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

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

// The check should fail since the Google Aviator log is no longer in the
// list after the update with an empty list.
EXPECT_EQ(CTPolicyCompliance::CT_POLICY_NOT_DIVERSE_SCTS,
chrome_policy_enforcer->CheckCompliance(chain_.get(), scts,
NetLogWithSource()));

// 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,
operated_by_google_logs);

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

} // namespace

} // namespace certificate_transparency
63 changes: 45 additions & 18 deletions services/network/network_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,30 @@ bool SCTAuditingDelegate::IsSCTAuditingEnabled() {
return false;
return context_->is_sct_auditing_enabled();
}

// Filters `log_list` for disqualified or Google-operated logs,
// returning them as sorted vectors in `disqualified_logs` and
// `operated_by_google_logs` suitable for use with a `CTPolicyEnforcer`.
void GetCTPolicyConfigForCTLogInfo(
const std::vector<mojom::CTLogInfoPtr>& log_list,
std::vector<std::pair<std::string, base::TimeDelta>>* disqualified_logs,
std::vector<std::string>* operated_by_google_logs) {
for (const auto& log : log_list) {
if (log->operated_by_google || log->disqualified_at) {
std::string log_id = crypto::SHA256HashString(log->public_key);
if (log->operated_by_google)
operated_by_google_logs->push_back(std::move(log_id));
if (log->disqualified_at) {
disqualified_logs->emplace_back(std::move(log_id),
log->disqualified_at.value());
}
}
}

std::sort(std::begin(*operated_by_google_logs),
std::end(*operated_by_google_logs));
std::sort(std::begin(*disqualified_logs), std::end(*disqualified_logs));
}
#endif // BUILDFLAG(IS_CT_SUPPORTED)

} // namespace
Expand Down Expand Up @@ -1248,6 +1272,19 @@ void NetworkContext::MaybeEnqueueSCTReport(
void NetworkContext::SetSCTAuditingEnabled(bool enabled) {
is_sct_auditing_enabled_ = enabled;
}

void NetworkContext::OnCTLogListUpdated(
const std::vector<network::mojom::CTLogInfoPtr>& log_list) {
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),
std::move(operated_by_google_logs));
}

#endif // BUILDFLAG(IS_CT_SUPPORTED)

void NetworkContext::CreateUDPSocket(
Expand Down Expand Up @@ -1961,25 +1998,15 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
if (params_->enforce_chrome_ct_policy) {
std::vector<std::pair<std::string, base::TimeDelta>> disqualified_logs;
std::vector<std::string> operated_by_google_logs;
for (const auto& log : network_service_->log_list()) {
if (log->operated_by_google || log->disqualified_at) {
std::string log_id = crypto::SHA256HashString(log->public_key);
if (log->operated_by_google)
operated_by_google_logs.push_back(log_id);
if (log->disqualified_at) {
disqualified_logs.emplace_back(log_id, log->disqualified_at.value());
}
}
}

std::sort(std::begin(operated_by_google_logs),
std::end(operated_by_google_logs));
std::sort(std::begin(disqualified_logs), std::end(disqualified_logs));

builder.set_ct_policy_enforcer(
GetCTPolicyConfigForCTLogInfo(network_service_->log_list(),
&disqualified_logs, &operated_by_google_logs);
auto ct_policy_enforcer =
std::make_unique<certificate_transparency::ChromeCTPolicyEnforcer>(
params_->ct_log_update_time, disqualified_logs,
operated_by_google_logs));
params_->ct_log_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));
}

builder.set_sct_auditing_delegate(
Expand Down
7 changes: 7 additions & 0 deletions services/network/network_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class URLRequestContext;

namespace certificate_transparency {
class ChromeRequireCTDelegate;
class ChromeCTPolicyEnforcer;
} // namespace certificate_transparency

namespace domain_reliability {
Expand Down Expand Up @@ -276,6 +277,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
const net::SignedCertificateTimestampAndStatusList&
signed_certificate_timestamps);
void SetSCTAuditingEnabled(bool enabled) override;
void OnCTLogListUpdated(
const std::vector<network::mojom::CTLogInfoPtr>& log_list);
bool is_sct_auditing_enabled() { return is_sct_auditing_enabled_; }
#endif // BUILDFLAG(IS_CT_SUPPORTED)
void CreateUDPSocket(
Expand Down Expand Up @@ -693,6 +696,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
std::queue<SetExpectCTTestReportCallback>
outstanding_set_expect_ct_callbacks_;

// Owned by the URLRequestContext.
certificate_transparency::ChromeCTPolicyEnforcer* ct_policy_enforcer_ =
nullptr;

bool is_sct_auditing_enabled_ = false;
#endif // BUILDFLAG(IS_CT_SUPPORTED)

Expand Down
3 changes: 3 additions & 0 deletions services/network/network_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,9 @@ void NetworkService::UpdateCtLogList(
certificate_transparency::features::
kCertificateTransparencyComponentUpdater)) {
ct_log_list_distributor_->OnNewCtConfig(log_list_);
for (auto* context : network_contexts_) {
context->OnCTLogListUpdated(log_list_);
}
}
}

Expand Down

0 comments on commit 34f0425

Please sign in to comment.