From f4053fcf3ac47cd5eae9314f582d12df7e3298ed Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Thu, 14 Aug 2014 16:39:53 +0200 Subject: [PATCH 1/2] Only chown on atomic move if invoked as root Linux and BSD derivatives do not allow unprivileged users to "give away" files to others for security reasons. (System V derivatives allow that but they're rare nowadays.) --- lib/ansible/module_utils/basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 3a541d67d65..0537dfd9bf3 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1146,7 +1146,7 @@ class AnsibleModule(object): self.set_context_if_different( tmp_dest.name, context, False) tmp_stat = os.stat(tmp_dest.name) - if dest_stat and (tmp_stat.st_uid != dest_stat.st_uid or tmp_stat.st_gid != dest_stat.st_gid): + if dest_stat and (tmp_stat.st_uid != dest_stat.st_uid or tmp_stat.st_gid != dest_stat.st_gid) and os.getuid() == 0: os.chown(tmp_dest.name, dest_stat.st_uid, dest_stat.st_gid) os.rename(tmp_dest.name, dest) except (shutil.Error, OSError, IOError), e: From 677de07f44d733a60fdea825d0dc8eb8fb23bcff Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Sun, 17 Aug 2014 22:08:45 +0200 Subject: [PATCH 2/2] Try optimistic chown, do not error out if not permitted to chown Add unit test --- lib/ansible/module_utils/basic.py | 10 +++++--- .../roles/test_copy/tasks/main.yml | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 0537dfd9bf3..c942c22ce15 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1145,9 +1145,13 @@ class AnsibleModule(object): if self.selinux_enabled(): self.set_context_if_different( tmp_dest.name, context, False) - tmp_stat = os.stat(tmp_dest.name) - if dest_stat and (tmp_stat.st_uid != dest_stat.st_uid or tmp_stat.st_gid != dest_stat.st_gid) and os.getuid() == 0: - os.chown(tmp_dest.name, dest_stat.st_uid, dest_stat.st_gid) + try: + tmp_stat = os.stat(tmp_dest.name) + if dest_stat and (tmp_stat.st_uid != dest_stat.st_uid or tmp_stat.st_gid != dest_stat.st_gid): + os.chown(tmp_dest.name, dest_stat.st_uid, dest_stat.st_gid) + except OSError, e: + if e.errno != errno.EPERM: + raise os.rename(tmp_dest.name, dest) except (shutil.Error, OSError, IOError), e: self.cleanup(tmp_dest.name) diff --git a/test/integration/roles/test_copy/tasks/main.yml b/test/integration/roles/test_copy/tasks/main.yml index e168bead9b1..1ddf1c1ba2b 100644 --- a/test/integration/roles/test_copy/tasks/main.yml +++ b/test/integration/roles/test_copy/tasks/main.yml @@ -180,3 +180,28 @@ - "copy_result6.changed" - "copy_result6.dest == '{{output_dir|expanduser}}/multiline.txt'" - "copy_result6.md5sum == '1627d51e7e607c92cf1a502bf0c6cce3'" + +# test overwriting a file as an unprivileged user (pull request #8624) +# this can't be relative to {{output_dir}} as ~root usually has mode 700 + +- name: create world writable directory + file: dest=/tmp/worldwritable state=directory mode=0777 + +- name: create world writable file + copy: dest=/tmp/worldwritable/file.txt content="bar" mode=0666 + +- name: overwrite the file as user nobody + copy: dest=/tmp/worldwritable/file.txt content="baz" + sudo: yes + sudo_user: nobody + register: copy_result7 + +- name: assert the file was overwritten + assert: + that: + - "copy_result7.changed" + - "copy_result7.dest == '/tmp/worldwritable/file.txt'" + - "copy_result7.md5sum == '73feffa4b7f6bb68e44cf984c85f6e88'" + +- name: clean up + file: dest=/tmp/worldwritable state=absent