Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Bug 493857 (CSP)

Implement Content Security Policy

ASSIGNED
Unassigned

Status

()

Core
Security
P1
enhancement
ASSIGNED
8 years ago
2 months ago

People

(Reporter: bsterne, Unassigned)

Tracking

(Depends on: 19 bugs, Blocks: 3 bugs, {doc-bug-filed})

Trunk
doc-bug-filed
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 -
wanted1.9.2 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 14 obsolete attachments)

18.61 KB, text/plain
Details
15.42 KB, application/x-javascript
Details
(Reporter)

Description

8 years ago
Implement the specification for Content Security Policy to mitigate code injection attacks.

Background information:
https://wiki.mozilla.org/Security/CSP
http://people.mozilla.org/~bsterne/content-security-policy/

Updated

8 years ago
Blocks: 485488
Umm... this part really worries me:
https://wiki.mozilla.org/Security/CSP/Spec#No_inline_JavaScript_will_execute

Specifically, breaking the onclick event listener and the <a
href="javascript:foo()"></a> functionality.  The onclick attribute is part of
the HTML 4.01 specification, and onclick has worked forever (DOM Level 0).

Also, your specification states: "The contents of internal <script> nodes" will
be restricted.  What's an internal script node (as opposed to an external one)?

Finally, under "Activation and Enforcement", I wonder what would happen were a
malicious script to do its dirty work, and then forcibly insert the meta tag.
(I'm not saying this is a problem - I'm just curious.)
Created attachment 378464 [details]
CSP data structures & policy methods

First rough impl. of the CSP data structures.  Eventually will turn into a Javascript module, thus the .jsm extension.  Does not fetch external policies yet.
Created attachment 378466 [details]
unit tests for the CSPUtils.jsm objects

Here are some unit tests for the CSPUtils.jsm data structures and methods.  They were kind of hacked out quickly and we could surely use more cases to test.  These are meant to be stand-alone tests, and don't rely on any other source code except the CSPUtils.

To run them, install some JS shell or interpreter of some sort (I used rhino) and load this TestCSPUtils.js file in the interpreter.  Make sure the other (CSPUtils.jsm) file is in the same directory since it's referenced from the TestCSPUtils.js file.
Attachment #378464 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Comment 4

8 years ago
(In reply to comment #1)
> Specifically, breaking the onclick event listener and the <a
> href="javascript:foo()"></a> functionality.  The onclick attribute is part of
> the HTML 4.01 specification, and onclick has worked forever (DOM Level 0).

It is, however, an opt-in mechanism.  We aren't breaking DOM across the board, only for sites that choose to use CSP.  You have highlighted, though, the biggest challenge for sites wanting to adopt CSP.  We plan to provide documentation and possibly tools to sites to help ease their transition.

> Also, your specification states: "The contents of internal <script> nodes" will
> be restricted.  What's an internal script node (as opposed to an external one)?

An internal <script> node is one in the top-level protected document.  An external <script> node is one in an externally-linked JavaScript file.

> Finally, under "Activation and Enforcement", I wonder what would happen were a
> malicious script to do its dirty work, and then forcibly insert the meta tag.
> (I'm not saying this is a problem - I'm just curious.)

Since the policy can only place additional "restrictions" on a page, we feel the risk of injected policy is very low.  For instance, and injected policy could prevent a resource's images or script files from loading, but couldn't de-escalate the security policy for a resource below the defaults (same-origin, etc.).
(In reply to comment #1)
> Specifically, breaking the onclick event listener and the <a
> href="javascript:foo()"></a> functionality.  The onclick attribute is part of
> the HTML 4.01 specification, and onclick has worked forever (DOM Level 0).

The highest priority goal of CSP is to help sites eliminate XSS, a problem which has been found to affect something like 80-90% of even the best-run sites. Funny you should mentions the onclick attribute as that one specifically is a popular one to abuse. Whether the burden of rewriting your site to the supported safe subset of HTML is worth it depends on how valuable the contents of your site are.

Note that we are not eliminating event handlers, just the ability to specify them inline. AddEventListener() will still work, as will setting the .click property of a DOM node. This is a little cumbersome, but there are already sites that do this for some of their content.

CSP is a gamble, it could be that the hurdle will turn out to be too high. But if we can get authors over that hurdle we can promise them a safer site.
Duplicate of this bug: 463948
(Reporter)

Comment 7

8 years ago
Created attachment 382041 [details] [diff] [review]
CSP work in progress

Adds CSP object as part of nsDocument with stub for RefinePolicy.  Adds nsIContentPolicy which locates the CSP object on the document when ShouldLoad is called.
Created attachment 382990 [details] [diff] [review]
CSP Work in Progress (v2)

CSP work in progress (updated)

Ties CSP policy data structure objects and enforcement hooks all together.  With this patch, policies are loaded from the HTTP response header and parsed, then enforced.

This patch also supports policy-uri (though synchronously, and probably with a bit of UI lag).
Attachment #382041 - Attachment is obsolete: true
Created attachment 384026 [details] [diff] [review]
CSP Work in Progress (v3)

This patch is an upgrade from the v2 patch and includes a rough implementation of Policy Violation reporting (via asynchronous XHR POST) and frame-ancestor checking.
Attachment #382990 - Attachment is obsolete: true
Alias: CSP
Severity: normal → enhancement
Created attachment 387098 [details] [diff] [review]
Incremental upgrade from v3

This patch includes:
- parsing the "inline" and "eval" keywords in the script-src directive
- suppresses cookies from requests sent for policy URI fetch and violation report sending
- converted hand-rolled unit tests to xpcshell tests (make -C caps/test xpcshell-tests)
Attachment #384026 - Attachment is obsolete: true
(Reporter)

Comment 11

8 years ago
Created attachment 387470 [details] [diff] [review]
no event listeners for event-handling attributes

Spoke with sicking and jst.  Returning early from nsEventListenerManager::AddScriptEventListener prevents event listeners from being added due to on<event> attributes.  I posted a test for this behavior here:
http://hackmill.com/csp/tests/event-handling-attrs.php
(Reporter)

Updated

8 years ago
Attachment #387470 - Attachment is patch: true
Created attachment 387569 [details] [diff] [review]
CSP Work in Progress (v3.6)

Updated patch to fix some URI scheme parsing issues.
Attachment #387098 - Attachment is obsolete: true
Created attachment 388837 [details] [diff] [review]
CSP Work in Progress (v4)

Adds "security.csp.enable" pref (default true) that can be set to false to disable CSP globally.  Also merges event listener patch with main WIP patch.
Attachment #387470 - Attachment is obsolete: true
Attachment #387569 - Attachment is obsolete: true
If this goes in 1.9.2 at all I think it needs to block the alpha.
Flags: blocking1.9.2+
Priority: -- → P1
Created attachment 389817 [details] [diff] [review]
CSP Work in Progress (v4.1)

Adds:
- Hooks into nsJSTimeoutHandler to turn off setTimeout() and setInterval() for string arguments (unless of course the CSP allows Eval stuff).
- Cleans out printf() statements, replacing them with PR_LOGGING stuff.  Still need to update the .js files for this, but can probably be done by changing CSPdebug, CSPError and CSPWarning.
Attachment #388837 - Attachment is obsolete: true
Comment on attachment 389817 [details] [diff] [review]
CSP Work in Progress (v4.1)

>+ * The Initial Developer of the Original Code is
>+ *   Sid Stamm <sid@mozilla.com>

No, the initial developer is MoCo or MoFo. You should just list yourself under contributors.
Created attachment 390366 [details] [diff] [review]
CSP Work in Progress (v5)

This patch includes miscellaneous fixes and rewriting of the parser.  Unit tests for CSPUtils.jsm have been heavily modified for correctness and to include "self" definitions where necessary.  (e.g., can't create a source "a.com" without knowing the scheme and port of 'self')  Highlights in this update from patch v4.1:
- Fixed license "initial developer" string (thanks reed, didn't know this was the common practice, now I do)
- 'self' keyword can only be stand-alone, not used used as anything other than scheme, host *and* port
- keywords are now quoted as per spec (e.g., 'none')
- host-less schemes (like data: and javascript:) are valid sources
- application/xml sent as MIME type for violation reports
- font-src and xml-src directives are now supported
- "report-only" variation based on the HTTP header name (https://wiki.mozilla.org/Security/CSP/Spec#Report-Only_mode)
Attachment #389817 - Attachment is obsolete: true
Per the platform meeting today, this is going to miss 1.9.2 and we don't want to take it after this week's beta, so this will have to wait for 1.9.3.
Flags: wanted1.9.2-
Flags: blocking1.9.2-
Flags: blocking1.9.2+
Created attachment 394402 [details] [diff] [review]
CSP Work in Progress (v5.1)

This patch includes miscellaneous fixes from v5.  Unit tests for CSPUtils.jsm have been fixed to support quoted keywords ('self', etc) and for correct behavior with source lists containing unidentifiable tokens.
- CSP parser enforces same-origin (scheme/host/port) for policy URI fetching
- CSP parser enforces ETLD+1 (public suffix + 1) matching for report URIs
- Variety of other minor fixes.
Attachment #390366 - Attachment is obsolete: true
Created attachment 394403 [details] [diff] [review]
CSP Work in Progress (v5.1 - repaired)

Oops, previous attachment for v5.1 was incomplete.  This attachment fixes that.
Attachment #394402 - Attachment is obsolete: true
Created attachment 394589 [details] [diff] [review]
CSP Work in Progress (v5.2)

- fixed a few policy initialization bugs ('inline' and 'eval' were ignored due to the way the policy was intersected with "allow *" initially)  'inline' and 'eval' are now digested properly
- added checking on link clicks to block javascript: URIs when 'inline' is not specified in the policy
Attachment #394403 - Attachment is obsolete: true
(Reporter)

Comment 22

8 years ago
Created attachment 395460 [details] [diff] [review]
Redirect handling PoC - applies to v5.2

Here is a proof of concept for a workaround to the redirects-don't-call-into-Content-Policy problem.  This patch only implements the restrictions for image loading, so a lot of other code would need to be added to handle all the other types of loads.

The basic idea is to let all new channel creation go through a helper function, NewChannelIfPolicyOK, which works like NS_NewChannel but also takes a CSP object and a load type.  The CSP and load type are added to the initial channel's property bag when it's created.  These can be propagated forward as a channel redirects and can be used at each hop to decide whether or not to allow the redirect.
Created attachment 396000 [details] [diff] [review]
CSP Work in Progress (v5.3)

This new version is a rewrite:
- Completely rewrote the CSP parser ... it's now not as fragile and easier to read
- Reviewed and revised unit tests so they reflect the spec
Attachment #394589 - Attachment is obsolete: true
Depends on: 515433
Depends on: 515437
Depends on: 515442
Depends on: 515443
Depends on: 515458
Depends on: 515460
(Reporter)

Updated

8 years ago
Attachment #396000 - Attachment is obsolete: true
Patches to deploy CSP are now split out into more bite-size pieces for the bits of functionality involved with what we call "CSP."  As a result, the patches in this bug are invalid: see the bugs this one depends on.
Status: NEW → ASSIGNED

Comment 25

8 years ago
I think someone should try posting about this at http://connect.microsoft.com/feedback/default.aspx?SiteID=136

With some luck this might make into ie too fairly soon.
Depends on: 490790
Related bug in webkit:
https://bugs.webkit.org/show_bug.cgi?id=30081
Keywords: dev-doc-needed
(Reporter)

Updated

8 years ago
Depends on: 544061

Updated

8 years ago
Blocks: 301375

Updated

8 years ago
Depends on: 548193
(Reporter)

Updated

8 years ago
Depends on: 548949
(Reporter)

Updated

8 years ago
Depends on: 548984

Updated

8 years ago
Depends on: 550442

Updated

8 years ago
Blocks: 457011
(Reporter)

Updated

7 years ago
Depends on: 552523
(Reporter)

Updated

7 years ago
Depends on: 556625
Depends on: 558429
Depends on: 558431
(Reporter)

Comment 27

7 years ago
Comment on attachment 395460 [details] [diff] [review]
Redirect handling PoC - applies to v5.2

Redirects in CSP are handled in bug 515797, bug 523239, and bug 515460.
Attachment #395460 - Attachment is obsolete: true
(Reporter)

Updated

7 years ago
Depends on: 560574

Updated

7 years ago
Depends on: 561051
(Reporter)

Updated

7 years ago
Depends on: 570017
Blocks: 569993
Depends on: 570505
We have initial documentation here:

https://developer.mozilla.org/en/Introducing_Content_Security_Policy

What more is needed?
Depends on: 576200

Updated

7 years ago
Depends on: 578075
Depends on: 594446
(Reporter)

Updated

7 years ago
Depends on: 600584
(Reporter)

Updated

7 years ago
Depends on: 602063
Lots of work done during the doc sprint today on CSP documentation. Would someone like to review it?

https://developer.mozilla.org/en/Security/CSP/Introducing_Content_Security_Policy
https://developer.mozilla.org/en/Security/CSP/Default_CSP_restrictions
https://developer.mozilla.org/en/Security/CSP/CSP_policy_directives
https://developer.mozilla.org/en/Security/CSP/Using_Content_Security_Policy

I'll be writing about CSP reports tomorrow.
(Reporter)

Comment 30

7 years ago
Awesome, sheppy.  I'll review the docs tomorrow.
I've added the doc on the policy violation reports here:

https://developer.mozilla.org/en/Security/CSP/Using_CSP_violation_reports

Paul has given the docs a once-over, and I've emailed a few questions to bsterne. Other than that, and bsterne's review, these are complete.
(Reporter)

Updated

7 years ago
Depends on: 604177
(Reporter)

Updated

7 years ago
Depends on: 606039
Depends on: 607067
Depends on: 607069
(Reporter)

Updated

7 years ago
Depends on: 608131
Depends on: 615707
Depends on: 615708
Depends on: 615711

Updated

7 years ago
No longer depends on: 615707
(Reporter)

Updated

7 years ago
Depends on: 617195
(Reporter)

Updated

7 years ago
Depends on: 631040
(Reporter)

Updated

7 years ago
Depends on: 634773
(Reporter)

Updated

7 years ago
Depends on: 634778
Depends on: 609748
(Reporter)

Updated

7 years ago
Depends on: 638320
(Reporter)

Updated

7 years ago
Depends on: 639533
(Reporter)

Updated

6 years ago
Depends on: 650386

Updated

6 years ago
Depends on: 671389
Depends on: 672961

Updated

6 years ago
Depends on: 673645
Depends on: 737064

Updated

5 years ago
Depends on: 746978
Depends on: 612391
Depends on: 663566
Depends on: 766536
No longer depends on: 766536
Depends on: 766536
Depends on: 766569

Updated

5 years ago
Depends on: 784315
Depends on: 779918

Updated

5 years ago
Depends on: 663570
Depends on: 788337
Depends on: 792214
Depends on: 792542
Depends on: 687086

Updated

5 years ago
Depends on: 802872
Depends on: 802905
Depends on: 805929
Depends on: 808292
Depends on: 809982
Depends on: 809983
Depends on: 824652

Updated

5 years ago
Depends on: 702176

Updated

5 years ago
Depends on: 832398
Depends on: 801783

Updated

5 years ago
Depends on: 832558

Updated

5 years ago
Depends on: 663567

Updated

5 years ago
Depends on: 792161
Depends on: 837682
Keywords: dev-doc-needed → doc-bug-filed

Updated

5 years ago
Depends on: 843311

Updated

4 years ago
Depends on: 846978

Updated

4 years ago
Depends on: 847081

Updated

4 years ago
Depends on: 847067

Updated

4 years ago
Component: DOM: Core & HTML → Security

Updated

4 years ago
Depends on: 529697

Updated

4 years ago
Depends on: 855326

Updated

4 years ago
Depends on: 858789

Updated

4 years ago
Depends on: 858787

Updated

4 years ago
Depends on: 858780

Updated

4 years ago
Depends on: 842657
Depends on: 864675

Updated

4 years ago
Depends on: 866522
Depends on: 841402

Updated

4 years ago
Depends on: 722547
No longer depends on: 824652
Depends on: 826805
Depends on: 886164
Blocks: 836922
No longer blocks: 836922
Depends on: 836922
Blocks: 898190
Depends on: 741019
Depends on: 909029
Hi Bradon. I want to test this feature but don't really know where to start from. Can you please provide any guildline please so I can create a test plan and some test cases for sign-off.
Flags: needinfo?(brandon)
Hi Bradon. I want to test this feature but don't really know where to start from. Can you please provide any guildline so I can create a test plan and some test cases for sign-off.
Mihai: Brandon is no longer actively working on this.

Here's the specification for the feature: http://www.w3.org/TR/CSP/

We have many tests for this in our mochitest suite already.  This is a metabug, please look at all the blocking bugs to see the real work.
Assignee: brandon → nobody
Flags: needinfo?(brandon)
Thanks Sid. If there are any test plan or additional test cases required please let me know.
QA Contact: mihai.morar
I ran on local machines following Run Tests and got 23 failures. Results: (164/187)

Test Runs:
http://csptesting.herokuapp.com/

Failures: Same failures for Windows 7 x64, Ubuntu 13.04 x86 and Mac OS 10.8 on FF24b7 BuildID: 20130829135643

13	Style in data-uri allowed
15	Use inline styles
17	Use inline style attributes
61	Style wants image, and allowed by img-src
78	Load embed from default-src 'self'
80	Load embed from object-src 'self'
83	Load embed from object-src with redirect from allowed to allowed
84	Load embed from default-src csptesting.herokuapp.com
89	Load embed from object-src with redirect from allowed to allowed
106	Load font from default-src 'self'
108	Load font from font-src 'self'
111	Load font from font-src with redirect from allowed to allowed
112	Load font from default-src csptesting.herokuapp.com
114	Load font from font-src csptesting.herokuapp.com
117	Load font from font-src with redirect from allowed to allowed
150	Load xhr from connect-src with redirect from allowed to allowed
156	Load xhr from connect-src with redirect from allowed to allowed
165	Load WebSockets from default-src ws://csptesting.herokuapp.com
167	Load WebSockets from connect-src ws://csptesting.herokuapp.com
171	SVG - scripting event handler
183	Sandbox
185	Sandbox

Any idea why all these test are failling?
Flags: needinfo?(sstamm)
There could be many reasons.  See all the bugs blocking this (and blocking the bug aliased to csp-w3c-1.0)?  Some of those might explain various failures.

Also, there could possibly be bugs in the tests. Not sure without digging into them more.

Garrett: you've probably looked at this stuff more recently than me, do you have any insight or should we file a bug to follow up with all these test failures?
Flags: needinfo?(sstamm) → needinfo?(grobinson)
Depends on: 915065
I've triaged all of the test failures from http://csptesting.herokuapp.com/. No new bugs need to be filed :)

13-117 all failed because the iframes used to load the test requests had 'style="display:none"', to avoid cluttering up the page. However, FF does not compute style (or perform certain other rendering tasks) on elements that are/are children of 'display:none'. These test failures were false negatives. They were addressed (swiftly!) by eoftedal in https://github.com/eoftedal/csp-testing/issues/6

150-167 were all failing because they were performing cross-domain XHR without CORS properly configured (these tests failed on Chrome as well). This was a bug in the test suite, and was addressed by eoftedal in https://github.com/eoftedal/csp-testing/issues/7

171 failed because it tests the execution of script in an onload event handler in a <g> child of an <svg> element. FF's SVG code purposely does not implement the "load event dispatched on every element" behavior for performance reasons. For context (thanks dholbert!), see

* https://bugzilla.mozilla.org/show_bug.cgi?id=552938#c27
* https://bugzilla.mozilla.org/show_bug.cgi?id=639950

Finally, 183 and 185 fail because we have not yet implemented the sandbox directive (optional in 1.0). See Bug 671389.

eoftedal quickly fixed the issues in the test suite. I now see 180/187 passing tests, 3 of which are 171, 183, and 185. I will further triage the remaining 4 tests.
Flags: needinfo?(grobinson)
Depends on: 903080
Depends on: 916054
Depends on: 918397
Depends on: 919209
Depends on: 918724
Blocks: 802882

Updated

4 years ago
Depends on: 925186

Comment 39

4 years ago
(In reply to Garrett Robinson [:grobinson] from comment #38)
> I've triaged all of the test failures from http://csptesting.herokuapp.com/.

I see 180 test passing in 25b6, but Nightly 27.0a1 (2013-10-11) only passes 142/187! Which version did you try this with? What's the reason for the regression?
Flags: needinfo?(grobinson)

Comment 40

4 years ago
(In reply to Florian Bender from comment #39)
> (In reply to Garrett Robinson [:grobinson] from comment #38)
> > I've triaged all of the test failures from http://csptesting.herokuapp.com/.
> 
> I see 180 test passing in 25b6, but Nightly 27.0a1 (2013-10-11) only passes
> 142/187! Which version did you try this with? What's the reason for the
> regression?

Could be bug 925186 or could be other regressions from bug 836922 possibly.
I just built mozilla-central, built and confirmed 142/187.  When I applied the fixes for bug 925186 and bug 924708 I get 180/187.  I probably regressed all the things with multipolicy support, but looks like we found most of 'em.
Flags: needinfo?(grobinson)
Thanks Sid for your feedback. I get Results: (142/187), same results as you got in Comment 41. I had used Windows 7 x64 and Latest Aurora 26 for testing. 

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 
BuildID: 20131014004003
Garrett, are there any additional tests required for testing this feature on it's enought to confirm that the fixed bugs from "Depends On" section are verified?

Updated

4 years ago
Depends on: 928719
No longer depends on: 928719

Updated

4 years ago
Depends on: 929653
(In reply to Mihai Morar, QA (:MihaiMorar) from comment #43)
> Garrett, are there any additional tests required for testing this feature on
> it's enought to confirm that the fixed bugs from "Depends On" section are
> verified?

All of the fixed bugs from "depends on" should have accompanying tests. Most of these are in content/base/test/csp. There are some tests related to Web Console logging in browser/devtools/webconsole/test.
Depends on: 933413
Depends on: 935690
Depends on: 938652
Depends on: 942345
Depends on: 925004
Blocks: 949533
Depends on: 915824
Depends on: 951457
Depends on: 587377
Depends on: 957980
Depends on: 963668
Depends on: 911547
Depends on: 605900
Depends on: 965273

Updated

4 years ago
Depends on: 883975

Updated

4 years ago
Depends on: 965727
Depends on: 949706
Depends on: 968586
Depends on: 984808
Depends on: 991972
Depends on: 964276
No longer blocks: 949533
Depends on: 949533
Depends on: 991474
Depends on: 991468
Depends on: 994782
Depends on: 994872
Depends on: 1000945
Blocks: 1011098
Depends on: 1014545
Depends on: 1012592
Depends on: 1021970
Blocks: 1024557
Blocks: 1024562
Depends on: 1021669
Depends on: 1027833
Depends on: 1027868
Depends on: 1028490
Depends on: 993477
Depends on: 1030936

Updated

3 years ago
Depends on: 1011841
Depends on: 1032303
Depends on: 1031658
Depends on: 1026520
Depends on: 1033675

Updated

3 years ago
Duplicate of this bug: 361915

Updated

3 years ago
No longer blocks: 390910
Duplicate of this bug: 390910
Depends on: 1034156
Depends on: 1033423
Depends on: 1005225

Updated

3 years ago
Depends on: 1048048
Depends on: 908933
Depends on: 1057376
Depends on: 1063021

Updated

3 years ago
Depends on: 1037768
Depends on: 1086999
Depends on: 1089746
Blocks: 1089912
Depends on: 1100630

Updated

3 years ago
Depends on: 1106775
Depends on: 1112782
Depends on: 878608
Depends on: 1037335
Depends on: 1045893
Depends on: 1089255
Depends on: 1052887
Depends on: 1134084
Depends on: 1199977
Depends on: 1231788
Depends on: 1192684
Depends on: 1265316
No longer blocks: 1024562
No longer blocks: 1024557
You need to log in before you can comment on or make changes to this bug.