From 52e9209491dee6a0c63edaa770b8601092248283 Mon Sep 17 00:00:00 2001
From: Toshio Kuratomi <toshio@fedoraproject.org>
Date: Mon, 21 Mar 2016 14:17:53 -0700
Subject: [PATCH] Don't create world-readable module and tempfiles without
 explicit user permission

---
 docsite/rst/become.rst                   | 59 +++++++++++----
 docsite/rst/intro_configuration.rst      |  2 +-
 examples/ansible.cfg                     |  8 ++
 lib/ansible/constants.py                 |  1 +
 lib/ansible/plugins/action/__init__.py   | 93 ++++++++++++++++++++----
 lib/ansible/plugins/action/assemble.py   | 10 +--
 lib/ansible/plugins/action/async.py      | 17 ++++-
 lib/ansible/plugins/action/copy.py       | 12 +--
 lib/ansible/plugins/action/patch.py      |  9 +--
 lib/ansible/plugins/action/script.py     | 12 +--
 lib/ansible/plugins/action/template.py   |  6 +-
 lib/ansible/plugins/action/unarchive.py  | 15 ++--
 lib/ansible/plugins/shell/__init__.py    | 35 ++++++++-
 test/units/plugins/action/test_action.py | 16 ++--
 14 files changed, 217 insertions(+), 78 deletions(-)

diff --git a/docsite/rst/become.rst b/docsite/rst/become.rst
index f7082d1d111..e72e40c7193 100644
--- a/docsite/rst/become.rst
+++ b/docsite/rst/become.rst
@@ -85,25 +85,33 @@ on how it works.  Users should be aware of these to avoid surprises.
 Becoming an Unprivileged User
 =============================
 
-Ansible has a limitation with regards to becoming an
+Ansible 2.0.x and below has a limitation with regards to becoming an
 unprivileged user that can be a security risk if users are not aware of it.
 Ansible modules are executed on the remote machine by first substituting the
 parameters into the module file, then copying the file to the remote machine,
-and finally executing it there.  If the module file is executed without using
-become, when the become user is root, or when the connection to the remote
-machine is made as root then the module file is created with permissions that
-only allow reading by the user and root.
+and finally executing it there.
 
-If the become user is an unprivileged user and then Ansible has no choice but
-to make the module file world readable as there's no other way for the user
-Ansible connects as to save the file so that the user that we're becoming can
-read it.
+Everything is fine if the module file is executed without using ``become``,
+when the ``become_user`` is root, or when the connection to the remote machine
+is made as root.  In these cases the module file is created with permissions
+that only allow reading by the user and root.
+
+The problem occurs when the ``become_user`` is an unprivileged user.  Ansible
+2.0.x and below make the module file world readable in this case as the module
+file is written as the user that Ansible connects as but the file needs to
+be reasable by the user Ansible is set to ``become``.
+
+.. note:: In Ansible 2.1, this window is further narrowed: If the connection
+    is made as a privileged user (root) then Ansible 2.1 and above will use
+    chown to set the file's owner to the unprivileged user being switched to.
+    This means both the user making the connection and the user being switched
+    to via ``become`` must be unprivileged in order to trigger this problem.
 
 If any of the parameters passed to the module are sensitive in nature then
-those pieces of data are readable by reading the module file for the duration
-of the Ansible module execution.  Once the module is done executing Ansible
-will delete the temporary file.  If you trust the client machines then there's
-no problem here.  If you do not trust the client machines then this is
+those pieces of data are located in a world readable module file for the
+duration of the Ansible module execution.  Once the module is done executing
+Ansible will delete the temporary file.  If you trust the client machines then
+there's no problem here.  If you do not trust the client machines then this is
 a potential danger.
 
 Ways to resolve this include:
@@ -113,9 +121,32 @@ Ways to resolve this include:
   the remote python interpreter's stdin.  Pipelining does not work for
   non-python modules.
 
+* (Available in Ansible 2.1) Install filesystem acl support on the managed
+  host.  If the temporary directory on the remote host is mounted with
+  filesystem acls enabled and the :command:`setfacl` tool is in the remote
+  ``PATH`` then Ansible will use filesystem acls to share the module file with
+  the second unprivileged instead of having to make the file readable by
+  everyone.
+
 * Don't perform an action on the remote machine by becoming an unprivileged
   user.  Temporary files are protected by UNIX file permissions when you
-  become root or do not use become.
+  ``become`` root or do not use ``become``.  In Ansible 2.1 and above, UNIX
+  file permissions are also secure if you make the connection to the managed
+  machine as root and then use ``become`` to an unprivileged account.
+
+.. versionchanged:: 2.1
+
+In addition to the additional means of doing this securely, Ansible 2.1 also
+makes it harder to unknowingly do this insecurely.  Whereas in Ansible 2.0.x
+and below, Ansible will silently allow the insecure behaviour if it was unable
+to find another way to share the files with the unprivileged user, in Ansible
+2.1 and above Ansible defaults to issuing an error if it can't do this
+securely.  If you can't make any of the changes above to resolve the problem
+and you decide that the machine you're running on is secure enough for the
+modules you want to run there to be world readable you can turn on
+``allow_world_readable_tmpfiles`` in the :file:`ansible.cfg` file.  Setting
+``allow_world_readable_tmpfiles`` will change this from an error into
+a warning and allow the task to run as it did prior to 2.1.
 
 Connection Plugin Support
 =========================
diff --git a/docsite/rst/intro_configuration.rst b/docsite/rst/intro_configuration.rst
index b10f460db28..1ca71e210e0 100644
--- a/docsite/rst/intro_configuration.rst
+++ b/docsite/rst/intro_configuration.rst
@@ -60,7 +60,7 @@ General defaults
 
 In the [defaults] section of ansible.cfg, the following settings are tunable:
 
-.. _action_plugins:
+.. _cfg_action_plugins:
 
 action_plugins
 ==============
diff --git a/examples/ansible.cfg b/examples/ansible.cfg
index 98657cc0622..19913af9aa8 100644
--- a/examples/ansible.cfg
+++ b/examples/ansible.cfg
@@ -212,6 +212,14 @@
 # prevents logging of tasks, but only on the targets, data is still logged on the master/controller
 #no_target_syslog = False
 
+# controls whether Ansible will raise an error or warning if a task has no
+# choice but to create world readable temporary files to execute a module on
+# the remote machine.  This option is False by default for security.  Users may
+# turn this on to have behaviour more like Ansible prior to 2.1.x.  See
+# https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user
+# for more secure ways to fix this than enabling this option.
+#allow_world_readable_tmpfiles = False
+
 # controls the compression level of variables sent to
 # worker processes. At the default of 0, no compression
 # is used. This value must be an integer from 0 to 9.
diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py
index ea4f909cf54..1a9cbbce739 100644
--- a/lib/ansible/constants.py
+++ b/lib/ansible/constants.py
@@ -165,6 +165,7 @@ DEFAULT_VAR_COMPRESSION_LEVEL = get_config(p, DEFAULTS, 'var_compression_level',
 # disclosure
 DEFAULT_NO_LOG           = get_config(p, DEFAULTS, 'no_log', 'ANSIBLE_NO_LOG', False, boolean=True)
 DEFAULT_NO_TARGET_SYSLOG   = get_config(p, DEFAULTS, 'no_target_syslog', 'ANSIBLE_NO_TARGET_SYSLOG', False, boolean=True)
+ALLOW_WORLD_READABLE_TMPFILES = get_config(p, DEFAULTS, 'allow_world_readable_tmpfiles', None, False, boolean=True)
 
 # selinux
 DEFAULT_SELINUX_SPECIAL_FS = get_config(p, 'selinux', 'special_context_filesystems', None, 'fuse, nfs, vboxsf, ramfs', islist=True)
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 8666329ffff..2ba0650de39 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -192,7 +192,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
             return True
         return False
 
-    def _make_tmp_path(self):
+    def _make_tmp_path(self, remote_user):
         '''
         Create and return a temporary path on a remote box.
         '''
@@ -200,12 +200,10 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         basefile = 'ansible-tmp-%s-%s' % (time.time(), random.randint(0, 2**48))
         use_system_tmp = False
 
-        if self._play_context.become and self._play_context.become_user != 'root':
+        if self._play_context.become and self._play_context.become_user not in ('root', remote_user):
             use_system_tmp = True
 
-        tmp_mode = None
-        if self._play_context.remote_user != 'root' or self._play_context.become and self._play_context.become_user != 'root':
-            tmp_mode = 0o755
+        tmp_mode = 0o700
 
         cmd = self._connection._shell.mkdtemp(basefile, use_system_tmp, tmp_mode)
         result = self._low_level_execute_command(cmd, sudoable=False)
@@ -255,6 +253,10 @@ class ActionBase(with_metaclass(ABCMeta, object)):
             # If ssh breaks we could leave tmp directories out on the remote system.
             self._low_level_execute_command(cmd, sudoable=False)
 
+    def _transfer_file(self, local_path, remote_path):
+        self._connection.put_file(local_path, remote_path)
+        return remote_path
+
     def _transfer_data(self, remote_path, data):
         '''
         Copies the module data out to the temporary module path.
@@ -269,25 +271,85 @@ class ActionBase(with_metaclass(ABCMeta, object)):
             data = to_bytes(data, errors='strict')
             afo.write(data)
         except Exception as e:
-            #raise AnsibleError("failure encoding into utf-8: %s" % str(e))
             raise AnsibleError("failure writing module data to temporary file for transfer: %s" % str(e))
 
         afo.flush()
         afo.close()
 
         try:
-            self._connection.put_file(afile, remote_path)
+            self._transfer_file(afile, remote_path)
         finally:
             os.unlink(afile)
 
         return remote_path
 
-    def _remote_chmod(self, mode, path, sudoable=False):
+    def _fixup_perms(self, remote_path, remote_user, execute=False, recursive=True):
+        """
+        If the become_user is unprivileged and different from the
+        remote_user then we need to make the files we've uploaded readable by them.
+        """
+        if remote_path is None:
+            # Sometimes code calls us naively -- it has a var which could
+            # contain a path to a tmp dir but doesn't know if it needs to
+            # exist or not.  If there's no path, then there's no need for us
+            # to do work
+            self._display.debug('_fixup_perms called with remote_path==None.  Sure this is correct?')
+            return remote_path
+
+        if self._play_context.become and self._play_context.become_user not in ('root', remote_user):
+            # Unprivileged user that's different than the ssh user.  Let's get
+            # to work!
+            if remote_user == 'root':
+                # SSh'ing as root, therefore we can chown
+                self._remote_chown(remote_path, self._play_context.become_user, recursive=recursive)
+                if execute:
+                    # root can read things that don't have read bit but can't
+                    # execute them.
+                    self._remote_chmod('u+x', remote_path, recursive=recursive)
+            else:
+                if execute:
+                    mode = 'rx'
+                else:
+                    mode = 'rX'
+                # Try to use fs acls to solve this problem
+                res = self._remote_set_user_facl(remote_path, self._play_context.become_user, mode, recursive=recursive, sudoable=False)
+                if res['rc'] != 0:
+                    if C.ALLOW_WORLD_READABLE_TMPFILES:
+                        # fs acls failed -- do things this insecure way only
+                        # if the user opted in in the config file
+                        self._display.warning('Using world-readable permissions for temporary files Ansible needs to create when becoming an unprivileged user which may be insecure. For information on securing this, see https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user')
+                        self._remote_chmod('a+%s' % mode, remote_path, recursive=recursive)
+                    else:
+                        raise AnsibleError('Failed to set permissions on the temporary files Ansible needs to create when becoming an unprivileged user. For information on working around this, see https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user')
+        elif execute:
+            # Can't depend on the file being transferred with execute
+            # permissions.  Only need user perms because no become was
+            # used here
+            self._remote_chmod('u+x', remote_path, recursive=recursive)
+
+        return remote_path
+
+    def _remote_chmod(self, mode, path, recursive=True, sudoable=False):
         '''
         Issue a remote chmod command
         '''
+        cmd = self._connection._shell.chmod(mode, path, recursive=recursive)
+        res = self._low_level_execute_command(cmd, sudoable=sudoable)
+        return res
+
+    def _remote_chown(self, path, user, group=None, recursive=True, sudoable=False):
+        '''
+        Issue a remote chown command
+        '''
+        cmd = self._connection._shell.chown(path, user, group, recursive=recursive)
+        res = self._low_level_execute_command(cmd, sudoable=sudoable)
+        return res
 
-        cmd = self._connection._shell.chmod(mode, path)
+    def _remote_set_user_facl(self, path, user, mode, recursive=True, sudoable=False):
+        '''
+        Issue a remote call to setfacl
+        '''
+        cmd = self._connection._shell.set_user_facl(path, user, mode, recursive=recursive)
         res = self._low_level_execute_command(cmd, sudoable=sudoable)
         return res
 
@@ -417,6 +479,9 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         else:
             module_args['_ansible_check_mode'] = False
 
+        # Get the connection user for permission checks
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
+
         # set no log in the module arguments, if required
         module_args['_ansible_no_log'] = self._play_context.no_log or C.DEFAULT_NO_TARGET_SYSLOG
 
@@ -437,7 +502,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         remote_module_path = None
         args_file_path = None
         if not tmp and self._late_needs_tmp_path(tmp, module_style):
-            tmp = self._make_tmp_path()
+            tmp = self._make_tmp_path(remote_user)
 
         if tmp:
             remote_module_filename = self._connection._shell.get_remote_filename(module_name)
@@ -462,11 +527,9 @@ class ActionBase(with_metaclass(ABCMeta, object)):
 
         environment_string = self._compute_environment_string()
 
-        if tmp and "tmp" in tmp and self._play_context.become and self._play_context.become_user != 'root':
-            # deal with possible umask issues once sudo'ed to other user
-            self._remote_chmod('a+r', remote_module_path)
-            if args_file_path is not None:
-                self._remote_chmod('a+r', args_file_path)
+        # Fix permissions of the tmp path and tmp files.  This should be
+        # called after all files have been transferred.
+        self._fixup_perms(tmp, remote_user, recursive=True)
 
         cmd = ""
         in_data = None
diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py
index eeb13c21ae9..9c7e7091225 100644
--- a/lib/ansible/plugins/action/assemble.py
+++ b/lib/ansible/plugins/action/assemble.py
@@ -98,8 +98,9 @@ class ActionModule(ActionBase):
             return result
 
         cleanup_remote_tmp = False
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
         if not tmp:
-            tmp = self._make_tmp_path()
+            tmp = self._make_tmp_path(remote_user)
             cleanup_remote_tmp = True
 
         if boolean(remote_src):
@@ -146,16 +147,15 @@ class ActionModule(ActionBase):
         )
 
         if path_checksum != dest_stat['checksum']:
-            resultant = file(path).read()
 
             if self._play_context.diff:
                 diff = self._get_diff_data(dest, path, task_vars)
 
-            xfered = self._transfer_data('src', resultant)
+            remote_path = self._connection._shell.join_path(tmp, 'src')
+            xfered = self._transfer_file(path, remote_path)
 
             # fix file permissions when the copy is done as a different user
-            if self._play_context.become and self._play_context.become_user != 'root':
-                self._remote_chmod('a+r', xfered)
+            self._fixup_perms(tmp, remote_user, recursive=True)
 
             new_module_args.update( dict( src=xfered,))
 
diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py
index 8a7175aeb86..b85f1c20f46 100644
--- a/lib/ansible/plugins/action/async.py
+++ b/lib/ansible/plugins/action/async.py
@@ -38,8 +38,9 @@ class ActionModule(ActionBase):
             result['msg'] = 'check mode not supported for this module'
             return result
 
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
         if not tmp:
-            tmp = self._make_tmp_path()
+            tmp = self._make_tmp_path(remote_user)
 
         module_name = self._task.action
         async_module_path  = self._connection._shell.join_path(tmp, 'async_wrapper')
@@ -54,15 +55,25 @@ class ActionModule(ActionBase):
         # configure, upload, and chmod the target module
         (module_style, shebang, module_data) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars)
         self._transfer_data(remote_module_path, module_data)
-        self._remote_chmod('a+rx', remote_module_path)
 
         # configure, upload, and chmod the async_wrapper module
         (async_module_style, shebang, async_module_data) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars)
         self._transfer_data(async_module_path, async_module_data)
-        self._remote_chmod('a+rx', async_module_path)
 
         argsfile = self._transfer_data(self._connection._shell.join_path(tmp, 'arguments'), json.dumps(module_args))
 
+        self._fixup_perms(tmp, remote_user, execute=True, recursive=True)
+        # Only the following two files need to be executable but we'd have to
+        # make three remote calls if we wanted to just set them executable.
+        # There's not really a problem with marking too many of the temp files
+        # executable so we go ahead and mark them all as executable in the
+        # line above (the line above is needed in any case [although
+        # execute=False is okay if we uncomment the lines below] so that all
+        # the files are readable in case the remote_user and become_user are
+        # different and both unprivileged)
+        #self._fixup_perms(remote_module_path, remote_user, execute=True, recursive=False)
+        #self._fixup_perms(async_module_path, remote_user, execute=True, recursive=False)
+
         async_limit = self._task.async
         async_jid   = str(random.randint(0, 999999999999))
 
diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py
index b8094b2bd61..9734a294446 100644
--- a/lib/ansible/plugins/action/copy.py
+++ b/lib/ansible/plugins/action/copy.py
@@ -141,9 +141,10 @@ class ActionModule(ActionBase):
         delete_remote_tmp = (len(source_files) == 1)
 
         # If this is a recursive action create a tmp path that we can share as the _exec_module create is too late.
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
         if not delete_remote_tmp:
             if tmp is None or "-tmp-" not in tmp:
-                tmp = self._make_tmp_path()
+                tmp = self._make_tmp_path(remote_user)
 
         # expand any user home dir specifier
         dest = self._remote_expand_user(dest)
@@ -196,7 +197,7 @@ class ActionModule(ActionBase):
                 # If this is recursive we already have a tmp path.
                 if delete_remote_tmp:
                     if tmp is None or "-tmp-" not in tmp:
-                        tmp = self._make_tmp_path()
+                        tmp = self._make_tmp_path(remote_user)
 
                 if self._play_context.diff and not raw:
                     diffs.append(self._get_diff_data(dest_file, source_full, task_vars))
@@ -211,16 +212,15 @@ class ActionModule(ActionBase):
                 tmp_src = self._connection._shell.join_path(tmp, 'source')
 
                 if not raw:
-                    self._connection.put_file(source_full, tmp_src)
+                    self._transfer_file(source_full, tmp_src)
                 else:
-                    self._connection.put_file(source_full, dest_file)
+                    self._transfer_file(source_full, dest_file)
 
                 # We have copied the file remotely and no longer require our content_tempfile
                 self._remove_tempfile_if_content_defined(content, content_tempfile)
 
                 # fix file permissions when the copy is done as a different user
-                if self._play_context.become and self._play_context.become_user != 'root':
-                    self._remote_chmod('a+r', tmp_src)
+                self._fixup_perms(tmp, remote_user, recursive=True)
 
                 if raw:
                     # Continue to next iteration if raw is defined.
diff --git a/lib/ansible/plugins/action/patch.py b/lib/ansible/plugins/action/patch.py
index c08f901cf58..4fbc69b66a9 100644
--- a/lib/ansible/plugins/action/patch.py
+++ b/lib/ansible/plugins/action/patch.py
@@ -34,6 +34,7 @@ class ActionModule(ActionBase):
 
         src        = self._task.args.get('src', None)
         remote_src = boolean(self._task.args.get('remote_src', 'no'))
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
 
         if src is None:
             result['failed'] = True
@@ -52,14 +53,12 @@ class ActionModule(ActionBase):
 
         # create the remote tmp dir if needed, and put the source file there
         if tmp is None or "-tmp-" not in tmp:
-            tmp = self._make_tmp_path()
+            tmp = self._make_tmp_path(remote_user)
 
         tmp_src = self._connection._shell.join_path(tmp, os.path.basename(src))
-        self._connection.put_file(src, tmp_src)
+        self._transfer_file(src, tmp_src)
 
-        if self._play_context.become and self._play_context.become_user != 'root':
-            if not self._play_context.check_mode:
-                self._remote_chmod('a+r', tmp_src)
+        self._fixup_perms(tmp, remote_user, recursive=True)
 
         new_module_args = self._task.args.copy()
         new_module_args.update(
diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py
index 5b0f324dfcf..a0d66405486 100644
--- a/lib/ansible/plugins/action/script.py
+++ b/lib/ansible/plugins/action/script.py
@@ -38,8 +38,9 @@ class ActionModule(ActionBase):
             result['msg'] = 'check mode not supported for this module'
             return result
 
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
         if not tmp:
-            tmp = self._make_tmp_path()
+            tmp = self._make_tmp_path(remote_user)
 
         creates = self._task.args.get('creates')
         if creates:
@@ -76,16 +77,11 @@ class ActionModule(ActionBase):
 
         # transfer the file to a remote tmp location
         tmp_src = self._connection._shell.join_path(tmp, os.path.basename(source))
-        self._connection.put_file(source, tmp_src)
+        self._transfer_file(source, tmp_src)
 
         sudoable = True
         # set file permissions, more permissive when the copy is done as a different user
-        if self._play_context.become and self._play_context.become_user != 'root':
-            chmod_mode = 'a+rx'
-            sudoable = False
-        else:
-            chmod_mode = '+rx'
-        self._remote_chmod(chmod_mode, tmp_src, sudoable=sudoable)
+        self._fixup_perms(tmp, remote_user, execute=True, recursive=True)
 
         # add preparation steps to one ssh roundtrip executing the script
         env_string = self._compute_environment_string()
diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py
index 5ddd624bde2..66320aaf293 100644
--- a/lib/ansible/plugins/action/template.py
+++ b/lib/ansible/plugins/action/template.py
@@ -138,8 +138,9 @@ class ActionModule(ActionBase):
             return result
 
         cleanup_remote_tmp = False
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
         if not tmp:
-            tmp = self._make_tmp_path()
+            tmp = self._make_tmp_path(remote_user)
             cleanup_remote_tmp = True
 
         local_checksum = checksum_s(resultant)
@@ -163,8 +164,7 @@ class ActionModule(ActionBase):
                 xfered = self._transfer_data(self._connection._shell.join_path(tmp, 'source'), resultant)
 
                 # fix file permissions when the copy is done as a different user
-                if self._play_context.become and self._play_context.become_user != 'root':
-                    self._remote_chmod('a+r', xfered)
+                self._fixup_perms(tmp, remote_user, recursive=True)
 
                 # run the copy module
                 new_module_args.update(
diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py
index b6c43a3c595..6ace6f99758 100644
--- a/lib/ansible/plugins/action/unarchive.py
+++ b/lib/ansible/plugins/action/unarchive.py
@@ -45,8 +45,9 @@ class ActionModule(ActionBase):
             result['msg'] = "src (or content) and dest are required"
             return result
 
+        remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user
         if not tmp:
-            tmp = self._make_tmp_path()
+            tmp = self._make_tmp_path(remote_user)
 
         if creates:
             # do not run the command if the line contains creates=filename
@@ -80,17 +81,15 @@ class ActionModule(ActionBase):
 
         if copy:
             # transfer the file to a remote tmp location
-            tmp_src = tmp + 'source'
-            self._connection.put_file(source, tmp_src)
+            tmp_src = self._connection._shell.join_path(tmp, 'source')
+            self._transfer_file(source, tmp_src)
 
         # handle diff mode client side
         # handle check mode client side
-        # fix file permissions when the copy is done as a different user
-        if copy:
-            if self._play_context.become and self._play_context.become_user != 'root':
-                if not self._play_context.check_mode:
-                    self._remote_chmod('a+r', tmp_src)
 
+        if copy:
+            # fix file permissions when the copy is done as a different user
+            self._fixup_perms(tmp, remote_user, recursive=True)
             # Build temporary module_args.
             new_module_args = self._task.args.copy()
             new_module_args.update(
diff --git a/lib/ansible/plugins/shell/__init__.py b/lib/ansible/plugins/shell/__init__.py
index 50530c6b691..daa011e3da0 100644
--- a/lib/ansible/plugins/shell/__init__.py
+++ b/lib/ansible/plugins/shell/__init__.py
@@ -52,9 +52,40 @@ class ShellBase(object):
     def path_has_trailing_slash(self, path):
         return path.endswith('/')
 
-    def chmod(self, mode, path):
+    def chmod(self, mode, path, recursive=True):
         path = pipes.quote(path)
-        return 'chmod %s %s' % (mode, path)
+        cmd = ['chmod', mode, path]
+        if recursive:
+            cmd.append('-R')
+        return ' '.join(cmd)
+
+    def chown(self, path, user, group=None, recursive=True):
+        path = pipes.quote(path)
+        user = pipes.quote(user)
+
+        if group is None:
+            cmd = ['chown', user, path]
+        else:
+            group = pipes.quote(group)
+            cmd = ['chown', '%s:%s' % (user, group), path]
+
+        if recursive:
+            cmd.append('-R')
+
+        return ' '.join(cmd)
+
+    def set_user_facl(self, path, user, mode, recursive=True):
+        """Only sets acls for users as that's really all we need"""
+        path = pipes.quote(path)
+        mode = pipes.quote(mode)
+        user = pipes.quote(user)
+
+        cmd = ['setfacl']
+        if recursive:
+            cmd.append('-R')
+        cmd.extend(('-m', 'u:%s:%s %s' % (user, mode, path)))
+
+        return ' '.join(cmd)
 
     def remove(self, path, recurse=False):
         path = pipes.quote(path)
diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py
index 4bb151f090f..8c97bf04155 100644
--- a/test/units/plugins/action/test_action.py
+++ b/test/units/plugins/action/test_action.py
@@ -392,27 +392,27 @@ class TestActionBase(unittest.TestCase):
 
         action_base._low_level_execute_command = MagicMock()
         action_base._low_level_execute_command.return_value = dict(rc=0, stdout='/some/path')
-        self.assertEqual(action_base._make_tmp_path(), '/some/path/')
+        self.assertEqual(action_base._make_tmp_path('root'), '/some/path/')
 
         # empty path fails
         action_base._low_level_execute_command.return_value = dict(rc=0, stdout='')
-        self.assertRaises(AnsibleError, action_base._make_tmp_path)
+        self.assertRaises(AnsibleError, action_base._make_tmp_path, 'root')
 
         # authentication failure
         action_base._low_level_execute_command.return_value = dict(rc=5, stdout='')
-        self.assertRaises(AnsibleError, action_base._make_tmp_path)
+        self.assertRaises(AnsibleError, action_base._make_tmp_path, 'root')
 
         # ssh error
         action_base._low_level_execute_command.return_value = dict(rc=255, stdout='', stderr='')
-        self.assertRaises(AnsibleError, action_base._make_tmp_path)
+        self.assertRaises(AnsibleError, action_base._make_tmp_path, 'root')
         play_context.verbosity = 5
-        self.assertRaises(AnsibleError, action_base._make_tmp_path)
+        self.assertRaises(AnsibleError, action_base._make_tmp_path, 'root')
 
         # general error
         action_base._low_level_execute_command.return_value = dict(rc=1, stdout='some stuff here', stderr='')
-        self.assertRaises(AnsibleError, action_base._make_tmp_path)
+        self.assertRaises(AnsibleError, action_base._make_tmp_path, 'root')
         action_base._low_level_execute_command.return_value = dict(rc=1, stdout='some stuff here', stderr='No space left on device')
-        self.assertRaises(AnsibleError, action_base._make_tmp_path)
+        self.assertRaises(AnsibleError, action_base._make_tmp_path, 'root')
 
     def test_action_base__remove_tmp_path(self):
         # create our fake task
@@ -567,8 +567,8 @@ class TestActionBase(unittest.TestCase):
         action_base._make_tmp_path = MagicMock()
         action_base._transfer_data = MagicMock()
         action_base._compute_environment_string = MagicMock()
-        action_base._remote_chmod = MagicMock()
         action_base._low_level_execute_command = MagicMock()
+        action_base._fixup_perms = MagicMock()
 
         action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data')
         action_base._late_needs_tmp_path.return_value = False