diff --git a/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers b/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers index fe3259dd1cd5ac..09ec14aaf6994d 100644 --- a/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers +++ b/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers @@ -3,4 +3,4 @@ Cache-Control: private Content-Type: text/html; charset=ISO-8859-1 X-Multiple-Entries: a X-Multiple-Entries: b -Public-Key-Pins-Report-Only: max-age=50000; pin-sha1="K9e3/nFL5j90GuVJOJBv6WXpvcs="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY="; pin-sha1="PshSs8WOjC7qwaYMv0T3rJDwKS4="; report-uri="https://hpkp-report.test" +Public-Key-Pins-Report-Only: max-age=50000; pin-sha256="9999999999999999999999999999999999999999999="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY="; report-uri="https://hpkp-report.test" diff --git a/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers b/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers index c1763cc9837601..27301366d4cf1c 100644 --- a/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers +++ b/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers @@ -3,4 +3,4 @@ Cache-Control: private Content-Type: text/html; charset=ISO-8859-1 X-Multiple-Entries: a X-Multiple-Entries: b -Public-Key-Pins: max-age=50000; pin-sha1="K9e3/nFL5j90GuVJOJBv6WXpvcs="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY="; pin-sha1="PshSs8WOjC7qwaYMv0T3rJDwKS4="; report-uri="https://hpkp-report.test" +Public-Key-Pins: max-age=50000; pin-sha256="9999999999999999999999999999999999999999999="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY="; report-uri="https://hpkp-report.test" diff --git a/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers b/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers index 2bcfd2aeb799d2..21d54aaf1db87b 100644 --- a/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers +++ b/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers @@ -5,4 +5,4 @@ X-Multiple-Entries: a X-Multiple-Entries: b Strict-Transport-Security: max-age=12300 Strict-Transport-Security: max-age=12300; includeSubdomains -Public-Key-Pins: max-age=50000; pin-sha1="Wws2/Z7YhKlX73v3rYHBBxO4OLE="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY=" +Public-Key-Pins: max-age=50000; pin-sha256="9999999999999999999999999999999999999999999="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY=" diff --git a/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers b/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers index f4b9aed1eb8eb3..45b26508fb72ca 100644 --- a/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers +++ b/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers @@ -4,5 +4,5 @@ Content-Type: text/html; charset=ISO-8859-1 X-Multiple-Entries: a X-Multiple-Entries: b Strict-Transport-Security: max-age=12300; includeSubdomains -Public-Key-Pins: max-age=50000; pin-sha1="K9e3/nFL5j90GuVJOJBv6WXpvcs="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY="; pin-sha1="PshSs8WOjC7qwaYMv0T3rJDwKS4=" -Public-Key-Pins: max-age=50000; pin-sha1="K9e3/nFL5j90GuVJOJBv6WXpvcs="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY="; pin-sha1="PshSs8WOjC7qwaYMv0T3rJDwKS4=" +Public-Key-Pins: max-age=50000; pin-sha256="9999999999999999999999999999999999999999999="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY=" +Public-Key-Pins: max-age=50000; pin-sha256="9999999999999999999999999999999999999999999="; pin-sha256="+TTrWvvJdM9gwuHiLTApo/2DBT2xb4hBPRJDI9pebXY=" diff --git a/net/http/http_response_headers_unittest.cc b/net/http/http_response_headers_unittest.cc index 0184885dcd3101..35a63608c051d1 100644 --- a/net/http/http_response_headers_unittest.cc +++ b/net/http/http_response_headers_unittest.cc @@ -467,8 +467,8 @@ const struct PersistData persistence_tests[] = { "Strict-Transport-Security: max-age=1576800\n" "Bar: 1\n" "Public-Key-Pins: max-age=100000; " - "pin-sha1=\"ObT42aoSpAqWdY9WfRfL7i0HsVk=\";" - "pin-sha1=\"7kW49EVwZG0hSNx41ZO/fUPN0ek=\"", + "pin-sha256=\"1111111111111111111111111111111111111111111=\";" + "pin-sha256=\"2222222222222222222222222222222222222222222=\"", "HTTP/1.1 200 OK\n" "Bar: 1\n"}, diff --git a/net/http/http_security_headers.cc b/net/http/http_security_headers.cc index 597adaa96bf873..b99b206c3f3214 100644 --- a/net/http/http_security_headers.cc +++ b/net/http/http_security_headers.cc @@ -147,17 +147,6 @@ bool ParseHPKPHeaderImpl(const std::string& value, return false; } parsed_max_age = true; - } else if (base::LowerCaseEqualsASCII( - base::StringPiece(name_value_pairs.name_begin(), - name_value_pairs.name_end()), - "pin-sha1")) { - // Pins are always quoted. - if (!name_value_pairs.value_is_quoted() || - !ParseAndAppendPin(name_value_pairs.value_begin(), - name_value_pairs.value_end(), HASH_VALUE_SHA1, - &pins)) { - return false; - } } else if (base::LowerCaseEqualsASCII( base::StringPiece(name_value_pairs.name_begin(), name_value_pairs.name_end()), diff --git a/net/http/http_security_headers_unittest.cc b/net/http/http_security_headers_unittest.cc index f9e3e18636c618..4a510f2f11d7f8 100644 --- a/net/http/http_security_headers_unittest.cc +++ b/net/http/http_security_headers_unittest.cc @@ -6,7 +6,6 @@ #include #include "base/base64.h" -#include "base/sha1.h" #include "base/strings/string_piece.h" #include "crypto/sha2.h" #include "net/base/host_port_pair.h" @@ -36,9 +35,6 @@ std::string GetTestPinImpl(uint8 label, HashValueTag tag, bool quoted) { std::string ret; switch (hash_value.tag) { - case HASH_VALUE_SHA1: - ret = "pin-sha1="; - break; case HASH_VALUE_SHA256: ret = "pin-sha256="; break; @@ -645,18 +641,10 @@ static void TestValidPKPHeaders(HashValueTag tag) { EXPECT_EQ(expect_report_uri, report_uri); } -TEST_F(HttpSecurityHeadersTest, BogusPinsHeadersSHA1) { - TestBogusPinsHeaders(HASH_VALUE_SHA1); -} - TEST_F(HttpSecurityHeadersTest, BogusPinsHeadersSHA256) { TestBogusPinsHeaders(HASH_VALUE_SHA256); } -TEST_F(HttpSecurityHeadersTest, ValidPKPHeadersSHA1) { - TestValidPKPHeaders(HASH_VALUE_SHA1); -} - TEST_F(HttpSecurityHeadersTest, ValidPKPHeadersSHA256) { TestValidPKPHeaders(HASH_VALUE_SHA256); } @@ -675,10 +663,10 @@ TEST_F(HttpSecurityHeadersTest, UpdateDynamicPKPOnly) { HashValueVector saved_hashes = static_pkp_state.spki_hashes; // Add a header, which should only update the dynamic state. - HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA1); - HashValue backup_hash = GetTestHashValue(2, HASH_VALUE_SHA1); - std::string good_pin = GetTestPin(1, HASH_VALUE_SHA1); - std::string backup_pin = GetTestPin(2, HASH_VALUE_SHA1); + HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA256); + HashValue backup_hash = GetTestHashValue(2, HASH_VALUE_SHA256); + std::string good_pin = GetTestPin(1, HASH_VALUE_SHA256); + std::string backup_pin = GetTestPin(2, HASH_VALUE_SHA256); GURL report_uri("http://google.com"); std::string header = "max-age = 10000; " + good_pin + "; " + backup_pin + ";report-uri=\"" + report_uri.spec() + "\""; @@ -756,9 +744,9 @@ TEST_F(HttpSecurityHeadersTest, UpdateDynamicPKPMaxAge0) { HashValueVector saved_hashes = static_pkp_state.spki_hashes; // Add a header, which should only update the dynamic state. - HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA1); - std::string good_pin = GetTestPin(1, HASH_VALUE_SHA1); - std::string backup_pin = GetTestPin(2, HASH_VALUE_SHA1); + HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA256); + std::string good_pin = GetTestPin(1, HASH_VALUE_SHA256); + std::string backup_pin = GetTestPin(2, HASH_VALUE_SHA256); std::string header = "max-age = 10000; " + good_pin + "; " + backup_pin; // Construct a fake SSLInfo that will pass AddHPKPHeader's checks. @@ -858,9 +846,9 @@ TEST_F(HttpSecurityHeadersTest, NoClobberPins) { TransportSecurityState::DISABLE_PIN_REPORTS, &failure_log)); // Add an HPKP header, which should only update the dynamic state. - HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA1); - std::string good_pin = GetTestPin(1, HASH_VALUE_SHA1); - std::string backup_pin = GetTestPin(2, HASH_VALUE_SHA1); + HashValue good_hash = GetTestHashValue(1, HASH_VALUE_SHA256); + std::string good_pin = GetTestPin(1, HASH_VALUE_SHA256); + std::string backup_pin = GetTestPin(2, HASH_VALUE_SHA256); std::string header = "max-age = 10000; " + good_pin + "; " + backup_pin; // Construct a fake SSLInfo that will pass AddHPKPHeader's checks. diff --git a/net/http/transport_security_persister.cc b/net/http/transport_security_persister.cc index 9dd5455a453595..78898c327e589e 100644 --- a/net/http/transport_security_persister.cc +++ b/net/http/transport_security_persister.cc @@ -189,6 +189,11 @@ bool TransportSecurityPersister::SerializeData(std::string* output) { serialized->SetDouble(kDynamicSPKIHashesExpiry, pkp_state.expiry.ToDoubleT()); + // TODO(svaldez): Historically, both SHA-1 and SHA-256 hashes were + // accepted in pins. Per spec, only SHA-256 is accepted now, however + // existing serialized pins are still processed. Migrate historical pins + // with SHA-1 hashes properly, either by dropping just the bad hashes or + // the entire pin. See https://crbug.com/448501. if (now < pkp_state.expiry) { serialized->Set(kDynamicSPKIHashes, SPKIHashesToListValue(pkp_state.spki_hashes)); diff --git a/net/http/transport_security_persister_unittest.cc b/net/http/transport_security_persister_unittest.cc index 7e460ad49865f7..0799f5820bdaf9 100644 --- a/net/http/transport_security_persister_unittest.cc +++ b/net/http/transport_security_persister_unittest.cc @@ -90,9 +90,9 @@ TEST_F(TransportSecurityPersisterTest, SerializeData2) { TEST_F(TransportSecurityPersisterTest, SerializeData3) { const GURL report_uri(kReportUri); // Add an entry. - HashValue fp1(HASH_VALUE_SHA1); + HashValue fp1(HASH_VALUE_SHA256); memset(fp1.data(), 0, fp1.size()); - HashValue fp2(HASH_VALUE_SHA1); + HashValue fp2(HASH_VALUE_SHA256); memset(fp2.data(), 1, fp2.size()); base::Time expiry = base::Time::Now() + base::TimeDelta::FromSeconds(1000); @@ -193,13 +193,13 @@ TEST_F(TransportSecurityPersisterTest, PublicKeyPins) { std::string failure_log; EXPECT_FALSE(pkp_state.CheckPublicKeyPins(hashes, &failure_log)); - HashValue sha1(HASH_VALUE_SHA1); - memset(sha1.data(), '1', sha1.size()); - pkp_state.spki_hashes.push_back(sha1); + HashValue sha256(HASH_VALUE_SHA256); + memset(sha256.data(), '1', sha256.size()); + pkp_state.spki_hashes.push_back(sha256); EXPECT_FALSE(pkp_state.CheckPublicKeyPins(hashes, &failure_log)); - hashes.push_back(sha1); + hashes.push_back(sha256); EXPECT_TRUE(pkp_state.CheckPublicKeyPins(hashes, &failure_log)); hashes[0].data()[0] = '2'; @@ -219,9 +219,9 @@ TEST_F(TransportSecurityPersisterTest, PublicKeyPins) { TransportSecurityState::PKPState new_pkp_state; EXPECT_TRUE(state_.GetDynamicPKPState(kTestDomain, &new_pkp_state)); EXPECT_EQ(1u, new_pkp_state.spki_hashes.size()); - EXPECT_EQ(sha1.tag, new_pkp_state.spki_hashes[0].tag); - EXPECT_EQ( - 0, memcmp(new_pkp_state.spki_hashes[0].data(), sha1.data(), sha1.size())); + EXPECT_EQ(sha256.tag, new_pkp_state.spki_hashes[0].tag); + EXPECT_EQ(0, memcmp(new_pkp_state.spki_hashes[0].data(), sha256.data(), + sha256.size())); EXPECT_EQ(report_uri, new_pkp_state.report_uri); } diff --git a/net/http/transport_security_state_unittest.cc b/net/http/transport_security_state_unittest.cc index 66bbdc4314a910..09afe7db4fce4b 100644 --- a/net/http/transport_security_state_unittest.cc +++ b/net/http/transport_security_state_unittest.cc @@ -49,19 +49,29 @@ const char kReportUri[] = "http://example.test/test"; // kGoodPath is blog.torproject.org. const char* const kGoodPath[] = { - "sha1/m9lHYJYke9k0GtVZ+bXSQYE8nDI=", "sha1/o5OZxATDsgmwgcIfIWIneMJ0jkw=", - "sha1/wHqYaI2J+6sFZAwRfap9ZbjKzE4=", NULL, + "sha1/Yz4vayd/83rQfDXkDPn2yhzIScw=", + "sha1/3lKvjNsfmrn+WmfDhvr2iVh/yRs=", + "sha1/gzF+YoVCU9bXeDGQ7JGQVumRueM=", + "sha256/4osU79hfY3P2+WJGlT2mxmSL+5FIwLEVxTQcavyBNgQ=", + "sha256/k2v657xBsOVe1PQRwOsHsw3bsGT2VzIqz5K+59sNQws=", + "sha256/WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18=", + nullptr, }; -const char kGoodPin1[] = "m9lHYJYke9k0GtVZ+bXSQYE8nDI="; -const char kGoodPin2[] = "o5OZxATDsgmwgcIfIWIneMJ0jkw="; -const char kGoodPin3[] = "wHqYaI2J+6sFZAwRfap9ZbjKzE4="; +const char kGoodPin1[] = "4osU79hfY3P2+WJGlT2mxmSL+5FIwLEVxTQcavyBNgQ="; +const char kGoodPin2[] = "k2v657xBsOVe1PQRwOsHsw3bsGT2VzIqz5K+59sNQws="; +const char kGoodPin3[] = "WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; // kBadPath is plus.google.com via Trustcenter, which is utterly wrong for // torproject.org. const char* const kBadPath[] = { - "sha1/4BjDjn8v2lWeUFQnqSs0BgbIcrU=", "sha1/gzuEEAB/bkqdQS3EIjk2by7lW+k=", - "sha1/SOZo+SvSspXXR9gjIBBPM5iQn9Q=", NULL, + "sha1/111111111111111111111111111=", + "sha1/222222222222222222222222222=", + "sha1/333333333333333333333333333=", + "sha256/1111111111111111111111111111111111111111111=", + "sha256/2222222222222222222222222222222222222222222=", + "sha256/3333333333333333333333333333333333333333333=", + nullptr, }; // A mock ReportSender that just remembers the latest report @@ -183,7 +193,7 @@ class TransportSecurityStateTest : public testing::Test { static HashValueVector GetSampleSPKIHashes() { HashValueVector spki_hashes; - HashValue hash(HASH_VALUE_SHA1); + HashValue hash(HASH_VALUE_SHA256); memset(hash.data(), 0, hash.size()); spki_hashes.push_back(hash); return spki_hashes; @@ -554,11 +564,11 @@ TEST_F(TransportSecurityStateTest, NewPinsOverride) { TransportSecurityState::PKPState pkp_state; const base::Time current_time(base::Time::Now()); const base::Time expiry = current_time + base::TimeDelta::FromSeconds(1000); - HashValue hash1(HASH_VALUE_SHA1); + HashValue hash1(HASH_VALUE_SHA256); memset(hash1.data(), 0x01, hash1.size()); - HashValue hash2(HASH_VALUE_SHA1); + HashValue hash2(HASH_VALUE_SHA256); memset(hash2.data(), 0x02, hash1.size()); - HashValue hash3(HASH_VALUE_SHA1); + HashValue hash3(HASH_VALUE_SHA256); memset(hash3.data(), 0x03, hash1.size()); state.AddHPKP("example.com", expiry, true, HashValueVector(1, hash1), @@ -1310,8 +1320,8 @@ TEST_F(TransportSecurityStateTest, HPKPReportOnly) { // Check that a report is not sent for a Report-Only header with no // violation. std::string header = - "pin-sha1=\"" + std::string(kGoodPin1) + "\";pin-sha1=\"" + - std::string(kGoodPin2) + "\";pin-sha1=\"" + std::string(kGoodPin3) + + "pin-sha256=\"" + std::string(kGoodPin1) + "\";pin-sha256=\"" + + std::string(kGoodPin2) + "\";pin-sha256=\"" + std::string(kGoodPin3) + "\";report-uri=\"" + report_uri.spec() + "\";includeSubdomains"; SSLInfo ssl_info; ssl_info.is_issued_by_known_root = true; @@ -1356,8 +1366,8 @@ TEST_F(TransportSecurityStateTest, HPKPReportOnlyOnLocalRoot) { ASSERT_TRUE(cert2); std::string header = - "pin-sha1=\"" + std::string(kGoodPin1) + "\";pin-sha1=\"" + - std::string(kGoodPin2) + "\";pin-sha1=\"" + std::string(kGoodPin3) + + "pin-sha256=\"" + std::string(kGoodPin1) + "\";pin-sha256=\"" + + std::string(kGoodPin2) + "\";pin-sha256=\"" + std::string(kGoodPin3) + "\";report-uri=\"" + report_uri.spec() + "\";includeSubdomains"; TransportSecurityState state; @@ -1392,9 +1402,9 @@ TEST_F(TransportSecurityStateTest, HPKPReportOnlyParseErrors) { ASSERT_TRUE(cert1); ASSERT_TRUE(cert2); - std::string header = "pin-sha1=\"" + std::string(kGoodPin1) + - "\";pin-sha1=\"" + std::string(kGoodPin2) + - "\";pin-sha1=\"" + std::string(kGoodPin3) + "\""; + std::string header = "pin-sha256=\"" + std::string(kGoodPin1) + + "\";pin-sha256=\"" + std::string(kGoodPin2) + + "\";pin-sha256=\"" + std::string(kGoodPin3) + "\""; TransportSecurityState state; MockCertificateReportSender mock_report_sender; diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index 96067128c2663a..ee8ee7d648c2f0 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -5462,8 +5462,10 @@ TEST_F(URLRequestTestHTTP, MAYBE_ProcessPKPAndSendReport) { HashValue hash2; // The values here don't matter, as long as they are different from // the mocked CertVerifyResult below. - ASSERT_TRUE(hash1.FromString("sha1/111111111111111111111111111=")); - ASSERT_TRUE(hash2.FromString("sha1/222222222222222222222222222=")); + ASSERT_TRUE( + hash1.FromString("sha256/1111111111111111111111111111111111111111111=")); + ASSERT_TRUE( + hash2.FromString("sha256/2222222222222222222222222222222222222222222=")); hashes.push_back(hash1); hashes.push_back(hash2); security_state.AddHPKP(test_server_hostname, expiry, @@ -5483,7 +5485,8 @@ TEST_F(URLRequestTestHTTP, MAYBE_ProcessPKPAndSendReport) { verify_result.verified_cert = cert; verify_result.is_issued_by_known_root = true; HashValue hash3; - ASSERT_TRUE(hash3.FromString("sha1/333333333333333333333333333=")); + ASSERT_TRUE( + hash3.FromString("sha256/3333333333333333333333333333333333333333333=")); verify_result.public_key_hashes.push_back(hash3); cert_verifier.AddResultForCert(cert.get(), verify_result, OK); @@ -5545,7 +5548,8 @@ TEST_F(URLRequestTestHTTP, MAYBE_ProcessPKPReportOnly) { HashValue hash; // This value doesn't matter, as long as it is different from the pins // for the request to hpkp-headers-report-only.html. - ASSERT_TRUE(hash.FromString("sha1/111111111111111111111111111=")); + ASSERT_TRUE( + hash.FromString("sha256/1111111111111111111111111111111111111111111=")); verify_result.public_key_hashes.push_back(hash); cert_verifier.AddResultForCert(cert.get(), verify_result, OK);