Commit 9d6f483b authored by Eriko Kurimoto's avatar Eriko Kurimoto Committed by Commit Bot

Worker: Create inside settings security origin from constructor origin

This CL changes how to create security origin in inside settings.
Previously, the security origin was created from request script url.
After this CL, it will be created from constructor origin if script url
is not data url, and set to an unique opaque origin if it is data url.

This CL focus on the behavior clearly defined in HTML spec. As for
chrome-extension who is not included in the spec, we still need further
discussion before deciding its behavior, so we keep the previous
behavior in this CL.

See the doc for more information:
https://docs.google.com/document/d/1Vfrvow1PO2OBr_3yI08aHYBl_YW1fQ87enqiHrdlqeA/edit?usp=sharing

Bug: 1053390
Change-Id: I64a2b63979e3ba2bb59673018b7cb89c0ef498f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2060224
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749704}
parent 9eff4ddd
......@@ -10,7 +10,9 @@
#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
#include "third_party/blink/renderer/core/workers/worker_global_scope.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
#include "third_party/blink/renderer/platform/loader/fetch/fetch_client_settings_object.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher_properties.h"
#include "third_party/blink/renderer/platform/network/content_security_policy_response_headers.h"
#include "third_party/blink/renderer/platform/network/http_names.h"
#include "third_party/blink/renderer/platform/network/network_utils.h"
......@@ -30,6 +32,8 @@ void WorkerModuleScriptFetcher::Fetch(
ModuleGraphLevel level,
ModuleScriptFetcher::Client* client) {
DCHECK(global_scope_->IsContextThread());
DCHECK(!fetch_client_settings_object_fetcher_);
fetch_client_settings_object_fetcher_ = fetch_client_settings_object_fetcher;
client_ = client;
level_ = level;
......@@ -51,6 +55,7 @@ void WorkerModuleScriptFetcher::Trace(Visitor* visitor) {
ModuleScriptFetcher::Trace(visitor);
visitor->Trace(client_);
visitor->Trace(global_scope_);
visitor->Trace(fetch_client_settings_object_fetcher_);
}
// https://html.spec.whatwg.org/C/#worker-processing-model
......@@ -71,16 +76,34 @@ void WorkerModuleScriptFetcher::NotifyFinished(Resource* resource) {
// TODO(nhiroki, hiroshige): Access to WorkerGlobalScope in module loaders
// is a layering violation. Also, updating WorkerGlobalScope ('module map
// settigns object') in flight can be dangerous because module loaders may
// refers to it. We should move these steps out of core/loader/modulescript/
// refer to it. We should move these steps out of core/loader/modulescript/
// and run them after module loading. This may require the spec change.
// (https://crbug.com/845285)
// Ensure redirects don't affect SecurityOrigin.
const KURL request_url = resource->Url();
const KURL response_url = resource->GetResponse().CurrentRequestUrl();
if (request_url != response_url &&
!global_scope_->GetSecurityOrigin()->IsSameOriginWith(
SecurityOrigin::Create(response_url).get())) {
DCHECK(fetch_client_settings_object_fetcher_->GetProperties()
.GetFetchClientSettingsObject()
.GetSecurityOrigin()
->CanReadContent(request_url))
<< "Top-level worker script request url must be same-origin with "
"outside settings constructor origin or permitted by the parent "
"chrome-extension.";
// |response_url| must be same-origin with request origin or its url's
// scheme must be "data".
//
// https://fetch.spec.whatwg.org/#concept-main-fetch
// Step 5:
// - request’s current URL’s origin is same origin with request’s
// origin (request's current URL indicates |response_url|)
// - request’s current URL’s scheme is "data"
// ---> Return the result of performing a scheme fetch using request.
// - request’s mode is "same-origin"
// ---> Return a network error. [spec text]
if (!SecurityOrigin::AreSameOrigin(request_url, response_url) &&
!response_url.ProtocolIsData()) {
error_messages.push_back(MakeGarbageCollected<ConsoleMessage>(
mojom::ConsoleMessageSource::kSecurity,
mojom::ConsoleMessageLevel::kError,
......
......@@ -40,6 +40,7 @@ class CORE_EXPORT WorkerModuleScriptFetcher final
const Member<WorkerGlobalScope> global_scope_;
Member<ResourceFetcher> fetch_client_settings_object_fetcher_;
Member<Client> client_;
ModuleGraphLevel level_;
};
......
......@@ -86,9 +86,56 @@ void RemoveURLFromMemoryCacheInternal(const KURL& url) {
}
scoped_refptr<SecurityOrigin> CreateSecurityOrigin(
GlobalScopeCreationParams* creation_params) {
scoped_refptr<SecurityOrigin> security_origin =
SecurityOrigin::Create(creation_params->script_url);
GlobalScopeCreationParams* creation_params,
ExecutionContext* execution_context) {
// A worker environment settings object's origin must be set as follows:
//
// - DedicatedWorkers and SharedWorkers
// https://html.spec.whatwg.org/C/#set-up-a-worker-environment-settings-object
// Step 2: Let inherited origin be outside settings's origin.
// Step 6: Let settings object be a new environment settings object whose
// algorithms are defined as follows:
// The origin -> Return a unique opaque origin if worker global scope's url's
// scheme is "data", and inherited origin otherwise. [spec text]
//
// - ServiceWorkers
// https://w3c.github.io/ServiceWorker/#run-service-worker-algorithm
// Step 7.4: Let settingsObject be a new environment settings object whose
// algorithms are defined as follows:
// The origin -> Return its registering service worker client's origin.
// [spec text]
//
// The algorithm in ServiceWorkers differ from DedicatedWorkers and
// SharedWorkers when worker global scope's url's scheme is "data", but
// "data" url script is not allowed for ServiceWorkers, so all workers' origin
// can be calculated in the same way.
// https://w3c.github.io/ServiceWorker/#start-register
// Step 3: If scriptURL’s scheme is not one of "http" and "https", reject
// promise with a TypeError and abort these steps. [spec text]
DCHECK(!execution_context->IsServiceWorkerGlobalScope() ||
!KURL(creation_params->script_url).ProtocolIsData());
// TODO(https://crbug.com/1058305) Inherit |agent_cluster_id_| for dedicated
// workers. DO NOT inherit for shared workers and service workers.
//
// Create a new SecurityOrigin via CreateFromUrlOrigin() so that worker's
// origin can avoid inheriting unnecessary capabilities from the starter
// origin, while the worker's origin inherits url:Origin's internal nonce.
scoped_refptr<SecurityOrigin> security_origin;
if (KURL(creation_params->script_url).ProtocolIsData()) {
security_origin = SecurityOrigin::CreateUniqueOpaque();
} else if (creation_params->starter_origin->Protocol() ==
"chrome-extension") {
// TODO(https://crbug.com/1059218) Whether the origin should be inherited
// for chrome-extension frame is under discussion. For now, the worker
// doesn't inherit the parent origin in this case to keep the previous
// behavior.
security_origin = SecurityOrigin::Create(creation_params->script_url);
} else {
security_origin = SecurityOrigin::CreateFromUrlOrigin(
creation_params->starter_origin->ToUrlOrigin());
}
if (creation_params->starter_origin) {
security_origin->TransferPrivilegesFrom(
creation_params->starter_origin->CreatePrivilegeData());
......@@ -426,7 +473,7 @@ WorkerGlobalScope::WorkerGlobalScope(
base::TimeTicks time_origin)
: WorkerOrWorkletGlobalScope(
thread->GetIsolate(),
CreateSecurityOrigin(creation_params.get()),
CreateSecurityOrigin(creation_params.get(), GetExecutionContext()),
MakeGarbageCollected<Agent>(
thread->GetIsolate(),
(creation_params->agent_cluster_id.is_empty()
......@@ -496,8 +543,7 @@ NOINLINE void WorkerGlobalScope::InitializeURL(const KURL& url) {
if (GetSecurityOrigin()->IsOpaque()) {
DCHECK(SecurityOrigin::Create(url)->IsOpaque());
} else {
DCHECK(GetSecurityOrigin()->IsSameOriginWith(
SecurityOrigin::Create(url).get()));
DCHECK(GetSecurityOrigin()->CanReadContent(url));
}
url_ = url;
}
......
......@@ -2,8 +2,8 @@ This is a testharness.js-based test.
PASS Worker has an opaque origin.
PASS Worker can read its own blobs.
PASS Worker can read its owners blobs.
FAIL Worker can XHR fetch a blob. assert_equals: expected "from page" but got ""
FAIL Worker can fetch a blob. promise_test: Unhandled rejection with value: object "TypeError: Failed to fetch"
PASS Worker can XHR fetch a blob.
PASS Worker can fetch a blob.
FAIL Worker can access BroadcastChannel Failed to construct 'BroadcastChannel': Can't create BroadcastChannel in an opaque origin
Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment