diff --git a/nova/conf/vnc.py b/nova/conf/vnc.py index 710bcdd3f364..608fd55808a3 100644 --- a/nova/conf/vnc.py +++ b/nova/conf/vnc.py @@ -230,7 +230,12 @@ first. Possible values: -* "none": allow connection without authentication +* ``none``: allow connection without authentication +* ``vencrypt``: use VeNCrypt authentication scheme + +Related options: + +* ``[vnc]vencrypt_client_key``, ``[vnc]vencrypt_client_cert``: must also be set """), cfg.StrOpt( 'vencrypt_client_key', @@ -241,8 +246,8 @@ proxy server presents to the compute node during VNC authentication. Related options: -* ``vnc.auth_schemes``: must include "vencrypt" -* ``vnc.vencrypt_client_cert```: must also be set +* ``vnc.auth_schemes``: must include ``vencrypt`` +* ``vnc.vencrypt_client_cert``: must also be set """), cfg.StrOpt( 'vencrypt_client_cert', @@ -253,8 +258,8 @@ the VNC proxy server presents to the compute node during VNC authentication. Realted options: -* ``vnc.auth_schemes``: must include "vencrypt" -* ``vnc.vencrypt_client_key```: must also be set +* ``vnc.auth_schemes``: must include ``vencrypt`` +* ``vnc.vencrypt_client_key``: must also be set """), cfg.StrOpt( 'vencrypt_ca_certs', @@ -265,7 +270,7 @@ for the certificate authorities used by the compute node VNC server. Related options: -* ``vnc.auth_schemes``: must include "vencrypt" +* ``vnc.auth_schemes``: must include ``vencrypt`` """), ] diff --git a/nova/console/rfb/auth.py b/nova/console/rfb/auth.py index 9a4a4ba28f84..f7d3d146593c 100644 --- a/nova/console/rfb/auth.py +++ b/nova/console/rfb/auth.py @@ -40,19 +40,6 @@ class AuthType(enum.IntEnum): MSLOGON = 0xfffffffa # Used by UltraVNC -class AuthVeNCryptSubtype(enum.IntEnum): - - PLAIN = 256 - TLSNONE = 257 - TLSVNC = 258 - TLSPLAIN = 259 - X509NONE = 260 - X509VNC = 261 - X509PLAIN = 262 - X509SASL = 263 - TLSSASL = 264 - - @six.add_metaclass(abc.ABCMeta) class RFBAuthScheme(object): @@ -74,5 +61,7 @@ class RFBAuthScheme(object): Should raise exception.RFBAuthHandshakeFailed if an error occurs + + :param compute_sock: socket connected to the compute node instance """ pass diff --git a/nova/console/rfb/auths.py b/nova/console/rfb/auths.py index 74828e161452..54b87e9afbc6 100644 --- a/nova/console/rfb/auths.py +++ b/nova/console/rfb/auths.py @@ -37,6 +37,13 @@ class RFBAuthSchemeList(object): self.schemes[scheme.security_type()] = scheme def find_scheme(self, desired_types): + """Find a suitable authentication scheme to use with compute node. + + Identify which of the ``desired_types`` we can accept. + + :param desired_types: A list of ints corresponding to the various + authentication types supported. + """ for security_type in desired_types: if security_type in self.schemes: return self.schemes[security_type] diff --git a/nova/console/rfb/authvencrypt.py b/nova/console/rfb/authvencrypt.py index 7e6fd873e296..6814ee97b07b 100644 --- a/nova/console/rfb/authvencrypt.py +++ b/nova/console/rfb/authvencrypt.py @@ -12,20 +12,39 @@ # License for the specific language governing permissions and limitations # under the License. +import enum import ssl import struct from oslo_config import cfg from oslo_log import log as logging +import six from nova.console.rfb import auth from nova import exception -from nova.i18n import _, _LI +from nova.i18n import _ LOG = logging.getLogger(__name__) CONF = cfg.CONF +class AuthVeNCryptSubtype(enum.IntEnum): + """Possible VeNCrypt subtypes. + + From https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst + """ + + PLAIN = 256 + TLSNONE = 257 + TLSVNC = 258 + TLSPLAIN = 259 + X509NONE = 260 + X509VNC = 261 + X509PLAIN = 262 + X509SASL = 263 + TLSSASL = 264 + + class RFBAuthSchemeVeNCrypt(auth.RFBAuthScheme): """A security proxy helper which uses VeNCrypt. @@ -81,17 +100,20 @@ class RFBAuthSchemeVeNCrypt(auth.RFBAuthScheme): LOG.debug("Server supports VeNCrypt sub-types %s", sub_types) - if auth.AuthVeNCryptSubtype.X509NONE not in sub_types: + # We use X509None as we're only seeking to encrypt the channel (ruling + # out PLAIN) and prevent MITM (ruling out TLS*, which uses trivially + # MITM'd Anonymous Diffie Hellmann (DH) cyphers) + if AuthVeNCryptSubtype.X509NONE not in sub_types: reason = _("Server does not support the x509None (%s) VeNCrypt" " sub-auth type") % \ - auth.AuthVeNCryptSubtype.X509NONE + AuthVeNCryptSubtype.X509NONE raise exception.RFBAuthHandshakeFailed(reason=reason) LOG.debug("Attempting to use the x509None (%s) auth sub-type", - auth.AuthVeNCryptSubtype.X509NONE) + AuthVeNCryptSubtype.X509NONE) compute_sock.sendall(struct.pack( - '!I', auth.AuthVeNCryptSubtype.X509NONE)) + '!I', AuthVeNCryptSubtype.X509NONE)) # NB(sross): the spec is missing a U8 here that's used in # multiple implementations (e.g. QEMU, GTK-VNC). 1 means @@ -120,9 +142,10 @@ class RFBAuthSchemeVeNCrypt(auth.RFBAuthScheme): cert_reqs=ssl.CERT_REQUIRED, ca_certs=CONF.vnc.vencrypt_ca_certs) - LOG.info(_LI("VeNCrypt security handshake accepted")) + LOG.info("VeNCrypt security handshake accepted") return wrapped_sock except ssl.SSLError as e: - reason = _("Error establishing TLS connection to server: %s") % e + reason = _("Error establishing TLS connection to server: %s") % ( + six.text_type(e)) raise exception.RFBAuthHandshakeFailed(reason=reason) diff --git a/nova/console/securityproxy/rfb.py b/nova/console/securityproxy/rfb.py index 6aea4712f947..c660f0f8fcea 100644 --- a/nova/console/securityproxy/rfb.py +++ b/nova/console/securityproxy/rfb.py @@ -23,7 +23,7 @@ from nova.console.rfb import auth from nova.console.rfb import auths from nova.console.securityproxy import base from nova import exception -from nova.i18n import _, _LI +from nova.i18n import _ LOG = logging.getLogger(__name__) @@ -46,6 +46,10 @@ class RFBSecurityProxy(base.SecurityProxy): See the general RFB specification at: https://tools.ietf.org/html/rfc6143 + + See an updated, maintained RDB specification at: + + https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst """ def __init__(self): @@ -67,7 +71,13 @@ class RFBSecurityProxy(base.SecurityProxy): # by sending the "Invalid" security type compute_sock.sendall(auth.AUTH_STATUS_FAIL) - def _parse_version(self, version_str): + @staticmethod + def _parse_version(version_str): + r"""Convert a version string to a float. + + >>> RFBSecurityProxy._parse_version('RFB 003.008\n') + 0.2 + """ maj_str = version_str[4:7] min_str = version_str[8:11] @@ -119,6 +129,7 @@ class RFBSecurityProxy(base.SecurityProxy): permitted_auth_types_cnt = six.byte2int(recv(compute_sock, 1)) if permitted_auth_types_cnt == 0: + # Decode the reason why the request failed reason_len_raw = recv(compute_sock, 4) reason_len = struct.unpack('!I', reason_len_raw)[0] reason = recv(compute_sock, reason_len) @@ -148,9 +159,8 @@ class RFBSecurityProxy(base.SecurityProxy): _("Only the security type None (%d) is supported") % auth.AuthType.NONE) - reason = _("Client requested a security type other than " - " None (%(none_code)d): " - "%(auth_type)s") % { + reason = _("Client requested a security type other than None " + "(%(none_code)d): %(auth_type)s") % { 'auth_type': client_auth, 'none_code': auth.AuthType.NONE} raise exception.SecurityProxyNegotiationFailed(reason=reason) @@ -179,10 +189,10 @@ class RFBSecurityProxy(base.SecurityProxy): _("Unable to negotiate security with server")) LOG.debug("Auth failed %s", six.text_type(e)) raise exception.SecurityProxyNegotiationFailed( - reason="Auth handshake failed") + reason=_("Auth handshake failed")) - LOG.info(_LI("Finished security handshake, resuming normal proxy " - "mode using secured socket")) + LOG.info("Finished security handshake, resuming normal proxy " + "mode using secured socket") # we can just proxy the security result -- if the server security # negotiation fails, we want the client to think it has failed diff --git a/nova/tests/unit/console/rfb/test_auth.py b/nova/tests/unit/console/rfb/test_auth.py index c3bc02daf759..c4026b6637a5 100644 --- a/nova/tests/unit/console/rfb/test_auth.py +++ b/nova/tests/unit/console/rfb/test_auth.py @@ -32,8 +32,8 @@ class RFBAuthSchemeListTestCase(test.NoDBTestCase): schemelist = auths.RFBAuthSchemeList() security_types = sorted(schemelist.schemes.keys()) - self.assertEqual(security_types, [auth.AuthType.NONE, - auth.AuthType.VENCRYPT]) + self.assertEqual([auth.AuthType.NONE, auth.AuthType.VENCRYPT], + security_types) def test_load_unknown(self): """Ensure invalid auth schemes are not supported. diff --git a/nova/tests/unit/console/rfb/test_authvencrypt.py b/nova/tests/unit/console/rfb/test_authvencrypt.py index 1c1c24cc49ce..f7fc31939e9c 100644 --- a/nova/tests/unit/console/rfb/test_authvencrypt.py +++ b/nova/tests/unit/console/rfb/test_authvencrypt.py @@ -58,8 +58,8 @@ class RFBAuthSchemeVeNCryptTestCase(test.NoDBTestCase): self._expect_recv(1, "\x00") self._expect_recv(1, "\x02") - subtypes_raw = [auth.AuthVeNCryptSubtype.X509NONE, - auth.AuthVeNCryptSubtype.X509VNC] + subtypes_raw = [authvencrypt.AuthVeNCryptSubtype.X509NONE, + authvencrypt.AuthVeNCryptSubtype.X509VNC] subtypes = struct.pack('!2I', *subtypes_raw) self._expect_recv(8, subtypes) @@ -89,8 +89,8 @@ class RFBAuthSchemeVeNCryptTestCase(test.NoDBTestCase): self._expect_recv(1, "\x00") self._expect_recv(1, "\x02") - subtypes_raw = [auth.AuthVeNCryptSubtype.X509NONE, - auth.AuthVeNCryptSubtype.X509VNC] + subtypes_raw = [authvencrypt.AuthVeNCryptSubtype.X509NONE, + authvencrypt.AuthVeNCryptSubtype.X509VNC] subtypes = struct.pack('!2I', *subtypes_raw) self._expect_recv(8, subtypes) @@ -140,7 +140,7 @@ class RFBAuthSchemeVeNCryptTestCase(test.NoDBTestCase): self._expect_recv(1, "\x00") self._expect_recv(1, "\x01") - subtypes_raw = [auth.AuthVeNCryptSubtype.X509VNC] + subtypes_raw = [authvencrypt.AuthVeNCryptSubtype.X509VNC] subtypes = struct.pack('!I', *subtypes_raw) self._expect_recv(4, subtypes) @@ -154,8 +154,8 @@ class RFBAuthSchemeVeNCryptTestCase(test.NoDBTestCase): self._expect_recv(1, "\x00") self._expect_recv(1, "\x02") - subtypes_raw = [auth.AuthVeNCryptSubtype.X509NONE, - auth.AuthVeNCryptSubtype.X509VNC] + subtypes_raw = [authvencrypt.AuthVeNCryptSubtype.X509NONE, + authvencrypt.AuthVeNCryptSubtype.X509VNC] subtypes = struct.pack('!2I', *subtypes_raw) self._expect_recv(8, subtypes) @@ -174,8 +174,8 @@ class RFBAuthSchemeVeNCryptTestCase(test.NoDBTestCase): self._expect_recv(1, "\x00") self._expect_recv(1, "\x02") - subtypes_raw = [auth.AuthVeNCryptSubtype.X509NONE, - auth.AuthVeNCryptSubtype.X509VNC] + subtypes_raw = [authvencrypt.AuthVeNCryptSubtype.X509NONE, + authvencrypt.AuthVeNCryptSubtype.X509VNC] subtypes = struct.pack('!2I', *subtypes_raw) self._expect_recv(8, subtypes) diff --git a/nova/tests/unit/console/securityproxy/test_rfb.py b/nova/tests/unit/console/securityproxy/test_rfb.py index e6c284e4d7f3..f53b3b805bb9 100644 --- a/nova/tests/unit/console/securityproxy/test_rfb.py +++ b/nova/tests/unit/console/securityproxy/test_rfb.py @@ -62,7 +62,7 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): self._expect_tenant_recv(auth.VERSION_LENGTH, full_version_str) def _to_binary(self, val): - if type(val) != six.binary_type: + if not isinstance(val, six.binary_type): val = six.binary_type(val, 'utf-8') return val @@ -87,6 +87,11 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): list(self.compute_sock.recv.side_effect) + [ret_val]) def test_fail(self): + """Validate behavior for invalid initial message from tenant. + + The spec defines the sequence that should be used in the handshaking + process. Anything outside of this is invalid. + """ self._expect_tenant_send("\x00\x00\x00\x01\x00\x00\x00\x04blah") self.proxy._fail(self.tenant_sock, None, 'blah') @@ -94,6 +99,11 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): self._assert_expected_calls() def test_fail_server_message(self): + """Validate behavior for invalid initial message from server. + + The spec defines the sequence that should be used in the handshaking + process. Anything outside of this is invalid. + """ self._expect_tenant_send("\x00\x00\x00\x01\x00\x00\x00\x04blah") self._expect_compute_send("\x00") @@ -102,20 +112,30 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): self._assert_expected_calls() def test_parse_version(self): + """Validate behavior of version parser.""" res = self.proxy._parse_version("RFB 012.034\n") self.assertEqual(12.34, res) def test_fails_on_compute_version(self): + """Validate behavior for unsupported compute RFB version. + + We only support RFB protocol version 3.8. + """ for full_version_str in ["RFB 003.007\n", "RFB 003.009\n"]: self._expect_compute_recv(auth.VERSION_LENGTH, full_version_str) - self.assertRaises(exception.SecurityProxyNegotiationFailed, - self.proxy.connect, - self.tenant_sock, - self.compute_sock) + ex = self.assertRaises(exception.SecurityProxyNegotiationFailed, + self.proxy.connect, + self.tenant_sock, + self.compute_sock) + self.assertIn('version 3.8, but server', six.text_type(ex)) self._assert_expected_calls() def test_fails_on_tenant_version(self): + """Validate behavior for unsupported tenant RFB version. + + We only support RFB protocol version 3.8. + """ full_version_str = "RFB 003.008\n" for full_version_str_invalid in ["RFB 003.007\n", "RFB 003.009\n"]: @@ -126,13 +146,19 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): self._expect_tenant_recv(auth.VERSION_LENGTH, full_version_str_invalid) - self.assertRaises(exception.SecurityProxyNegotiationFailed, - self.proxy.connect, - self.tenant_sock, - self.compute_sock) + ex = self.assertRaises(exception.SecurityProxyNegotiationFailed, + self.proxy.connect, + self.tenant_sock, + self.compute_sock) + self.assertIn('version 3.8, but tenant', six.text_type(ex)) self._assert_expected_calls() def test_fails_on_sec_type_cnt_zero(self): + """Validate behavior if a server returns 0 supported security types. + + This indicates a random issue and the cause of that issues should be + decoded and reported in the exception. + """ self.proxy._fail = mock.Mock() self._version_handshake() @@ -142,15 +168,17 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): self._expect_compute_recv(6, "cheese") self._expect_tenant_send("\x00\x00\x00\x00\x06cheese") - self.assertRaises(exception.SecurityProxyNegotiationFailed, - self.proxy.connect, - self.tenant_sock, - self.compute_sock) + ex = self.assertRaises(exception.SecurityProxyNegotiationFailed, + self.proxy.connect, + self.tenant_sock, + self.compute_sock) + self.assertIn('cheese', six.text_type(ex)) self._assert_expected_calls() @mock.patch.object(authnone.RFBAuthSchemeNone, "security_handshake") def test_full_run(self, mock_handshake): + """Validate correct behavior.""" new_sock = mock.MagicMock() mock_handshake.return_value = new_sock @@ -171,6 +199,7 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): self._assert_expected_calls() def test_client_auth_invalid_fails(self): + """Validate behavior if no security types are supported.""" self.proxy._fail = self.manager.proxy._fail self.proxy.security_handshake = self.manager.proxy.security_handshake @@ -195,6 +224,7 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): self._assert_expected_calls() def test_exception_in_choose_security_type_fails(self): + """Validate behavior if a given security type isn't supported.""" self.proxy._fail = self.manager.proxy._fail self.proxy.security_handshake = self.manager.proxy.security_handshake @@ -220,6 +250,7 @@ class RFBSecurityProxyTestCase(test.NoDBTestCase): @mock.patch.object(authnone.RFBAuthSchemeNone, "security_handshake") def test_exception_security_handshake_fails(self, mock_auth): + """Validate behavior if the security handshake fails for any reason.""" self.proxy._fail = self.manager.proxy._fail self._version_handshake() diff --git a/releasenotes/notes/websocket-proxy-to-host-security-c3eca0647b0cbc02.yaml b/releasenotes/notes/websocket-proxy-to-host-security-c3eca0647b0cbc02.yaml index f9588c3b4b84..2fa2188db4af 100644 --- a/releasenotes/notes/websocket-proxy-to-host-security-c3eca0647b0cbc02.yaml +++ b/releasenotes/notes/websocket-proxy-to-host-security-c3eca0647b0cbc02.yaml @@ -17,8 +17,7 @@ features: ensure it is connecting to a valid compute node. The proxy can also send its own x509 cert to allow the compute node to validate that the connection comes from the official proxy server. -upgrade: - - | + To make use of VeNCrypt, configuration steps are required for both the `nova-novncproxy` service and libvirt on all the compute nodes. The ``/etc/libvirt/qemu.conf`` file should be modified to set the ``vnc_tls``