1341982 - Consider severing opener on insecure->secure context

Status: RESOLVED WONTFIX

(Core :: DOM: Security, defect, P2)

Description

[Jonathan Kingston [:jkt] he/him]

This bug is to consider sever the window.opener when the reference is from an insecure context.

When an insecure page loads a secure page using _blank links or window.open the secure page isn't considered a secure context.

This hampers the ability for the page to use secure context id's

Comment 1

[Jonathan Kingston [:jkt] he/him]

For this it would be good to get telemetry on:
- How often users see isSecureContext=false but isSecureContextIfOpenerIgnored=true
- How often insecure contexts are accessing a WindowProxy for methods on a secure context
- How often a secure context is accessing the WindowProxy methods of an insecure context

Comment 2

[Johann Hofmann [:johannh]]

Note that Google Chrome seems to not sever window.opener and still sets isSecureContext=true, which I believe is against the spec. I believe we don't have to do that since our browser-chrome code can just use isSecureContextIfOpenerIgnored.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

This would need to be spec'd.  Lots of discussion around this kind of thing:

https://github.com/w3c/webappsec/issues/139
https://github.com/w3c/webappsec/issues/517
https://github.com/w3c/webappsec-secure-contexts/issues/42
https://github.com/w3c/ServiceWorker/issues/890#issuecomment-234972242

Comment 4

[Jonathan Kingston [:jkt] he/him]

Thanks for the links Ben I had seen some of them in raising this.

Both these are about having some control of the opener or 'openee' to sever the owner relationship, we really should consider these but in a separate bug:
https://github.com/w3c/webappsec/issues/139
https://github.com/w3c/webappsec/issues/517

The solution by Jake was to sever the secure context when the opener is insecure for Service Workers(https://github.com/w3c/webappsec-secure-contexts/issues/42). However that isn't catching pages that may want to use any of the APIs described in https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts This also will hamper our adoption of a valid interpretation of isSecureContext. Sure we could ignore the spec by using isSecureContextIfOpenerIgnored for newer APIs however that isn't actually fixing the problem either.

The other option is to find a definition that is more strict but would modify the specification of isSecureContext. For example we could consider preventing post messages but allowing the opener context to close windows.

The specification change I would expect would relate to the HTML specification on severing the opener context. We could also consider it as an 'intervention' if other browsers are not willing to adopt (https://github.com/WICG/interventions/issues).

I'm not sure if a standard disown would suffice here as Mike West has already specified that window.opener=null wouldn't fix padlocks after the fact and isSecureContext would be consider still false (https://github.com/w3c/webappsec/issues/139#issuecomment-234968247).

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

AIUI sites that want to use secure APIs could set the proposed CSP header to prevent ever having an opener.

In any case, my point was more that we can't act unilaterally here.  The discussion really needs to happen at the spec level.

Comment 6

[Jonathan Kingston [:jkt] he/him]

https://w3c.github.io/webappsec-secure-contexts/#issue-8ea95bab is in the spec itself I think instead we could actually consider ignoring this point in the isSecureContext spec but warn of deprecation for when this point isn't met.

> document has an creator browsing context (context), and context’s creator context security is "Not Secure".

So consider as secure and break the specification in the same way however aim for this eventually to be the case.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

Jonathan, assigning this to you mainly to resolve the spec issue. thanks!

Comment 8

[Jonathan Kingston [:jkt] he/him]

Given the spec has been resolved I think we should close this in favour of Bug 1410364.

I think we can readdress this in the future however building the telemetry to decide if it is a low risk idea is pretty messy and likely would require a lot of time to consider all the edge cases.