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
pull/70990/head
Sam Doran 4 years ago committed by GitHub
parent 14dc4de424
commit dc79528cc6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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))

@ -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

@ -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…
Cancel
Save