From 75e73778dce1cb7a2816a936240ef75adfbd6ed9 Mon Sep 17 00:00:00 2001

From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

Date: Thu, 16 Jul 2020 17:21:28 +0530

Subject: [PATCH] rtslib: safely call shutil.copy()

Previously we had to replace shutil.copyfile() with shutil.copy(),

because we want to copy the file permissions to the destination file

along with the data.

It appears that shutil.copy() is opening the destination file with

wide access (0666) first, and then it starts copying the data and

at the end it is copying the permissions from source file to destination.

If we closely notice there appears a window between destination file

is opened vs permissions are set on the destination file, which could

allow a user to get the contents of the file when opening it at the

right time.

The behavior is a bit unsteady here, it is noticed that, when

saveconfig.json file exists, then on shutil.copy(), destination file is

opened and a mask 0600 is applied on the file, in case shutil.copy() had

to open a new destination saveconfig.json file, then mask 0644 is applied.

Thanks and Credits to 'Stefan Cornelius <scorneli@redhat.com>' for

reporting this, here is the strace he shared from RHEL-7/python-2.7.5

env:

Case 1: When /etc/target/saveconfig.json doesn't exist:

open("/etc/target/saveconfig.json.temp", O_RDONLY) = 3

open("/etc/target/saveconfig.json", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 4

fstat(3, {st_mode=S_IFREG|0600, st_size=71, ...}) = 0

fstat(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0

[...]

chmod("/etc/target/saveconfig.json", 0600) = 0}")")}")

Case 2: When /etc/target/saveconfig.json already exist:

open("/etc/target/saveconfig.json.temp", O_RDONLY) = 3

open("/etc/target/saveconfig.json", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 4

fstat(3, {st_mode=S_IFREG|0600, st_size=71, ...}) = 0

fstat(4, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0

[...]

chmod("/etc/target/saveconfig.json", 0600) = 0}")")}")

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

---

 rtslib/root.py | 5 +++--

 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/rtslib/root.py b/rtslib/root.py

index 2c5cf43..4ecc03c 100644

--- a/rtslib/root.py

+++ b/rtslib/root.py

@@ -476,8 +476,8 @@ class RTSRoot(CFSNode):

         # prevent the file from being created if it exists due to a race

         try:

             fdesc = os.open(tmp_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, mode)

-        finally:

-            os.umask(umask_original)

+        except OSError:

+            raise ExecutionError("Could not open %s" % tmp_file)

         with os.fdopen(fdesc, 'w') as f:

             f.write(json.dumps(saveconf, sort_keys=True, indent=2))

@@ -488,6 +488,7 @@ class RTSRoot(CFSNode):

         # copy along with permissions

         shutil.copy(tmp_file, save_file)

+        os.umask(umask_original)

         os.remove(tmp_file)

     def restore_from_file(self, restore_file=None, clear_existing=True,

-- 

2.26.2