Skip to content

Commit 43c748e

Browse files
committed
Allow editing of the CSP trusted image sources.
1 parent 79971cb commit 43c748e

File tree

3 files changed

+97
-1
lines changed

3 files changed

+97
-1
lines changed

application/forms/Config/General/ApplicationConfigForm.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55

66
use Icinga\Application\Icinga;
77
use Icinga\Data\ResourceFactory;
8+
use Icinga\Security\SecurityException;
89
use Icinga\Web\Form;
10+
use Icinga\Util\Csp;
11+
use Icinga\Web\Notification;
912

1013
/**
1114
* Configuration form for general application options
@@ -67,6 +70,23 @@ public function createElements(array $formData)
6770
]
6871
);
6972

73+
$securityImageSourceWhitelistHidden =
74+
!(boolean) Icinga::app()->getConfig()->get("security", "use_strict_csp", false);
75+
$securityImageSourceWhitelistHiddenClass = $securityImageSourceWhitelistHidden ? 'input-hidden' : '';
76+
$this->addElement(
77+
'text',
78+
'security_image_source_whitelist',
79+
[
80+
'label' => $this->translate('Domains whitelist for the CSP image source'),
81+
'placeholder' => $this->translate('No external domains whitelisted'),
82+
'class' => $securityImageSourceWhitelistHiddenClass,
83+
'description' => $this->translate(
84+
'This whitelist allows you to specify a comma separated list of trusted hosts, '
85+
. 'from which images for the icingaweb2 UI can be loaded.'
86+
)
87+
]
88+
);
89+
7090
$this->addElement(
7191
'text',
7292
'global_module_path',
@@ -102,4 +122,23 @@ public function createElements(array $formData)
102122

103123
return $this;
104124
}
125+
126+
/**
127+
* @inheritDoc
128+
*/
129+
public function isValid($formData)
130+
{
131+
$imageSourceWhitelist = $formData["security_image_source_whitelist"] ?? "";
132+
$valid = true;
133+
foreach (explode(",", $imageSourceWhitelist) as $imageSource) {
134+
try {
135+
Csp::validateImageSourceWhitelistItem($imageSource);
136+
}catch (SecurityException $e) {
137+
Notification::error($e->getMessage());
138+
$valid = false;
139+
}
140+
}
141+
142+
return $valid && parent::isValid($formData);
143+
}
105144
}

library/Icinga/Util/Csp.php

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@
44

55
namespace Icinga\Util;
66

7+
use Icinga\Application\Icinga;
8+
use Icinga\Application\Logger;
9+
use Icinga\Security\SecurityException;
710
use Icinga\Web\Response;
811
use Icinga\Web\Window;
12+
use ipl\I18n\StaticTranslator;
913
use RuntimeException;
1014

1115
use function ipl\Stdlib\get_php_type;
@@ -51,9 +55,13 @@ public static function addHeader(Response $response): void
5155
throw new RuntimeException('No nonce set for CSS');
5256
}
5357

58+
$header = "default-src 'self'; style-src 'self' 'nonce-{$csp->styleNonce}'; ";
59+
$imageSourceWhitelist = Icinga::app()->getConfig()->get("security", "image_source_whitelist", "");
60+
$header = $header . Csp::getImageSourceDirective($imageSourceWhitelist);
61+
5462
$response->setHeader(
5563
'Content-Security-Policy',
56-
"script-src 'self'; style-src 'self' 'nonce-$csp->styleNonce';",
64+
$header,
5765
true
5866
);
5967
}
@@ -79,6 +87,9 @@ public static function createNonce(): void
7987
*/
8088
public static function getStyleNonce(): ?string
8189
{
90+
if (Icinga::app()->isCli()) {
91+
return null;
92+
}
8293
return static::getInstance()->styleNonce;
8394
}
8495

@@ -108,4 +119,46 @@ protected static function getInstance(): self
108119

109120
return static::$instance;
110121
}
122+
123+
public static function getImageSourceDirective(string $whitelist): string {
124+
$directives = ["img-src", "'self'", "data:"];
125+
foreach (explode(",", $whitelist) as $domain) {
126+
try {
127+
$directives[] = Csp::validateImageSourceWhitelistItem($domain);
128+
} catch (SecurityException $e) {
129+
Logger::error("Ignoring domain '$domain' as it is not valid. $e");
130+
}
131+
}
132+
133+
return implode(' ', $directives) . ";";
134+
}
135+
136+
/**
137+
* Validates and trims an item that is used as a domain in the img-src directive.
138+
* Will throw an error, if the user tries to whitelist everything (*) or tries
139+
* to inject special characters that might allow to escape and edit the whole csp.
140+
*
141+
* @throws SecurityException
142+
*/
143+
public static function validateImageSourceWhitelistItem(string $item): string {
144+
$item = trim($item);
145+
// Don't allow general whitelisting of all domains
146+
if ($item == '*') {
147+
throw new SecurityException(
148+
StaticTranslator::$instance->translate("Whitelisting all domains is not allowed.")
149+
);
150+
}
151+
152+
// Don't allow special characters that might allow to escape and edit the whole csp.
153+
// Otherwise, e.g. "example.com; script-src *" will allow the user to change other directives.
154+
$csp_config_regex = "|^\*?[a-zA-Z0-9+._\-:]*$|";
155+
if (!preg_match($csp_config_regex, $item)) {
156+
throw new SecurityException(
157+
StaticTranslator::$instance->translate("The following domain is invalid: ")
158+
. $item
159+
);
160+
}
161+
162+
return $item;
163+
}
111164
}

public/css/icinga/forms.less

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ form.icinga-form {
3333
margin-bottom: 0;
3434
}
3535
}
36+
37+
&:has(.input-hidden) {
38+
display: none;
39+
}
3640
}
3741

3842
.control-group > :not(.control-label-group) {

0 commit comments

Comments
 (0)