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

Use file list, not recursion, in _fixup_perms. (#16924)

Browse source 
Run setfacl/chown/chmod on each temp dir and file.

This fixes temp file permissions handling on platforms such as FreeBSD
which always return success when using find -exec. This is done by
eliminating the use of find when setting up temp files and directories.

(cherry picked from commit 72cca01cd4)
This commit is contained in:
Matt Clay 2016-08-05 18:40:28 -07:00
parent f956ff9619
commit cf9ef724e9
10 changed files with 54 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
lib/ansible/plugins/action/__init__.py
View file

@ -293,7 +293,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
return remote_path
def _fixup_perms(self, remote_path, remote_user, execute=False, recursive=True):
def _fixup_perms(self, remote_paths, remote_user, execute=True):
"""
We need the files we upload to be readable (and sometimes executable)
by the user being sudo'd to but we want to limit other people's access
@ -319,15 +319,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
if self._connection._shell.SHELL_FAMILY == 'powershell':
# This won't work on Powershell as-is, so we'll just completely skip until
# we have a need for it, at which point we'll have to do something different.
return remote_path
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
display.debug('_fixup_perms called with remote_path==None. Sure this is correct?')
return remote_path
return remote_paths
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
@ -340,17 +332,17 @@ class ActionBase(with_metaclass(ABCMeta, object)):
else:
mode = 'rX'
res = self._remote_set_user_facl(remote_path, self._play_context.become_user, mode, recursive=recursive, sudoable=False)
res = self._remote_set_user_facl(remote_paths, self._play_context.become_user, mode)
if res['rc'] != 0:
# File system acls failed; let's try to use chown next
# Set executable bit first as on some systems an
# unprivileged user can use chown
if execute:
res = self._remote_chmod('u+x', remote_path, recursive=recursive)
res = self._remote_chmod(remote_paths, 'u+x')
if res['rc'] != 0:
raise AnsibleError('Failed to set file mode on remote temporary files (rc: {0}, err: {1})'.format(res['rc'], res['stderr']))
res = self._remote_chown(remote_path, self._play_context.become_user, recursive=recursive)
res = self._remote_chown(remote_paths, self._play_context.become_user)
if res['rc'] != 0 and remote_user == 'root':
# chown failed even if remove_user is root
raise AnsibleError('Failed to change ownership of the temporary files Ansible needs to create despite connecting as root. Unprivileged become user would be unable to read the file.')
@ -359,7 +351,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
# chown and fs acls failed -- do things this insecure
# way only if the user opted in in the config file
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')
res = self._remote_chmod('a+%s' % mode, remote_path, recursive=recursive)
res = self._remote_chmod(remote_paths, 'a+%s' % mode)
if res['rc'] != 0:
raise AnsibleError('Failed to set file mode on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr']))
else:
@ -368,33 +360,33 @@ class ActionBase(with_metaclass(ABCMeta, object)):
# Can't depend on the file being transferred with execute
# permissions. Only need user perms because no become was
# used here
res = self._remote_chmod('u+x', remote_path, recursive=recursive)
res = self._remote_chmod(remote_paths, 'u+x')
if res['rc'] != 0:
raise AnsibleError('Failed to set file mode on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr']))
return remote_path
return remote_paths
def _remote_chmod(self, mode, path, recursive=True, sudoable=False):
def _remote_chmod(self, paths, mode, sudoable=False):
'''
Issue a remote chmod command
'''
cmd = self._connection._shell.chmod(mode, path, recursive=recursive)
cmd = self._connection._shell.chmod(paths, mode)
res = self._low_level_execute_command(cmd, sudoable=sudoable)
return res
def _remote_chown(self, path, user, group=None, recursive=True, sudoable=False):
def _remote_chown(self, paths, user, sudoable=False):
'''
Issue a remote chown command
'''
cmd = self._connection._shell.chown(path, user, group, recursive=recursive)
cmd = self._connection._shell.chown(paths, user)
res = self._low_level_execute_command(cmd, sudoable=sudoable)
return res
def _remote_set_user_facl(self, path, user, mode, recursive=True, sudoable=False):
def _remote_set_user_facl(self, paths, user, mode, sudoable=False):
'''
Issue a remote call to setfacl
'''
cmd = self._connection._shell.set_user_facl(path, user, mode, recursive=recursive)
cmd = self._connection._shell.set_user_facl(paths, user, mode)
res = self._low_level_execute_command(cmd, sudoable=sudoable)
return res
@ -609,9 +601,17 @@ class ActionBase(with_metaclass(ABCMeta, object)):
environment_string = self._compute_environment_string()
remote_files = None
if args_file_path:
remote_files = tmp, remote_module_path, args_file_path
elif remote_module_path:
remote_files = tmp, remote_module_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)
if remote_files:
self._fixup_perms(remote_files, remote_user)
cmd = ""
in_data = None

2
lib/ansible/plugins/action/assemble.py
View file

@ -153,7 +153,7 @@ class ActionModule(ActionBase):
xfered = self._transfer_file(path, remote_path)
# fix file permissions when the copy is done as a different user
self._fixup_perms(tmp, remote_user, recursive=True)
self._fixup_perms((tmp, remote_path), remote_user)
new_module_args.update( dict( src=xfered,))

18
lib/ansible/plugins/action/async.py
View file

@ -70,17 +70,13 @@ class ActionModule(ActionBase):
args_data += '%s="%s" ' % (k, pipes.quote(to_unicode(v)))
argsfile = self._transfer_data(self._connection._shell.join_path(tmp, 'arguments'), args_data)
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)
remote_paths = tmp, remote_module_path, async_module_path
# argsfile doesn't need to be executable, but this saves an extra call to the remote host
if argsfile:
remote_paths += argsfile,
self._fixup_perms(remote_paths, remote_user, execute=True)
async_limit = self._task.async
async_jid = str(random.randint(0, 999999999999))

7
lib/ansible/plugins/action/copy.py
View file

@ -217,8 +217,10 @@ class ActionModule(ActionBase):
# Define a remote directory that we will copy the file to.
tmp_src = self._connection._shell.join_path(tmp, 'source')
remote_path = None
if not raw:
self._transfer_file(source_full, tmp_src)
remote_path = self._transfer_file(source_full, tmp_src)
else:
self._transfer_file(source_full, dest_file)
@ -227,7 +229,8 @@ class ActionModule(ActionBase):
self._loader.cleanup_tmp_file(source_full)
# fix file permissions when the copy is done as a different user
self._fixup_perms(tmp, remote_user, recursive=True)
if remote_path:
self._fixup_perms((tmp, remote_path), remote_user)
if raw:
# Continue to next iteration if raw is defined.

2
lib/ansible/plugins/action/patch.py
View file

@ -59,7 +59,7 @@ class ActionModule(ActionBase):
tmp_src = self._connection._shell.join_path(tmp, os.path.basename(src))
self._transfer_file(src, tmp_src)
self._fixup_perms(tmp, remote_user, recursive=True)
self._fixup_perms((tmp, tmp_src), remote_user)
new_module_args = self._task.args.copy()
new_module_args.update(

2
lib/ansible/plugins/action/script.py
View file

@ -79,7 +79,7 @@ class ActionModule(ActionBase):
self._transfer_file(source, tmp_src)
# set file permissions, more permissive when the copy is done as a different user
self._fixup_perms(tmp, remote_user, execute=True, recursive=True)
self._fixup_perms((tmp, tmp_src), remote_user, execute=True)
# add preparation steps to one ssh roundtrip executing the script
env_string = self._compute_environment_string()

2
lib/ansible/plugins/action/template.py
View file

@ -164,7 +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
self._fixup_perms(tmp, remote_user, recursive=True)
self._fixup_perms((tmp, xfered), remote_user)
# run the copy module
new_module_args.update(

2
lib/ansible/plugins/action/unarchive.py
View file

@ -93,7 +93,7 @@ class ActionModule(ActionBase):
if copy:
# fix file permissions when the copy is done as a different user
self._fixup_perms(tmp, remote_user, recursive=True)
self._fixup_perms((tmp, tmp_src), remote_user)
# Build temporary module_args.
new_module_args = self._task.args.copy()
new_module_args.update(

42
lib/ansible/plugins/shell/__init__.py
View file

@ -56,45 +56,25 @@ class ShellBase(object):
def path_has_trailing_slash(self, path):
return path.endswith('/')
def chmod(self, mode, path, recursive=True):
path = pipes.quote(path)
cmd = ['chmod']
if recursive:
cmd.append('-R') # many chmods require -R before file list
cmd.extend([mode, path])
def chmod(self, paths, mode):
cmd = ['chmod', mode]
cmd.extend(paths)
cmd = [pipes.quote(c) for c in cmd]
return ' '.join(cmd)
def chown(self, path, user, group=None, recursive=True):
path = pipes.quote(path)
user = pipes.quote(user)
cmd = ['chown']
if recursive:
cmd.append('-R') # many chowns require -R before file list
if group is None:
cmd.extend([user, path])
else:
group = pipes.quote(group)
cmd.extend(['%s:%s' % (user, group), path])
def chown(self, paths, user):
cmd = ['chown', user]
cmd.extend(paths)
cmd = [pipes.quote(c) for c in cmd]
return ' '.join(cmd)
def set_user_facl(self, path, user, mode, recursive=True):
def set_user_facl(self, paths, user, mode):
"""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', '-m', 'u:%s:%s' % (user, mode)]
if recursive:
cmd = ['find', path, '-exec'] + cmd + ["'{}'", "'+'"]
else:
cmd.append(path)
cmd.extend(paths)
cmd = [pipes.quote(c) for c in cmd]
return ' '.join(cmd)

6
lib/ansible/plugins/shell/powershell.py
View file

@ -66,13 +66,13 @@ class ShellModule(object):
path = self._unquote(path)
return path.endswith('/') or path.endswith('\\')
def chmod(self, mode, path, recursive=True):
def chmod(self, paths, mode):
raise NotImplementedError('chmod is not implemented for Powershell')
def chown(self, path, user, group=None, recursive=True):
def chown(self, paths, user):
raise NotImplementedError('chown is not implemented for Powershell')
def set_user_facl(self, path, user, mode, recursive=True):
def set_user_facl(self, paths, user, mode):
raise NotImplementedError('set_user_facl is not implemented for Powershell')
def remove(self, path, recurse=False):