1899963 - Investigate why Workers do not properly inherit CSP's `upgrade-insecure-requests`

Status: ASSIGNED

(Core :: DOM: Workers, task)

Description

[Frederik Braun [:freddy]]

Looking at the WPT dashboard for upgrade-insecure-requests tests, it seems that every test failure is somehow associated to a worker.

In theory, upgradeInsecureRequests is a flag in the loadInfo that should be available and checked pre-request. Given that we have a worker-specific issue I am assuming that we don't copy that from whatever global settings object the worker is using from the originating document. I will find out why.

Comment 1

[Frederik Braun [:freddy]]

First finding:

• For nested workers, nsresult WorkerPrivate::GetLoadInfo() seems to copy a lot of loadInfo flags from aParent (another WorkerPrivate) and definitely omits various flags.

Comment 2

[Frederik Braun [:freddy]]

Looks like there are various code paths that copy select loadInfo flags from various other sources.
I think I'll find the the right kind of path that is omitting this flag for the purpose of fixing the wpt but it's not exactly trivial to know which kind of flags were omitted intentionally or accidentally with the existing pattern.

(I wonder if there's value in moving copyLoadInfoForNestedWorker, copyLoadInfoForWorker, ... functions as a follow up)

Comment 3

[Frederik Braun [:freddy]]

Ah no. WorkerLoadInfo is not LoadInfo. Interesting.

Comment 4

[Frederik Braun [:freddy]]

with the environment variable MOZLOG=CSMLog:5 I definitely get a request where the loadInfo flag upgradeInsecureRequests is false but the CSP contains the "upgrade-insecure-requests" directive.

This is one of the subtests https://wpt.fyi/results/upgrade-insecure-requests/gen/top.http-rp/upgrade/sharedworker-module.https.html?label=experimental&label=master&aligned

[Child 18288: Main Thread]: V/CSMLog doContentSecurityCheck:
[Child 18288: Main Thread]: V/CSMLog   processType: "webIsolated=https://wpt.live"
[Child 18288: Main Thread]: V/CSMLog   channelURI: "https://wpt.live/common/security-features/subresource/shared-worker.py?redirection=downgrade&action=purge&key=2a0c65c3-8ac3-4393-9105-8aa17f62ac38&path=%2Fmixed-content"
[Child 18288: Main Thread]: V/CSMLog   httpMethod: GET
[Child 18288: Main Thread]: D/CSMLog   loadingPrincipal: "https://wpt.live/upgrade-insecure-requests/gen/top.http-rp/upgrade/sharedworker-module.https.html"
[Child 18288: Main Thread]: D/CSMLog   triggeringPrincipal: "https://wpt.live/upgrade-insecure-requests/gen/top.http-rp/upgrade/sharedworker-module.https.html"
[Child 18288: Main Thread]: D/CSMLog   principalToInherit: nullptr
[Child 18288: Main Thread]: V/CSMLog   redirectChain:
[Child 18288: Main Thread]: V/CSMLog   internalContentPolicyType: TYPE_INTERNAL_SHARED_WORKER
[Child 18288: Main Thread]: V/CSMLog   externalContentPolicyType: TYPE_SCRIPT
[Child 18288: Main Thread]: V/CSMLog   upgradeInsecureRequests: false
[Child 18288: Main Thread]: V/CSMLog   initialSecurityChecksDone: false
[Child 18288: Main Thread]: V/CSMLog   allowDeprecatedSystemRequests: false
[Child 18288: Main Thread]: V/CSMLog   wasSchemeless: false
[Child 18288: Main Thread]: D/CSMLog   CSP:
[Child 18288: Main Thread]: D/CSMLog   - "upgrade-insecure-requests"
[Child 18288: Main Thread]: V/CSMLog   securityFlags:
[Child 18288: Main Thread]: V/CSMLog     - SEC_REQUIRE_SAME_ORIGIN_DATA_IS_BLOCKED
[Child 18288: Main Thread]: V/CSMLog     - SEC_COOKIES_SAME_ORIGIN
[Child 18288: Main Thread]: V/CSMLog     - SEC_COOKIES_OMIT
[Child 18288: Main Thread]: V/CSMLog   httpsOnlyFirstStatus:
[Child 18288: Main Thread]: V/CSMLog     - HTTPS_ONLY_UNINITIALIZED
[Child 18288: Main Thread]: D/CSMLog

Comment 5

[Frederik Braun [:freddy]]

Mhh, I've been staring at a debugger for too long. I am having a hard time finding where those values come from. I hope someone with a better understanding of WorkerPrivate would get there more easily. I see it's taking copies of various security flags (why? for thread safety?) but obviously not all of them. But then I don't see how they are being used in their channel and where I would add the right one.

Comment 6

[Frederik Braun [:freddy]]

Maybe the answer is for WorkerPrivate to have an upgradeinsecurerequests-field and a getter/setter and then make sure that's being queried somewhere during construction so that it can inherit from it's parent (which is either a document or a worker. I saw the if/else).

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Note that there's some spec issues discussed in https://github.com/w3c/webappsec-csp/issues/336 (now closed) which begat https://github.com/whatwg/html/issues/9571 and where our implementation when the script loader was updated to handle modules intentionally went with diverging behavior.