diff --git a/README.md b/README.md index 5cde2cb9..a5b46a9b 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,7 @@ This releases include a security fix. If you are using an earlier version, you s * Support MarkupSafe<3 for Debian bookworm * Mitigate CSRF on user's notification settings #216 [CVE-2022-3233](https://nvd.nist.gov/vuln/detail/CVE-2022-3233) * Mitigate CSRF on repository settings #217 +* Use 'Secure' Attribute with Sensitive Cookie in HTTPS Session on HTTP Error #218 [CVE-2022-3174](https://nvd.nist.gov/vuln/detail/CVE-2022-3174) ## 2.4.5 (2002-09-16) diff --git a/rdiffweb/controller/tests/test_secure_headers.py b/rdiffweb/controller/tests/test_secure_headers.py index 58063fe3..8255828c 100644 --- a/rdiffweb/controller/tests/test_secure_headers.py +++ b/rdiffweb/controller/tests/test_secure_headers.py @@ -19,6 +19,8 @@ @author: Patrik Dufresne """ +from parameterized import parameterized + import rdiffweb.test @@ -46,6 +48,24 @@ def test_cookie_with_https(self): # Given an https request made to rdiffweb self.getPage('/', headers=[('X-Forwarded-Proto', 'https')]) # When receiving the response + self.assertStatus(200) + # Then the header contains Set-Cookie with Secure + cookie = self.assertHeader('Set-Cookie') + self.assertIn('Secure', cookie) + + @parameterized.expand( + [ + ('/invalid', 404), + ('/browse/invalid', 404), + ('/login', 301), + ('/logout', 303), + ] + ) + def test_cookie_with_https_http_error(self, url, expected_error_code): + # Given an https request made to rdiffweb + self.getPage(url, headers=[('X-Forwarded-Proto', 'https')]) + # When receiving the response + self.assertStatus(expected_error_code) # Then the header contains Set-Cookie with Secure cookie = self.assertHeader('Set-Cookie') self.assertIn('Secure', cookie) diff --git a/rdiffweb/tools/secure_headers.py b/rdiffweb/tools/secure_headers.py index 202fc26b..c5d2bb70 100644 --- a/rdiffweb/tools/secure_headers.py +++ b/rdiffweb/tools/secure_headers.py @@ -19,7 +19,6 @@ import logging import cherrypy -from cherrypy._cptools import HandlerTool # Define the logger logger = logging.getLogger(__name__) @@ -32,7 +31,7 @@ http.cookies.Morsel._reserved['samesite'] = 'SameSite' -class SecureHeaders(HandlerTool): +def set_headers(): """ This tool provide CSRF mitigation. @@ -45,39 +44,27 @@ class SecureHeaders(HandlerTool): https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html https://cheatsheetseries.owasp.org/cheatsheets/Clickjacking_Defense_Cheat_Sheet.html """ + if cherrypy.request.method in ['POST', 'PUT', 'PATCH', 'DELETE']: + # Check if Origin matches our target. + origin = cherrypy.request.headers.get('Origin', None) + if origin and not origin.startswith(cherrypy.request.base): + raise cherrypy.HTTPError(403, 'Unexpected Origin header') - def __init__(self): - HandlerTool.__init__(self, self.run, name='secure_headers') - # Make sure to run before authform (priority 71) - self._priority = 71 + response = cherrypy.serving.response + # Define X-Frame-Options to avoid Clickjacking + response.headers['X-Frame-Options'] = 'DENY' - def _setup(self): - cherrypy.request.hooks.attach('before_finalize', self._set_headers) - return super()._setup() + # Enforce security on cookies + cookie = response.cookie.get('session_id', None) + if cookie: + # Awaiting bug fix in cherrypy + # https://github.com/cherrypy/cherrypy/issues/1767 + # Force SameSite to Lax + cookie['samesite'] = 'Lax' + # Check if https is enabled + https = cherrypy.request.base.startswith('https') + if https: + cookie['secure'] = 1 - def _set_headers(self): - response = cherrypy.serving.response - # Define X-Frame-Options to avoid Clickjacking - response.headers['X-Frame-Options'] = 'DENY' - # Enforce security on cookies - cookie = response.cookie.get('session_id', None) - if cookie: - # Awaiting bug fix in cherrypy - # https://github.com/cherrypy/cherrypy/issues/1767 - # Force SameSite to Lax - cookie['samesite'] = 'Lax' - # Check if https is enabled - https = cherrypy.request.base.startswith('https') - if https: - cookie['secure'] = 1 - - def run(self): - if cherrypy.request.method in ['POST', 'PUT', 'PATCH', 'DELETE']: - # Check if Origin matches our target. - origin = cherrypy.request.headers.get('Origin', None) - if origin and not origin.startswith(cherrypy.request.base): - raise cherrypy.HTTPError(403, 'Unexpected Origin header') - - -cherrypy.tools.secure_headers = SecureHeaders() +cherrypy.tools.secure_headers = cherrypy.Tool('before_request_body', set_headers, priority=71)