Add parsing for Public-Key-Pins-Report-Only header

net/http/http_security_headers.h

Index: net/http/http_security_headers.h
diff --git a/net/http/http_security_headers.h b/net/http/http_security_headers.h
index e03da85f38ffd36825b607bc2e9d6c08beb0785e..4870ba6cf5bf57db193af9a57b0874bdb8b88ca4 100644
--- a/net/http/http_security_headers.h
+++ b/net/http/http_security_headers.h
@@ -33,9 +33,9 @@ bool NET_EXPORT_PRIVATE ParseHSTSHeader(const std::string& value,
                                         bool* include_subdomains);
 
 // Parses |value| as a Public-Key-Pins header value. If successful, returns
-// true and populates the |*max_age|, |*include_subdomains|, and |*hashes|
-// values. Otherwise returns false and leaves the output parameters
-// unchanged.
+// true and populates the |*max_age|, |*include_subdomains|, |*hashes|, and
+// |*report_uri| values. Otherwise returns false and leaves the output
+// parameters unchanged.
 //
 // value is the right-hand side of:
 //
@@ -58,6 +58,22 @@ bool NET_EXPORT_PRIVATE ParseHPKPHeader(const std::string& value,
                                         HashValueVector* hashes,
                                         GURL* report_uri);
 
+// Parses |value| as a Public-Key-Pins-Report-Only header value. If
+// successful, returns true and populates the |*hashes| and
+// |*report_uri| values. Otherwise returns false and leaves the output
+// parameters unchanged.
+//
+// value is the right-hand side of:
+//
+// "Public-Key-Pins-Report-Only" ":"
+//     [ "max-age" "=" delta-seconds ";" ]
+//     "pin-" algo "=" base64 [ ";" ... ]
+//     [ ";" "includeSubdomains" ]
+//     [ ";" "report-uri" "=" uri-reference ]
+//
+bool NET_EXPORT_PRIVATE ParseHPKPReportOnlyHeader(const std::string& value,
+                                                  HashValueVector* hashes,
+                                                  GURL* report_uri);

[Ryan Sleevi, 2015/07/30 01:43:50]

Is there any reason to introduce a separate method? Given that the PKP-RO spec
uses the same directive parsing, it seems perfectly fine to use ParsePKPHeader.

(2.1.2 indicates max-age should just be ignored, 2.1.3 leaves includeSubDomains
as optional - _but meaningful_, and 2.1.4 of course encourages report-uri for
RO)

[estark, 2015/07/30 02:43:49]

The main reason is that ParseHPKPHeader() does some validation that doesn't
apply to PKP-RO, namely requiring max-age and checking IsPinListValid(). If we
wanted to use ParseHPKPHeader() for both, we could pull those validation steps
out and have the caller do them. But there were two things that seemed weird
about that: 1. it seemed weird to have ParseHPKPHeader() doing some of the
validation (like report_uri.is_valid()) but not all, and 2. ParseHPKPHeader()
would have to take an additional parameter or somehow indicate whether a max-age
was parsed, so that the caller can check that a max-age is present if required.
So I started to get a fitting-a-square-peg-in-a-round-hole kind of feeling.

However, I did overlook the fact that we can't just ignore includeSubdomains for
PKP-RO because it needs to be included in the report, so if we stick with the
two separate functions I'll add an |include_subdomains| parameter to
ParseHPKPReportOnlyHeader().

[Ryan Sleevi, 2015/07/30 02:52:25]

Both of these apply.

       UAs MUST ignore any header fields containing directives, or other
       header field value data, that do not conform to the syntax
       defined in this specification.  In particular, UAs must not
       attempt to fix malformed header fields.

Or is your point the presence check (that max-age is REQUIRED)?

The IsPinListValid argument makes sense though.
Fair point.
Yeah, I think your argument for why two functions makes sense.

[estark, 2015/07/30 15:21:21]

Yep, sorry, I meant the presence check.
Cool, thanks.

 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_SECURITY_HEADERS_H_