Webapps.jsm needs to pass securityFlag to NetUtil.newChannel()

RESOLVED FIXED in Firefox 47

Status

()

Core
DOM: Security
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ckerschb, Assigned: ckerschb)

Tracking

unspecified
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

2 years ago
Assignee: nobody → mozilla
Blocks: 1232430
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8717673 [details] [diff] [review]
bug_1247108_webapps_securityflag.patch

Hey Fabrice, within Bug 1087728 you helped us find the right principal; now we have to pass a securityFlag to newChannel for webapps so that we are performing the right securityChecks when calling asyncOpen2 for that channel.

We have to use one of the securityFlags from [1]. What I remember is that this code is installing an app either from a local file or from the internet, right? I suppose that does not need to be same-origin and it also doesn't need CORS, right?

So we have two flags we could choose from:
1) SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS
where, when loading a data: uri we are inherting the principal, or
2) SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL
where, when loading data: we are applying a newly created nullPrincipal.

I assume you are fine with SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS, otherwise please let me know.

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsILoadInfo.idl#44
Attachment #8717673 - Flags: review?(fabrice)
(Assignee)

Updated

2 years ago
Blocks: 1232906
(Assignee)

Comment 2

2 years ago
Created attachment 8717700 [details] [diff] [review]
bug_1247108_webapps_securityflag.patch

Fabrice, the more I think about it, the more I think that this channel is actually initiated by the system, hence I think that load should use the SystemPrincipal instead of creating a principal.

With the (obsolete) patch from this bug I actually encounter the following test error (see underneath) when running
> dom/apps/tests/test_operator_app_install.xul

The app:// is opening a channel to a file://, which the scriptSecurityManager actually forbids [1]. I don't know if we actually have machinery in place so we could just easily make that load succeed. Obviously, if you insist we can make that work, but I think using the systemPrincipal for that load would be the better solution.

Ultimately it's your call. Please let me know how you would like to proceed.

[1] http://mxr.mozilla.org/mozilla-central/source/caps/nsScriptSecurityManager.cpp#670 

%---snip---

doContentSecurityCheck {
  channelURI: file:///tmp/tmp_2233756/singlevariantapps/testOperatorApp1/application.zip
  loadingPrincipal: app://{707e695a-3f9a-4a63-adba-a25f3a0f18e3}/
  triggeringPrincipal: app://{707e695a-3f9a-4a63-adba-a25f3a0f18e3}/
  contentPolicyType: 1
  securityMode: 4
  initalSecurityChecksDone: no
  enforceSecurity: no
}
[32012] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /home/ckerschb/moz/mc/dom/security/nsContentSecurityManager.cpp, line 62
[32012] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /home/ckerschb/moz/mc/dom/security/nsContentSecurityManager.cpp, line 538
[32012] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /home/ckerschb/moz/mc/dom/security/nsContentSecurityManager.cpp, line 410
[32012] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /home/ckerschb/moz/mc/netwerk/base/nsBaseChannel.cpp, line 694
Attachment #8717673 - Attachment is obsolete: true
Attachment #8717673 - Flags: review?(fabrice)
(Assignee)

Updated

2 years ago
Attachment #8717700 - Flags: review?(fabrice)
Attachment #8717700 - Flags: review?(fabrice) → review+

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cd35429adc7

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6cd35429adc7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.