Fix warning for new default permissions when mode is not specified (#70976)
Follow up to #70221 Related to #67794 CVE-2020-1736 When set_mode_if_different() is called with mode of 'None', ensure we issue a warning about the change in default permissions. Add integration tests to ensure the warning works properly. * Fix tests - actually use custom module 🤦♂️ - verify file permission on created files - use remote_tmp_dir so we're ready for split controller - improve test module so we can skip the call to set_fs_attributes_if_different() - fix tests for CentOS 6
This commit is contained in:
parent
14dc4de424
commit
dc79528cc6
7 changed files with 80 additions and 4 deletions
|
@ -0,0 +1,4 @@
|
|||
bugfixes:
|
||||
- >
|
||||
Fix warning for default permission change when no mode is specified. Follow up
|
||||
to https://github.com/ansible/ansible/issues/67794. (CVE-2020-1736)
|
|
@ -48,7 +48,7 @@ A new warning will be displayed when all of the following conditions are true:
|
|||
|
||||
- The file at the final destination, not the temporary file, does not exist
|
||||
- A module supports setting ``mode`` but it was not specified for the task
|
||||
- The module calls ``atomic_move()`` but does not later call ``set_fs_attributes_if_different()`` or ``set_mode_if_different()``
|
||||
- The module calls ``atomic_move()`` but does not later call ``set_fs_attributes_if_different()`` or ``set_mode_if_different()`` with a ``mode`` specified
|
||||
|
||||
The following modules call ``atomic_move()`` but do not call ``set_fs_attributes_if_different()`` or ``set_mode_if_different()`` and do not support setting ``mode``. This means for files they create, the default permissions have changed and there is no indication:
|
||||
|
||||
|
|
|
@ -1126,6 +1126,9 @@ class AnsibleModule(object):
|
|||
|
||||
def set_mode_if_different(self, path, mode, changed, diff=None, expand=True):
|
||||
|
||||
if mode is None:
|
||||
return changed
|
||||
|
||||
# Remove paths so we do not warn about creating with default permissions
|
||||
# since we are calling this method on the path and setting the specified mode.
|
||||
try:
|
||||
|
@ -1133,9 +1136,6 @@ class AnsibleModule(object):
|
|||
except KeyError:
|
||||
pass
|
||||
|
||||
if mode is None:
|
||||
return changed
|
||||
|
||||
b_path = to_bytes(path, errors='surrogate_or_strict')
|
||||
if expand:
|
||||
b_path = os.path.expanduser(os.path.expandvars(b_path))
|
||||
|
|
1
test/integration/targets/module_utils_basic/aliases
Normal file
1
test/integration/targets/module_utils_basic/aliases
Normal file
|
@ -0,0 +1 @@
|
|||
shippable/posix/group5
|
|
@ -0,0 +1,36 @@
|
|||
#!/usr/bin/python
|
||||
# -*- coding: utf-8 -*-
|
||||
# Copyright (c) 2020 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
|
||||
|
||||
import tempfile
|
||||
|
||||
from ansible.module_utils.basic import AnsibleModule
|
||||
|
||||
|
||||
def main():
|
||||
module = AnsibleModule(
|
||||
argument_spec={
|
||||
'dest': {'type': 'path'},
|
||||
'call_fs_attributes': {'type': 'bool', 'default': True},
|
||||
},
|
||||
add_file_common_args=True,
|
||||
)
|
||||
|
||||
results = {}
|
||||
|
||||
with tempfile.NamedTemporaryFile(delete=False) as tf:
|
||||
file_args = module.load_file_common_arguments(module.params)
|
||||
module.atomic_move(tf.name, module.params['dest'])
|
||||
|
||||
if module.params['call_fs_attributes']:
|
||||
results['changed'] = module.set_fs_attributes_if_different(file_args, True)
|
||||
|
||||
module.exit_json(**results)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
main()
|
|
@ -0,0 +1,2 @@
|
|||
dependencies:
|
||||
- setup_remote_tmp_dir
|
33
test/integration/targets/module_utils_basic/tasks/main.yml
Normal file
33
test/integration/targets/module_utils_basic/tasks/main.yml
Normal file
|
@ -0,0 +1,33 @@
|
|||
- name: Run task with no mode
|
||||
test_perm_warning:
|
||||
dest: "{{ remote_tmp_dir }}/endangerdisown"
|
||||
register: no_mode_results
|
||||
|
||||
- name: Run task with mode
|
||||
test_perm_warning:
|
||||
mode: '0644'
|
||||
dest: "{{ remote_tmp_dir }}/groveestablish"
|
||||
register: with_mode_results
|
||||
|
||||
- name: Run task without calling set_fs_attributes_if_different()
|
||||
test_perm_warning:
|
||||
call_fs_attributes: no
|
||||
dest: "{{ remote_tmp_dir }}/referabletank"
|
||||
register: skip_fs_attributes
|
||||
|
||||
- stat:
|
||||
path: "{{ remote_tmp_dir }}/{{ item }}"
|
||||
loop:
|
||||
- endangerdisown
|
||||
- groveestablish
|
||||
register: files
|
||||
|
||||
- name: Ensure we get a warning when appropriate
|
||||
assert:
|
||||
that:
|
||||
- no_mode_results.warnings | default([], True) | length == 1
|
||||
- "'created with default permissions' in no_mode_results.warnings[0]"
|
||||
- files.results[0]['stat']['mode'] == '0600'
|
||||
- files.results[1]['stat']['mode'] == '0644'
|
||||
- with_mode_results.warnings is not defined # The Jinja version on CentOS 6 does not support default([], True)
|
||||
- skip_fs_attributes.warnings | default([], True) | length == 1
|
Loading…
Reference in a new issue