0

Make some TransportSecurityState methods take std::string_views.

The most important of which is AddHSTSHeader(). The CL also makes the
primary caller of that function use the new
HttpResponseHeaders::EnumerateHeader() overload to avoid a string copy.

Bug: None
Change-Id: Iddb3044d059b99a6d8bd19f7ff1d4fba8683ba5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5893454
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: mmenke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1361410}
This commit is contained in:
Matt Menke
2024-09-27 23:26:42 +00:00
committed by Chromium LUCI CQ
parent df9214e28c
commit 1773c8b0f7
11 changed files with 32 additions and 29 deletions

@@ -176,7 +176,7 @@ ChromeRequireCTDelegate::~ChromeRequireCTDelegate() {}
net::TransportSecurityState::RequireCTDelegate::CTRequirementLevel net::TransportSecurityState::RequireCTDelegate::CTRequirementLevel
ChromeRequireCTDelegate::IsCTRequiredForHost( ChromeRequireCTDelegate::IsCTRequiredForHost(
const std::string& hostname, std::string_view hostname,
const net::X509Certificate* chain, const net::X509Certificate* chain,
const net::HashValueVector& spki_hashes) { const net::HashValueVector& spki_hashes) {
if (MatchHostname(hostname) || MatchSPKI(chain, spki_hashes)) { if (MatchHostname(hostname) || MatchSPKI(chain, spki_hashes)) {
@@ -203,7 +203,7 @@ void ChromeRequireCTDelegate::UpdateCTPolicies(
ParseSpkiHashes(excluded_spkis, &spkis_); ParseSpkiHashes(excluded_spkis, &spkis_);
} }
bool ChromeRequireCTDelegate::MatchHostname(const std::string& hostname) const { bool ChromeRequireCTDelegate::MatchHostname(std::string_view hostname) const {
if (url_matcher_->IsEmpty()) if (url_matcher_->IsEmpty())
return false; return false;

@@ -7,6 +7,7 @@
#include <memory> #include <memory>
#include <string> #include <string>
#include <string_view>
#include <vector> #include <vector>
#include "base/component_export.h" #include "base/component_export.h"
@@ -44,7 +45,7 @@ class COMPONENT_EXPORT(CERTIFICATE_TRANSPARENCY) ChromeRequireCTDelegate
// RequireCTDelegate implementation // RequireCTDelegate implementation
CTRequirementLevel IsCTRequiredForHost( CTRequirementLevel IsCTRequiredForHost(
const std::string& hostname, std::string_view hostname,
const net::X509Certificate* chain, const net::X509Certificate* chain,
const net::HashValueVector& spki_hashes) override; const net::HashValueVector& spki_hashes) override;
@@ -62,7 +63,7 @@ class COMPONENT_EXPORT(CERTIFICATE_TRANSPARENCY) ChromeRequireCTDelegate
// Returns true if a policy to disable Certificate Transparency for |hostname| // Returns true if a policy to disable Certificate Transparency for |hostname|
// is found. // is found.
bool MatchHostname(const std::string& hostname) const; bool MatchHostname(std::string_view hostname) const;
// Returns true if a policy to disable Certificate Transparency for |chain|, // Returns true if a policy to disable Certificate Transparency for |chain|,
// which contains the SPKI hashes |hashes|, is found. // which contains the SPKI hashes |hashes|, is found.

@@ -69,7 +69,7 @@ bool MaxAgeToLimitedInt(std::string_view s, uint32_t limit, uint32_t* result) {
// the UA, the UA MUST ignore the unrecognized directives and if the // the UA, the UA MUST ignore the unrecognized directives and if the
// STS header field otherwise satisfies the above requirements (1 // STS header field otherwise satisfies the above requirements (1
// through 4), the UA MUST process the recognized directives. // through 4), the UA MUST process the recognized directives.
bool ParseHSTSHeader(const std::string& value, bool ParseHSTSHeader(std::string_view value,
base::TimeDelta* max_age, base::TimeDelta* max_age,
bool* include_subdomains) { bool* include_subdomains) {
uint32_t max_age_value = 0; uint32_t max_age_value = 0;

@@ -7,7 +7,7 @@
#include <stdint.h> #include <stdint.h>
#include <string> #include <string_view>
#include "base/time/time.h" #include "base/time/time.h"
#include "net/base/hash_value.h" #include "net/base/hash_value.h"
@@ -29,7 +29,7 @@ const uint32_t kMaxHPKPAgeSecs = 86400 * 60; // 60 days
// //
// "Strict-Transport-Security" ":" // "Strict-Transport-Security" ":"
// [ directive ] *( ";" [ directive ] ) // [ directive ] *( ";" [ directive ] )
bool NET_EXPORT_PRIVATE ParseHSTSHeader(const std::string& value, bool NET_EXPORT_PRIVATE ParseHSTSHeader(std::string_view value,
base::TimeDelta* max_age, base::TimeDelta* max_age,
bool* include_subdomains); bool* include_subdomains);

@@ -87,7 +87,7 @@ bool AddHash(const char* sha256_hash, HashValueVector* out) {
// Converts |hostname| from dotted form ("www.google.com") to the form // Converts |hostname| from dotted form ("www.google.com") to the form
// used in DNS: "\x03www\x06google\x03com", lowercases that, and returns // used in DNS: "\x03www\x06google\x03com", lowercases that, and returns
// the result. // the result.
std::vector<uint8_t> CanonicalizeHost(const std::string& host) { std::vector<uint8_t> CanonicalizeHost(std::string_view host) {
// We cannot perform the operations as detailed in the spec here as `host` // We cannot perform the operations as detailed in the spec here as `host`
// has already undergone IDN processing before it reached us. Thus, we // has already undergone IDN processing before it reached us. Thus, we
// lowercase the input (probably redudnant since most input here has been // lowercase the input (probably redudnant since most input here has been
@@ -335,7 +335,6 @@ TransportSecurityState::CheckCTRequirements(
const X509Certificate* validated_certificate_chain, const X509Certificate* validated_certificate_chain,
ct::CTPolicyCompliance policy_compliance) { ct::CTPolicyCompliance policy_compliance) {
using CTRequirementLevel = RequireCTDelegate::CTRequirementLevel; using CTRequirementLevel = RequireCTDelegate::CTRequirementLevel;
std::string hostname = host_port_pair.host();
// If CT is emergency disabled, we don't require CT for any host. // If CT is emergency disabled, we don't require CT for any host.
if (ct_emergency_disable_) { if (ct_emergency_disable_) {
@@ -361,7 +360,7 @@ TransportSecurityState::CheckCTRequirements(
if (require_ct_delegate_) { if (require_ct_delegate_) {
// Allow the delegate to override the CT requirement state. // Allow the delegate to override the CT requirement state.
ct_required = require_ct_delegate_->IsCTRequiredForHost( ct_required = require_ct_delegate_->IsCTRequiredForHost(
hostname, validated_certificate_chain, public_key_hashes); host_port_pair.host(), validated_certificate_chain, public_key_hashes);
} }
switch (ct_required) { switch (ct_required) {
case CTRequirementLevel::REQUIRED: case CTRequirementLevel::REQUIRED:
@@ -405,7 +404,7 @@ void TransportSecurityState::UpdatePinList(
} }
void TransportSecurityState::AddHSTSInternal( void TransportSecurityState::AddHSTSInternal(
const std::string& host, std::string_view host,
TransportSecurityState::STSState::UpgradeMode upgrade_mode, TransportSecurityState::STSState::UpgradeMode upgrade_mode,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains) { bool include_subdomains) {
@@ -434,7 +433,7 @@ void TransportSecurityState::AddHSTSInternal(
DirtyNotify(); DirtyNotify();
} }
void TransportSecurityState::AddHPKPInternal(const std::string& host, void TransportSecurityState::AddHPKPInternal(std::string_view host,
const base::Time& last_observed, const base::Time& last_observed,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains, bool include_subdomains,
@@ -565,8 +564,8 @@ void TransportSecurityState::DirtyNotify() {
delegate_->StateIsDirty(this); delegate_->StateIsDirty(this);
} }
bool TransportSecurityState::AddHSTSHeader(const std::string& host, bool TransportSecurityState::AddHSTSHeader(std::string_view host,
const std::string& value) { std::string_view value) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
base::Time now = base::Time::Now(); base::Time now = base::Time::Now();
@@ -588,14 +587,14 @@ bool TransportSecurityState::AddHSTSHeader(const std::string& host,
return true; return true;
} }
void TransportSecurityState::AddHSTS(const std::string& host, void TransportSecurityState::AddHSTS(std::string_view host,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains) { bool include_subdomains) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
AddHSTSInternal(host, STSState::MODE_FORCE_HTTPS, expiry, include_subdomains); AddHSTSInternal(host, STSState::MODE_FORCE_HTTPS, expiry, include_subdomains);
} }
void TransportSecurityState::AddHPKP(const std::string& host, void TransportSecurityState::AddHPKP(std::string_view host,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains, bool include_subdomains,
const HashValueVector& hashes) { const HashValueVector& hashes) {

@@ -12,6 +12,7 @@
#include <optional> #include <optional>
#include <set> #include <set>
#include <string> #include <string>
#include <string_view>
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/functional/callback.h" #include "base/functional/callback.h"
@@ -105,7 +106,7 @@ class NET_EXPORT TransportSecurityState {
// |chain| are not guaranteed to be in the same order - that is, the first // |chain| are not guaranteed to be in the same order - that is, the first
// hash in |hashes| is NOT guaranteed to be for the leaf cert in |chain|. // hash in |hashes| is NOT guaranteed to be for the leaf cert in |chain|.
virtual CTRequirementLevel IsCTRequiredForHost( virtual CTRequirementLevel IsCTRequiredForHost(
const std::string& hostname, std::string_view hostname,
const X509Certificate* chain, const X509Certificate* chain,
const HashValueVector& hashes) = 0; const HashValueVector& hashes) = 0;
@@ -417,17 +418,17 @@ class NET_EXPORT TransportSecurityState {
// Processes an HSTS header value from the host, adding entries to // Processes an HSTS header value from the host, adding entries to
// dynamic state if necessary. // dynamic state if necessary.
bool AddHSTSHeader(const std::string& host, const std::string& value); bool AddHSTSHeader(std::string_view host, std::string_view value);
// Adds explicitly-specified data as if it was processed from an // Adds explicitly-specified data as if it was processed from an
// HSTS header (used for net-internals and unit tests). // HSTS header (used for net-internals and unit tests).
void AddHSTS(const std::string& host, void AddHSTS(std::string_view host,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains); bool include_subdomains);
// Adds explicitly-specified data as if it was processed from an HPKP header. // Adds explicitly-specified data as if it was processed from an HPKP header.
// Note: dynamic PKP data is not persisted. // Note: dynamic PKP data is not persisted.
void AddHPKP(const std::string& host, void AddHPKP(std::string_view host,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains, bool include_subdomains,
const HashValueVector& hashes); const HashValueVector& hashes);
@@ -485,11 +486,11 @@ class NET_EXPORT TransportSecurityState {
// any previous state for the |host|, including static entries. // any previous state for the |host|, including static entries.
// //
// The new state for |host| is persisted using the Delegate (if any). // The new state for |host| is persisted using the Delegate (if any).
void AddHSTSInternal(const std::string& host, void AddHSTSInternal(std::string_view host,
STSState::UpgradeMode upgrade_mode, STSState::UpgradeMode upgrade_mode,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains); bool include_subdomains);
void AddHPKPInternal(const std::string& host, void AddHPKPInternal(std::string_view host,
const base::Time& last_observed, const base::Time& last_observed,
const base::Time& expiry, const base::Time& expiry,
bool include_subdomains, bool include_subdomains,

@@ -99,7 +99,7 @@ const char* const kBadPath[] = {
class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate { class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate {
public: public:
MOCK_METHOD3(IsCTRequiredForHost, MOCK_METHOD3(IsCTRequiredForHost,
CTRequirementLevel(const std::string& hostname, CTRequirementLevel(std::string_view hostname,
const X509Certificate* chain, const X509Certificate* chain,
const HashValueVector& hashes)); const HashValueVector& hashes));
}; };

@@ -71,7 +71,7 @@ class FailsTestCertVerifier : public CertVerifier {
class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate { class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate {
public: public:
MOCK_METHOD3(IsCTRequiredForHost, MOCK_METHOD3(IsCTRequiredForHost,
CTRequirementLevel(const std::string& host, CTRequirementLevel(std::string_view host,
const X509Certificate* chain, const X509Certificate* chain,
const HashValueVector& hashes)); const HashValueVector& hashes));
}; };

@@ -589,7 +589,7 @@ class DeleteSocketCallback : public TestCompletionCallbackBase {
class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate { class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate {
public: public:
MOCK_METHOD3(IsCTRequiredForHost, MOCK_METHOD3(IsCTRequiredForHost,
CTRequirementLevel(const std::string& host, CTRequirementLevel(std::string_view host,
const X509Certificate* chain, const X509Certificate* chain,
const HashValueVector& hashes)); const HashValueVector& hashes));
}; };

@@ -111,7 +111,7 @@ base::TimeTicks InstantaneousReads() {
class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate { class MockRequireCTDelegate : public TransportSecurityState::RequireCTDelegate {
public: public:
MOCK_METHOD3(IsCTRequiredForHost, MOCK_METHOD3(IsCTRequiredForHost,
CTRequirementLevel(const std::string& host, CTRequirementLevel(std::string_view host,
const X509Certificate* chain, const X509Certificate* chain,
const HashValueVector& hashes)); const HashValueVector& hashes));
}; };

@@ -1213,9 +1213,11 @@ void URLRequestHttpJob::ProcessStrictTransportSecurityHeader() {
// message over secure transport, then the UA MUST process only the // message over secure transport, then the UA MUST process only the
// first such header field. // first such header field.
HttpResponseHeaders* headers = GetResponseHeaders(); HttpResponseHeaders* headers = GetResponseHeaders();
std::string value; std::optional<std::string_view> value;
if (headers->EnumerateHeader(nullptr, "Strict-Transport-Security", &value)) if ((value =
security_state->AddHSTSHeader(request_info_.url.host(), value); headers->EnumerateHeader(nullptr, "Strict-Transport-Security"))) {
security_state->AddHSTSHeader(request_info_.url.host(), *value);
}
} }
void URLRequestHttpJob::OnStartCompleted(int result) { void URLRequestHttpJob::OnStartCompleted(int result) {