Skip to content

Commit

Permalink
Revert 224269 "Don't persist HPKP if PrivacyMode is enabled."
Browse files Browse the repository at this point in the history
It broke Google Chrome ChromeOS bot.
http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20ChromeOS/builds/58548/steps/compile/logs/stdio#error1
FAILED: g++ ... -c ../../net/socket/ssl_client_socket_nss.cc -o obj/net/socket/net.ssl_client_socket_nss.o
../../net/socket/ssl_client_socket_nss.cc: In member function 'int net::SSLClientSocketNSS::DoVerifyCertComplete(int)':
../../net/socket/ssl_client_socket_nss.cc:3445:64:error: no matching function for call to 'net::TransportSecurityState::GetDomainState(const string&, bool&, net::TransportSecurityState::DomainState*)'
../../net/socket/ssl_client_socket_nss.cc:3445:64: note: candidate is:
../../net/http/transport_security_state.h:212:8: note: bool net::TransportSecurityState::GetDomainState(const string&, bool, bool, net::TransportSecurityState::DomainState*)
../../net/http/transport_security_state.h:212:8: note:   candidate expects 4 arguments, 3 provided

> Don't persist HPKP if PrivacyMode is enabled.
> 
> BUG=258667
> 
> Review URL: https://chromiumcodereview.appspot.com/19269012

TBR=mef@chromium.org

Review URL: https://codereview.chromium.org/24251011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224275 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tkent@chromium.org committed Sep 20, 2013
1 parent d9343b3 commit 22e045f
Show file tree
Hide file tree
Showing 17 changed files with 109 additions and 197 deletions.
18 changes: 8 additions & 10 deletions chrome/browser/net/transport_security_persister_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ TEST_F(TransportSecurityPersisterTest, SerializeData2) {
const base::Time expiry = current_time + base::TimeDelta::FromSeconds(1000);
static const char kYahooDomain[] = "yahoo.com";

EXPECT_FALSE(state_.GetDomainState(kYahooDomain, true, true, &domain_state));
EXPECT_FALSE(state_.GetDomainState(kYahooDomain, true, &domain_state));

bool include_subdomains = true;
state_.AddHSTS(kYahooDomain, expiry, include_subdomains);
Expand All @@ -77,22 +77,20 @@ TEST_F(TransportSecurityPersisterTest, SerializeData2) {
EXPECT_TRUE(persister_->SerializeData(&output));
EXPECT_TRUE(persister_->LoadEntries(output, &dirty));

EXPECT_TRUE(state_.GetDomainState(kYahooDomain, true, true, &domain_state));
EXPECT_TRUE(state_.GetDomainState(kYahooDomain, true, &domain_state));
EXPECT_EQ(domain_state.upgrade_mode,
TransportSecurityState::DomainState::MODE_FORCE_HTTPS);
EXPECT_TRUE(state_.GetDomainState("foo.yahoo.com", true, true,
&domain_state));
EXPECT_TRUE(state_.GetDomainState("foo.yahoo.com", true, &domain_state));
EXPECT_EQ(domain_state.upgrade_mode,
TransportSecurityState::DomainState::MODE_FORCE_HTTPS);
EXPECT_TRUE(state_.GetDomainState("foo.bar.yahoo.com", true, true,
&domain_state));
EXPECT_TRUE(state_.GetDomainState("foo.bar.yahoo.com", true, &domain_state));
EXPECT_EQ(domain_state.upgrade_mode,
TransportSecurityState::DomainState::MODE_FORCE_HTTPS);
EXPECT_TRUE(state_.GetDomainState("foo.bar.baz.yahoo.com", true,
true, &domain_state));
&domain_state));
EXPECT_EQ(domain_state.upgrade_mode,
TransportSecurityState::DomainState::MODE_FORCE_HTTPS);
EXPECT_FALSE(state_.GetDomainState("com", true, true, &domain_state));
EXPECT_FALSE(state_.GetDomainState("com", true, &domain_state));
}

TEST_F(TransportSecurityPersisterTest, SerializeData3) {
Expand Down Expand Up @@ -179,7 +177,7 @@ TEST_F(TransportSecurityPersisterTest, SerializeDataOld) {
TEST_F(TransportSecurityPersisterTest, PublicKeyHashes) {
TransportSecurityState::DomainState domain_state;
static const char kTestDomain[] = "example.com";
EXPECT_FALSE(state_.GetDomainState(kTestDomain, false, true, &domain_state));
EXPECT_FALSE(state_.GetDomainState(kTestDomain, false, &domain_state));
net::HashValueVector hashes;
EXPECT_FALSE(domain_state.CheckPublicKeyPins(hashes));

Expand All @@ -205,7 +203,7 @@ TEST_F(TransportSecurityPersisterTest, PublicKeyHashes) {
EXPECT_TRUE(persister_->SerializeData(&ser));
bool dirty;
EXPECT_TRUE(persister_->LoadEntries(ser, &dirty));
EXPECT_TRUE(state_.GetDomainState(kTestDomain, false, true, &domain_state));
EXPECT_TRUE(state_.GetDomainState(kTestDomain, false, &domain_state));
EXPECT_EQ(1u, domain_state.dynamic_spki_hashes.size());
EXPECT_EQ(sha1.tag, domain_state.dynamic_spki_hashes[0].tag);
EXPECT_EQ(0, memcmp(domain_state.dynamic_spki_hashes[0].data(), sha1.data(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,18 @@ void ChromeResourceDispatcherHostDelegate::OnResponseStarted(
const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);

if (request->url().SchemeIsSecure()) {
if (request->GetHSTSRedirect(NULL)) {
const net::URLRequestContext* context = request->context();
net::TransportSecurityState* state = context->transport_security_state();
if (state) {
net::TransportSecurityState::DomainState domain_state;
bool has_sni = net::SSLConfigService::IsSNIAvailable(
context->ssl_config_service());
if (state->GetDomainState(request->url().host(), has_sni,
&domain_state) &&
domain_state.ShouldUpgradeToSSL()) {
sender->Send(new ChromeViewMsg_AddStrictSecurityHost(
info->GetRouteID(), request->url().host()));
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/webui/net_internals/net_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1260,7 +1260,7 @@ void NetInternalsMessageHandler::IOThreadImpl::OnHSTSQuery(
} else {
net::TransportSecurityState::DomainState state;
const bool found = transport_security_state->GetDomainState(
domain, true, true, &state);
domain, true, &state);

result->SetBoolean("result", found);
if (found) {
Expand Down
1 change: 0 additions & 1 deletion chrome_frame/test/net/fake_external_tab.cc
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ void FilterDisabledTests() {
// functionality they test (HTTP Strict Transport Security and
// HTTP-based Public Key Pinning) does not work in Chrome Frame anyway.
"URLRequestTestHTTP.ProcessPKP",
"URLRequestTestHTTP.ProcessPKP_PrivacyMode",
"URLRequestTestHTTP.ProcessSTS",
"URLRequestTestHTTP.ProcessSTSOnce",
"URLRequestTestHTTP.ProcessSTSAndPKP",
Expand Down
4 changes: 2 additions & 2 deletions net/http/http_security_headers_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ TEST_F(HttpSecurityHeadersTest, UpdateDynamicPKPOnly) {

// docs.google.com has preloaded pins.
std::string domain = "docs.google.com";
EXPECT_TRUE(state.GetDomainState(domain, true, true, &domain_state));
EXPECT_TRUE(state.GetDomainState(domain, true, &domain_state));
EXPECT_GT(domain_state.static_spki_hashes.size(), 1UL);
HashValueVector saved_hashes = domain_state.static_spki_hashes;

Expand Down Expand Up @@ -487,7 +487,7 @@ TEST_F(HttpSecurityHeadersTest, UpdateDynamicPKPOnly) {
EXPECT_NE(dynamic_domain_state.dynamic_spki_hashes.end(), hash);

// Expect the overall state to reflect the header, too.
EXPECT_TRUE(state.GetDomainState(domain, true, true, &domain_state));
EXPECT_TRUE(state.GetDomainState(domain, true, &domain_state));
EXPECT_EQ(2UL, domain_state.dynamic_spki_hashes.size());

hash = std::find_if(domain_state.dynamic_spki_hashes.begin(),
Expand Down
10 changes: 2 additions & 8 deletions net/http/transport_security_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ bool TransportSecurityState::DeleteDynamicDataForHost(const std::string& host) {

bool TransportSecurityState::GetDomainState(const std::string& host,
bool sni_enabled,
bool allow_dynamic,
DomainState* result) {
DCHECK(CalledOnValidThread());

Expand All @@ -148,12 +147,6 @@ bool TransportSecurityState::GetDomainState(const std::string& host,

bool has_preload = GetStaticDomainState(canonicalized_host, sni_enabled,
&state);
// If |allow_dynamic| is false, then return static state to the caller.
if (!allow_dynamic) {
if (has_preload)
*result = state;
return has_preload;
}
std::string canonicalized_preload = CanonicalizeHost(state.domain);
GetDynamicDomainState(host, &state);

Expand Down Expand Up @@ -843,7 +836,8 @@ TransportSecurityState::DomainState::DomainState()
: upgrade_mode(MODE_DEFAULT),
created(base::Time::Now()),
sts_include_subdomains(false),
pkp_include_subdomains(false) {}
pkp_include_subdomains(false) {
}

TransportSecurityState::DomainState::~DomainState() {
}
Expand Down
4 changes: 0 additions & 4 deletions net/http/transport_security_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,13 @@ class NET_EXPORT TransportSecurityState
// If |sni_enabled| is true, searches the static pins defined for
// SNI-using hosts as well as the rest of the pins.
//
// If |allow_dynamic| is true, then dynamic state is returned if present,
// otherwise only static state is used..
//
// If |host| matches both an exact entry and is a subdomain of another
// entry, the exact match determines the return value.
//
// Note that this method is not const because it opportunistically removes
// entries that have expired.
bool GetDomainState(const std::string& host,
bool sni_enabled,
bool allow_dynamic,
DomainState* result);

// Processes an HSTS header value from the host, adding entries to
Expand Down
Loading

0 comments on commit 22e045f

Please sign in to comment.