Skip to content

Commit 2a9adc0

Browse files
w1ll-i-codew1ll-i-code
committed
Allow modules to adjust the CSP headers through a dedicated hook.
1 parent 79971cb commit 2a9adc0

File tree

5 files changed

+241
-62
lines changed

5 files changed

+241
-62
lines changed

‎application/controllers/NavigationController.php‎

Lines changed: 2 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
use Icinga\Application\Config;
88
use Icinga\Exception\NotFoundError;
99
use Icinga\Data\DataArray\ArrayDatasource;
10-
use Icinga\Data\Filter\FilterMatchCaseInsensitive;
1110
use Icinga\Forms\ConfirmRemovalForm;
1211
use Icinga\Forms\Navigation\NavigationConfigForm;
12+
use Icinga\Util\NavigationItemHelper;
1313
use Icinga\Web\Controller;
1414
use Icinga\Web\Form;
1515
use Icinga\Web\Menu;
@@ -65,63 +65,12 @@ protected function listItemTypes()
6565
return $types;
6666
}
6767

68-
/**
69-
* Return all shared navigation item configurations
70-
*
71-
* @param string $owner A username if only items shared by a specific user are desired
72-
*
73-
* @return array
74-
*/
75-
protected function fetchSharedNavigationItemConfigs($owner = null)
76-
{
77-
$configs = array();
78-
foreach ($this->itemTypeConfig as $type => $_) {
79-
$config = Config::navigation($type);
80-
$config->getConfigObject()->setKeyColumn('name');
81-
$query = $config->select();
82-
if ($owner !== null) {
83-
$query->applyFilter(new FilterMatchCaseInsensitive('owner', '=', $owner));
84-
}
85-
86-
foreach ($query as $itemConfig) {
87-
$configs[] = $itemConfig;
88-
}
89-
}
90-
91-
return $configs;
92-
}
93-
94-
/**
95-
* Return all user navigation item configurations
96-
*
97-
* @param string $username
98-
*
99-
* @return array
100-
*/
101-
protected function fetchUserNavigationItemConfigs($username)
102-
{
103-
$configs = array();
104-
foreach ($this->itemTypeConfig as $type => $_) {
105-
$config = Config::navigation($type, $username);
106-
$config->getConfigObject()->setKeyColumn('name');
107-
foreach ($config->select() as $itemConfig) {
108-
$configs[] = $itemConfig;
109-
}
110-
}
111-
112-
return $configs;
113-
}
114-
11568
/**
11669
* Show the current user a list of their navigation items
11770
*/
11871
public function indexAction()
11972
{
120-
$user = $this->Auth()->getUser();
121-
$ds = new ArrayDatasource(array_merge(
122-
$this->fetchSharedNavigationItemConfigs($user->getUsername()),
123-
$this->fetchUserNavigationItemConfigs($user->getUsername())
124-
));
73+
$ds = new ArrayDatasource(NavigationItemHelper::fetchUserNavigationItems($this->Auth()->getUser()));
12574
$query = $ds->select();
12675

12776
$this->view->types = $this->listItemTypes();

‎application/forms/Config/General/ApplicationConfigForm.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
namespace Icinga\Forms\Config\General;
55

66
use Icinga\Application\Icinga;
7+
use Icinga\Authentication\Auth;
78
use Icinga\Data\ResourceFactory;
89
use Icinga\Web\Form;
10+
use Icinga\Util\Csp;
911

1012
/**
1113
* Configuration form for general application options
@@ -60,13 +62,20 @@ public function createElements(array $formData)
6062
'security_use_strict_csp',
6163
[
6264
'label' => $this->translate('Enable strict content security policy'),
65+
'autosubmit' => true,
6366
'description' => $this->translate(
6467
'Set whether to use strict content security policy (CSP).'
6568
. ' This setting helps to protect from cross-site scripting (XSS).'
6669
)
6770
]
6871
);
6972

73+
if ($formData['security_use_strict_csp']) {
74+
Csp::createNonce();
75+
$header = Csp::getContentSecurityPolicy(Auth::getInstance()->getUser());
76+
$this->addHint("Content-Security-Policy: $header");
77+
}
78+
7079
$this->addElement(
7180
'text',
7281
'global_module_path',

‎library/Icinga/Application/Hook/CspDirectiveHook.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
3+
4+
namespace Icinga\Application\Hook;
5+
6+
abstract class CspDirectiveHook
7+
{
8+
/**
9+
* Allow the module to provide custom directives for the CSP header. The return value should be an array
10+
* with directive as the key and the policies in an array as the value. The valid values can either be
11+
* a concrete host, whitelisting subdomains for hosts or a custom nonce for that module.
12+
*
13+
* Example: [ 'img-src' => [ 'https://*.media.tumblr.com', 'https://http.cat/' ] ]
14+
*
15+
* @return array<string, string[]> The CSP directives are the keys and the policies the values.
16+
*/
17+
abstract public function getCspDirectives(): array;
18+
}

‎library/Icinga/Util/Csp.php‎

Lines changed: 134 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@
44

55
namespace Icinga\Util;
66

7+
use Icinga\Application\Hook;
8+
use Icinga\Application\Hook\CspDirectiveHook;
9+
use Icinga\Application\Icinga;
10+
use Icinga\Application\Logger;
11+
use Icinga\Authentication\Auth;
12+
use Icinga\Data\ConfigObject;
13+
use Icinga\User;
714
use Icinga\Web\Response;
815
use Icinga\Web\Window;
916
use RuntimeException;
@@ -45,17 +52,131 @@ private function __construct()
4552
*/
4653
public static function addHeader(Response $response): void
4754
{
55+
$user = Auth::getInstance()->getUser();
56+
$header = static::getContentSecurityPolicy($user);
57+
Logger::debug("Setting Content-Security-Policy header for user {$user->getUsername()} to $header");
58+
$response->setHeader('Content-Security-Policy', $header, true);
59+
}
60+
61+
/**
62+
* Get the Content-Security-Policy for a specific user.
63+
*
64+
* @param User $user
65+
*
66+
* @throws RuntimeException If no nonce set for CSS
67+
*
68+
* @return string Returns the generated header value.
69+
*/
70+
public static function getContentSecurityPolicy(User $user): string {
4871
$csp = static::getInstance();
4972

5073
if (empty($csp->styleNonce)) {
5174
throw new RuntimeException('No nonce set for CSS');
5275
}
5376

54-
$response->setHeader(
55-
'Content-Security-Policy',
56-
"script-src 'self'; style-src 'self' 'nonce-$csp->styleNonce';",
57-
true
58-
);
77+
// These are the default directives that should always be enforced. 'self' is valid for all
78+
// directives and will therefor not be listed here.
79+
$cspDirectives = [
80+
'style-src' => ["'nonce-{$csp->styleNonce}'"],
81+
'font-src' => ["data:"],
82+
'img-src' => ["data:"],
83+
'frame-src' => []
84+
];
85+
86+
// Whitelist the hosts in the custom NavigationItems configured for the user,
87+
// so that the iframes can be rendered properly.
88+
/** @var array<ConfigObject> $navigationItems */
89+
$navigationItems = NavigationItemHelper::fetchUserNavigationItems($user);
90+
foreach ($navigationItems as $navigationItem) {
91+
92+
// Skip the host if the link gets opened in a new window.
93+
if ($navigationItem->get("target", "") === "_blank") {
94+
continue;
95+
}
96+
97+
$name = $navigationItem->get("name", "");
98+
$url = $navigationItem->get("url", "");
99+
100+
$scheme = parse_url($url, PHP_URL_SCHEME);
101+
$host = parse_url($url, PHP_URL_HOST);
102+
103+
if ($host === null || !static::validateCspPolicy("NavigationItem '$name'", "frame-src", $host)) {
104+
continue;
105+
}
106+
107+
$policy = $host;
108+
if ($scheme !== null) {
109+
$policy = "$scheme://$host";
110+
}
111+
112+
$cspDirectives['frame-src'][] = $policy;
113+
}
114+
115+
// Allow modules to add their own csp directives in a limited fashion.
116+
/** @var CspDirectiveHook $hook */
117+
foreach (Hook::all('CspDirective') as $hook) {
118+
foreach ($hook->getCspDirectives() as $directive => $policies) {
119+
120+
// policy names contain only lowercase letters and '-'. Reject anything else.
121+
if (!preg_match('|^[a-z\-]+$|', $directive)) {
122+
$errorSource = get_class($hook);
123+
Logger::debug("$errorSource: Invalid CSP directive found: $directive");
124+
continue;
125+
}
126+
127+
// The default-src can only ever be 'self'. Disallow any updates to it.
128+
if ($directive === "default-src") {
129+
$errorSource = get_class($hook);
130+
Logger::debug("$errorSource: Changing default-src is forbidden.");
131+
continue;
132+
}
133+
134+
$cspDirectives[$directive] = $cspDirectives[$directive] ?? [];
135+
foreach ($policies as $policy) {
136+
$source = get_class($hook);
137+
if (!static::validateCspPolicy($source, $directive, $policy)) {
138+
continue;
139+
}
140+
141+
$cspDirectives[$directive][] = $policy;
142+
}
143+
}
144+
}
145+
146+
$header = "default-src 'self'; ";
147+
foreach ($cspDirectives as $directive => $policies) {
148+
if (!empty($policies)) {
149+
$header .= ' ' . implode(' ', array_merge([$directive, "'self'"], array_unique($policies))) . ';';
150+
}
151+
}
152+
153+
return $header;
154+
}
155+
156+
public static function validateCspPolicy(string $source, string $directive, string $policy): bool {
157+
// We accept the following policies:
158+
// 1. Hosts: Modules can whitelist certain domains as sources for the CSP header directives.
159+
// - A host can have a specific scheme (http or https).
160+
// - A host can whitelist all subdomains with *
161+
// - A host can contain all alphanumeric characters as well as '+', '-', '_', '.', and ':'
162+
// 2. Nonce: Modules are allowed to specify custom nonce for some directives.
163+
// - A nonce is enclosed in single-quotes: "'"
164+
// - A nonce begins with 'nonce-' followed by at least 22 significant characters of base64 encoded data.
165+
// as recommended by the standard: https://content-security-policy.com/nonce/
166+
if (! preg_match("/^((https?:\/\/)?\*?[a-zA-Z0-9+._\-:]+|'nonce-[a-zA-Z0-9+\/]{22,}={0,3}')$/", $policy)) {
167+
Logger::debug("$source: Invalid CSP policy found: $directive $policy");
168+
return false;
169+
}
170+
171+
// We refuse all overly aggressive whitelisting by default. This includes:
172+
// 1. Whitelisting all Hosts with '*'
173+
// 2. Whitelisting all Hosts in a tld, e.g. 'http://*.com'
174+
if (preg_match('|\*(\.[a-zA-Z]+)?$|', $directive)) {
175+
Logger::debug("$source: Disallowing whitelisting all hosts. $directive");
176+
return false;
177+
}
178+
179+
return true;
59180
}
60181

61182
/**
@@ -67,9 +188,10 @@ public static function addHeader(Response $response): void
67188
public static function createNonce(): void
68189
{
69190
$csp = static::getInstance();
70-
$csp->styleNonce = base64_encode(random_bytes(16));
71-
72-
Window::getInstance()->getSessionNamespace('csp')->set('style_nonce', $csp->styleNonce);
191+
if ($csp->styleNonce === null) {
192+
$csp->styleNonce = base64_encode(random_bytes(16));
193+
Window::getInstance()->getSessionNamespace('csp')->set('style_nonce', $csp->styleNonce);
194+
}
73195
}
74196

75197
/**
@@ -79,7 +201,10 @@ public static function createNonce(): void
79201
*/
80202
public static function getStyleNonce(): ?string
81203
{
82-
return static::getInstance()->styleNonce;
204+
if (Icinga::app()->isWeb()) {
205+
return static::getInstance()->styleNonce;
206+
}
207+
return null;
83208
}
84209

85210
/**

‎library/Icinga/Util/NavigationItemHelper.php‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<?php
2+
3+
namespace Icinga\Util;
4+
5+
use Icinga\Application\Config;
6+
use Icinga\Data\Filter\FilterMatchCaseInsensitive;
7+
use Icinga\User;
8+
use Icinga\Web\Navigation\Navigation;
9+
10+
class NavigationItemHelper
11+
{
12+
13+
protected static $navigationItemCache = null;
14+
15+
public static function fetchUserNavigationItems(User $user)
16+
{
17+
if (self::$navigationItemCache !== null) {
18+
return self::$navigationItemCache;
19+
}
20+
21+
$itemTypeConfig = Navigation::getItemTypeConfiguration();
22+
$username = $user->getUsername();
23+
self::$navigationItemCache = array_merge(
24+
static::fetchSharedNavigationItemConfigs($itemTypeConfig, $username),
25+
static::fetchUserNavigationItemConfigs($itemTypeConfig, $username)
26+
);
27+
28+
return self::$navigationItemCache;
29+
}
30+
31+
/**
32+
* Return all shared navigation item configurations
33+
*
34+
* @param string $owner A username if only items shared by a specific user are desired
35+
*
36+
* @return array
37+
*/
38+
protected static function fetchSharedNavigationItemConfigs($itemTypeConfig, string $owner)
39+
{
40+
$configs = array();
41+
foreach ($itemTypeConfig as $type => $_) {
42+
$config = Config::navigation($type);
43+
$config->getConfigObject()->setKeyColumn('name');
44+
$query = $config->select();
45+
if ($owner !== null) {
46+
$query->applyFilter(new FilterMatchCaseInsensitive('owner', '=', $owner));
47+
}
48+
49+
foreach ($query as $itemConfig) {
50+
$configs[] = $itemConfig;
51+
}
52+
}
53+
54+
return $configs;
55+
}
56+
57+
/**
58+
* Return all user navigation item configurations
59+
*
60+
* @param string $username
61+
*
62+
* @return array
63+
*/
64+
protected static function fetchUserNavigationItemConfigs($itemTypeConfig, string $username)
65+
{
66+
$configs = array();
67+
foreach ($itemTypeConfig as $type => $_) {
68+
$config = Config::navigation($type, $username);
69+
$config->getConfigObject()->setKeyColumn('name');
70+
foreach ($config->select() as $itemConfig) {
71+
$configs[] = $itemConfig;
72+
}
73+
}
74+
75+
return $configs;
76+
}
77+
78+
}

0 commit comments

Comments
 (0)