Parse HPKP report-uri and persist in TransportSecurityPersister

net/http/http_security_headers.cc

 // 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/basictypes.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/strings/string_util.h"
 #include "net/http/http_security_headers.h"
@@ skipping 88 matching lines @@
   std::string::const_iterator start = source.begin();
   std::string::const_iterator end = source.end();
   HttpUtil::TrimLWS(&start, &end);
   return std::string(start, end);
 }
 
 typedef std::pair<std::string, std::string> StringPair;
 
 StringPair Split(const std::string& source, char delimiter) {
   StringPair pair;
-  size_t point = source.find(delimiter);
+  size_t point = HttpUtil::FindDelimiter(source, 0, delimiter);

[Ryan Sleevi, 2015/06/27 12:13:59]

Hrm, this was previously dead code, and nothing uses this function - not even
unit tests!

It makes me wonder if we should be tweaking HttpUtil::NameValuePairsIterator to
allow optionally omitted values, which would at least be consistent with all the
other code we use.

[davidben, 2015/06/29 22:38:43]

That sounds reasonable to me, assuming you mean that value-less is some opt-in
mode for NameValuePairsIterator. Too bad there isn't a clear name for this
syntax that specs and NameValuePairsIterator could clearly refer to. There seem
to be a handful of parsing problems with the current code, like the one Emily
caught. We also don't accept quoted max-ages right now, if I'm reading this code
right, but RFC 6797 explicitly allows it quoted. <insert davidben's usual rant
about text-based encodings here>

Although note, by the way, that NameValuePairsIterator also accepts this broken
syntax:
http://crbug.com/39836


[Aside: Is HPKP quiiite the same syntax? I suspect this is a spec error but:

http://tools.ietf.org/html/rfc6797#section-6.1
This cites the "implied *LWS" variant and so it allows spaces (and the
line-folding craziness!) everywhere.

http://tools.ietf.org/html/rfc7469#section-2.1
This cites the RFC 7230 formulation (which says, if obsolete line folding is
supported, it's replaced with one or more SP before further processing) and has
explicit OWS in places. But there isn't OWS around the =.

Anyway, I'm guessing this is a spec error and so it's safe to use an omitted
values variant of NameValuePairsIterator.]

 
   pair.first = source.substr(0, point);
-  if (std::string::npos != point)
+
+  if (source.size() != point)
     pair.second = source.substr(point + 1);
 
   return pair;
 }
 
 bool ParseAndAppendPin(const std::string& value,
                        HashValueTag tag,
                        HashValueVector* hashes) {
   // Pins are always quoted.
   if (value.empty() || !HttpUtil::IsQuote(value[0]))
@@ skipping 143 matching lines @@
     case START:
     case AFTER_MAX_AGE_LABEL:
     case AFTER_MAX_AGE_EQUALS:
       return false;
     default:
       NOTREACHED();
       return false;
   }
 }
 
-// "Public-Key-Pins" ":"
+// "Public-Key-Pins[-Report-Only]" ":"
 //     "max-age" "=" delta-seconds ";"
 //     "pin-" algo "=" base64 [ ";" ... ]
+//     [ ";" "includeSubdomains" ]
+//     [ ";" "report-uri" "=" uri-reference ]
 bool ParseHPKPHeader(const std::string& value,
                      const HashValueVector& chain_hashes,
                      base::TimeDelta* max_age,
                      bool* include_subdomains,
-                     HashValueVector* hashes) {
+                     HashValueVector* hashes,
+                     std::string* report_uri) {
   bool parsed_max_age = false;
   bool include_subdomains_candidate = false;
   uint32 max_age_candidate = 0;
   HashValueVector pins;
 
   std::string source = value;
 
   while (!source.empty()) {
     StringPair semicolon = Split(source, ';');
     semicolon.first = Strip(semicolon.first);
@@ skipping 10 matching lines @@
       }
       parsed_max_age = true;
     } else if (base::LowerCaseEqualsASCII(equals.first, "pin-sha1")) {
       if (!ParseAndAppendPin(equals.second, HASH_VALUE_SHA1, &pins))
         return false;
     } else if (base::LowerCaseEqualsASCII(equals.first, "pin-sha256")) {
       if (!ParseAndAppendPin(equals.second, HASH_VALUE_SHA256, &pins))
         return false;
     } else if (base::LowerCaseEqualsASCII(equals.first, "includesubdomains")) {
       include_subdomains_candidate = true;
+    } else if (base::LowerCaseEqualsASCII(equals.first, "report-uri")) {
+      // report-uris are always quoted.

[davidben, 2015/06/29 22:38:43]

Not quoting would be insane, but I don't actually see anything in the spec that
requires that. (Spec error? Pins explicitly say they have to be quoted.)

+      if (equals.second.empty() || !HttpUtil::IsQuote(equals.second[0]) ||
+          equals.second[0] != *equals.second.rbegin())
+        return false;
+
+      *report_uri = HttpUtil::Unquote(equals.second);
+      if (report_uri->empty())
+        return false;
     } else {
       // Silently ignore unknown directives for forward compatibility.
     }
 
     source = semicolon.second;
   }
 
   if (!parsed_max_age)
     return false;
 
   if (!IsPinListValid(pins, chain_hashes))
     return false;
 
   *max_age = base::TimeDelta::FromSeconds(max_age_candidate);
   *include_subdomains = include_subdomains_candidate;
   hashes->swap(pins);
 
   return true;
 }
 
 }  // namespace net