Closed Bug 1192840 Opened 5 years ago Closed 4 years ago

csp-report does not set content-type header to application/csp-report

Categories

(Core :: DOM: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bkelly, Assigned: freddy)

References

()

Details

(Whiteboard: [domsecurity-backlog])

Attachments

(2 files, 2 obsolete files)

Zack Tollman wrote an excellent post exploring how the different browsers sense csp-reports:

  https://www.tollmanz.com/content-security-policy-report-samples/

He writes:

  "The current Blink CSP implementation is nearly perfect as far as I can tell, which means Chrome and Opera are delivering excellent CSP reports. Webkit and Gecko are all over the place with different variations of reports."

For example he notes we do not send "application/csp-report" for the content-type header.  I believe there are other incompatibilities in the post as well.  We should probably hang these as separate bugs off here.
Hi @bkelly,

Thanks for filing this ticket!

I have created 3 specific tickets so far:

* Fragment-less blocked-uris: https://bugzilla.mozilla.org/show_bug.cgi?id=1192681
* Data only blocked-uris: https://bugzilla.mozilla.org/show_bug.cgi?id=1192682
* Add effective-directive and status-code: https://bugzilla.mozilla.org/show_bug.cgi?id=1192684

For this ticket, perhaps we just focus on the "application/csp-report" issue.
Sounds good.  Thanks for filing bugs!
Summary: investigate where csp-report deviates from spec → csp-report does not set content-type header to application/csp-report
This should solve the problem.
Ben, can you recommend a person to review Matt's patch? Thanks!
Flags: needinfo?(bkelly)
Comment on attachment 8672797 [details] [diff] [review]
Use application/csp-report instead of application/json

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

Christop, can you review this CSP patch?
Attachment #8672797 - Flags: review?(mozilla)
Flags: needinfo?(bkelly)
Comment on attachment 8672797 [details] [diff] [review]
Use application/csp-report instead of application/json

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

Thanks for fixing; CSP spec [1] explicitly states '... with a Content-Type header field of application/csp-report'.
r=me

[1] http://www.w3.org/TR/CSP11/#violation-reports
Attachment #8672797 - Flags: review?(mozilla) → review+
Thanks guys, is there anything else I need to do here to get this landed?
Hey Matt, the first thing will be to format the patch and commit message according to patch guidelines:

<https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F>

See also <https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#I%27m_all_used_to_Git_but_how_can_I_provide_Mercurial-ready_patches> if you're using a git repo.

So something like "Bug 1192840. Fix CSP report content-type. r=ckerschb" for you commit message (or similar). 

Once you upload that here, I can help push the patch to our try servers to make sure it doesn't break any tests.
Attachment #8672797 - Attachment is obsolete: true
Thanks Mike, I've uploaded a new patch that should be formatted correctly. :)
Great! 

You can check up on this link in a while (hour or so, dunno) and if there aren't any failures caused by the patchset you can set the "checkin-needed" keyword: 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=477c3b8aa3f4
Hey Matt, none of the failures from try run look relevant to your change, I'd say it's safe to request checkin-needed.

For extra credit, you could tweak the following test to check the expected content-type of the incoming report: 

https://dxr.mozilla.org/mozilla-central/source/dom/security/test/unit/test_csp_reports.js

(Or file a follow-up bug and write a patch for that).
Flags: needinfo?(matt)
(In reply to Mike Taylor [:miketaylr] (PTO March 10 - 21) from comment #12)
> Hey Matt, none of the failures from try run look relevant to your change,
> I'd say it's safe to request checkin-needed.
> 
> For extra credit, you could tweak the following test to check the expected
> content-type of the incoming report: 
> 
> https://dxr.mozilla.org/mozilla-central/source/dom/security/test/unit/
> test_csp_reports.js
> 
> (Or file a follow-up bug and write a patch for that).

Matt, any chance you wanna tweak one of the tests to check for the header? If not, I'll find someone to finish up that work. Please let me know.
Whiteboard: [domsecurity-backlog]
Assignee: nobody → fbraun
Status: NEW → ASSIGNED
Comment on attachment 8739404 [details]
MozReview Request: Bug 1192840 - fix tests to expect correct csp report content-type r?ckerschb

https://reviewboard.mozilla.org/r/45259/#review42247

LGTM, r=me
Attachment #8739404 - Flags: review+
Comment on attachment 8739404 [details]
MozReview Request: Bug 1192840 - fix tests to expect correct csp report content-type r?ckerschb

https://reviewboard.mozilla.org/r/45259/#review42253
Attachment #8739404 - Flags: review+
Comment on attachment 8739404 [details]
MozReview Request: Bug 1192840 - fix tests to expect correct csp report content-type r?ckerschb

MozReview still gives me trouble when setting the right flags - anyway - thanks for fixing.
Attachment #8739404 - Flags: review?(ckerschb) → review+
try-run successful, Please check in both patches.
Flags: needinfo?(matt)
Keywords: checkin-needed
Backed out for failure in modified test_csp_reports.js.

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/01eb273e37b6043dd197a539f6d7f99c9eee8ae2

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=0c21f872515b2b49c53bd22b12d8a3e472dbd0b2
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=25724626&repo=mozilla-inbound

10:01:59     INFO -  TEST-START | dom/security/test/unit/test_csp_reports.js
10:01:59  WARNING -  TEST-UNEXPECTED-FAIL | dom/security/test/unit/test_csp_reports.js | xpcshell return code: 0
10:01:59     INFO -  TEST-INFO took 587ms
10:01:59     INFO -  >>>>>>>
10:01:59     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
10:01:59     INFO -  (xpcshell/head.js) | test pending (2)
10:01:59     INFO -  PROCESS | 15412 | Created test 0 : default-src 'none'; report-uri http://localhost:50295/test0
10:01:59     INFO -  TEST-PASS | dom/security/test/unit/test_csp_reports.js | run_test/< - [run_test/< : 120] true == true
10:01:59     INFO -  (xpcshell/head.js) | test pending (3)
10:01:59     INFO -  PROCESS | 15412 | Created test 1 : default-src 'none'; report-uri http://localhost:50295/test1
10:01:59     INFO -  TEST-PASS | dom/security/test/unit/test_csp_reports.js | run_test/< - [run_test/< : 130] true == true
10:01:59     INFO -  TEST-PASS | dom/security/test/unit/test_csp_reports.js | run_test/< - [run_test/< : 132] true == true
10:01:59     INFO -  (xpcshell/head.js) | test pending (4)
10:01:59     INFO -  PROCESS | 15412 | Created test 2 : default-src 'none'; report-uri http://localhost:50295/test2
10:01:59     INFO -  (xpcshell/head.js) | test pending (5)
10:01:59     INFO -  PROCESS | 15412 | Created test 3 : default-src 'none'; report-uri http://localhost:50295/test3
10:01:59     INFO -  TEST-PASS | dom/security/test/unit/test_csp_reports.js | run_test/< - [run_test/< : 161] true == true
10:01:59     INFO -  (xpcshell/head.js) | test pending (6)
10:01:59     INFO -  PROCESS | 15412 | Created test 4 : default-src 'none'; report-uri http://localhost:50295/test4
10:01:59     INFO -  TEST-PASS | dom/security/test/unit/test_csp_reports.js | run_test/< - [run_test/< : 171] true == true
10:01:59     INFO -  TEST-PASS | dom/security/test/unit/test_csp_reports.js | run_test/< - [run_test/< : 173] true == true
10:01:59     INFO -  (xpcshell/head.js) | test pending (7)
10:01:59     INFO -  PROCESS | 15412 | Created test 5 : default-src 'none'; report-uri http://localhost:50295/test5
10:01:59     INFO -  (xpcshell/head.js) | test pending (8)
10:01:59     INFO -  PROCESS | 15412 | Created test 6 : default-src 'none'; report-uri http://localhost:50295/test6
10:01:59     INFO -  (xpcshell/head.js) | test pending (9)
10:01:59     INFO -  PROCESS | 15412 | Created test 7 : default-src 'none'; report-uri http://localhost:50295/test7
10:01:59     INFO -  (xpcshell/head.js) | test MAIN run_test finished (9)
10:01:59     INFO -  running event loop
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings blocked the loading of a resource at self ("default-src 'none'")." {file: "http://localhost:50295/foo/self" line: 0}]"
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings blocked the loading of a resource at self ("default-src 'none'")." {file: "http://localhost:50295/foo/self" line: 1 column: 0 source: "script sample"}]"
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings blocked the loading of a resource at http://blocked.test/foo.js ("default-src 'none'")."]"
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings observed the loading of a resource at self ("default-src 'none'"). A CSP report is being sent." {file: "http://localhost:50295/foo/self" line: 0}]"
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings observed the loading of a resource at self ("default-src 'none'"). A CSP report is being sent." {file: "http://localhost:50295/foo/self" line: 4 column: 0 source: "script sample"}]"
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings blocked the loading of a resource at data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg== ("default-src 'none'")."]"
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings blocked the loading of a resource at intent://mymaps.com/maps?um=1&ie=UTF-8&fb=1&sll ("default-src 'none'")."]"
10:01:59     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page's settings blocked the loading of a resource at http://localhost:50295/foo/self/foo.js#bar ("default-src 'none'")."]"
10:01:59     INFO -  violation report should have the 'application/csp-report' content-type, when in fact it is application/json
10:01:59     INFO -  /builds/slave/test/build/tests/xpcshell/tests/dom/security/test/unit/test_csp_reports.js:makeReportHandler/<:40
10:01:59     INFO -  resource://testing-common/httpd.js:ServerHandler.prototype.handleResponse:2374
10:01:59     INFO -  resource://testing-common/httpd.js:Connection.prototype.process:1225
10:01:59     INFO -  resource://testing-common/httpd.js:RequestReader.prototype._handleResponse:1679
10:01:59     INFO -  resource://testing-common/httpd.js:RequestReader.prototype._processBody:1527
10:01:59     INFO -  resource://testing-common/httpd.js:RequestReader.prototype.onInputStreamReady:1395
10:01:59     INFO -  /builds/slave/test/build/tests/xpcshell/head.js:_do_main:209
10:01:59     INFO -  /builds/slave/test/build/tests/xpcshell/head.js:_execute_test:533
10:01:59     INFO -  -e:null:1
10:01:59     INFO -  exiting test
<several repeats>
10:02:00     INFO -  PROCESS | 15412 | !!! error running onStopped callback: TypeError: callback is not a function
10:02:00     INFO -  <<<<<<<
Flags: needinfo?(fbraun)
has conflicts like:

adding 1192840 to series file
renamed 1192840 -> file_1192840.txt
applying file_1192840.txt
patching file dom/security/nsCSPContext.cpp
Hunk #1 FAILED at 878
1 out of 1 hunks FAILED -- saving rejects to file dom/security/nsCSPContext.cpp.rej
can you take a look ? thanks
rebasing patch
Attachment #8675072 - Attachment is obsolete: true
Flags: needinfo?(fbraun)
Attachment #8741323 - Attachment is patch: true
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.