797657 - Change default CSP to desired policy for certified apps

Status: RESOLVED FIXED

(Core :: General, defect, P2)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Right now we had to adjust the policy to allow inline scripts and stylesheets since otherwise some apps broke. We should flip it back and find all cases which gives warnings and fix those.

Comment 1

[dynamis (Tomoya ASAI)]

Why don't they use 'unsafe-inline' (or obsolete 'inline-script')?
Why don't we require to use CSP properly to certify them?

Privileged or certified apps should be more secure than normal web apps.
I believe it's better to help app developers, not loose security of our OS.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

That's exactly why we have this bug filed.

Comment 3

[Lucas Adamski [:ladamski]]

I set the CSP prefs manually in system/b2g/defaults/pref/user.js and everything seems to still work.  Would be nice if someone else wants to verify, but then I say we flip the switch.  I used:
pref("security.apps.certified.CSP.default", "default-src *; script-src 'self'; object-src 'none'; style-src 'self'");

Comment 4

[Alex Keybl [:akeybl]]

Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 797657: Change default CSP to desired policy for certified apps

Comment 6

[Alexandre Poirot [:ochameau]]

Comment on attachment 682108 [details] [diff] [review]
Bug 797657: Change default CSP to desired policy for certified apps

See bug 796739 comment 30 for a description about existing CSP violation still alive in gaia. We would like to wait for bug 796739 to be closed before landing this patch.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 682108 [details] [diff] [review]
Bug 797657: Change default CSP to desired policy for certified apps

Review of attachment 682108 [details] [diff] [review]:
-----------------------------------------------------------------

Assuming that you've done a lot of testing to make sure that this doesn't break anything, r=me

Comment 8

[Andrew Overholt [:overholt]]

Is this ready to land, Alex?

Comment 9

[Alexandre Poirot [:ochameau]]

Still need attachment 682083 [details] from bug 796739.
I'll try to get it reviewed'n landed tomorrow.

Comment 10

[Alexandre Poirot [:ochameau]]

We are now ready to flip to safe default, only UITest app is going to be broken.
But it is only used for manual testing and will hopefully be fixed easily thanks to bug 801783.

But I had a relevant comment from James, why is document.write allowed ?
It doesn't seem to be blocked by CSP.

Comment 11

[Ian Melven :imelven]

(In reply to Alexandre Poirot (:ochameau) from comment #10)
> We are now ready to flip to safe default, only UITest app is going to be
> broken.
> But it is only used for manual testing and will hopefully be fixed easily
> thanks to bug 801783.
> 
> But I had a relevant comment from James, why is document.write allowed ?
> It doesn't seem to be blocked by CSP.

I discussed this with Sid, our understanding is that document.write is not intended to be blocked by CSP, but the things it might inject e.g. inline scripts should be blocked (assuming the policy hasn't opted in to unsafe-inline for scripts, of course).

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b65787162e89

Comment 13

[Ryan VanderMeulen [:RyanVM]]

This was backed out while investigating some other bustage that landed in the same push.

Re-landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/374ff1d28158

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/374ff1d28158

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/eefe52b408c4
https://hg.mozilla.org/releases/mozilla-beta/rev/e00e150b5bee