[svn] commit: r2326 - in /branches/trac180/src/bin/bind10: bind10.py.in tests/args_test.py

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Jun 29 17:42:23 UTC 2010


Author: shane
Date: Tue Jun 29 17:42:22 2010
New Revision: 2326

Log:
Changes to fix issues raised in review of ticket #180 (drop 
priviliges in the boss process).


Modified:
    branches/trac180/src/bin/bind10/bind10.py.in
    branches/trac180/src/bin/bind10/tests/args_test.py

Modified: branches/trac180/src/bin/bind10/bind10.py.in
==============================================================================
--- branches/trac180/src/bin/bind10/bind10.py.in (original)
+++ branches/trac180/src/bin/bind10/bind10.py.in Tue Jun 29 17:42:22 2010
@@ -111,13 +111,15 @@
             when = time.time()
         return max(when, self.restart_time)
 
+class ProcessInfoError(Exception): pass
+
 class ProcessInfo:
     """Information about a process"""
 
     dev_null = open(os.devnull, "w")
 
     def __init__(self, name, args, env={}, dev_null_stdout=False,
-                 dev_null_stderr=False, uid=None):
+                 dev_null_stderr=False, uid=None, username=None):
         self.name = name 
         self.args = args
         self.env = env
@@ -125,13 +127,22 @@
         self.dev_null_stderr = dev_null_stderr
         self.restart_schedule = RestartSchedule()
         self.uid = uid
+        self.username = username
         self._spawn()
 
     def _setuid(self):
-        if not self.uid is None:
-            sys.stderr.write("setting uid\n")
-            posix.setuid(self.uid)
-            sys.stderr.write("set it\n")
+        """Function used before running a program that needs to run as a
+        different user."""
+        if self.uid is not None:
+            try:
+                posix.setuid(self.uid)
+            except OSError as e:
+                if e.errno == errno.EPERM:
+                    # if we failed to change user due to permission report that
+                    raise ProcessInfoError("Unable to change to user %s (uid %d)" % (self.username, self.uid))
+                else:
+                    # otherwise simply re-raise whatever error we found
+                    raise
 
     def _spawn(self):
         if self.dev_null_stdout:
@@ -142,12 +153,14 @@
             spawn_stderr = self.dev_null
         else:
             spawn_stderr = None
+#        spawn_stdout = None
+#        spawn_stderr = None
         # Environment variables for the child process will be a copy of those
         # of the boss process with any additional specific variables given
         # on construction (self.env).
         spawn_env = os.environ
         spawn_env.update(self.env)
-        if not 'B10_FROM_SOURCE' in os.environ:
+        if 'B10_FROM_SOURCE' not in os.environ:
             spawn_env['PATH'] = "@@LIBEXECDIR@@:" + spawn_env['PATH']
         self.process = subprocess.Popen(self.args,
                                         stdin=subprocess.PIPE,
@@ -166,7 +179,7 @@
     """Boss of BIND class."""
     
     def __init__(self, msgq_socket_file=None, auth_port=5300, verbose=False,
-                 setuid=None):
+                 setuid=None, username=None):
         """Initialize the Boss of BIND. This is a singleton (only one
         can run).
         
@@ -183,6 +196,7 @@
         self.dead_processes = {}
         self.runnable = False
         self.uid = setuid
+        self.username = username
 
     def config_handler(self, new_config):
         if self.verbose:
@@ -237,7 +251,8 @@
                 sys.stdout.write("[bind10] Starting b10-msgq\n")
         try:
             c_channel = ProcessInfo("b10-msgq", ["b10-msgq"], c_channel_env,
-                                    True, not self.verbose, uid=self.uid)
+                                    True, not self.verbose, uid=self.uid,
+                                    username=self.username)
         except Exception as e:
             return "Unable to start b10-msgq; " + str(e)
         self.processes[c_channel.pid] = c_channel
@@ -263,7 +278,8 @@
             sys.stdout.write("[bind10] Starting b10-cfgmgr\n")
         try:
             bind_cfgd = ProcessInfo("b10-cfgmgr", ["b10-cfgmgr"],
-                                    c_channel_env, uid=self.uid)
+                                    c_channel_env, uid=self.uid,
+                                    username=self.username)
         except Exception as e:
             c_channel.process.kill()
             return "Unable to start b10-cfgmgr; " + str(e)
@@ -305,7 +321,7 @@
             sys.stdout.write("[bind10] Started b10-auth (PID %d)\n" % auth.pid)
 
         # everything after the authoritative server can run as non-root
-        if not self.uid is None:
+        if self.uid is not None:
             posix.setuid(self.uid)
 
         # start the xfrout before auth-server, to make sure every xfr-query can
@@ -455,7 +471,12 @@
                 sys.stdout.write("[bind10] Unknown child pid %d exited.\n" % pid)
 
     def restart_processes(self):
-        """Restart any dead processes."""
+        """Restart any dead processes.
+        Returns the time when the next process is ready to be restarted. 
+          If the server is shutting down, returns 0.
+          If there are no processes, returns None.
+        The values returned can be safely passed into select() as the 
+        timeout value."""
         next_restart = None
         # if we're shutting down, then don't restart
         if not self.runnable:
@@ -554,18 +575,26 @@
 
     # Check user ID.
     setuid = None
+    username = None
     if options.user:
+        # Try getting information about the user, assuming UID passed.
         try:
             pw_ent = pwd.getpwuid(int(options.user))
             setuid = pw_ent.pw_uid
+            username = pw_ent.pw_name
         except ValueError:
             pass
         except KeyError:
             pass
 
+        # Next try getting information about the user, assuming user name 
+        # passed.
+        # If the information is both a valid user name and user number, we
+        # prefer the name because we try it second. A minor point, hopefully.
         try:
             pw_ent = pwd.getpwnam(options.user)
             setuid = pw_ent.pw_uid
+            username = pw_ent.pw_name
         except KeyError:
             pass
 
@@ -594,7 +623,7 @@
 
     # Go bob!
     boss_of_bind = BoB(options.msgq_socket_file, int(options.auth_port),
-                       options.verbose, setuid)
+                       options.verbose, setuid, username)
     startup_result = boss_of_bind.startup()
     if startup_result:
         sys.stderr.write("[bind10] Error on startup: %s\n" % startup_result)

Modified: branches/trac180/src/bin/bind10/tests/args_test.py
==============================================================================
--- branches/trac180/src/bin/bind10/tests/args_test.py (original)
+++ branches/trac180/src/bin/bind10/tests/args_test.py Tue Jun 29 17:42:22 2010
@@ -8,6 +8,7 @@
 import subprocess
 import select
 import time
+import pwd
 
 # Set to a valid user name on the system to run setuid() test
 #SUID_USER=None
@@ -32,6 +33,7 @@
         return found_string
 
     def testNoArgs(self):
+        """Run bind10 without any arguments"""
         bob = subprocess.Popen(args=(BIND10_EXE,),
                                stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
         started_ok = self._waitForString(bob, '[bind10] BIND 10 started')
@@ -41,6 +43,7 @@
         self.assertTrue(started_ok)
 
     def testBadOption(self):
+        """Run bind10 with a bogus option"""
         bob = subprocess.Popen(args=(BIND10_EXE, "--badoption"),
                                stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
         failed = self._waitForString(bob, 'bind10: error: no such option: --badoption')
@@ -50,6 +53,7 @@
         self.assertTrue(failed)
 
     def testArgument(self):
+        """Run bind10 with an argument (this is not allowed)"""
         bob = subprocess.Popen(args=(BIND10_EXE, "argument"),
                                stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
         failed = self._waitForString(bob, 'Usage: bind10 [options]')
@@ -59,6 +63,7 @@
         self.assertTrue(failed)
 
     def testBadUser(self):
+        """Run bind10 with a bogus user"""
         bob = subprocess.Popen(args=(BIND10_EXE, "-u", "bogus_user"),
                                stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
         failed = self._waitForString(bob, "bind10: invalid user: 'bogus_user'")
@@ -68,6 +73,7 @@
         self.assertTrue(failed)
 
     def testBadUid(self):
+        """Run bind10 with a bogus user ID"""
         bob = subprocess.Popen(args=(BIND10_EXE, "-u", "999999999"),
                                stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
         failed = self._waitForString(bob, "bind10: invalid user: '999999999'")
@@ -76,7 +82,24 @@
         self.assertTrue(bob.wait() == 1)
         self.assertTrue(failed)
 
+    def testFailSetUser(self):
+        """Try the -u option when we don't run as root"""
+        global SUID_USER
+        if SUID_USER is None:
+            self.skipTest("test needs a valid user (set when run)")
+        if os.getuid() == 0:
+            self.skipTest("test must not be run as root (uid is 0)")
+        # XXX: we depend on the "nobody" user
+        bob = subprocess.Popen(args=(BIND10_EXE, "-u", "nobody"),
+                               stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+        failed = self._waitForString(bob, "[bind10] Error on startup: Unable to start b10-msgq; Unable to change to user nobody")
+        time.sleep(0.1)
+        bob.terminate()
+        self.assertTrue(bob.wait() == 1)
+        self.assertTrue(failed)
+
     def testSetUser(self):
+        """Try the -u option"""
         global SUID_USER
         if SUID_USER is None:
             self.skipTest("test needs a valid user (set when run)")




More information about the bind10-changes mailing list