1848315 - CSP: Provide the effective directive in console error messages

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P3)

Description

[Tom Schuster]

Currently the error message for CSP blocking just contains the "violated directive", e.g. the directive in the website's CSP that actually matched.

For example, in cases like this (bug 1846302):

Content-Security-Policy: The page’s settings blocked the loading of a resource at inline (“default-src”).

It is impossible to know that this was caused by a blocked (inline) style or script.

For comparison Chrome error message for a similar testcase looks like this:

Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'none'". Either the 'unsafe-inline' keyword, a hash ('sha256-F602irze5WpgKxQ4WBmbH1lheK/4Ejv/Vaxh9ajWedQ='), or a nonce ('nonce-...') is required to enable inline execution.

Comment 1

[Tom Schuster]

Attachment: Bug 1848315 - CSP: Remove unused console messages. r?freddyb

Comment 2

[Tom Schuster]

Attachment: Bug 1848315 - CSP: Pass effective directive as an enum to error reporting. r?freddyb

Depends on D186140

Comment 3

[Tom Schuster]

Attachment: Bug 1848315 - CSP: Provide the whole violated directive as a string. r?freddyb

Depends on D186141

Comment 4

[Tom Schuster]

Attachment: Bug 1848315 - CSP: Introduce new and more detailed error messages. r?freddyb

Depends on D186142

Comment 5

[Tom Schuster]

In D186143 I started writing some custom error messages for specific situations, instead of the generic message we had before.

• The page’s settings blocked an inline style from being applied because it violates the following CSP: “%1$S”
• The page’s settings blocked an inline script from being executed because it violates the following CSP: “%1$S”
• The page’s settings blocked a JavaScript eval from being executed because it violates the following CSP: “%1$S” (Missing 'unsafe-eval')
• The page’s settings blocked WebAssembly from being executed because it violates the following CSP: “%1$S” (Missing 'wasm-unsafe-eval' or 'unsafe-eval')

I would love to get some suggestions on how to word these ... I am also wondering if we should directly include the effective directive as well, so e.g. inline style = style-src-elem.

Comment 6

[Frederik Braun [:freddy]]

First of all, this is lovely. These messages are already much better.

(In reply to Tom Schuster (MoCo) from comment #5)

I would love to get some suggestions on how to word these ... I am also wondering if we should directly include the effective directive as well, so e.g. inline style = style-src-elem.

I think this is a fine idea. I wonder if we even need the full CSP. Maybe the developer wants the effective directive only?
E.g.

• The page´s settings blocked an inline style from being appliedbecause it violates the following CSP directive: (effective directive here)

What do you think?

Comment 7

[Tom Schuster]

I wonder if we even need the full CSP. Maybe the developer wants the effective directive only?

We aren't providing the full CSP. Instead it's just the effective directive, but including all its sources. e.g. default-src 'unsafe-inline' 'nonce-foo' http://example.org.

Comment 8

[Tom Schuster]

I've updated the CSP error message and for example they now look like the following.

An onload event handler:

Content-Security-Policy: The page’s settings blocked an event handler (script-src-attr) from being executed because it violates the following CSP: “default-src 'none'”
Source: alert('onload');

An image:

Content-Security-Policy: The page’s settings blocked the loading of a resource (img-src) at http://0.0.0.0:8000/image.png because it violates the following CSP: “default-src 'none'”

A normal script:

Content-Security-Policy: The page’s settings blocked a script (script-src-elem) at http://0.0.0.0:8000/report.js from being executed because it violates the following CSP: “default-src 'none'”

I am still a bit unsure about "Report-Only" messages, but I roughly did this:

Content-Security-Policy: (Report-Only policy) The page’s settings would block the loading of a resource (img-src) at http://0.0.0.0:8000/image.png because it violates the following CSP: “default-src 'none'”

I would love to get some feedback about the general contents and wording of these messages, before I start updating whatever test those changes will break.

Comment 9

[Daniel Veditz [:dveditz]]

I've hated our information-free CSP console messages, and it's especially maddening because I know we have all the information. I always end up having to use the web console to figure things out (only works when the error I care about happens after initial page load)

  window.addEventListener("securitypolicyviolation", (e)=>console.log(e))

a) YES to showing the full directive that was violated, not just the directive name.
b) YES to letting people know what was blocked, not just "a resource"

we might have to truncate the "full directive" some times -- we've seen some really long ones. But let's go with full for now. Actually, do these messages have to go over IPC to make it to the console? We want to make sure the mssage is shorter than whatever the limit is where the parent will kill the child process for spamming :-) I suppose if we've been able to POST CSP violation reports without problems it must be short enough.

I had a similar reaction to Freddy about the "violates the following CSP:" -- it sounds like what follows might be the whole policy (I know you said it isn't, but might confuse others). One option might be to rework the whole frame: "The page's CSP blocked..... because it violates the policy ${}" (or "...violates the following policy:" is fine too). The "page's settings" always seemed kind of vague to me.

I like the specificity of including the literal effective directive, but it's kind of awkward in these sentences. I'm not coming up with a better alternative at the moment... maybe after I get a good sleep.

Comment 10

[Tom Schuster]

I had a similar reaction to Freddy about the "violates the following CSP:" -- it sounds like what follows might be the whole policy (I know you said it isn't, but might confuse others). One option might be to rework the whole frame: "The page's CSP blocked..... because it violates the policy ${}" (or "...violates the following policy:" is fine too). The "page's settings" always seemed kind of vague to me.

Maybe we can just say directive instead instead of policy? I guess the "page's settings" part is mostly meant for less experienced users, to tell them that the page is bad and not Firefox or their own settings?

Comment 11

[Frederik Braun [:freddy]]

Like the suggestion in comment 8 and +1 to saying "directive".

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17f05d5f385d
CSP: Remove unused console messages. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/2e823f3a6f99
CSP: Pass effective directive as an enum to error reporting. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/722163aa402f
CSP: Provide the whole violated directive as a string. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/7ef3e9f17105
CSP: Introduce new and more detailed error messages. r=freddyb,devtools-reviewers

Comment 15

[Sandor Molnar[:smolnar]]

Backed out for causing devtools/client/webconsole/test/browser/browser_webconsole_csp_violation.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/fb8130b825e78b40ce9a7c81208be46e6a4a5131

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser/browser_webconsole_csp_violation.js | Uncaught exception in test bound

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b57c4274e7ab
CSP: Remove unused console messages. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/028e9468f0a3
CSP: Pass effective directive as an enum to error reporting. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/0872a066e6fd
CSP: Provide the whole violated directive as a string. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/b921bb1aee11
CSP: Introduce new and more detailed error messages. r=freddyb,devtools-reviewers

Comment 17

[Iulian Moraru]

Backed out for causing multiple failures.

• Push with failures

• Failure log for dt failures

• Failure log for bc failures

• Backout link

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/502e1fbf7dd6
CSP: Remove unused console messages. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/ba824e4c3705
CSP: Pass effective directive as an enum to error reporting. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/524069fb3173
CSP: Provide the whole violated directive as a string. r=freddyb
https://hg.mozilla.org/integration/autoland/rev/80236b2d1f68
CSP: Introduce new and more detailed error messages. r=freddyb,devtools-reviewers,flod

Comment 19

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/502e1fbf7dd6
https://hg.mozilla.org/mozilla-central/rev/ba824e4c3705
https://hg.mozilla.org/mozilla-central/rev/524069fb3173
https://hg.mozilla.org/mozilla-central/rev/80236b2d1f68