796739 - CSP violations warnings

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect, P3)

Description

[GH to BZ]

[GitHub issue by fabricedesre on 2012-09-25T20:56:31Z, https://github.com/mozilla-b2g/gaia/issues/5173]
When the system app loads, we now have these warnings:

E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onload attribute on UNKNOWN element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onsubmit attribute on FORM element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on SECTION element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on BUTTON element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on DIV element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on DIV element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on DIV element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on DIV element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on BUTTON element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on DIV element"}]
E/GeckoConsole(  106): [JavaScript Warning: "CSP WARN:  Directive inline script base restriction violated
E/GeckoConsole(  106): " {file: "app://system.gaiamobile.org/index.html" line: 0 column: 0 source: "onmousedown attribute on BUTTON element"}]

Comment 1

[GH to BZ]

[GitHub comment by nhirata on 2012-09-26T21:59:36Z]
is this a dup of #5177?

Comment 3

[James Lal [:lightsofapollo]]

@dale bug 803178 is under the calendar component and unrelated to this.

Comment 4

[Dale Harvey (:daleharvey)]

oh sorry didnt notice the component

Comment 5

[Alexandre Poirot [:ochameau]]

I'll take a look at this if noone started working on this.

Comment 6

[David Scravaglieri [:scravag]]

renominated for triage

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 5982 - Fix settings

Here is settings fixes.
I'll do one pull request per app.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 5983 - fix clock

The one for clock app.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 674640 [details]
Pull request 5983 - fix clock

https://github.com/mozilla-b2g/gaia/commit/35dbf43c7272b82a3233c259f82ba804a130cadc

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6118 - fix keyboard

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/a40549a32e778de1bf1db4cd83a7c0f82bfaab4d

Comment 12

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6165 - fix email

Comment 13

[Lucas Adamski [:ladamski]]

Are there other outstanding issues stopped us from enabling the full CSP policy?  I'd like to wrap this up by the end of this week.

Comment 14

[Alexandre Poirot [:ochameau]]

There is still one CSP violation in communications, then I think we are good to go.
I've seen some inline script in showcase apps, but I think they are not involved in CSP secured policy, right?
I'm currently working on the patch for communications.

Comment 15

[Lucas Adamski [:ladamski]]

I don't think the showcase apps are packaged or certified apps?  If not then we should have a problem there.

Comment 16

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6175 - fix communications

Didn't knew who to ask for this review, feel free to forward it.
But that isn't necessary a review for communications app developers as it is mostly about root `webapps.js` file.

Comment 17

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6178 - optional communications fix

Vivien, same thing here. Please forward this r? to the right person. This time it is really for someone involved in Communnication!

Comment 18

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6181 - fix cubevid and crystallskull

Lucas, I think we will be able to enable strict policy with all these pull requests being reviewed and merged.
Feel free to give it a try in the meantime as I may easily have missed some CSP violation.

Comment 19

[David Flanagan [:djf]]

Comment on attachment 678505 [details]
Pull request 6181 - fix cubevid and crystallskull

Looks good.

Comment 20

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6244 - try to fix uitest by changing its csp

There is lot of CSP violations in UITests app.
It doesn't seem worthwile to fix them, so I tried to use `csp` field in webapp manifest, without success. Would you happen to know why it doesn't work?
https://developer.mozilla.org/en-US/docs/Apps/Manifest#Manifest_fields
(Implemented in bug 773891)

Comment 21

[Lucas Adamski [:ladamski]]

Comment on attachment 679164 [details]
Pull request 6244 - try to fix uitest by changing its csp

The CSP property in the manifest is intersected with the system-enforced CSP policy, and so can only tighten the system policy further.

Comment 22

[Alexandre Poirot [:ochameau]]

(In reply to Lucas Adamski from comment #21)
> The CSP property in the manifest is intersected with the system-enforced CSP
> policy, and so can only tighten the system policy further.

Hum, even when we remove `type: "certified"` ?
Otherwise do you think about any workaround that would prevent patching all these violations?

Comment 23

[Lucas Adamski [:ladamski]]

Non-certified/privileged applications don't have a required CSP policy.  But then other things would break (like direct access to the Contacts API).

I don't know of a strightforward way to solve this without changing the device-wide CSP pref, but that would not help with the core purpose of testing and would make anyone who wants to do testing would have to open a big security hole on their device.

One way of doing it could be to have a device pref to enable an exception for enforcing the minimum CSP for specific apps, but that feels only slightly less terrible than the above suggestion.

Comment 24

[Lucas Adamski [:ladamski]]

Just found one more: SMS app
[JavaScript Warning: "CSP WARN:  Directive eval script base restriction violated
" {file: "app://sms.gaiamobile.org/shared/js/phoneNumberJS/PhoneNumber.js" line: 56 column: 0 source: "call to eval() or related function blocked by CSP"}]

App seems to work ok.

Comment 25

[Lucas Adamski [:ladamski]]

Update on c24: Per agal PhoneNumber.js is moving to Gecko and can be ignored.  App seems to work otherwise.

Comment 26

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 678475 [details]
Pull request 6178 - optional communications fix

Jose changes has landed at: https://github.com/mozilla-b2g/gaia/commit/98dab6b2b1df54762c0886ae8f5ba70bd643b82f

Comment 27

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6438 - fix bluetooth

Comment 28

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6437 - fix FTU

Comment 29

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 682079 [details]
Pull request 6438 - fix bluetooth

It seems like onsubmit on buttons does nothing :)

Comment 30

[Alexandre Poirot [:ochameau]]

So here is the current status about CSP:
- We have some minor violations in bluetooth and ftu apps,
- tons of violations in uitest app, but we shouldn't block on this, and hopefully bug 801783 will land soon and we will then be able to make it work again,
- one hard to fix violation in SMS app that isn't really harmfull as it doesn't completely break SMS app but kill the phone number normalization done in SMS. So if we consider bug 811539 as being bb+ and ensure that it is fixed before shipping, we can consider switching to strict CSP now.

Comment 31

[Evelyn Hung]

Comment on attachment 682079 [details]
Pull request 6438 - fix bluetooth

Ian, could you please take a look of this? I guess there are some follow-up works in your JS code. Thanks.

Comment 32

[Alexandre Poirot [:ochameau]]

Attachment 678505 [details] landed:
https://github.com/mozilla-b2g/gaia/commit/8b2ff150687e58845ae3dd44e729c8f1a0532a16

Comment 33

[[e-mail], [e-mail]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6554

Pointer to Github pull-request

Comment 34

[[e-mail], [e-mail]]

Comment on attachment 683978 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6554

We should add preventDefault() for the type="submit" button.
Evelyn, could you help me to review the pr?
Thank you.

Comment 35

[[e-mail], [e-mail]]

Comment on attachment 682079 [details]
Pull request 6438 - fix bluetooth

We should add the patch https://github.com/mozilla-b2g/gaia/pull/6554 with pr 6438.
It will work fine.
r=me

Comment 36

[Evelyn Hung]

Comment on attachment 683978 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6554

r=me, looks good.

Comment 37

[Alexandre Poirot [:ochameau]]

Attachment 682079 [details] landed:
https://github.com/mozilla-b2g/gaia/commit/aa6d1fff35cd8f91d6921c0870c98842fc9ce213

Attachment 683978 [details] landed:
https://github.com/mozilla-b2g/gaia/commit/a74981cafa2f325f74cd7cdcf646bb5a3a48eb9f

Comment 38

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6437 - fix FTU

Yet another rebase.
That's the last CSP fix before enabling it by default.

Comment 39

[Alexandre Poirot [:ochameau]]

Attachment 686327 [details] landed:
https://github.com/mozilla-b2g/gaia/commit/4f5f66d6a6583402259133f189e9cda83cddb4a0

Comment 40

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 6715 - fix test-agent

One last, the test-agent app used to run gaia unit tests.

Comment 41

[Alexandre Poirot [:ochameau]]

So with this last pull request applied, only some test applications still fail on CSP violation: uitest and test receivers.

Tested: FTU, sim dialog, all apps from apps folder (no deep test except dialer/sms), games.

Comment 42

[Alexandre Poirot [:ochameau]]

Attachment 686582 [details] landed:
https://github.com/mozilla-b2g/gaia/commit/2182d0a601bc194452284af65e3787331ded7e07