[Server-devel] [PATCH] More checking and logging of registration information.

Douglas Bagnall douglas at paradise.net.nz
Mon Aug 11 19:28:07 EDT 2008


    
    The create_user script tries to give useful information to syslogd, and
    ensures that the username it is given is a valid XO serial.  It also checks
    the ssh public key, but is not terribly strict.  If a system user is created
    but some later process fails, create_user tries to remove the user.
    
    server.py also takes better notice the values it receives, and will fail if
    the username or public key contain multiple lines. The way errors are
    handled has also been changed, to reduce dupication.
    
    If the server.py script is called without a pidfile argument, it runs in the
    foreground and prints messages on stderr. Previously it would have crashed.

diff --git a/conf.schoolserver/create_user b/conf.schoolserver/create_user
index 90d9315..4a6f90f 100755
--- a/conf.schoolserver/create_user
+++ b/conf.schoolserver/create_user
@@ -21,22 +21,33 @@
 # This script creates a user account for the registration server
 #
 # echo entering as `whoami` > /tmp/create_user.log
+LOG_LEVEL=user.warning
+LOG_TAG=olpc_idmgr
 
 if [ `whoami` != "root" ]; then
-        # echo execing as root >> /tmp/create_user.log
-        exec sudo -S $0 >> /tmp/create_user.log 2>&1
+        exec sudo -S $0
 fi
 
+log() {
+    echo "$1" | logger -t $LOG_TAG -s -p $LOG_LEVEL
+}
+
 die() {
-    echo "$1" 1>&2
+    log "$1"
     exit 1
 }
 
 read username
 read full_name
-read uuid
+read uuid       #unused!
 read pubkey
 
+# check for sane values
+export LC_ALL=C
+echo "$username" | grep -s -E '^[A-Z]{3}[A-F0-9]{8}$' &> /dev/null || die "bad
username"
+echo "$pubkey" | grep -s -E '^[A-Za-z0-9+/=]+$' &> /dev/null || die "bad public
key"
+
+
 homedir=/library/users/$username
 XO_USERS_GROUP=xousers
 
@@ -46,16 +57,34 @@ getent group $XO_USERS_GROUP > /dev/null 2>&1 || groupadd
$XO_USERS_GROUP
 if getent passwd "$username" > /dev/null 2>&1; then
     # $fullname may have changed.
     /usr/sbin/usermod -c "$full_name" "$username" || die "unable to change full
name"
-else 
+    NEW_USER=0
+else
     /usr/sbin/useradd -c "$full_name" -d "$homedir"  \
         -G $XO_USERS_GROUP -s /usr/bin/rssh "$username" \
         || die "Unable to create user"
+    NEW_USER=1
 fi
 
+#from here, if a new user was created, a failure will leave the user
+#there but unconfigured. So rather than simply dying, we try to clean
+#up first.
+
+clean_up_and_die(){
+    log "$1"
+    if [ $NEW_USER == 1 ]; then
+        log "deleting half-created user"
+        /usr/sbin/userdel "$username" || log "... failed!"
+    fi
+    exit 1
+}
+
+
 userhome=`getent passwd "$username" | awk -F: '{print $6}'`
-cd $userhome
+cd $userhome || clean_up_and_die "Couldn't cd into user's home directory"
+
+mkdir -p --mode=700 .ssh || clean_up_and_die "Unable to mkdir .ssh"
+echo "ssh-dss $pubkey" >> .ssh/authorized_keys || clean_up_and_die "Unable to
set up authorized_keys"
+chmod 600 .ssh/authorized_keys  || clean_up_and_die "Unable to chmod
authorized_keys"
+chown -R $username .ssh || clean_up_and_die "Unable to chown .ssh"
 
-mkdir -p --mode=700 .ssh || die "Unable to mkdir .ssh"
-echo "ssh-dss $pubkey" >> .ssh/authorized_keys || die "Unable to set up
authorized_keys"
-chmod 600 .ssh/authorized_keys  || die "Unable to chmod authorized_keys"
-chown -R $username .ssh || die "Unable to chown .ssh"
+#clean_up_and_die goodbye
\ No newline at end of file
diff --git a/idmgr/server.py b/idmgr/server.py
index 06b9bcb..9a4c164 100755
--- a/idmgr/server.py
+++ b/idmgr/server.py
@@ -42,11 +42,16 @@ IDMGR_CONFIG_FILE = '/etc/idmgr.conf'
 # Default maximum for the number of available file descriptors.
 MAXFD = 1024
 
+# Port on which to listen
+
+PORT = 8080
+
 # The specs are a little unclear on the encoding of UUIDs, so be
 # flexible in what we accept
 
 uuidre = re.compile(r'[a-fA-F0-9-]{32,40}')
 serialre   = re.compile(r'^[A-Z]{3}[A-F0-9]{8}$')
+base64re   = re.compile(r'^[A-Fa-z0-9+/=]+$')
 
 # The standard I/O file descriptors are redirected to /dev/null by default.
 if (hasattr(os, "devnull")):
@@ -69,12 +74,12 @@ def createDaemon( pidfilename ):
         # unless we stop executing due to failure.
         # import signal           # Set handlers for asynchronous events.
         # signal.signal(signal.SIGHUP, signal.SIG_IGN)
-        
+
         try:
             pid = os.fork()# Fork a second child.
         except OSError, e:
             raise Exception, "%s [%d]" % (e.strerror, e.errno)
-        
+
         if (pid == 0):   # The second child.
             # Since the current working directory may be a mounted filesystem,
             # we avoid the issue of not being able to unmount the filesystem at
@@ -87,10 +92,10 @@ def createDaemon( pidfilename ):
             try:
                 pidfile = open( pidfilename, "w" );
                 pidfile.write( str( pid ) );
-                pidfile.write( "\n" );            
+                pidfile.write( "\n" );
                 pidfile.close();
             except OSError, e:
-                syslog.openlog( 'olpc_idmgr', 0, syslog.LOG_DAEMON )
+                syslog.openlog( 'olpc_idmgr', 0, syslog.LOG_USER )
                 syslog.syslog( syslog.LOG_ALERT, "Writing PID file: %s [%d]" %
(e.strerror, e.errno) )
                 syslog.closelog()
                 raise Exception, \
@@ -98,7 +103,7 @@ def createDaemon( pidfilename ):
             os._exit(0)  # Exit parent (the first child) of the second child.
     else:
         os._exit(0)# Exit parent of the first child.
-        
+
     # Close all open file descriptors.
     # Use the getrlimit method to retrieve the maximum file descriptor
     # number that can be opened by this process.  If there is not limit
@@ -108,38 +113,45 @@ def createDaemon( pidfilename ):
     maxfd = resource.getrlimit(resource.RLIMIT_NOFILE)[1]
     if (maxfd == resource.RLIM_INFINITY):
         maxfd = MAXFD
-        
+
     # Iterate through and close all file descriptors.
     for fd in range(0, maxfd):
         try:
             os.close(fd)
         except OSError: # ERROR, fd wasn't open to begin with (ignored)
             pass
-        
+
     # Redirect the standard I/O file descriptors to the specified file.  Since
     # the daemon has no controlling terminal, most daemons redirect stdin,
     # stdout, and stderr to /dev/null.  This is done to prevent side-effects
     # from reads and writes to the standard I/O file descriptors.
-    
+
     # This call to open is guaranteed to return the lowest file descriptor,
     # which will be 0 (stdin), since it was closed above.
     os.open(REDIRECT_TO, os.O_RDWR)    # standard input (0)
-    
+
     # Duplicate standard input to standard output and standard error.
     os.dup2(0, 1)# standard output (1)
     os.dup2(0, 2)# standard error (2)
     return(0)
 
+class ServerError(Exception):
+    pass
+
 class RegisterController(object):
     def register(self, serial, nickname, uuid, pubkey):
         response = {}
         try:
+            #Sanitise all input
             if not serialre.match(serial):
-                syslog.syslog(syslog.LOG_WARNING, "Invalid serial: %s" % (serial,))
-                raise Exception("Invalid serial: %s" % (serial,))
+                raise ServerError("Invalid serial: %s" % (serial,))
             if not uuidre.match(uuid):
-                syslog.syslog(syslog.LOG_WARNING, "Invalid UUID: %s" % (uuid,))
-                raise Exception( "Invalid UUID: %s" % (uuid,))
+                raise ServerError( "Invalid UUID: %s" % (uuid,))
+            if "\n" in nickname:
+                raise ServerError( "Invalid nickname: %s" % nickname)
+            if "\n" in pubkey:
+                raise ServerError("Invalid public key: %s" % pubkey)
+
             # Check for an existing laptop entry
             laptop = model.Laptop.get(serial)
             if laptop is None:
@@ -152,12 +164,13 @@ class RegisterController(object):
             laptop.save_or_update()
             laptop.flush()
         except Exception, e:
+            log(syslog.LOG_ERR, str(e))
             response['success'] = 'ERR'
             response['error'] = str(e)
         else:
-            syslog.syslog( syslog.LOG_NOTICE,
-                          "Registered user %s with serial %s"
-                          % (nickname.encode('utf-8'), serial))
+            log( syslog.LOG_NOTICE,
+                 "Registered user %s with serial %s"
+                 % (nickname.encode('utf-8'), serial))
             #  Make a guess at reasonable defaults
             backup_host = subprocess.Popen(["hostname", "--fqdn"],
                                           
stdout=subprocess.PIPE).communicate()[0].strip()
@@ -191,14 +204,21 @@ class RegisterController(object):
         proc.stdin.write(pubkey + "\n")
         ret = proc.wait()
         if ret != 0:
-            syslog.syslog( syslog.LOG_ERR, "create_user terminated with code
%d" % (ret,) )
-            raise Exception( "create_user terminated with code %d" % (ret,) )
+            raise ServerError( "create_user terminated with code %d" % (ret,) )
 
 if __name__ == "__main__":
-    retCode = createDaemon( sys.argv[1] )
-    syslog.openlog( 'olpc_idmgr', 0, syslog.LOG_DAEMON )
-    syslog.syslog( 'Starting OLPC ID Manager' )
-    server = SimpleXMLRPCServer(("", 8080))
+    #if run with no pid file defined, do not daemonise and log to
+    #stdout
+    if len(sys.argv) > 1:
+        retCode = createDaemon( sys.argv[1] )
+        syslog.openlog( 'olpc_idmgr', 0, syslog.LOG_USER )
+        log = syslog.syslog
+    else:
+        def log(*args):
+            print args
+
+    log( syslog.LOG_NOTICE, 'Starting OLPC ID Manager' )
+    server = SimpleXMLRPCServer(("", PORT))
     server.register_instance(RegisterController())
     model.connect(CONFIG.dburi)
     server.serve_forever()



More information about the Server-devel mailing list