Skip to content

Commit

Permalink
Mask the token used to allow access to consoles
Browse files Browse the repository at this point in the history
Hide the novncproxy token from the logs.

When backported this patch needs to be extended to handle the same issue
in the consoleauth service.

Co-Authored-By:paul-carlton2 <paul.carlton2@hp.com>
Co-Authored-By:Tristan Cacqueray <tdecacqu@redhat.com>

Change-Id: I5b8fa4233d297722c3af08176901d12887bae3de
Closes-Bug: #1492140
(cherry picked from commit 26d4047)
(cherry picked from commit d7826bc)
(cherry picked from commit d8fbf04)
  • Loading branch information
Balazs Gibizer committed Jan 26, 2020
1 parent 8606719 commit 08f1f91
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 7 deletions.
6 changes: 5 additions & 1 deletion nova/console/websocketproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
Leverages websockify.py by Joel Martin
'''

import copy
import socket
import sys

Expand Down Expand Up @@ -248,7 +249,10 @@ def new_websocket_client(self):
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'])

Expand Down
9 changes: 4 additions & 5 deletions nova/consoleauth/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,8 @@ def authorize_console(self, context, token, console_type, host, port,

self.mc_instance.set(instance_uuid.encode('UTF-8'),
jsonutils.dumps(tokens))

LOG.info("Received Token: %(token)s, %(token_dict)s",
{'token': token, 'token_dict': token_dict})
token_dict['token'] = '***'
LOG.info("Received Token: %(token_dict)s", {'token_dict': token_dict})

def _validate_token(self, context, token):
instance_uuid = token['instance_uuid']
Expand Down Expand Up @@ -130,8 +129,8 @@ def _validate_token(self, context, token):
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",
{'token': token, 'token_valid': token_valid})
LOG.info("Checking that token is known: %(token_valid)s",
{'token_valid': token_valid})
if token_valid:
token = jsonutils.loads(token_str)
if self._validate_token(context, token):
Expand Down
3 changes: 3 additions & 0 deletions nova/tests/unit/console/test_websocketproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ def test_new_websocket_client(self, validate, check_port):
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')
Expand Down
21 changes: 20 additions & 1 deletion nova/tests/unit/consoleauth/test_consoleauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ def fake_validate_console_port(self, ctxt, instance,
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
Expand Down Expand Up @@ -139,8 +150,9 @@ def test_delete_tokens_for_instance_no_tokens(self):
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'
Expand All @@ -151,6 +163,13 @@ def test_wrong_token_has_port(self, mock_get):
'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())
Expand Down

0 comments on commit 08f1f91

Please sign in to comment.