Skip to content

Extend browser.createUserContext with acceptInsecureCerts parameter #895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
75 changes: 73 additions & 2 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ Test Suite: https://github.com/web-platform-tests/wpt/tree/master/webdriver/test
</pre>

<pre class=anchors>
spec: RFC5280; urlPrefix: https://datatracker.ietf.org/doc/html/rfc5280
type: dfn
text: Basic Certificate Processing; url: section-6.1.3
spec: RFC6455; urlPrefix: https://datatracker.ietf.org/doc/html/rfc6455
type: dfn
text: WebSocket URI; url: section-3
Expand Down Expand Up @@ -51,6 +54,7 @@ spec: RFC6265bis; urlPrefix: https://datatracker.ietf.org/doc/html/draft-ietf-ht
spec: WEBDRIVER; urlPrefix: https://w3c.github.io/webdriver/
type: dfn
text: WebDriver new session algorithm; url: dfn-webdriver-new-session-algorithms
text: accept insecure TLS; url: dfn-accept-insecure-tls
text: actions; url: actions
text: actions options; url: dfn-actions-options
text: active sessions; url: dfn-active-sessions
Expand Down Expand Up @@ -1504,6 +1508,10 @@ time. However they are not required to remove such [=user contexts=]. [=User
contexts=] that are not [=empty user contexts=] must not be removed from the
[=set of user contexts=].

A [=BiDi session=] has a
<dfn>user contexts with insecure certificates overrides</dfn>, which is a [=/set=] of
[=user contexts=].

When a [=user context=] is [=set/remove|removed=] from the
[=set of user contexts=], [=remove user context subscriptions=].

Expand Down Expand Up @@ -1607,6 +1615,14 @@ To <dfn>cleanup the session</dfn> given |session|:

1. [=Close the WebSocket connections=] with |session|.

1. For each |user context| in the |session|'s
[=user contexts with insecure certificates overrides=]:

1. [=Restore insecure certificates behavior=] with |user context|.

1. [=set/Remove=] |user context| from |session|'s
[=user contexts with insecure certificates overrides=].

1. If [=active sessions=] is [=list/empty=], [=cleanup remote end state=].

1. Perform any implementation-specific cleanup steps.
Expand Down Expand Up @@ -2655,8 +2671,12 @@ The <dfn export for=commands>browser.createUserContext</dfn> command creates a
<pre class="cddl remote-cddl">
browser.CreateUserContext = (
method: "browser.createUserContext",
params: EmptyParams,
params: browser.CreateUserContextParameters,
)

browser.CreateUserContextParameters = {
? acceptInsecureCerts: bool
}
</pre>
</dd>
<dt>Return Type</dt>
Expand All @@ -2669,10 +2689,61 @@ The <dfn export for=commands>browser.createUserContext</dfn> command creates a

<div algorithm="remote end steps for browser.createUserContext">

The [=remote end steps=] are:
<div algorithm>
To <dfn>restore insecure certificates behavior</dfn> given |user context|:

1. If [=endpoint node=]'s [=accept insecure TLS=] flag is true,
[=override insecure certificates behavior=] with |user context| and true.

1. Otherwise, [=override insecure certificates behavior=] with |user context| and
false.

</div>

<div algorithm>
To <dfn>override insecure certificates behavior</dfn> given |user context| and
|acceptInsecureCerts|:

1. If |acceptInsecureCerts| is true:

1. If [=endpoint node=] doesn't support accepting insecure TLS connections,
return [=error=] with [=error code=] [=unsupported operation=].

1. Perform implementation-defined steps to skip steps 6.1.3.a and any other
implementation-specific validation steps of [=Basic Certificate Processing=]
for all network requests associated with |user context|. This should affect all
the network requests associated with the |user context|, e.g. initiated by
[=/navigables=] whose [=associated user context=] is |user context|.

1. Return.

1. Assert |acceptInsecureCerts| is false.

1. Perform implementation-defined steps to restore step 6.1.3.a and all other skipped
validation steps of [=Basic Certificate Processing=] for all network requests
associated with |user context|. This should affect all the network requests
associated with the |user context|, e.g. initiated by [=/navigables=] whose
[=associated user context=] is |user context|.

TODO: mention service workers.

</div>

The [=remote end steps=] with |session| and |command parameters| are:

1. Let |user context| be a new [=user context=].

1. If |command parameters| [=map/contain=] "<code>acceptInsecureCerts</code>":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to integrate this better with the WebDriver classic spec (which admittedly is a mess).

For example: what happens if the WebDriver session ends? If you end a classic session then the flag is unset, and cert checking resumes. But BiDi supports multiple sessions, and this is global state, so do we need to revert it only when the last session ends? Or when the session that set the flag ends (I guess the latter). Currently it seems like it would persist indefinitely. Also, in theory, the capability can be unsupported. It seems reasonable that we handle the unsupported case here too (e.g. by returning an error).

(A really good patch here would have a check in fetch at the point at which a request is started. That would allow answering questions like "what about service workers?". But I understand that's a lot of work, so I don't consider it a blocker)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the WebDriver session ends?

I believe the behavior of acceptInsecureCerts and proxy should be identical. As long as we cannot configure proxy AFTER the user context is created, I'd propose to keep the acceptInsecureCerts and proxy behavior even after the WebDriver BiDi session is closed.

In order to prevent conflicts with Classic sessions, I'd propose to make the command params acceptInsecureCerts and proxy mutually exclusive with the capability.

Currently it seems like it would persist indefinitely.

This is intentional.

Also, in theory, the capability can be unsupported. It seems reasonable that we handle the unsupported case here too (e.g. by returning an error).

Good point. Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done:

  • acceptInsecureCerts is tight to the session created the user context. When the session is disconnected, the default behavior taken from WebDriverClassic is restored.
  • If the acceptInsecureCerts is not supported, error will be returned.

Open question:

  • WRT the service workers, IDK how to tight one to the user context. Do you have any ideas of that, or do you think we can leave it for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW: I believe the TLS check happens in fetch: create a connection, step 2, "... create a connection ...".

Let connection be a new connection whose key is key, origin is origin, credentials is credentials, and timing info is timingInfo. Record connection timing info given connection and use connection to establish an HTTP connection to host, taking proxy and origin into account, with the following caveats: [HTTP] [HTTP1] [TLS]


Note: If "<code>acceptInsecureCerts</code>" is set, it overrides the
[=accept insecure TLS=] flag's behavior.

1. [=Override insecure certificates behavior=] with |user context| and
|command parameters|["<code>acceptInsecureCerts</code>"].

1. [=set/Append=] |user context| to |session|'s
[=user contexts with insecure certificates overrides=].

1. [=set/Append=] |user context| to the [=set of user contexts=].

1. Let |user context info| be a [=/map=] matching the
Expand Down