From 08bdcdb5b6866c2b6bf084344cca4dd07b960133 Mon Sep 17 00:00:00 2001 From: Nathan Kinder Date: Fri, 30 Aug 2019 12:24:03 -0700 Subject: [PATCH] Allow TLS ciphers/protocols to be configurable for console proxies The console proxies (VNC, SPICE, etc) currently don't allow the allowed TLS ciphers and protocol versions to be configurable. This results in the defaults being used from the underlying system, which may not be secure enough for many deployments. This patch allows for the ciphers and minimum SSL/TLS protocol version for each console proxy to be configured in nova's config. We utilize websockify underneath our console proxies, which added support for allowed ciphers and the SSL/TLS version to be configurable as of version 0.9.0. This change updates the lower constraint for this dependency. Closes-Bug: #1842149 Related-Bug: #1771773 Change-Id: I23ac1cc79482d0fabb359486a4b934463854cae5 --- doc/source/admin/remote-console-access.rst | 6 +++ lower-constraints.txt | 2 +- nova/cmd/baseproxy.py | 2 + nova/conf/console.py | 38 +++++++++++++++++++ nova/conf/novnc.py | 28 ++++++++++++-- nova/console/websocketproxy.py | 11 ++++++ nova/tests/unit/cmd/test_baseproxy.py | 29 +++++++++++--- .../tests/unit/console/test_websocketproxy.py | 16 ++++++++ .../notes/bug-1842149-5ba20d57872e9996.yaml | 17 +++++++++ requirements.txt | 2 +- 10 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/bug-1842149-5ba20d57872e9996.yaml diff --git a/doc/source/admin/remote-console-access.rst b/doc/source/admin/remote-console-access.rst index 2df3bd6739a3..f4fdbe94e193 100644 --- a/doc/source/admin/remote-console-access.rst +++ b/doc/source/admin/remote-console-access.rst @@ -105,6 +105,8 @@ The :program:`nova-novncproxy` service accepts the following options: - :oslo.config:option:`cert` - :oslo.config:option:`key` - :oslo.config:option:`web` +- :oslo.config:option:`console.ssl_ciphers` +- :oslo.config:option:`console.ssl_minimum_version` - :oslo.config:option:`vnc.novncproxy_host` - :oslo.config:option:`vnc.novncproxy_port` @@ -326,6 +328,8 @@ The :program:`nova-spicehtml5proxy` service accepts the following options. - :oslo.config:option:`cert` - :oslo.config:option:`key` - :oslo.config:option:`web` +- :oslo.config:option:`console.ssl_ciphers` +- :oslo.config:option:`console.ssl_minimum_version` - :oslo.config:option:`spice.html5proxy_host` - :oslo.config:option:`spice.html5proxy_port` @@ -407,6 +411,8 @@ The :program:`nova-serialproxy` service accepts the following options. - :oslo.config:option:`cert` - :oslo.config:option:`key` - :oslo.config:option:`web` +- :oslo.config:option:`console.ssl_ciphers` +- :oslo.config:option:`console.ssl_minimum_version` - :oslo.config:option:`serial_console.serialproxy_host` - :oslo.config:option:`serial_console.serialproxy_port` diff --git a/lower-constraints.txt b/lower-constraints.txt index 18652f76e83c..f127d6249964 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -166,7 +166,7 @@ vine==1.1.4 voluptuous==0.11.1 warlock==1.2.0 WebOb==1.8.2 -websockify==0.8.0 +websockify==0.9.0 wrapt==1.10.11 wsgi-intercept==1.7.0 zVMCloudConnector==1.3.0 diff --git a/nova/cmd/baseproxy.py b/nova/cmd/baseproxy.py index 020d0aadf788..c2693e00f5cd 100644 --- a/nova/cmd/baseproxy.py +++ b/nova/cmd/baseproxy.py @@ -72,6 +72,8 @@ def proxy(host, port, security_proxy=None): cert=CONF.cert, key=CONF.key, ssl_only=CONF.ssl_only, + ssl_ciphers=CONF.console.ssl_ciphers, + ssl_minimum_version=CONF.console.ssl_minimum_version, daemon=CONF.daemon, record=CONF.record, traffic=not CONF.daemon, diff --git a/nova/conf/console.py b/nova/conf/console.py index adbf23de2a52..0d21321938b1 100644 --- a/nova/conf/console.py +++ b/nova/conf/console.py @@ -40,6 +40,44 @@ values other than host are allowed in the origin header. Possible values: * A list where each element is an allowed origin hostnames, else an empty list +"""), + cfg.StrOpt('ssl_ciphers', + help=""" +OpenSSL cipher preference string that specifies what ciphers to allow for TLS +connections from clients. For example:: + + ssl_ciphers = "kEECDH+aECDSA+AES:kEECDH+AES+aRSA:kEDH+aRSA+AES" + +See the man page for the OpenSSL `ciphers` command for details of the cipher +preference string format and allowed values:: + + https://www.openssl.org/docs/man1.1.0/man1/ciphers.html + +Related options: + +* [DEFAULT] cert +* [DEFAULT] key +"""), + cfg.StrOpt('ssl_minimum_version', + default='default', + choices=[ + # These values must align with SSL_OPTIONS in + # websockify/websocketproxy.py + ('default', 'Use the underlying system OpenSSL defaults'), + ('tlsv1_1', + 'Require TLS v1.1 or greater for TLS connections'), + ('tlsv1_2', + 'Require TLS v1.2 or greater for TLS connections'), + ('tlsv1_3', + 'Require TLS v1.3 or greater for TLS connections'), + ], + help=""" +Minimum allowed SSL/TLS protocol version. + +Related options: + +* [DEFAULT] cert +* [DEFAULT] key """), ] diff --git a/nova/conf/novnc.py b/nova/conf/novnc.py index d47156fc2d01..dcabd43fb718 100644 --- a/nova/conf/novnc.py +++ b/nova/conf/novnc.py @@ -27,15 +27,37 @@ If this is not set, no recording will be done. help="Run as a background process."), cfg.BoolOpt('ssl_only', default=False, - help="Disallow non-encrypted connections."), + help=""" +Disallow non-encrypted connections. + +Related options: + +* cert +* key +"""), cfg.BoolOpt('source_is_ipv6', default=False, help="Set to True if source host is addressed with IPv6."), cfg.StrOpt('cert', default='self.pem', - help="Path to SSL certificate file."), + help=""" +Path to SSL certificate file. + +Related options: + +* key +* ssl_only +* [console] ssl_ciphers +* [console] ssl_minimum_version +"""), cfg.StrOpt('key', - help="SSL key file (if separate from cert)."), + help=""" +SSL key file (if separate from cert). + +Related options: + +* cert +"""), cfg.StrOpt('web', default='/usr/share/spice-html5', help=""" diff --git a/nova/console/websocketproxy.py b/nova/console/websocketproxy.py index e13b3c0fe153..1b099051608e 100644 --- a/nova/console/websocketproxy.py +++ b/nova/console/websocketproxy.py @@ -316,6 +316,17 @@ class NovaWebSocketProxy(websockify.WebSocketProxy): with the compute node. """ self.security_proxy = kwargs.pop('security_proxy', None) + + # If 'default' was specified as the ssl_minimum_version, we leave + # ssl_options unset to default to the underlying system defaults. + # We do this to avoid using websockify's behaviour for 'default' + # in select_ssl_version(), which hardcodes the versions to be + # quite relaxed and prevents us from using sytem crypto policies. + ssl_min_version = kwargs.pop('ssl_minimum_version', None) + if ssl_min_version and ssl_min_version != 'default': + kwargs['ssl_options'] = websockify.websocketproxy. \ + select_ssl_version(ssl_min_version) + super(NovaWebSocketProxy, self).__init__(*args, **kwargs) @staticmethod diff --git a/nova/tests/unit/cmd/test_baseproxy.py b/nova/tests/unit/cmd/test_baseproxy.py index 7b2d53b12089..01e87c532288 100644 --- a/nova/tests/unit/cmd/test_baseproxy.py +++ b/nova/tests/unit/cmd/test_baseproxy.py @@ -57,17 +57,20 @@ class BaseProxyTestCase(test.NoDBTestCase): @mock.patch.object(logging, 'setup') @mock.patch.object(gmr.TextGuruMeditation, 'setup_autorun') @mock.patch('nova.console.websocketproxy.NovaWebSocketProxy.__init__', - return_value=None) + return_value=None) @mock.patch('nova.console.websocketproxy.NovaWebSocketProxy.start_server') - def test_proxy(self, mock_start, mock_init, mock_gmr, mock_log, - mock_exists): + @mock.patch('websockify.websocketproxy.select_ssl_version', + return_value=None) + def test_proxy(self, mock_select_ssl_version, mock_start, mock_init, + mock_gmr, mock_log, mock_exists): baseproxy.proxy('0.0.0.0', '6080') mock_log.assert_called_once_with(baseproxy.CONF, 'nova') mock_gmr.assert_called_once_with(version, conf=baseproxy.CONF) mock_init.assert_called_once_with( listen_host='0.0.0.0', listen_port='6080', source_is_ipv6=False, - cert='self.pem', key=None, ssl_only=False, - daemon=False, record=None, security_proxy=None, traffic=True, + cert='self.pem', key=None, ssl_only=False, ssl_ciphers=None, + ssl_minimum_version='default', daemon=False, record=None, + security_proxy=None, traffic=True, web='/usr/share/spice-html5', file_only=True, RequestHandlerClass=websocketproxy.NovaProxyRequestHandler) mock_start.assert_called_once_with() @@ -81,3 +84,19 @@ class BaseProxyTestCase(test.NoDBTestCase): self.assertEqual(self.stderr.getvalue(), "SSL only and self.pem not found\n") mock_exit.assert_called_once_with(-1) + + @mock.patch('os.path.exists', return_value=True) + @mock.patch('nova.console.websocketproxy.NovaWebSocketProxy.__init__', + return_value=None) + @mock.patch('nova.console.websocketproxy.NovaWebSocketProxy.start_server') + def test_proxy_ssl_settings(self, mock_start, mock_init, mock_exists): + self.flags(ssl_minimum_version='tlsv1_3', group='console') + self.flags(ssl_ciphers='ALL:!aNULL', group='console') + baseproxy.proxy('0.0.0.0', '6080') + mock_init.assert_called_once_with( + listen_host='0.0.0.0', listen_port='6080', source_is_ipv6=False, + cert='self.pem', key=None, ssl_only=False, + ssl_ciphers='ALL:!aNULL', ssl_minimum_version='tlsv1_3', + daemon=False, record=None, security_proxy=None, traffic=True, + web='/usr/share/spice-html5', file_only=True, + RequestHandlerClass=websocketproxy.NovaProxyRequestHandler) diff --git a/nova/tests/unit/console/test_websocketproxy.py b/nova/tests/unit/console/test_websocketproxy.py index 98e162d59cb6..0d8f9153afcc 100644 --- a/nova/tests/unit/console/test_websocketproxy.py +++ b/nova/tests/unit/console/test_websocketproxy.py @@ -626,6 +626,22 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): self.wh.server.top_new_client(conn, address) self.assertIsNone(self.wh._compute_rpcapi) + @mock.patch('websockify.websocketproxy.select_ssl_version') + def test_ssl_min_version_is_not_set(self, mock_select_ssl): + websocketproxy.NovaWebSocketProxy() + self.assertFalse(mock_select_ssl.called) + + @mock.patch('websockify.websocketproxy.select_ssl_version') + def test_ssl_min_version_not_set_by_default(self, mock_select_ssl): + websocketproxy.NovaWebSocketProxy(ssl_minimum_version='default') + self.assertFalse(mock_select_ssl.called) + + @mock.patch('websockify.websocketproxy.select_ssl_version') + def test_non_default_ssl_min_version_is_set(self, mock_select_ssl): + minver = 'tlsv1_3' + websocketproxy.NovaWebSocketProxy(ssl_minimum_version=minver) + mock_select_ssl.assert_called_once_with(minver) + class NovaWebsocketSecurityProxyTestCase(test.NoDBTestCase): diff --git a/releasenotes/notes/bug-1842149-5ba20d57872e9996.yaml b/releasenotes/notes/bug-1842149-5ba20d57872e9996.yaml new file mode 100644 index 000000000000..3fb073462bc5 --- /dev/null +++ b/releasenotes/notes/bug-1842149-5ba20d57872e9996.yaml @@ -0,0 +1,17 @@ +--- +other: + - | + A new pair of ``ssl_ciphers`` and ``ssl_minimum_version`` configuration + options have been introduced for use by the ``nova-novncproxy``, + ``nova-serialproxy``, and ``nova-spicehtml5proxy`` services. These new + options allow one to configure the allowed TLS ciphers and minimum protocol + version to enforce for incoming client connections to the proxy services. + + This aims to address the issues reported in `bug 1842149`_, where it + describes that the proxy services can inherit insecure TLS ciphers + and protocol versions from the compiled-in defaults of the OpenSSL + library on the underlying system. The proxy services provided no way + to override such insecure defaults with current day generally accepted + secure TLS settings. + + .. _bug 1842149: https://bugs.launchpad.net/nova/+bug/1842149 diff --git a/requirements.txt b/requirements.txt index 282220fc4f01..1726e191dfc8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,7 +32,7 @@ python-glanceclient>=2.8.0 # Apache-2.0 requests>=2.14.2 # Apache-2.0 six>=1.10.0 # MIT stevedore>=1.20.0 # Apache-2.0 -websockify>=0.8.0 # LGPLv3 +websockify>=0.9.0 # LGPLv3 oslo.cache>=1.26.0 # Apache-2.0 oslo.concurrency>=3.26.0 # Apache-2.0 oslo.config>=6.1.0 # Apache-2.0