search

WICG / cookie-store

Asynchronous access to cookies from JavaScript
https://wicg.github.io/cookie-store/
Apache License 2.0
143 stars 35 forks source link

Add references to secure/insecure contexts/origins and clarify the case of local files. #193

Open fred-wang opened 3 years ago

fred-wang commented 3 years ago

This is mentioned at several places, but AFAIK the only reference is on the IDL:

https://heycam.github.io/webidl/#SecureContext

which in turn links to

https://html.spec.whatwg.org/multipage/webappapis.html#secure-context

and in turn to

https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy

This seems to mean we should treat localhost, loopback and files:// addresses as secure when deciding whether a cookie can be modified. For consistency with document.cookie, it seems the chromium people want to allow cookie store writing API on localhost/loopback but not files:// (see https://groups.google.com/a/chromium.org/g/blink-dev/c/ekdeaj09c0w/m/HVpvpMmHBQAJ ).

So I think:

  1. The spec could more explicitly reference the secure contexts spec in the various places "secure" or "insecure" is used.
  2. The spec should explicitly exclude file:// for consistency with document.cookie.

Probably 2. can be done in a local definition based on https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url ; and 1. can link to that new definition.

inexorabletash commented 3 years ago

Re: (1) "the only reference is on the IDL" - this is good, isn't it? The IDL bindings take care of limiting use, we shouldn't add redundant prose?

Re: (2) Yeah - if we want to go beyond [SecureContext] to block file://, then we'll need to add it. Basically, where the prose currently has "If origin is an opaque origin, then..." we could instead have a local definition of allowed origin and define an allowed origin as not opaque and not file://.

Want to spin up a PR for that?

fred-wang commented 3 years ago

Re: (1) "the only reference is on the IDL" - this is good, isn't it? The IDL bindings take care of limiting use, we shouldn't add redundant prose?

Right, by "reference" I meant the [SecureContext] in the IDL is the only place where there is a link to an actual definition.

When "secure context" is used in the text, it's probably https://html.spec.whatwg.org/multipage/webappapis.html#secure-context but a link can still help.

In other places, it's not necessary clear what "secure/insecure" refer to. I'm not sure what "secure origin" refers to for example. Adding a link to one of the existing definition I linked above would help to clarify things, without adding more prose.

Re: (2) Yeah - if we want to go beyond [SecureContext] to block file://, then we'll need to add it. Basically, where the prose currently has "If origin is an opaque origin, then..." we could instead have a local definition of allowed origin and define an allowed origin as not opaque and not file://.

So the additional restriction that Chromium implements is forbidding writing/delete cookies when the cookie's url is not potentially trustworthy or is a local scheme:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/cookie_store/cookie_store.cc;l=126;drc=13aae191604726d8c248100b2d89596ad7344721

That way it behaves the same as document.cookie.

Want to spin up a PR for that?

Sure I can give a try but I'm not exactly always sure what is meant when "secure" is used in the spec.

About the second part, I reported this because it's how it's implemented in Chromium but I guess this is something to be decided by the CG...

fred-wang commented 3 years ago

I opened https://github.com/WICG/cookie-store/pull/194 for secure context / unsecure context.