Trust Tokens: Expand key commitment parsing to handle multiple issuers

This CL makes a couple key commitment parsing and storage changes in
preparation for receiving JSON-encoded keys (rather than Mojo structs)
from the browser:

1. It modifies the NetworkService SetTrustTokenKeyCommitments interface
to take a string rather than a map<issuer, key commitment result>;
2. It expands the key commitment parsing logic in the network service
with a new method capable of parsing a collection of (issuer -> key
commitment result) entries.
3. It adds a new TrustTokenKeyCommitments setter taking a raw string and
passing it to the new parsing interface, rather than taking a map.

R=csharrison

Bug: 1068651
Change-Id: I930e797c39c4adac0da3919cbc462283c89d6f3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2139416
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759185}

services/network/network_service.cc

@@ -712,9 +712,10 @@ void NetworkService::SetEnvironment(
 }
 
 void NetworkService::SetTrustTokenKeyCommitments(
-    base::flat_map<url::Origin, mojom::TrustTokenKeyCommitmentResultPtr>
-        commitments) {
-  trust_token_key_commitments_->Set(std::move(commitments));
+    const std::string& raw_commitments,
+    base::OnceClosure done) {
+  trust_token_key_commitments_->ParseAndSet(raw_commitments);
+  std::move(done).Run();
 }
 
 #if defined(OS_ANDROID)

services/network/network_service.h

@@ -175,9 +175,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
 #endif
   void SetEnvironment(
       std::vector<mojom::EnvironmentVariablePtr> environment) override;
-  void SetTrustTokenKeyCommitments(
-      base::flat_map<url::Origin, mojom::TrustTokenKeyCommitmentResultPtr>
-          commitments) override;
+  void SetTrustTokenKeyCommitments(const std::string& raw_commitments,
+                                   base::OnceClosure done) override;
 
 #if defined(OS_ANDROID)
   void DumpWithoutCrashing(base::Time dump_request_time) override;

services/network/network_service_unittest.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/containers/span.h"
 #include "base/files/file_util.h"
@@ -1230,25 +1231,25 @@ TEST_F(NetworkServiceTestWithService, SetsTrustTokenKeyCommitments) {
   ASSERT_TRUE(service_->trust_token_key_commitments());
 
   auto expectation = mojom::TrustTokenKeyCommitmentResult::New();
-  expectation->batch_size = mojom::TrustTokenKeyCommitmentBatchSize::New(5);
+  ASSERT_TRUE(base::Base64Decode(
+      "aaaa", &expectation->signed_redemption_record_verification_key));
 
-  url::Origin issuer_origin =
-      url::Origin::Create(GURL("https://issuer.example"));
-
-  base::flat_map<url::Origin, mojom::TrustTokenKeyCommitmentResultPtr> to_set;
-  to_set.insert_or_assign(issuer_origin, expectation.Clone());
-  network_service_->SetTrustTokenKeyCommitments(std::move(to_set));
-  network_service_.FlushForTesting();
+  base::RunLoop run_loop;
+  network_service_->SetTrustTokenKeyCommitments(
+      R"( { "https://issuer.example": { "srrkey": "aaaa" } } )",
+      run_loop.QuitClosure());
+  run_loop.Run();
 
   mojom::TrustTokenKeyCommitmentResultPtr result;
   bool ran = false;
 
   service_->trust_token_key_commitments()->Get(
-      issuer_origin, base::BindLambdaForTesting(
-                         [&](mojom::TrustTokenKeyCommitmentResultPtr ptr) {
-                           result = std::move(ptr);
-                           ran = true;
-                         }));
+      *SuitableTrustTokenOrigin::Create(GURL("https://issuer.example")),
+      base::BindLambdaForTesting(
+          [&](mojom::TrustTokenKeyCommitmentResultPtr ptr) {
+            result = std::move(ptr);
+            ran = true;
+          }));
 
   ASSERT_TRUE(ran);
 

services/network/public/mojom/network_service.mojom

@@ -387,11 +387,12 @@ interface NetworkService {
   // |environment| array.
   SetEnvironment(array<EnvironmentVariable> environment);
 
-  // Sets Trust Tokens key commitment state. |commitments| is a map from issuer
-  // origins to key commitment results (each result contains a collection of
-  // keys and some associated metadata).
-  SetTrustTokenKeyCommitments(
-      map<url.mojom.Origin, TrustTokenKeyCommitmentResult> commitments);
+  // Sets Trust Tokens key commitment state. |raw_commitments| is the
+  // JSON-encoded string representation of a collection of issuers' key
+  // commitments according to the format specified, for now, in the Trust
+  // Tokens design doc:
+  // https://docs.google.com/document/d/1TNnya6B8pyomDK2F1R9CL3dY10OAmqWlnCxsWyOBDVQ/edit#heading=h.z52drgpfgulz.
+  SetTrustTokenKeyCommitments(string raw_commitments) => ();
 
   // Calls base::debug::DumpWithoutCrashing for the network process.
   // TODO(http://crbug.com/934317): Remove this once done debugging renderer

services/network/trust_tokens/BUILD.gn

@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 import("//services/network/public/cpp/features.gni")
+import("//testing/libfuzzer/fuzzer_test.gni")
 import("//third_party/protobuf/proto_library.gni")
 
 source_set("trust_tokens") {
@@ -158,3 +159,14 @@ proto_library("storage_proto") {
 
   extra_configs = [ "//build/config/compiler:wexit_time_destructors" ]
 }
+
+fuzzer_test("trust_token_key_commitment_parser_fuzzer") {
+  sources = [ "test/trust_token_key_commitment_parser_fuzzer.cc" ]
+  deps = [
+    ":trust_tokens",
+    "//base",
+    "//services/network/public/mojom",
+  ]
+  dict = "test/trust_token_key_commitment_parser_fuzzer.dict"
+  seed_corpus = "//third_party/grpc/src/test/core/json/corpus/"
+}

services/network/trust_tokens/test/trust_token_key_commitment_parser_fuzzer.cc

+// Copyright 2020 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 "services/network/trust_tokens/trust_token_key_commitment_parser.h"
+
+#include <memory>
+
+#include "services/network/public/mojom/trust_tokens.mojom.h"
+
+namespace network {
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+  base::StringPiece string_input(reinterpret_cast<const char*>(data), size);
+  TrustTokenKeyCommitmentParser().Parse(string_input);
+  TrustTokenKeyCommitmentParser().ParseMultipleIssuers(string_input);
+  return 0;
+}
+
+}  // namespace network

services/network/trust_tokens/test/trust_token_key_commitment_parser_fuzzer.dict

+# Copyright 2020 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.
+
+"{"
+"}"
+"["
+"]"
+"\""
+"'"
+"\\"
+"//"
+":"
+","
+" "
+"\\n"
+"\\r"
+"/*"
+"*/"
+"true"
+"false"
+"null"
+"\\u"
+"\\b"
+"\\f"
+"\\t"
+"."
+"e"
+"e+"
+"e-"
+"E"
+"E+"
+"E-"
+"srrkey"
+"batchsize"
+"Y"
+"expiry"
+"https"

services/network/trust_tokens/trust_token_key_commitment_parser.cc

@@ -10,7 +10,9 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/values.h"
+#include "services/network/public/mojom/trust_tokens.mojom-forward.h"
 #include "services/network/public/mojom/trust_tokens.mojom.h"
+#include "services/network/trust_tokens/suitable_trust_token_origin.h"
 
 namespace network {
 
@@ -70,56 +72,27 @@ ParseKeyResult ParseSingleKeyExceptLabel(
   return ParseKeyResult::kSucceed;
 }
 
-}  // namespace
-
-const char kTrustTokenKeyCommitmentBatchsizeField[] = "batchsize";
-const char kTrustTokenKeyCommitmentSrrkeyField[] = "srrkey";
-const char kTrustTokenKeyCommitmentExpiryField[] = "expiry";
-const char kTrustTokenKeyCommitmentKeyField[] = "Y";
-
-// https://docs.google.com/document/d/1TNnya6B8pyomDK2F1R9CL3dY10OAmqWlnCxsWyOBDVQ/edit#bookmark=id.6wh9crbxdizi
-// {
-//   "batchsize" : ..., // Optional batch size; value of type int.
-//   "srrkey" : ...,    // Required Signed Redemption Record (SRR)
-//                      // verification key, in base64.
-//
-//   "1" : {            // Key label, a number in uint32_t range.
-//     "Y" : ...,       // Required token issuance verification key, in
-//                      // base64.
-//     "expiry" : ...,  // Required token issuance key expiry time, in
-//                      // microseconds since the Unix epoch.
-//   },
-//   "17" : {           // No guarantee that key labels (1, 17) are dense.
-//     "Y" : ...,
-//     "expiry" : ...,
-//   }
-// }
-mojom::TrustTokenKeyCommitmentResultPtr TrustTokenKeyCommitmentParser::Parse(
-    base::StringPiece response_body) {
-  base::Optional<base::Value> maybe_value =
-      base::JSONReader::Read(response_body);
-  if (!maybe_value)
-    return nullptr;
-
-  if (!maybe_value->is_dict())
+mojom::TrustTokenKeyCommitmentResultPtr ParseSingleIssuer(
+    const base::Value& value) {
+  if (!value.is_dict())
     return nullptr;
 
   auto result = mojom::TrustTokenKeyCommitmentResult::New();
 
   // Confirm that the batchsize field is type-safe, if it's present.
-  if (maybe_value->FindKey(kTrustTokenKeyCommitmentBatchsizeField) &&
-      !maybe_value->FindIntKey(kTrustTokenKeyCommitmentBatchsizeField)) {
+  if (value.FindKey(kTrustTokenKeyCommitmentBatchsizeField) &&
+      !value.FindIntKey(kTrustTokenKeyCommitmentBatchsizeField)) {
     return nullptr;
   }
   if (base::Optional<int> maybe_batch_size =
-          maybe_value->FindIntKey(kTrustTokenKeyCommitmentBatchsizeField)) {
+          value.FindIntKey(kTrustTokenKeyCommitmentBatchsizeField)) {
     result->batch_size =
         mojom::TrustTokenKeyCommitmentBatchSize::New(*maybe_batch_size);
   }
 
   // Confirm that the srrkey field is present and base64-encoded.
   const std::string* maybe_srrkey =
-      maybe_value->FindStringKey(kTrustTokenKeyCommitmentSrrkeyField);
+      value.FindStringKey(kTrustTokenKeyCommitmentSrrkeyField);
   if (!maybe_srrkey)
     return nullptr;
   if (!base::Base64Decode(*maybe_srrkey,
@@ -129,7 +102,7 @@ mojom::TrustTokenKeyCommitmentResultPtr TrustTokenKeyCommitmentParser::Parse(
 
   // Parse the key commitments in the result (these are exactly the
   // key-value pairs in the dictionary with dictionary-typed values).
-  for (const auto& kv : maybe_value->DictItems()) {
+  for (const auto& kv : value.DictItems()) {
     const base::Value& item = kv.second;
     if (!item.is_dict())
       continue;
@@ -152,4 +125,117 @@ mojom::TrustTokenKeyCommitmentResultPtr TrustTokenKeyCommitmentParser::Parse(
   return result;
 }
 
+// Entry is a convenience struct used as an intermediate representation when
+// parsing multiple issuers. In addition to a parsed canonicalized issuer, it
+// preserves the raw JSON string key (the second entry) in order
+// deterministically to deduplicate entries with keys canonicalizing to the same
+// issuer.
+using Entry = std::tuple<SuitableTrustTokenOrigin,  // canonicalized issuer
+                         std::string,               // raw key from the JSON
+                         mojom::TrustTokenKeyCommitmentResultPtr>;
+SuitableTrustTokenOrigin& canonicalized_issuer(Entry& e) {
+  return std::get<0>(e);
+}
+mojom::TrustTokenKeyCommitmentResultPtr& commitment(Entry& e) {
+  return std::get<2>(e);
+}
+
+}  // namespace
+
+const char kTrustTokenKeyCommitmentBatchsizeField[] = "batchsize";
+const char kTrustTokenKeyCommitmentSrrkeyField[] = "srrkey";
+const char kTrustTokenKeyCommitmentExpiryField[] = "expiry";
+const char kTrustTokenKeyCommitmentKeyField[] = "Y";
+
+// https://docs.google.com/document/d/1TNnya6B8pyomDK2F1R9CL3dY10OAmqWlnCxsWyOBDVQ/edit#bookmark=id.6wh9crbxdizi
+// {
+//   "batchsize" : ..., // Optional batch size; value of type int.
+//   "srrkey" : ...,    // Required Signed Redemption Record (SRR)
+//                      // verification key, in base64.
+//
+//   "1" : {            // Key label, a number in uint32_t range.
+//     "Y" : ...,       // Required token issuance verification key, in
+//                      // base64.
+//     "expiry" : ...,  // Required token issuance key expiry time, in
+//                      // microseconds since the Unix epoch.
+//   },
+//   "17" : {           // No guarantee that key labels (1, 17) are dense.
+//     "Y" : ...,
+//     "expiry" : ...,
+//   }
+// }
+mojom::TrustTokenKeyCommitmentResultPtr TrustTokenKeyCommitmentParser::Parse(
+    base::StringPiece response_body) {
+  base::Optional<base::Value> maybe_value =
+      base::JSONReader::Read(response_body);
+  if (!maybe_value)
+    return nullptr;
+
+  return ParseSingleIssuer(std::move(*maybe_value));
+}
+
+std::unique_ptr<base::flat_map<SuitableTrustTokenOrigin,
+                               mojom::TrustTokenKeyCommitmentResultPtr>>
+TrustTokenKeyCommitmentParser::ParseMultipleIssuers(
+    base::StringPiece response_body) {
+  base::Optional<base::Value> maybe_value =
+      base::JSONReader::Read(response_body);
+  if (!maybe_value)
+    return nullptr;
+
+  if (!maybe_value->is_dict())
+    return nullptr;
+
+  // The configuration might contain conflicting lists of keys for issuers with
+  // the same canonicalized URLs but different string representations provided
+  // by the server. In order to handle these deterministically, first transfer
+  // the entries to intermediate storage while maintaining the initial JSON
+  // keys; then deduplicate based on identical entries' JSON keys' lexicographic
+  // value.
+
+  std::vector<Entry> parsed_entries;
+
+  for (const auto& kv : maybe_value->DictItems()) {
+    const std::string& raw_key_from_json = kv.first;
+    base::Optional<SuitableTrustTokenOrigin> maybe_issuer =
+        SuitableTrustTokenOrigin::Create(GURL(raw_key_from_json));
+
+    if (!maybe_issuer)
+      continue;
+
+    mojom::TrustTokenKeyCommitmentResultPtr commitment_result =
+        ParseSingleIssuer(kv.second);
+
+    if (!commitment_result)
+      continue;
+
+    parsed_entries.emplace_back(Entry(std::move(*maybe_issuer),
+                                      raw_key_from_json,
+                                      std::move(commitment_result)));
+  }
+
+  // Deterministically deduplicate entries corresponding to the same issuer,
+  // with the largest JSON key lexicographically winning.
+  std::sort(parsed_entries.begin(), parsed_entries.end(), std::greater<>());
+  parsed_entries.erase(std::unique(parsed_entries.begin(), parsed_entries.end(),
+                                   [](Entry& lhs, Entry& rhs) -> bool {
+                                     return canonicalized_issuer(lhs) ==
+                                            canonicalized_issuer(rhs);
+                                   }),
+                       parsed_entries.end());
+
+  // Finally, discard the raw JSON strings and construct a map to return.
+  std::vector<std::pair<SuitableTrustTokenOrigin,
+                        mojom::TrustTokenKeyCommitmentResultPtr>>
+      map_storage;
+  map_storage.reserve(parsed_entries.size());
+  for (Entry& e : parsed_entries) {
+    map_storage.emplace_back(std::move(canonicalized_issuer(e)),
+                             std::move(commitment(e)));
+  }
+  return std::make_unique<base::flat_map<
+      SuitableTrustTokenOrigin, mojom::TrustTokenKeyCommitmentResultPtr>>(
+      std::move(map_storage));
+}
+
 }  // namespace network

services/network/trust_tokens/trust_token_key_commitment_parser.h

@@ -9,6 +9,7 @@
 
 #include "base/strings/string_piece_forward.h"
 #include "services/network/public/mojom/trust_tokens.mojom-forward.h"
+#include "services/network/trust_tokens/suitable_trust_token_origin.h"
 #include "services/network/trust_tokens/trust_token_key_commitment_controller.h"
 
 namespace network {
@@ -46,6 +47,30 @@ class TrustTokenKeyCommitmentParser
   //   epoch) storing a time in the future.
   mojom::TrustTokenKeyCommitmentResultPtr Parse(
       base::StringPiece response_body) override;
+
+  // Like |Parse|, except that the input is expected to be of the form
+  // { "https://some-issuer.example": <JSON in the form expected by |Parse|>
+  //   "https://some-other-issuer.example":
+  //     <JSON in the form expected by |Parse|>,
+  //   ...  }
+  //
+  // Returns nullptr if the input is not a dictionary.
+  //
+  // WARNING: If there are multiple keys that are exactly equal strings,
+  // deduplicates these entries arbitrarily (due to the behavior of
+  // base::JSONReader). For instance, if these keys are arriving through the
+  // component updater, you might want to guarantee that the server-side logic
+  // producing these structures guarantees no duplicate keys.
+  //
+  // If there are multiple keys that are not exact duplicates but correspond to
+  // the same issuer, drops all but the entry with the largest key
+  // lexicographically.
+  //
+  // Skips key-value pairs where the key is not a suitable Trust Tokens origin
+  // or the value fails to parse.
+  std::unique_ptr<base::flat_map<SuitableTrustTokenOrigin,
+                                 mojom::TrustTokenKeyCommitmentResultPtr>>
+  ParseMultipleIssuers(base::StringPiece response_body);
 };
 
 }  // namespace network

services/network/trust_tokens/trust_token_key_commitments.cc

@@ -9,6 +9,7 @@
 #include "base/optional.h"
 #include "services/network/public/mojom/trust_tokens.mojom-forward.h"
 #include "services/network/trust_tokens/suitable_trust_token_origin.h"
+#include "services/network/trust_tokens/trust_token_key_commitment_parser.h"
 
 namespace network {
 
@@ -43,6 +44,13 @@ void TrustTokenKeyCommitments::Set(
   map_.replace(std::move(filtered));
 }
 
+void TrustTokenKeyCommitments::ParseAndSet(base::StringPiece raw_commitments) {
+  TrustTokenKeyCommitmentParser parser;
+  if (auto commitments = parser.ParseMultipleIssuers(raw_commitments)) {
+    map_.swap(*commitments);
+  }
+}
+
 void TrustTokenKeyCommitments::Get(
     const url::Origin& origin,
     base::OnceCallback<void(mojom::TrustTokenKeyCommitmentResultPtr)> done)

services/network/trust_tokens/trust_token_key_commitments.h

@@ -33,6 +33,13 @@ class TrustTokenKeyCommitments : public TrustTokenKeyCommitmentGetter {
   void Set(
       base::flat_map<url::Origin, mojom::TrustTokenKeyCommitmentResultPtr> map);
 
+  // Overwrites the current issuers-to-commitments map with the values in
+  // |raw_commitments|, which should be the JSON-encoded string representation
+  // of a collection of issuers' key commitments according to the format
+  // specified, for now, in the Trust Tokens design doc:
+  // https://docs.google.com/document/d/1TNnya6B8pyomDK2F1R9CL3dY10OAmqWlnCxsWyOBDVQ/edit#heading=h.z52drgpfgulz.
+  void ParseAndSet(base::StringPiece raw_commitments);
+
   // TrustTokenKeyCommitmentGetter implementation:
   //
   // If |origin| is a suitable Trust Tokens origin (in the sense of

services/network/trust_tokens/trust_token_key_commitments_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/test/bind_test_util.h"
 #include "services/network/public/mojom/trust_tokens.mojom-forward.h"
 #include "services/network/public/mojom/trust_tokens.mojom.h"
+#include "services/network/trust_tokens/suitable_trust_token_origin.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 #include "url/origin.h"
@@ -122,4 +123,14 @@ TEST(TrustTokenKeyCommitments, MultipleOrigins) {
   }
 }
 
+TEST(TrustTokenKeyCommitments, ParseAndSet) {
+  TrustTokenKeyCommitments commitments;
+  commitments.ParseAndSet(
+      R"( { "https://issuer.example": { "srrkey": "aaaa" } } )");
+
+  EXPECT_TRUE(GetCommitmentForOrigin(
+      commitments,
+      *SuitableTrustTokenOrigin::Create(GURL("https://issuer.example"))));
+}
+
 }  // namespace network