From: Brian Coca Date: Mon, 28 Oct 2024 12:47:08 -0400 Subject: CVE-2024-9902 user module avoid chmoding symlink'd home file (#83956) (#84081) also added tests origin: https://github.com/ansible/ansible/commit/3b5a4319985e1eabe7fc0410bf8308b671f4f586 bug: https://github.com/ansible/ansible/pull/84081 bug-debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1086883 --- changelogs/fragments/user_action_fix.yml | 2 + lib/ansible/modules/system/user.py | 4 +- .../targets/user/files/skel/.ssh/known_hosts | 1 + .../user/tasks/test_create_user_home.yml | 86 +++++++++++++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/user_action_fix.yml create mode 100644 test/integration/targets/user/files/skel/.ssh/known_hosts diff --git a/changelogs/fragments/user_action_fix.yml b/changelogs/fragments/user_action_fix.yml new file mode 100644 index 00000000..64ee997d --- /dev/null +++ b/changelogs/fragments/user_action_fix.yml @@ -0,0 +1,2 @@ +bugfixes: + - user module now avoids changing ownership of files symlinked in provided home dir skeleton diff --git a/lib/ansible/modules/system/user.py b/lib/ansible/modules/system/user.py index fd56fc68..9d47425f 100644 --- a/lib/ansible/modules/system/user.py +++ b/lib/ansible/modules/system/user.py @@ -1154,7 +1154,9 @@ class User(object): for d in dirs: os.chown(os.path.join(root, d), uid, gid) for f in files: - os.chown(os.path.join(root, f), uid, gid) + full_path = os.path.join(root, f) + if not os.path.islink(full_path): + os.chown(full_path, uid, gid) except OSError as e: self.module.exit_json(failed=True, msg="%s" % to_native(e)) diff --git a/test/integration/targets/user/files/skel/.ssh/known_hosts b/test/integration/targets/user/files/skel/.ssh/known_hosts new file mode 100644 index 00000000..f5b72a21 --- /dev/null +++ b/test/integration/targets/user/files/skel/.ssh/known_hosts @@ -0,0 +1 @@ +test file, not real ssh hosts file diff --git a/test/integration/targets/user/tasks/test_create_user_home.yml b/test/integration/targets/user/tasks/test_create_user_home.yml index 1b529f76..3bc1023b 100644 --- a/test/integration/targets/user/tasks/test_create_user_home.yml +++ b/test/integration/targets/user/tasks/test_create_user_home.yml @@ -134,3 +134,89 @@ name: randomuser state: absent remove: yes + +- name: Create user home directory with /dev/null as skeleton, https://github.com/ansible/ansible/issues/75063 + # create_homedir is mostly used by linux, rest of OSs take care of it themselves via -k option (which fails this task) + # OS X actuall breaks since it does not implement getpwnam() + when: ansible_system == 'Linux' + block: + - name: "Create user home directory with /dev/null as skeleton" + user: + name: withskeleton + state: present + skeleton: "/dev/null" + createhome: yes + register: create_user_with_skeleton_dev_null + always: + - name: "Remove test user" + user: + name: withskeleton + state: absent + remove: yes + +- name: Create user home directory with skel that contains symlinks + tags: symlink_home + when: ansible_system == 'Linux' + become: True + vars: + flag: '{{tempdir.path}}/root_flag.conf' + block: + - name: make tempdir for skel + tempfile: state=directory + register: tempdir + + - name: create flag file + file: path={{flag}} owner=root state=touch + + - name: copy skell to target + copy: + dest: '{{tempdir.path}}/skel' + src: files/skel + register: skel + + - name: create the bad symlink + file: + src: '{{flag}}' + dest: '{{tempdir.path}}/skel/should_not_change_own' + state: link + + - name: "Create user home directory with skeleton" + user: + name: withskeleton + state: present + skeleton: "{{tempdir.path}}/skel" + createhome: yes + home: /home/missing/withskeleton + register: create_user_with_skeleton_symlink + + - name: Check flag + stat: path={{flag}} + register: test_flag + + - name: ensure we didn't change owner for flag + assert: + that: + - test_flag.stat.uid != create_user_with_skeleton_symlink.uid + + always: + - name: "Remove test user" + user: + name: withskeleton + state: absent + remove: yes + + - name: get files to delete + find: path="{{tempdir.path}}" + register: remove + when: + - tempdir is defined + - tempdir is success + + - name: "Remove temp files" + file: + path: '{{item}}' + state: absent + loop: "{{remove.files|default([])}}" + when: + - remove is success + -- 2.47.0