Allow macOS ACLs to work for unpriv -> unpriv (#70785)

Change:
- Use `chmod +a` in the fallback chain to allow MacOS to use ACLs to
  allow an unprivileged user to become an unprivileged user.

Test Plan:
- CI, new tests

Tickets:
- Fixes #70648

Signed-off-by: Rick Elrod <rick@elrod.me>

changelogs/fragments/macos-chmod-acl.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - When connecting as an unprivileged user, and becoming an unprivileged user, we now fall back to also trying ``chmod +a`` which works on macOS and makes use of ACLs.

docs/docsite/rst/user_guide/become.rst

@@ -147,7 +147,10 @@ Next, if POSIX ACLs are **not** available or :command:`setfacl` could not be
 run, Ansible will attempt to change ownership of the module file using
 :command:`chown` for systems which support doing so as an unprivileged user.
 
-New in Ansible 2.10, if the :command:`chown` fails, Ansible will then check the
+New in Ansible 2.11, at this point, Ansible will try :command:`chmod +a` which
+is a macOS-specific way of setting ACLs on files.
+
+New in Ansible 2.10, if all of the above fails, Ansible will then check the
 value of the configuration setting ``ansible_common_remote_group``. Many
 systems will allow a given user to change the group ownership of a file to a
 group the user is in. As a result, if the second unprivileged user (the

lib/ansible/plugins/action/__init__.py

@@ -518,7 +518,9 @@ class ActionBase(with_metaclass(ABCMeta, object)):
           file with chown which only works in case the remote_user is
           privileged or the remote systems allows chown calls by unprivileged
           users (e.g. HP-UX)
-        * If the chown fails, we check if ansible_common_remote_group is set.
+        * If the above fails, we next try 'chmod +a' which is a macOS way of
+          setting ACLs on files.
+        * If the above fails, we check if ansible_common_remote_group is set.
           If it is, we attempt to chgrp the file to its value. This is useful
           if the remote_user has a group in common with the become_user. As the
           remote_user, we can chgrp the file to that group and allow the
@@ -571,12 +573,16 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         if execute:
             chmod_mode = 'rx'
             setfacl_mode = 'r-x'
+            # Apple patches their "file_cmds" chmod with ACL support
+            chmod_acl_mode = '{0} allow read,execute'.format(become_user)
         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'
+            # Apple
+            chmod_acl_mode = '{0} allow read'.format(become_user)
 
         # Step 3a: Are we able to use setfacl to add user ACLs to the file?
         res = self._remote_set_user_facl(
@@ -613,7 +619,15 @@ class ActionBase(with_metaclass(ABCMeta, object)):
                 'Unprivileged become user would be unable to read the '
                 'file.')
 
-        # Step 3d: Common group
+        # Step 3d: Try macOS's special chmod + ACL
+        # macOS chmod's +a flag takes its own argument. As a slight hack, we
+        # pass that argument as the first element of remote_paths. So we end
+        # up running `chmod +a [that argument] [file 1] [file 2] ...`
+        res = self._remote_chmod([chmod_acl_mode] + remote_paths, '+a')
+        if res['rc'] == 0:
+            return remote_paths
+
+        # Step 3e: 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.

test/integration/targets/become_unprivileged/aliases

@@ -2,3 +2,4 @@ destructive
 shippable/posix/group1
 skip/aix
 needs/ssh
+needs/root

test/integration/targets/become_unprivileged/chmod_acl_macos/test.yml

@@ -0,0 +1,26 @@
+- name: Tests for chmod +a ACL functionality on macOS
+  hosts: ssh
+  gather_facts: yes
+  remote_user: unpriv1
+  become: yes
+  become_user: unpriv2
+
+  tasks:
+    - name: Get AnsiballZ temp directory
+      action: tmpdir
+      register: tmpdir
+      become_user: unpriv2
+      become: yes
+
+    - name: run whoami
+      command: whoami
+      register: whoami
+
+    - name: Ensure we used the right fallback
+      shell: ls -le /var/tmp/ansible*/*_command.py
+      register: ls
+
+    - assert:
+        that:
+          - whoami.stdout == "unpriv2"
+          - "'user:unpriv2 allow read' in ls.stdout"

test/integration/targets/become_unprivileged/runme.sh

@@ -2,6 +2,11 @@
 
 set -eux
 
+export ANSIBLE_KEEP_REMOTE_FILES=True
+ANSIBLE_ACTION_PLUGINS="$(pwd)/action_plugins"
+export ANSIBLE_ACTION_PLUGINS
+export ANSIBLE_BECOME_PASS='iWishIWereCoolEnoughForRoot!'
+
 begin_sandwich() {
     ansible-playbook setup_unpriv_users.yml -i inventory -v "$@"
 }
@@ -24,12 +29,24 @@ end_sandwich() {
 trap "end_sandwich \"\$@\"" EXIT
 
 # Common group tests
-begin_sandwich "$@"
+# Skip on macOS, chmod fallback will take over.
-  ansible-playbook common_remote_group/setup.yml -i inventory -v "$@"
+# 1) chmod is stupidly hard to disable, so hitting this test case on macOS would
-  export ANSIBLE_KEEP_REMOTE_FILES=True
+#    be a suuuuuuper edge case scenario
-  export ANSIBLE_COMMON_REMOTE_GROUP=commongroup
+# 2) even if we can trick it so chmod doesn't exist, then other things break.
-  export ANSIBLE_BECOME_PASS='iWishIWereCoolEnoughForRoot!'
+#    Ansible wants a `chmod` around, even if it's not the final thing that gets
-  ANSIBLE_ACTION_PLUGINS="$(pwd)/action_plugins"
+#    us enough permission to run the task.
-  export ANSIBLE_ACTION_PLUGINS
+if [[ "$OSTYPE" != darwin* ]]; then
-  ansible-playbook common_remote_group/test.yml -i inventory -v "$@"
+  begin_sandwich "$@"
-end_sandwich "$@"
+    ansible-playbook common_remote_group/setup.yml -i inventory -v "$@"
+    export ANSIBLE_COMMON_REMOTE_GROUP=commongroup
+    ansible-playbook common_remote_group/test.yml -i inventory -v "$@"
+  end_sandwich "$@"
+fi
+
+if [[ "$OSTYPE" == darwin* ]]; then
+  begin_sandwich "$@"
+    # In the default case this should happen on macOS, so no need for a setup
+    # It should just work.
+    ansible-playbook chmod_acl_macos/test.yml -i inventory -v "$@"
+  end_sandwich "$@"
+fi

test/units/plugins/action/test_action.py

@@ -358,6 +358,9 @@ class TestActionBase(unittest.TestCase):
                 return args_kv.get(option, default)
             return _helper
 
+        action_base.get_become_option = MagicMock()
+        action_base.get_become_option.return_value = 'remoteuser2'
+
         # Step 1: On windows, we just return remote_paths
         action_base._connection._shell._IS_WINDOWS = True
         assertSuccess(execute=False)
@@ -412,12 +415,6 @@ class TestActionBase(unittest.TestCase):
             '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',
@@ -426,6 +423,12 @@ class TestActionBase(unittest.TestCase):
         assertThrowRegex(
             'Failed to set file mode on remote temporary file',
             execute=True)
+        action_base._remote_chmod.return_value = {
+            'rc': 0,
+            'stdout': 'some stuff here',
+            'stderr': '',
+        }
+        assertSuccess(execute=True)
 
         # Step 3c: chown
         action_base._remote_chown = MagicMock()
@@ -446,7 +449,18 @@ class TestActionBase(unittest.TestCase):
         assertThrowRegex('user would be unable to read the file.')
         remote_user = 'remoteuser1'
 
-        # Step 3d: Common group
+        # Step 3d: chmod +a on osx
+        assertSuccess()
+        action_base._remote_chmod.assert_called_with(
+            ['remoteuser2 allow read'] + remote_paths,
+            '+a')
+
+        # Step 3e: Common group
+        action_base._remote_chmod = MagicMock()
+        action_base._remote_chmod.side_effect = \
+            lambda x, y: \
+            dict(rc=1, stdout='', stderr='') if y == '+a' \
+            else dict(rc=0, stdout='', stderr='')
 
         get_shell_option = action_base.get_shell_option
         action_base.get_shell_option = MagicMock()
@@ -463,11 +477,6 @@ class TestActionBase(unittest.TestCase):
         }
         # 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,
@@ -486,6 +495,7 @@ class TestActionBase(unittest.TestCase):
             'stderr': '',
         }
         assertSuccess()
+        action_base._remote_chmod = MagicMock()
         action_base._remote_chmod.return_value = {
             'rc': 1,
             'stdout': 'some stuff here',