Modify First-Party Set parsing to store owners explicitly, and disallow

singleton sets.

This is required since we need to be able to (quickly) distinguish sites
that are members of a non-singleton set from those that are members of a
singleton set (i.e. they are not a member of any set provided by
Component Updater or on the command line). This, in turn, is needed
because we need Chrome to ignore the SameParty attribute for cookies set
by sites that are not in a First-Party Set, in order to avoid site
breakage in the event of a race condition between the site registering
their First-Party Set (and applying the SameParty attribute), and Chrome
receiving the updated set list from Component Updater.

Bug: 1143756
Change-Id: If77b60538c084a306b1c7af3d1bdabdb9fd1b0e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560899
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833353}

content/browser/network_service_browsertest.cc

@@ -459,11 +459,11 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceWithFirstPartySetBrowserTest,
   if (IsInProcessNetworkService())
     return;
 
-  EXPECT_EQ(GetPreloadedFirstPartySetCountFromNetworkService(), 2);
+  EXPECT_EQ(GetPreloadedFirstPartySetCountFromNetworkService(), 3);
 
   SimulateNetworkServiceCrash();
 
-  EXPECT_EQ(GetPreloadedFirstPartySetCountFromNetworkService(), 2);
+  EXPECT_EQ(GetPreloadedFirstPartySetCountFromNetworkService(), 3);
 }
 
 // Tests that CORS is performed by the network service when |factory_override|

services/network/first_party_sets/first_party_set_parser.cc

@@ -58,12 +58,13 @@ base::Optional<net::SchemefulSite> Canonicalize(base::StringPiece origin_string,
 const char kFirstPartySetOwnerField[] = "owner";
 const char kFirstPartySetMembersField[] = "members";
 
-// Parses a single First-Party Set into a map from member to owner (not
-// including the owner). Note that this is intended for use *only* on sets that
+// Parses a single First-Party Set into a map from member to owner (including an
+// entry owner -> owner). Note that this is intended for use *only* on sets that
 // were preloaded via the component updater, so this does not check assertions
 // or versions. It rejects sets which are non-disjoint with
 // previously-encountered sets (i.e. sets which have non-empty intersections
-// with `elements`).
+// with `elements`), and singleton sets (i.e. sets must have an owner and at
+// least one valid member).
 //
 // Uses `elements` to check disjointness of sets; builds the mapping in `map`;
 // and augments `elements` to include the elements of the set that was parsed.
@@ -92,6 +93,7 @@ bool ParsePreloadedSet(
     return false;
 
   elements.insert(*canonical_owner);
+  map.emplace(*canonical_owner, *canonical_owner);
 
   // Confirm that the members field is present, and is an array of strings.
   const base::Value* maybe_members_list =
@@ -112,7 +114,7 @@ bool ParsePreloadedSet(
     map.emplace(*member, *canonical_owner);
     elements.insert(std::move(*member));
   }
-  return true;
+  return !maybe_members_list->GetList().empty();
 }
 
 }  // namespace

services/network/first_party_sets/first_party_set_parser_unittest.cc

@@ -59,6 +59,19 @@ TEST(FirstPartySetParser, AcceptsTrivial) {
               Pointee(IsEmpty()));
 }
 
+TEST(FirstPartySetParser, RejectsSingletonSet) {
+  const std::string input =
+      R"([{
+        "owner": "https://example.test",
+        "members": []
+        }])";
+
+  // Sanity check that the input is actually valid JSON.
+  ASSERT_TRUE(base::JSONReader::Read(input));
+
+  EXPECT_FALSE(FirstPartySetParser::ParsePreloadedSets(input));
+}
+
 TEST(FirstPartySetParser, AcceptsMinimal) {
   const std::string input =
       R"([{
@@ -71,6 +84,8 @@ TEST(FirstPartySetParser, AcceptsMinimal) {
 
   EXPECT_THAT(FirstPartySetParser::ParsePreloadedSets(input),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://aaaa.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -206,6 +221,8 @@ TEST(FirstPartySetParser, TruncatesSubdomain_Owner) {
 
   EXPECT_THAT(FirstPartySetParser::ParsePreloadedSets(input),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://aaaa.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -222,6 +239,8 @@ TEST(FirstPartySetParser, TruncatesSubdomain_Member) {
 
   EXPECT_THAT(FirstPartySetParser::ParsePreloadedSets(input),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://aaaa.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -245,8 +264,12 @@ TEST(FirstPartySetParser, AcceptsMultipleSets) {
 
   EXPECT_THAT(
       FirstPartySetParser::ParsePreloadedSets(input),
-      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://member1.test"),
+      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://example.test"),
+                                        SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://member1.test"),
                                         SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://foo.test"),
+                                        SerializesTo("https://foo.test")),
                                    Pair(SerializesTo("https://member2.test"),
                                         SerializesTo("https://foo.test")))));
 }
@@ -307,6 +330,8 @@ TEST(FirstPartySetParser, AllowsTrailingCommas) {
 
   EXPECT_THAT(FirstPartySetParser::ParsePreloadedSets(input),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member1.test"),
                        SerializesTo("https://example.test")))));
 }

services/network/first_party_sets/preloaded_first_party_sets.cc

@@ -87,13 +87,34 @@ void PreloadedFirstPartySets::ApplyManuallySpecifiedSet() {
   const base::flat_set<net::SchemefulSite>& manual_members =
       manually_specified_set_->second;
 
-  sets_.erase(
-      base::ranges::remove_if(sets_,
-                              [&manual_members, &manual_owner](const auto& p) {
-                                return p.first == manual_owner ||
-                                       p.second == manual_owner ||
-                                       manual_members.contains(p.first) ||
-                                       manual_members.contains(p.second);
+  const auto was_manually_provided =
+      [&manual_members, &manual_owner](const net::SchemefulSite& site) {
+        return site == manual_owner || manual_members.contains(site);
+      };
+
+  // Erase the intersection between the manually-specified set and the
+  // CU-supplied set, and any members whose owner was in the intersection.
+  sets_.erase(base::ranges::remove_if(sets_,
+                                      [&was_manually_provided](const auto& p) {
+                                        return was_manually_provided(p.first) ||
+                                               was_manually_provided(p.second);
+                                      }),
+              sets_.end());
+
+  // Now remove singleton sets. We already removed any sites that were part
+  // of the intersection, or whose owner was part of the intersection. This
+  // leaves sites that *are* owners, which no longer have any (other)
+  // members.
+  std::set<net::SchemefulSite> owners_with_members;
+  for (const auto& it : sets_) {
+    if (it.first != it.second)
+      owners_with_members.insert(it.second);
+  }
+  sets_.erase(base::ranges::remove_if(
+                  sets_,
+                  [&owners_with_members](const auto& p) {
+                    return p.first == p.second &&
+                           !base::Contains(owners_with_members, p.first);
                   }),
               sets_.end());
 
@@ -101,6 +122,7 @@ void PreloadedFirstPartySets::ApplyManuallySpecifiedSet() {
   for (const net::SchemefulSite& member : manual_members) {
     sets_.emplace(member, manual_owner);
   }
+  sets_.emplace(manual_owner, manual_owner);
 }
 
 }  // namespace network

services/network/first_party_sets/preloaded_first_party_sets.h

@@ -50,6 +50,9 @@ class PreloadedFirstPartySets {
   // `manually_specified_set_`.
   void ApplyManuallySpecifiedSet();
 
+  // Represents the mapping of site -> site, where keys are members of sets, and
+  // values are owners of the sets. Owners are explicitly represented as members
+  // of the set.
   base::flat_map<net::SchemefulSite, net::SchemefulSite> sets_;
   base::Optional<
       std::pair<net::SchemefulSite, base::flat_set<net::SchemefulSite>>>

services/network/first_party_sets/preloaded_first_party_sets_unittest.cc

@@ -39,6 +39,8 @@ TEST(PreloadedFirstPartySets, AcceptsMinimal) {
 
   EXPECT_THAT(PreloadedFirstPartySets().ParseAndSet(input),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://aaaa.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -60,8 +62,12 @@ TEST(PreloadedFirstPartySets, AcceptsMultipleSets) {
 
   EXPECT_THAT(
       PreloadedFirstPartySets().ParseAndSet(input),
-      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://member1.test"),
+      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://example.test"),
+                                        SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://member1.test"),
                                         SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://foo.test"),
+                                        SerializesTo("https://foo.test")),
                                    Pair(SerializesTo("https://member2.test"),
                                         SerializesTo("https://foo.test")))));
 }
@@ -84,8 +90,12 @@ TEST(PreloadedFirstPartySets, ClearsPreloadedOnError) {
   PreloadedFirstPartySets sets;
   EXPECT_THAT(
       sets.ParseAndSet(input),
-      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://member1.test"),
+      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://example.test"),
+                                        SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://member1.test"),
                                         SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://foo.test"),
+                                        SerializesTo("https://foo.test")),
                                    Pair(SerializesTo("https://member2.test"),
                                         SerializesTo("https://foo.test")))));
 
@@ -189,6 +199,8 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_Valid_SingleMember) {
   sets.SetManuallySpecifiedSet("https://example.test,https://member.test");
   EXPECT_THAT(sets.ParseAndSet("[]"),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -200,6 +212,8 @@ TEST(PreloadedFirstPartySets,
       "https://www.example.test,https://www.member.test");
   EXPECT_THAT(sets.ParseAndSet("[]"),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -210,6 +224,8 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_Valid_MultipleMembers) {
       "https://example.test,https://member1.test,https://member2.test");
   EXPECT_THAT(sets.ParseAndSet("[]"),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member1.test"),
                        SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member2.test"),
@@ -228,6 +244,8 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_Valid_OwnerIsMember) {
       "https://example.test,https://example.test,https://member1.test");
   EXPECT_THAT(sets.ParseAndSet("[]"),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member1.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -241,6 +259,8 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_Valid_RepeatedMember) {
        https://member1.test)");
   EXPECT_THAT(sets.ParseAndSet("[]"),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member1.test"),
                        SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member2.test"),
@@ -267,10 +287,14 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_DeduplicatesOwnerOwner) {
       "https://example.test,https://member1.test,https://member2.test");
   EXPECT_THAT(
       sets.ParseAndSet(input),
-      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://member1.test"),
+      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://example.test"),
+                                        SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://member1.test"),
                                         SerializesTo("https://example.test")),
                                    Pair(SerializesTo("https://member2.test"),
                                         SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://bar.test"),
+                                        SerializesTo("https://bar.test")),
                                    Pair(SerializesTo("https://member4.test"),
                                         SerializesTo("https://bar.test")))));
 }
@@ -295,8 +319,12 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_DeduplicatesOwnerMember) {
       "https://example.test,https://member1.test,https://member3.test");
   EXPECT_THAT(sets.ParseAndSet(input),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member1.test"),
                        SerializesTo("https://example.test")),
+                  Pair(SerializesTo("https://bar.test"),
+                       SerializesTo("https://bar.test")),
                   Pair(SerializesTo("https://member2.test"),
                        SerializesTo("https://bar.test")),
                   Pair(SerializesTo("https://member3.test"),
@@ -322,10 +350,14 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_DeduplicatesMemberOwner) {
   sets.SetManuallySpecifiedSet("https://example.test,https://member3.test");
   EXPECT_THAT(sets.ParseAndSet(input),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://foo.test"),
+                       SerializesTo("https://foo.test")),
                   Pair(SerializesTo("https://member1.test"),
                        SerializesTo("https://foo.test")),
                   Pair(SerializesTo("https://member2.test"),
                        SerializesTo("https://foo.test")),
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member3.test"),
                        SerializesTo("https://example.test")))));
 }
@@ -350,12 +382,18 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_DeduplicatesMemberMember) {
       "https://example.test,https://member1.test,https://member2.test");
   EXPECT_THAT(
       sets.ParseAndSet(input),
-      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://member1.test"),
+      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://example.test"),
+                                        SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://member1.test"),
                                         SerializesTo("https://example.test")),
                                    Pair(SerializesTo("https://member2.test"),
                                         SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://foo.test"),
+                                        SerializesTo("https://foo.test")),
                                    Pair(SerializesTo("https://member3.test"),
                                         SerializesTo("https://foo.test")),
+                                   Pair(SerializesTo("https://bar.test"),
+                                        SerializesTo("https://bar.test")),
                                    Pair(SerializesTo("https://member4.test"),
                                         SerializesTo("https://bar.test")))));
 }
@@ -376,19 +414,49 @@ TEST(PreloadedFirstPartySets, SetsManuallySpecified_ClearsPreloadedOnError) {
       "https://example.test,https://member1.test,https://member2.test");
   EXPECT_THAT(
       sets.ParseAndSet(input),
-      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://member1.test"),
+      Pointee(UnorderedElementsAre(Pair(SerializesTo("https://example.test"),
+                                        SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://member1.test"),
                                         SerializesTo("https://example.test")),
                                    Pair(SerializesTo("https://member2.test"),
                                         SerializesTo("https://example.test")),
+                                   Pair(SerializesTo("https://bar.test"),
+                                        SerializesTo("https://bar.test")),
                                    Pair(SerializesTo("https://member3.test"),
                                         SerializesTo("https://bar.test")))));
 
   EXPECT_THAT(sets.ParseAndSet("{}"),
               Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member1.test"),
                        SerializesTo("https://example.test")),
                   Pair(SerializesTo("https://member2.test"),
                        SerializesTo("https://example.test")))));
 }
 
+TEST(PreloadedFirstPartySets, SetsManuallySpecified_PrunesInducedSingletons) {
+  const std::string input = R"(
+  [
+    {
+      "owner": "https://foo.test",
+      "members": ["https://member1.test"]
+    }
+  ]
+  )";
+  ASSERT_TRUE(base::JSONReader::Read(input));
+
+  PreloadedFirstPartySets sets;
+  sets.SetManuallySpecifiedSet("https://example.test,https://member1.test");
+  // If we just erased entries that overlapped with the manually-supplied set,
+  // https://foo.test would be left as a singleton set. But since we disallow
+  // singleton sets, we ensure that such cases are caught and removed.
+  EXPECT_THAT(sets.ParseAndSet(input),
+              Pointee(UnorderedElementsAre(
+                  Pair(SerializesTo("https://example.test"),
+                       SerializesTo("https://example.test")),
+                  Pair(SerializesTo("https://member1.test"),
+                       SerializesTo("https://example.test")))));
+}
+
 }  // namespace network