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

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"
@@ skipping 15 matching lines @@
 //
 // value is the right-hand side of:
 //
 // "Strict-Transport-Security" ":"
 //     [ directive ]  *( ";" [ directive ] )
 bool NET_EXPORT_PRIVATE ParseHSTSHeader(const std::string& value,
                                         base::TimeDelta* max_age,
                                         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:
 //
 // "Public-Key-Pins" ":"
 //     "max-age" "=" delta-seconds ";"
 //     "pin-" algo "=" base64 [ ";" ... ]
 //     [ ";" "includeSubdomains" ]
 //     [ ";" "report-uri" "=" uri-reference ]
 //
 // For this function to return true, the key hashes specified by the HPKP
 // header must pass two additional checks. There MUST be at least one key
 // hash which matches the SSL certificate chain of the current site (as
 // specified by the chain_hashes) parameter. In addition, there MUST be at
 // least one key hash which does NOT match the site's SSL certificate chain
 // (this is the "backup pin").
 bool NET_EXPORT_PRIVATE ParseHPKPHeader(const std::string& value,
                                         const HashValueVector& chain_hashes,
                                         base::TimeDelta* max_age,
                                         bool* include_subdomains,
                                         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_