XSS mitigation: add support for Content-Security-Policy

[u238]

Is your feature request related to a problem? Please describe.

Different attack scenarios demonstrate the high impact that XSS vulnerabilities in an arbitrary module can have on an icingaweb2 installation. We showed during an internal audit that through one single XSS vulnerability we where able to fully compromise the monitoring infrastructure.

Describe the solution you'd like

Support CSP headers in oder to mitigate XSS attacks.

[Al2Klimov]

Hello @u238 and thank you for reporting!

I assume with support you mean a user's ability to do

[root@icinga-web-2-development ~]# tail -n 1 /etc/httpd/conf/httpd.conf
Header set Content-Security-Policy "default-src 'self';"
[root@icinga-web-2-development ~]#

while icingaweb2 just keeps working. (Correct?)

All of the above plus your report implies that you've tested it. What exactly doesn’t work for you?

After all XSS is about scripts. Would script-src 'self'; be enough for you and why not?

Best,
A/K

[u238]

Hi,
yes, I expect that it keeps working (without any errors).
Yes I tested it on a virgin system and lots of errors are logged in the console and the UI is kind of broken:

After all XSS is about scripts. Would script-src 'self'; be enough for you and why not?

Not really. That setting will not prevent other attacks like dangling markup and clickjacking attacks etc all described in the link I provided.
For my tests I used the following configuration:

Header always set Strict-Transport-Security "max-age=31536000; includeSubdomains; preload"
Header always set X-Frame-Options SAMEORIGIN
Header always set X-XSS-Protection "1; mode=block"
Header always set X-Content-Type-Options nosniff
Header always set Content-Security-Policy "default-src 'self';"
Header always set X-Permitted-Cross-Domain-Policies "none"
Header always edit Set-Cookie (.*) "$1; SameSite=Lax"

[Al2Klimov]

An an idealist I personally prefer to get rid of XSS holes not to require this kind of hardening.

But as a realist I see that this would be nearly impossible. As well as I see that cars have airbags and seat belts for a reason.

However the "seat belt" here – Content-Security-Policy "default-src 'self';" (I didn’t test the others, yet) – would (in contrast to script-src 'self';) basically require to refactor IW2+mods to this state:

Whatever your HTML (and CSS/...?) contains inline and is not HTML (CSS, JS, ...): outline it to a URL (of a dev's choice) below the same WWW doc root.

I.e.: no inline JS (w/ script-src 'self'; too, but we don’t have that much of this), no inline CSS, no data: URLs, ...

Before I do anything here I'd like to hear from both

• @lippserd and/or @nilmerg (POV: dev/ PM allocating dev HR/ security?)
• @julianbrost and/or @pdolinic (POV: security)

who considers which security mitigation of

• script-src 'self';
• default-src 'self';
• at your option any of other the headers OP listed above which could break IW2

worth refactoring and possibly additional requests (yes, I know about HTTP 2 optimistic pushing, but let's assume the worst – i.e. HTTP/1.x – for now).

Oh, and remember: We're not talking about an IW2 mod shipping malicious PHP code. In that case you can only pray. We're only talking about (unintentional) security holes – missing escaping of a string inside HTML in privileged context originated from a user input in unprivileged context, etc..

ref/IP/42393

[julianbrost]

Well you won't get me to argue against CSP, it's a useful tool.

An an idealist I personally prefer to get rid of XSS holes not to require this kind of hardening.

No, you certainly don't want to use CSP as an excuse to ignore XSS holes, but rather do both, so if something is discovered, you don't have to panic as it's not exploitable trivially and you can take care of it without a hurry.

Whatever your HTML (and CSS/...?) contains inline and is not HTML (CSS, JS, ...): outline it to a URL (of a dev's choice) below the same WWW doc root.

There's also the option to authenticate inline-content with a nonce or hash.

[Al2Klimov]

There's also the option to authenticate inline-content with a nonce or hash.

Oh, nice! But our PMs would still have to allocate dev HR for touching either the inline JS or inline everything (at their option).

[Al2Klimov]

However I'd say only the self source is better. The hashes/nonces would have to be synced IW2<->HTTPd otherwise.

[Al2Klimov]

Inline JS found so far

• application/layouts/scripts/layout.phtml
• application/views/scripts/authentication/logout.phtml
• application/views/scripts/config/devtools.phtml
• modules/director/application/controllers/ImportsourceController.php

[Al2Klimov]

@nilmerg With Header set Content-Security-Policy "style-src 'self';"

diff --git a/application/layouts/scripts/layout.phtml b/application/layouts/scripts/layout.phtml
index 1800f2c15..488380255 100644
--- a/application/layouts/scripts/layout.phtml
+++ b/application/layouts/scripts/layout.phtml
@@ -50,6 +50,13 @@ $innerLayoutScript = $this->layout()->innerLayout . '.phtml';
   <link rel="stylesheet" href="<?= $this->href($cssfile) ?>" media="all" type="text/css" />
   <link type="image/png" rel="shortcut icon" href="<?= $this->baseUrl('img/favicon.png') ?>" />
   <link rel="apple-touch-icon" href="<?= $this->baseUrl('img/touch-icon.png') ?>">
+    <style>
+      /*
+        #layout {
+          filter: hue-rotate(240deg);
+        }
+      */
+    </style>
 </head>
 <body id="body" class="loading">
 <pre id="responsive-debug"></pre>
@@ -103,6 +110,7 @@ var icinga = new Icinga({
   locale: '<?= $lang; ?>',
   timezone: '<?= $timezone ?>'
 });
+// $('div#layout')[0].style='filter: hue-rotate(240deg);';
 </script>
 </body>
 </html>

the style tag is rejected, but not the inJScted one.

[julianbrost]

However I'd say only the self source is better. The hashes/nonces would have to be synced IW2<->HTTPd otherwise.

At least for using the nonce, the application has to send the header, if you wouldn't use a new one for every response, this would defeat the purpose.

[Al2Klimov]

💡 Actually we could send a CSP header. HTTPd would have to append its own stuff to it rather than replacing.

[Al2Klimov]

Hello again @u238!

I've talked to @nilmerg. Securing all types of content as with default-src 'self'; is unfortunately a bottomless pit.

We can offer you one of the following (at our option, we didn’t fully decide yet):

• We outline the JS in Icinga Web and Director. Your HTTPd can set script-src 'self';.
• We add nonces to our inline JS in Icinga Web and Director and send script-src 'self' 'nonce-...';. Your HTTPd can append additional policies at your option.

Please let me know if your customer(s) accept that and we shall proceed.

Best,
A/K

@nilmerg The first option would obviously require all modules to outline JS to be fancy again. The second would require calling a hook like

?><script nonce="<?= GEN_NONCE_AND_APPEND_IT_TO_SCRIPT_SRC() ?>">alert('LOLCAT!')</script><?php

[nilmerg]

The first option would obviously require all modules to outline JS to be fancy again. The second would require calling a hook like

Modules were never required to do this. If that's the case, an alternative must be used. The nonces would only be required for our own framework code.

We add nonces to our inline JS in Icinga Web and Director and send script-src 'self' 'nonce-...';

We should not send script-src by default. This would inevitably break modules for anyone upgrading. Can't we not just send the nonces?

[Al2Klimov]

1. How many modules we don’t control explicitly use mission-critical inline JS? I consider it acceptable collateral damage to require to change that one script tag.
2. Break is a hard word. I'd say temporary make a little less fancy. And even that would apply only to modules using inline JS. And yes, we can. As a no-op.

[nilmerg]

And yes, we can. As a no-op.

Then we do just that. Whoever wants to enable CSP can do so explicitly in the webserver configuration.

[Al2Klimov]

No, that should have been sarcasm. I meant: you can put the nonces in a header of your choice which... will have no effect in any case. Neither by default, nor on users who try to enable CSP.

Anyway: to me it sounds like you clearly prefer the first option of mine as it doesn’t imply IW2 sending CSP by itself.

• We outline the JS in Icinga Web and Director. Your HTTPd can set script-src 'self';.

With a controller taking URL params and generating JS, of course. 😎 :P

[u238]

Hi all,
sorry for the late response.

Please let me know if your customer(s) accept that and we shall proceed.

I discussed this internally and we think this is a very good step in the right direction.
Customers can benefit from this big improvement and we can work for further improvements in the future in another issue.
I also agree that implementing default-src 'self' all in one step is way too much, we should address it one peace at a time and script-src is the best where to start!

Thank you all for your time!

[Al2Klimov]

@nilmerg @u238 💡 What about sending a header with nonces and unsafe-whatever? Everything would still work and someone's HTTPd could remove /( unsafe-.*);/.

[u238]

Hi @Al2Klimov , i did not exaclty understand your solution.

[Al2Klimov]

Instead of making everything in framework + Director script-src-self-compliant, I'd add nonces to the framework's inline JS (to reduce requests). Icinga Web 2 would send a CSP with script-src nonce1, nonce2, ... nonceN, self, unsafe-inline or so. This wouldn’t break anything. To activate this you'd have to edit (not set!) the CSP header via HTTPd by removing everything from including " unsafe-" up to and not including ";". (Header edit Content-Security-Policy "( 'unsafe-.*);" ";" I guess.)

[lippserd]

I would say we stop the discussion here until we have talked about it internally.

[TAINCER]

I searched where we currently have inline CSS or JS. (excluding: SVG, demos, examples, external libraries, and inline font usage)
This is not an exhaustive list, we may find more once we start implementing fixes.

Inline CSS and JS

• icingaweb2/application/layouts/scripts/layout.phtml
• icingaweb2/application/views/scripts/dashboard/settings.phtml
• icingaweb2/library/Icinga/Web/Widget/Announcements.php
• icingaweb2/library/Icinga/Web/Widget/ApplicationStateMessages.php
• icingaweb2/library/Icinga/Web/Widget/Chart/HistoryColorGrid.php
• icingaweb2/library/Icinga/Web/Widget/FilterEditor.php
• icingaweb2/library/Icinga/Web/Widget/Tab.php
• icingaweb2/modules/doc/application/views/scripts/style/font/phtml
• icingaweb2/modules/monitoring/application/views/helpers/PluginOutput.php
• icingaweb2/modules/monitoring/application/views/scripts/list/eventgrid.phtml
• icingaweb2/modules/monitoring/application/views/scripts/show/contact.phtml
• icingaweb2/modules/setup/application/views/scripts/index/index.phtml
• icingaweb2/public/js/icinga/behavior/application-state.js
• icingaweb2/application/views/scripts/authentication/logout.phtml
• icingaweb2/application/views/scripts/config/devtools.phtml
• modules/director/application/controllers/ImportsourceController.php