[master 2/5] Fix some bad uses of chmod.

dashea installerbot-noreply at redhat.com
Tue Jul 14 21:56:34 UTC 2015


From: David Shea <dshea at redhat.com>

When using permission bits to restrict access to the contents of a file,
the permission bits must be set when the file is created, otherwise,
depending on the current umask, there may be a period where any user can
open the file, and the contents will be readable as long as this file
descriptor is open.

Move setting permission bits to the calls to open().
---
 pyanaconda/bootloader.py | 10 +++-------
 pyanaconda/install.py    |  9 ++-------
 pyanaconda/iutil.py      |  3 +--
 pyanaconda/users.py      |  5 ++---
 utils/dd/rpmutils.c      | 21 ++++++++++++++-------
 5 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py
index 65eea56..2aac710 100644
--- a/pyanaconda/bootloader.py
+++ b/pyanaconda/bootloader.py
@@ -953,10 +953,7 @@ def write_config_images(self, config):
         raise NotImplementedError()
 
     def write_config_post(self):
-        try:
-            iutil.eintr_retry_call(os.chmod, iutil.getSysroot() + self.config_file, self.config_file_mode)
-        except OSError as e:
-            log.error("failed to set config file permissions: %s", e)
+        pass
 
     def write_config(self):
         """ Write the bootloader configuration. """
@@ -967,7 +964,7 @@ def write_config(self):
         if os.access(config_path, os.R_OK):
             os.rename(config_path, config_path + ".anacbak")
 
-        config = open(config_path, "w")
+        config = iutil.open_with_perm(config_path, "w", self.config_file_mode)
         self.write_config_header(config)
         self.write_config_images(config)
         config.close()
@@ -1559,7 +1556,7 @@ def write_password_config(self):
             return
 
         users_file = iutil.getSysroot() + "/etc/grub.d/01_users"
-        header = open(users_file, "w")
+        header = iutil.open_with_perm(users_file, "w", 0o700)
         header.write("#!/bin/sh -e\n\n")
         header.write("cat << \"EOF\"\n")
         # XXX FIXME: document somewhere that the username is "root"
@@ -1570,7 +1567,6 @@ def write_password_config(self):
         header.write("%s\n" % password_line)
         header.write("EOF\n")
         header.close()
-        iutil.eintr_retry_call(os.chmod, users_file, 0o700)
 
     def write_config(self):
         self.write_config_console(None)
diff --git a/pyanaconda/install.py b/pyanaconda/install.py
index 305bb32..43655c9 100644
--- a/pyanaconda/install.py
+++ b/pyanaconda/install.py
@@ -28,7 +28,6 @@
 from pyanaconda.users import createLuserConf, getPassAlgo, Users
 from pyanaconda import flags
 from pyanaconda import iutil
-from pyanaconda.iutil import open   # pylint: disable=redefined-builtin
 from pyanaconda import timezone
 from pyanaconda import network
 from pyanaconda.i18n import _
@@ -40,8 +39,6 @@
 log = logging.getLogger("anaconda")
 
 def _writeKS(ksdata):
-    import os
-
     path = iutil.getSysroot() + "/root/anaconda-ks.cfg"
 
     # Clear out certain sensitive information that kickstart doesn't have a
@@ -50,11 +47,9 @@ def _writeKS(ksdata):
                ksdata.partition.dataList() + ksdata.raid.dataList():
         obj.passphrase = ""
 
-    with open(path, "w") as f:
-        f.write(str(ksdata))
-
     # Make it so only root can read - could have passwords
-    iutil.eintr_retry_call(os.chmod, path, 0o600)
+    with iutil.open_with_perm(path, "w", 0o600) as f:
+        f.write(str(ksdata))
 
 def doConfiguration(storage, payload, ksdata, instClass):
     willWriteNetwork = not flags.flags.imageInstall and not flags.flags.dirInstall
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index 858c9b1..7a261a1 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -795,7 +795,7 @@ def dracut_eject(device):
     try:
         if not os.path.exists(DRACUT_SHUTDOWN_EJECT):
             mkdirChain(os.path.dirname(DRACUT_SHUTDOWN_EJECT))
-            f = open(DRACUT_SHUTDOWN_EJECT, "w")
+            f = open_with_perm(DRACUT_SHUTDOWN_EJECT, "w", 0o755)
             f.write("#!/bin/sh\n")
             f.write("# Created by Anaconda\n")
         else:
@@ -803,7 +803,6 @@ def dracut_eject(device):
 
         f.write("eject %s\n" % (device,))
         f.close()
-        eintr_retry_call(os.chmod, DRACUT_SHUTDOWN_EJECT, 0o755)
         log.info("Wrote dracut shutdown eject hook for %s", device)
     except (IOError, OSError) as e:
         log.error("Error writing dracut shutdown eject hook for %s: %s", device, e)
diff --git a/pyanaconda/users.py b/pyanaconda/users.py
index ce8e718..5eee4fc 100644
--- a/pyanaconda/users.py
+++ b/pyanaconda/users.py
@@ -452,12 +452,11 @@ def setUserSshKey(self, username, key, **kwargs):
 
             authfile = os.path.join(sshdir, "authorized_keys")
             authfile_existed = os.path.exists(authfile)
-            with open(authfile, "a") as f:
+            with iutil.open_with_perm(authfile, "a", 0o600) as f:
                 f.write(key + "\n")
 
-            # Only change mode and ownership if we created it
+            # Only change ownership if we created it
             if not authfile_existed:
-                iutil.eintr_retry_call(os.chmod, authfile, 0o600)
                 iutil.eintr_retry_call(os.chown, authfile, user.get(libuser.UIDNUMBER)[0], user.get(libuser.GIDNUMBER)[0])
                 iutil.execWithRedirect("restorecon", ["-r", sshdir])
             os._exit(0)
diff --git a/utils/dd/rpmutils.c b/utils/dd/rpmutils.c
index 9f06790..fbbde06 100644
--- a/utils/dd/rpmutils.c
+++ b/utils/dd/rpmutils.c
@@ -28,6 +28,11 @@
 #include <stdio.h>
 #include <string.h>
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
 #include <rpm/rpmlib.h>		/* rpmReadPackageFile .. */
 #include <rpm/rpmtag.h>
 #include <rpm/rpmio.h>
@@ -391,20 +396,22 @@ int explodeDDRPM(const char *source,
 
         /* Regular file */
         if (towrite>=2) {
-            FILE *fdout = fopen(filename+offset, "w");
+            int fd = open(filename+offset, O_WRONLY, fstat->st_mode);
 
-            if (fdout==NULL){
+            if (fd==-1){
                 rc = 33;
                 break;
             }
 
-            rc = archive_read_data_into_fd(cpio, fileno(fdout));
-            if (rc==ARCHIVE_OK) {
-                /* set permissions on the new file */
-                chmod(filename+offset, fstat->st_mode);
+            /* call chmod to set any bits that were removed by umask during open */
+            if (fchmod(fd, fstat->st_mode) != 0) {
+                rc = 33;
+                break;
             }
 
-            fclose(fdout);
+            rc = archive_read_data_into_fd(cpio, fd);
+
+            close(fd);
         }
 
         /* symlink, we assume that the path contained in symlink


-- 
To view this commit on github, visit https://github.com/rhinstaller/anaconda/commit/b7a19349918bfa4b9f1fa9ee0c959d85809d5221


More information about the anaconda-patches mailing list