From 9a7e00dc9d22986a4293b02343ccd53033c83d31 Mon Sep 17 00:00:00 2001 From: --global Date: Mon, 12 Aug 2019 10:22:50 +0300 Subject: [PATCH 1/5] Re-enable HTTPS as an option. HTTPS was previously disabled in standalone mode (not running with a TLS termination proxy) due to difficulties in getting it properly running. Now the problems are fixed and running with HTTPS has been enabled again. Also added CLI options for specifying the SSL certificates for HTTPS. --- swift_browser_ui/server.py | 50 ++++++++++++++++++++++++-------------- swift_browser_ui/shell.py | 21 ++++++++++++++-- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/swift_browser_ui/server.py b/swift_browser_ui/server.py index b0d2f7ca9..8c436fe3b 100644 --- a/swift_browser_ui/server.py +++ b/swift_browser_ui/server.py @@ -1,13 +1,13 @@ """swift_browser_ui server related convenience functions.""" # Generic imports -# import ssl import logging import time import sys import asyncio import hashlib import os +import ssl import uvloop import cryptography.fernet @@ -124,23 +124,37 @@ async def servinit(): return app -# def run_server_secure(app): -# """ -# Run the server securely with a given ssl context. - -# While this function is incomplete, the project is safe to run in -# production only via a TLS termination proxy with e.g. NGINX. -# """ - # Setup ssl context - # sslcontext = ssl.create_default_context() - # sslcontext.set_ciphers( - # 'ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE' + - # '-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-' + - # 'AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-' + - # 'SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-' + - # 'RSA-AES128-SHA256' - # ) - # aiohttp.web.run_app(app, ssl_context=sslcontext) +def run_server_secure(app, cert_file, cert_key): + """ + Run the server securely with a given ssl context. + + While this function is incomplete, the project is safe to run in + production only via a TLS termination proxy with e.g. NGINX. + """ + logger = logging.getLogger("swift-browser-ui") + logger.debug("Running server securely.") + logger.debug("Setting up SSL context for the server.") + sslcontext = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH) + cipher_str = ( + 'ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE' + + '-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-' + + 'AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-' + + 'SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-' + + 'RSA-AES128-SHA256' + ) + logger.debug( + "Setting following ciphers for SSL context: \n%s", + cipher_str + ) + sslcontext.set_ciphers(cipher_str) + logger.debug("Loading cert chain.") + sslcontext.load_cert_chain(cert_file, cert_key) + aiohttp.web.run_app( + app, + access_log=aiohttp.web.logging.getLogger('aiohttp.access'), + port=setd['port'], + ssl_context=sslcontext, + ) def run_server_insecure(app): diff --git a/swift_browser_ui/shell.py b/swift_browser_ui/shell.py index 5159c3288..c06fdecc3 100644 --- a/swift_browser_ui/shell.py +++ b/swift_browser_ui/shell.py @@ -10,7 +10,7 @@ from .__init__ import __version__ from .settings import setd, set_key, FORMAT -from .server import servinit, run_server_insecure +from .server import servinit, run_server_insecure, run_server_secure from ._convenience import setup_logging as conv_setup_logging @@ -92,6 +92,18 @@ def cli(verbose, debug, logfile): @click.option( '--set-session-devmode', is_flag=True, default=False, hidden=True, ) +@click.option( + '--secure', is_flag=True, default=False, + help="Enable secure running, i.e. enable HTTPS." +) +@click.option( + '--ssl-cert-file', default=None, type=str, + help="Specify the certificate to use with SSL." +) +@click.option( + '--ssl-cert-key', default=None, type=str, + help="Specify the certificate key to use with SSL." +) def start( port, auth_endpoint_url, @@ -99,6 +111,9 @@ def start( dry_run, set_origin_address, set_session_devmode, + secure, + ssl_cert_file, + ssl_cert_key, ): """Start the browser backend and server.""" logging.debug( @@ -129,8 +144,10 @@ def start( logging.debug( "Running settings directory:%s", str(setd) ) - if not dry_run: + if not dry_run and not secure: run_server_insecure(servinit()) + if not dry_run and secure: + run_server_secure(servinit(), ssl_cert_file, ssl_cert_key) def main(): From 3434b55c0a8588ba7f52653ef3f1d908b2a74498 Mon Sep 17 00:00:00 2001 From: --global Date: Mon, 12 Aug 2019 10:30:16 +0300 Subject: [PATCH 2/5] Update documentation after introducing SSL. --- docs/source/instructions.rst | 9 ++++----- docs/source/usage.rst | 9 +++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/source/instructions.rst b/docs/source/instructions.rst index 3c98670e0..bc64005a1 100644 --- a/docs/source/instructions.rst +++ b/docs/source/instructions.rst @@ -55,11 +55,10 @@ For the Pouta production environment for testing unsecurely without trust:: Setting up TLS termination proxy ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The backend itself is not meant to be run as standalone in a production -environment. Therefore in a running config a TLS termination proxy should be -used to make the service secure. Setting up TLS termination is outside the -scope of this documentation, but a few useful links are provided along with -the necessary configs regarding this service. [#]_ [#]_ +The backend can be run in secure mode, i.e. with HTTPS enabled, but for +scaling up a TLS termination proxy is recommended. Setting up TLS termination +is outside the scope of this documentation, but a few useful links are +provided along with the necessary configs regarding this service. [#]_ [#]_ Scaling up the service ---------------------- diff --git a/docs/source/usage.rst b/docs/source/usage.rst index a36efb874..82a41bfcb 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -68,6 +68,9 @@ The following command line arguments are available for server startup. trusted_dashboards in the specified address. --set-origin-address TEXT Set the address that the program will be redirected to from WebSSO + --secure Enable secure running, i.e. enable HTTPS. + --ssl-cert-file TEXT Specify the certificate to use with SSL. + --ssl-cert-key TEXT Specify the certificate key to use with SSL. --help Show this message and exit. @@ -81,5 +84,11 @@ The following command line arguments are available for server startup. authentication endpoint, i.e. if the program has been listed on the respective Openstack keystone trusted_dashboard list. [#]_ +--secure Enable HTTPS on the server, to enable secure + requests if there's no TLS termination proxy. +--ssl-cert-file TEXT Specify SSL certificate file. Required when + running in secure mode. +--ssl-cert-key TEXT Specify SSL certificate key. Required when + running in secure mode. .. [#] https://docs.openstack.org/keystone/pike/advanced-topics/federation/websso.html From 234c6e6904bc276936e102b8aa276ce5f9ebb244 Mon Sep 17 00:00:00 2001 From: --global Date: Mon, 12 Aug 2019 11:11:32 +0300 Subject: [PATCH 3/5] Add server run and shutdown function tests. --- tests/test_server.py | 52 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tests/test_server.py b/tests/test_server.py index 1de067754..c1e153f3f 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -6,13 +6,19 @@ import os +import unittest +import ssl from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop +import aiohttp import asynctest -from swift_browser_ui.server import servinit +from swift_browser_ui.server import servinit, run_server_insecure +from swift_browser_ui.server import kill_sess_on_shutdown, run_server_secure from swift_browser_ui.settings import setd +from .creation import get_request_with_mock_openstack + # Set static folder in settings so it can be tested setd['static_directory'] = os.getcwd() + '/swift_browser_ui_frontend' @@ -29,6 +35,50 @@ async def test_servinit(self): self.assertTrue(app is not None) +class TestServerShutdownHandler(asynctest.TestCase): + """Test case for the server graceful shutdown handler.""" + + async def test_kill_sess_on_shutdown(self): + """Test kill_sess_on_shutdown function.""" + session, req = get_request_with_mock_openstack() + + await kill_sess_on_shutdown(req.app) + + self.assertNotIn(session, req.app["Creds"].keys()) + + +class TestRunServerFunctions(unittest.TestCase): + """Test class for server run functions.""" + + @staticmethod + def mock_ssl_context_creation(purpose=None): + """Return a MagicMock instance of an ssl context.""" + return unittest.mock.MagicMock(ssl.create_default_context)() + + def test_run_server_secure(self): + """Test run_server_secure function.""" + run_app_mock = unittest.mock.MagicMock(aiohttp.web.run_app) + patch_run_app = unittest.mock.patch( + "swift_browser_ui.server.aiohttp.web.run_app", run_app_mock + ) + patch_ssl_defcontext = unittest.mock.patch( + "swift_browser_ui.server.ssl.create_default_context", + self.mock_ssl_context_creation + ) + with patch_run_app, patch_ssl_defcontext: + run_server_secure(None, None, None) + run_app_mock.assert_called_once() + + def test_run_server_insecure(self): + """Test run_server_insecure function.""" + run_app_mock = unittest.mock.MagicMock(aiohttp.web.run_app) + with unittest.mock.patch( + "swift_browser_ui.server.aiohttp.web.run_app", run_app_mock + ): + run_server_insecure(None) + run_app_mock.assert_called_once() + + # After testing the server initialization, we can use the correctly starting # server for testing other modules. class AppTestCase(AioHTTPTestCase): From 24c521a7ca32e077d7eacf32fa5f2e9455fc339d Mon Sep 17 00:00:00 2001 From: --global Date: Mon, 19 Aug 2019 13:22:47 +0300 Subject: [PATCH 4/5] Add source for the SSL/TLS ciphers. --- swift_browser_ui/server.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/swift_browser_ui/server.py b/swift_browser_ui/server.py index 8c436fe3b..ee210534f 100644 --- a/swift_browser_ui/server.py +++ b/swift_browser_ui/server.py @@ -131,6 +131,9 @@ def run_server_secure(app, cert_file, cert_key): While this function is incomplete, the project is safe to run in production only via a TLS termination proxy with e.g. NGINX. """ + # The chiphers are from the Mozilla project wiki, as a recommendation for + # the most secure and up-to-date build. + # https://wiki.mozilla.org/Security/Server_Side_TLS logger = logging.getLogger("swift-browser-ui") logger.debug("Running server securely.") logger.debug("Setting up SSL context for the server.") From 04fac28483dc86157d3731a67535a50084fcad32 Mon Sep 17 00:00:00 2001 From: --global Date: Mon, 19 Aug 2019 13:30:38 +0300 Subject: [PATCH 5/5] Update ciphers, lock out TLSv1 and TLSv1_1 as deprecated --- swift_browser_ui/server.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/swift_browser_ui/server.py b/swift_browser_ui/server.py index ee210534f..2ddb9a218 100644 --- a/swift_browser_ui/server.py +++ b/swift_browser_ui/server.py @@ -139,17 +139,18 @@ def run_server_secure(app, cert_file, cert_key): logger.debug("Setting up SSL context for the server.") sslcontext = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH) cipher_str = ( - 'ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE' + - '-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-' + - 'AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-' + - 'SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-' + - 'RSA-AES128-SHA256' + "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE" + + "-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA" + + "-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM" + + "-SHA256:DHE-RSA-AES256-GCM-SHA384" ) logger.debug( "Setting following ciphers for SSL context: \n%s", cipher_str ) sslcontext.set_ciphers(cipher_str) + sslcontext.options |= ssl.OP_NO_TLSv1 + sslcontext.options |= ssl.OP_NO_TLSv1_1 logger.debug("Loading cert chain.") sslcontext.load_cert_chain(cert_file, cert_key) aiohttp.web.run_app(