When enforcing HTTPS, enforce cookies to be HTTPS-Only too

[Tilman Baumann]

A customer approached us about this "Severe Security Vulnerability".

Missing Secure Flag From SSL Cookie (http-cookie-secure-flag)
Description:
The Secure attribute tells the browser to only send the cookie if the request is being sent over a secure channel such as HTTPS. This will help protect the cookie from being passed over unencrypted requests. If the application can be accessed over both HTTP and HTTPS, then there is the potential that the cookie can be sent in clear text.

It seems reasonably simple and useful to set that Cookie in the Apache SSL frontend when https is enforced.

[Sahid Orentino (sahid-ferdjaoui)]

Agree it seems to be a reasonable ask.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/649274

[Nobuto Murata (nobuto)]

Hmm, I thought we set the secure flag in Django instead of Apache:
https://git.openstack.org/cgit/openstack/charm-openstack-dashboard/commit/?id=47396b52dea795548a22e71fe8e51a414c7d300d

[Tilman Baumann (tilmanbaumann)]

That seems even better. I just went the path of least resistance with the charm change.

The test was done with Horizon version: 3:11.0.4-0ubuntu1~cloud2.5

[Tilman Baumann (tilmanbaumann)]

While we are at it, the same "Security Scan" revealed that there is no CSR policy. Which is apparently "Critical" too. :D

I have thought of this below.
But honestly, I feel uneasy about jailing up someone else's code like that. I doubt the gains justify the potential fails.
Especially since it's hard to test I imagine.
I stole that from https://bugs.launchpad.net/openstack-ansible/+bug/1717321 which wasn't even for horizon necessarily.
This is probably the right way to go about it, revive this https://bugs.launchpad.net/horizon/+bug/1618024

diff --git a/templates/default-ssl b/templates/default-ssl
index 2a58808..ee57895 100644
--- a/templates/default-ssl
+++ b/templates/default-ssl
@@ -42,6 +42,8 @@
 {% endif %}
         Header set X-XSS-Protection "1; mode=block"
         Header set X-Content-Type-Options "nosniff"
+ Header set X-Frame-Options SAMEORIGIN
+ Header set Content-Security-Policy "default-src 'self' https: wss:;"
         <FilesMatch "\.(cgi|shtml|phtml|php)$">
                 SSLOptions +StdEnvVars
         </FilesMatch>

[Sahid Orentino (sahid-ferdjaoui)]

Hello Tilman,

Based on comment #3 It seems that we already set the correct flags for the cookies via Django. Can you provide more details on why you think it's not well configured?

Thanks,

[Xav Paice (xavpaice)]

A more recent scan of a cloud (Bionic, Queens, latest charms) resulted in a slightly better result, naming just two cookies that don't have the 'secure' flag set.

The output from the scan:

DESCRIPTION:
Secure is an additional flag included in a Set-Cookie HTTP response header. Using the Secure flag when generating a cookie helps mitigate the risk of interception of cookies sent over encrypted
communications, as otherwise they could be accessed outside of the Secure session. If the Secure flag is set on a cookie, then browsers will not submit the cookie in any requests that use an unencrypted HTTP connection, thereby preventing the cookie from being trivially intercepted by an attacker monitoring network traffic. If the Secure flag is not set, then the cookie will be transmitted in
cleartext whenever the user visits any HTTP URLs within the cookie's scope. An attacker may be able
to induce this event by feeding a user suitable links, either directly or via another web site. Even if the domain which issued the cookie does not host any content that is accessed over HTTP, an attacker may be able to use links of the form http://example.com:443/ to perform the same attack.
Note: The team found that all authorisation session cookies were configured securely.
EVIDENCE:
The following shows the cookies being set on OpenStack:
Set-Cookie: login_region="https://<keystone-url>:5000/v3";
expires=Thu, 21-May-2020 11:39:33 GMT; Max-Age=31536000; Path=/
Set-Cookie: login_domain=<domain>; expires=Thu, 21-May-2020 11:39:33 GMT; MaxAge=31536000; Path=/
AFFECTS:
The following host(s)/area(s) are affected:
• Openstack Cookies
o login_region
o login_domain

[Xav Paice (xavpaice)]

Subscribed field-medium as this is a commercial requirement, for a site running Bionic/queens.

[Xav Paice (xavpaice)]

It seems that even with the Django settings mentioned in #3, https://review.opendev.org/#/c/649274/ is still relevant as those particular settings are not included.

[Sahid Orentino (sahid-ferdjaoui)]

The Django settings mentioned in #3 seem to be related to the cookies managed by Django session. Horizon is managing some by is own [0].

We could probably start a discussion with horizon to fix that but at least we can add in charm another layer of security. The review [1] may be a good start.

[0] https://github.com/openstack/horizon/blob/master/openstack_auth/views.py
[1] https://review.opendev.org/#/c/649274/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/649274
Committed: https://git.openstack.org/cgit/openstack/charm-openstack-dashboard/commit/?id=101098a1c2c1d7e30f5d4406e599074dee5d3ce7
Submitter: Zuul
Branch: master

commit 101098a1c2c1d7e30f5d4406e599074dee5d3ce7
Author: Sahid Orentino Ferdjaoui <email address hidden>
Date: Tue Apr 2 12:14:23 2019 +0200

    apache2: add secure flag header when enforce_ssl

    The Secure attribute tells the browser to only send the cookie if the
    request is being sent over a secure channel such as HTTPS. This will
    help protect the cookie from being passed over unencrypted requests.

    Change-Id: I1ded951d79ad9fa832d1e88f656a1e064b1ef007
    Closes-bug: #1822751
    Signed-off-by: Sahid Orentino Ferdjaoui <email address hidden>

[Alex Kavanagh (ajkavanagh)]

Unfortunately, due to https://bugs.launchpad.net/charm-openstack-dashboard/+bug/1853173 this needs to be re-opened. The fix broke the dashboard with ssl-enforce is on, probably because the Angular code can't read the CSRF cookie.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/695918

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/19.10
Review: https://review.opendev.org/695920

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/695918
Committed: https://git.openstack.org/cgit/openstack/charm-openstack-dashboard/commit/?id=5375df0b12657e3494a966a48d04878aba8d153a
Submitter: Zuul
Branch: master

commit 5375df0b12657e3494a966a48d04878aba8d153a
Author: Alex Kavanagh <email address hidden>
Date: Mon Nov 25 13:05:40 2019 +0000

    Remove Set-Cookie .... HttpOnly;secure to allow CSRF access

    Angular (running in the page) can't access the CSRF token if the cookie
    is set to secure. This is a temporary patch to resolve the issue whilst
    a more permanent fix is found.

    This reverts patch I1ded951d79ad9fa832d1e88f656a1e064b1ef007
    (essentially).

    Change-Id: Id99abb429a0dc541ab5a3603962db8a563835eea
    Related-Bug: #1822751
    Closes-Bug: #1853173

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/695920
Committed: https://git.openstack.org/cgit/openstack/charm-openstack-dashboard/commit/?id=2ac72c39b480ae07df50edaac9802a7049773ca6
Submitter: Zuul
Branch: stable/19.10

commit 2ac72c39b480ae07df50edaac9802a7049773ca6
Author: Alex Kavanagh <email address hidden>
Date: Mon Nov 25 13:05:40 2019 +0000

    Remove Set-Cookie .... HttpOnly;secure to allow CSRF access

    Angular (running in the page) can't access the CSRF token if the cookie
    is set to secure. This is a temporary patch to resolve the issue whilst
    a more permanent fix is found.

    This reverts patch I1ded951d79ad9fa832d1e88f656a1e064b1ef007
    (essentially).

    Change-Id: Ied9d0f5486c260a17da9375ec6347d0952154225
    Related-Bug: #1822751
    Closes-Bug: #1853173

[OpenStack Infra (hudson-openstack)]

Change abandoned by Ryan Beisner (<email address hidden>) on branch: master
Review: https://review.opendev.org/649253
Reason: I believe this change is obseleted by https://review.opendev.org/#/c/649274. If that is not the case, please rebase and reopen your review. Thank you.

[Ryan Beisner (1chb1n)]

I believe this was fix-released @ https://review.opendev.org/#/q/topic:bug/1822751, but the automatic bug management didn't occur. If that is not the case, please reset this bug to a NEW status. Thank you.

[Alex Kavanagh (ajkavanagh)]

Due to https://review.opendev.org/#/c/695918 I've re-opened the bug, as the aforementioned review reverted the 'fix'.

tldr; the secure on breaks the page, due to Angular (in the horizon page) needing access to the CSRF token.

Re-reading the fix, if the "HttpOnly" bit is dropped, but the Secure left in, then the cookie will only be sent over the secure SSL channel, but Angular will still be able to read the cookie (not HttpOnly).

[Adam Dyess (addyess)]

The continued inability to set the flags on deployments where horizon is SSL, continues to trigger security vulnerability scans.

It was identified the template files for local_settings.py has a `ssl_configured` flag which isn't set in the context and in fact the context was deleted by https://github.com/openstack/charm-openstack-dashboard/commit/19915f68

Thanks for finding a way to address this bug

[Chris MacNaughton (chris.macnaughton)]

I'm removing field-medium from this bug as there is no contribution needing review.

[Chris MacNaughton (chris.macnaughton)]

Or maybe not, it looks like I can't remove that subscription. Either way, there isn't a change ready for review so this bug doesn't really qualify for field-medium

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/charm-openstack-dashboard/+/821546

[Alex Kavanagh (ajkavanagh)]

The associated review is being abandoned as it hasn't been worked on for a long time. I'm setting this back to incomplete to see if it is still an issue. If it is, please add a comment. Thanks.

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Alex Kavanagh <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/charm-openstack-dashboard/+/821546
Reason: Abandoning as not been worked on for 18 months.

[Launchpad Janitor (janitor)]

[Expired for OpenStack Dashboard Charm because there has been no activity for 60 days.]