1584543 - Cert errors in sandboxed iframes are still broken

Status: VERIFIED FIXED

(Firefox :: Security, defect, P1)

Description

[Johann Hofmann [:johannh]]

...because the sandbox gives them a null principal and the RPM check fails.

STR:

data:text/html,<iframe sandbox="allow-scripts" src="https://expired.badssl.com"></iframe>

And open the devtools console to see the RPM error.

Comment 1

[Johann Hofmann [:johannh]]

Turns out that our WebIDL-exposed info doesn't work either, I think the proper way to fix this is to use documentURI for these checks instead of the principal. With the principal we would do URI-based comparisons anyway, there's no reason to use it as there's no possibility we would need the other information the principal contains.

Comment 2

[Johann Hofmann [:johannh]]

Attachment: Bug 1584543 - Make checks for in-content functionality depend on documentURI instead of principal URI. r=ckerschb!,Gijs!

This is a necessary change that was done for Fluent access in bug 1573276. In almost all cases,
we want to rely on the principal for making security decisions, but the principal does not
store the original URI in cases where an about: page was sandboxed (it becomes a null principal URI),
and thus we need to use the documentURI here.

Comment 3

[Johann Hofmann [:johannh]]

We should consider uplifting this

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10c7400edbc1
Make checks for in-content functionality depend on documentURI instead of principal URI. r=ckerschb,Gijs

Comment 5

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/10c7400edbc1

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 7

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Do you think this would be OK to uplift for beta 13 (of 14)?

Comment 8

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Oh, would this fix bug 1573276?

Comment 9

[:Gijs (he/him)]

(In reply to Liz Henry (:lizzard) from comment #8)

Oh, would this fix bug 1573276?

Not on its own, we need both fixes for error pages to be fully functional when in sandboxed iframes.

Comment 10

[Johann Hofmann [:johannh]]

Comment on attachment 9097308 [details]
Bug 1584543 - Make checks for in-content functionality depend on documentURI instead of principal URI. r=ckerschb!,Gijs!

Beta/Release Uplift Approval Request

• User impact if declined: Certificate error pages in iframes are lacking some functionality (only the basic error is displayed, but the user can't really interact)
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: :

• Open the URL from comment 0
• Click on "Advanced" in the iframe
• Make sure that all advanced information is displayed and that you can click on "View Certificate" and other elements

• List of other uplifts needed: Bug 1573276
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The fix is relatively straightforward, but it modifies code that checks security boundaries, so it's always a good idea to be a little cautious with that. On the other hand this has a lot of automated tests.
• String changes made/needed: None

Comment 11

[ovidiu boca[:Ovidiu]]

I tested this issue on Windows 10, Mac OS X 10.14 and Ubuntu 18.04 with FF Nightly 71.0a1(2019-10-06) and I can confirm the fix.

Comment 12

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 9097308 [details]
Bug 1584543 - Make checks for in-content functionality depend on documentURI instead of principal URI. r=ckerschb!,Gijs!

OK for uplift for beta 13 along with the patch in bug 1573276.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This conflicts with bug 1561443 which is only on central. Please provide a patch for beta.

Comment 14

[Johann Hofmann [:johannh]]

Attachment: Patch for beta uplift

This is the patch for beta.

Thanks!

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d043869202a1

Comment 16

[ovidiu boca[:Ovidiu]]

I tested this issue on Windows 10, Mac OS X 10.14 and Ubuntu 18.04 with FF Beta 70.0b14 and I can confirm the fix.