Francesco Romani has uploaded a new change for review.

Change subject: mkimage: do not create world-readable image
......................................................................

mkimage: do not create world-readable image

mkimage generates world-readable images by default.
mkimage spawns a genisoimage process through the cpopen package
to do the actual work.
It is of course possible to fix the permissions once the ISO image
is created with a trivial os.chmod() call, but this will leave
open a window opportunity to still exploit the bug.
A more correct and secure way to fix the permissions of the newly
generated image is to setup the umask just before to exec.

The current python-cpopen package lacks a way to set the umask
of the child before to run it, so this patch depends on an enhanced
python-cpopen, temporarily located here:

https://github.com/mojaves/python-cpopen

With this enhanced cpopen, fixing the permissions is trivially made
by passing the correct umask at the ISO image creation.

Change-Id: I893a1310d9988c52cec9f48dfd17dfa1647da4dc
Bug-Url: https://bugzilla.redhat.com/1034172
Signed-off-by: Francesco Romani <from...@redhat.com>
---
M lib/vdsm/utils.py
M vdsm/mkimage.py
2 files changed, 4 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/21946/1

diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index ebd36b8..8173916 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -501,7 +501,7 @@
 def execCmd(command, sudo=False, cwd=None, data=None, raw=False, logErr=True,
             printable=None, env=None, sync=True, nice=None, ioclass=None,
             ioclassdata=None, setsid=False, execCmdLogger=logging.root,
-            deathSignal=0):
+            deathSignal=0, childUmask=None):
     """
     Executes an external command, optionally via sudo.
 
@@ -531,7 +531,7 @@
     execCmdLogger.debug("%s (cwd %s)", cmdline, cwd)
 
     p = BetterPopen(command, close_fds=True, cwd=cwd, env=env,
-                    deathSignal=deathSignal)
+                    deathSignal=deathSignal, childUmask=childUmask)
     p = AsyncProc(p)
     if not sync:
         if data is not None:
diff --git a/vdsm/mkimage.py b/vdsm/mkimage.py
index a4716c7..4daefec 100644
--- a/vdsm/mkimage.py
+++ b/vdsm/mkimage.py
@@ -117,7 +117,8 @@
         if volumeName is not None:
             command.extend(['-V', volumeName])
         command.extend([dirname])
-        rc, out, err = storage.misc.execCmd(command, raw=True)
+        rc, out, err = storage.misc.execCmd(command, raw=True,
+                                            childUmask=0o007)
         if rc:
             raise OSError(errno.EIO, "could not create iso file: "
                           "code %s, out %s\nerr %s" % (rc, out, err))


-- 
To view, visit http://gerrit.ovirt.org/21946
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I893a1310d9988c52cec9f48dfd17dfa1647da4dc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <from...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to