Enforce no mojom::FetchRequestMode::kNavigate from renderer processes.
Bug: 953315
Change-Id: I211289d620b62488d53ba3ccbe65e6805008fde5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1574144
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#654060}
diff --git a/services/network/cors/cors_url_loader_factory.cc b/services/network/cors/cors_url_loader_factory.cc
index 991ada8a..7b47b43 100644
--- a/services/network/cors/cors_url_loader_factory.cc
+++ b/services/network/cors/cors_url_loader_factory.cc
@@ -4,11 +4,16 @@
#include "services/network/cors/cors_url_loader_factory.h"
+#include <utility>
+
#include "base/bind.h"
#include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
+#include "mojo/public/cpp/bindings/message.h"
#include "net/base/load_flags.h"
#include "services/network/cors/cors_url_loader.h"
#include "services/network/cors/preflight_controller.h"
+#include "services/network/initiator_lock_compatibility.h"
#include "services/network/network_context.h"
#include "services/network/public/cpp/cors/cors.h"
#include "services/network/public/cpp/features.h"
@@ -31,9 +36,11 @@
: context_(context),
disable_web_security_(params->disable_web_security),
process_id_(params->process_id),
+ request_initiator_site_lock_(params->request_initiator_site_lock),
origin_access_list_(origin_access_list) {
DCHECK(context_);
DCHECK(origin_access_list_);
+ DCHECK_NE(mojom::kInvalidProcessId, process_id_);
factory_bound_origin_access_list_ = std::make_unique<OriginAccessList>();
if (params->factory_bound_allow_patterns.size()) {
DCHECK(params->request_initiator_site_lock);
@@ -129,6 +136,7 @@
request.fetch_request_mode != mojom::FetchRequestMode::kNoCors) {
LOG(WARNING) << "|fetch_request_mode| is " << request.fetch_request_mode
<< ", but |request_initiator| is not set.";
+ mojo::ReportBadMessage("CorsURLLoaderFactory: cors without initiator");
return false;
}
@@ -140,9 +148,53 @@
(request.load_flags & load_flags_pattern) != load_flags_pattern) {
LOG(WARNING) << "|fetch_credentials_mode| and |load_flags| contradict each "
"other.";
+ mojo::ReportBadMessage(
+ "CorsURLLoaderFactory: omit-credentials vs load_flags");
return false;
}
+ // Ensure that renderer requests are covered either by CORS or CORB.
+ if (process_id_ != mojom::kBrowserProcessId) {
+ switch (request.fetch_request_mode) {
+ case mojom::FetchRequestMode::kNavigate:
+ // Only the browser process can initiate navigations. This helps ensure
+ // that a malicious/compromised renderer cannot bypass CORB by issuing
+ // kNavigate, rather than kNoCors requests. (CORB should apply only to
+ // no-cors requests as tracked in https://crbug.com/953315 and as
+ // captured in https://fetch.spec.whatwg.org/#main-fetch).
+ mojo::ReportBadMessage(
+ "CorsURLLoaderFactory: navigate from non-browser-process");
+ return false;
+
+ case mojom::FetchRequestMode::kSameOrigin:
+ case mojom::FetchRequestMode::kCors:
+ case mojom::FetchRequestMode::kCorsWithForcedPreflight:
+ // SOP enforced by CORS.
+ break;
+
+ case mojom::FetchRequestMode::kNoCors:
+ // SOP enforced by CORB.
+ break;
+ }
+ }
+
+ InitiatorLockCompatibility initiator_lock_compatibility =
+ process_id_ == mojom::kBrowserProcessId
+ ? InitiatorLockCompatibility::kBrowserProcess
+ : VerifyRequestInitiatorLock(request_initiator_site_lock_,
+ request.request_initiator);
+ UMA_HISTOGRAM_ENUMERATION(
+ "NetworkService.URLLoader.RequestInitiatorOriginLockCompatibility",
+ initiator_lock_compatibility);
+ // TODO(lukasza): Enforce the origin lock.
+ // - https://crbug.com/766694: In the long-term kIncorrectLock should trigger
+ // a renderer kill, but this can't be done until HTML Imports are gone.
+ // - https://crbug.com/515309: The lock should apply to Origin header (and
+ // SameSite cookies) in addition to CORB (which was taken care of in
+ // https://crbug.com/871827). Here enforcement most likely would mean
+ // setting |url_request_|'s initiator to something other than
+ // |request.request_initiator| (opaque origin? lock origin?).
+
if (context) {
net::HttpRequestHeaders::Iterator header_iterator(
request.cors_exempt_headers);
diff --git a/services/network/cors/cors_url_loader_factory.h b/services/network/cors/cors_url_loader_factory.h
index 987241d..89616115 100644
--- a/services/network/cors/cors_url_loader_factory.h
+++ b/services/network/cors/cors_url_loader_factory.h
@@ -6,6 +6,7 @@
#define SERVICES_NETWORK_CORS_CORS_URL_LOADER_FACTORY_H_
#include <memory>
+#include <set>
#include "base/callback_forward.h"
#include "base/containers/unique_ptr_adapters.h"
@@ -67,8 +68,7 @@
void DeleteIfNeeded();
- static bool IsSane(const NetworkContext* context,
- const ResourceRequest& request);
+ bool IsSane(const NetworkContext* context, const ResourceRequest& request);
mojo::BindingSet<mojom::URLLoaderFactory> bindings_;
@@ -77,9 +77,10 @@
NetworkContext* const context_ = nullptr;
scoped_refptr<ResourceSchedulerClient> resource_scheduler_client_;
+ // Retained from URLLoaderFactoryParams:
const bool disable_web_security_;
-
- const uint32_t process_id_;
+ const uint32_t process_id_ = mojom::kInvalidProcessId;
+ const base::Optional<url::Origin> request_initiator_site_lock_;
// Relative order of |network_loader_factory_| and |loaders_| matters -
// URLLoaderFactory needs to live longer than URLLoaders created using the
diff --git a/services/network/cors/cors_url_loader_unittest.cc b/services/network/cors/cors_url_loader_unittest.cc
index e1b60cf..dec4d51 100644
--- a/services/network/cors/cors_url_loader_unittest.cc
+++ b/services/network/cors/cors_url_loader_unittest.cc
@@ -4,6 +4,8 @@
#include "services/network/cors/cors_url_loader.h"
+#include <memory>
+#include <string>
#include <utility>
#include <vector>
@@ -17,6 +19,8 @@
#include "base/strings/stringprintf.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/scoped_task_environment.h"
+#include "mojo/core/embedder/embedder.h"
+#include "mojo/public/cpp/bindings/message.h"
#include "net/base/load_flags.h"
#include "net/http/http_request_headers.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
@@ -34,6 +38,7 @@
#include "services/network/resource_scheduler.h"
#include "services/network/resource_scheduler_client.h"
#include "services/network/test/test_url_loader_client.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace network {
@@ -42,6 +47,8 @@
namespace {
+const uint32_t kRendererProcessId = 573;
+
class TestURLLoaderFactory : public mojom::URLLoaderFactory {
public:
TestURLLoaderFactory() : weak_factory_(this) {}
@@ -155,7 +162,7 @@
network_service_.get(), mojo::MakeRequest(&network_context_ptr_),
std::move(context_params));
- ResetFactory(base::nullopt);
+ ResetFactory(base::nullopt, kRendererProcessId);
}
void CreateLoaderAndStart(const GURL& origin,
@@ -215,10 +222,9 @@
void FollowRedirect() {
DCHECK(url_loader_);
- url_loader_->FollowRedirect({}, // removed_headers
- {}, // modified_headers
- base::nullopt // new_url
- );
+ url_loader_->FollowRedirect({}, // removed_headers
+ {}, // modified_headers
+ base::nullopt); // new_url
}
const ResourceRequest& GetRequest() const {
@@ -279,7 +285,7 @@
factory_bound_allow_patterns_.push_back(mojom::CorsOriginPattern::New(
protocol, domain, mode,
mojom::CorsOriginAccessMatchPriority::kDefaultPriority));
- ResetFactory(source_origin);
+ ResetFactory(source_origin, kRendererProcessId);
}
static net::RedirectInfo CreateRedirectInfo(
@@ -297,8 +303,8 @@
return redirect_info;
}
- private:
- void ResetFactory(base::Optional<url::Origin> initiator) {
+ void ResetFactory(base::Optional<url::Origin> initiator,
+ uint32_t process_id) {
std::unique_ptr<TestURLLoaderFactory> factory =
std::make_unique<TestURLLoaderFactory>();
test_url_loader_factory_ = factory->GetWeakPtr();
@@ -311,12 +317,12 @@
cloned_patterns.push_back(item.Clone());
factory_params->factory_bound_allow_patterns = std::move(cloned_patterns);
}
- constexpr int kProcessId = 573;
- factory_params->process_id = kProcessId;
+ factory_params->process_id = process_id;
+ factory_params->is_corb_enabled = (process_id != mojom::kBrowserProcessId);
constexpr int kRouteId = 765;
auto resource_scheduler_client =
base::MakeRefCounted<ResourceSchedulerClient>(
- kProcessId, kRouteId, &resource_scheduler_,
+ process_id, kRouteId, &resource_scheduler_,
url_request_context_->network_quality_estimator());
cors_url_loader_factory_ = std::make_unique<CorsURLLoaderFactory>(
network_context_.get(), std::move(factory_params),
@@ -325,6 +331,7 @@
std::move(factory));
}
+ private:
// Testing instance to enable kOutOfBlinkCors feature.
base::test::ScopedFeatureList feature_list_;
@@ -358,7 +365,38 @@
DISALLOW_COPY_AND_ASSIGN(CorsURLLoaderTest);
};
-TEST_F(CorsURLLoaderTest, SameOriginWithoutInitiator) {
+class CorsURLLoaderBadMessageTest : public CorsURLLoaderTest {
+ public:
+ CorsURLLoaderBadMessageTest()
+ : dummy_message_(0, 0, 0, 0, nullptr), context_(&dummy_message_) {
+ mojo::core::SetDefaultProcessErrorCallback(base::BindRepeating(
+ &CorsURLLoaderBadMessageTest::OnBadMessage, base::Unretained(this)));
+ }
+
+ ~CorsURLLoaderBadMessageTest() override {
+ mojo::core::SetDefaultProcessErrorCallback(
+ mojo::core::ProcessErrorCallback());
+ }
+
+ protected:
+ const std::vector<std::string>& bad_message_reports() const {
+ return bad_message_reports_;
+ }
+
+ private:
+ void OnBadMessage(const std::string& reason) {
+ bad_message_reports_.push_back(reason);
+ }
+
+ std::vector<std::string> bad_message_reports_;
+
+ mojo::Message dummy_message_;
+ mojo::internal::MessageDispatchContext context_;
+
+ DISALLOW_COPY_AND_ASSIGN(CorsURLLoaderBadMessageTest);
+};
+
+TEST_F(CorsURLLoaderBadMessageTest, SameOriginWithoutInitiator) {
ResourceRequest request;
request.fetch_request_mode = mojom::FetchRequestMode::kSameOrigin;
request.fetch_credentials_mode = mojom::FetchCredentialsMode::kInclude;
@@ -373,6 +411,9 @@
EXPECT_FALSE(client().has_received_response());
EXPECT_TRUE(client().has_received_completion());
EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code);
+ EXPECT_THAT(
+ bad_message_reports(),
+ ::testing::ElementsAre("CorsURLLoaderFactory: cors without initiator"));
}
TEST_F(CorsURLLoaderTest, NoCorsWithoutInitiator) {
@@ -394,7 +435,7 @@
EXPECT_EQ(net::OK, client().completion_status().error_code);
}
-TEST_F(CorsURLLoaderTest, CorsWithoutInitiator) {
+TEST_F(CorsURLLoaderBadMessageTest, CorsWithoutInitiator) {
ResourceRequest request;
request.fetch_request_mode = mojom::FetchRequestMode::kCors;
request.fetch_credentials_mode = mojom::FetchCredentialsMode::kInclude;
@@ -409,9 +450,14 @@
EXPECT_FALSE(client().has_received_response());
EXPECT_TRUE(client().has_received_completion());
EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code);
+ EXPECT_THAT(
+ bad_message_reports(),
+ ::testing::ElementsAre("CorsURLLoaderFactory: cors without initiator"));
}
-TEST_F(CorsURLLoaderTest, NavigateWithoutInitiator) {
+TEST_F(CorsURLLoaderBadMessageTest, NavigateWithoutInitiator) {
+ ResetFactory(base::nullopt /* initiator */, mojom::kBrowserProcessId);
+
ResourceRequest request;
request.fetch_request_mode = mojom::FetchRequestMode::kNavigate;
request.fetch_credentials_mode = mojom::FetchCredentialsMode::kInclude;
@@ -430,7 +476,8 @@
EXPECT_EQ(net::OK, client().completion_status().error_code);
}
-TEST_F(CorsURLLoaderTest, CredentialsModeAndLoadFlagsContradictEachOther1) {
+TEST_F(CorsURLLoaderBadMessageTest,
+ CredentialsModeAndLoadFlagsContradictEachOther1) {
ResourceRequest request;
request.fetch_request_mode = mojom::FetchRequestMode::kNavigate;
request.fetch_credentials_mode = mojom::FetchCredentialsMode::kOmit;
@@ -447,9 +494,13 @@
EXPECT_FALSE(client().has_received_response());
EXPECT_TRUE(client().has_received_completion());
EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code);
+ EXPECT_THAT(bad_message_reports(),
+ ::testing::ElementsAre(
+ "CorsURLLoaderFactory: omit-credentials vs load_flags"));
}
-TEST_F(CorsURLLoaderTest, CredentialsModeAndLoadFlagsContradictEachOther2) {
+TEST_F(CorsURLLoaderBadMessageTest,
+ CredentialsModeAndLoadFlagsContradictEachOther2) {
ResourceRequest request;
request.fetch_request_mode = mojom::FetchRequestMode::kNavigate;
request.fetch_credentials_mode = mojom::FetchCredentialsMode::kOmit;
@@ -466,9 +517,13 @@
EXPECT_FALSE(client().has_received_response());
EXPECT_TRUE(client().has_received_completion());
EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code);
+ EXPECT_THAT(bad_message_reports(),
+ ::testing::ElementsAre(
+ "CorsURLLoaderFactory: omit-credentials vs load_flags"));
}
-TEST_F(CorsURLLoaderTest, CredentialsModeAndLoadFlagsContradictEachOther3) {
+TEST_F(CorsURLLoaderBadMessageTest,
+ CredentialsModeAndLoadFlagsContradictEachOther3) {
ResourceRequest request;
request.fetch_request_mode = mojom::FetchRequestMode::kNavigate;
request.fetch_credentials_mode = mojom::FetchCredentialsMode::kOmit;
@@ -485,6 +540,28 @@
EXPECT_FALSE(client().has_received_response());
EXPECT_TRUE(client().has_received_completion());
EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code);
+ EXPECT_THAT(bad_message_reports(),
+ ::testing::ElementsAre(
+ "CorsURLLoaderFactory: omit-credentials vs load_flags"));
+}
+
+TEST_F(CorsURLLoaderBadMessageTest, NavigationFromRenderer) {
+ ResourceRequest request;
+ request.fetch_request_mode = mojom::FetchRequestMode::kNavigate;
+ request.url = GURL("http://example.com/");
+ request.request_initiator = base::nullopt;
+
+ CreateLoaderAndStart(request);
+ RunUntilComplete();
+
+ EXPECT_FALSE(IsNetworkLoaderStarted());
+ EXPECT_FALSE(client().has_received_redirect());
+ EXPECT_FALSE(client().has_received_response());
+ EXPECT_TRUE(client().has_received_completion());
+ EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code);
+ EXPECT_THAT(bad_message_reports(),
+ ::testing::ElementsAre(
+ "CorsURLLoaderFactory: navigate from non-browser-process"));
}
TEST_F(CorsURLLoaderTest, SameOriginRequest) {
diff --git a/services/network/initiator_lock_compatibility.cc b/services/network/initiator_lock_compatibility.cc
index 1fcec799..76455f7 100644
--- a/services/network/initiator_lock_compatibility.cc
+++ b/services/network/initiator_lock_compatibility.cc
@@ -4,6 +4,8 @@
#include "services/network/initiator_lock_compatibility.h"
+#include <string>
+
#include "base/feature_list.h"
#include "base/logging.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
@@ -58,16 +60,6 @@
return InitiatorLockCompatibility::kIncorrectLock;
}
-InitiatorLockCompatibility VerifyRequestInitiatorLock(
- const mojom::URLLoaderFactoryParams& factory_params,
- const ResourceRequest& request) {
- if (factory_params.process_id == mojom::kBrowserProcessId)
- return InitiatorLockCompatibility::kBrowserProcess;
-
- return VerifyRequestInitiatorLock(factory_params.request_initiator_site_lock,
- request.request_initiator);
-}
-
url::Origin GetTrustworthyInitiator(
const base::Optional<url::Origin>& request_initiator_site_lock,
const net::URLRequest& request) {
diff --git a/services/network/initiator_lock_compatibility.h b/services/network/initiator_lock_compatibility.h
index d0d7ff8..edb3132 100644
--- a/services/network/initiator_lock_compatibility.h
+++ b/services/network/initiator_lock_compatibility.h
@@ -15,8 +15,6 @@
namespace network {
-struct ResourceRequest;
-
namespace mojom {
class URLLoaderFactoryParams;
} // namespace mojom
@@ -56,10 +54,6 @@
// |factory_params.request_initiator_site_lock|.
COMPONENT_EXPORT(NETWORK_SERVICE)
InitiatorLockCompatibility VerifyRequestInitiatorLock(
- const mojom::URLLoaderFactoryParams& factory_params,
- const ResourceRequest& request);
-COMPONENT_EXPORT(NETWORK_SERVICE)
-InitiatorLockCompatibility VerifyRequestInitiatorLock(
const base::Optional<url::Origin>& request_initiator_site_lock,
const base::Optional<url::Origin>& request_initiator);
diff --git a/services/network/initiator_lock_compatibility_unittest.cc b/services/network/initiator_lock_compatibility_unittest.cc
index 753e434..3c9c4cb 100644
--- a/services/network/initiator_lock_compatibility_unittest.cc
+++ b/services/network/initiator_lock_compatibility_unittest.cc
@@ -12,20 +12,6 @@
namespace network {
-InitiatorLockCompatibility VerifyRequestInitiatorSiteLock(
- int process_id,
- base::Optional<url::Origin> lock,
- base::Optional<url::Origin> initiator) {
- auto factory_params = mojom::URLLoaderFactoryParams::New();
- factory_params->process_id = process_id;
- factory_params->request_initiator_site_lock = lock;
-
- ResourceRequest request;
- request.request_initiator = initiator;
-
- return VerifyRequestInitiatorLock(*factory_params, request);
-}
-
TEST(InitiatorLockCompatibilityTest, VerifyRequestInitiatorSiteLock) {
url::Origin opaque_origin = url::Origin();
url::Origin opaque_origin2 = url::Origin();
@@ -44,51 +30,39 @@
url::Origin::Create(GURL("http://bar.foo.example.com"));
url::Origin other_site = url::Origin::Create(GURL("http://other.com"));
- constexpr int kRendererProcessId = 123;
// Cases without a lock.
- EXPECT_EQ(InitiatorLockCompatibility::kBrowserProcess,
- VerifyRequestInitiatorSiteLock(mojom::kBrowserProcessId,
- base::nullopt, base::nullopt));
EXPECT_EQ(InitiatorLockCompatibility::kNoLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, base::nullopt,
- base::nullopt));
+ VerifyRequestInitiatorLock(base::nullopt, base::nullopt));
// Opaque initiator is always safe (and so results in kCompatibleLock).
// OTOH, opaque lock is only compatible with an opaque initiator.
EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId,
- bar_foo_example_com, opaque_origin));
+ VerifyRequestInitiatorLock(bar_foo_example_com, opaque_origin));
EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, opaque_origin,
- opaque_origin2));
+ VerifyRequestInitiatorLock(opaque_origin, opaque_origin2));
EXPECT_EQ(InitiatorLockCompatibility::kIncorrectLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, opaque_origin,
- bar_foo_example_com));
+ VerifyRequestInitiatorLock(opaque_origin, bar_foo_example_com));
// Regular origin equality.
- EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(
- kRendererProcessId, bar_foo_example_com, bar_foo_example_com));
+ EXPECT_EQ(
+ InitiatorLockCompatibility::kCompatibleLock,
+ VerifyRequestInitiatorLock(bar_foo_example_com, bar_foo_example_com));
// Regular origin inequality.
EXPECT_EQ(InitiatorLockCompatibility::kIncorrectLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId,
- bar_foo_example_com, other_site));
+ VerifyRequestInitiatorLock(bar_foo_example_com, other_site));
// IP addresses have to be special-cased in some places (e.g. they shouldn't
// be subject to DomainIs / eTLD+1 comparisons).
EXPECT_EQ(InitiatorLockCompatibility::kIncorrectLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, ip_origin1,
- ip_origin2));
+ VerifyRequestInitiatorLock(ip_origin1, ip_origin2));
EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, ip_origin1,
- ip_origin1));
+ VerifyRequestInitiatorLock(ip_origin1, ip_origin1));
// Compatibility check shouldn't strip the lock down to eTLD+1.
EXPECT_EQ(InitiatorLockCompatibility::kIncorrectLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, foo_example_com,
- bar_example_com));
+ VerifyRequestInitiatorLock(foo_example_com, bar_example_com));
// Site-URL-based comparisons.
//
@@ -96,20 +70,16 @@
// request_initiator_site_lock becomes request_initiator_origin_lock - see
// https://crbug.com/888079 and https://crbug.com/891872.
EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, example_com,
- bar_foo_example_com));
+ VerifyRequestInitiatorLock(example_com, bar_foo_example_com));
EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, foo_example_com,
- bar_foo_example_com));
+ VerifyRequestInitiatorLock(foo_example_com, bar_foo_example_com));
// The trailing dot is not important (at least for site-URL-based
// comparisons).
EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(
- kRendererProcessId, foo_example_com_dot, foo_example_com));
+ VerifyRequestInitiatorLock(foo_example_com_dot, foo_example_com));
EXPECT_EQ(InitiatorLockCompatibility::kCompatibleLock,
- VerifyRequestInitiatorSiteLock(kRendererProcessId, foo_example_com,
- foo_example_com_dot));
+ VerifyRequestInitiatorLock(foo_example_com, foo_example_com_dot));
}
} // namespace network
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
index 21bfa6dd..6f11791 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -347,6 +347,7 @@
fetch_window_id_(request.fetch_window_id),
weak_ptr_factory_(this) {
DCHECK(delete_callback_);
+ DCHECK(factory_params_);
if (!base::FeatureList::IsEnabled(features::kNetworkService)) {
CHECK(!url_loader_client_.internal_state()
->handle()
@@ -402,17 +403,6 @@
throttling_token_ = network::ScopedThrottlingToken::MaybeCreate(
url_request_->net_log().source().id, request.throttling_profile_id);
- UMA_HISTOGRAM_ENUMERATION(
- "NetworkService.URLLoader.RequestInitiatorOriginLockCompatibility",
- VerifyRequestInitiatorLock(*factory_params_, request));
- // TODO(lukasza): Enforce the origin lock.
- // - https://crbug.com/766694: In the long-term kIncorrectLock should trigger
- // a renderer kill, but this can't be done until HTML Imports are gone.
- // - https://crbug.com/515309: The lock should apply to Origin header (and
- // SameSite cookies) in addition to CORB (which was taken care of in
- // https://crbug.com/871827). Here enforcement most likely would mean
- // setting |url_request_|'s initiator to something other than
- // |request.request_initiator| (opaque origin? lock origin?).
url_request_->set_initiator(request.request_initiator);
if (request.update_first_party_url_on_redirect) {
diff --git a/third_party/blink/renderer/core/fetch/fetch_manager.cc b/third_party/blink/renderer/core/fetch/fetch_manager.cc
index 030bce2..fd601158 100644
--- a/third_party/blink/renderer/core/fetch/fetch_manager.cc
+++ b/third_party/blink/renderer/core/fetch/fetch_manager.cc
@@ -692,8 +692,10 @@
request.SetFetchRequestMode(fetch_request_data_->Mode());
break;
case FetchRequestMode::kNavigate:
- // Using kSameOrigin here to reduce the security risk.
- // "navigate" request is only available in ServiceWorker.
+ // NetworkService (i.e. CorsURLLoaderFactory::IsSane) rejects kNavigate
+ // requests coming from renderers, so using kSameOrigin here.
+ // TODO(lukasza): Tweak CorsURLLoaderFactory::IsSane to accept kNavigate
+ // if request_initiator and the target are same-origin.
request.SetFetchRequestMode(FetchRequestMode::kSameOrigin);
break;
}
