Closed Bug 1561079 Opened 1 year ago Closed 1 year ago

Add a way to communicate parent/referrer session for toplevel navigations

Categories

(GeckoView :: General, task, P1)

Unspecified
All

Tracking

(firefox-esr60 unaffected, firefox-esr68 unaffected, firefox68 wontfix, firefox69 wontfix, firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: snorp, Assigned: snorp, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: sec-other, Whiteboard: [geckoview:fenix:m7][post-critsmash-triage][adv-main70-])

Attachments

(4 files)

There are a few web security features, such as Content Security Policy[0], which affect how navigations are performed. Currently, the way the "open in new tab" feature in GV-based browsers work drops all of the information about the origin, making it impossible to enforce CSP and similar things.

I see a couple ways to solve this, and I'm not 100% sure which one is more correct -- or if we perhaps need both:

  1. Add something to GeckoSession to indicate the parent/referrer relationship. This could perhaps be done as an additional parameter to GeckoSession.open(). We would then be able to fetch any info from the parent session at opening time and use it later as needed.

  2. Add an overload of GeckoSession.loadUri() that accepts the parent session as an argument. This would allow us to go fetch the content principal, csp, and whatever else we need before loading the URI.

[0] https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP

Summary: Add concept of "referring" session to `GeckoSession.loadUri()` → Add a way to communicate parent/referrer session for toplevel navigations

Another option would be to encapsulate the principal, csp, and any other relevant info in an opaque SessionInfo (or better name?) object. This would be used the same way as GeckoSession above, but has the additional benefit that the parent session no longer needs to be alive.

We'll still need to expose things like <a noreferrer> to the app for the "open in new tab" case.

Priority: -- → P1
Whiteboard: [geckoview:fenix:m7]
Type: defect → task
Keywords: sec-other
Assignee: nobody → snorp

NI for phabricator comments

Flags: needinfo?(ckerschb)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #5)

NI for phabricator comments

answered on phab :-)

Flags: needinfo?(ckerschb)

Backed out for mochitest failures at dom/base/test/test_bug375314.html:

https://hg.mozilla.org/integration/autoland/rev/7e55309837b20bf0c4beebdc2812e5d50d9773b1

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=usercancel%2Cretry%2Ctestfailed%2Cbusted%2Cexception&revision=0d1eaf86253f2e00bacc7a18ab37068685da6adf
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=258142917&repo=autoland

[task 2019-07-24T17:26:10.164Z] 17:26:10 INFO - 525 INFO TEST-START | dom/base/test/test_bug375314.html
[task 2019-07-24T17:32:44.702Z] 17:32:44 INFO - wait for org.mozilla.geckoview.test complete; top activity=org.mozilla.geckoview.test
[task 2019-07-24T17:32:44.806Z] 17:32:44 INFO - org.mozilla.geckoview.test unexpectedly found running. Killing...
[task 2019-07-24T17:32:44.806Z] 17:32:44 INFO - TEST-INFO | started process screentopng
[task 2019-07-24T17:32:45.157Z] 17:32:45 INFO - TEST-INFO | screentopng: exit 0
[task 2019-07-24T17:32:59.507Z] 17:32:59 WARNING - TEST-UNEXPECTED-FAIL | dom/base/test/test_bug375314.html | application timed out after 370 seconds with no output

Flags: needinfo?(snorp)

This patch is problematic because we apparently depend on the current behavior (to not show an error page for CSP denials) in at least one test. Christoph, do you have any guidance here? Should we change the test?

Flags: needinfo?(snorp) → needinfo?(ckerschb)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #9)

This patch is problematic because we apparently depend on the current behavior (to not show an error page for CSP denials) in at least one test. Christoph, do you have any guidance here? Should we change the test?

Just to make sure I understand the problem correctly

  • Desktop displays an error page if the load is blocked by content Policies except when it's a CSP blockage?
  • Android now always show an error page if content policies block a load (including CSP)?

If that is the case we should fix Desktop to also always show an error page. This will probably become more important once we fix Bug 1529068 (implement CSP navigate-to directive).

Feel free to ping me on slack for quicker turnaround.

Flags: needinfo?(ckerschb)
Regressions: 1569746

I'm going to go out on a limb here and assume that we're not going to attempt to backport this.

Whiteboard: [geckoview:fenix:m7] → [geckoview:fenix:m7][post-critsmash-triage]

No advisory because AFAIK GV is not in any release-track product. ni if incorrect.

Whiteboard: [geckoview:fenix:m7][post-critsmash-triage] → [geckoview:fenix:m7][post-critsmash-triage][adv-main70-]

(In reply to Tom Ritter [:tjr] (needinfo for responses to sec-[approval/ratings/advisories]) from comment #14)

No advisory because AFAIK GV is not in any release-track product. ni if incorrect.

The release versions of Focus and Firefox Reality ship snapshots of GV Nightly. Does that count?

Fenix ships GV Beta, but I guess Fenix is still considered a product preview.

Flags: needinfo?(tom)

They're not listed at https://www.mozilla.org/en-US/security/known-vulnerabilities/ so I don't think it's encompassed in the advisory process? I'll let Dan confirm.

Flags: needinfo?(tom) → needinfo?(dveditz)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.