Skip to content

Commit

Permalink
Worker: Apply origin trial tokens after off-the-main-thread shared/se…
Browse files Browse the repository at this point in the history
…rvice worker script fetch

> TL;DR

Inside origin trial tokens
- Dedicated Workers:
  - On-the-main-thread:  no change (inherit outside tokens)
  - Off-the-main-thread: no change (inherit outside tokens)
- Shared/Service Workers:
  - On-the-main-thread:  no change (use response's tokens)
  - Off-the-main-thread: change (no tokens => use response's tokens)

Outside origin trial tokens used for script fetch
- Dedicated Workers:
  - On-the-main-thread:  no change (use outside tokens)
  - Off-the-main-thread: no change (use outsied tokens)
- Shared/Service Workers:
  - On-the-main-thread:  no change (no tokens)
  - Off-the-main-thread: no change (no tokens)

> Details

For origin trial tokens, expected behavior is as follows:

- Dedicated Workers inherit outside's origin trial tokens. On the other hand,
  Shared Workers and Service Workers use their own origin trial tokens served
  via script response's header. This behavior was described in the CL that
  implemented the mechanism (https://codereview.chromium.org/1828063002).
- Installed Service Workers use origin trial tokens stored in the service worker
  script storage.

Currently, on-the-main-thread worker script fetch implements this behavior, but
off-the-main-thread workers script fetch other than dedicated workers completely
ignores the origin trial tokens.

To fix it, this CL moves OriginTrialContext::AddTokens() calls from
WorkerGlobalScope constructor to WorkerGlobalScope::Initialize() except for
DedicatedWorkerGlobalScope, and makes sure the origin trial tokens are applied
in any code paths. DedicatedWorkerGlobalScope still applies the origin trial
tokens to itself in its constructor (see below for the reason).

This affects the existing behavior as follows:

- For on-the-main-thread worker script fetch, this doesn't change the existing
  expected behavior because Initialize() is called immediately after the
  constructor, and OriginTrialContext is not used during the period.
- For off-the-main-thread shared/service worker script fetch including module
  workers, this changes the existing behavior. Before this CL, they don't apply
  origin trial tokens, and some tests fail. After this CL, they apply the tokens
  in Initialize() after script fetch, and the tests pass.
- For off-the-main-thread dedicated worker script fetch including module
  workers, this doesn't change the existing behavior because dedicated workers
  still apply the origin trial tokens in the constructor.
- During off-the-main-thread top-level shared/service worker script fetch,
  origin trial tokens are not available. This is aligned with the existing
  behavior, but probably we might want to make fetch client's origin trial
  tokens available during the period. This needs more discussion, and it is out
  of the scope of this CL. On the other hand, off-the-main-thread top-level
  dedicated worker script fetch can use the fetch client's origin trial tokens
  because Dedicated Workers applies the outside's origin trial tokens in the
  constructor. This is also aligned with the existing behavior.

Design doc:
https://docs.google.com/document/d/1JCv8TD2nPLNC2iRCp_D1OM4I3uTS0HoEobuTymaMqgw/edit?usp=sharing

Bug: 945215
Change-Id: I619baf0e52f8b6cc89f696cf9bb2f2b5e234d545
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1599756
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659845}
  • Loading branch information
nhiroki authored and Commit Bot committed May 15, 2019
1 parent 8785a2d commit 85b70f1
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ void InstalledServiceWorkerModuleScriptFetcher::Fetch(

global_scope_->Initialize(response_url, response_referrer_policy,
script_data->GetResponseAddressSpace(),
response_content_security_policy->Headers());
response_content_security_policy->Headers(),
script_data->CreateOriginTrialTokens().get());
}

ModuleScriptCreationParams params(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "services/network/public/mojom/referrer_policy.mojom-shared.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
#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/loader/fetch/resource_fetcher.h"
#include "third_party/blink/renderer/platform/network/content_security_policy_response_headers.h"
Expand Down Expand Up @@ -113,10 +114,15 @@ void WorkerModuleScriptFetcher::NotifyFinished(Resource* resource) {
response_content_security_policy->DidReceiveHeaders(
ContentSecurityPolicyResponseHeaders(resource->GetResponse()));

std::unique_ptr<Vector<String>> response_origin_trial_tokens =
OriginTrialContext::ParseHeaderValue(
resource->GetResponse().HttpHeaderField(http_names::kOriginTrial));

// Step 12.3-12.6 are implemented in Initialize().
global_scope_->Initialize(response_url, response_referrer_policy,
response_address_space,
response_content_security_policy->Headers());
response_content_security_policy->Headers(),
response_origin_trial_tokens.get());
}

ModuleScriptCreationParams params(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,16 @@ DedicatedWorkerGlobalScope* DedicatedWorkerGlobalScope::Create(
std::unique_ptr<GlobalScopeCreationParams> creation_params,
DedicatedWorkerThread* thread,
base::TimeTicks time_origin) {
std::unique_ptr<Vector<String>> outside_origin_trial_tokens =
std::move(creation_params->origin_trial_tokens);

// Off-the-main-thread worker script fetch:
// Initialize() is called after script fetch.
if (creation_params->off_main_thread_fetch_option ==
OffMainThreadWorkerScriptFetchOption::kEnabled) {
return MakeGarbageCollected<DedicatedWorkerGlobalScope>(
std::move(creation_params), thread, time_origin);
std::move(creation_params), thread, time_origin,
std::move(outside_origin_trial_tokens));
}

// Legacy on-the-main-thread worker script fetch (to be removed):
Expand All @@ -76,19 +80,27 @@ DedicatedWorkerGlobalScope* DedicatedWorkerGlobalScope::Create(
mojom::IPAddressSpace response_address_space =
*creation_params->response_address_space;
auto* global_scope = MakeGarbageCollected<DedicatedWorkerGlobalScope>(
std::move(creation_params), thread, time_origin);
// A dummy CSP headers is passed here as it is superseded by outside's CSP
// headers in Initialize().
std::move(creation_params), thread, time_origin,
std::move(outside_origin_trial_tokens));
// Pass dummy CSP headers here as it is superseded by outside's CSP headers in
// Initialize().
// Pass dummy origin trial tokens here as it is already set to outside's
// origin trial tokens in DedicatedWorkerGlobalScope's constructor.
global_scope->Initialize(response_script_url, response_referrer_policy,
response_address_space, Vector<CSPHeaderAndType>());
response_address_space, Vector<CSPHeaderAndType>(),
nullptr /* response_origin_trial_tokens */);
return global_scope;
}

DedicatedWorkerGlobalScope::DedicatedWorkerGlobalScope(
std::unique_ptr<GlobalScopeCreationParams> creation_params,
DedicatedWorkerThread* thread,
base::TimeTicks time_origin)
: WorkerGlobalScope(std::move(creation_params), thread, time_origin) {}
base::TimeTicks time_origin,
std::unique_ptr<Vector<String>> outside_origin_trial_tokens)
: WorkerGlobalScope(std::move(creation_params), thread, time_origin) {
// Inherit the outside's origin trial tokens.
OriginTrialContext::AddTokens(this, outside_origin_trial_tokens.get());
}

DedicatedWorkerGlobalScope::~DedicatedWorkerGlobalScope() = default;

Expand All @@ -101,7 +113,8 @@ void DedicatedWorkerGlobalScope::Initialize(
const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& /* not used */) {
const Vector<CSPHeaderAndType>& /* response_csp_headers */,
const Vector<String>* /* response_origin_trial_tokens */) {
// Step 12.3. "Set worker global scope's url to response's url."
InitializeURL(response_url);

Expand All @@ -118,11 +131,15 @@ void DedicatedWorkerGlobalScope::Initialize(

// Step 12.6. "Execute the Initialize a global object's CSP list algorithm
// on worker global scope and response. [CSP]"
// DedicatedWorkerGlobalScope inherits the outside's CSP instead of the passed
// CSP. These should be called after SetAddressSpace() to correctly override
// the address space by the "treat-as-public-address" CSP directive.
// DedicatedWorkerGlobalScope inherits the outside's CSP instead of the
// response CSP headers. These should be called after SetAddressSpace() to
// correctly override the address space by the "treat-as-public-address" CSP
// directive.
InitContentSecurityPolicyFromVector(OutsideContentSecurityPolicyHeaders());
BindContentSecurityPolicyToExecutionContext();

// DedicatedWorkerGlobalScope inherits the outside's OriginTrialTokens in the
// constructor instead of the response origin trial tokens.
}

// https://html.spec.whatwg.org/C/#worker-processing-model
Expand Down Expand Up @@ -260,11 +277,14 @@ void DedicatedWorkerGlobalScope::DidFetchClassicScript(
}

// Step 12.3-12.6 are implemented in Initialize().
// A dummy CSP headers is passed here as it is superseded by outside's CSP
// headers in Initialize().
// Pass dummy CSP headers here as it is superseded by outside's CSP headers in
// Initialize().
// Pass dummy origin trial tokens here as it is already set to outside's
// origin trial tokens in DedicatedWorkerGlobalScope's constructor.
Initialize(classic_script_loader->ResponseURL(), response_referrer_policy,
classic_script_loader->ResponseAddressSpace(),
Vector<CSPHeaderAndType>());
Vector<CSPHeaderAndType>(),
nullptr /* response_origin_trial_tokens */);

// Step 12.7. "Asynchronously complete the perform the fetch steps with
// response."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ class CORE_EXPORT DedicatedWorkerGlobalScope final : public WorkerGlobalScope {

// Do not call this. Use Create() instead. This is public only for
// MakeGarbageCollected.
DedicatedWorkerGlobalScope(std::unique_ptr<GlobalScopeCreationParams>,
DedicatedWorkerThread*,
base::TimeTicks time_origin);
DedicatedWorkerGlobalScope(
std::unique_ptr<GlobalScopeCreationParams>,
DedicatedWorkerThread*,
base::TimeTicks time_origin,
std::unique_ptr<Vector<String>> outside_origin_trial_tokens);

~DedicatedWorkerGlobalScope() override;

Expand All @@ -74,11 +76,11 @@ class CORE_EXPORT DedicatedWorkerGlobalScope final : public WorkerGlobalScope {
const AtomicString& InterfaceName() const override;

// Implements WorkerGlobalScope.
void Initialize(
const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers) override;
void Initialize(const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers,
const Vector<String>* response_origin_trial_tokens) override;
void FetchAndRunClassicScript(
const KURL& script_url,
const FetchClientSettingsObjectSnapshot& outside_settings_object,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "third_party/blink/renderer/core/workers/experimental/thread_pool_thread.h"

#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
#include "third_party/blink/renderer/core/workers/experimental/task_worklet_global_scope.h"
#include "third_party/blink/renderer/core/workers/global_scope_creation_params.h"
#include "third_party/blink/renderer/core/workers/threaded_object_proxy_base.h"
Expand Down Expand Up @@ -36,11 +37,11 @@ class ThreadPoolWorkerGlobalScope final : public WorkerGlobalScope {
}

// WorkerGlobalScope
void Initialize(
const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers) override {
void Initialize(const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers,
const Vector<String>* response_origin_trial_tokens) override {
InitializeURL(response_url);
SetReferrerPolicy(response_referrer_policy);
SetAddressSpace(response_address_space);
Expand All @@ -49,6 +50,8 @@ class ThreadPoolWorkerGlobalScope final : public WorkerGlobalScope {
// address space by the "treat-as-public-address" CSP directive.
InitContentSecurityPolicyFromVector(response_csp_headers);
BindContentSecurityPolicyToExecutionContext();

OriginTrialContext::AddTokens(this, response_origin_trial_tokens);
}
void FetchAndRunClassicScript(
const KURL& script_url,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
#include "third_party/blink/renderer/core/inspector/worker_thread_debugger.h"
#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
#include "third_party/blink/renderer/core/probe/core_probes.h"
#include "third_party/blink/renderer/core/workers/shared_worker_thread.h"
#include "third_party/blink/renderer/core/workers/worker_classic_script_loader.h"
Expand Down Expand Up @@ -74,10 +75,13 @@ SharedWorkerGlobalScope* SharedWorkerGlobalScope::Create(
// off-the-main-thread worker script fetch by default.
Vector<CSPHeaderAndType> response_csp_headers =
creation_params->outside_content_security_policy_headers;
std::unique_ptr<Vector<String>> response_origin_trial_tokens =
std::move(creation_params->origin_trial_tokens);
auto* global_scope = MakeGarbageCollected<SharedWorkerGlobalScope>(
std::move(creation_params), thread, time_origin);
global_scope->Initialize(response_script_url, response_referrer_policy,
response_address_space, response_csp_headers);
response_address_space, response_csp_headers,
response_origin_trial_tokens.get());
return global_scope;
}

Expand All @@ -98,7 +102,8 @@ void SharedWorkerGlobalScope::Initialize(
const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers) {
const Vector<CSPHeaderAndType>& response_csp_headers,
const Vector<String>* response_origin_trial_tokens) {
// Step 12.3. "Set worker global scope's url to response's url."
InitializeURL(response_url);

Expand All @@ -119,6 +124,8 @@ void SharedWorkerGlobalScope::Initialize(
// address space by the "treat-as-public-address" CSP directive.
InitContentSecurityPolicyFromVector(response_csp_headers);
BindContentSecurityPolicyToExecutionContext();

OriginTrialContext::AddTokens(this, response_origin_trial_tokens);
}

// https://html.spec.whatwg.org/C/#worker-processing-model
Expand Down Expand Up @@ -226,7 +233,8 @@ void SharedWorkerGlobalScope::DidFetchClassicScript(
classic_script_loader->ResponseAddressSpace(),
classic_script_loader->GetContentSecurityPolicy()
? classic_script_loader->GetContentSecurityPolicy()->Headers()
: Vector<CSPHeaderAndType>());
: Vector<CSPHeaderAndType>(),
classic_script_loader->OriginTrialTokens());

// Step 12.7. "Asynchronously complete the perform the fetch steps with
// response."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ class CORE_EXPORT SharedWorkerGlobalScope final : public WorkerGlobalScope {
const AtomicString& InterfaceName() const override;

// WorkerGlobalScope
void Initialize(
const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers) override;
void Initialize(const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers,
const Vector<String>* response_origin_trial_tokens) override;
void FetchAndRunClassicScript(
const KURL& script_url,
const FetchClientSettingsObjectSnapshot& outside_settings_object,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
#include "third_party/blink/renderer/core/inspector/worker_thread_debugger.h"
#include "third_party/blink/renderer/core/loader/threadable_loader.h"
#include "third_party/blink/renderer/core/messaging/message_port.h"
#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
#include "third_party/blink/renderer/core/probe/core_probes.h"
#include "third_party/blink/renderer/core/trustedtypes/trusted_script_url.h"
#include "third_party/blink/renderer/core/trustedtypes/trusted_type_policy_factory.h"
Expand Down Expand Up @@ -450,8 +449,6 @@ WorkerGlobalScope::WorkerGlobalScope(
SetOutsideContentSecurityPolicyHeaders(
creation_params->outside_content_security_policy_headers);
SetWorkerSettings(std::move(creation_params->worker_settings));
OriginTrialContext::AddTokens(this,
creation_params->origin_trial_tokens.get());

// TODO(sammc): Require a valid |creation_params->interface_provider| once all
// worker types provide a valid |creation_params->interface_provider|.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ class CORE_EXPORT WorkerGlobalScope
const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers) = 0;
const Vector<CSPHeaderAndType>& response_csp_headers,
const Vector<String>* response_origin_trial_tokens) = 0;

// These methods should be called in the scope of a pausable
// task runner. ie. They should not be called when the context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "third_party/blink/renderer/core/frame/settings.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
#include "third_party/blink/renderer/core/inspector/worker_devtools_params.h"
#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
#include "third_party/blink/renderer/core/script/script.h"
#include "third_party/blink/renderer/core/workers/global_scope_creation_params.h"
#include "third_party/blink/renderer/core/workers/parent_execution_context_task_runners.h"
Expand Down Expand Up @@ -57,16 +58,21 @@ class FakeWorkerGlobalScope : public WorkerGlobalScope {
}

// WorkerGlobalScope
void Initialize(
const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers) override {
void Initialize(const KURL& response_url,
network::mojom::ReferrerPolicy response_referrer_policy,
mojom::IPAddressSpace response_address_space,
const Vector<CSPHeaderAndType>& response_csp_headers,
const Vector<String>* response_origin_trial_tokens) override {
InitializeURL(response_url);
SetReferrerPolicy(response_referrer_policy);
SetAddressSpace(response_address_space);

// These should be called after SetAddressSpace() to correctly override the
// address space by the "treat-as-public-address" CSP directive.
InitContentSecurityPolicyFromVector(response_csp_headers);
BindContentSecurityPolicyToExecutionContext();

OriginTrialContext::AddTokens(this, response_origin_trial_tokens);
}
void FetchAndRunClassicScript(
const KURL& script_url,
Expand Down
Loading

0 comments on commit 85b70f1

Please sign in to comment.