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 fromani@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))
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5868/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5072/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5960/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5909/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5113/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6001/ : FAILURE
Yaniv Bronhaim has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 2: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 2: Code-Review-1
please add the required cpopen version to the vdsm.spec.in and debian/control files.
Yaniv Bronhaim has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 3: Verified-1
(1 comment)
.................................................... File vdsm.spec.in Line 100: Requires: iproute Line 101: Requires: python-netaddr Line 102: Requires: python-inotify Line 103: Requires: python-argparse Line 104: Requires: python-cpopen >= 1.2.4-1 there is no such version. we need to build it first Line 105: Requires: python-ethtool >= 0.6-3 Line 106: Requires: %{name}-python-zombiereaper = %{version}-%{release} Line 107: Requires: rpm-python Line 108: Requires: nfs-utils
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6050/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5263/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6153/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/128/ : FAILURE
Yaniv Bronhaim has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 4:
(1 comment)
.................................................... File vdsm.spec.in Line 100: Requires: iproute Line 101: Requires: python-netaddr Line 102: Requires: python-inotify Line 103: Requires: python-argparse Line 104: Requires: python-cpopen >= 1.2.3-5 it is not available yet.. please update with verification when it does Line 105: Requires: python-ethtool >= 0.6-3 Line 106: Requires: %{name}-python-zombiereaper = %{version}-%{release} Line 107: Requires: rpm-python Line 108: Requires: nfs-utils
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 4:
(1 comment)
.................................................... File vdsm.spec.in Line 100: Requires: iproute Line 101: Requires: python-netaddr Line 102: Requires: python-inotify Line 103: Requires: python-argparse Line 104: Requires: python-cpopen >= 1.2.3-5 Will do. Line 105: Requires: python-ethtool >= 0.6-3 Line 106: Requires: %{name}-python-zombiereaper = %{version}-%{release} Line 107: Requires: rpm-python Line 108: Requires: nfs-utils
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 5: Verified-1
Verified against a running VDSM instance. Marking as -1 because the package isn't yet in Fedora.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 5: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6233/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5446/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6339/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/176/ : FAILURE
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 5:
bodhi feedback: the cpopen update can be pushed to stable.
Yaniv Bronhaim has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 5:
(1 comment)
cpopen 1.2.3-5 is now available . please verify
.................................................... File debian/control Line 92: open-iscsi (>= 2.0.873), Line 93: policycoreutils (>= 2.1.10), Line 94: psmisc (>= 22.6), Line 95: python (>= 2.7.3), Line 96: python-cpopen (>= 1.2.3), so 1.2.3-5 Line 97: python-dmidecode, Line 98: python-ethtool (>= 0.8), Line 99: python-ethtool (>= 0.8), Line 100: python-libvirt (>= 1.0.2),
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 5:
(1 comment)
.................................................... File debian/control Line 92: open-iscsi (>= 2.0.873), Line 93: policycoreutils (>= 2.1.10), Line 94: psmisc (>= 22.6), Line 95: python (>= 2.7.3), Line 96: python-cpopen (>= 1.2.3), Yes.
I was unsure if debian packaging allows to specify the revision in the dependency (http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version)
Looks like it is possible, altough discouraged: http://lintian.debian.org/tags/build-depends-on-1-revision.html
I'll update the dependency accordingly. Line 97: python-dmidecode, Line 98: python-ethtool (>= 0.8), Line 99: python-ethtool (>= 0.8), Line 100: python-libvirt (>= 1.0.2),
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6: Verified+1
Changes:
* fixed debian/control dependency
verified both unit-tests and clound-init on
* EL6.5 * FC19
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6448/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/185/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5555/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6361/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
I tested using the packages manually downloaded and then yum localinstall-ed because, due to repository configuration, vdsm-python-cpopen still has greater priority than python-cpopen on my boxes.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6448/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5555/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6361/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/186/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6448/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5555/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6361/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/187/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6448/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5555/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6361/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/188/ : FAILURE
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
The jenkins boxes still have an old python-cpopen or maybe vdsm-python-cpopen I guess
Yaniv Bronhaim has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
it shouldn't. please speak with eedri and fix the jenkins errors
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6719/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/248/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6632/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5827/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6720/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/249/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6633/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5828/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Jenkins box still not updated fully. python-cpopen needs to move from updates-testing to updates; I already gave karma (of course after another bit of testing) to it.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5929/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/267/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6822/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6702/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests/19/ : FAILURE
Yaniv Bronhaim has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
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.
PatchSet v2: the cpopen patch has been merged upstream:
https://github.com/ficoos/cpopen/commits/master
PatchSet v3: * updated vdsm.spec.in and debian.control to require an updated cpopen.
PatchSet v4: * the minimum required cpopen version is 1.2.3-5
PatchSet v5: * rebased against master * make iso image not group-writable * added explicit permission tests (umask tests alread committed in the cpopen package)
Change-Id: I893a1310d9988c52cec9f48dfd17dfa1647da4dc Bug-Url: https://bugzilla.redhat.com/1034172 Signed-off-by: Francesco Romani fromani@redhat.com Reviewed-on: http://gerrit.ovirt.org/21946 Reviewed-by: Yaniv Bronhaim ybronhei@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M debian/control M lib/vdsm/utils.py M tests/mkimageTests.py M vdsm.spec.in M vdsm/mkimage.py 5 files changed, 17 insertions(+), 5 deletions(-)
Approvals: Yaniv Bronhaim: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified
oVirt Jenkins CI Server has posted comments on this change.
Change subject: mkimage: do not create world-readable image ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/277/ : SUCCESS
vdsm-patches@lists.fedorahosted.org