Commit 9b977740 authored by Guido Trotter's avatar Guido Trotter
Browse files

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
parent 1dff8e07
......@@ -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()
......
......@@ -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):
......
......@@ -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.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment