Closed
Bug 1192840
Opened 9 years ago
Closed 8 years ago
csp-report does not set content-type header to application/csp-report
Categories
(Core :: DOM: Security, defect)
Core
DOM: Security
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.
Reporter | ||
Comment 2•9 years ago
|
||
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
Comment 3•9 years ago
|
||
This should solve the problem.
Comment 4•9 years ago
|
||
Ben, can you recommend a person to review Matt's patch? Thanks!
Flags: needinfo?(bkelly)
Reporter | ||
Comment 5•9 years ago
|
||
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)
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(bkelly)
Comment 6•9 years ago
|
||
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+
Comment 7•9 years ago
|
||
Thanks guys, is there anything else I need to do here to get this landed?
Comment 8•9 years ago
|
||
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.
Comment 9•9 years ago
|
||
Attachment #8672797 -
Attachment is obsolete: true
Comment 10•9 years ago
|
||
Thanks Mike, I've uploaded a new patch that should be formatted correctly. :)
Comment 11•9 years ago
|
||
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
Comment 12•9 years ago
|
||
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)
Comment 13•8 years ago
|
||
(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.
Updated•8 years ago
|
Whiteboard: [domsecurity-backlog]
Assignee | ||
Comment 14•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/45259/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45259/
Attachment #8739404 -
Flags: review?(mozilla)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → fbraun
Status: NEW → ASSIGNED
Comment 15•8 years ago
|
||
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 16•8 years ago
|
||
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 17•8 years ago
|
||
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+
Assignee | ||
Comment 18•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=153438cc97f5
Assignee | ||
Comment 19•8 years ago
|
||
try-run successful, Please check in both patches.
Flags: needinfo?(matt)
Keywords: checkin-needed
Comment 20•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c21f872515b
Keywords: checkin-needed
Comment 21•8 years ago
|
||
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)
Comment 22•8 years ago
|
||
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
Assignee | ||
Comment 23•8 years ago
|
||
rebasing patch
Attachment #8675072 -
Attachment is obsolete: true
Flags: needinfo?(fbraun)
Updated•8 years ago
|
Attachment #8741323 -
Attachment is patch: true
Comment 24•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5028bf34b108 https://hg.mozilla.org/integration/fx-team/rev/30010c0e58af
Assignee | ||
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 25•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5028bf34b108 https://hg.mozilla.org/mozilla-central/rev/30010c0e58af
You need to log in
before you can comment on or make changes to this bug.
Description
•