diff --git a/doc/source/admin/remote-console-access.rst b/doc/source/admin/remote-console-access.rst index c59d0ce532f7..39f21ce782bc 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/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 5972ef83b85e..b8e9e99c3822 100644 --- a/nova/console/websocketproxy.py +++ b/nova/console/websocketproxy.py @@ -299,6 +299,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 531da1e73a4c..3c234df891ce 100644 --- a/nova/tests/unit/console/test_websocketproxy.py +++ b/nova/tests/unit/console/test_websocketproxy.py @@ -626,6 +626,22 @@ class NovaProxyRequestHandlerTestCase(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