1570243 - window.isSecureContext is incorrect in fission when child frame is https

Status: RESOLVED FIXED

(Core :: DOM: Security, task, P3)

Description

[Neil Deakin]

1. Have an HTTP page with a child HTTPS iframe.
2. The iframe's window.isSecureContext should return false, but when fission is enabled, it returns true.

This issue has two implications for the login manager:

1. The detection of insecure login forms to display the insecure login UI is incorrect.
2. The test browser_http_autofill.js , for example, verifies that http logins do not get filled into insecure context. The checks for isSecureContext are within InsecurePasswordUtils.jsm

Comment 1

[Daniel Veditz [:dveditz]]

Does that mean all the mixed-content-blocker stuff is broken as well? I'm thinking of the inverse, of an https parent with an https frame that tries to navigate to an http page (I assume an initial http frame src would be blocked from loading by the parent frame without getting into this problem).

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Jonathan, if that is a problem, than (as discussed) most likely our mixed content blocker implementation is also affected. Can you flip the pref and run all tests within dom/security/mixedcontent (and potentially wpt tests as well)?

I would imagine that there is some API that fission uses to query that information. Currently we most likely just stop traversing docshells when we hit the end of it - which in that case is the iframe.

Comment 3

[Neil Deakin]

This affects both the browser_hasInsecureLoginForms.js and browser_http_autofill.js tests with fission enabled.

Comment 4

[Neha Kochar [:neha]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #2)

Jonathan, if that is a problem, than (as discussed) most likely our mixed content blocker implementation is also affected. Can you flip the pref and run all tests within dom/security/mixedcontent (and potentially wpt tests as well)?

I would imagine that there is some API that fission uses to query that information. Currently we most likely just stop traversing docshells when we hit the end of it - which in that case is the iframe.

All of the security/manager/ssl/tests/mochitest/mixedcontent/ tests are failing with fission, they are marked as skipped in the Fission Mochitests spreadsheet.

Comment 5

[Jonathan Kingston [:jkt] he/him]

So I was able to replicate the issue with the MCB code and I was looking into that, in few places we are traversing docshells and checking if the document is secure, the issue with that in post fission is we don't have access to the docshell.

So I'm experimenting currently by using BrowsingContext fields to allow each docshell to update the browsing context for when it's https or not. This then should actually simplify lots of the code as we just then traverse the browsingcontexts rather than additionally checking if we have a mixedContent channel.

Comment 6

[Jonathan Kingston [:jkt] he/him]

Attachment: Bug 1570243 - Make isSecureContext compatible with fission

Comment 7

[Chris Peterson [:cpeterson]]

Roll some unfixed bugs from Fission Milestone M4 to M5

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Comment 8

[Neha Kochar [:neha]]

Moving to M4.1 since it blocks some mochitests-fission.

Comment 11

[Chris Peterson [:cpeterson]]

Temporarily reassigning these DOM Security Fission bugs to ckerschb for re-triage.

Comment 12

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1570243 - Make WindowContext CanSet functions check if the WindowContext is owned by the setting process, not the BrowsingContext. r?nika

Depends on D73996

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1570243 - Support checking IsSecureContext on OOP ancestors. r?kmag!,ckerschb!

Depends on D73997

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ed69648db69
Make WindowContext CanSet functions check if the WindowContext is owned by the setting process, not the BrowsingContext. r=nika

Comment 15

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/1ed69648db69

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1813cb675a68
Support checking IsSecureContext on OOP ancestors. r=kmag,ckerschb

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/1813cb675a68