From fcd6d7010d579780137d765d5a5b4883c009af39 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 22 Apr 2016 08:40:34 -0700 Subject: [PATCH] Fixup perms dont rely on privileged user named root (#15482) * Don't rely on username to check for root privileges The SSH username isn't a reliable way to check if we've got root privileges on the remote system (think "toor" on FreeBSD). Because of this check, Ansible previously tried to use the fallback solutions for granting file access (ACLs, world-readable files) even on systems where it had root privileges when the remote username didn't match the literal string "root". Instead of running checks on the username, just try using `chmod` in any case and fall back to the previous "non-root" solution when that fails. * Fail if we are root and changing ownership failed Since this code is security sensitive we document exactly the expected permissions of the temporary files once this function has run. That way if a flaw is found in one end-result we know more precisely what scenarios are affected and which are not. --- lib/ansible/plugins/action/__init__.py | 46 ++++++++++++++++++++------ 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 3f9ef230fd9..9a0e99fb8d6 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -293,8 +293,26 @@ class ActionBase(with_metaclass(ABCMeta, object)): 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. + 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 + (because the files could contain passwords or other private + information. We achieve this in one of these ways: + + * If no sudo is performed or the remote_user is sudo'ing to + themselves, we don't have to change permisions. + * If the remote_user sudo's to a privileged user (for instance, root), + we don't have to change permissions + * If the remote_user is a privileged user and sudo's to an + unprivileged user then we change the owner of the file to the + unprivileged user so they can read it. + * If the remote_user is an unprivieged user and we're sudo'ing to + a second unprivileged user then we attempt to grant the second + unprivileged user access via file system acls. + * If granting file system acls fails we can set the file to be world + readable so that the second unprivileged user can read the file. + Since this could allow other users to get access to private + information we only do this ansible is configured with + "allow_world_readable_tmpfiles" in the ansible.cfg """ if self._connection._shell.SHELL_FAMILY == 'powershell': # This won't work on Powershell as-is, so we'll just completely skip until @@ -312,18 +330,26 @@ class ActionBase(with_metaclass(ABCMeta, object)): 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 - res = self._remote_chown(remote_path, self._play_context.become_user, recursive=recursive) - if res['rc'] != 0: - raise AnsibleError('Failed to set owner on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr'])) + + # Try chown'ing the file. This will only work if our SSH user has + # root privileges, but since we can't reliably determine that from + # the username (think "toor" on FreeBSD), let's just try first and + # apologize later: + res = self._remote_chown(remote_path, self._play_context.become_user, recursive=recursive) + if res['rc'] == 0: + # root can read things that don't have read bit but can't + # execute them without the execute bit, so we might need to + # set that even if we're root. We just ran chown successfully, + # so apparently we are root. if execute: - # root can read things that don't have read bit but can't - # execute them. res = self._remote_chmod('u+x', remote_path, recursive=recursive) if res['rc'] != 0: - raise AnsibleError('Failed to set file mode on remote files (rc: {0}, err: {1})'.format(res['rc'], res['stderr'])) + raise AnsibleError('Failed to set file mode on remote temporary files (rc: {0}, err: {1})'.format(res['rc'], res['stderr'])) + + elif remote_user == '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.') else: + # Chown'ing failed. We're probably lacking root privileges; let's try something else. if execute: mode = 'rx' else: