From 6e5055e7868414ce1aeab48756177350c4dd6ba0 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 20 Oct 2015 18:51:34 -0700 Subject: [PATCH] Update the use of no_log values to cover everything that heuristic_log_sanitize does. Fixes #12792 --- lib/ansible/module_utils/basic.py | 165 +++++++++--------- .../module_utils/basic/test_exit_json.py | 140 +++++++++++++++ .../basic/test_heuristic_log_sanitize.py | 18 +- test/units/module_utils/basic/test_log.py | 2 +- test/units/module_utils/basic/test_no_log.py | 53 +----- 5 files changed, 229 insertions(+), 149 deletions(-) create mode 100644 test/units/module_utils/basic/test_exit_json.py diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 82de3d5317..52e2f090ec 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -364,7 +364,53 @@ def json_dict_bytes_to_unicode(d, encoding='utf-8'): else: return d -def heuristic_log_sanitize(data): +def return_values(obj): + """ Return stringified values from datastructures. For use with removing + sensitive values pre-jsonification.""" + if isinstance(obj, basestring): + if obj: + yield obj + return + elif isinstance(obj, Sequence): + for element in obj: + for subelement in return_values(element): + yield subelement + elif isinstance(obj, Mapping): + for element in obj.items(): + for subelement in return_values(element[1]): + yield subelement + elif isinstance(obj, (bool, NoneType)): + # This must come before int because bools are also ints + return + elif isinstance(obj, NUMBERTYPES): + yield str(obj) + else: + raise TypeError('Unknown parameter type: %s, %s' % (type(obj), obj)) + +def remove_values(value, no_log_strings): + """ Remove strings in no_log_strings from value. If value is a container + type, then remove a lot more""" + if isinstance(value, basestring): + if value in no_log_strings: + return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' + for omit_me in no_log_strings: + value = value.replace(omit_me, '*' * 8) + elif isinstance(value, Sequence): + return [remove_values(elem, no_log_strings) for elem in value] + elif isinstance(value, Mapping): + return dict((k, remove_values(v, no_log_strings)) for k, v in value.items()) + elif isinstance(value, tuple(chain(NUMBERTYPES, (bool, NoneType)))): + stringy_value = str(value) + if stringy_value in no_log_strings: + return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' + for omit_me in no_log_strings: + if omit_me in stringy_value: + return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' + else: + raise TypeError('Value of unknown type: %s, %s' % (type(value), value)) + return value + +def heuristic_log_sanitize(data, no_log_values=None): ''' Remove strings that look like passwords from log messages ''' # Currently filters: # user:pass@foo/whatever and http://username:pass@wherever/foo @@ -421,53 +467,10 @@ def heuristic_log_sanitize(data): output.insert(0, data[begin:sep + 1]) prev_begin = begin - return ''.join(output) - -def _return_values(obj): - """ Return stringified values from datastructures. For use with removing - sensitive values pre-jsonification.""" - if isinstance(obj, basestring): - if obj: - yield obj - return - elif isinstance(obj, Sequence): - for element in obj: - for subelement in _return_values(element): - yield subelement - elif isinstance(obj, Mapping): - for element in obj.items(): - for subelement in _return_values(element[1]): - yield subelement - elif isinstance(obj, (bool, NoneType)): - # This must come before int because bools are also ints - return - elif isinstance(obj, NUMBERTYPES): - yield str(obj) - else: - raise TypeError('Unknown parameter type: %s, %s' % (type(obj), obj)) - -def _remove_values(value, no_log_strings): - """ Remove strings in no_log_strings from value. If value is a container - type, then remove a lot more""" - if isinstance(value, basestring): - if value in no_log_strings: - return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' - for omit_me in no_log_strings: - value = value.replace(omit_me, '*' * 8) - elif isinstance(value, Sequence): - return [_remove_values(elem, no_log_strings) for elem in value] - elif isinstance(value, Mapping): - return dict((k, _remove_values(v, no_log_strings)) for k, v in value.items()) - elif isinstance(value, tuple(chain(NUMBERTYPES, (bool, NoneType)))): - stringy_value = str(value) - if stringy_value in no_log_strings: - return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' - for omit_me in no_log_strings: - if omit_me in stringy_value: - return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' - else: - raise TypeError('Value of unknown type: %s, %s' % (type(value), value)) - return value + output = ''.join(output) + if no_log_values: + output = remove_values(output, no_log_values) + return output def is_executable(path): '''is the given path executable?''' @@ -502,12 +505,22 @@ class AnsibleModule(object): if k not in self.argument_spec: self.argument_spec[k] = v + self.params = self._load_params() + + # Save parameter values that should never be logged + self.no_log_values = set() + # Use the argspec to determine which args are no_log + for arg_name, arg_opts in self.argument_spec.items(): + if arg_opts.get('no_log', False): + # Find the value for the no_log'd param + no_log_object = self.params.get(arg_name, None) + if no_log_object: + self.no_log_values.update(return_values(no_log_object)) + # check the locale as set by the current environment, and # reset to LANG=C if it's an invalid/unavailable locale self._check_locale() - self.params = self._load_params() - self._legal_inputs = ['_ansible_check_mode', '_ansible_no_log', '_ansible_debug'] # append to legal_inputs and then possibly check against them @@ -540,6 +553,7 @@ class AnsibleModule(object): self._check_required_if(required_if) self._set_defaults(pre=False) + if not self.no_log: self._log_invocation() @@ -1337,20 +1351,16 @@ class AnsibleModule(object): # We want journal to always take text type # syslog takes bytes on py2, text type on py3 - if sys.version_info >= (3,): - if isinstance(msg, bytes): - journal_msg = msg.decode('utf-8', 'replace') - syslog_msg = journal_msg - else: - # TODO: surrogateescape is a danger here - journal_msg = syslog_msg = msg + if isinstance(msg, bytes): + journal_msg = remove_values(msg.decode('utf-8', 'replace'), self.no_log_values) else: - if isinstance(msg, bytes): - journal_msg = msg.decode('utf-8', 'replace') - syslog_msg = msg - else: - journal_msg = msg - syslog_msg = msg.encode('utf-8', 'replace') + # TODO: surrogateescape is a danger here on Py3 + journal_msg = remove_values(msg, self.no_log_values) + + if sys.version_info >= (3,): + syslog_msg = journal_msg + else: + syslog_msg = journal_msg.encode('utf-8', 'replace') if has_journal: journal_args = [("MODULE", os.path.basename(__file__))] @@ -1386,7 +1396,7 @@ class AnsibleModule(object): param_val = str(param_val) elif isinstance(param_val, unicode): param_val = param_val.encode('utf-8') - log_args[param] = heuristic_log_sanitize(param_val) + log_args[param] = heuristic_log_sanitize(param_val, self.no_log_values) msg = [] for arg in log_args: @@ -1492,30 +1502,12 @@ class AnsibleModule(object): for path in self.cleanup_files: self.cleanup(path) - def remove_no_log_values(self, to_jsonify): - """ Strip values associated with no_log parameters from output. - Note: does not strip dict keys, only dict values. - """ - no_log_strings = set() - # Use the argspec to determine which args are no_log - for arg_name, arg_opts in self.argument_spec.items(): - if arg_opts.get('no_log', False): - # Find the value for the no_log'd param - no_log_object = self.params.get(arg_name, None) - if no_log_object: - no_log_strings.update(_return_values(no_log_object)) - - for field, value in to_jsonify.items(): - to_jsonify[field] = _remove_values(value, no_log_strings) - - return to_jsonify - def exit_json(self, **kwargs): ''' return from the module, without error ''' self.add_path_info(kwargs) if not 'changed' in kwargs: kwargs['changed'] = False - self.remove_no_log_values(kwargs) + kwargs = remove_values(kwargs, self.no_log_values) self.do_cleanup_files() print(self.jsonify(kwargs)) sys.exit(0) @@ -1525,7 +1517,7 @@ class AnsibleModule(object): self.add_path_info(kwargs) assert 'msg' in kwargs, "implementation error -- msg to explain the error is required" kwargs['failed'] = True - self.remove_no_log_values(kwargs) + kwargs = remove_values(kwargs, self.no_log_values) self.do_cleanup_files() print(self.jsonify(kwargs)) sys.exit(1) @@ -1776,7 +1768,8 @@ class AnsibleModule(object): continue else: is_passwd = True - clean_args.append(heuristic_log_sanitize(arg)) + arg = heuristic_log_sanitize(arg, self.no_log_values) + clean_args.append(arg) clean_args = ' '.join(pipes.quote(arg) for arg in clean_args) if data: @@ -1872,7 +1865,7 @@ class AnsibleModule(object): self.fail_json(rc=257, msg=traceback.format_exc(), cmd=clean_args) if rc != 0 and check_rc: - msg = heuristic_log_sanitize(stderr.rstrip()) + msg = heuristic_log_sanitize(stderr.rstrip(), self.no_log_values) self.fail_json(cmd=clean_args, rc=rc, stdout=stdout, stderr=stderr, msg=msg) # reset the pwd diff --git a/test/units/module_utils/basic/test_exit_json.py b/test/units/module_utils/basic/test_exit_json.py new file mode 100644 index 0000000000..498a53e53d --- /dev/null +++ b/test/units/module_utils/basic/test_exit_json.py @@ -0,0 +1,140 @@ +# -*- coding: utf-8 -*- +# (c) 2015, Toshio Kuratomi +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +# Make coding more python3-ish +from __future__ import (absolute_import, division) +__metaclass__ = type + +import copy +import json +import sys + +from ansible.compat.tests import unittest +from ansible.compat.six import StringIO + +from ansible.module_utils import basic +from ansible.module_utils.basic import heuristic_log_sanitize +from ansible.module_utils.basic import return_values, remove_values + +@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)") +class TestAnsibleModuleExitJson(unittest.TestCase): + def setUp(self): + self.COMPLEX_ARGS = basic.MODULE_COMPLEX_ARGS + basic.MODULE_COMPLEX_ARGS = '{}' + + self.old_stdout = sys.stdout + self.fake_stream = StringIO() + sys.stdout = self.fake_stream + + self.module = basic.AnsibleModule(argument_spec=dict()) + + def tearDown(self): + basic.MODULE_COMPLEX_ARGS = self.COMPLEX_ARGS + sys.stdout = self.old_stdout + + def test_exit_json_no_args_exits(self): + with self.assertRaises(SystemExit) as ctx: + self.module.exit_json() + self.assertEquals(ctx.exception.code, 0) + return_val = json.loads(self.fake_stream.getvalue()) + self.assertEquals(return_val, dict(changed=False)) + + def test_exit_json_args_exits(self): + with self.assertRaises(SystemExit) as ctx: + self.module.exit_json(msg='message') + self.assertEquals(ctx.exception.code, 0) + return_val = json.loads(self.fake_stream.getvalue()) + self.assertEquals(return_val, dict(msg="message", changed=False)) + + def test_fail_json_exits(self): + with self.assertRaises(SystemExit) as ctx: + self.module.fail_json(msg='message') + self.assertEquals(ctx.exception.code, 1) + return_val = json.loads(self.fake_stream.getvalue()) + self.assertEquals(return_val, dict(msg="message", failed=True)) + + def test_exit_json_proper_changed(self): + with self.assertRaises(SystemExit) as ctx: + self.module.exit_json(changed=True, msg='success') + return_val = json.loads(self.fake_stream.getvalue()) + self.assertEquals(return_val, dict(changed=True, msg='success')) + +@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)") +class TestAnsibleModuleExitValuesRemoved(unittest.TestCase): + OMIT = 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' + dataset = ( + (dict(username='person', password='$ecret k3y'), + dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/', + not_secret='following the leader', msg='here'), + dict(one=1, pwd=OMIT, url='https://username:password12345@foo.com/login/', + not_secret='following the leader', changed=False, msg='here') + ), + (dict(username='person', password='password12345'), + dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/', + not_secret='following the leader', msg='here'), + dict(one=1, pwd='$ecret k3y', url='https://username:********@foo.com/login/', + not_secret='following the leader', changed=False, msg='here') + ), + (dict(username='person', password='$ecret k3y'), + dict(one=1, pwd='$ecret k3y', url='https://username:$ecret k3y@foo.com/login/', + not_secret='following the leader', msg='here'), + dict(one=1, pwd=OMIT, url='https://username:********@foo.com/login/', + not_secret='following the leader', changed=False, msg='here') + ), + ) + + def setUp(self): + self.COMPLEX_ARGS = basic.MODULE_COMPLEX_ARGS + self.old_stdout = sys.stdout + + def tearDown(self): + basic.MODULE_COMPLEX_ARGS = self.COMPLEX_ARGS + sys.stdout = self.old_stdout + + def test_exit_json_removes_values(self): + for args, return_val, expected in self.dataset: + sys.stdout = StringIO() + basic.MODULE_COMPLEX_ARGS = json.dumps(args) + module = basic.AnsibleModule( + argument_spec = dict( + username=dict(), + password=dict(no_log=True), + token=dict(no_log=True), + ), + ) + with self.assertRaises(SystemExit) as ctx: + self.assertEquals(module.exit_json(**return_val), expected) + self.assertEquals(json.loads(sys.stdout.getvalue()), expected) + + def test_fail_json_removes_values(self): + for args, return_val, expected in self.dataset: + expected = copy.deepcopy(expected) + del expected['changed'] + expected['failed'] = True + sys.stdout = StringIO() + basic.MODULE_COMPLEX_ARGS = json.dumps(args) + module = basic.AnsibleModule( + argument_spec = dict( + username=dict(), + password=dict(no_log=True), + token=dict(no_log=True), + ), + ) + with self.assertRaises(SystemExit) as ctx: + self.assertEquals(module.fail_json(**return_val), expected) + self.assertEquals(json.loads(sys.stdout.getvalue()), expected) diff --git a/test/units/module_utils/basic/test_heuristic_log_sanitize.py b/test/units/module_utils/basic/test_heuristic_log_sanitize.py index 2540e34ef6..51a5c11adf 100644 --- a/test/units/module_utils/basic/test_heuristic_log_sanitize.py +++ b/test/units/module_utils/basic/test_heuristic_log_sanitize.py @@ -85,18 +85,6 @@ class TestHeuristicLogSanitize(unittest.TestCase): self.assertTrue(ssh_output.endswith("}")) self.assertIn(":********@foo.com/data'", ssh_output) - -class TestStripNoLog(unittest.TestCase): - def setUp(self): - data = '' - - def test_return_strings(self): - pass - - def test_strip_no_log(self): - pass - -class TestAnsibleModuleStripNoLogValues(unittest.TestCase): - pass - - + def test_hides_parameter_secrets(self): + output = heuristic_log_sanitize('token="secret", user="person", token_entry="test=secret"', frozenset(['secret'])) + self.assertNotIn('secret', output) diff --git a/test/units/module_utils/basic/test_log.py b/test/units/module_utils/basic/test_log.py index 6998106c73..27e56db1cb 100644 --- a/test/units/module_utils/basic/test_log.py +++ b/test/units/module_utils/basic/test_log.py @@ -103,7 +103,7 @@ class TestAnsibleModuleLogSyslog(unittest.TestCase): u'Toshio くらとみ non-ascii test': u'Toshio くらとみ non-ascii test'.encode('utf-8'), b'Byte string': b'Byte string', u'Toshio くらとみ non-ascii test'.encode('utf-8'): u'Toshio くらとみ non-ascii test'.encode('utf-8'), - b'non-utf8 :\xff: test': b'non-utf8 :\xff: test' + b'non-utf8 :\xff: test': b'non-utf8 :\xff: test'.decode('utf-8', 'replace').encode('utf-8'), } py3_output_data = { diff --git a/test/units/module_utils/basic/test_no_log.py b/test/units/module_utils/basic/test_no_log.py index b4847fcde7..24d38ddcfa 100644 --- a/test/units/module_utils/basic/test_no_log.py +++ b/test/units/module_utils/basic/test_no_log.py @@ -29,7 +29,7 @@ from ansible.compat.tests.mock import patch, MagicMock from ansible.module_utils import basic from ansible.module_utils.basic import heuristic_log_sanitize -from ansible.module_utils.basic import _return_values, _remove_values +from ansible.module_utils.basic import return_values, remove_values class TestReturnValues(unittest.TestCase): @@ -50,10 +50,10 @@ class TestReturnValues(unittest.TestCase): def test_return_values(self): for data, expected in self.dataset: - self.assertEquals(frozenset(_return_values(data)), expected) + self.assertEquals(frozenset(return_values(data)), expected) def test_unknown_type(self): - self.assertRaises(TypeError, frozenset, _return_values(object())) + self.assertRaises(TypeError, frozenset, return_values(object())) class TestRemoveValues(unittest.TestCase): @@ -98,54 +98,13 @@ class TestRemoveValues(unittest.TestCase): def test_no_removal(self): for value, no_log_strings in self.dataset_no_remove: - self.assertEquals(_remove_values(value, no_log_strings), value) + self.assertEquals(remove_values(value, no_log_strings), value) def test_strings_to_remove(self): for value, no_log_strings, expected in self.dataset_remove: - self.assertEquals(_remove_values(value, no_log_strings), expected) + self.assertEquals(remove_values(value, no_log_strings), expected) def test_unknown_type(self): - self.assertRaises(TypeError, _remove_values, object(), frozenset()) + self.assertRaises(TypeError, remove_values, object(), frozenset()) -@unittest.skipIf(sys.version_info[0] >= 3, "Python 3 is not supported on targets (yet)") -class TestAnsibleModuleRemoveNoLogValues(unittest.TestCase): - OMIT = 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' - dataset = ( - (dict(username='person', password='$ecret k3y'), - dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/', - not_secret='following the leader'), - dict(one=1, pwd=OMIT, url='https://username:password12345@foo.com/login/', - not_secret='following the leader') - ), - (dict(username='person', password='password12345'), - dict(one=1, pwd='$ecret k3y', url='https://username:password12345@foo.com/login/', - not_secret='following the leader'), - dict(one=1, pwd='$ecret k3y', url='https://username:********@foo.com/login/', - not_secret='following the leader') - ), - (dict(username='person', password='$ecret k3y'), - dict(one=1, pwd='$ecret k3y', url='https://username:$ecret k3y@foo.com/login/', - not_secret='following the leader'), - dict(one=1, pwd=OMIT, url='https://username:********@foo.com/login/', - not_secret='following the leader') - ), - ) - - def setUp(self): - self.COMPLEX_ARGS = basic.MODULE_COMPLEX_ARGS - - def tearDown(self): - basic.MODULE_COMPLEX_ARGS = self.COMPLEX_ARGS - - def test_remove_no_log_values(self): - for args, return_val, expected in self.dataset: - basic.MODULE_COMPLEX_ARGS = json.dumps(args) - module = basic.AnsibleModule( - argument_spec = dict( - username=dict(), - password=dict(no_log=True), - token=dict(no_log=True), - ), - ) - self.assertEquals(module.remove_no_log_values(return_val), expected)