Harden GUI by adding a CSP, XSS protection and referrer hiding #2212

fabianfrz · 2018-02-18T19:41:29Z

this should stay on master for longer time since it may have side effects like blocking resources.

…iding

fabianfrz

Added some description

fabianfrz · 2018-02-18T19:43:17Z

src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerBase.php

@@ -204,5 +204,10 @@ public function beforeExecuteRoute($dispatcher)

        // append ACL object to view
        $this->view->acl = new \OPNsense\Core\ACL();
+        $this->response->setHeader('Content-Security-Policy', "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' 'unsafe-eval';");


will only allow resources directly from the firewall and additional embedded in the HTML, style attributes and JavaScript eval.

fabianfrz · 2018-02-18T19:44:06Z

src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerBase.php

@@ -204,5 +204,10 @@ public function beforeExecuteRoute($dispatcher)

        // append ACL object to view
        $this->view->acl = new \OPNsense\Core\ACL();
+        $this->response->setHeader('Content-Security-Policy', "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' 'unsafe-eval';");
+        $this->response->setHeader('X-Frame-Options', "SAMEORIGIN");


Allow framing only in the context of the firewall itself (no framing by another website) - avoid clickjacking

@fabianfrz this one is already in there https://github.com/opnsense/core/blob/18.1.2/src/opnsense/mvc/app/controllers/OPNsense/Base/IndexController.php#L77 , we could move it to Base and make sure the classes deriving from index move to base. What do you think?

So it should be removed from the index controller since it inherits controllerbase and security should be enforced on all controllers - even those which are not IndexControllers.

Short note: There is currently no controller, which is based on ControllerBase.

fabianfrz · 2018-02-18T19:44:35Z

src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerBase.php

+        $this->response->setHeader('Content-Security-Policy', "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' 'unsafe-eval';");
+        $this->response->setHeader('X-Frame-Options', "SAMEORIGIN");
+        $this->response->setHeader('X-Content-Type-Options', "nosniff");
+        $this->response->setHeader('X-XSS-Protection', "1; mode=block");


block entire page if the browser detects XSS

fabianfrz · 2018-02-18T19:45:16Z

src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerBase.php

+        $this->response->setHeader('X-Frame-Options', "SAMEORIGIN");
+        $this->response->setHeader('X-Content-Type-Options', "nosniff");
+        $this->response->setHeader('X-XSS-Protection', "1; mode=block");
+        $this->response->setHeader('Referrer-Policy', "same-origin");


do not show the referrer to other pages if the user clicks a link in the GUI.

AdSchellevis · 2018-02-18T20:09:22Z

@fabianfrz we should indeed be careful with this, let me review and test this as soon as I can find some time for it.

L1ghtn1ng · 2018-02-18T22:40:14Z

You can test the csp using report-uri.com as that will let you know of any issues

…

________________________________ From: Fabian Franz BSc <notifications@github.com> Sent: Sunday, February 18, 2018 7:41:30 PM To: opnsense/core Cc: Subscribed Subject: [opnsense/core] Harden GUI Controllers by adding a CSP, XSS protection and referrer hiding (#2212) this should stay on master for longer time since it may have side effects like blocking resources.

________________________________ You can view, comment on, or merge this pull request online at: #2212 Commit Summary * Harden GUI Controllers by adding a CSP, XSS protection and referrer hiding File Changes * M src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerBase.php<https://github.com/opnsense/core/pull/2212/files#diff-0> (5) Patch Links: * https://github.com/opnsense/core/pull/2212.patch * https://github.com/opnsense/core/pull/2212.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#2212>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ADddQmjpKEjtt4M1QYPEQbYXNnCSiFrZks5tWHzqgaJpZM4SJ0NT>.

fabianfrz · 2018-02-19T16:03:34Z

looks like it works on static pages as well.

AdSchellevis · 2018-02-24T14:43:49Z

src/www/head.inc

+header('X-Content-Type-Options: nosniff');
+header('X-XSS-Protection: 1; mode=block');
+header('Referrer-Policy: same-origin');
+


probably better to move the headers here https://github.com/opnsense/core/blob/18.1.2/src/www/guiconfig.inc#L38 as the csrf checks are also there.

…ase now

fichtner

I have a question but no objections. Please don't merge before 18.1.3 is out though.

fichtner · 2018-02-25T16:00:03Z

src/opnsense/mvc/app/controllers/OPNsense/Base/IndexController.php

-     */
-    public function beforeExecuteRoute($dispatcher)
-    {
-        parent::beforeExecuteRoute($dispatcher);


Wasn‘t this here before we added the x header below? Should it be deleted?

@fichtner should be safe to delete, falls back to ControllerBase->beforeExecuteRoute()

fabianfrz · 2018-03-27T10:19:09Z

ping

AdSchellevis · 2018-03-27T11:07:53Z

pong, sorry a bit busy. will pull it in now.

AdSchellevis · 2018-04-19T10:57:12Z

@fabianfrz I'm going to drop the "default-src 'self';" in the security policy for the mvc code, this prevents plugins from using anything like open streetmap, which causes issues on some of our plugins. Not sure if this is the only issue with the more tightened security yet though. We might look into something at a later point to add trusted domains in the controller area, although I'm not sure what the best option is there yet.

fabianfrz · 2018-04-19T20:04:04Z

Maybe inject or replace the values with properties like the values for the mutable controller. So secure defaults are kept.

BTW: Which plugin is using OSM? It should only require image-src to add the domain.
Openlayers or leaflet should be placed in the plugin.

AdSchellevis · 2018-04-20T06:03:25Z

some custom work. adding the desired domains would indeed be better, but this needs a bit of work for a clear hook.

fabianfrz · 2018-04-20T16:15:07Z

I dont think so. Adding a property $csp-image-source to the controller base should work and the defaults are the values of my pull request. Any controller that inherits the base controller, can override the default just by adding it in the class. In that case it would be controller wide which should not be a huge issue because most controllers do have 1-3 actions

… see #2212

AdSchellevis · 2018-04-20T16:44:37Z

@fabianfrz this a8f54d2 should do the trick

fabianfrz · 2018-04-20T17:54:50Z

Exactly the implementation I had in mind

…#2212

… see opnsense/core#2212

Harden GUI Controllers by adding a CSP, XSS protection and referrer h…

88a6d4e

…iding

fabianfrz commented Feb 18, 2018

View reviewed changes

AdSchellevis self-assigned this Feb 18, 2018

csp and other security headers also for legacy pages

8264711

fabianfrz changed the title ~~Harden GUI Controllers by adding a CSP, XSS protection and referrer hiding~~ Harden GUI by adding a CSP, XSS protection and referrer hiding Feb 19, 2018

AdSchellevis reviewed Feb 24, 2018

View reviewed changes

fabianfrz added 3 commits February 24, 2018 18:51

remove same origin frame from IndexController as it is in ControllerB…

bcfc9a6

…ase now

mv CSP, XSS protection from head to guiconfig

ff41901

zap additional line

3027ed3

fichtner approved these changes Feb 28, 2018

View reviewed changes

AdSchellevis merged commit 3a49512 into opnsense:master Mar 27, 2018

AdSchellevis added a commit that referenced this pull request Apr 19, 2018

remove Content-Security-Policy "default-src 'self'; see #2212

c9132db

fabianfrz deleted the harden_gui_controllers branch April 19, 2018 19:54

AdSchellevis added a commit that referenced this pull request Apr 20, 2018

MVC/Content-Security-Policy settings, allow per controller overrides,…

a8f54d2

… see #2212

AdSchellevis added a commit to OPNids/core that referenced this pull request Apr 30, 2018

remove Content-Security-Policy "default-src 'self'; see opnsense/core…

65cc2b5

…#2212

AdSchellevis added a commit to OPNids/core that referenced this pull request Apr 30, 2018

MVC/Content-Security-Policy settings, allow per controller overrides,…

3bf2caa

… see opnsense/core#2212

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harden GUI by adding a CSP, XSS protection and referrer hiding #2212

Harden GUI by adding a CSP, XSS protection and referrer hiding #2212

fabianfrz commented Feb 18, 2018

fabianfrz left a comment

fabianfrz Feb 18, 2018

fabianfrz Feb 18, 2018

AdSchellevis Feb 24, 2018

fabianfrz Feb 24, 2018 •

edited

Loading

fabianfrz Feb 18, 2018

fabianfrz Feb 18, 2018

AdSchellevis commented Feb 18, 2018

L1ghtn1ng commented Feb 18, 2018 via email

fabianfrz commented Feb 19, 2018

AdSchellevis Feb 24, 2018

fichtner left a comment •

edited

Loading

fichtner Feb 25, 2018

AdSchellevis Feb 28, 2018

fabianfrz commented Mar 27, 2018

AdSchellevis commented Mar 27, 2018

AdSchellevis commented Apr 19, 2018

fabianfrz commented Apr 19, 2018

AdSchellevis commented Apr 20, 2018

fabianfrz commented Apr 20, 2018

AdSchellevis commented Apr 20, 2018

fabianfrz commented Apr 20, 2018

Harden GUI by adding a CSP, XSS protection and referrer hiding #2212

Harden GUI by adding a CSP, XSS protection and referrer hiding #2212

Conversation

fabianfrz commented Feb 18, 2018

fabianfrz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabianfrz Feb 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdSchellevis commented Feb 18, 2018

L1ghtn1ng commented Feb 18, 2018 via email

fabianfrz commented Feb 19, 2018

Choose a reason for hiding this comment

fichtner left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabianfrz commented Mar 27, 2018

AdSchellevis commented Mar 27, 2018

AdSchellevis commented Apr 19, 2018

fabianfrz commented Apr 19, 2018

AdSchellevis commented Apr 20, 2018

fabianfrz commented Apr 20, 2018

AdSchellevis commented Apr 20, 2018

fabianfrz commented Apr 20, 2018

fabianfrz Feb 24, 2018 •

edited

Loading

fichtner left a comment •

edited

Loading