Separate http_security_headers from transport_security_state

net/http/http_security_headers_unittest.cc

Index: net/http/http_security_headers_unittest.cc
===================================================================
--- net/http/http_security_headers_unittest.cc	(revision 0)
+++ net/http/http_security_headers_unittest.cc	(revision 0)
@@ -0,0 +1,478 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/base64.h"
+#include "base/sha1.h"
+#include "base/string_piece.h"
+#include "crypto/sha2.h"
+#include "net/base/asn1_util.h"
+#include "net/base/cert_test_util.h"
+#include "net/base/cert_verifier.h"
+#include "net/base/cert_verify_result.h"
+#include "net/base/net_log.h"
+#include "net/base/ssl_info.h"
+#include "net/base/test_completion_callback.h"
+#include "net/base/test_root_certs.h"
+#include "net/http/http_security_headers.h"
+#include "net/http/http_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+
+namespace net {
+
+class HttpSecurityHeadersTest : public testing::Test {
+};
+
+static bool GetPublicKeyHash(const net::X509Certificate::OSCertHandle& cert,

[Ryan Sleevi, 2012/10/29 09:01:24]

style nit: see previous comments re: unnamed namespaces vs static
style nit: omit net:: from these values, since you're within net:: (line 22)

[unsafe, 2012/10/29 22:30:55]

Fixed (this was pre-existing code, fwiw).

+                             HashValue* hash) {
+  std::string der_bytes;
+  if (!net::X509Certificate::GetDEREncoded(cert, &der_bytes))
+    return false;
+  base::StringPiece spki;
+  if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki))
+    return false;
+
+  switch (hash->tag) {
+    case HASH_VALUE_SHA1:
+      base::SHA1HashBytes(reinterpret_cast<const unsigned char*>(spki.data()),
+                          spki.size(), hash->data());
+      break;
+    case HASH_VALUE_SHA256:
+      crypto::SHA256HashString(spki, hash->data(), crypto::kSHA256Length);
+      break;
+    default:
+      NOTREACHED() << "Unknown HashValueTag " << hash->tag;
+      return false;
+  }
+
+  return true;
+}
+
+static std::string GetPinFromCert(X509Certificate* cert, HashValueTag tag) {
+  HashValue spki_hash(tag);
+  EXPECT_TRUE(GetPublicKeyHash(cert->os_cert_handle(), &spki_hash));
+
+  std::string base64;
+  base::Base64Encode(base::StringPiece(
+      reinterpret_cast<char*>(spki_hash.data()), spki_hash.size()), &base64);
+
+  std::string label;
+  switch (tag) {
+    case HASH_VALUE_SHA1:
+      label = "pin-sha1=";
+      break;
+    case HASH_VALUE_SHA256:
+      label = "pin-sha256=";
+      break;
+    default:
+      NOTREACHED() << "Unknown HashValueTag " << tag;
+  }
+
+  return label + HttpUtil::Quote(base64);
+}
+
+TEST_F(HttpSecurityHeadersTest, BogusHeaders) {
+  base::Time now = base::Time::Now();
+  base::Time expiry = now;
+  bool include_subdomains = false;
+
+  EXPECT_FALSE(ParseHSTSHeader(now, "", &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "    ", &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "abc", &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "  abc", &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "  abc   ", &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age", &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "  max-age", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "  max-age  ", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=", &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "   max-age=", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "   max-age  =", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "   max-age=   ", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "   max-age  =     ", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "   max-age  =     xy", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "   max-age  =     3488a923", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488a923  ", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-ag=3488923", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-aged=3488923", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age==3488923", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "amax-age=3488923", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=-3488923", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488923;", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488923     e", &expiry,
+                               &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now,
+                               "max-age=3488923     includesubdomain",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488923includesubdomains",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488923=includesubdomains",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488923 includesubdomainx",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488923 includesubdomain=",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now,
+                               "max-age=3488923 includesubdomain=true",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=3488923 includesubdomainsx",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now,
+                               "max-age=3488923 includesubdomains x",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=34889.23 includesubdomains",
+                               &expiry, &include_subdomains));
+  EXPECT_FALSE(ParseHSTSHeader(now, "max-age=34889 includesubdomains",
+                               &expiry, &include_subdomains));
+
+  // Check the out args were not updated by checking the default
+  // values for its predictable fields.
+  EXPECT_EQ(now, expiry);
+  EXPECT_FALSE(include_subdomains);
+}
+
+static void TestBogusPinsHeaders(HashValueTag tag) {
+  base::Time now = base::Time::Now();
+  base::Time expiry = now;
+  HashValueVector hashes;
+
+  SSLInfo ssl_info;
+  ssl_info.cert =
+      ImportCertFromFile(GetTestCertsDirectory(), "test_mail_google_com.pem");
+  std::string good_pin = GetPinFromCert(ssl_info.cert, tag);
+
+  // The backup pin is fake --- it just has to not be in the chain.
+  std::string backup_pin = "pin-sha1=" +
+      HttpUtil::Quote("6dcfXufJLW3J6S/9rRe4vUlBj5g=");
+
+  EXPECT_FALSE(ParseHPKPHeader(now, "", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "    ", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "abc", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "  abc", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "  abc   ", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "  max-age", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "  max-age  ", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age=", ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "   max-age=", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "   max-age  =", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "   max-age=   ", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "   max-age  =     ", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "   max-age  =     xy", ssl_info,
+                               &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now,
+                               "   max-age  =     3488a923",
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age=3488a923  ", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now,
+                               "max-ag=3488923pins=" + good_pin + "," +
+                               backup_pin,
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-aged=3488923" + backup_pin,
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-aged=3488923; " + backup_pin,
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now,
+                               "max-aged=3488923; " + backup_pin + ";" +
+                               backup_pin,
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now,
+                               "max-aged=3488923; " + good_pin + ";" +
+                               good_pin,
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-aged=3488923; " + good_pin,
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age==3488923", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "amax-age=3488923", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age=-3488923", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age=3488923;", ssl_info, &expiry,
+                               &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age=3488923     e", ssl_info,
+                               &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now,
+                               "max-age=3488923     includesubdomain",
+                               ssl_info, &expiry, &hashes));
+  EXPECT_FALSE(ParseHPKPHeader(now, "max-age=34889.23", ssl_info, &expiry,
+                               &hashes));
+
+  // Check the out args were not updated by checking the default
+  // values for its predictable fields.
+  EXPECT_EQ(now, expiry);
+  EXPECT_EQ(hashes.size(), (size_t)0);
+}
+
+TEST_F(HttpSecurityHeadersTest, ValidSTSHeaders) {
+  base::Time now = base::Time::Now();
+  base::Time expiry = now;
+  base::Time expect_expiry = now;
+  bool include_subdomains = false;
+
+  EXPECT_TRUE(ParseHSTSHeader(now, "max-age=243", &expiry,
+                              &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(243);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_FALSE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now, "  Max-agE    = 567", &expiry,
+                              &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(567);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_FALSE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now, "  mAx-aGe    = 890      ", &expiry,
+                              &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(890);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_FALSE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now, "max-age=123;incLudesUbdOmains", &expiry,
+                              &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now, "incLudesUbdOmains; max-age=123", &expiry,
+                              &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now, "   incLudesUbdOmains; max-age=123",
+                              &expiry, &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now,
+      "   incLudesUbdOmains; max-age=123; pumpkin=kitten", &expiry,
+                                   &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now,
+      "   pumpkin=894; incLudesUbdOmains; max-age=123  ", &expiry,
+                                   &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now,
+      "   pumpkin; incLudesUbdOmains; max-age=123  ", &expiry,
+                                   &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now,
+      "   pumpkin; incLudesUbdOmains; max-age=\"123\"  ", &expiry,
+                                   &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now,
+      "animal=\"squirrel; distinguished\"; incLudesUbdOmains; max-age=123",
+                                   &expiry, &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(now, "max-age=394082;  incLudesUbdOmains",
+                              &expiry, &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(394082);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(
+      now, "max-age=39408299  ;incLudesUbdOmains", &expiry,
+      &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(
+    std::min(kMaxHSTSAgeSecs, (int64)39408299l));
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(
+      now, "max-age=394082038  ; incLudesUbdOmains", &expiry,
+      &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(
+    std::min(kMaxHSTSAgeSecs, (int64)394082038l));
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(
+      now, "  max-age=0  ;  incLudesUbdOmains   ", &expiry,
+      &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(0);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+
+  EXPECT_TRUE(ParseHSTSHeader(
+      now,
+      "  max-age=999999999999999999999999999999999999999999999  ;"
+      "  incLudesUbdOmains   ", &expiry, &include_subdomains));
+  expect_expiry = now + base::TimeDelta::FromSeconds(
+      kMaxHSTSAgeSecs);
+  EXPECT_EQ(expect_expiry, expiry);
+  EXPECT_TRUE(include_subdomains);
+}
+
+static void TestValidPinsHeaders(HashValueTag tag) {
+  base::Time now = base::Time::Now();
+  base::Time expiry = now;
+  base::Time expect_expiry = now;
+  HashValueVector hashes;
+
+  // Set up a realistic SSLInfo with a realistic cert chain.
+  FilePath certs_dir = GetTestCertsDirectory();
+  scoped_refptr<X509Certificate> ee_cert =
+      ImportCertFromFile(certs_dir,
+                         "2048-rsa-ee-by-2048-rsa-intermediate.pem");
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), ee_cert);
+  scoped_refptr<X509Certificate> intermediate =
+      ImportCertFromFile(certs_dir, "2048-rsa-intermediate.pem");
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), intermediate);
+  X509Certificate::OSCertHandles intermediates;
+  intermediates.push_back(intermediate->os_cert_handle());
+  SSLInfo ssl_info;
+  ssl_info.cert = X509Certificate::CreateFromHandle(ee_cert->os_cert_handle(),
+                                                    intermediates);
+
+  // Add the root that signed the intermediate for this test.
+  scoped_refptr<X509Certificate> root_cert =
+      ImportCertFromFile(certs_dir, "2048-rsa-root.pem");
+  ASSERT_NE(static_cast<X509Certificate*>(NULL), root_cert);
+  ScopedTestRoot scoped_root(root_cert);
+
+  // Verify has the side-effect of populating public_key_hashes, which
+  // ParsePinsHeader needs. (It wants to check pins against the validated
+  // chain, not just the presented chain.)
+  int rv = ERR_FAILED;
+  CertVerifyResult result;
+  scoped_ptr<CertVerifier> verifier(CertVerifier::CreateDefault());

[Ryan Sleevi, 2012/10/29 09:01:24]

There's no need to use a real CertVerifier - ergo, no need to use a
ScopedTestRoot.

Such code is *completely* orthogonal to what is being tested here. You're just
testing header code - so you should not need to do anything at all with
certificate validation.

All of the test setup from like 382 and above is thus unrelated to this test.

[unsafe, 2012/10/29 22:30:55]

This code is needed to setup the ssl_info, isn't it?  This entire test was just
moved from TSS_unittest with minimal changes to remove DomainState, all of the
setup stuff is unchanged, so I don't know why 381 would fail on Android.  Did
TSS_unittest used to work?  Is ImportCertFromFile() somehow different in this
context? 

[Ryan Sleevi, 2012/10/29 23:46:25]

I believe TSS_unittest is excluded from compilation on Android.

I pointed out to palmer@ that we shouldn't be using the CertVerifier to do this,
and should just be mocking the class. Looks like you got caught in the acquired
technical debt - which is why I'm being admittedly so picky regarding new debt
:-/

As to why it doesn't work on Android, in general, pinning and TACK do not and
will not work on (real) android builds. This is because Android does not have
any native Java API for certificate verification that can return the constructed
certificate chain (on success or error). Since we use the Java APIs for cert
validation on Android, this means that pinning/TACK will not be able to be
implemented on Chrome for Android until Android exposes additional APIs.

The best I can say at this point is the relevant bugs are filed and the
appropriate teams are aware of this.

Using a manually created/fake ssl_info will thus cause Android to succeed,
because it's not limited by the Android cert verification. Alternatively, using
a MockCertVerifier will bypass the system verification routines, and thus will
also not be limited by the Android cert verification.

Hence why I said to rip it all out :)

+  TestCompletionCallback callback;
+  CertVerifier::RequestHandle handle = NULL;
+  rv = verifier->Verify(ssl_info.cert, "127.0.0.1", 0, NULL, &result,
+                        callback.callback(), &handle, BoundNetLog());
+  rv = callback.GetResult(rv);
+  ASSERT_EQ(OK, rv);
+  // Normally, ssl_client_socket_nss would do this, but for a unit test we
+  // fake it.
+  ssl_info.public_key_hashes = result.public_key_hashes;
+  std::string good_pin = GetPinFromCert(ssl_info.cert, /*tag*/HASH_VALUE_SHA1);

[Ryan Sleevi, 2012/10/29 09:01:24]

style nit: Don't do this (/*tag*/)
BUG? You don't respect |tag| (line 344), thus don't end up testing SHA-256

[unsafe, 2012/10/29 22:30:55]

Fixed (pre-existing).

[Ryan Sleevi, 2012/10/29 23:46:25]

Dang. On the upside, spotting all these pre-existing bugs and nits makes for a
more compelling refactoring argument :)

+  DLOG(WARNING) << "good pin: " << good_pin;
+
+  // The backup pin is fake --- we just need an SPKI hash that does not match
+  // the hash of any SPKI in the certificate chain.
+  std::string backup_pin = "pin-sha1=" +
+      HttpUtil::Quote("6dcfXufJLW3J6S/9rRe4vUlBj5g=");

[Ryan Sleevi, 2012/10/29 09:01:24]

There is no need to construct this as a string with quotes - just use a const
char[] kBackupPin constant

[unsafe, 2012/10/29 22:30:55]

Fixed - changed to string literals (preexisting)..

+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      "max-age=243; " + good_pin + ";" + backup_pin,
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(243);
+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      "   " + good_pin + "; " + backup_pin + "  ; Max-agE    = 567",
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(567);
+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      good_pin + ";" + backup_pin + "  ; mAx-aGe    = 890      ",
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(890);
+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      good_pin + ";" + backup_pin + "; max-age=123;IGNORED;",
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(123);
+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      "max-age=394082;" + backup_pin + ";" + good_pin + ";  ",
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(394082);
+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      "max-age=39408299  ;" + backup_pin + ";" + good_pin + ";  ",
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(
+    std::min(kMaxHSTSAgeSecs, (int64)39408299l));

[Ryan Sleevi, 2012/10/29 09:01:24]

BUG: Do not create int64 constants like this. They do not work reliably
cross-platform. Use GG_INT64_C (src/base/port.h)

style: Even if the above was valid, the further bug would be assuming l would be
sufficient for int64, and using a C-style cast rather than a C++ style cast

[unsafe, 2012/10/29 22:30:55]

Fixed.

+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      "max-age=39408038  ;    cybers=39408038  ;  " +
+          good_pin + ";" + backup_pin + ";   ",
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(
+    std::min(kMaxHSTSAgeSecs, (int64)394082038l));
+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      "  max-age=0  ;  " + good_pin + ";" + backup_pin,
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now + base::TimeDelta::FromSeconds(0);
+  EXPECT_EQ(expect_expiry, expiry);
+
+  EXPECT_TRUE(ParseHPKPHeader(
+      now,
+      "  max-age=999999999999999999999999999999999999999999999  ;  " +
+          backup_pin + ";" + good_pin + ";   ",
+      ssl_info, &expiry, &hashes));
+  expect_expiry = now +
+      base::TimeDelta::FromSeconds(kMaxHSTSAgeSecs);
+  EXPECT_EQ(expect_expiry, expiry);
+}
+
+TEST_F(HttpSecurityHeadersTest, BogusPinsHeadersSHA1) {
+  TestBogusPinsHeaders(HASH_VALUE_SHA1);
+}
+
+TEST_F(HttpSecurityHeadersTest, BogusPinsHeadersSHA256) {
+  TestBogusPinsHeaders(HASH_VALUE_SHA256);
+}
+
+TEST_F(HttpSecurityHeadersTest, ValidPinsHeadersSHA1) {
+  TestValidPinsHeaders(HASH_VALUE_SHA1);
+}
+
+TEST_F(HttpSecurityHeadersTest, ValidPinsHeadersSHA256) {
+  TestValidPinsHeaders(HASH_VALUE_SHA256);
+}
+};