From 9b97774028b1a6fb9f3d388ec761af5882cfce4f Mon Sep 17 00:00:00 2001
From: Guido Trotter <ultrotter@google.com>
Date: Thu, 12 Feb 2009 09:15:52 +0000
Subject: [PATCH] Apply the right permissions to /etc/hosts
In the current Ganeti version when modifying /etc/hosts we mistakenly
give it the permissions of the temporary file we create to define its
content, which is by default 0600. This breaks most non-root
applications, and thus must be corrected. This patch forces the mode to
be 0644 (but we might decide to just use the mode of the previous
/etc/hosts, if we want to be more polite against any eventual
administrative choice). We also add a new assertFileMode() method for
unit tests and actually check in the SetEtcHostsEntry and
RemoveEtcHostsEntry tests that the mode is correct, to be sure not to
reintroduce this bug again. Also, a FIXME is added in the original
functions stating that it would be nice to use WriteFile+fn() rather
than reimplementing its functionality again.
Reviewed-by: iustinp
---
lib/utils.py | 4 ++++
test/ganeti.utils_unittest.py | 7 +++++++
test/testutils.py | 16 +++++++++++++++-
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/lib/utils.py b/lib/utils.py
index f88730385..8bd1000b5 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -895,6 +895,7 @@ def SetEtcHostsEntry(file_name, ip, hostname, aliases):
@param aliases: the list of aliases to add for the hostname
"""
+ # FIXME: use WriteFile + fn rather than duplicating its efforts
# Ensure aliases are unique
aliases = UniqueSequence([hostname] + aliases)[1:]
@@ -917,6 +918,7 @@ def SetEtcHostsEntry(file_name, ip, hostname, aliases):
out.flush()
os.fsync(out)
+ os.chmod(tmpname, 0644)
os.rename(tmpname, file_name)
finally:
f.close()
@@ -950,6 +952,7 @@ def RemoveEtcHostsEntry(file_name, hostname):
@param hostname: the hostname to be removed
"""
+ # FIXME: use WriteFile + fn rather than duplicating its efforts
fd, tmpname = tempfile.mkstemp(dir=os.path.dirname(file_name))
try:
out = os.fdopen(fd, 'w')
@@ -971,6 +974,7 @@ def RemoveEtcHostsEntry(file_name, hostname):
out.flush()
os.fsync(out)
+ os.chmod(tmpname, 0644)
os.rename(tmpname, file_name)
finally:
f.close()
diff --git a/test/ganeti.utils_unittest.py b/test/ganeti.utils_unittest.py
index 1050002e6..1c2992c3d 100755
--- a/test/ganeti.utils_unittest.py
+++ b/test/ganeti.utils_unittest.py
@@ -527,6 +527,7 @@ class TestEtcHosts(testutils.GanetiTestCase):
"127.0.0.1\tlocalhost\n"
"192.168.1.1 router gw\n"
"1.2.3.4\tmyhost.domain.tld myhost\n")
+ self.assertFileMode(self.tmpname, 0644)
def testSettingExistingIp(self):
SetEtcHostsEntry(self.tmpname, '192.168.1.1', 'myhost.domain.tld',
@@ -536,6 +537,7 @@ class TestEtcHosts(testutils.GanetiTestCase):
"# This is a test file for /etc/hosts\n"
"127.0.0.1\tlocalhost\n"
"192.168.1.1\tmyhost.domain.tld myhost\n")
+ self.assertFileMode(self.tmpname, 0644)
def testSettingDuplicateName(self):
SetEtcHostsEntry(self.tmpname, '1.2.3.4', 'myhost', ['myhost'])
@@ -545,6 +547,7 @@ class TestEtcHosts(testutils.GanetiTestCase):
"127.0.0.1\tlocalhost\n"
"192.168.1.1 router gw\n"
"1.2.3.4\tmyhost\n")
+ self.assertFileMode(self.tmpname, 0644)
def testRemovingExistingHost(self):
RemoveEtcHostsEntry(self.tmpname, 'router')
@@ -553,6 +556,7 @@ class TestEtcHosts(testutils.GanetiTestCase):
"# This is a test file for /etc/hosts\n"
"127.0.0.1\tlocalhost\n"
"192.168.1.1 gw\n")
+ self.assertFileMode(self.tmpname, 0644)
def testRemovingSingleExistingHost(self):
RemoveEtcHostsEntry(self.tmpname, 'localhost')
@@ -560,6 +564,7 @@ class TestEtcHosts(testutils.GanetiTestCase):
self.assertFileContent(self.tmpname,
"# This is a test file for /etc/hosts\n"
"192.168.1.1 router gw\n")
+ self.assertFileMode(self.tmpname, 0644)
def testRemovingNonExistingHost(self):
RemoveEtcHostsEntry(self.tmpname, 'myhost')
@@ -568,6 +573,7 @@ class TestEtcHosts(testutils.GanetiTestCase):
"# This is a test file for /etc/hosts\n"
"127.0.0.1\tlocalhost\n"
"192.168.1.1 router gw\n")
+ self.assertFileMode(self.tmpname, 0644)
def testRemovingAlias(self):
RemoveEtcHostsEntry(self.tmpname, 'gw')
@@ -576,6 +582,7 @@ class TestEtcHosts(testutils.GanetiTestCase):
"# This is a test file for /etc/hosts\n"
"127.0.0.1\tlocalhost\n"
"192.168.1.1 router\n")
+ self.assertFileMode(self.tmpname, 0644)
class TestShellQuoting(unittest.TestCase):
diff --git a/test/testutils.py b/test/testutils.py
index f2980ddc9..c0932cc06 100644
--- a/test/testutils.py
+++ b/test/testutils.py
@@ -22,6 +22,7 @@
"""Utilities for unit testing"""
import os
+import stat
import tempfile
import unittest
@@ -46,7 +47,7 @@ class GanetiTestCase(unittest.TestCase):
pass
def assertFileContent(self, file_name, expected_content):
- """Checks the content of a file is what we expect.
+ """Checks that the content of a file is what we expect.
@type file_name: str
@param file_name: the file whose contents we should check
@@ -57,6 +58,19 @@ class GanetiTestCase(unittest.TestCase):
actual_content = utils.ReadFile(file_name)
self.assertEqual(actual_content, expected_content)
+ def assertFileMode(self, file_name, expected_mode):
+ """Checks that the mode of a file is what we expect.
+
+ @type file_name: str
+ @param file_name: the file whose contents we should check
+ @type expected_mode: int
+ @param expected_mode: the mode we expect
+
+ """
+ st = os.stat(file_name)
+ actual_mode = stat.S_IMODE(st.st_mode)
+ self.assertEqual(actual_mode, expected_mode)
+
@staticmethod
def _TestDataFilename(name):
"""Returns the filename of a given test data file.
--
GitLab