From 3b0f4cf6bea33e6ee1893f6e872d968b0c309f88 Mon Sep 17 00:00:00 2001 From: John Herndon Date: Tue, 19 Feb 2013 22:53:49 +0000 Subject: [PATCH] Flush tokens on instance delete Force console auth service to flush all tokens associated with an instance when it is deleted. This will fix bug 1125378, where the console for the wrong instance can be connected to via the console if the correct circumstances occur. This change also adds a call to validate the token when it is used. This check will ensure that all tokens are valid for their target instances. Tokens can become scrambled when a compute node is restarted, because the virt driver may not assign ports in the same way. Change-Id: I0d83ec6c4dbfef1af912a200ee15f8052f72da96 fixes: bug 1125378 --- nova/common/memorycache.py | 5 ++ nova/compute/api.py | 12 ++-- nova/compute/cells_api.py | 6 +- nova/compute/manager.py | 19 +++++- nova/compute/rpcapi.py | 10 +++ nova/consoleauth/manager.py | 45 ++++++++++++- nova/consoleauth/rpcapi.py | 14 +++- nova/tests/compute/test_compute.py | 77 ++++++++++++++++++++-- nova/tests/compute/test_rpcapi.py | 6 ++ nova/tests/consoleauth/test_consoleauth.py | 64 +++++++++++++++++- nova/tests/consoleauth/test_rpcapi.py | 8 ++- 11 files changed, 247 insertions(+), 19 deletions(-) diff --git a/nova/common/memorycache.py b/nova/common/memorycache.py index f89e4b265d3..c124784d5c0 100644 --- a/nova/common/memorycache.py +++ b/nova/common/memorycache.py @@ -83,3 +83,8 @@ def incr(self, key, delta=1): new_value = int(value) + delta self.cache[key] = (self.cache[key][0], str(new_value)) return new_value + + def delete(self, key, time=0): + """Deletes the value associated with a key.""" + if key in self.cache: + del self.cache[key] diff --git a/nova/compute/api.py b/nova/compute/api.py index cc07a998a05..f917e379d77 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2189,8 +2189,9 @@ def get_vnc_console(self, context, instance, console_type): instance=instance, console_type=console_type) self.consoleauth_rpcapi.authorize_console(context, - connect_info['token'], console_type, connect_info['host'], - connect_info['port'], connect_info['internal_access_path']) + connect_info['token'], console_type, + connect_info['host'], connect_info['port'], + connect_info['internal_access_path'], instance['uuid']) return {'url': connect_info['access_url']} @@ -2207,10 +2208,11 @@ def get_spice_console(self, context, instance, console_type): """Get a url to an instance Console.""" connect_info = self.compute_rpcapi.get_spice_console(context, instance=instance, console_type=console_type) - + print connect_info self.consoleauth_rpcapi.authorize_console(context, - connect_info['token'], console_type, connect_info['host'], - connect_info['port'], connect_info['internal_access_path']) + connect_info['token'], console_type, + connect_info['host'], connect_info['port'], + connect_info['internal_access_path'], instance['uuid']) return {'url': connect_info['access_url']} diff --git a/nova/compute/cells_api.py b/nova/compute/cells_api.py index 1e30331bc25..22e31a8e1b0 100644 --- a/nova/compute/cells_api.py +++ b/nova/compute/cells_api.py @@ -465,7 +465,8 @@ def get_vnc_console(self, context, instance, console_type): self.consoleauth_rpcapi.authorize_console(context, connect_info['token'], console_type, connect_info['host'], - connect_info['port'], connect_info['internal_access_path']) + connect_info['port'], connect_info['internal_access_path'], + instance_uuid=instance['uuid']) return {'url': connect_info['access_url']} @wrap_check_policy @@ -480,7 +481,8 @@ def get_spice_console(self, context, instance, console_type): self.consoleauth_rpcapi.authorize_console(context, connect_info['token'], console_type, connect_info['host'], - connect_info['port'], connect_info['internal_access_path']) + connect_info['port'], connect_info['internal_access_path'], + instance_uuid=instance['uuid']) return {'url': connect_info['access_url']} @validate_cell diff --git a/nova/compute/manager.py b/nova/compute/manager.py index afeb9f02e28..4a6ab8eb50d 100755 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -50,6 +50,7 @@ from nova.compute import utils as compute_utils from nova.compute import vm_states from nova import conductor +from nova import consoleauth import nova.context from nova import exception from nova import hooks @@ -317,7 +318,7 @@ def agent_build_get_by_triple(self, context, hypervisor, os, architecture): class ComputeManager(manager.SchedulerDependentManager): """Manages the running instances from creation to destruction.""" - RPC_API_VERSION = '2.25' + RPC_API_VERSION = '2.26' def __init__(self, compute_driver=None, *args, **kwargs): """Load configuration options and connect to the hypervisor.""" @@ -335,6 +336,8 @@ def __init__(self, compute_driver=None, *args, **kwargs): self.conductor_api = conductor.API() self.is_quantum_security_groups = ( openstack_driver.is_quantum_security_groups()) + self.consoleauth_rpcapi = consoleauth.rpcapi.ConsoleAuthAPI() + super(ComputeManager, self).__init__(service_name="compute", *args, **kwargs) @@ -1223,6 +1226,10 @@ def _delete_instance(self, context, instance, bdms): self._notify_about_instance_usage(context, instance, "delete.end", system_metadata=system_meta) + if CONF.vnc_enabled or CONF.spice.enabled: + self.consoleauth_rpcapi.delete_tokens_for_instance(context, + instance['uuid']) + @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) @wrap_instance_event @wrap_instance_fault @@ -2555,6 +2562,16 @@ def get_spice_console(self, context, console_type, instance): return connect_info + @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) + @wrap_instance_fault + def validate_console_port(self, ctxt, instance, port, console_type): + if console_type == "spice-html5": + console_info = self.driver.get_spice_console(instance) + else: + console_info = self.driver.get_vnc_console(instance) + + return console_info['port'] == port + def _attach_volume_boot(self, context, instance, volume, mountpoint): """Attach a volume to an instance at boot time. So actual attach is done by instance creation""" diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 0be9972da88..67dfc6a6b12 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -161,6 +161,8 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy): 2.23 - Remove network_info from reboot_instance 2.24 - Added get_spice_console method 2.25 - Add attach_interface() and detach_interface() + 2.26 - Add validate_console_token to ensure the service connects to + vnc on the correct port ''' # @@ -321,6 +323,14 @@ def get_spice_console(self, ctxt, instance, console_type): topic=_compute_topic(self.topic, ctxt, None, instance), version='2.24') + def validate_console_port(self, ctxt, instance, port, console_type): + instance_p = jsonutils.to_primitive(instance) + return self.call(ctxt, self.make_msg('validate_console_port', + instance=instance_p, port=port, console_type=console_type), + topic=_compute_topic(self.topic, ctxt, + None, instance), + version='2.26') + def host_maintenance_mode(self, ctxt, host_param, mode, host): '''Set host maintenance mode diff --git a/nova/consoleauth/manager.py b/nova/consoleauth/manager.py index 74321a27b12..56e94dffd6b 100644 --- a/nova/consoleauth/manager.py +++ b/nova/consoleauth/manager.py @@ -23,6 +23,8 @@ from oslo.config import cfg from nova.common import memorycache +from nova.compute import rpcapi as compute_rpcapi +from nova.conductor import api as conductor_api from nova import manager from nova.openstack.common import jsonutils from nova.openstack.common import log as logging @@ -46,15 +48,27 @@ class ConsoleAuthManager(manager.Manager): """Manages token based authentication.""" - RPC_API_VERSION = '1.1' + RPC_API_VERSION = '1.2' def __init__(self, scheduler_driver=None, *args, **kwargs): super(ConsoleAuthManager, self).__init__(*args, **kwargs) self.mc = memorycache.get_client() + self.compute_rpcapi = compute_rpcapi.ComputeAPI() + self.conductor_api = conductor_api.API() + + def _get_tokens_for_instance(self, instance_uuid): + tokens_str = self.mc.get(instance_uuid.encode('UTF-8')) + if not tokens_str: + tokens = [] + else: + tokens = jsonutils.loads(tokens_str) + return tokens def authorize_console(self, context, token, console_type, host, port, - internal_access_path): + internal_access_path, instance_uuid=None): + token_dict = {'token': token, + 'instance_uuid': instance_uuid, 'console_type': console_type, 'host': host, 'port': port, @@ -62,14 +76,39 @@ def authorize_console(self, context, token, console_type, host, port, 'last_activity_at': time.time()} data = jsonutils.dumps(token_dict) self.mc.set(token.encode('UTF-8'), data, CONF.console_token_ttl) + if instance_uuid is not None: + tokens = self._get_tokens_for_instance(instance_uuid) + tokens.append(token) + self.mc.set(instance_uuid.encode('UTF-8'), + jsonutils.dumps(tokens)) + LOG.audit(_("Received Token: %(token)s, %(token_dict)s)"), locals()) + def _validate_token(self, context, token): + instance_uuid = token['instance_uuid'] + if instance_uuid is None: + return False + instance = self.conductor_api.instance_get_by_uuid(context, + instance_uuid) + return self.compute_rpcapi.validate_console_port(context, + instance, + token['port'], + token['console_type']) + def check_token(self, context, token): token_str = self.mc.get(token.encode('UTF-8')) token_valid = (token_str is not None) LOG.audit(_("Checking Token: %(token)s, %(token_valid)s)"), locals()) if token_valid: - return jsonutils.loads(token_str) + token = jsonutils.loads(token_str) + if self._validate_token(context, token): + return token + + def delete_tokens_for_instance(self, context, instance_uuid): + tokens = self._get_tokens_for_instance(instance_uuid) + for token in tokens: + self.mc.delete(token) + self.mc.delete(instance_uuid.encode('UTF-8')) def get_backdoor_port(self, context): return self.backdoor_port diff --git a/nova/consoleauth/rpcapi.py b/nova/consoleauth/rpcapi.py index 813143f7635..474f3ad19a1 100644 --- a/nova/consoleauth/rpcapi.py +++ b/nova/consoleauth/rpcapi.py @@ -32,6 +32,8 @@ class ConsoleAuthAPI(nova.openstack.common.rpc.proxy.RpcProxy): 1.0 - Initial version. 1.1 - Added get_backdoor_port() + 1.2 - Added instance_uuid to authorize_console, and + delete_tokens_for_instance ''' # @@ -50,18 +52,26 @@ def __init__(self): default_version=self.BASE_RPC_API_VERSION) def authorize_console(self, ctxt, token, console_type, host, port, - internal_access_path): + internal_access_path, instance_uuid=None): # The remote side doesn't return anything, but we want to block # until it completes. return self.call(ctxt, self.make_msg('authorize_console', token=token, console_type=console_type, host=host, port=port, - internal_access_path=internal_access_path)) + internal_access_path=internal_access_path, + instance_uuid=instance_uuid), + version="1.2") def check_token(self, ctxt, token): return self.call(ctxt, self.make_msg('check_token', token=token)) + def delete_tokens_for_instance(self, ctxt, instance_uuid): + return self.call(ctxt, + self.make_msg('delete_tokens_for_instance', + instance_uuid=instance_uuid), + version="1.2") + def get_backdoor_port(self, ctxt, host): return self.call(ctxt, self.make_msg('get_backdoor_port'), version='1.1') diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index e19470db55e..02bbaaa627b 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -1420,6 +1420,54 @@ def test_novnc_vnc_console(self): self.compute.terminate_instance(self.context, instance=instance) + def test_validate_console_port_vnc(self): + self.flags(vnc_enabled=True) + self.flags(enabled=True, group='spice') + instance = jsonutils.to_primitive(self._create_fake_instance()) + + def fake_driver_get_console(*args, **kwargs): + return {'host': "fake_host", 'port': "5900", + 'internal_access_path': None} + self.stubs.Set(self.compute.driver, "get_vnc_console", + fake_driver_get_console) + + self.assertTrue(self.compute.validate_console_port(self.context, + instance, + "5900", + "novnc")) + + def test_validate_console_port_spice(self): + self.flags(vnc_enabled=True) + self.flags(enabled=True, group='spice') + instance = jsonutils.to_primitive(self._create_fake_instance()) + + def fake_driver_get_console(*args, **kwargs): + return {'host': "fake_host", 'port': "5900", + 'internal_access_path': None} + self.stubs.Set(self.compute.driver, "get_spice_console", + fake_driver_get_console) + + self.assertTrue(self.compute.validate_console_port(self.context, + instance, + "5900", + "spice-html5")) + + def test_validate_console_port_wrong_port(self): + self.flags(vnc_enabled=True) + self.flags(enabled=True, group='spice') + instance = jsonutils.to_primitive(self._create_fake_instance()) + + def fake_driver_get_console(*args, **kwargs): + return {'host': "fake_host", 'port': "5900", + 'internal_access_path': None} + self.stubs.Set(self.compute.driver, "get_vnc_console", + fake_driver_get_console) + + self.assertFalse(self.compute.validate_console_port(self.context, + instance, + "wrongport", + "spice-html5")) + def test_xvpvnc_vnc_console(self): # Make sure we can a vnc console for an instance. self.flags(vnc_enabled=True) @@ -1715,6 +1763,25 @@ def fake_cleanup_volumes(context, instance): instance=jsonutils.to_primitive(instance), bdms={}) + def test_delete_instance_deletes_console_auth_tokens(self): + instance = self._create_fake_instance() + self.flags(vnc_enabled=True) + + self.tokens_deleted = False + + def fake_delete_tokens(*args, **kwargs): + self.tokens_deleted = True + + cauth_rpcapi = self.compute.consoleauth_rpcapi + self.stubs.Set(cauth_rpcapi, 'delete_tokens_for_instance', + fake_delete_tokens) + + self.compute._delete_instance(self.context, + instance=jsonutils.to_primitive(instance), + bdms={}) + + self.assertTrue(self.tokens_deleted) + def test_instance_termination_exception_sets_error(self): """Test that we handle InstanceTerminationFailure which is propagated up from the underlying driver. @@ -5735,7 +5802,8 @@ def test_vnc_console(self): 'console_type': fake_console_type, 'host': 'fake_console_host', 'port': 'fake_console_port', - 'internal_access_path': 'fake_access_path'} + 'internal_access_path': 'fake_access_path', + 'instance_uuid': fake_instance['uuid']} fake_connect_info2 = copy.deepcopy(fake_connect_info) fake_connect_info2['access_url'] = 'fake_console_url' @@ -5747,7 +5815,7 @@ def test_vnc_console(self): 'version': compute_rpcapi.ComputeAPI.BASE_RPC_API_VERSION} rpc_msg2 = {'method': 'authorize_console', 'args': fake_connect_info, - 'version': '1.0'} + 'version': '1.2'} rpc.call(self.context, 'compute.%s' % fake_instance['host'], rpc_msg1, None).AndReturn(fake_connect_info2) @@ -5779,7 +5847,8 @@ def test_spice_console(self): 'console_type': fake_console_type, 'host': 'fake_console_host', 'port': 'fake_console_port', - 'internal_access_path': 'fake_access_path'} + 'internal_access_path': 'fake_access_path', + 'instance_uuid': fake_instance['uuid']} fake_connect_info2 = copy.deepcopy(fake_connect_info) fake_connect_info2['access_url'] = 'fake_console_url' @@ -5791,7 +5860,7 @@ def test_spice_console(self): 'version': '2.24'} rpc_msg2 = {'method': 'authorize_console', 'args': fake_connect_info, - 'version': '1.0'} + 'version': '1.2'} rpc.call(self.context, 'compute.%s' % fake_instance['host'], rpc_msg1, None).AndReturn(fake_connect_info2) diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py index a78a1388360..6c40a95e2dd 100644 --- a/nova/tests/compute/test_rpcapi.py +++ b/nova/tests/compute/test_rpcapi.py @@ -171,6 +171,12 @@ def test_get_spice_console(self): instance=self.fake_instance, console_type='type', version='2.24') + def test_validate_console_port(self): + self._test_compute_api('validate_console_port', 'call', + instance=self.fake_instance, port="5900", + console_type="novnc", + version="2.26") + def test_host_maintenance_mode(self): self._test_compute_api('host_maintenance_mode', 'call', host_param='param', mode='mode', host='host') diff --git a/nova/tests/consoleauth/test_consoleauth.py b/nova/tests/consoleauth/test_consoleauth.py index 15397a40010..54e3d226126 100644 --- a/nova/tests/consoleauth/test_consoleauth.py +++ b/nova/tests/consoleauth/test_consoleauth.py @@ -42,12 +42,74 @@ def test_tokens_expire(self): self.useFixture(test.TimeOverride()) token = 'mytok' self.flags(console_token_ttl=1) + + def fake_validate_console_port(*args, **kwargs): + return True + self.stubs.Set(self.manager.compute_rpcapi, + "validate_console_port", + fake_validate_console_port) + self.manager.authorize_console(self.context, token, 'novnc', - '127.0.0.1', 'host', '') + '127.0.0.1', '8080', 'host', + 'instance') self.assertTrue(self.manager.check_token(self.context, token)) timeutils.advance_time_seconds(1) self.assertFalse(self.manager.check_token(self.context, token)) + def test_multiple_tokens_for_instance(self): + tokens = ["token" + str(i) for i in xrange(10)] + instance = "12345" + + def fake_validate_console_port(*args, **kwargs): + return True + + self.stubs.Set(self.manager.compute_rpcapi, + "validate_console_port", + fake_validate_console_port) + for token in tokens: + self.manager.authorize_console(self.context, token, 'novnc', + '127.0.0.1', '8080', 'host', + instance) + + for token in tokens: + self.assertTrue(self.manager.check_token(self.context, token)) + + def test_delete_tokens_for_instance(self): + instance = "12345" + tokens = ["token" + str(i) for i in xrange(10)] + for token in tokens: + self.manager.authorize_console(self.context, token, 'novnc', + '127.0.0.1', '8080', 'host', + instance) + self.manager.delete_tokens_for_instance(self.context, instance) + stored_tokens = self.manager._get_tokens_for_instance(instance) + + self.assertEqual(len(stored_tokens), 0) + + for token in tokens: + self.assertFalse(self.manager.check_token(self.context, token)) + + def test_wrong_token_has_port(self): + token = 'mytok' + + def fake_validate_console_port(*args, **kwargs): + return False + + self.stubs.Set(self.manager.compute_rpcapi, + "validate_console_port", + fake_validate_console_port) + + self.manager.authorize_console(self.context, token, 'novnc', + '127.0.0.1', '8080', 'host', + instance_uuid='instance') + self.assertFalse(self.manager.check_token(self.context, token)) + + def test_console_no_instance_uuid(self): + self.manager.authorize_console(self.context, "token", 'novnc', + '127.0.0.1', '8080', 'host', + instance_uuid=None) + self.assertFalse(self.manager.check_token(self.context, "token")) + def test_get_backdoor_port(self): self.manager.backdoor_port = 59697 port = self.manager.get_backdoor_port(self.context) diff --git a/nova/tests/consoleauth/test_rpcapi.py b/nova/tests/consoleauth/test_rpcapi.py index 15af5fdcf80..53ca2e5d6d5 100644 --- a/nova/tests/consoleauth/test_rpcapi.py +++ b/nova/tests/consoleauth/test_rpcapi.py @@ -65,11 +65,17 @@ def _fake_call(_ctxt, _topic, _msg, _timeout): def test_authorize_console(self): self._test_consoleauth_api('authorize_console', token='token', console_type='ctype', host='h', port='p', - internal_access_path='iap') + internal_access_path='iap', instance_uuid="instance", + version="1.2") def test_check_token(self): self._test_consoleauth_api('check_token', token='t') + def test_delete_tokens_for_instnace(self): + self._test_consoleauth_api('delete_tokens_for_instance', + instance_uuid="instance", + version='1.2') + def test_get_backdoor_port(self): self._test_consoleauth_api('get_backdoor_port', host='fake_host', version='1.1')