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)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
blocking-b2g 2.1+
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: pauljt, Assigned: fabrice)

References

Details

Attachments

(1 file)

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.
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)
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)
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)
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
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)
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.
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)
(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)
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)
(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)
(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)
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
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)
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.
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.
(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.
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?
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)
Depends on: 773891
This has been nominated as a 2.1 blocker for a while. Who is the right person to triage this and set the flag?
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)
Attached patch sideload-csp.patch (deleted) — Splinter Review
Adding csp support for sideloaded apps. Also updated tests for both packaged and hosted apps.
Attachment #8486876 - Flags: review?(poirot.alex)
(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 on attachment 8486876 [details] [diff] [review] sideload-csp.patch Review of attachment 8486876 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8486876 - Flags: review?(poirot.alex) → review+
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)
I raised 1066462 for this issue.
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.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
I guess you can self-approve this for Aurora :)
Flags: needinfo?(fabrice)
Target Milestone: --- → mozilla35
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)
Attachment #8486876 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: