Separate http_security_headers from transport_security_state

net/http/http_security_headers.h

+// 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.
+
+#ifndef NET_HTTP_HTTP_SECURITY_HEADERS_H_
+#define NET_HTTP_HTTP_SECURITY_HEADERS_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/gtest_prod_util.h"
+#include "base/time.h"
+#include "base/values.h"
+#include "net/base/net_export.h"
+#include "net/base/ssl_info.h"
+#include "net/base/x509_cert_types.h"
+
+namespace net {
+
+const long kMaxHSTSAgeSecs = 86400 * 365;  // 1 year

[Ryan Sleevi, 2012/10/25 01:59:09]

For seconds, we tend to use either int or use int64 values (see
TimeDelta::FromSeconds)

[unsafe, 2012/10/25 06:59:54]

This was existing code; changed to int64.

+
+// Parses |value| as a Strict-Transport-Security header value. If successful,
+// returns true and populates the expiry and include_subdomains values.

[Ryan Sleevi, 2012/10/25 01:59:09]

nit: and sets |*expiry| and and |*include_subdomains|.

[unsafe, 2012/10/25 06:59:54]

Done.

+// Otherwise returns false and leaves the output parameters unchanged.
+// Interprets the max-age directive relative to |now|.
+//
+// value is the right-hand side of:
+//
+// "Strict-Transport-Security" ":"
+//     [ directive ]  *( ";" [ directive ] )
+bool ParseHSTSHeader(const base::Time& now, const std::string& value,

[Ryan Sleevi, 2012/10/25 01:59:09]

style nit: |now| and |value| belong on separate lines, as per
http://dev.chromium.org/developers/coding-style#Code_Formatting

nit: If you don't actually need to modify |value|, using a base::StringPiece
here *may* offer a no-copy implementation of header parsing - which can be used
by the HTTP header enumeration logic.

[unsafe, 2012/10/25 06:59:54]

What rule applies here?  Couldn't figure it out!
This was as-before, I don't know the conventions regarding std::string vs
StringPiece?

+                     base::Time* expiry,         // OUT
+                     bool* include_subdomains);  // OUT

[Ryan Sleevi, 2012/10/25 01:59:09]

Neither Google C++ Guide nor Chromium style guide require the use of this // OUT
annotation. Instead, it's specified in the comments.

[unsafe, 2012/10/25 06:59:54]

Should I remove them?

+
+// Parses |value| as a Public-Key-Pins header value. If successful,
+// returns true and populates the expiry and hashes values.
+// Otherwise returns false and leaves the output parameters unchanged.
+// Interprets the max-age directive relative to |now|.
+// Checks that the header's public key pins overlaps with the SSL chain
+// as specified in ssl_info.

[Ryan Sleevi, 2012/10/25 01:59:09]

nit: This API behaviour should be separated out.

One function to parse the header.
One function to check the validity of the header, based upon |ssl_info|
(although really, it should only be necessary to supply a HashValueVector I
believe)

The latter function belongs with/in the PKP handling logic.

[unsafe, 2012/10/25 06:59:54]

I think I agree, this was how the function was before, but it makes sense to
move the parsing into net/http and keep the logic elsewhere...

+//
+// value is the right-hand side of:
+//
+// "Public-Key-Pins" ":"
+//     "max-age" "=" delta-seconds ";"
+//     "pin-" algo "=" base64 [ ";" ... ]
+bool ParseHPKPHeader(const base::Time& now,
+                     const std::string& value,
+                     const SSLInfo& ssl_info,
+                     base::Time* expiry,         // OUT
+                     HashValueVector* hashes);   // OUT
+
+}  // namespace net
+
+
+#endif  // NET_HTTP_HTTP_SECURITY_HEADERS_H_
+