From: JINMEI Tatuya Date: Mon, 31 Dec 2012 19:01:39 +0000 (-0800) Subject: [711] moved setg/uid code to a new Bob.change_user method. X-Git-Tag: bind10-1.0.0-rc-release~99^2~22^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1c2b85d7d1c1448d78b8f2a9927b7952ef42e39d;p=thirdparty%2Fkea.git [711] moved setg/uid code to a new Bob.change_user method. The SockCreator class now just calls it, unconditionally, rather than inspecting gid/uid attributes of Bob. at a higher level this is basically an internal refactoring: no externally visible behavior is changed. And, it's a matter of taste in some sense, but since we are going to do a bit more work for this task, and it seemed too intrusive to refer to attributes like gid/uid directly, I chose to introduce this change. --- diff --git a/src/bin/bind10/bind10_src.py.in b/src/bin/bind10/bind10_src.py.in index 882653b3ba..a40c315a40 100755 --- a/src/bin/bind10/bind10_src.py.in +++ b/src/bin/bind10/bind10_src.py.in @@ -269,6 +269,21 @@ class BoB: # Update the configuration self._component_configurator.reconfigure(comps) + def change_user(self): + '''Change the user and group to those specified on construction. + + This method is expected to be called by a component on initial + startup when the system is ready to switch the user and group + (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) + def config_handler(self, new_config): # If this is initial update, don't do anything now, leave it to startup if not self.runnable: diff --git a/src/bin/bind10/tests/bind10_test.py.in b/src/bin/bind10/tests/bind10_test.py.in index 409790c810..ead517834c 100644 --- a/src/bin/bind10/tests/bind10_test.py.in +++ b/src/bin/bind10/tests/bind10_test.py.in @@ -341,6 +341,14 @@ class TestCacheCommands(unittest.TestCase): class TestBoB(unittest.TestCase): + def setUp(self): + self.__orig_setgid = bind10_src.posix.setgid + self.__orig_setuid = bind10_src.posix.setuid + + def tearDown(self): + bind10_src.posix.setgid = self.__orig_setgid + bind10_src.posix.setuid = self.__orig_setuid + def test_init(self): bob = BoB() self.assertEqual(bob.verbose, False) @@ -353,6 +361,31 @@ class TestBoB(unittest.TestCase): self.assertEqual(bob.username, None) self.assertIsNone(bob._socket_cache) + def __setgid(self, gid): + self.__gid_set = gid + + def __setuid(self, uid): + self.__uid_set = uid + + def test_change_user(self): + bind10_src.posix.setgid = self.__setgid + bind10_src.posix.setuid = self.__setuid + + self.__gid_set = None + self.__uid_set = None + bob = BoB() + bob.change_user() + # No gid/uid set in boss, nothing called. + self.assertIsNone(self.__gid_set) + self.assertIsNone(self.__uid_set) + + bob.gid = 4200 + bob.uid = 42 + bob.change_user() + # This time, it get's called + self.assertEqual(4200, self.__gid_set) + self.assertEqual(42, self.__uid_set) + def test_set_creator(self): """ Test the call to set_creator. First time, the cache is created diff --git a/src/lib/python/isc/bind10/special_component.py b/src/lib/python/isc/bind10/special_component.py index 688ccf5f85..dcd9b64bbe 100644 --- a/src/lib/python/isc/bind10/special_component.py +++ b/src/lib/python/isc/bind10/special_component.py @@ -17,11 +17,7 @@ from isc.bind10.component import Component, BaseComponent import isc.bind10.sockcreator from bind10_config import LIBEXECPATH import os -import posix import isc.log -from isc.log_messages.bind10_messages import * - -logger = isc.log.Logger("boss") class SockCreator(BaseComponent): """ @@ -36,8 +32,6 @@ class SockCreator(BaseComponent): def __init__(self, process, boss, kind, address=None, params=None): BaseComponent.__init__(self, boss, kind) self.__creator = None - self.__uid = boss.uid - self.__gid = boss.gid def _start_internal(self): self._boss.curproc = 'b10-sockcreator' @@ -46,12 +40,9 @@ class SockCreator(BaseComponent): self._boss.register_process(self.pid(), self) self._boss.set_creator(self.__creator) self._boss.log_started(self.pid()) - 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) + + # We are now ready for switching user. + self._boss.change_user() def _stop_internal(self): self.__creator.terminate() diff --git a/src/lib/python/isc/bind10/tests/component_test.py b/src/lib/python/isc/bind10/tests/component_test.py index 18efea7451..8603201e87 100644 --- a/src/lib/python/isc/bind10/tests/component_test.py +++ b/src/lib/python/isc/bind10/tests/component_test.py @@ -104,10 +104,10 @@ class ComponentTests(BossUtils, unittest.TestCase): self.__stop_process_params = None self.__start_simple_params = None # Pretending to be boss - self.gid = None - self.__gid_set = None - self.uid = None - self.__uid_set = None + self.__change_user_called = False + + def change_user(self): + self.__change_user_called = True # just record the fact it's called def __start(self): """ @@ -624,12 +624,6 @@ class ComponentTests(BossUtils, unittest.TestCase): self.assertTrue(process.killed) self.assertFalse(process.terminated) - def setgid(self, gid): - self.__gid_set = gid - - def setuid(self, uid): - self.__uid_set = uid - class FakeCreator: def pid(self): return 42 @@ -655,35 +649,19 @@ class ComponentTests(BossUtils, unittest.TestCase): """ component = isc.bind10.special_component.SockCreator(None, self, 'needed', None) - orig_setgid = isc.bind10.special_component.posix.setgid - orig_setuid = isc.bind10.special_component.posix.setuid - isc.bind10.special_component.posix.setgid = self.setgid - isc.bind10.special_component.posix.setuid = self.setuid orig_creator = \ isc.bind10.special_component.isc.bind10.sockcreator.Creator # Just ignore the creator call isc.bind10.special_component.isc.bind10.sockcreator.Creator = \ lambda path: self.FakeCreator() component.start() - # No gid/uid set in boss, nothing called. - self.assertIsNone(self.__gid_set) - self.assertIsNone(self.__uid_set) + self.assertTrue(self.__change_user_called) # Doesn't do anything, but doesn't crash component.stop() component.kill() component.kill(True) - self.gid = 4200 - self.uid = 42 component = isc.bind10.special_component.SockCreator(None, self, 'needed', None) - component.start() - # This time, it get's called - self.assertEqual(4200, self.__gid_set) - self.assertEqual(42, self.__uid_set) - isc.bind10.special_component.posix.setgid = orig_setgid - isc.bind10.special_component.posix.setuid = orig_setuid - isc.bind10.special_component.isc.bind10.sockcreator.Creator = \ - orig_creator class TestComponent(BaseComponent): """