Francesco Romani has uploaded a new change for review.
Change subject: mkimage: create files with explicit permissions
......................................................................
mkimage: create files with explicit permissions
Files in the ISO image, most notably `user_data` and `meta_data.json`
files on the config-drive have world readable permissions.
They contain sensitive data, so it is unsafe to do so.
This patch address the issue by adding the capability to set
per-file permissions at creation time in
mkimage._decodeFilesIntoDir
current default is 0o640, so we just avoid to create any world-readable
file at all.
Another approach to obtain exactly the same result could be to
leverate the -file-mode option of the `genisoimage` program.
But doing into the python code has the following advantages:
* creates the correct permissions right from the start, effectively
eliding the vulnerability window; if we let genisoimage fix things,
there is a (very short, indeed) time window on which the newly created
files are still world-readable.
* this paves the road to fine-grained, per-file permissions
with negligibile extra cost (in short, this is more forward-compatible).
Bug-Url:
https://bugzilla.redhat.com/1034247
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
Change-Id: Id1a3baca2ec1bf2dd0762641633a749573c40147
---
M tests/mkimageTests.py
M vdsm/mkimage.py
2 files changed, 29 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/21956/1
diff --git a/tests/mkimageTests.py b/tests/mkimageTests.py
index 28b35bb..ed6df6a 100644
--- a/tests/mkimageTests.py
+++ b/tests/mkimageTests.py
@@ -28,6 +28,7 @@
from base64 import b64encode
import os
+import stat
from shutil import rmtree
from tempfile import mkdtemp
@@ -88,6 +89,17 @@
#pylint: disable=W0212
mkimage._P_PAYLOAD_IMAGES = self.orig_mkimage["_P_PAYLOAD_IMAGES"]
+ def _check_permissions(self, filepath, permsMask):
+ """
+ Ensure the file at `filepath' has the permissions coherent
+ with the given mask.
+ The mask may specifiy the required presence, or absence, of a
+ permission bit.
+ """
+ data = os.stat(filepath)
+ for perm, expected in permsMask:
+ self.assertEqual(bool(data.st_mode & perm), expected)
+
def _check_content(self):
"""
Ensure that the workdir contains what we want
@@ -101,7 +113,12 @@
self.assertTrue(filename in out_dir)
else:
self.assertTrue(os.path.basename(filename) in out_subdir)
- with open(os.path.join(self.workdir, filename), "r") as fd:
+ filepath = os.path.join(self.workdir, filename)
+ self._check_permissions(filepath,
+ ((stat.S_IROTH, False),
+ (stat.S_IWOTH, False),
+ (stat.S_IXOHT, False))
+ with open(filepath, "r") as fd:
content = fd.read()
self.assertEqual(content, self.expected_results[filename])
diff --git a/vdsm/mkimage.py b/vdsm/mkimage.py
index a4716c7..ca8585e 100644
--- a/vdsm/mkimage.py
+++ b/vdsm/mkimage.py
@@ -35,6 +35,16 @@
_P_PAYLOAD_IMAGES = os.path.join(P_VDSM_RUN, 'payload')
+def _openFile(filename, mode, perms=0o644):
+ '''
+ opens a filename allowing to specify the unix permissions
+ right from the start, to avoid world-readable files
+ with sensitive informations.
+ '''
+ fd = os.open(filename, os.O_CREAT|os.O_TRUNC|os.O_RDWR, perms)
+ return os.fdopen(fd, mode)
+
+
def _decodeFilesIntoDir(files, parentdir):
'''
create temp files from files list
@@ -55,7 +65,7 @@
except OSError as e:
if e.errno != os.errno.EEXIST:
raise
- with file(filename, 'w') as f:
+ with _openFile(filename, 'w', perms=0o640) as f:
f.write(base64.b64decode(content))
--
To view, visit
http://gerrit.ovirt.org/21956
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id1a3baca2ec1bf2dd0762641633a749573c40147
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>