diff --git a/changelogs/fragments/fixup_perms2-cleanup.yml b/changelogs/fragments/fixup_perms2-cleanup.yml new file mode 100644 index 00000000000..61881aa6fb1 --- /dev/null +++ b/changelogs/fragments/fixup_perms2-cleanup.yml @@ -0,0 +1,2 @@ +minor_changes: + - Restructured _fixup_perms2() in ansible.plugins.action to make it more linear diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 199f04d29c2..62bdd851158 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -532,103 +532,149 @@ class ActionBase(with_metaclass(ABCMeta, object)): if remote_user is None: remote_user = self._get_remote_user() + # Step 1: Are we on windows? if getattr(self._connection._shell, "_IS_WINDOWS", False): - # 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. + # 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_paths - if self._is_become_unprivileged(): - # Unprivileged user that's different than the ssh user. Let's get - # to work! - - become_user = self.get_become_option('become_user') - - # Try to use file system acls to make the files readable for sudo'd - # user + # Step 2: If we're not becoming an unprivileged user, we are roughly + # done. Make the files +x if we're asked to, and return. + if not self._is_become_unprivileged(): if execute: - chmod_mode = 'rx' - setfacl_mode = 'r-x' - else: - chmod_mode = 'rX' - # NOTE: this form fails silently on freebsd. We currently - # never call _fixup_perms2() with execute=False but if we - # start to we'll have to fix this. - setfacl_mode = 'r-X' + # 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(remote_paths, 'u+x') + if res['rc'] != 0: + raise AnsibleError( + 'Failed to set execute bit on remote files ' + '(rc: {0}, err: {1})'.format( + res['rc'], + to_native(res['stderr']))) + return remote_paths - res = self._remote_set_user_facl(remote_paths, become_user, setfacl_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(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'], to_native(res['stderr']))) + # If we're still here, we have an unprivileged user that's different + # than the ssh user. + become_user = self.get_become_option('become_user') - res = self._remote_chown(remote_paths, become_user) - if res['rc'] != 0: - # First check if we are an admin/root user. If we are - # and failed here, something weird has happened. - if remote_user in self._get_admin_users(): - # chown failed even if remote_user is administrator/root - raise AnsibleError('Failed to change ownership of the temporary files Ansible needs to create despite connecting as a privileged user. ' - 'Unprivileged become user would be unable to read the file.') - - # Otherwise, we're a normal user. We failed to chown the - # paths to the unprivileged user, but if we have a common - # group with them, we should be able to chown it to that. - # - # Note that we have no way of knowing if this will actually - # work... just because chgrp exits successfully does not - # mean that Ansible will work. We could check if the become - # user is in the group, but this would create an extra - # round trip. - # - # Also note that due to the above, this can prevent the - # ALLOW_WORLD_READABLE_TMPFILES logic below from ever - # getting called. We leave this up to the user to rectify - # if they have both of these features enabled. - group = self.get_shell_option('common_remote_group') - if group is not None: - res = self._remote_chgrp(remote_paths, group) - if res['rc'] == 0: - # If ALLOW_WORLD_READABLE_TMPFILES is set, we should warn the user - # that something might go weirdly here. - if C.ALLOW_WORLD_READABLE_TMPFILES: - display.warning('Both common_remote_group and allow_world_readable_tmpfiles are set. chgrp was successful, but there is no ' - 'guarantee that Ansible will be able to read the files after this operation, particularly if ' - 'common_remote_group was set to a group of which the unprivileged become user is not a member. In this ' - 'situation, allow_world_readable_tmpfiles is a no-op. See the "Risks of becoming an unprivileged user" section ' - 'of the "Understanding privilege escalation: become" user guide documentation for more information') - if execute: - group_mode = 'g+rwx' - else: - group_mode = 'g+rw' - res = self._remote_chmod(remote_paths, group_mode) + # Try to use file system acls to make the files readable for sudo'd + # user + if execute: + chmod_mode = 'rx' + setfacl_mode = 'r-x' + else: + chmod_mode = 'rX' + # TODO: this form fails silently on freebsd. We currently + # never call _fixup_perms2() with execute=False but if we + # start to we'll have to fix this. + setfacl_mode = 'r-X' + + # Step 3a: Are we able to use setfacl to add user ACLs to the file? + res = self._remote_set_user_facl( + remote_paths, + become_user, + setfacl_mode) + + if res['rc'] == 0: + return remote_paths - if res['rc'] != 0: - if self.get_shell_option('world_readable_temp', C.ALLOW_WORLD_READABLE_TMPFILES): - # 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. ' - 'This may be insecure. For information on securing this, see ' - 'https://docs.ansible.com/ansible/user_guide/become.html#risks-of-becoming-an-unprivileged-user') - res = self._remote_chmod(remote_paths, 'a+%s' % chmod_mode) - if res['rc'] != 0: - raise AnsibleError('Failed to set file mode on remote files (rc: {0}, err: {1})'.format(res['rc'], to_native(res['stderr']))) - else: - raise AnsibleError('Failed to set permissions on the temporary files Ansible needs to create when becoming an unprivileged user ' - '(rc: %s, err: %s}). For information on working around this, see ' - 'https://docs.ansible.com/ansible/become.html#becoming-an-unprivileged-user' - % (res['rc'], to_native(res['stderr']))) - elif execute: - # Can't depend on the file being transferred with execute permissions. - # Only need user perms because no become was used here + # Step 3b: Set execute if we need to. We do this before anything else + # because some of the methods below might work but not let us set +x + # as part of them. + if execute: res = self._remote_chmod(remote_paths, 'u+x') if res['rc'] != 0: - raise AnsibleError('Failed to set execute bit on remote files (rc: {0}, err: {1})'.format(res['rc'], to_native(res['stderr']))) + raise AnsibleError( + 'Failed to set file mode on remote temporary files ' + '(rc: {0}, err: {1})'.format( + res['rc'], + to_native(res['stderr']))) + + # Step 3c: File system ACLs failed above; try falling back to chown. + res = self._remote_chown(remote_paths, become_user) + if res['rc'] == 0: + return remote_paths - return remote_paths + # Check if we are an admin/root user. If we are and got here, it means + # we failed to chown as root and something weird has happened. + if remote_user in self._get_admin_users(): + raise AnsibleError( + 'Failed to change ownership of the temporary files Ansible ' + 'needs to create despite connecting as a privileged user. ' + 'Unprivileged become user would be unable to read the ' + 'file.') + + # Step 3d: Common group + # Otherwise, we're a normal user. We failed to chown the paths to the + # unprivileged user, but if we have a common group with them, we should + # be able to chown it to that. + # + # Note that we have no way of knowing if this will actually work... just + # because chgrp exits successfully does not mean that Ansible will work. + # We could check if the become user is in the group, but this would + # create an extra round trip. + # + # Also note that due to the above, this can prevent the + # ALLOW_WORLD_READABLE_TMPFILES logic below from ever getting called. We + # leave this up to the user to rectify if they have both of these + # features enabled. + group = self.get_shell_option('common_remote_group') + if group is not None: + res = self._remote_chgrp(remote_paths, group) + if res['rc'] == 0: + # If ALLOW_WORLD_READABLE_TMPFILES is set, we should warn the + # user that something might go weirdly here. + if C.ALLOW_WORLD_READABLE_TMPFILES: + display.warning( + 'Both common_remote_group and ' + 'allow_world_readable_tmpfiles are set. chgrp was ' + 'successful, but there is no guarantee that Ansible ' + 'will be able to read the files after this operation, ' + 'particularly if common_remote_group was set to a ' + 'group of which the unprivileged become user is not a ' + 'member. In this situation, ' + 'allow_world_readable_tmpfiles is a no-op. See this ' + 'URL for more details: ' + 'https://docs.ansible.com/ansible/become.html' + '#becoming-an-unprivileged-user') + if execute: + group_mode = 'g+rwx' + else: + group_mode = 'g+rw' + res = self._remote_chmod(remote_paths, group_mode) + if res['rc'] == 0: + return remote_paths + + # Step 4: World-readable temp directory + if self.get_shell_option( + 'world_readable_temp', + C.ALLOW_WORLD_READABLE_TMPFILES): + # 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. This may ' + 'be insecure. For information on securing this, see ' + 'https://docs.ansible.com/ansible/user_guide/become.html' + '#risks-of-becoming-an-unprivileged-user') + res = self._remote_chmod(remote_paths, 'a+%s' % chmod_mode) + if res['rc'] == 0: + return remote_paths + raise AnsibleError( + 'Failed to set file mode on remote files ' + '(rc: {0}, err: {1})'.format( + res['rc'], + to_native(res['stderr']))) + + raise AnsibleError( + 'Failed to set permissions on the temporary files Ansible needs ' + 'to create when becoming an unprivileged user ' + '(rc: %s, err: %s}). For information on working around this, see ' + 'https://docs.ansible.com/ansible/become.html' + '#becoming-an-unprivileged-user' % ( + res['rc'], + to_native(res['stderr']))) def _remote_chmod(self, paths, mode, sudoable=False): ''' diff --git a/test/integration/targets/become_unprivileged/action_plugins/tmpdir.py b/test/integration/targets/become_unprivileged/action_plugins/tmpdir.py new file mode 100644 index 00000000000..b7cbb7a7e41 --- /dev/null +++ b/test/integration/targets/become_unprivileged/action_plugins/tmpdir.py @@ -0,0 +1,14 @@ +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.plugins.action import ActionBase + + +class ActionModule(ActionBase): + + def run(self, tmp=None, task_vars=None): + result = super(ActionModule, self).run(tmp, task_vars) + result.update(self._execute_module('ping', task_vars=task_vars)) + result['tmpdir'] = self._connection._shell.tmpdir + return result diff --git a/test/integration/targets/become_unprivileged/cleanup.yml b/test/integration/targets/become_unprivileged/cleanup.yml deleted file mode 100644 index d0e3d53bc65..00000000000 --- a/test/integration/targets/become_unprivileged/cleanup.yml +++ /dev/null @@ -1,8 +0,0 @@ -- name: Clean up host - hosts: ssh - gather_facts: yes - - # Default, just noted here to be explicit about what is happening: - remote_user: root - roles: - - cleanup_become_unprivileged diff --git a/test/integration/targets/become_unprivileged/cleanup_unpriv_users.yml b/test/integration/targets/become_unprivileged/cleanup_unpriv_users.yml new file mode 100644 index 00000000000..8be2fe60fc9 --- /dev/null +++ b/test/integration/targets/become_unprivileged/cleanup_unpriv_users.yml @@ -0,0 +1,53 @@ +- name: Clean up host and remove unprivileged users + hosts: ssh + gather_facts: yes + remote_user: root + tasks: + # Do this first so we can use tilde notation while the user still exists + - name: Delete homedirs + file: + path: '~{{ item }}' + state: absent + with_items: + - unpriv1 + - unpriv2 + + - name: Delete users + user: + name: "{{ item }}" + state: absent + force: yes # I think this is needed in case pipelining is used and the session remains open + with_items: + - unpriv1 + - unpriv2 + + - name: Delete groups + group: + name: "{{ item }}" + state: absent + with_items: + - acommongroup + - unpriv1 + - unpriv2 + + - name: Fix sudoers.d path for FreeBSD + set_fact: + sudoers_etc: /usr/local/etc + when: ansible_distribution == 'FreeBSD' + + - name: Fix sudoers.d path for everything else + set_fact: + sudoers_etc: /etc + when: ansible_distribution != 'FreeBSD' + + - name: Undo OpenSUSE + lineinfile: + path: "{{ sudoers_etc }}/sudoers" + regexp: '^### Defaults targetpw' + line: 'Defaults targetpw' + backrefs: yes + + - name: Nuke custom sudoers file + file: + path: "{{ sudoers_etc }}/sudoers.d/unpriv1" + state: absent diff --git a/test/integration/targets/become_unprivileged/common_remote_group/cleanup.yml b/test/integration/targets/become_unprivileged/common_remote_group/cleanup.yml new file mode 100644 index 00000000000..41784fcc0d8 --- /dev/null +++ b/test/integration/targets/become_unprivileged/common_remote_group/cleanup.yml @@ -0,0 +1,35 @@ +- name: Cleanup (as root) + hosts: ssh + gather_facts: yes + remote_user: root + tasks: + - name: Remove group for unprivileged users + group: + name: commongroup + state: absent + + - name: Check if /usr/bin/setfacl exists + stat: + path: /usr/bin/setfacl + register: usr_bin_setfacl + + - name: Check if /bin/setfacl exists + stat: + path: /bin/setfacl + register: bin_setfacl + + - name: Set path to setfacl + set_fact: + setfacl_path: /usr/bin/setfacl + when: usr_bin_setfacl.stat.exists + + - name: Set path to setfacl + set_fact: + setfacl_path: /bin/setfacl + when: bin_setfacl.stat.exists + + - name: chmod +x setfacl + file: + path: "{{ setfacl_path }}" + mode: a+x + when: setfacl_path is defined diff --git a/test/integration/targets/become_unprivileged/common_remote_group/setup.yml b/test/integration/targets/become_unprivileged/common_remote_group/setup.yml new file mode 100644 index 00000000000..1e799c47a47 --- /dev/null +++ b/test/integration/targets/become_unprivileged/common_remote_group/setup.yml @@ -0,0 +1,43 @@ +- name: Prep (as root) + hosts: ssh + gather_facts: yes + remote_user: root + tasks: + - name: Create group for unprivileged users + group: + name: commongroup + + - name: Add them to the group + user: + name: "{{ item }}" + groups: commongroup + append: yes + with_items: + - unpriv1 + - unpriv2 + + - name: Check if /usr/bin/setfacl exists + stat: + path: /usr/bin/setfacl + register: usr_bin_setfacl + + - name: Check if /bin/setfacl exists + stat: + path: /bin/setfacl + register: bin_setfacl + + - name: Set path to setfacl + set_fact: + setfacl_path: /usr/bin/setfacl + when: usr_bin_setfacl.stat.exists + + - name: Set path to setfacl + set_fact: + setfacl_path: /bin/setfacl + when: bin_setfacl.stat.exists + + - name: chmod -x setfacl to disable it + file: + path: "{{ setfacl_path }}" + mode: a-x + when: setfacl_path is defined diff --git a/test/integration/targets/become_unprivileged/common_remote_group/test.yml b/test/integration/targets/become_unprivileged/common_remote_group/test.yml new file mode 100644 index 00000000000..4bc51f8495f --- /dev/null +++ b/test/integration/targets/become_unprivileged/common_remote_group/test.yml @@ -0,0 +1,36 @@ +- name: Tests for ANSIBLE_COMMON_REMOTE_GROUP functionality + hosts: ssh + gather_facts: yes + remote_user: unpriv1 + + tasks: + - name: foo + action: tmpdir + register: tmpdir + become_user: unpriv2 + become: yes + + - name: run whoami with become + command: whoami + register: whoami + become_user: unpriv2 + become: yes + + - set_fact: + stat_cmd: stat -c '%U %G' {{ tmpdir.tmpdir }}/* + when: ansible_distribution not in ['MacOSX', 'FreeBSD'] + + - set_fact: + stat_cmd: stat -f '%Su %Sg' {{ tmpdir.tmpdir }}/* + when: ansible_distribution in ['MacOSX', 'FreeBSD'] + + - name: Ensure we tested the right fallback + shell: "{{ stat_cmd }}" + register: stat + become_user: unpriv2 + become: yes + + - assert: + that: + - whoami.stdout == "unpriv2" + - stat.stdout == 'unpriv1 commongroup' diff --git a/test/integration/targets/become_unprivileged/inventory b/test/integration/targets/become_unprivileged/inventory index ba06ca1c78b..025d8cf9ccc 100644 --- a/test/integration/targets/become_unprivileged/inventory +++ b/test/integration/targets/become_unprivileged/inventory @@ -1,7 +1,10 @@ [ssh] -ssh-pipelining ansible_ssh_pipelining=true -#ssh-no-pipelining ansible_ssh_pipelining=false +#ssh-pipelining ansible_ssh_pipelining=true +ssh-no-pipelining ansible_ssh_pipelining=false [ssh:vars] ansible_host=localhost ansible_connection=ssh ansible_python_interpreter="{{ ansible_playbook_python }}" + +[all:vars] +ansible_python_interpreter="{{ ansible_playbook_python }}" \ No newline at end of file diff --git a/test/integration/targets/become_unprivileged/roles/become_unprivileged/files/baz.txt b/test/integration/targets/become_unprivileged/roles/become_unprivileged/files/baz.txt deleted file mode 100644 index 40a246846b4..00000000000 --- a/test/integration/targets/become_unprivileged/roles/become_unprivileged/files/baz.txt +++ /dev/null @@ -1 +0,0 @@ -hello I was copied diff --git a/test/integration/targets/become_unprivileged/roles/become_unprivileged/tasks/main.yml b/test/integration/targets/become_unprivileged/roles/become_unprivileged/tasks/main.yml deleted file mode 100644 index 770a26d104b..00000000000 --- a/test/integration/targets/become_unprivileged/roles/become_unprivileged/tasks/main.yml +++ /dev/null @@ -1,58 +0,0 @@ -- name: Run a command - shell: whoami - register: whoami - -# TODO: We ignore_errors here because atomic_move has some really weird edge -# cases and gives different behavior based on whether the tmpdir we are copying -# from is on the same partition as the target or not, among other things. There -# is probably work to be done there to either unify the behavior if possible, or -# if not, document/add a warning. -# -# In what follows, unpriv1 is remote_user and unpriv2 is become_user. Both -# users are unprivileged. -# -# In particular, given a system (FreeBSD in my testing, but probably any *nix) -# with a single partition, when we connect (as unpriv1) and become unpriv2, -# the file ends up being unpriv1:commongroup. We can't chown it after that -# since we are become_user, so the file remains owned by unpriv1. -# -# But when we have multiple partitions, os.rename() in atomic_move fails, and -# we end up falling back to a whole new bunch of logic. In the end the file -# ends up being creted as unpriv2 and is unpriv2:unpriv2_login_group. -# -# This creates a bunch of inconsistency and really should be documented better -# but the relevant part for *this* test is that in the single-partition case, -# we cannot chmod in the `if creating` branch of atomic_move since we do not -# own the file. That will generate an error. -- name: Copy a file - copy: - src: baz.txt - dest: ~/uh-oh - owner: unpriv2 - group: notcoolenoughforroot - mode: 0644 - ignore_errors: yes - -- name: See if the file exists - stat: - path: ~/uh-oh - register: uh_oh_stat - -#- name: Get files in /var/tmp -# find: -# paths: "/var/tmp/" -# patterns: 'ansible*' -# file_type: directory -# register: found -# -#- name: Get latest ansible tmp dir -# set_fact: -# tmpdir: "{{ found.files | sort(attribute='mtime') | last }}" -# -#- debug: var=tmpdir - -- assert: - that: - - whoami.stdout == 'unpriv2' - - uh_oh_stat.stat.exists - #- tmpdir.gr_name == 'notcoolenoughforroot' diff --git a/test/integration/targets/become_unprivileged/roles/cleanup_become_unprivileged/tasks/main.yml b/test/integration/targets/become_unprivileged/roles/cleanup_become_unprivileged/tasks/main.yml deleted file mode 100644 index 8de432bba34..00000000000 --- a/test/integration/targets/become_unprivileged/roles/cleanup_become_unprivileged/tasks/main.yml +++ /dev/null @@ -1,74 +0,0 @@ -# Do this first so we can use tilde notation while the user still exists -- name: Delete homedirs - file: - path: '~{{ item }}' - state: absent - with_items: - - unpriv1 - - unpriv2 - -- name: Delete users - user: - name: "{{ item }}" - state: absent - force: yes # I think this is needed in case pipelining is used and the session remains open - with_items: - - unpriv1 - - unpriv2 - -- name: Delete groups - group: - name: "{{ item }}" - state: absent - with_items: - - notcoolenoughforroot - - unpriv1 - - unpriv2 - -- name: Fix sudoers.d path for FreeBSD - set_fact: - sudoers_etc: /usr/local/etc - when: ansible_distribution == 'FreeBSD' - -- name: Fix sudoers.d path for everything else - set_fact: - sudoers_etc: /etc - when: ansible_distribution != 'FreeBSD' - -- name: Undo OpenSUSE - lineinfile: - path: "{{ sudoers_etc }}/sudoers" - regexp: '^### Defaults targetpw' - line: 'Defaults targetpw' - backrefs: yes - -- name: Nuke custom sudoers file - file: - path: "{{ sudoers_etc }}/sudoers.d/unpriv1" - state: absent - -- name: Check if /usr/bin/setfacl exists - stat: - path: /usr/bin/setfacl - register: usr_bin_setfacl - -- name: Check if the /bin/setfacl exists - stat: - path: /bin/setfacl - register: bin_setfacl - -- name: Set path to setfacl - set_fact: - setfacl_path: /usr/bin/setfacl - when: usr_bin_setfacl.stat.exists - -- name: Set path to setfacl - set_fact: - setfacl_path: /bin/setfacl - when: bin_setfacl.stat.exists - -- name: chmod +x setfacl - file: - path: "{{ setfacl_path }}" - mode: +x - when: setfacl_path is defined diff --git a/test/integration/targets/become_unprivileged/roles/setup_become_unprivileged/tasks/main.yml b/test/integration/targets/become_unprivileged/roles/setup_become_unprivileged/tasks/main.yml deleted file mode 100644 index bbfe798950b..00000000000 --- a/test/integration/targets/become_unprivileged/roles/setup_become_unprivileged/tasks/main.yml +++ /dev/null @@ -1,128 +0,0 @@ ---- -#################################################################### -# NOTE! Any destructive changes you make here... Undo them in -# cleanup_become_unprivileged so that they don't affect other tests. -#################################################################### - -- name: Create groups for unprivileged users - group: - name: "{{ item }}" - with_items: - - notcoolenoughforroot - - unpriv1 - - unpriv2 - -# MacOS requires unencrypted password -- name: Set password for unpriv1 (MacOSX) - set_fact: - password: 'iWishIWereCoolEnoughForRoot!' - when: ansible_distribution == 'MacOSX' - -- name: Set password for unpriv1 (everything else) - set_fact: - password: $6$CRuKRUfAoVwibjUI$1IEOISMFAE/a0VG73K9QsD0uruXNPLNkZ6xWg4Sk3kZIXwv6.YJLECzfNjn6pu8ay6XlVcj2dUvycLetL5Lgx1 - when: ansible_distribution != 'MacOSX' - -# This user is special. It gets a password so we can sudo as it -# (we set the sudo password in runme.sh) and it gets wheel so it can -# `become` unpriv2 without an overly complex sudoers file. -- name: Create first unprivileged user - user: - name: unpriv1 - groups: unpriv1,notcoolenoughforroot - append: yes - password: "{{ password }}" - -- name: Create second unprivileged user - user: - name: unpriv2 - groups: unpriv2,notcoolenoughforroot - append: yes - -- name: Create .ssh for unpriv1 - file: - path: ~unpriv1/.ssh - state: directory - owner: unpriv1 - group: unpriv1 - mode: 0700 - -- name: Set authorized key for unpriv1 - copy: - src: ~root/.ssh/authorized_keys - dest: ~unpriv1/.ssh/authorized_keys - remote_src: yes - owner: unpriv1 - group: unpriv1 - mode: 0600 - -# Without this we get: -# "Failed to connect to the host via ssh: "System is booting up. Unprivileged -# users are not permitted to log in yet. Please come back later." -- name: Nuke /run/nologin - file: - path: /run/nologin - state: absent - -- name: Fix sudoers.d path for FreeBSD - set_fact: - sudoers_etc: /usr/local/etc - when: ansible_distribution == 'FreeBSD' - -- name: Fix sudoers.d path for everything else - set_fact: - sudoers_etc: /etc - when: sudoers_etc is not defined - -- name: Chown group for bsd and osx - set_fact: - chowngroup: wheel - when: ansible_distribution in ('FreeBSD', 'MacOSX') - -- name: Chown group for everything else - set_fact: - chowngroup: root - when: chowngroup is not defined - -- name: Make it so unpriv1 can sudo (Chapter 1) - copy: - dest: "{{ sudoers_etc }}/sudoers.d/unpriv1" - content: unpriv1 ALL=(ALL) ALL - owner: root - group: "{{ chowngroup }}" - mode: 0644 - -# OpenSUSE has a weird sudo default here and requires the root pw -# instead of the user pw. Undo that setting, we can clean it up later. -- name: Make it so unpriv1 can sudo (Chapter 2 - The Return Of the OpenSUSE) - lineinfile: - dest: "{{ sudoers_etc }}/sudoers" - regexp: '^Defaults targetpw' - line: '### Defaults targetpw' - backrefs: yes - -- name: Check if /usr/bin/setfacl exists - stat: - path: /usr/bin/setfacl - register: usr_bin_setfacl - -- name: Check if the /bin/setfacl exists - stat: - path: /bin/setfacl - register: bin_setfacl - -- name: Set path to setfacl - set_fact: - setfacl_path: /usr/bin/setfacl - when: usr_bin_setfacl.stat.exists - -- name: Set path to setfacl - set_fact: - setfacl_path: /bin/setfacl - when: bin_setfacl.stat.exists - -- name: chmod -x setfacl - file: - path: "{{ setfacl_path }}" - mode: -x - when: setfacl_path is defined diff --git a/test/integration/targets/become_unprivileged/runme.sh b/test/integration/targets/become_unprivileged/runme.sh index 6d52501fcc9..b7f38612920 100755 --- a/test/integration/targets/become_unprivileged/runme.sh +++ b/test/integration/targets/become_unprivileged/runme.sh @@ -1,14 +1,35 @@ #!/usr/bin/env bash -set -ux +set -eux -ansible-playbook setup.yml -i inventory -v "$@" +begin_sandwich() { + ansible-playbook setup_unpriv_users.yml -i inventory -v "$@" +} -export ANSIBLE_KEEP_REMOTE_FILES=True -export ANSIBLE_COMMON_REMOTE_GROUP=notcoolenoughforroot -export ANSIBLE_BECOME_PASS='iWishIWereCoolEnoughForRoot!' +end_sandwich() { + unset ANSIBLE_KEEP_REMOTE_FILES + unset ANSIBLE_COMMON_REMOTE_GROUP + unset ANSIBLE_BECOME_PASS -ansible-playbook test.yml -i inventory -v "$@" + # Do a few cleanup tasks (nuke users, groups, and homedirs, undo config changes) + ansible-playbook cleanup_unpriv_users.yml -i inventory -v "$@" -# Do a few cleanup tasks (nuke users, groups, and homedirs, undo config changes) -ansible-playbook cleanup.yml -i inventory -v "$@" + # We do these last since they do things like remove groups and will error + # if there are still users in them. + for pb in */cleanup.yml; do + ansible-playbook "$pb" -i inventory -v "$@" + done +} + +trap "end_sandwich \"\$@\"" EXIT + +# Common group tests +begin_sandwich "$@" + ansible-playbook common_remote_group/setup.yml -i inventory -v "$@" + export ANSIBLE_KEEP_REMOTE_FILES=True + export ANSIBLE_COMMON_REMOTE_GROUP=commongroup + export ANSIBLE_BECOME_PASS='iWishIWereCoolEnoughForRoot!' + ANSIBLE_ACTION_PLUGINS="$(pwd)/action_plugins" + export ANSIBLE_ACTION_PLUGINS + ansible-playbook common_remote_group/test.yml -i inventory -v "$@" +end_sandwich "$@" diff --git a/test/integration/targets/become_unprivileged/setup.yml b/test/integration/targets/become_unprivileged/setup.yml deleted file mode 100644 index 8f631db69c3..00000000000 --- a/test/integration/targets/become_unprivileged/setup.yml +++ /dev/null @@ -1,8 +0,0 @@ -- name: Set up host - hosts: ssh - gather_facts: yes - - # Default, just noted here to be explicit about what is happening: - remote_user: root - roles: - - setup_become_unprivileged diff --git a/test/integration/targets/become_unprivileged/setup_unpriv_users.yml b/test/integration/targets/become_unprivileged/setup_unpriv_users.yml new file mode 100644 index 00000000000..4f677413058 --- /dev/null +++ b/test/integration/targets/become_unprivileged/setup_unpriv_users.yml @@ -0,0 +1,109 @@ +#################################################################### +# NOTE! Any destructive changes you make here... Undo them in +# cleanup_become_unprivileged so that they don't affect other tests. +#################################################################### +- name: Set up host and create unprivileged users + hosts: ssh + gather_facts: yes + remote_user: root + tasks: + - name: Create groups for unprivileged users + group: + name: "{{ item }}" + with_items: + - unpriv1 + - unpriv2 + + # MacOS requires unencrypted password + - name: Set password for unpriv1 (MacOSX) + set_fact: + password: 'iWishIWereCoolEnoughForRoot!' + when: ansible_distribution == 'MacOSX' + + - name: Set password for unpriv1 (everything else) + set_fact: + password: $6$CRuKRUfAoVwibjUI$1IEOISMFAE/a0VG73K9QsD0uruXNPLNkZ6xWg4Sk3kZIXwv6.YJLECzfNjn6pu8ay6XlVcj2dUvycLetL5Lgx1 + when: ansible_distribution != 'MacOSX' + + # This user is special. It gets a password so we can sudo as it + # (we set the sudo password in runme.sh) and it gets wheel so it can + # `become` unpriv2 without an overly complex sudoers file. + - name: Create first unprivileged user + user: + name: unpriv1 + group: unpriv1 + password: "{{ password }}" + + - name: Create second unprivileged user + user: + name: unpriv2 + group: unpriv2 + + - name: Special case group add for macOS + user: + name: unpriv1 + groups: com.apple.access_ssh + append: yes + when: ansible_distribution == 'MacOSX' + + - name: Create .ssh for unpriv1 + file: + path: ~unpriv1/.ssh + state: directory + owner: unpriv1 + group: unpriv1 + mode: 0700 + + - name: Set authorized key for unpriv1 + copy: + src: ~root/.ssh/authorized_keys + dest: ~unpriv1/.ssh/authorized_keys + remote_src: yes + owner: unpriv1 + group: unpriv1 + mode: 0600 + + # Without this we get: + # "Failed to connect to the host via ssh: "System is booting up. Unprivileged + # users are not permitted to log in yet. Please come back later." + - name: Nuke /run/nologin + file: + path: /run/nologin + state: absent + + - name: Fix sudoers.d path for FreeBSD + set_fact: + sudoers_etc: /usr/local/etc + when: ansible_distribution == 'FreeBSD' + + - name: Fix sudoers.d path for everything else + set_fact: + sudoers_etc: /etc + when: sudoers_etc is not defined + + - name: Set chown group for bsd and osx + set_fact: + chowngroup: wheel + when: ansible_distribution in ('FreeBSD', 'MacOSX') + + - name: Chown group for everything else + set_fact: + chowngroup: root + when: chowngroup is not defined + + - name: Make it so unpriv1 can sudo (Chapter 1) + copy: + dest: "{{ sudoers_etc }}/sudoers.d/unpriv1" + content: unpriv1 ALL=(ALL) ALL + owner: root + group: "{{ chowngroup }}" + mode: 0644 + + # OpenSUSE has a weird sudo default here and requires the root pw + # instead of the user pw. Undo that setting, we can clean it up later. + - name: Make it so unpriv1 can sudo (Chapter 2 - The Return Of the OpenSUSE) + lineinfile: + dest: "{{ sudoers_etc }}/sudoers" + regexp: '^Defaults targetpw' + line: '### Defaults targetpw' + backrefs: yes diff --git a/test/integration/targets/become_unprivileged/test.yml b/test/integration/targets/become_unprivileged/test.yml deleted file mode 100644 index 6905469d25b..00000000000 --- a/test/integration/targets/become_unprivileged/test.yml +++ /dev/null @@ -1,8 +0,0 @@ -- name: Run the test - hosts: ssh - gather_facts: yes - remote_user: unpriv1 - become: yes - become_user: unpriv2 - roles: - - become_unprivileged diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 124880196ad..4ac793d57e6 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -316,6 +316,189 @@ class TestActionBase(unittest.TestCase): 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, 'root') + def test_action_base__fixup_perms2(self): + mock_task = MagicMock() + mock_connection = MagicMock() + play_context = PlayContext() + action_base = DerivedActionBase( + task=mock_task, + connection=mock_connection, + play_context=play_context, + loader=None, + templar=None, + shared_loader_obj=None, + ) + action_base._low_level_execute_command = MagicMock() + remote_paths = ['/tmp/foo/bar.txt', '/tmp/baz.txt'] + remote_user = 'remoteuser1' + + def runWithNoExpectation(execute=False): + return action_base._fixup_perms2( + remote_paths, + remote_user=remote_user, + execute=execute) + + def assertSuccess(execute=False): + self.assertEqual(runWithNoExpectation(execute), remote_paths) + + def assertThrowRegex(regex, execute=False): + self.assertRaisesRegexp( + AnsibleError, + regex, + action_base._fixup_perms2, + remote_paths, + remote_user=remote_user, + execute=execute) + + def get_shell_option_for_arg(args_kv, default): + '''A helper for get_shell_option. Returns a function that, if + called with ``option`` that exists in args_kv, will return the + value, else will return ``default`` for every other given arg''' + def _helper(option, *args, **kwargs): + return args_kv.get(option, default) + return _helper + + # Step 1: On windows, we just return remote_paths + action_base._connection._shell._IS_WINDOWS = True + assertSuccess(execute=False) + assertSuccess(execute=True) + + # But if we're not on windows....we have more work to do. + action_base._connection._shell._IS_WINDOWS = False + + # Step 2: We're /not/ becoming an unprivileged user + action_base._remote_chmod = MagicMock() + action_base._is_become_unprivileged = MagicMock() + action_base._is_become_unprivileged.return_value = False + # Two subcases: + # - _remote_chmod rc is 0 + # - _remote-chmod rc is not 0, something failed + action_base._remote_chmod.return_value = { + 'rc': 0, + 'stdout': 'some stuff here', + 'stderr': '', + } + assertSuccess(execute=True) + + # When execute=False, we just get the list back. But add it here for + # completion. chmod is never called. + assertSuccess() + + action_base._remote_chmod.return_value = { + 'rc': 1, + 'stdout': 'some stuff here', + 'stderr': 'and here', + } + assertThrowRegex( + 'Failed to set execute bit on remote files', + execute=True) + + # Step 3: we are becoming unprivileged + action_base._is_become_unprivileged.return_value = True + + # Step 3a: setfacl + action_base._remote_set_user_facl = MagicMock() + action_base._remote_set_user_facl.return_value = { + 'rc': 0, + 'stdout': '', + 'stderr': '', + } + assertSuccess() + + # Step 3b: chmod +x if we need to + # To get here, setfacl failed, so mock it as such. + action_base._remote_set_user_facl.return_value = { + 'rc': 1, + 'stdout': '', + 'stderr': '', + } + action_base._remote_chmod.return_value = { + 'rc': 0, + 'stdout': 'some stuff here', + 'stderr': '', + } + assertSuccess(execute=True) + action_base._remote_chmod.return_value = { + 'rc': 1, + 'stdout': 'some stuff here', + 'stderr': '', + } + assertThrowRegex( + 'Failed to set file mode on remote temporary file', + execute=True) + + # Step 3c: chown + action_base._remote_chown = MagicMock() + action_base._remote_chown.return_value = { + 'rc': 0, + 'stdout': '', + 'stderr': '', + } + assertSuccess() + action_base._remote_chown.return_value = { + 'rc': 1, + 'stdout': '', + 'stderr': '', + } + remote_user = 'root' + action_base._get_admin_users = MagicMock() + action_base._get_admin_users.return_value = ['root'] + assertThrowRegex('user would be unable to read the file.') + remote_user = 'remoteuser1' + + # Step 3d: Common group + + get_shell_option = action_base.get_shell_option + action_base.get_shell_option = MagicMock() + action_base.get_shell_option.side_effect = get_shell_option_for_arg( + { + 'common_remote_group': 'commongroup', + }, + None) + action_base._remote_chgrp = MagicMock() + action_base._remote_chgrp.return_value = { + 'rc': 0, + 'stdout': '', + 'stderr': '', + } + # TODO: Add test to assert warning is shown if + # ALLOW_WORLD_READABLE_TMPFILES is set in this case. + action_base._remote_chmod.return_value = { + 'rc': 0, + 'stdout': '', + 'stderr': '', + } + assertSuccess() + action_base._remote_chgrp.assert_called_once_with( + remote_paths, + 'commongroup') + + # Step 4: world-readable tmpdir + action_base.get_shell_option.side_effect = get_shell_option_for_arg( + { + 'world_readable_temp': True, + 'common_remote_group': None, + }, + None) + action_base._remote_chmod.return_value = { + 'rc': 0, + 'stdout': 'some stuff here', + 'stderr': '', + } + assertSuccess() + action_base._remote_chmod.return_value = { + 'rc': 1, + 'stdout': 'some stuff here', + 'stderr': '', + } + assertThrowRegex('Failed to set file mode on remote files') + + # Otherwise if we make it here in this state, we hit the catch-all + action_base.get_shell_option.side_effect = get_shell_option_for_arg( + {}, + None) + assertThrowRegex('on the temporary files Ansible needs to create') + def test_action_base__remove_tmp_path(self): # create our fake task mock_task = MagicMock()