Skip to content

Commit 168628c

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

File tree

3 files changed

+107
-1
lines changed

3 files changed

+107
-1
lines changed

application/forms/Config/General/ApplicationConfigForm.php

Lines changed: 43 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,27 @@ 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+
if (empty($imageSourceWhitelist) || !$this->getRequest()->isPost()) {
133+
return $this;
134+
}
135+
136+
$valid = true;
137+
foreach (explode(",", $imageSourceWhitelist) as $imageSource) {
138+
try {
139+
Csp::validateImageSourceWhitelistItem($imageSource);
140+
}catch (SecurityException $e) {
141+
Notification::warning($e->getMessage());
142+
$valid = false;
143+
}
144+
}
145+
146+
return $valid && parent::isValid($formData);
147+
}
105148
}

library/Icinga/Util/Csp.php

Lines changed: 60 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,52 @@ protected static function getInstance(): self
108119

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

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)