From: JINMEI Tatuya Date: Mon, 31 Dec 2012 21:14:44 +0000 (-0800) Subject: [711] made BoB.uid/gid "private", and raise a separate exception if setXX fails X-Git-Tag: bind10-1.0.0-rc-release~99^2~22^2~4 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b0732ce0447db537b3d643735f16aad09ddd4c7c;p=thirdparty%2Fkea.git [711] made BoB.uid/gid "private", and raise a separate exception if setXX fails --- diff --git a/src/bin/bind10/bind10_src.py.in b/src/bin/bind10/bind10_src.py.in index a40c315a40..edf9b9067d 100755 --- a/src/bin/bind10/bind10_src.py.in +++ b/src/bin/bind10/bind10_src.py.in @@ -105,6 +105,16 @@ _BASETIME = time.gmtime() class ProcessInfoError(Exception): pass +class ChangeUserError(Exception): + '''Exception raised when setuid/setgid fails. + + When raised, it's expected to be propagated via underlying component + management modules to the top level so that it will help provide useful + fatal error message. + + ''' + pass + class ProcessInfo: """Information about a process""" @@ -206,8 +216,8 @@ class BoB: # restart. Components manage their own restart schedule now self.components_to_restart = [] self.runnable = False - self.uid = setuid - self.gid = setgid + self.__uid = setuid + self.__gid = setgid self.username = username self.verbose = verbose self.nokill = nokill @@ -277,12 +287,22 @@ class BoB: (i.e., once all components that need the privilege of the original user have started). ''' - if self.gid is not None: - logger.info(BIND10_SETGID, self.gid) - posix.setgid(self.gid) - if self.uid is not None: - logger.info(BIND10_SETUID, self.uid) - posix.setuid(self.uid) + try: + if self.__gid is not None: + logger.info(BIND10_SETGID, self.__gid) + posix.setgid(self.__gid) + except Exception as ex: + raise ChangeUserError('failed to change group: ' + str(ex)) + + try: + if self.__uid is not None: + posix.setuid(self.__uid) + # We use one-shot logger after setuid here. This will + # detect any permission issue regarding logging due to the + # result of setuid at the earliest opportunity. + isc.log.Logger("boss").info(BIND10_SETUID, self.__uid) + except Exception as ex: + raise ChangeUserError('failed to change user: ' + str(ex)) def config_handler(self, new_config): # If this is initial update, don't do anything now, leave it to startup @@ -593,7 +613,7 @@ class BoB: are pure speculation. As with the auth daemon, they should be read from the configuration database. """ - if self.uid is not None and self.__started: + if self.__uid is not None and self.__started: logger.warn(BIND10_START_AS_NON_ROOT_RESOLVER) self.curproc = "b10-resolver" # XXX: this must be read from the configuration manager in the future diff --git a/src/bin/bind10/tests/bind10_test.py.in b/src/bin/bind10/tests/bind10_test.py.in index ead517834c..dcb2d419af 100644 --- a/src/bin/bind10/tests/bind10_test.py.in +++ b/src/bin/bind10/tests/bind10_test.py.in @@ -342,12 +342,16 @@ class TestCacheCommands(unittest.TestCase): class TestBoB(unittest.TestCase): def setUp(self): + # Save original values that may be tweaked in some tests self.__orig_setgid = bind10_src.posix.setgid self.__orig_setuid = bind10_src.posix.setuid + self.__orig_logger_class = isc.log.Logger def tearDown(self): + # Restore original values saved in setUp() bind10_src.posix.setgid = self.__orig_setgid bind10_src.posix.setuid = self.__orig_setuid + isc.log.Logger = self.__orig_logger_class def test_init(self): bob = BoB() @@ -357,7 +361,6 @@ class TestBoB(unittest.TestCase): self.assertEqual(bob.ccs, None) self.assertEqual(bob.components, {}) self.assertEqual(bob.runnable, False) - self.assertEqual(bob.uid, None) self.assertEqual(bob.username, None) self.assertIsNone(bob._socket_cache) @@ -379,13 +382,35 @@ class TestBoB(unittest.TestCase): self.assertIsNone(self.__gid_set) self.assertIsNone(self.__uid_set) - bob.gid = 4200 - bob.uid = 42 - bob.change_user() + BoB(setuid=42, setgid=4200).change_user() # This time, it get's called self.assertEqual(4200, self.__gid_set) self.assertEqual(42, self.__uid_set) + def raising_set_xid(gid_or_uid): + ex = OSError() + ex.errno, ex.strerror = errno.EPERM, 'Operation not permitted' + raise ex + + # Let setgid raise an exception + bind10_src.posix.setgid = raising_set_xid + bind10_src.posix.setuid = self.__setuid + self.assertRaises(bind10_src.ChangeUserError, + BoB(setuid=42, setgid=4200).change_user) + + # Let setuid raise an exception + bind10_src.posix.setgid = self.__setgid + bind10_src.posix.setuid = raising_set_xid + self.assertRaises(bind10_src.ChangeUserError, + BoB(setuid=42, setgid=4200).change_user) + + # Let initial log output after setuid raise an exception + bind10_src.posix.setgid = self.__setgid + bind10_src.posix.setuid = self.__setuid + isc.log.Logger = raising_set_xid + self.assertRaises(bind10_src.ChangeUserError, + BoB(setuid=42, setgid=4200).change_user) + def test_set_creator(self): """ Test the call to set_creator. First time, the cache is created @@ -456,7 +481,6 @@ class TestBoB(unittest.TestCase): self.assertEqual(bob.ccs, None) self.assertEqual(bob.components, {}) self.assertEqual(bob.runnable, False) - self.assertEqual(bob.uid, None) self.assertEqual(bob.username, None) def test_command_handler(self):