[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 882653b3ba39ec14127d8b21edd390dd64751fe5..a40c315a40c22f339f2c0ac4cb7400c33d5fd3ba 100755 (executable)
--- 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 409790c8100d21d86ed4ba5ee54aabaadbac6c2b..ead517834c191b4480aaa0ca3041645db6e4a48a 100644 (file)
--- 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 688ccf5f85771065311a2a0fe4d8b034edb88501..dcd9b64bbe15791a85bdf45df1346843d4f25cc0 100644 (file)
--- 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 18efea745191c72ca82ca606455d4cec843ab9b4..8603201e871494121cd3dc5635828c298f4ba8d2 100644 (file)
--- 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):
     """