diff --git a/doc/source/admin/remote-console-access.rst b/doc/source/admin/remote-console-access.rst index 2df3bd6739..f4fdbe94e1 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 18652f76e8..f127d62499 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 020d0aadf7..c2693e00f5 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 adbf23de2a..0d21321938 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 d47156fc2d..dcabd43fb7 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 e13b3c0fe1..1b09905160 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 7b2d53b120..01e87c5322 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 98e162d59c..0d8f9153af 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 0000000000..3fb073462b --- /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 282220fc4f..1726e191df 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