Closed
Bug 1021972
Opened 10 years ago
Closed 10 years ago
csp attribute in manifest doesnt actually apply a CSP
Categories
(Core :: DOM: Security, defect)
Tracking
()
People
(Reporter: pauljt, Assigned: fabrice)
References
Details
Attachments
(1 file)
(deleted),
patch
|
ochameau
:
review+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
While testing CSP with the new parser on flame device (ie with security.csp.speccompliant set to true), i noticted that the csp attribute doesnt actually tighten the CSP as it is supposed to.
STR:
1. Install the app from https://github.com/pauljt/csptest using app manager
2. use the app, observer the CSP blocks the inline styles
3. change the app type to privileged or web
4. try the test cases again
Expected:
The forbidden styles should still be blocked, since the mainfest applies a csp.
Actual:
Inline styles are not blocked.
Reporter | ||
Comment 1•10 years ago
|
||
This will likely block 2.1 I expect there be a requirement to be able to apply specific CSP to apps. We will also need to be very careful how we solve this, since there are already a number of apps in the appstore which use a CSP.
cr, how many apps are using this csp attribute (out of how many packaged applications)
blocking-b2g: --- → 2.1?
Flags: needinfo?(cr)
Reporter | ||
Updated•10 years ago
|
Keywords: regression
Comment 2•10 years ago
|
||
I am counting 29 unique app IDs with CSP set, 6 of them being privileged apps, out of 400 privileged app IDs, and out of 2443 app IDs in total in my s3 mirror.
Mind that I am counting per app ID and not multiple versions here, but I don't differentiate by publication status.
The current list is:
420638/public/transgo-immh-1.0/manifest.webapp script-src 'self' 'unsafe-inline';
423632/public/iroller-3.2.0/manifest.webapp allow 'self'; script-src 'self' ; style-src 'self';
429092/pending/whiteout-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
432494/public/splat-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
432866/public/otpwcalc-0.7.3/manifest.webapp default-src *.europalab.com; script-src 'self' 'unsafe-inline'; object-src 'none'; style-src 'self' 'unsafe-inline'
432866/public/otpwcalc-0.7.4/manifest.webapp default-src *.europalab.com; script-src 'self' 'unsafe-inline'; object-src 'none'; style-src 'self' 'unsafe-inline'
432866/public/otpwcalc-0.7.5/manifest.webapp default-src *.europalab.com; script-src 'self' 'unsafe-inline'; object-src 'none'; style-src 'self' 'unsafe-inline'
440302/pending/nomorpenting-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
440750/public/loyer-impaye-1.1/manifest.webapp default-src *; default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline'
446280/public/calculator-13-1.0/manifest.webapp default-src *; script-src 'self' 'unsafe-eval'; object-src 'none'; style-src 'self' 'unsafe-inline'
446280/public/calculator-13-1.0.1/manifest.webapp default-src *; script-src 'self' 'unsafe-eval'; object-src 'none'; style-src 'self' 'unsafe-inline'
449898/public/contacts-backup-0.2/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-0.9/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-1.01/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-1.1/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
450660/public/bitcasa-1-1.11/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
451154/public/local-fresh-1.1/manifest.webapp default-src *; script-src 'self' http://*.googleapis.com; object-src 'none'; style-src 'self' 'unsafe-inline'; connect-src *
453212/public/gisthub-1.0.1/manifest.webapp default-src *; script-src 'self' 'unsafe-eval'; object-src 'none'; style-src 'self' 'unsafe-inline'
456318/pending/orsys-1.0/manifest.webapp default-src *; default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
456320/public/orsys-1-1.0/manifest.webapp default-src *; default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
456320/public/orsys-1-1.2/manifest.webapp default-src *; default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
456320/public/orsys-1-1.3/manifest.webapp default-src *; default-src 'none'; script-src *; object-src 'none'; style-src 'self' 'unsafe-inline' 'inline-script'
457642/public/ripples-1-1.0/manifest.webapp default-src 'self'; img-src 'self'; script-src 'self';
457909/public/foxmote-0.3/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self'
457909/public/foxmote-0.4/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self'
457909/public/foxmote-0.6/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self'
457909/public/foxmote-0.8/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self'
458778/public/sr100simon-1.1/manifest.webapp default-src 'self'; img-src 'self'; script-src 'self';
461644/public/katagana-1.1/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
461644/public/katagana-1.2/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
462457/pending/mobile-app-1.0/manifest.webapp default-src *; script-src 'self' http://*.googleapis.com; object-src 'none'; style-src 'self' 'unsafe-inline'; connect-src *
464126/public/black-star-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
468310/public/mahjong-helper-2.40.2/manifest.webapp default-src 'self' mahjonghelper.com
471904/pending/guiato-0.1/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self'
474628/public/lottoszam-generator-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
476422/pending/debt-list-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
478383/public/magaly-1-2.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
478383/public/magaly-1-3.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
483047/pending/flamita-3.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
483337/pending/asmaul-husna-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
483419/public/asmaul-husna-1-1.0/manifest.webapp default-src *; script-src 'self'; object-src 'none'; style-src 'self' 'unsafe-inline'
484230/public/arrow-1.2.0/manifest.webapp allow 'self'; img-src 'self'; script-src 'self'; style-src 'self';
Flags: needinfo?(cr)
Comment 3•10 years ago
|
||
Is this limited to inline style blocking?
Is security.csp.speccompliant set to true?
Paul: can you try the STR with an image load that would violate the CSP in manifest but not the app default CSP?
Flags: needinfo?(ptheriault)
Comment 4•10 years ago
|
||
I'm adding qawanted to branch checks.
Is there a specific visual we can check for here to know if the expected or actual results are hit?
Keywords: qawanted
Reporter | ||
Comment 5•10 years ago
|
||
re: Jason - I tried to add explanatory comments to the app. But for the moment these explanations aren't correct since we allow inline styles for certified apps. In order to test as intended you need to set the certified CSP back to blocking script:
pref("security.apps.certified.CSP.default", "default-src *; script-src 'self'; object-src 'none'; style-src 'self'");
re: Sid: added a simple image test the test app, and test with multiple CSP permutations.
STR:
1. install the app from https://github.com/pauljt/csptest
2. open the app, scroll to the bottom and press the "external image" link
3. If you see a picture of gunter the penguin, the external image is loading.
The csp I tested with is:
"csp": "image-src 'self'; style-src 'self';"
But I also tried:
"csp": "image-src 'none'";
And I tried with both privileged and certified app types - the image was never blocked.
Flags: needinfo?(ptheriault)
Reporter | ||
Comment 6•10 years ago
|
||
PS I am using a nightly build, so the speccompliant is set to true. I checked this by manually toggling the pref in prefs.js.
Comment 7•10 years ago
|
||
Paul, any chance you can try removing the fast path and rebuild gecko to see if that's causing the issue? I don't have a B2G build spinned up so it might be faster for you to test this out than for me.
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPService.cpp#111
Flags: needinfo?(ptheriault)
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #7)
> Paul, any chance you can try removing the fast path and rebuild gecko to see
> if that's causing the issue? I don't have a B2G build spinned up so it
> might be faster for you to test this out than for me.
>
> http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPService.
> cpp#111
Sure thing, building at the moment. But I see the same result for both certified and privileged apps and fast path only seems to apply to certified iiuc. So not sure this is it.
Flags: needinfo?(ptheriault)
Reporter | ||
Comment 9•10 years ago
|
||
Actually, from a post Andrew made on the marketplace list, I think this might be a side-effect of push apps rather going through the proper install process. Can you confirm this is the case Andrew, Fabrice?
Flags: needinfo?(fabrice)
Flags: needinfo?(awilliamson)
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Paul Theriault [:pauljt] from comment #9)
> Actually, from a post Andrew made on the marketplace list, I think this
> might be a side-effect of push apps rather going through the proper install
> process. Can you confirm this is the case Andrew, Fabrice?
very likely (and a valid bug). Can you paste your /data/local/webapps/webapps.json file after you pushed the app?
Flags: needinfo?(fabrice)
Comment 11•10 years ago
|
||
(In reply to Paul Theriault [:pauljt] from comment #9)
> Actually, from a post Andrew made on the marketplace list, I think this
> might be a side-effect of push apps rather going through the proper install
> process. Can you confirm this is the case Andrew, Fabrice?
if its been pushed via app-manager then the CSP rules get bypassed, yeah. Or at least have done up to now - I don't know about effect of the parser mentioned in comment #0. The zip would have to be installed via the normal js functions. If its privileged then first uploaded to somewhere valid (e.g. marketplace-dev.allizom.org) and installed from there. I can give you reviewer access to -dev to do your own approvals if that helps.
Flags: needinfo?(awilliamson)
Comment 12•10 years ago
|
||
I'm clearing qawanted because the bug as filed here is targeting an app manager workflow, which is only dev driven, not user driven. We need proof of this happening from installation from the web to consider blocking on this btw.
I'm also doubtful this is a regression. We've had many bugs historically happen where we didn't ever bother implementing the workflow in the app manager workflow.
I'm adding needinfo on Alexandre to check to see if my theory above is correct that this is an issue only seen if the app is pushed directly to the device, not by installation off the web.
Flags: needinfo?(poirot.alex)
Keywords: qawanted,
regression
Comment 13•10 years ago
|
||
Not a regression from app manager side. I don't think we ever supported csp defined in manifest.
If somone confirms that install workflow from marketplace works as expected, do not hesitate to move this bug to Firefox::Devtools Web IDE component.
Flags: needinfo?(poirot.alex)
Reporter | ||
Comment 14•10 years ago
|
||
So I installed the "arrow" app from the marketplace. It has a csp, which you can verify in the app manager console:
> app.manifest.csp
< "allow 'self'; img-src 'self'; script-src 'self'; style-src 'self';"
So it has a csp set that should block inline styles and scripts.
But then in app manager, I can do:
window.eval('1+1');
Maybe app manager has super powers to bypass CSP restrictions in apps? Seems unlikely though. I'll try with a self installed app.
Reporter | ||
Comment 15•10 years ago
|
||
Ok testing locally on 2.0 simulator, with the following csp attribute for a "web" type app works.
"csp":"img-src 'self'; script-src 'self'; style-src 'self';"
If I then type "eval(1)" into app manager, I get a csp error.
So I guess next step is checking with an app that I wrote myself installed from staging marketplace.But I am out of time for this week, so I will have to try this next week.
Reporter | ||
Comment 16•10 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #10)
> (In reply to Paul Theriault [:pauljt] from comment #9)
> > Actually, from a post Andrew made on the marketplace list, I think this
> > might be a side-effect of push apps rather going through the proper install
> > process. Can you confirm this is the case Andrew, Fabrice?
>
> very likely (and a valid bug). Can you paste your
> /data/local/webapps/webapps.json file after you pushed the app?
FWIW, here what the entry for my earlier attempt (sideloaded app via app manager):
"0f848c91-0e82-8f49-982b-6b53a38ea5dc": {
"origin": "app://0f848c91-0e82-8f49-982b-6b53a38ea5dc",
"installOrigin": "app://0f848c91-0e82-8f49-982b-6b53a38ea5dc",
"manifestURL": "app://0f848c91-0e82-8f49-982b-6b53a38ea5dc/manifest.webapp",
"appStatus": 3,
"receipts": [],
"installTime": 1404778234593,
"installState": "installed",
"removable": true,
"id": "0f848c91-0e82-8f49-982b-6b53a38ea5dc",
"basePath": "/data/local/webapps",
"localId": 1068,
"name": "CSP Test",
"redirects": null
}
As expected, csp parameter is not set.
Reporter | ||
Comment 17•10 years ago
|
||
Ok this is wierd. For the arrow app i see this in webapps.json:
...
"{6cc39c39-df1e-43e4-b372-bf0aa90ec65c}": {
"name": "Arrow",
"csp": "",
"installOrigin": "https://marketplace.firefox.com",
...
But if you look at the manifest for Arrow,
root@flame:/data/local/webapps/{6cc39c39-df1e-43e4-b372-bf0aa90ec65c} # cat manifest.webapp
{"name":"Arrow","description":"The Web Synthesizer","version":"1.2.0","csp":"allow 'self'; img-src 'self'; script-src 'self'; style-src 'self';","launch_path":"/index.html","orientation":["landscape"],"fullscreen":"true","icons":{"16":"/img/webapp/icon16.png","30":"/img/webapp/icon30.png","32":"/img/webapp/icon32.png","48":"/img/webapp/icon48.png","60":"/img/webapp/icon60.png","64":"/img/webapp/icon64.png","90":"/img/webapp/icon90.png","120":"/img/webapp/icon120.png","128":"/img/webapp/icon128.png","256":"/img/webapp/icon256.png","512":"/img/webapp/icon512.png"},"developer":{"name":"Daniele Veneroni","url":"http://venerons.github.io/"},"default_locale":"en","locales":{"it":{"description":"The Web Synthesizer"}}}
Is that expected?
Assignee | ||
Comment 18•10 years ago
|
||
I can't imagine that to be hard to fix. I'll take a look next week if no-one steps up.
Flags: needinfo?(fabrice)
Updated•10 years ago
|
Comment 19•10 years ago
|
||
This has been nominated as a 2.1 blocker for a while. Who is the right person to triage this and set the flag?
Assignee | ||
Comment 20•10 years ago
|
||
Paul, I'm a bit confused by your comments. Do you see issues only when sideloading apps, or also when doing a normal installation?
Assignee: nobody → fabrice
blocking-b2g: 2.1? → 2.1+
Flags: needinfo?(fabrice) → needinfo?(ptheriault)
Assignee | ||
Comment 21•10 years ago
|
||
Adding csp support for sideloaded apps. Also updated tests for both packaged and hosted apps.
Attachment #8486876 -
Flags: review?(poirot.alex)
Reporter | ||
Comment 22•10 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #20)
> Paul, I'm a bit confused by your comments. Do you see issues only when
> sideloading apps, or also when doing a normal installation?
Sorry its been a while and my comments are confusing but I think it was for both. I installed the arrow app, which I expected to have a more strict CSP than the default one, but it didn't seem too. In comment 17, i meant that the manifest contains a CSP, but when looking in webapps.json, the csp attribute was blank. If this test is accurate, but if it is the case I suppose its separate issue to the issue of the sideloading not applying a CSP.
But I will retest tomorrow to make sure (leaving need-info to remind me).
Comment 23•10 years ago
|
||
Comment on attachment 8486876 [details] [diff] [review]
sideload-csp.patch
Review of attachment 8486876 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks!
Attachment #8486876 -
Flags: review?(poirot.alex) → review+
Assignee | ||
Comment 24•10 years ago
|
||
Reporter | ||
Comment 25•10 years ago
|
||
Ok so this seems to work, but I think I found a bug with CSP. But just to record my test result, I installed an app using web-ide, which had the following in the manifest:
...
"type": "certified",
"csp":"img-src 'self'; script-src 'self'; style-src 'self';"
...
Looking at webapps.json, I can see that this custom CSP has been recorded correctly:
"3e56c15c-e7cc-9c43-84ab-ac40f8fe9593": {
"origin": "app://3e56c15c-e7cc-9c43-84ab-ac40f8fe9593",
"installOrigin": "app://3e56c15c-e7cc-9c43-84ab-ac40f8fe9593",
"manifestURL": "app://3e56c15c-e7cc-9c43-84ab-ac40f8fe9593/manifest.webapp",
"appStatus": 3,
"receipts": [],
"kind": "packaged",
"installTime": 1410501162287,
"installState": "installed",
"removable": true,
"id": "3e56c15c-e7cc-9c43-84ab-ac40f8fe9593",
"basePath": "/data/local/webapps",
"localId": 1034,
"name": "CSP Test",
"csp": "img-src 'self'; script-src 'self'; style-src 'self';",
"role": "",
"redirects": null
}
The issue is that even with this CSP applied, this page still loads an external image:
https://github.com/pauljt/csptest/blob/master/image.html
Flags: needinfo?(ptheriault)
Reporter | ||
Comment 26•10 years ago
|
||
I raised 1066462 for this issue.
Reporter | ||
Comment 27•10 years ago
|
||
Note that I also tested this directly by loading these pages on a webserver the image was in fact blocked in a web page loaded in the browser on the same phone.
Assignee | ||
Comment 28•10 years ago
|
||
That landed on m-c: https://hg.mozilla.org/mozilla-central/rev/faa9786636cd
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 29•10 years ago
|
||
I guess you can self-approve this for Aurora :)
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
status-firefox33:
--- → wontfix
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
Flags: needinfo?(fabrice)
Target Milestone: --- → mozilla35
Assignee | ||
Comment 30•10 years ago
|
||
Comment on attachment 8486876 [details] [diff] [review]
sideload-csp.patch
Approval Request Comment
[Feature/regressing bug #]: Missing feature
[User impact if declined]: Users could sideload apps that access resources they should not get.
[Describe test coverage new/current, TBPL]: xpchsell tests.
[Risks and why]: No special risk, we're hardening app sideloading support.
[String/UUID change made/needed]: n/a
Attachment #8486876 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(fabrice)
Updated•10 years ago
|
Attachment #8486876 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 31•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•