Merge "Mask the token used to allow access to consoles" into stable/rocky

nova/console/websocketproxy.py

@@ -18,6 +18,7 @@ Websocket proxy that is compatible with OpenStack Nova.
 Leverages websockify.py by Joel Martin
 '''
 
+import copy
 import socket
 import sys
 
@@ -248,7 +249,10 @@ class NovaProxyRequestHandlerBase(object):
                 detail = _("Origin header protocol does not match this host.")
                 raise exception.ValidationError(detail=detail)
 
-        self.msg(_('connect info: %s'), str(connect_info))
+        sanitized_info = copy.copy(connect_info)
+        sanitized_info['token'] = '***'
+        self.msg(_('connect info: %s'), sanitized_info)
+
         host = connect_info['host']
         port = int(connect_info['port'])
 

nova/consoleauth/manager.py

@@ -100,9 +100,8 @@ class ConsoleAuthManager(manager.Manager):
 
         self.mc_instance.set(instance_uuid.encode('UTF-8'),
                              jsonutils.dumps(tokens))
-
+        token_dict['token'] = '***'
-        LOG.info("Received Token: %(token)s, %(token_dict)s",
+        LOG.info("Received Token: %(token_dict)s", {'token_dict': token_dict})
-                 {'token': token, 'token_dict': token_dict})
 
     def _validate_token(self, context, token):
         instance_uuid = token['instance_uuid']
@@ -130,8 +129,8 @@ class ConsoleAuthManager(manager.Manager):
     def check_token(self, context, token):
         token_str = self.mc.get(token.encode('UTF-8'))
         token_valid = (token_str is not None)
-        LOG.info("Checking Token: %(token)s, %(token_valid)s",
+        LOG.info("Checking that token is known: %(token_valid)s",
-                 {'token': token, 'token_valid': token_valid})
+                 {'token_valid': token_valid})
         if token_valid:
             token = jsonutils.loads(token_str)
             if self._validate_token(context, token):

nova/tests/unit/console/test_websocketproxy.py

@@ -295,6 +295,9 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
         validate.assert_called_with(mock.ANY, "123-456-789")
         self.wh.socket.assert_called_with('node1', 10000, connect=True)
         self.wh.do_proxy.assert_called_with('<socket>')
+        # ensure that token is masked when logged
+        connection_info = self.wh.msg.mock_calls[0][1][1]
+        self.assertEqual('***', connection_info['token'])
 
     @mock.patch('nova.console.websocketproxy.NovaProxyRequestHandlerBase.'
                 '_check_console_port')

nova/tests/unit/consoleauth/test_consoleauth.py

@@ -88,6 +88,17 @@ class ConsoleauthTestCase(test.NoDBTestCase):
         self.stub_out(self.rpcapi + 'validate_console_port',
                       fake_validate_console_port)
 
+    @mock.patch('nova.consoleauth.manager.LOG.info')
+    def test_authorize_does_not_log_token_secrete(self, mock_info):
+        self.manager_api.authorize_console(
+            self.context, 'secret', 'novnc', '127.0.0.1', '8080', 'host',
+            self.instance_uuid)
+
+        mock_info.assert_called_once_with(
+            'Received Token: %(token_dict)s', test.MatchType(dict))
+        self.assertEqual(
+            '***', mock_info.mock_calls[0][1][1]['token_dict']['token'])
+
     @mock.patch('nova.objects.instance.Instance.get_by_uuid')
     def test_multiple_tokens_for_instance(self, mock_get):
         mock_get.return_value = None
@@ -139,8 +150,9 @@ class ConsoleauthTestCase(test.NoDBTestCase):
             mock_delete.assert_called_once_with(
                 self.instance_uuid.encode('UTF-8'))
 
+    @mock.patch('nova.consoleauth.manager.LOG.info')
     @mock.patch('nova.objects.instance.Instance.get_by_uuid')
-    def test_wrong_token_has_port(self, mock_get):
+    def test_wrong_token_has_port(self, mock_get, mock_log):
         mock_get.return_value = None
 
         token = u'mytok'
@@ -151,6 +163,13 @@ class ConsoleauthTestCase(test.NoDBTestCase):
                                         '127.0.0.1', '8080', 'host',
                                         instance_uuid=self.instance_uuid)
         self.assertIsNone(self.manager_api.check_token(self.context, token))
+        mock_log.assert_has_calls([
+            mock.call(
+                'Received Token: %(token_dict)s', mock.ANY),
+            mock.call(
+                'Checking that token is known: %(token_valid)s',
+                {'token_valid': True}),
+        ])
 
     def test_delete_expired_tokens(self):
         self.useFixture(test.TimeOverride())