Add WebDriver BiDi CSP bypass checks

[juliandescottes]

Fixes #798

Updates several points (eg. document initialization as well as individual enforcement checks) to check if WebDriver BiDi CSP bypass is enabled for the relevant navigable.

If the bypass is enabled, the policy will not be added / applied.

The goal is to use this feature only in automation / remote control scenarios when the browser is controlled by a WebDriver BiDi session.

Tests will be added as wdspec tests in https://github.com/web-platform-tests/wpt/tree/master/webdriver/tests/bidi once the corresponding WebDriver BiDi PR is close to being accepted.

---

Preview | Diff

[juliandescottes]

At this point, I am mostly looking for feedback to know if the approach seems acceptable on your side, as it may impact the WebDriver BiDi PR I opened at w3c/webdriver-bidi#1068

There are 2 commits in this PR. The first one only integrates with the document initialization. From what I checked in the current implementation of the (similar) Page.setBypassCSP command in CDP, this is the only thing Chrome implements.

However is doesn't cover cases where policies are dynamically added after the document initialization or when the document hasn't been initialized yet. So I have a second commit which covers additional integration points.

Let me know if on your side you have a preference for only doing the simpler check at document initialization. I'm also checking the feasability of those additional checks on the implementation side, but for now the only prior art ios to disable CSP checks at initialization time.

[juliandescottes]

There are 2 commits in this PR. The first one only integrates with the document initialization. From what I checked in the current implementation of the (similar) Page.setBypassCSP command in CDP, this is the only thing Chrome implements.

We discussed this item in scope of the BiDi PR, w3c/webdriver-bidi#1068 (comment) and on our side we would prefer to cover as many integration points as possible.

Tagging @mikewest for feedback

[juliandescottes]

cc @dveditz

[dveditz]

Don't put this first in this section. Fetch, HTML, and ECMAScript integrations are fundamental to the core of what CSP is and intends to do. WebDriver integration is incidental to the purpose of CSP—the integration is being added to serve WebDriver—and should go at the end of the integrations section

The WebRTC integration shouldn't be where it is, either (I'll file an separate issue on that).

[juliandescottes]

Sounds good, moving it at the end.

[dveditz]

WebDriver is pretty tangential to CSP so I'm not all that keen on how much brainspace these changes give it in the CSP spec; can you at least combine these two steps into one, like

If the result of #does-webdriver-disable-CSP given |request| is true, return "Allowed".

What I'd really like is to hide all this even deeper, like have "Retrieve the CSP List of an object" do the check. The title of that section kind of sounds like it ought to be the choke point on getting a CSP List, but I haven't found anywhere that actually references it. Everywhere else says to get the "policy container's CSP list" directly so I'm not sure what §4.2.2 actually does. It sure seems like the better approach is to make the CSP List appear null or actually be null when SetBypassCSP is true rather than patch all the other algorithms that are about to use the CSP List. The current approach in your patch means we risk forgetting to add these steps when some future algorithm is added.

Maybe @mikewest or @antosart would have some ideas here.

[juliandescottes]

Combining the two steps for now, but otherwise I agree that having a single point where the BiDi hook is checked would make it much easier in terms of maintenance.

[juliandescottes]

I'm not sure what §4.2.2 actually does.

On that topic, it seems referenced here: https://www.w3.org/TR/CSP3/#global-object-csp-list:~:text=A%20global%20object%E2%80%99s%20CSP%20list%20is%20the%20result%20of%20executing%20%C2%A7%E2%80%AF4%2E2%2E2%20Retrieve%20the%20CSP%20list%20of%20an%20object%20with%20the%20global%20object%20as%20the%20object%2E

A global object’s CSP list is the result of executing § 4.2.2 Retrieve the CSP list of an object with the global object as the object.

But as you said, many spots in the spec simply go directly to the "policy container's CSP list". And at the moment the policy container is simply a struct, so I'm not sure we should change it in order to check the BiDi hook before returning the list.

However what could be a better approach, still contained in the webapp-csp spec, could be to add an algorithm to retrieve the CSP List for a given policy container + related navigable. And this algorithm would be the central place where we do the check?

[juliandescottes]

@dveditz I just updated the patch to introduce a single algorithm which returns the effective CSP policies for a given CSP List + navigable.

Let me know if that could work.

[dveditz]

I'm supportive of what you're trying to do, but the number of explicit hooks into WebDriver BiDi here seem excessive. Especially since Chrome has supported an equivalent for CDP without needing to change any specs. Could this be accomplshed by checking the WebDriver hook when getting a CSP List, or when parsing policies so nothing gets added to the CSP List? Or maybe in the HTML spec to have WebDriver set a flag on the policy container that controls whether the CSP List is returned or not.

Different choices might lead to edge cases where the WebDriver/Firefox behavior differs from what Chrome does with CDP, and the point of adding this to the spec is to get consistent behavior.

[juliandescottes]

Thanks for the review @dveditz

I'm supportive of what you're trying to do, but the number of explicit hooks into WebDriver BiDi here seem excessive. Especially since Chrome has supported an equivalent for CDP without needing to change any specs.

(just noting that nothing in CDP follows any specification, it is one of the main motivations behind WebDriver BiDi)

Could this be accomplshed by checking the WebDriver hook when getting a CSP List, or when parsing policies so nothing gets added to the CSP List? Or maybe in the HTML spec to have WebDriver set a flag on the policy container that controls whether the CSP List is returned or not.

Different choices might lead to edge cases where the WebDriver/Firefox behavior differs from what Chrome does with CDP, and the point of adding this to the spec is to get consistent behavior.

FWIW, @sadym-chromium (google) was in favor of making the spec as thorough as possible even if it didn't match the current CDP implementation when we discussed the topic in the BTT group (discussion minutes here).

I'll take a look at the suggestions to simplify the integration with the BiDi spec, thanks again.

[juliandescottes]

@dveditz Any thoughts about the updated version?