From 05a3374992bc8ba53ddc9c491b51c4b59eed0a72 Mon Sep 17 00:00:00 2001 From: John Herndon Date: Fri, 22 Feb 2013 20:43:58 +0000 Subject: [PATCH] VNC Token Validation Force console auth service to flush all tokens associated with an instance when it is deleted. This will fix a bug where the console for the wrong instance can be connected to via the console if the correct circumstances occur. This change also makes a call to veriry vnc console tokens when a user attempts to connect to a console. This ensures the user is connecting to the correct console. bug 1125378 Change-Id: I0d83ec6c4dbfef1af912a200ee15f8052f72da96 --- nova/common/memorycache.py | 5 ++ nova/compute/api.py | 3 +- nova/compute/manager.py | 12 ++++ nova/compute/rpcapi.py | 7 +++ nova/consoleauth/manager.py | 41 ++++++++++++- nova/consoleauth/rpcapi.py | 10 +++- nova/tests/compute/test_compute.py | 52 ++++++++++++++++- nova/tests/compute/test_rpcapi.py | 5 ++ nova/tests/consoleauth/test_consoleauth.py | 67 +++++++++++++++++++++- nova/tests/consoleauth/test_rpcapi.py | 6 +- 10 files changed, 200 insertions(+), 8 deletions(-) diff --git a/nova/common/memorycache.py b/nova/common/memorycache.py index 502f8338114..3a1935ece2b 100644 --- a/nova/common/memorycache.py +++ b/nova/common/memorycache.py @@ -62,3 +62,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 df7b21555c8..38602263c05 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1853,7 +1853,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"]) return {'url': connect_info['access_url']} diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 90de5a473ec..5b0d1eae173 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -52,6 +52,7 @@ from nova.compute import task_states from nova.compute import utils as compute_utils from nova.compute import vm_states +from nova import consoleauth import nova.context from nova import exception from nova import flags @@ -235,6 +236,7 @@ def __init__(self, compute_driver=None, *args, **kwargs): self.compute_api = compute.API() self.compute_rpcapi = compute_rpcapi.ComputeAPI() self.scheduler_rpcapi = scheduler_rpcapi.SchedulerAPI() + self.consoleauth_rpcapi = consoleauth.rpcapi.ConsoleAuthAPI() super(ComputeManager, self).__init__(service_name="compute", *args, **kwargs) @@ -926,6 +928,10 @@ def _delete_instance(self, context, instance): self._notify_about_instance_usage(context, instance, "delete.end", system_metadata=system_meta) + if FLAGS.vnc_enabled: + self.consoleauth_rpcapi.delete_tokens_for_instance(context, + instance["uuid"]) + @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) @wrap_instance_fault def terminate_instance(self, context, instance): @@ -1988,6 +1994,12 @@ def _attach_volume_boot(self, context, instance, volume, mountpoint): self.volume_api.attach(context, volume, instance_uuid, mountpoint) return connection_info + @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) + @wrap_instance_fault + def validate_console_port(self, ctxt, instance, port, console_type): + console_info = self.driver.get_vnc_console(instance) + return console_info['port'] == port + @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) @reverts_task_state @wrap_instance_fault diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 2e3873c1918..afec290dffd 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -259,6 +259,13 @@ def get_vnc_console(self, ctxt, instance, console_type): instance=instance_p, console_type=console_type), topic=_compute_topic(self.topic, ctxt, None, instance)) + 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)) + 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 61efdd0ae72..e715f989221 100644 --- a/nova/consoleauth/manager.py +++ b/nova/consoleauth/manager.py @@ -20,6 +20,8 @@ import time +from nova.compute import rpcapi as compute_rpcapi +from nova.db import api as db from nova import flags from nova import manager from nova.openstack.common import cfg @@ -56,10 +58,21 @@ def __init__(self, scheduler_driver=None, *args, **kwargs): from nova.common import memorycache as memcache self.mc = memcache.Client(FLAGS.memcached_servers, debug=0) + self.compute_rpcapi = compute_rpcapi.ComputeAPI() + + 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, @@ -67,11 +80,35 @@ 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, FLAGS.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 = db.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')) diff --git a/nova/consoleauth/rpcapi.py b/nova/consoleauth/rpcapi.py index 2fafe3fd007..b3b34c1a7dd 100644 --- a/nova/consoleauth/rpcapi.py +++ b/nova/consoleauth/rpcapi.py @@ -49,14 +49,20 @@ 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)) 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)) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 10b89fd955f..b745798b409 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -1372,6 +1372,24 @@ def fake_cleanup_volumes(context, instance): self.compute._delete_instance(self.context, instance=jsonutils.to_primitive(instance)) + 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)) + + 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. @@ -4505,7 +4523,9 @@ 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' @@ -4539,6 +4559,36 @@ def test_get_vnc_console_no_host(self): db.instance_destroy(self.context, instance['uuid']) + def test_validate_console_port(self): + self.flags(vnc_enabled=True) + 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_wrong_port(self): + self.flags(vnc_enabled=True) + 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", + "novnc")) + def test_console_output(self): fake_instance = {'uuid': 'fake_uuid', 'host': 'fake_compute_host'} diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py index 559a56a0a26..c42047bcea9 100644 --- a/nova/tests/compute/test_rpcapi.py +++ b/nova/tests/compute/test_rpcapi.py @@ -168,6 +168,11 @@ def test_get_vnc_console(self): self._test_compute_api('get_vnc_console', 'call', instance=self.fake_instance, console_type='type') + def test_validate_console_port(self): + self._test_compute_api('validate_console_port', 'call', + instance=self.fake_instance, port="5900", + console_type="novnc") + 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 da50eb83b5c..4bea85ae19f 100644 --- a/nova/tests/consoleauth/test_consoleauth.py +++ b/nova/tests/consoleauth/test_consoleauth.py @@ -45,8 +45,73 @@ def test_tokens_expire(self): """Test that tokens expire correctly.""" token = 'mytok' self.flags(console_token_ttl=1) + + def fake_validate_token(*args, **kwargs): + return True + self.stubs.Set(self.manager, + "_validate_token", + fake_validate_token) + self.manager.authorize_console(self.context, token, 'novnc', - '127.0.0.1', 'host', '') + '127.0.0.1', '8080', 'host', "1234") self.assertTrue(self.manager.check_token(self.context, token)) time.sleep(1.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_token(*args, **kwargs): + return True + + self.stubs.Set(self.manager, "_validate_token", + fake_validate_token) + 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)] + + def fake_validate_token(*args, **kwargs): + return True + self.stubs.Set(self.manager, "_validate_token", + fake_validate_token) + + 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_token(*args, **kwargs): + return False + + self.stubs.Set(self.manager, "_validate_token", + fake_validate_token) + + 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")) diff --git a/nova/tests/consoleauth/test_rpcapi.py b/nova/tests/consoleauth/test_rpcapi.py index 10484c7d9da..c1e7a4681ea 100644 --- a/nova/tests/consoleauth/test_rpcapi.py +++ b/nova/tests/consoleauth/test_rpcapi.py @@ -68,7 +68,11 @@ 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="1234") 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")