cmueller/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Properly mask no_log values is sub parameters during failure (#63405)

Browse source 
* Get no_log parameters from subspec

* Add changelog and unit tests

* Handle list of dicts in suboptions

Add fancy error message (this will probably haunt me)

* Update unit tests to test for list of dicts in suboptions

* Add integration tests

* Validate parameters in dict and list

In case it comes in as a string

* Make changes based on feedback, fix tests

* Simplify validators since we only need to validate dicts

Add test for suboptions passed in as strings to ensure they get validated properly and turned into a dictionary.

ci_complete

* Add a few more integration tests
This commit is contained in:
Sam Doran 2019-10-14 12:20:07 -04:00 committed by Toshio Kuratomi
parent 876a2d57be
commit e9d29b1fe4
8 changed files with 359 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
changelogs/fragments/no-log-sub-options-invalid-parameter.yaml Normal file
View file

@ -0,0 +1,2 @@
bugfixes:
- '**security issue** - properly hide parameters marked with ``no_log`` in suboptions when invalid parameters are passed to the module (CVE-2019-14858)'

5
lib/ansible/module_utils/basic.py
View file

@ -1447,7 +1447,11 @@ class AnsibleModule(object):
if param is None:
param = self.params
try:
self.no_log_values.update(list_no_log_values(spec, param))
except TypeError as te:
self.fail_json(msg="Failure when processing no_log parameters. Module invocation will be hidden. "
"%s" % to_native(te), invocation={'module_args': 'HIDDEN DUE TO FAILURE'})
self._deprecations.extend(list_deprecations(spec, param))
def _check_arguments(self, check_invalid_arguments, spec=None, param=None, legal_inputs=None):
@ -1722,7 +1726,6 @@ class AnsibleModule(object):
self._set_fallbacks(spec, param)
options_aliases = self._handle_aliases(spec, param, option_prefix=new_prefix)
self._handle_no_log_values(spec, param)
options_legal_inputs = list(spec.keys()) + list(options_aliases.keys())
self._check_arguments(self.check_invalid_arguments, spec, param, options_legal_inputs)

26
lib/ansible/module_utils/common/parameters.py
View file

@ -8,10 +8,12 @@ __metaclass__ = type
from ansible.module_utils._text import to_native
from ansible.module_utils.common._collections_compat import Mapping
from ansible.module_utils.common.collections import is_iterable
from ansible.module_utils.common.validation import check_type_dict
from ansible.module_utils.six import (
binary_type,
integer_types,
string_types,
text_type,
)
@ -86,6 +88,30 @@ def list_no_log_values(argument_spec, params):
if no_log_object:
no_log_values.update(_return_datastructure_name(no_log_object))
# Get no_log values from suboptions
sub_argument_spec = arg_opts.get('options')
if sub_argument_spec is not None:
wanted_type = arg_opts.get('type')
sub_parameters = params.get(arg_name)
if sub_parameters is not None:
if wanted_type == 'dict' or (wanted_type == 'list' and arg_opts.get('elements', '') == 'dict'):
# Sub parameters can be a dict or list of dicts. Ensure parameters are always a list.
if not isinstance(sub_parameters, list):
sub_parameters = [sub_parameters]
for sub_param in sub_parameters:
# Validate dict fields in case they came in as strings
if isinstance(sub_param, string_types):
sub_param = check_type_dict(sub_param)
if not isinstance(sub_param, Mapping):
raise TypeError("Value '{1}' in the sub parameter field '{0}' must by a {2}, "
"not '{1.__class__.__name__}'".format(arg_name, sub_param, wanted_type))
no_log_values.update(list_no_log_values(sub_argument_spec, sub_param))
return no_log_values

45
test/integration/targets/no_log/library/module.py Normal file
View file

@ -0,0 +1,45 @@
#!/usr/bin/python
# -*- coding: utf-8 -*-
# Copyright (c) 2019 Ansible Project
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
from __future__ import absolute_import, division, print_function
__metaclass__ = type
from ansible.module_utils.basic import AnsibleModule
def main():
module = AnsibleModule(
argument_spec={
'state': {},
'secret': {'no_log': True},
'subopt_dict': {
'type': 'dict',
'options': {
'str_sub_opt1': {'no_log': True},
'str_sub_opt2': {},
'nested_subopt': {
'type': 'dict',
'options': {
'n_subopt1': {'no_log': True},
}
}
}
},
'subopt_list': {
'type': 'list',
'elements': 'dict',
'options': {
'subopt1': {'no_log': True},
'subopt2': {},
}
}
}
)
module.exit_json(msg='done')
if __name__ == '__main__':
main()

24
test/integration/targets/no_log/no_log_suboptions.yml Normal file
View file

@ -0,0 +1,24 @@
- name: test no log with suboptions
hosts: testhost
gather_facts: no
tasks:
- name: Task with suboptions
module:
secret: GLAMOROUS
subopt_dict:
str_sub_opt1: AFTERMATH
str_sub_opt2: otherstring
nested_subopt:
n_subopt1: MANPOWER
subopt_list:
- subopt1: UNTAPPED
subopt2: thridstring
- subopt1: CONCERNED
- name: Task with suboptions as string
module:
secret: MARLIN
subopt_dict: str_sub_opt1=FLICK

45
test/integration/targets/no_log/no_log_suboptions_invalid.yml Normal file
View file

@ -0,0 +1,45 @@
- name: test no log with suboptions
hosts: testhost
gather_facts: no
ignore_errors: yes
tasks:
- name: Task with suboptions and invalid parameter
module:
secret: SUPREME
invalid: param
subopt_dict:
str_sub_opt1: IDIOM
str_sub_opt2: otherstring
nested_subopt:
n_subopt1: MOCKUP
subopt_list:
- subopt1: EDUCATED
subopt2: thridstring
- subopt1: FOOTREST
- name: Task with suboptions as string with invalid parameter
module:
secret: FOOTREST
invalid: param
subopt_dict: str_sub_opt1=CRAFTY
- name: Task with suboptions with dict instead of list
module:
secret: FELINE
subopt_dict:
str_sub_opt1: CRYSTAL
str_sub_opt2: otherstring
nested_subopt:
n_subopt1: EXPECTANT
subopt_list:
foo: bar
- name: Task with suboptions with incorrect data type
module:
secret: AGROUND
subopt_dict: 9068.21361
subopt_list:
- subopt1: GOLIATH
- subopt1: FREEFALL

6
test/integration/targets/no_log/runme.sh
View file

@ -13,3 +13,9 @@ set -eux
# no log disabled, should produce 0 censored
[ "$(ansible-playbook dynamic.yml -i ../../inventory -vvvvv "$@" -e unsafe_show_logs=yes|grep -c 'output has been hidden')" = "0" ]
# test no log for sub options
[ "$(ansible-playbook no_log_suboptions.yml -i ../../inventory -vvvvv "$@" | grep -Ec '(MANPOWER|UNTAPPED|CONCERNED|MARLIN|FLICK)')" = "0" ]
# test invalid data passed to a suboption
[ "$(ansible-playbook no_log_suboptions_invalid.yml -i ../../inventory -vvvvv "$@" | grep -Ec '(SUPREME|IDIOM|MOCKUP|EDUCATED|FOOTREST|CRAFTY|FELINE|CRYSTAL|EXPECTANT|AGROUND|GOLIATH|FREEFALL)')" = "0" ]

219
test/units/module_utils/common/parameters/test_list_no_log_values.py
View file

@ -11,31 +11,218 @@ from ansible.module_utils.common.parameters import list_no_log_values
@pytest.fixture
def params():
return {
'secret': 'undercookwovennativity',
'other_secret': 'cautious-slate-makeshift',
'state': 'present',
'value': 5,
}
def test_list_no_log_values(params):
argument_spec = {
def argument_spec():
# Allow extra specs to be passed to the fixture, which will be added to the output
def _argument_spec(extra_opts=None):
spec = {
'secret': {'type': 'str', 'no_log': True},
'other_secret': {'type': 'str', 'no_log': True},
'state': {'type': 'str'},
'value': {'type': 'int'},
}
result = set(('undercookwovennativity', 'cautious-slate-makeshift'))
assert result == list_no_log_values(argument_spec, params)
if extra_opts:
spec.update(extra_opts)
return spec
return _argument_spec
def test_list_no_log_values_no_secrets(params):
@pytest.fixture
def module_parameters():
# Allow extra parameters to be passed to the fixture, which will be added to the output
def _module_parameters(extra_params=None):
params = {
'secret': 'under',
'other_secret': 'makeshift',
'state': 'present',
'value': 5,
}
if extra_params:
params.update(extra_params)
return params
return _module_parameters
def test_list_no_log_values_no_secrets(module_parameters):
argument_spec = {
'other_secret': {'type': 'str', 'no_log': False},
'state': {'type': 'str'},
'value': {'type': 'int'},
}
result = set()
assert result == list_no_log_values(argument_spec, params)
expected = set()
assert expected == list_no_log_values(argument_spec, module_parameters)
def test_list_no_log_values(argument_spec, module_parameters):
expected = set(('under', 'makeshift'))
assert expected == list_no_log_values(argument_spec(), module_parameters())
@pytest.mark.parametrize('extra_params', [
{'subopt1': 1},
{'subopt1': 3.14159},
{'subopt1': ['one', 'two']},
{'subopt1': ('one', 'two')},
])
def test_list_no_log_values_invalid_suboptions(argument_spec, module_parameters, extra_params):
extra_opts = {
'subopt1': {
'type': 'dict',
'options': {
'sub_1_1': {},
}
}
}
with pytest.raises(TypeError, match=r"(Value '.*?' in the sub parameter field '.*?' must by a dict, not '.*?')"
r"|(dictionary requested, could not parse JSON or key=value)"):
list_no_log_values(argument_spec(extra_opts), module_parameters(extra_params))
def test_list_no_log_values_suboptions(argument_spec, module_parameters):
extra_opts = {
'subopt1': {
'type': 'dict',
'options': {
'sub_1_1': {'no_log': True},
'sub_1_2': {'type': 'list'},
}
}
}
extra_params = {
'subopt1': {
'sub_1_1': 'bagel',
'sub_1_2': ['pebble'],
}
}
expected = set(('under', 'makeshift', 'bagel'))
assert expected == list_no_log_values(argument_spec(extra_opts), module_parameters(extra_params))
def test_list_no_log_values_sub_suboptions(argument_spec, module_parameters):
extra_opts = {
'sub_level_1': {
'type': 'dict',
'options': {
'l1_1': {'no_log': True},
'l1_2': {},
'l1_3': {
'type': 'dict',
'options': {
'l2_1': {'no_log': True},
'l2_2': {},
}
}
}
}
}
extra_params = {
'sub_level_1': {
'l1_1': 'saucy',
'l1_2': 'napped',
'l1_3': {
'l2_1': 'corporate',
'l2_2': 'tinsmith',
}
}
}
expected = set(('under', 'makeshift', 'saucy', 'corporate'))
assert expected == list_no_log_values(argument_spec(extra_opts), module_parameters(extra_params))
def test_list_no_log_values_suboptions_list(argument_spec, module_parameters):
extra_opts = {
'subopt1': {
'type': 'list',
'elements': 'dict',
'options': {
'sub_1_1': {'no_log': True},
'sub_1_2': {},
}
}
}
extra_params = {
'subopt1': [
{
'sub_1_1': ['playroom', 'luxury'],
'sub_1_2': 'deuce',
},
{
'sub_1_2': ['squishier', 'finished'],
}
]
}
expected = set(('under', 'makeshift', 'playroom', 'luxury'))
assert expected == list_no_log_values(argument_spec(extra_opts), module_parameters(extra_params))
def test_list_no_log_values_sub_suboptions_list(argument_spec, module_parameters):
extra_opts = {
'subopt1': {
'type': 'list',
'elements': 'dict',
'options': {
'sub_1_1': {'no_log': True},
'sub_1_2': {},
'subopt2': {
'type': 'list',
'elements': 'dict',
'options': {
'sub_2_1': {'no_log': True, 'type': 'list'},
'sub_2_2': {},
}
}
}
}
}
extra_params = {
'subopt1': {
'sub_1_1': ['playroom', 'luxury'],
'sub_1_2': 'deuce',
'subopt2': [
{
'sub_2_1': ['basis', 'gave'],
'sub_2_2': 'liquid',
},
{
'sub_2_1': ['composure', 'thumping']
},
]
}
}
expected = set(('under', 'makeshift', 'playroom', 'luxury', 'basis', 'gave', 'composure', 'thumping'))
assert expected == list_no_log_values(argument_spec(extra_opts), module_parameters(extra_params))
@pytest.mark.parametrize('extra_params, expected', (
({'subopt_dict': 'dict_subopt1=rekindle-scandal,dict_subopt2=subgroupavenge'}, ('rekindle-scandal',)),
({'subopt_dict': 'dict_subopt1=aversion-mutable dict_subopt2=subgroupavenge'}, ('aversion-mutable',)),
({'subopt_dict': ['dict_subopt1=blip-marine,dict_subopt2=subgroupavenge', 'dict_subopt1=tipping,dict_subopt2=hardening']}, ('blip-marine', 'tipping')),
))
def test_string_suboptions_as_string(argument_spec, module_parameters, extra_params, expected):
extra_opts = {
'subopt_dict': {
'type': 'dict',
'options': {
'dict_subopt1': {'no_log': True},
'dict_subopt2': {},
},
},
}
result = set(('under', 'makeshift'))
result.update(expected)
assert result == list_no_log_values(argument_spec(extra_opts), module_parameters(extra_params))