From 8439d1813e23e036623de0040a6832256e1e54c6 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 4 Jan 2017 12:31:29 -0800 Subject: [PATCH] Fix for atomic_move on RHEL5 When becoming an unprivileged user using non-sudo on a platform where getlogin() failed in our situation we were not able to detect that the user had switched. This meant that all of our logic to use move vs copy if the user had switched was attempting the wrong thing. This change tries the to do the right thing but then falls back to an acceptable second choice if it doesn't work. The bug wasn't easily detected because: * sudo was not affected because sudo records that the user's have been switched so we were able to detect that. * getlogin() works on most platforms. RHEL5 with python-2.4 seems to be the only platform we still care about where getlogin() fails for this case. * It had to be becoming an unprivileged user. When becoming a privileged user, the user would be able to successfully perform the best case tasks. (cherry picked from commit 02e3f4b52685f9ccb8f45bd4fc78b9fcc04dd577) --- lib/ansible/module_utils/basic.py | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index a2e7296b5c0..0171fefafcc 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1926,18 +1926,6 @@ class AnsibleModule(object): creating = not os.path.exists(b_dest) - try: - login_name = os.getlogin() - except OSError: - # not having a tty can cause the above to fail, so - # just get the LOGNAME environment variable instead - login_name = os.environ.get('LOGNAME', None) - - # if the original login_name doesn't match the currently - # logged-in user, or if the SUDO_USER environment variable - # is set, then this user has switched their credentials - switched_user = login_name and login_name != pwd.getpwuid(os.geteuid())[0] or os.environ.get('SUDO_USER') - try: # Optimistically try a rename, solves some corner cases and can avoid useless work, throws exception if not atomic. os.rename(b_src, b_dest) @@ -1976,12 +1964,13 @@ class AnsibleModule(object): # close tmp file handle before file operations to prevent text file busy errors on vboxfs synced folders (windows host) os.close(tmp_dest_fd) # leaves tmp file behind when sudo and not root - if switched_user and os.geteuid() != 0: + try: + shutil.move(b_src, b_tmp_dest_name) + except OSError: # cleanup will happen by 'rm' of tempdir # copy2 will preserve some metadata shutil.copy2(b_src, b_tmp_dest_name) - else: - shutil.move(b_src, b_tmp_dest_name) + if self.selinux_enabled(): self.set_context_if_different( b_tmp_dest_name, context, False) @@ -2013,8 +2002,12 @@ class AnsibleModule(object): umask = os.umask(0) os.umask(umask) os.chmod(b_dest, DEFAULT_PERM & ~umask) - if switched_user: + try: os.chown(b_dest, os.geteuid(), os.getegid()) + except OSError: + # We're okay with trying our best here. If the user is not + # root (or old Unices) they won't be able to chown. + pass if self.selinux_enabled(): # rename might not preserve context