Skip to content

Commit be875ff

Browse files
LukasReschkeLukasReschke
committed
Allow AppFramework applications to specify a custom CSP header
This change allows AppFramework applications to specify a custom CSP header for example when the default policy is too strict. Furthermore this allows us to partially migrate away from CSS and allowed eval() in our JavaScript components. Legacy ownCloud components will still use the previous policy. Application developers can use this as following in their controllers: ```php $response = new TemplateResponse('activity', 'list', []); $cspHelper = new ContentSecurityPolicyHelper(); $cspHelper->addAllowedScriptDomain('www.owncloud.org'); $response->addHeader('Content-Security-Policy', $cspHelper->getPolicy()); return $response; ``` Fixes #11857 which is a pre-requisite for #13458 and #11925
1 parent e2d4b3c commit be875ff

File tree

8 files changed

+493
-22
lines changed

8 files changed

+493
-22
lines changed

‎config/config.sample.php

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -925,15 +925,6 @@
925925
'mssql'
926926
),
927927

928-
/**
929-
* Custom CSP policy, changing this will overwrite the standard policy
930-
*/
931-
'custom_csp_policy' =>
932-
"default-src 'self'; script-src 'self' 'unsafe-eval'; ".
933-
"style-src 'self' 'unsafe-inline'; frame-src *; img-src *; ".
934-
"font-src 'self' data:; media-src *; connect-src *",
935-
936-
937928
/**
938929
* All other config options
939930
*/

‎lib/private/response.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ static public function sendFile($filepath) {
188188
}
189189
}
190190

191-
/*
191+
/**
192192
* This function adds some security related headers to all requests served via base.php
193193
* The implementation of this function has to happen here to ensure that all third-party
194194
* components (e.g. SabreDAV) also benefit from this headers.
@@ -203,17 +203,20 @@ public static function addSecurityHeaders() {
203203
header('X-Frame-Options: Sameorigin'); // Disallow iFraming from other domains
204204
}
205205

206-
// Content Security Policy
207-
// If you change the standard policy, please also change it in config.sample.php
208-
$policy = OC_Config::getValue('custom_csp_policy',
209-
'default-src \'self\'; '
206+
/**
207+
* FIXME: Content Security Policy for legacy ownCloud components. This
208+
* can be removed once \OCP\AppFramework\Http\Response from the AppFramework
209+
* is used everywhere.
210+
* @see \OCP\AppFramework\Http\Response::getHeaders
211+
*/
212+
$policy = 'default-src \'self\'; '
210213
. 'script-src \'self\' \'unsafe-eval\'; '
211214
. 'style-src \'self\' \'unsafe-inline\'; '
212215
. 'frame-src *; '
213216
. 'img-src *; '
214217
. 'font-src \'self\' data:; '
215218
. 'media-src *; '
216-
. 'connect-src *');
219+
. 'connect-src *';
217220
header('Content-Security-Policy:' . $policy);
218221

219222
// https://developers.google.com/webmasters/control-crawl-index/docs/robots_meta_tag

‎lib/public/appframework/http/contentsecuritypolicyhelper.php

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
<?php
2+
/**
3+
* Copyright (c) 2015 Lukas Reschke [email protected]
4+
* This file is licensed under the Affero General Public License version 3 or
5+
* later.
6+
* See the COPYING-README file.
7+
*/
8+
9+
namespace OCP\AppFramework\Http;
10+
11+
use OCP\AppFramework\Http;
12+
13+
/**
14+
* Class ContentSecurityPolicyHelper is a simple helper which allows applications
15+
* to modify the Content-Security-Policy sent by ownCloud. Per default only
16+
* JavaScript, stylesheets, images, fonts, media and connections from the same
17+
* domain ('self') are allowed.
18+
*
19+
* Even if a value gets modified above defaults will still get appended. Please
20+
* notice that ownCloud ships already with sensible defaults and those policies
21+
* should require no modification at all for most use-cases.
22+
*
23+
* @package OCP\AppFramework\Http
24+
*/
25+
class ContentSecurityPolicyHelper {
26+
/** @var bool Whether inline JS snippets are allowed */
27+
private $inlineScriptAllowed = false;
28+
/**
29+
* @var bool Whether eval in JS scripts is allowed
30+
* TODO: Disallow per default
31+
* @link https://github.com/owncloud/core/issues/11925
32+
*/
33+
private $evalScriptAllowed = true;
34+
/** @var array Domains from which scripts can get loaded */
35+
private $allowedScriptDomains = [
36+
'\'self\'',
37+
];
38+
/**
39+
* @var bool Whether inline CSS is allowed
40+
* TODO: Disallow per default
41+
* @link https://github.com/owncloud/core/issues/13458
42+
*/
43+
private $inlineStyleAllowed = true;
44+
/** @var array Domains from which CSS can get loaded */
45+
private $allowedStyleDomains = [
46+
'\'self\'',
47+
];
48+
/** @var array Domains from which images can get loaded */
49+
private $allowedImageDomains = [
50+
'\'self\'',
51+
];
52+
/** @var array Domains to which connections can be done */
53+
private $allowedConnectDomains = [
54+
'\'self\'',
55+
];
56+
/** @var array Domains from which media elements can be loaded */
57+
private $allowedMediaDomains = [
58+
'\'self\'',
59+
];
60+
/** @var array Domains from which object elements can be loaded */
61+
private $allowedObjectDomains = [];
62+
/** @var array Domains from which iframes can be loaded */
63+
private $allowedFrameDomains = [];
64+
/** @var array Domains from which fonts can be loaded */
65+
private $allowedFontDomains = [
66+
'\'self\'',
67+
];
68+
69+
/**
70+
* Whether inline JavaScript snippets are allowed or forbidden
71+
* @param bool $state
72+
* @return $this
73+
*/
74+
public function inlineScriptState($state = false) {
75+
$this->inlineScriptAllowed = $state;
76+
return $this;
77+
}
78+
79+
/**
80+
* Whether eval in JavaScript is allowed or forbidden
81+
* @param bool $state
82+
* @return $this
83+
*/
84+
public function evalScriptState($state = true) {
85+
$this->evalScriptAllowed= $state;
86+
return $this;
87+
}
88+
89+
/**
90+
* Allows to execute JavaScript files from a specific domain. Use * to
91+
* allow JavaScript from all domains.
92+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
93+
* @return $this
94+
*/
95+
public function addAllowedScriptDomain($domain) {
96+
$this->allowedScriptDomains[] = $domain;
97+
return $this;
98+
}
99+
100+
/**
101+
* Whether inline CSS snippets are allowed or forbidden
102+
* @param bool $state
103+
* @return $this
104+
*/
105+
public function inlineStyleState($state = true) {
106+
$this->inlineStyleAllowed = $state;
107+
return $this;
108+
}
109+
110+
/**
111+
* Allows to execute CSS files from a specific domain. Use * to allow
112+
* CSS from all domains.
113+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
114+
* @return $this
115+
*/
116+
public function addAllowedStyleDomain($domain) {
117+
$this->allowedStyleDomains[] = $domain;
118+
return $this;
119+
}
120+
121+
/**
122+
* Allows using fonts from a specific domain. Use * to allow
123+
* fonts from all domains.
124+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
125+
* @return $this
126+
*/
127+
public function addAllowedFontDomain($domain) {
128+
$this->allowedFontDomains[] = $domain;
129+
return $this;
130+
}
131+
132+
/**
133+
* Allows embedding images from a specific domain. Use * to allow
134+
* images from all domains.
135+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
136+
* @return $this
137+
*/
138+
public function addAllowedImageDomain($domain) {
139+
$this->allowedImageDomains[] = $domain;
140+
return $this;
141+
}
142+
143+
/**
144+
* To which remote domains the JS can bind to.
145+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
146+
* @return $this
147+
*/
148+
public function addAllowedConnectDomain($domain) {
149+
$this->allowedConnectDomains[] = $domain;
150+
return $this;
151+
}
152+
153+
/**
154+
* To which remote domains the JS can bind to.
155+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
156+
* @return $this
157+
*/
158+
public function addAllowedMediaDomain($domain) {
159+
$this->allowedMediaDomains[] = $domain;
160+
return $this;
161+
}
162+
163+
/**
164+
* From which domains objects such as <object>, <embed> or <applet> are executed
165+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
166+
* @return $this
167+
*/
168+
public function addAllowedObjectDomain($domain) {
169+
$this->allowedObjectDomains[] = $domain;
170+
return $this;
171+
}
172+
173+
/**
174+
* Which domains can be embedded in an iframe
175+
* @param string $domain Domain to whitelist. Any passed value needs to be properly sanitized.
176+
* @return $this
177+
*/
178+
public function addAllowedFrameDomain($domain) {
179+
$this->allowedFrameDomains[] = $domain;
180+
return $this;
181+
}
182+
183+
/**
184+
* @return string
185+
*/
186+
public function getPolicy() {
187+
$policy = "default-src 'none';";
188+
189+
if(!empty($this->allowedScriptDomains)) {
190+
$policy .= 'script-src ' . implode(' ', $this->allowedScriptDomains);
191+
if($this->inlineScriptAllowed) {
192+
$policy .= ' \'unsafe-inline\'';
193+
}
194+
if($this->evalScriptAllowed) {
195+
$policy .= ' \'unsafe-eval\'';
196+
}
197+
$policy .= ';';
198+
}
199+
200+
if(!empty($this->allowedStyleDomains)) {
201+
$policy .= 'style-src ' . implode(' ', $this->allowedStyleDomains);
202+
if($this->inlineStyleAllowed) {
203+
$policy .= ' \'unsafe-inline\'';
204+
}
205+
$policy .= ';';
206+
}
207+
208+
if(!empty($this->allowedImageDomains)) {
209+
$policy .= 'img-src ' . implode(' ', $this->allowedImageDomains);
210+
$policy .= ';';
211+
}
212+
213+
if(!empty($this->allowedFontDomains)) {
214+
$policy .= 'font-src ' . implode(' ', $this->allowedFontDomains);
215+
$policy .= ';';
216+
}
217+
218+
if(!empty($this->allowedConnectDomains)) {
219+
$policy .= 'connect-src ' . implode(' ', $this->allowedConnectDomains);
220+
$policy .= ';';
221+
}
222+
223+
if(!empty($this->allowedMediaDomains)) {
224+
$policy .= 'media-src ' . implode(' ', $this->allowedMediaDomains);
225+
$policy .= ';';
226+
}
227+
228+
if(!empty($this->allowedObjectDomains)) {
229+
$policy .= 'object-src ' . implode(' ', $this->allowedObjectDomains);
230+
$policy .= ';';
231+
}
232+
233+
if(!empty($this->allowedFrameDomains)) {
234+
$policy .= 'frame-src ' . implode(' ', $this->allowedFrameDomains);
235+
$policy .= ';';
236+
}
237+
238+
return rtrim($policy, ';');
239+
}
240+
}

‎lib/public/appframework/http/response.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,13 +186,18 @@ public function setHeaders(array $headers) {
186186
* @return array the headers
187187
*/
188188
public function getHeaders() {
189-
$mergeWith = array();
189+
$mergeWith = [];
190190

191191
if($this->lastModified) {
192192
$mergeWith['Last-Modified'] =
193193
$this->lastModified->format(\DateTime::RFC2822);
194194
}
195195

196+
if(!isset($this->headers['Content-Security-Policy'])) {
197+
$contentSecurityPolicy = new ContentSecurityPolicyHelper();
198+
$this->headers['Content-Security-Policy'] = $contentSecurityPolicy->getPolicy();
199+
}
200+
196201
if($this->ETag) {
197202
$mergeWith['ETag'] = '"' . $this->ETag . '"';
198203
}

‎tests/lib/appframework/controller/ControllerTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,12 @@ public function testFormat() {
171171

172172

173173
public function testFormatDataResponseJSON() {
174-
$expectedHeaders = array(
174+
$expectedHeaders = [
175175
'test' => 'something',
176176
'Cache-Control' => 'no-cache, must-revalidate',
177-
'Content-Type' => 'application/json; charset=utf-8'
178-
);
177+
'Content-Type' => 'application/json; charset=utf-8',
178+
'Content-Security-Policy' => "default-src 'none';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self';font-src 'self';connect-src 'self';media-src 'self'",
179+
];
179180

180181
$response = $this->controller->customDataResponse(array('hi'));
181182
$response = $this->controller->buildResponse($response, 'json');

‎tests/lib/appframework/http/DataResponseTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ public function testConstructorAllowsToSetHeaders() {
6666
$headers = array('test' => 'something');
6767
$response = new DataResponse($data, $code, $headers);
6868

69-
$expectedHeaders = array('Cache-Control' => 'no-cache, must-revalidate');
69+
$expectedHeaders = [
70+
'Cache-Control' => 'no-cache, must-revalidate',
71+
'Content-Security-Policy' => "default-src 'none';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self';font-src 'self';connect-src 'self';media-src 'self'",
72+
];
7073
$expectedHeaders = array_merge($expectedHeaders, $headers);
7174

7275
$this->assertEquals($data, $response->getData());

‎tests/lib/appframework/http/ResponseTest.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,25 @@ public function testAddHeader(){
4949
}
5050

5151

52-
function testSetHeaders(){
52+
public function testSetHeaders() {
5353
$expected = array(
5454
'Last-Modified' => 1,
5555
'ETag' => 3,
5656
'Something-Else' => 'hi'
5757
);
5858

59+
$this->childResponse->setHeaders($expected);
60+
$headers = $this->childResponse->getHeaders();
61+
$expected['Content-Security-Policy'] = "default-src 'none';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self';font-src 'self';connect-src 'self';media-src 'self'";
62+
63+
$this->assertEquals($expected, $headers);
64+
}
65+
66+
public function testOverwriteCSP() {
67+
$expected = [
68+
'Content-Security-Policy' => 'MyCustomPolicy',
69+
];
70+
5971
$this->childResponse->setHeaders($expected);
6072
$headers = $this->childResponse->getHeaders();
6173

@@ -66,7 +78,7 @@ function testSetHeaders(){
6678
public function testAddHeaderValueNullDeletesIt(){
6779
$this->childResponse->addHeader('hello', 'world');
6880
$this->childResponse->addHeader('hello', null);
69-
$this->assertEquals(1, count($this->childResponse->getHeaders()));
81+
$this->assertEquals(2, count($this->childResponse->getHeaders()));
7082
}
7183

7284

0 commit comments

Comments
 (0)