Fix warning for new default permissions when mode is not specified (#…

…70976) (#70985)

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

(cherry picked from commit dc79528)

changelogs/fragments/67794-default-permissions-warning-fix.yml

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

docs/docsite/rst/porting_guides/porting_guide_2.10.rst

@@ -58,7 +58,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:
 

lib/ansible/module_utils/basic.py

@@ -1125,16 +1125,16 @@ def set_group_if_different(self, path, group, changed, diff=None, expand=True):
 
     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:
             self._created_files.remove(path)
         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))

test/integration/targets/module_utils_basic/aliases

@@ -0,0 +1 @@
+shippable/posix/group5

test/integration/targets/module_utils_basic/library/test_perm_warning.py

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

test/integration/targets/module_utils_basic/meta/main.yml

@@ -0,0 +1,2 @@
+dependencies:
+  - setup_remote_tmp_dir

test/integration/targets/module_utils_basic/tasks/main.yml

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