Add parsers for the Encryption and Encryption-Key HTTP headers.

components/gcm_driver/crypto/encryption_header_parsers.cc

+// Copyright 2015 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 "components/gcm_driver/crypto/encryption_header_parsers.h"
+
+#include "base/base64.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
+#include "net/http/http_util.h"
+
+namespace gcm {
+
+namespace {
+
+// The default record size in bytes, as defined in section two of
+// https://tools.ietf.org/html/draft-thomson-http-encryption-01.
+const uint64_t kDefaultRecordSizeBytes = 4096;
+
+// TODO(peter): Unify the base64url implementations. https://crbug.com/536745
+bool Base64URLDecode(const base::StringPiece& input, std::string* output) {
+  // Bail on malformed strings, which already contain a '+' or a '/'. All valid
+  // strings should escape these special characters as '-' and '_',
+  // respectively.
+  if (input.find_first_of("+/") != std::string::npos)
+    return false;
+
+  size_t padded_size =
+    input.size() % 4 ? input.size() + 4 - (input.size() % 4) : input.size();

[Ryan Sleevi, 2015/09/28 19:35:48]

Erm, can't this overflow in some (extremely pedantic) scenarios?

[Peter Beverloo, 2015/10/01 14:56:20]

Yes.

However, the only scenario in which *this* would overflow, and not
std::string::size() (which is also of type size_t), is when the size of the
input string is (> SIZE_T_MAX - 3) and (<= SIZE_T_MAX).

This class will only handle input that is received from Google GCM servers,
which impose a limit of 4K - six orders of magnitude lower on 32-bit systems, 15
orders of magnitude on 64-bit systems :-).

Switched to a CheckedNumeric<size_t>.

+
+  // Add padding to |input|.
+  std::string padded_input(input.begin(), input.end());
+  padded_input.resize(padded_size, '=');

[Ryan Sleevi, 2015/09/28 19:35:48]

Blergh; having to copy this string makes it all the more questionable whether or
not we should just fix this in //base first, so that we can do a copy-less
operation and just signify via the API whether or not padding is expected.

[Peter Beverloo, 2015/10/01 14:56:20]

Acknowledged.

+
+  // Convert to standard base64 alphabet.
+  base::ReplaceChars(padded_input, "-", "+", &padded_input);
+  base::ReplaceChars(padded_input, "_", "/", &padded_input);
+
+  return base::Base64Decode(padded_input, output);
+}
+
+bool ValueToDecodedString(const std::string::const_iterator& begin,
+                          const std::string::const_iterator& end,
+                          std::string* salt) {
+  const base::StringPiece value(begin, end);
+  if (value.empty())
+    return false;
+
+  return Base64URLDecode(value, salt);
+}
+
+bool RecordSizeToInt(const std::string::const_iterator& begin,
+                     const std::string::const_iterator& end,
+                     uint64_t* rs) {
+  const base::StringPiece value(begin, end);
+  if (value.empty())
+    return false;
+
+  if (!base::StringToUint64(value, rs))
+    return false;
+
+  // The record size MUST be greater than 1.
+  return *rs > 1;
+}
+
+bool ParseEncryptionHeaderImpl(const std::string& input,
+                               std::string* keyid,
+                               std::string* salt,
+                               uint64_t* rs) {
+  net::HttpUtil::NameValuePairsIterator name_value_pairs(
+      input.begin(), input.end(), ';',
+      net::HttpUtil::NameValuePairsIterator::VALUES_NOT_OPTIONAL);
+
+  while (name_value_pairs.GetNext()) {
+    const base::StringPiece name(name_value_pairs.name_begin(),
+                                 name_value_pairs.name_end());
+
+    if (base::LowerCaseEqualsASCII(name, "keyid")) {
+      keyid->assign(name_value_pairs.value_begin(),
+                    name_value_pairs.value_end());
+    } else if (base::LowerCaseEqualsASCII(name, "salt")) {
+      if (!ValueToDecodedString(name_value_pairs.value_begin(),
+                                name_value_pairs.value_end(), salt)) {
+        return false;
+      }
+    } else if (base::LowerCaseEqualsASCII(name, "rs")) {
+      if (!RecordSizeToInt(name_value_pairs.value_begin(),
+                           name_value_pairs.value_end(), rs)) {
+        return false;
+      }
+    } else {
+      // Silently ignore unknown directives for forward compatibility.
+    }
+  }
+
+  return name_value_pairs.valid();
+}
+
+bool ParseEncryptionKeyHeaderImpl(const std::string& input,
+                                  std::string* keyid,
+                                  std::string* key,
+                                  std::string* dh) {
+  net::HttpUtil::NameValuePairsIterator name_value_pairs(
+      input.begin(), input.end(), ';',
+      net::HttpUtil::NameValuePairsIterator::VALUES_NOT_OPTIONAL);
+
+  while (name_value_pairs.GetNext()) {
+    const base::StringPiece name(name_value_pairs.name_begin(),
+                                 name_value_pairs.name_end());
+
+    if (base::LowerCaseEqualsASCII(name, "keyid")) {
+      keyid->assign(name_value_pairs.value_begin(),
+                    name_value_pairs.value_end());
+    } else if (base::LowerCaseEqualsASCII(name, "key")) {
+      if (!ValueToDecodedString(name_value_pairs.value_begin(),
+                                name_value_pairs.value_end(), key)) {
+        return false;
+      }
+    } else if (base::LowerCaseEqualsASCII(name, "dh")) {
+      if (!ValueToDecodedString(name_value_pairs.value_begin(),
+                                name_value_pairs.value_end(), dh)) {
+        return false;
+      }
+    } else {
+      // Silently ignore unknown directives for forward compatibility.
+    }
+  }
+
+  return name_value_pairs.valid();
+}
+
+}  // namespace
+
+// "Encryption" ":"
+//     [ "keyid" "=" string ]
+//     [ ";" "salt" "=" base64url ]
+//     [ ";" "rs" "=" octet-count ]

[Ryan Sleevi, 2015/09/28 19:35:48]

Comment gripe: So, this isn't what the spec says, but it conveniently highlights
the bug in your implementation

"Encryption" follows the list-rule of #encryption_params, which means multiple
headers can be coalesced via commas. Your parsing ignores the commas, which
seems like it can lead to all sorts of weird situations.

Consider the following header:
Encryption: keyid=a,keyid=b;salt=foo;rs=8

You'd end up parsing this as "keyid" = "a,keyid=b", "salt" = "foo", "rs" = "8".

So I think you have to account for the comma separation here, and I'd argue it's
cleaner & clearer to use the ABNF from the RFC rather than this form, along with
perhaps a table of expected forms.

If we're not going to support multiple encryption layers, that's fine, but we
should parse the header at least.

[Peter Beverloo, 2015/10/01 14:56:20]

Done.

+bool ParseEncryptionHeader(const std::string& input,
+                           std::string* keyid,
+                           std::string* salt,
+                           uint64_t* rs) {
+  std::string candidate_keyid;
+  std::string candidate_salt;
+  uint64_t candidate_rs = kDefaultRecordSizeBytes;
+
+  if (!ParseEncryptionHeaderImpl(input, &candidate_keyid, &candidate_salt,
+                                 &candidate_rs)) {
+    return false;
+  }
+
+  keyid->swap(candidate_keyid);
+  salt->swap(candidate_salt);
+  *rs = candidate_rs;
+  return true;
+}
+
+// "Encryption-Key" ":"
+//     [ "keyid" "=" string ]
+//     [ ";" "key" "=" base64url ]
+//     [ ";" "dh" "=" base64url ]
+bool ParseEncryptionKeyHeader(const std::string& input,
+                              std::string* keyid,
+                              std::string* key,
+                              std::string* dh) {
+  std::string candidate_keyid;
+  std::string candidate_key;
+  std::string candidate_dh;
+
+  if (!ParseEncryptionKeyHeaderImpl(input, &candidate_keyid, &candidate_key,
+                                    &candidate_dh)) {
+    return false;
+  }
+
+  keyid->swap(candidate_keyid);
+  key->swap(candidate_key);
+  dh->swap(candidate_dh);
+  return true;
+}
+
+}  // namespace gcm