Allow custom csp header by TheSyscall · Pull Request #5477 · Icinga/icingaweb2

TheSyscall · 2026-03-12T13:02:12Z

Taking over #5337 (#5337 (comment)) and implementing an override for a completely custom CSP-Header.

As well as adding a table below the form which displays the source of the automatically generated CSP-Header.

Styling for this table is still WIP.

requires Icinga/ipl-web#358
requires Icinga/ipl-web#361

closes #5337
closes #5333

bobapple · 2026-03-13T11:09:08Z

@cla-bot check

Al2Klimov

13 review comments should be enough for a Friday 13th. :D

library/Icinga/Web/Response.php

application/forms/Config/General/ApplicationConfigForm.php

Al2Klimov · 2026-03-13T14:36:05Z

application/forms/Config/General/CspConfigForm.php

+        $useCustomCsp = $this->getPopulatedValue('use_custom_csp', 'n') === 'y';
+        if ($useCustomCsp) {
+            $this->addHtml((new Callout(
+                CalloutType::Warning,


Get Introduce Callout ipl-web#358 done before merging this PR

Already listed as a requirement

application/forms/Config/General/CspConfigForm.php

application/controllers/ConfigController.php

library/Icinga/Util/Csp.php

library/Icinga/Web/Widget/CspConfigurationTable.php

TheSyscall · 2026-03-16T11:58:19Z

application/forms/Config/Security/CspConfigForm.php

+use ipl\Web\Compat\CompatForm;
+use ipl\Web\Widget\Callout;
+
+class CspConfigForm extends CompatForm


This currently doesn't output the config.ini to the screen like the old ConfigForm did.
I think changing the form type to a CompatForm is still the right call, because we don't want to rely on Zend_Forms forever.
If this behavior essential, we should reimplement the behavior of ConfigFrom as a CompatForm instead.

Even in new modules we reimplemented this behavior: Icinga/icingadb-web#1269

So I'd not downgrade this functionality.

PR for this is open at #5480

application/forms/Config/Security/CspConfigForm.php

public/css/icinga/widgets.less

application/forms/Config/Security/CspConfigForm.php

Al2Klimov · 2026-03-19T15:44:35Z

library/Icinga/Web/Widget/CspConfigurationTable.php

+        );
+
+        $this->addPolicyTable(
+            t('Dashboard'),


If you're concerned about #5477 (comment), you should be about this one too:

As not a IW2 dev, I'd have absolutely no clue that these are only my dashboards (if any!) and others may have their own.

Consider listing the dashboard-CSPs of all users, after all we have them (by name) in the preference store or something idk.

Module-provided dashboards should already be included as you should see all modules as admin. I guess they may contain external URLs as well.

I'm not sure if we always have access to all users in something like LDAP.
Also listing all dashboards and menu entries for all users in an organization on every initial request seems pretty expensive.

If a module wants something whitelisted beyond what is automatically detected, it can already do that.

We always have access to all users' dashboards and menu entries.

Imagine you make a custom CSP based on the generated one, but latter misses non-your dashboards.

application/forms/Config/General/CspConfigForm.php

application/forms/Config/Security/CspConfigForm.php

application/controllers/ConfigController.php

library/Icinga/Web/Widget/CspConfigurationTable.php

library/Icinga/Util/Csp.php

flourish86 · 2026-03-23T12:38:49Z

So, here would be my recommendation as discussed.

The simplified version

Step 1

Strict CSP ist disabled

Step 2

Enable CSP to see a table of generated CSP dependending on system and module usage.

Step 3

Toggle Custom CSP. This disables the generated CSP

Improvements

Proper hierarchy for the csp elements
Make obvious, what is enabled and disabled with "opacity"
Keep the table visible for reference, even when custom csp overrides the generated table
Add heltexts to clarify how it works

Additionally

Not in the mockups: Add toggles for Dashboard and Navigation section

flourish86 · 2026-03-23T13:10:36Z

Advanced version of the editor

Rows can be toggled separately.

Disabled rows will fade out

Custom CSP sources can be added additionally

Generated CSP can be disabled completely

Only Custom CSP Rules will be used

- Reload of form change if Csp was previously enabled in `ConfigController` - Use default attributes in `CspConfigurationTable` - Rename `$policyDirectives` to `$directivePolicies` in `Csp`

This tab requires the new config/security permission

…nonce

This allows for checkboxes integrated inside the table. This commit also adds disabling modules, dashboards and navigation items individualy.

…fore

Al2Klimov

(Nächste Aufgabe V. Don't hurry. I'm not even done, yet...)

My test setup

Install Fedora 42
Apply https://github.com/Al2Klimov/ansible-icinga-fedora
Enable strict CSP
Upgrade to https://git.icinga.com/packages/icinga-php-library/-/jobs/846553
Downgrade to https://git.icinga.com/packages/icingaweb/-/jobs/846577

Preparation

Icinga Web 2 packages

https://git.icinga.com/packages/icingaweb/-/jobs/846577

Build icingaweb2 packages from allow-custom-csp-header-5333 branch.

ipl packages

https://git.icinga.com/packages/icinga-php-library/-/jobs/846553

git -C ipl-web checkout origin/main
git -C ipl-web merge --no-edit origin/cps origin/callouts
git -C ipl-web switch -C $USER/review-csp
git -C ipl-web push -fu
cd icinga-php-library
git checkout origin/main
perl -pi -e 'if (m~ipl/web~) { s~main~$ENV{USER}/review-csp~ }' bin/make-snapshot.sh
perl -pi -e 's~^(PHP_VERSION)=.*~$1=8.2~' bin/make-snapshot.sh
bin/make-snapshot.sh $USER/review-csp
git diff --stat origin/snapshot/nightly..HEAD
git push -fu
Build icinga-php-library packages from $USER/review-csp branch

Al2Klimov · 2026-03-30T15:43:03Z

library/Icinga/Util/Csp.php

+        }
+
+        try {
+            if ($config->get('csp_enable_dashboards', '1')) {


When I upgrade IW2 with CSP already being enabled, ...

Yes, they would be enabled by default. The way I understand @lippserd is that things should just work out of the box.
I would expect most people to have CSP disabled in their setup because of its current limitations.

Just so that I understand what you are saying, it should work like this:

CSP is off by default, if the admin enables it via the config form, all three of these toggles should automatically enable (despite no entry in the config) and be stored in their on state once the admin submits the form.

If CSP was already enabled beforehand, they should stay off (again no value in the configuration)

Exactly! Then it would "just work out of the box" for people who enable it later.

Upgraders who already have CSP enabled did latter because the v2.13 behavior fits their needs. They simply don't expect this behavior change coming from behind through the cold kitchen.

Al2Klimov · 2026-03-30T15:44:10Z

library/Icinga/Util/Csp.php

+        }
+
+        try {
+            if ($config->get('csp_enable_navigation', '1')) {


... wouldn't these two toggles suddenly be enabled "by default" and widen my CSP w/o asking?

(I mean, we could default-enable them IN THE FORM, while someone enables CSP at all, but here... ?)

application/forms/Config/Security/CspConfigForm.php

Al2Klimov · 2026-03-30T15:57:47Z

application/forms/Config/Security/CspConfigForm.php

+                        'info-circle',
+                        [
+                            'class' => 'csp-policy-info',
+                            'title' => t('This is an automatically generated nonce. Its value is unique per request.'),


Unique per request? Are you sure? Because I toggle the CSP settings container reload and get the same nonce.

It is unique per initial request to icingaweb2 (without javascript XHR request).

If you know a better word, please suggest one.

request: not clear enough

session: I would say that refers to a user session (being logged in)

page load: icinga loads pages all the time, user might not know that (same problem as request)

leaving it out: But when is it generated?

window reload maybe?

@flourish86 I'm not sure what the best wording for this is. Technically it is for each initial request that doesn't come from JavaScript.

Co-authored-by: Alexander Aleksandrovič Klimov <alexander.klimov@icinga.com>

Al2Klimov

31 review comments at once should be enough for a Tuesday 31st. :)

Al2Klimov · 2026-03-31T10:21:25Z

application/forms/Config/Security/CspConfigForm.php

+        $csps = Csp::load(new ConfigObject([
+            'csp_enable_modules' => '1',
+            'csp_enable_dashboards' => '1',
+            'csp_enable_navigation' => '1',


Changes to the CSP and enabled modules already trigger page reloads. This is good.

In contrast, dashboard/navigation changes in the GUI don't trigger page reloads, even with these two toggles being enabled. This is bad.

After adding iframe?url=https://packages.icinga.com to the dashboard, Safari complains: "Refused to load https://packages.icinga.com/ because it does not appear in the frame-src directive of the Content Security Policy." I had to open the dev console to see it! And to reload the window to fix it.

I'm not sure what the best way to solve this is. We can of course reload the page if the user edits one of these two. But that doesn't solve the problem for other users if the item ends up being shared between users.

That's fine.

Al2Klimov · 2026-03-31T10:33:17Z

application/forms/Config/Security/CspConfigForm.php

+
+        if (count($rows) === 0) {
+            $this->add(
+                HtmlElement::create('p', ['class' => 'csp-form-hint'], sprintf('No %s policies found.', $title))


Not only this reads "No Modules policies found", this is a nightmare for translation.

Al2Klimov · 2026-03-31T10:44:13Z

application/forms/Config/Security/CspConfigForm.php

+
+        if ($field !== null) {
+            $this->addElement('checkbox', $field, [
+                'label' => sprintf($this->translate('Enable %s'), $title),


Yet another nightmare for translation...

Al2Klimov · 2026-03-31T10:47:21Z

application/forms/Config/Security/CspConfigForm.php

+     *
+     * @return void
+     */
+    protected function addPolicyTable(


Navigation literally says Type: "menu-item". In the UI. What about mapping such rather technical strings to translatable, human-readable text?

Menu items are extensible and modules can create new types if they desire. So while we can create a mapping it would only ever cover out own types.

Types from extensions can be displayed literally, unless:

Please check whether we could extend the respective hook with an optional method providing a (possibly translated) display name.

There is no hook for this. This is part of the modules configuration.php.
I will use the label attribute of the defined navigation type, it is the same as in the configuration screen for the user's navigation.

Al2Klimov · 2026-03-31T10:48:36Z

application/forms/Config/Security/CspConfigForm.php

+                $this->addHtml((new Callout(
+                    CalloutType::Warning,
+                    $this->translate(
+                        'Be aware that the custom CSP-Header completely overrides the automatically generated one.'


I miss instructions for the nonce. Keep this in mind when you write the docs.

Al2Klimov · 2026-03-31T11:41:47Z

library/Icinga/Security/Csp/Reason/ModuleCspReason.php

+     * @param string $module the module to load the CSP directive for
+     */
+    public function __construct(
+        public readonly string $module,


Yes, but consider making the whole class readonly instead (as in #5442) while on it.

Al2Klimov · 2026-03-31T11:42:07Z

library/Icinga/Security/Csp/Reason/NavigationCspReason.php

+{
+    public function __construct(
+        public readonly string $type,
+        public readonly NavigationItem $item,


Yes, but consider making the whole class readonly instead (as in #5442).

Al2Klimov · 2026-03-31T11:43:22Z

library/Icinga/Security/Csp/Reason/NavigationCspReason.php

+class NavigationCspReason extends CspReason
+{
+    public function __construct(
+        public readonly string $type,


Sounds like an enum makes sense here. I'll let it up to you.

Al2Klimov · 2026-03-31T11:43:37Z

library/Icinga/Security/Csp/Reason/StaticCspReason.php

+     * @param string $name the name to display for CSP reason
+     */
+    public function __construct(
+        public readonly string $name,


Yes, but consider making the whole class readonly instead (as in #5442) while on it.

Al2Klimov · 2026-03-31T11:45:51Z

library/Icinga/Security/Csp/Loader/CspLoader.php

+ * Base class for CSP loaders.
+ * A loader is responsible for loading CSP directives from a specific source.
+ */
+abstract class CspLoader


Whenever I have the completely free choice of making a class or interface, I make an interface. (PHP doesn't support multiple inheritance.)

Al2Klimov · 2026-03-31T13:37:29Z

library/Icinga/Security/Csp/Loader/DashboardCspLoader.php

+    {
+        $user = Auth::getInstance()->getUser();
+        if ($user === null) {
+            throw new RuntimeException('No user logged in');


I know this only ends up as log warning. But even for a warning, this is a too small "problem" IMAO. I mean, sure, no user logged in. And... now what? Panic? Is this such an uncommon scenario where:

Admin allows dashboard CSP

User visits the login page to log in

Al2Klimov · 2026-03-31T13:39:25Z

library/Icinga/Security/Csp/Loader/NavigationCspLoader.php

+
+        $auth = Auth::getInstance();
+        if (! $auth->isAuthenticated()) {
+            throw new RuntimeException('No user logged in');


I wonder whether a simple return would be enough...

Al2Klimov · 2026-03-31T13:49:35Z

library/Icinga/Security/Csp/LoadedCsp.php

+    public static function fromCsp(Csp $csp, CspReason $reason): static
+    {
+        $instance = new static($reason);
+        $instance->directives = $csp->directives;


Yes, directives is protected. Hence accessible. I wonder, though, whether this -and Csp as base class- is actually necessary or a dead simple readonly class with two properties would do it as well. You decide.

In Icinga/icinga2#10397, to be fair, I also enthusiastically used advanced inheritance tricks, but colleagues told me to just make it a property. (K.I.S.S.)

Al2Klimov · 2026-03-31T15:08:00Z

library/Icinga/Util/Csp.php

+        $header = static::getHeader();
+        $response->setHeader('Content-Security-Policy', $header, true);


Suggested change

$header = static::getHeader();

$response->setHeader('Content-Security-Policy', $header, true);

$response->setHeader('Content-Security-Policy', static::getHeader(), true);

Al2Klimov · 2026-03-31T15:39:00Z

application/forms/Config/Security/CspConfigForm.php

+        $section['use_custom_csp'] = $this->getValue('use_custom_csp');
+        $section['custom_csp'] = $this->getValue('custom_csp');
+
+        $this->changed = ! empty(array_diff_assoc(


Strange, my CSP page reloads on EVERY saving...

TheSyscall requested a review from nilmerg March 12, 2026 13:02

TheSyscall self-assigned this Mar 12, 2026

This comment was marked as duplicate.

Sign in to view

TheSyscall requested a review from Al2Klimov March 12, 2026 13:03

This comment was marked as duplicate.

Sign in to view

TheSyscall mentioned this pull request Mar 12, 2026

Allow editing of the CSP trusted image sources. #5337

Open

Al2Klimov mentioned this pull request Mar 12, 2026

Introduce \Icinga\Application\Hook\Essentials #5474

Open

This comment was marked as resolved.

Sign in to view

cla-bot bot added the cla/signed label Mar 13, 2026

TheSyscall force-pushed the allow-custom-csp-header-5333 branch from 735beb2 to cdef4c4 Compare March 13, 2026 13:18

This comment was marked as resolved.

Sign in to view

TheSyscall requested a review from Al2Klimov March 13, 2026 13:25

Al2Klimov removed the request for review from nilmerg March 13, 2026 14:44

Al2Klimov requested changes Mar 13, 2026

View reviewed changes

TheSyscall commented Mar 16, 2026

View reviewed changes

TheSyscall requested a review from Al2Klimov March 16, 2026 11:59

This comment was marked as resolved.

Sign in to view

Al2Klimov reviewed Mar 18, 2026

View reviewed changes

application/forms/Config/Security/CspConfigForm.php Outdated Show resolved Hide resolved

TheSyscall requested a review from Al2Klimov March 19, 2026 08:58

Al2Klimov requested changes Mar 19, 2026

View reviewed changes

Al2Klimov mentioned this pull request Mar 20, 2026

Implement Webdriver Icinga/icingaweb2-module-pdfexport#88

Open

TheSyscall force-pushed the allow-custom-csp-header-5333 branch from f9f19b4 to 3a4e7ac Compare March 23, 2026 10:21

TheSyscall requested a review from Al2Klimov March 24, 2026 10:59

lippserd added this to the 2.13 milestone Mar 24, 2026

Al2Klimov mentioned this pull request Mar 24, 2026

Implement Csp Icinga/ipl-web#361

Open

TheSyscall added 16 commits March 25, 2026 10:00

revert! fixup! Move CSP table into its own Widget

2c3efbd

Code review changes

b2bfa2b

- Reload of form change if Csp was previously enabled in `ConfigController` - Use default attributes in `CspConfigurationTable` - Rename `$policyDirectives` to `$directivePolicies` in `Csp`

Add a toggle to enable user content

187351e

Move CSP-Form into a newly created Security tab.

309d291

This tab requires the new config/security permission

Code review suggestions

c458abe

Use new Csp class in ipl-web

9ca25e9

Prefixed CSS-classes with csp-

c0d3d33

Code style changes

99305bc

Rework Csp to no longer rely on a private instance just to store the …

6877535

…nonce

Add form validation

00cd0dc

Merge CspConfigurationTable with form

1a9ea23

This allows for checkboxes integrated inside the table. This commit also adds disabling modules, dashboards and navigation items individualy.

fixup! Store custom_csp

f38fb7a

fixup! Remove large margin-bottom from table

44af9c8

Default use_custom_csp to 0

c4c796c

Store security seection in config even if the section didn't exist be…

a2eaa51

…fore

Log errors during Csp loading

8c73a47

TheSyscall force-pushed the allow-custom-csp-header-5333 branch from 711be73 to 8c73a47 Compare March 25, 2026 09:27

TheSyscall added 2 commits March 25, 2026 11:16

Return Csp instances instead of raw arrays

12676fe

Change Hook name to CspPolicyProvider

f25a499

TheSyscall force-pushed the allow-custom-csp-header-5333 branch from be79bd9 to f25a499 Compare March 25, 2026 11:49

TheSyscall requested a review from Al2Klimov March 25, 2026 14:02

lippserd removed this from the 2.13 milestone Mar 25, 2026

Al2Klimov requested changes Mar 30, 2026

View reviewed changes

Al2Klimov self-requested a review March 30, 2026 15:59

fixup! typo

3a24ab1

Co-authored-by: Alexander Aleksandrovič Klimov <alexander.klimov@icinga.com>

Al2Klimov mentioned this pull request Mar 31, 2026

Introduce Callout Icinga/ipl-web#358

Open

Al2Klimov requested changes Mar 31, 2026

View reviewed changes

Al2Klimov self-requested a review March 31, 2026 11:47

TheSyscall mentioned this pull request Mar 31, 2026

Implement password policy with hook #5419

Open

Al2Klimov reviewed Mar 31, 2026

View reviewed changes

		$header = static::getHeader();
		$response->setHeader('Content-Security-Policy', $header, true);

Conversation

TheSyscall commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

bobapple commented Mar 13, 2026

Uh oh!

This comment was marked as resolved.

Al2Klimov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheSyscall Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flourish86 commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Step 1

Step 2

Step 3

Improvements

Additionally

Uh oh!

flourish86 commented Mar 23, 2026

Rows can be toggled separately.

Disabled rows will fade out

Custom CSP sources can be added additionally

Generated CSP can be disabled completely

Uh oh!

Al2Klimov left a comment

Choose a reason for hiding this comment

My test setup

Preparation

Icinga Web 2 packages

ipl packages

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheSyscall commented Mar 12, 2026 •

edited

Loading

TheSyscall Mar 25, 2026 •

edited

Loading

flourish86 commented Mar 23, 2026 •

edited

Loading