Bring SecurityPolicy::GenerateReferrer in sync with //content

Currently, we have three places where we compute requests' referrers,
nominally according to [1, Algorithm 8.1].

This change makes a couple alterations to
SecurityPolicy::GenerateReferrer (one of the places) to bring it in line
with the behavior in content::Referrer::SanitizeForRequest (another
implementation of the algorithm). After this change,
SecurityPolicy::GenerateReferrer will:
- truncate requests with referrers longer than 4096 bytes to the
initiating origin, as specified in [1, Alg. 8.1, step 6];
- strip URLs for use as referrers as part of the referrer generation
process [step 4]; and
- explicitly return "no referrer" when given an invalid URL (motivation:
this will align the behavior with the other two implementations of the
algorithm once they also strip URLs for use as referrers as part of the
referrer generation process, because this is how GURL::GetAsReferrer
behaves).

In order to implement the second of these in a manner consistent with
the non-Blink code, this change alters KURL's referrer stripping logic
to move from "return a default value if the URL is not http(s)" to
"return a default value if the scheme is not suitable for use in a
referrer", bringing it in line with GURL::GetAsReferrer.

[1]:
https://w3c.github.io/webappsec-referrer-policy/#parse-referrer-policy-from-header

Bug: 1094526
Change-Id: I515fd9e832e99c1fdd2ca207560b865d06edc39c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2246987Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781386}

third_party/blink/renderer/platform/weborigin/kurl.cc

@@ -30,6 +30,7 @@
 #include <algorithm>
 
 #include "third_party/blink/renderer/platform/weborigin/known_ports.h"
+#include "third_party/blink/renderer/platform/weborigin/scheme_registry.h"
 #include "third_party/blink/renderer/platform/wtf/math_extras.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_hash.h"
@@ -126,19 +127,21 @@ bool IsValidProtocol(const String& protocol) {
   return true;
 }
 
-String KURL::StrippedForUseAsReferrer() const {
-  if (!ProtocolIsInHTTPFamily())
-    return String();
+KURL KURL::UrlStrippedForUseAsReferrer() const {
+  if (!SchemeRegistry::ShouldTreatURLSchemeAsAllowedForReferrer(Protocol()))
+    return KURL();
 
-  if (parsed_.username.is_nonempty() || parsed_.password.is_nonempty() ||
-      parsed_.ref.is_valid()) {
-    KURL referrer(*this);
-    referrer.SetUser(String());
-    referrer.SetPass(String());
-    referrer.RemoveFragmentIdentifier();
-    return referrer.GetString();
-  }
-  return GetString();
+  KURL referrer(*this);
+
+  referrer.SetUser(String());
+  referrer.SetPass(String());
+  referrer.RemoveFragmentIdentifier();
+
+  return referrer;
+}
+
+String KURL::StrippedForUseAsReferrer() const {
+  return UrlStrippedForUseAsReferrer().GetString();
 }
 
 String KURL::StrippedForUseAsHref() const {

third_party/blink/renderer/platform/weborigin/kurl.h

@@ -114,6 +114,7 @@ class PLATFORM_EXPORT KURL {
 
   ~KURL();
 
+  KURL UrlStrippedForUseAsReferrer() const;
   String StrippedForUseAsReferrer() const;
   String StrippedForUseAsHref() const;
 

third_party/blink/renderer/platform/weborigin/kurl_test.cc

@@ -38,6 +38,7 @@
 #include "base/stl_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/platform/scheduler/public/thread.h"
+#include "third_party/blink/renderer/platform/weborigin/scheme_registry.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 #include "url/url_util.h"
@@ -850,27 +851,63 @@ TEST(KURLTest, ProtocolIs) {
   EXPECT_EQ(capital.Protocol(), "http");
 }
 
+TEST(KURLTest, urlStrippedForUseAsReferrer) {
+  struct ReferrerCase {
+    const String input;
+    const String output;
+  } referrer_cases[] = {
+      {"data:text/html;charset=utf-8,<html></html>", String()},
+      {"javascript:void(0);", String()},
+      {"about:config", String()},
+      {"https://www.google.com/", "https://www.google.com/"},
+      {"http://me@news.google.com:8888/", "http://news.google.com:8888/"},
+      {"http://:pass@news.google.com:8888/foo",
+       "http://news.google.com:8888/foo"},
+      {"http://me:pass@news.google.com:8888/", "http://news.google.com:8888/"},
+      {"https://www.google.com/a?f#b", "https://www.google.com/a?f"},
+      {"file:///tmp/test.html", String()},
+      {"https://www.google.com/#", "https://www.google.com/"},
+  };
+
+  for (const ReferrerCase& referrer_case : referrer_cases) {
+    const KURL kurl(referrer_case.input);
+    EXPECT_EQ(KURL(referrer_case.output), kurl.UrlStrippedForUseAsReferrer());
+  }
+}
+
+TEST(KURLTest, urlStrippedForUseAsReferrerRespectsReferrerScheme) {
+  const KURL example_http_url = KURL("http://example.com/");
+  const KURL foobar_url = KURL("foobar://somepage/");
+  const String foobar_scheme = String::FromUTF8("foobar");
+
+  EXPECT_EQ("", foobar_url.StrippedForUseAsReferrer().Utf8());
+
+  SchemeRegistry::RegisterURLSchemeAsAllowedForReferrer(foobar_scheme);
+  EXPECT_EQ("foobar://somepage/", foobar_url.StrippedForUseAsReferrer());
+  SchemeRegistry::RemoveURLSchemeAsAllowedForReferrer(foobar_scheme);
+}
+
 TEST(KURLTest, strippedForUseAsReferrer) {
   struct ReferrerCase {
     const char* input;
-    const char* output;
+    const String output;
   } referrer_cases[] = {
-      {"data:text/html;charset=utf-8,<html></html>", ""},
-      {"javascript:void(0);", ""},
-      {"about:config", ""},
+      {"data:text/html;charset=utf-8,<html></html>", String()},
+      {"javascript:void(0);", String()},
+      {"about:config", String()},
       {"https://www.google.com/", "https://www.google.com/"},
       {"http://me@news.google.com:8888/", "http://news.google.com:8888/"},
       {"http://:pass@news.google.com:8888/foo",
        "http://news.google.com:8888/foo"},
       {"http://me:pass@news.google.com:8888/", "http://news.google.com:8888/"},
       {"https://www.google.com/a?f#b", "https://www.google.com/a?f"},
-      {"file:///tmp/test.html", ""},
+      {"file:///tmp/test.html", String()},
       {"https://www.google.com/#", "https://www.google.com/"},
   };
 
-  for (size_t i = 0; i < base::size(referrer_cases); i++) {
-    const KURL kurl(referrer_cases[i].input);
-    EXPECT_EQ(referrer_cases[i].output, kurl.StrippedForUseAsReferrer().Utf8());
+  for (const ReferrerCase& referrer_case : referrer_cases) {
+    const KURL kurl(referrer_case.input);
+    EXPECT_EQ(referrer_case.output, kurl.StrippedForUseAsReferrer());
   }
 }
 

third_party/blink/renderer/platform/weborigin/security_policy.cc

@@ -112,63 +112,56 @@ Referrer SecurityPolicy::GenerateReferrer(
     return Referrer(Referrer::NoReferrer(), referrer_policy_no_default);
   DCHECK(!referrer.IsEmpty());
 
-  KURL referrer_url = KURL(NullURL(), referrer);
-  String scheme = referrer_url.Protocol();
-  if (!SchemeRegistry::ShouldTreatURLSchemeAsAllowedForReferrer(scheme))
+  KURL referrer_url = KURL(NullURL(), referrer).UrlStrippedForUseAsReferrer();
+
+  if (!referrer_url.IsValid())
     return Referrer(Referrer::NoReferrer(), referrer_policy_no_default);
 
   if (SecurityOrigin::ShouldUseInnerURL(url))
     return Referrer(Referrer::NoReferrer(), referrer_policy_no_default);
 
+  // 5. Let referrerOrigin be the result of stripping referrerSource for use as
+  // a referrer, with the origin-only flag set to true.
+  KURL referrer_origin = referrer_url;
+  referrer_origin.SetPath(String());
+  referrer_origin.SetQuery(String());
+
+  // 6. If the result of serializing referrerURL is a string whose length is
+  // greater than 4096, set referrerURL to referrerOrigin.
+  if (referrer_url.GetString().length() > 4096)
+    referrer_url = referrer_origin;
+
   switch (referrer_policy_no_default) {
     case network::mojom::ReferrerPolicy::kNever:
       return Referrer(Referrer::NoReferrer(), referrer_policy_no_default);
     case network::mojom::ReferrerPolicy::kAlways:
-      return Referrer(referrer, referrer_policy_no_default);
+      return Referrer(referrer_url, referrer_policy_no_default);
     case network::mojom::ReferrerPolicy::kOrigin: {
-      String origin = SecurityOrigin::Create(referrer_url)->ToString();
-      // A security origin is not a canonical URL as it lacks a path. Add /
-      // to turn it into a canonical URL we can use as referrer.
-      return Referrer(origin + "/", referrer_policy_no_default);
+      return Referrer(referrer_origin, referrer_policy_no_default);
     }
     case network::mojom::ReferrerPolicy::kOriginWhenCrossOrigin: {
-      scoped_refptr<const SecurityOrigin> referrer_origin =
-          SecurityOrigin::Create(referrer_url);
-      scoped_refptr<const SecurityOrigin> url_origin =
-          SecurityOrigin::Create(url);
-      if (!url_origin->IsSameOriginWith(referrer_origin.get())) {
-        String origin = referrer_origin->ToString();
-        return Referrer(origin + "/", referrer_policy_no_default);
+      if (!SecurityOrigin::AreSameOrigin(referrer_url, url)) {
+        return Referrer(referrer_origin, referrer_policy_no_default);
       }
       break;
     }
     case network::mojom::ReferrerPolicy::kSameOrigin: {
-      scoped_refptr<const SecurityOrigin> referrer_origin =
-          SecurityOrigin::Create(referrer_url);
-      scoped_refptr<const SecurityOrigin> url_origin =
-          SecurityOrigin::Create(url);
-      if (!url_origin->IsSameOriginWith(referrer_origin.get())) {
+      if (!SecurityOrigin::AreSameOrigin(referrer_url, url)) {
         return Referrer(Referrer::NoReferrer(), referrer_policy_no_default);
       }
-      return Referrer(referrer, referrer_policy_no_default);
+      return Referrer(referrer_url, referrer_policy_no_default);
     }
     case network::mojom::ReferrerPolicy::kStrictOrigin: {
-      String origin = SecurityOrigin::Create(referrer_url)->ToString();
       return Referrer(ShouldHideReferrer(url, referrer_url)
                           ? Referrer::NoReferrer()
-                          : origin + "/",
+                          : referrer_origin,
                       referrer_policy_no_default);
     }
     case network::mojom::ReferrerPolicy::kStrictOriginWhenCrossOrigin: {
-      scoped_refptr<const SecurityOrigin> referrer_origin =
-          SecurityOrigin::Create(referrer_url);
-      scoped_refptr<const SecurityOrigin> url_origin =
-          SecurityOrigin::Create(url);
-      if (!url_origin->IsSameOriginWith(referrer_origin.get())) {
-        String origin = referrer_origin->ToString();
+      if (!SecurityOrigin::AreSameOrigin(referrer_url, url)) {
         return Referrer(ShouldHideReferrer(url, referrer_url)
                             ? Referrer::NoReferrer()
-                            : origin + "/",
+                            : referrer_origin,
                         referrer_policy_no_default);
       }
       break;
@@ -180,9 +173,9 @@ Referrer SecurityPolicy::GenerateReferrer(
       break;
   }
 
-  return Referrer(
-      ShouldHideReferrer(url, referrer_url) ? Referrer::NoReferrer() : referrer,
-      referrer_policy_no_default);
+  return Referrer(ShouldHideReferrer(url, referrer_url) ? Referrer::NoReferrer()
+                                                        : referrer_url,
+                  referrer_policy_no_default);
 }
 
 void SecurityPolicy::AddOriginToTrustworthySafelist(

third_party/blink/renderer/platform/weborigin/security_policy_test.cc

@@ -101,6 +101,7 @@ TEST(SecurityPolicyTest, GenerateReferrer) {
   const char kBlobURL[] =
       "blob:http://a.test/b3aae9c8-7f90-440d-8d7c-43aa20d72fde";
   const char kFilesystemURL[] = "filesystem:http://a.test/path/t/file.html";
+  const char kInvalidURL[] = "not-a-valid-url";
 
   bool reduced_granularity =
       RuntimeEnabledFeatures::ReducedReferrerGranularityEnabled();
@@ -226,7 +227,7 @@ TEST(SecurityPolicyTest, GenerateReferrer) {
       {network::mojom::ReferrerPolicy::kStrictOriginWhenCrossOrigin,
        kSecureURLA, kInsecureURLB, nullptr},
 
-      // blob and filesystem URL handling
+      // blob, filesystem, and invalid URL handling
       {network::mojom::ReferrerPolicy::kAlways, kInsecureURLA, kBlobURL,
        nullptr},
       {network::mojom::ReferrerPolicy::kAlways, kBlobURL, kInsecureURLA,
@@ -235,6 +236,10 @@ TEST(SecurityPolicyTest, GenerateReferrer) {
        nullptr},
       {network::mojom::ReferrerPolicy::kAlways, kFilesystemURL, kInsecureURLA,
        nullptr},
+      {network::mojom::ReferrerPolicy::kAlways, kInsecureURLA, kInvalidURL,
+       kInsecureURLA},
+      {network::mojom::ReferrerPolicy::kAlways, kInvalidURL, kInsecureURLA,
+       nullptr},
   };
 
   for (TestCase test : inputs) {
@@ -244,7 +249,8 @@ TEST(SecurityPolicyTest, GenerateReferrer) {
     if (test.expected) {
       EXPECT_EQ(String::FromUTF8(test.expected), result.referrer)
           << "'" << test.referrer << "' to '" << test.destination
-          << "' should have been '" << test.expected << "': was '"
+          << "' with policy=" << static_cast<int>(test.policy)
+          << " should have been '" << test.expected << "': was '"
           << result.referrer.Utf8() << "'.";
     } else {
       EXPECT_TRUE(result.referrer.IsEmpty())
@@ -267,6 +273,42 @@ TEST(SecurityPolicyTest, GenerateReferrer) {
   }
 }
 
+TEST(SecurityPolicyTest, GenerateReferrerTruncatesLongUrl) {
+  char buffer[4097];
+  std::fill_n(std::begin(buffer), 4097, 'a');
+
+  String base = "https://a.com/";
+  String string_with_4096 = base + String(buffer, 4096 - base.length());
+  ASSERT_EQ(string_with_4096.length(), 4096u);
+
+  network::mojom::ReferrerPolicy kAlways =
+      network::mojom::ReferrerPolicy::kAlways;
+  EXPECT_EQ(SecurityPolicy::GenerateReferrer(
+                kAlways, KURL("https://destination.example"), string_with_4096)
+                .referrer,
+            string_with_4096);
+
+  String string_with_4097 = base + String(buffer, 4097 - base.length());
+  ASSERT_EQ(string_with_4097.length(), 4097u);
+  EXPECT_EQ(SecurityPolicy::GenerateReferrer(
+                kAlways, KURL("https://destination.example"), string_with_4097)
+                .referrer,
+            "https://a.com/");
+
+  // Since refs get stripped from outgoing referrers prior to the "if the length
+  // is greater than 4096, strip the referrer to its origin" check, a
+  // referrer with length > 4096 due to its path should not get stripped to its
+  // outgoing origin.
+  String string_with_4097_because_of_long_ref =
+      base + "path#" + String(buffer, 4097 - 5 - base.length());
+  ASSERT_EQ(string_with_4097_because_of_long_ref.length(), 4097u);
+  EXPECT_EQ(SecurityPolicy::GenerateReferrer(
+                kAlways, KURL("https://destination.example"),
+                string_with_4097_because_of_long_ref)
+                .referrer,
+            "https://a.com/path");
+}
+
 TEST(SecurityPolicyTest, ReferrerPolicyFromHeaderValue) {
   struct TestCase {
     const char* header;