Fix cookie exclusion for fetch CORS pre-flight requests (fixes #3596)

Cookies (and other credentials) will be excluded when appropriate by
downgrading |credentials_mode| from kSameOrigin to kOmit.

Improve logic for Origin header inclusion, including a fix for
Referrer/Origin calculation in URLRequestJob::ComputeReferrerForPolicy
when used with custom standard schemes.

Specify correct CookiePartitionKeyCollection when loading cookies.

To test:
- Run tests from https://browseraudit.com/ with and without
  `--disable-request-handling-for-testing`. Results are the same.
- Run `ceftests --gtest_filter=CorsTest.*`.

libcef/browser/net_service/cookie_helper.cc

@@ -249,11 +249,19 @@ void LoadCookies(const CefBrowserContext::Getter& browser_context_getter,
     return;
   }
 
+  net::CookiePartitionKeyCollection partition_key_collection;
+  if (request.trusted_params.has_value() &&
+      !request.trusted_params->isolation_info.IsEmpty()) {
+    partition_key_collection = net::CookiePartitionKeyCollection::FromOptional(
+        net::CookiePartitionKey::FromNetworkIsolationKey(
+            request.trusted_params->isolation_info.network_isolation_key()));
+  }
+
   CEF_POST_TASK(
       CEF_UIT,
       base::BindOnce(LoadCookiesOnUIThread, browser_context_getter, request.url,
                      GetCookieOptions(request, /*for_loading_cookies=*/true),
-                     net::CookiePartitionKeyCollection(), allow_cookie_callback,
+                     std::move(partition_key_collection), allow_cookie_callback,
                      std::move(done_callback)));
 }
 

libcef/browser/net_service/proxy_url_loader_factory.cc

@@ -22,6 +22,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/referrer.h"
 #include "mojo/public/cpp/base/big_buffer.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "net/http/http_status_code.h"
@@ -30,6 +31,7 @@
 #include "services/network/public/cpp/cors/cors.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/early_hints.mojom.h"
+#include "third_party/blink/public/common/loader/referrer_utils.h"
 
 namespace net_service {
 
@@ -65,6 +67,21 @@ bool DisableRequestHandlingForTesting() {
   return disabled;
 }
 
+// Match logic in devtools_url_loader_interceptor.cc
+// InterceptionJob::CalculateResponseTainting.
+network::mojom::FetchResponseType CalculateResponseTainting(
+    bool should_check_cors,
+    network::mojom::RequestMode mode,
+    bool tainted_origin) {
+  if (should_check_cors) {
+    return network::mojom::FetchResponseType::kCors;
+  }
+  if (mode == network::mojom::RequestMode::kNoCors && tainted_origin) {
+    return network::mojom::FetchResponseType::kOpaque;
+  }
+  return network::mojom::FetchResponseType::kBasic;
+}
+
 }  // namespace
 
 // Owns all of the ProxyURLLoaderFactorys for a given BrowserContext. Since
@@ -451,16 +468,20 @@ void InterceptedRequest::Restart() {
   current_request_uses_header_client_ =
       factory_->url_loader_header_client_receiver_.is_bound();
 
-  if (request_.request_initiator &&
+  const bool is_cross_origin =
-      network::cors::ShouldCheckCors(request_.url, request_.request_initiator,
+      request_.request_initiator &&
-                                     request_.mode)) {
+      !request_.request_initiator->IsSameOriginWith(request_.url);
-    if (scheme::IsCorsEnabledScheme(request_.url.scheme())) {
+  const bool is_cors_enabled_scheme =
-      // Add the Origin header for CORS-enabled scheme requests.
+      scheme::IsCorsEnabledScheme(request_.url.scheme());
-      request_.headers.SetHeaderIfMissing(
+
-          net::HttpRequestHeaders::kOrigin,
+  // Match logic in network::cors::ShouldCheckCors.
-          request_.request_initiator->Serialize());
+  bool should_check_cors =
-    } else if (!HasCrossOriginWhitelistEntry(
+      is_cross_origin &&
-                   *request_.request_initiator,
+      request_.mode != network::mojom::RequestMode::kNavigate &&
+      request_.mode != network::mojom::RequestMode::kNoCors;
+
+  if (should_check_cors && !is_cors_enabled_scheme &&
+      !HasCrossOriginWhitelistEntry(*request_.request_initiator,
                                     url::Origin::Create(request_.url))) {
     // Fail requests if a CORS check is required and the scheme is not CORS
     // enabled. This matches the error condition that would be generated by
@@ -470,6 +491,48 @@ void InterceptedRequest::Restart() {
             network::mojom::CorsError::kCorsDisabledScheme)));
     return;
   }
+
+  // Maybe update |credentials_mode| for fetch requests.
+  if (request_.credentials_mode ==
+      network::mojom::CredentialsMode::kSameOrigin) {
+    // Match logic in devtools_url_loader_interceptor.cc
+    // InterceptionJob::FollowRedirect.
+    bool tainted_origin = false;
+    if (redirect_in_progress_ && request_.request_initiator &&
+        !url::IsSameOriginWith(request_.url, original_url_) &&
+        !request_.request_initiator->IsSameOriginWith(original_url_)) {
+      tainted_origin = true;
+    }
+
+    // Match logic in CorsURLLoader::StartNetworkRequest.
+    const auto response_tainting = CalculateResponseTainting(
+        should_check_cors, request_.mode, tainted_origin);
+    request_.credentials_mode =
+        network::cors::CalculateCredentialsFlag(request_.credentials_mode,
+                                                response_tainting)
+            ? network::mojom::CredentialsMode::kInclude
+            : network::mojom::CredentialsMode::kOmit;
+  }
+
+  const bool should_add_origin_header =
+      // Cross-origin requests that are not kNavigate nor kNoCors.
+      should_check_cors ||
+      // Same-origin requests except for GET and HEAD.
+      (!is_cross_origin &&
+       request_.method != net::HttpRequestHeaders::kGetMethod &&
+       request_.method != net::HttpRequestHeaders::kHeadMethod);
+
+  if (should_add_origin_header) {
+    // Match logic in navigation_request.cc AddAdditionalRequestHeaders.
+    url::Origin origin_header_value =
+        request_.request_initiator.value_or(url::Origin());
+    origin_header_value = content::Referrer::SanitizeOriginForRequest(
+        request_.url, origin_header_value,
+        blink::ReferrerUtils::NetToMojoReferrerPolicy(
+            request_.referrer_policy));
+
+    request_.headers.SetHeaderIfMissing(net::HttpRequestHeaders::kOrigin,
+                                        origin_header_value.Serialize());
   }
 
   const GURL original_url = request_.url;

libcef/common/app_manager.cc

@@ -39,7 +39,7 @@ CefAppManager::~CefAppManager() {
   g_manager = nullptr;
 }
 
-void CefAppManager::AddCustomScheme(CefSchemeInfo* scheme_info) {
+void CefAppManager::AddCustomScheme(const CefSchemeInfo* scheme_info) {
   DCHECK(!scheme_info_list_locked_);
   scheme_info_list_.push_back(*scheme_info);
 

libcef/common/app_manager.h

@@ -36,7 +36,7 @@ class CefAppManager {
   // (url/url_util.h) via ContentClient::AddAdditionalSchemes which calls
   // AddCustomScheme, and second with Blink (SchemeRegistry) via
   // ContentRendererClient::WebKitInitialized which calls GetCustomSchemes.
-  void AddCustomScheme(CefSchemeInfo* scheme_info);
+  void AddCustomScheme(const CefSchemeInfo* scheme_info);
   bool HasCustomScheme(const std::string& scheme_name);
 
   using SchemeInfoList = std::list<CefSchemeInfo>;

libcef/common/net/scheme_registration.cc

@@ -40,22 +40,26 @@ void AddInternalSchemes(content::ContentClient::Schemes* schemes) {
   // with Blink only.
   for (size_t i = 0; i < sizeof(internal_schemes) / sizeof(internal_schemes[0]);
        ++i) {
-    if (internal_schemes[i].is_standard) {
+    const auto& scheme = internal_schemes[i];
-      schemes->standard_schemes.push_back(internal_schemes[i].scheme_name);
+    if (scheme.is_standard) {
+      schemes->standard_schemes.push_back(scheme.scheme_name);
+      if (!scheme.is_local && !scheme.is_display_isolated) {
+        schemes->referrer_schemes.push_back(scheme.scheme_name);
       }
-    if (internal_schemes[i].is_local) {
-      schemes->local_schemes.push_back(internal_schemes[i].scheme_name);
     }
-    if (internal_schemes[i].is_secure) {
+    if (scheme.is_local) {
-      schemes->secure_schemes.push_back(internal_schemes[i].scheme_name);
+      schemes->local_schemes.push_back(scheme.scheme_name);
     }
-    if (internal_schemes[i].is_cors_enabled) {
+    if (scheme.is_secure) {
-      schemes->cors_enabled_schemes.push_back(internal_schemes[i].scheme_name);
+      schemes->secure_schemes.push_back(scheme.scheme_name);
     }
-    if (internal_schemes[i].is_csp_bypassing) {
+    if (scheme.is_cors_enabled) {
-      schemes->csp_bypassing_schemes.push_back(internal_schemes[i].scheme_name);
+      schemes->cors_enabled_schemes.push_back(scheme.scheme_name);
     }
-    CefAppManager::Get()->AddCustomScheme(&internal_schemes[i]);
+    if (scheme.is_csp_bypassing) {
+      schemes->csp_bypassing_schemes.push_back(scheme.scheme_name);
+    }
+    CefAppManager::Get()->AddCustomScheme(&scheme);
   }
 }
 

libcef/common/scheme_registrar_impl.cc

@@ -49,6 +49,9 @@ bool CefSchemeRegistrarImpl::AddCustomScheme(const CefString& scheme_name,
   // with Blink only.
   if (is_standard) {
     schemes_.standard_schemes.push_back(scheme);
+    if (!is_local && !is_display_isolated) {
+      schemes_.referrer_schemes.push_back(scheme);
+    }
   }
   if (is_local) {
     schemes_.local_schemes.push_back(scheme);
@@ -74,6 +77,7 @@ bool CefSchemeRegistrarImpl::AddCustomScheme(const CefString& scheme_name,
 void CefSchemeRegistrarImpl::GetSchemes(
     content::ContentClient::Schemes* schemes) {
   AppendArray(schemes_.standard_schemes, &schemes->standard_schemes);
+  AppendArray(schemes_.referrer_schemes, &schemes->referrer_schemes);
   AppendArray(schemes_.local_schemes, &schemes->local_schemes);
   AppendArray(schemes_.secure_schemes, &schemes->secure_schemes);
   AppendArray(schemes_.cors_enabled_schemes, &schemes->cors_enabled_schemes);

patch/patch.cfg

@@ -678,5 +678,11 @@ patches = [
     # https://chromium-review.googlesource.com/c/chromium/src/+/4829483
     # https://bugs.chromium.org/p/chromium/issues/detail?id=1470837#c22
     'name': 'rfh_navigation_4829483'
+  },
+  {
+    # Fix Referrer & Origin calculation for secure referrer (custom standard
+    # scheme) with insecure destination.
+    # https://github.com/chromiumembedded/cef/issues/3596
+    'name': 'net_url_request_3596'
   }
 ]

patch/patches/net_url_request_3596.patch

@@ -0,0 +1,39 @@
+diff --git net/url_request/url_request_job.cc net/url_request/url_request_job.cc
+index 0e585570d3fa6..7158d4e8df44e 100644
+--- net/url_request/url_request_job.cc
++++ net/url_request/url_request_job.cc
+@@ -34,6 +34,7 @@
+ #include "net/ssl/ssl_private_key.h"
+ #include "net/url_request/redirect_util.h"
+ #include "net/url_request/url_request_context.h"
++#include "url/url_util.h"
+ 
+ namespace net {
+ 
+@@ -46,6 +47,16 @@ base::Value::Dict SourceStreamSetParams(SourceStream* source_stream) {
+   return event_params;
+ }
+ 
++bool IsSecureScheme(const GURL& url) {
++  if (!url.has_scheme()) {
++    return false;
++  }
++  if (GURL::SchemeIsCryptographic(url.scheme_piece())) {
++    return true;
++  }
++  return base::Contains(url::GetSecureSchemes(), url.scheme_piece());
++}
++
+ }  // namespace
+ 
+ // Each SourceStreams own the previous SourceStream in the chain, but the
+@@ -334,8 +345,7 @@ GURL URLRequestJob::ComputeReferrerForPolicy(
+   }
+ 
+   bool secure_referrer_but_insecure_destination =
+-      original_referrer.SchemeIsCryptographic() &&
+-      !destination.SchemeIsCryptographic();
++      IsSecureScheme(original_referrer) && !IsSecureScheme(destination);
+ 
+   switch (policy) {
+     case ReferrerPolicy::CLEAR_ON_TRANSITION_FROM_SECURE_TO_INSECURE:

tests/ceftests/cors_unittest.cc

@@ -4,6 +4,7 @@
 
 #include <algorithm>
 #include <set>
+#include <sstream>
 #include <vector>
 
 #include "include/base/cef_callback.h"
@@ -67,6 +68,14 @@ enum class HandlerType {
 std::string GetOrigin(HandlerType handler) {
   switch (handler) {
     case HandlerType::SERVER:
+      // TODO: Only call test_server::GetOrigin() after test server
+      // initialization.
+      if (!kUseHttpsServerScheme) {
+        std::stringstream ss;
+        ss << "http://" << test_server::kHttpServerAddress << ":"
+           << test_server::kHttpServerPort;
+        return ss.str();
+      }
       return test_server::GetOrigin(kUseHttpsServerScheme);
     case HandlerType::HTTP_SCHEME:
       // Use HTTPS because requests from HTTP to the loopback address will be

tests/ceftests/server_unittest.cc

@@ -6,6 +6,7 @@
 #include <map>
 #include <memory>
 #include <set>
+#include <sstream>
 
 #include "include/base/cef_callback.h"
 #include "include/base/cef_ref_counted.h"