Browse Source

[trac760] replace logger calls

Also did one security-related fix; if you try to log in with a bad username or password, the message sent back is now the same (it used to send a different error depending on whether the username or password was bad)
Jelte Jansen 14 years ago
parent
commit
4dc1b4bf31
3 changed files with 100 additions and 28 deletions
  1. 33 26
      src/bin/cmdctl/cmdctl.py.in
  2. 65 0
      src/bin/cmdctl/cmdctl_messages.mes
  3. 2 2
      src/bin/cmdctl/tests/cmdctl_test.py

+ 33 - 26
src/bin/cmdctl/cmdctl.py.in

@@ -47,6 +47,18 @@ import isc.net.parse
 from optparse import OptionParser, OptionValueError
 from optparse import OptionParser, OptionValueError
 from hashlib import sha1
 from hashlib import sha1
 from isc.util import socketserver_mixin
 from isc.util import socketserver_mixin
+from cmdctl_messages import *
+
+# TODO: these debug-levels are hard-coded here; we are planning on
+# creating a general set of debug levels, see ticket #1074. When done,
+# we should remove these values and use the general ones in the
+# logger.debug calls
+
+# Debug level for communication with BIND10
+DBG_CMDCTL_MESSAGING = 30
+
+isc.log.init("b10-cmdctl")
+logger = isc.log.Logger("cmdctl")
 
 
 try:
 try:
     import threading
     import threading
@@ -173,7 +185,8 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
         if not user_name:
         if not user_name:
             return False, ["need user name"]
             return False, ["need user name"]
         if not self.server.get_user_info(user_name):
         if not self.server.get_user_info(user_name):
-            return False, ["user doesn't exist"]
+            logger.info(CMDCTL_NO_SUCH_USER, user_name)
+            return False, ["username or password error"]
 
 
         user_pwd = user_info.get('password')
         user_pwd = user_info.get('password')
         if not user_pwd:
         if not user_pwd:
@@ -181,7 +194,8 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
         local_info = self.server.get_user_info(user_name)
         local_info = self.server.get_user_info(user_name)
         pwd_hashval = sha1((user_pwd + local_info[1]).encode())
         pwd_hashval = sha1((user_pwd + local_info[1]).encode())
         if pwd_hashval.hexdigest() != local_info[0]:
         if pwd_hashval.hexdigest() != local_info[0]:
-            return False, ["password doesn't match"] 
+            logger.info(CMDCTL_BAD_PASSWORD, user_name)
+            return False, ["username or password error"]
 
 
         return True, None
         return True, None
    
    
@@ -238,7 +252,8 @@ class CommandControl():
         self._cc = isc.cc.Session()
         self._cc = isc.cc.Session()
         self._module_cc = isc.config.ModuleCCSession(SPECFILE_LOCATION,
         self._module_cc = isc.config.ModuleCCSession(SPECFILE_LOCATION,
                                               self.config_handler,
                                               self.config_handler,
-                                              self.command_handler)
+                                              self.command_handler,
+                                              None, True)
         self._module_name = self._module_cc.get_module_spec().get_module_name()
         self._module_name = self._module_cc.get_module_spec().get_module_name()
         self._cmdctl_config_data = self._module_cc.get_full_config()
         self._cmdctl_config_data = self._module_cc.get_full_config()
         self._module_cc.start()
         self._module_cc.start()
@@ -281,7 +296,7 @@ class CommandControl():
                 errstr = 'unknown config item: ' + key
                 errstr = 'unknown config item: ' + key
             
             
             if errstr != None:
             if errstr != None:
-                self.log_info('Fail to apply config data, ' + errstr) 
+                logger.error(CMDCTL_BAD_CONFIG_DATA, errstr);
                 return ccsession.create_answer(1, errstr)
                 return ccsession.create_answer(1, errstr)
 
 
         return ccsession.create_answer(0)
         return ccsession.create_answer(0)
@@ -387,8 +402,8 @@ class CommandControl():
         '''Send the command from bindctl to proper module. '''
         '''Send the command from bindctl to proper module. '''
         errstr = 'unknown error'
         errstr = 'unknown error'
         answer = None
         answer = None
-        if self._verbose:
-            self.log_info("Begin send command '%s' to module '%s'" %(command_name, module_name))
+        logger.debug(DBG_CMDCTL_MESSAGING, CMDCTL_SEND_COMMAND,
+                     command_name, module_name)
 
 
         if module_name == self._module_name:
         if module_name == self._module_name:
             # Process the command sent to cmdctl directly. 
             # Process the command sent to cmdctl directly. 
@@ -396,15 +411,14 @@ class CommandControl():
         else:
         else:
             msg = ccsession.create_command(command_name, params)
             msg = ccsession.create_command(command_name, params)
             seq = self._cc.group_sendmsg(msg, module_name)
             seq = self._cc.group_sendmsg(msg, module_name)
+            logger.debug(DBG_CMDCTL_MESSAGING, CMDCTL_COMMAND_SENT,
+                         command_name, module_name)
             #TODO, it may be blocked, msqg need to add a new interface waiting in timeout.
             #TODO, it may be blocked, msqg need to add a new interface waiting in timeout.
             try:
             try:
                 answer, env = self._cc.group_recvmsg(False, seq)
                 answer, env = self._cc.group_recvmsg(False, seq)
             except isc.cc.session.SessionTimeout:
             except isc.cc.session.SessionTimeout:
                 errstr = "Module '%s' not responding" % module_name
                 errstr = "Module '%s' not responding" % module_name
 
 
-        if self._verbose:
-            self.log_info("Finish send command '%s' to module '%s'" % (command_name, module_name))
-
         if answer:
         if answer:
             try:
             try:
                 rcode, arg = ccsession.parse_answer(answer)
                 rcode, arg = ccsession.parse_answer(answer)
@@ -415,16 +429,13 @@ class CommandControl():
                     else:
                     else:
                         return rcode, {}
                         return rcode, {}
                 else:
                 else:
-                    # TODO: exception
                     errstr = str(answer['result'][1])
                     errstr = str(answer['result'][1])
             except ccsession.ModuleCCSessionError as mcse:
             except ccsession.ModuleCCSessionError as mcse:
                 errstr = str("Error in ccsession answer:") + str(mcse)
                 errstr = str("Error in ccsession answer:") + str(mcse)
-                self.log_info(errstr)
+
+        logger.error(CMDCTL_COMMAND_ERROR, conmmand_name, module_name, errstr)
         return 1, {'error': errstr}
         return 1, {'error': errstr}
     
     
-    def log_info(self, msg):
-        sys.stdout.write("[b10-cmdctl] %s\n" % str(msg))
-
     def get_cmdctl_config_data(self):
     def get_cmdctl_config_data(self):
         ''' If running in source code tree, use keyfile, certificate
         ''' If running in source code tree, use keyfile, certificate
         and user accounts file in source code. '''
         and user accounts file in source code. '''
@@ -481,14 +492,15 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
                 for row in reader:
                 for row in reader:
                     self._user_infos[row[0]] = [row[1], row[2]]
                     self._user_infos[row[0]] = [row[1], row[2]]
             except (IOError, IndexError) as e:
             except (IOError, IndexError) as e:
-                self.log_info("Fail to read user database, %s" % e)                
+                logger.error(CMDCTL_USER_DATABASE_READ_ERROR,
+                             accounts_file, e)
             finally:
             finally:
                 if csvfile:
                 if csvfile:
                     csvfile.close()
                     csvfile.close()
 
 
         self._accounts_file = accounts_file
         self._accounts_file = accounts_file
         if len(self._user_infos) == 0:
         if len(self._user_infos) == 0:
-            self.log_info("Fail to get user information, will deny any user")                
+            logger.error(CMDCTL_NO_USER_ENTRIES_READ)
          
          
     def get_user_info(self, username):
     def get_user_info(self, username):
         '''Get user's salt and hashed string. If the user
         '''Get user's salt and hashed string. If the user
@@ -520,7 +532,7 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
                                       ssl_version = ssl.PROTOCOL_SSLv23)
                                       ssl_version = ssl.PROTOCOL_SSLv23)
             return ssl_sock 
             return ssl_sock 
         except (ssl.SSLError, CmdctlException) as err :
         except (ssl.SSLError, CmdctlException) as err :
-            self.log_info("Deny client's connection because %s" % str(err))
+            logger.info(CMDCTL_SSL_ERROR_USER_DENIED, err)
             self.close_request(sock)
             self.close_request(sock)
             # raise socket error to finish the request
             # raise socket error to finish the request
             raise socket.error
             raise socket.error
@@ -547,9 +559,6 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
     def send_command_to_module(self, module_name, command_name, params):
     def send_command_to_module(self, module_name, command_name, params):
         return self.cmdctl.send_command_with_check(module_name, command_name, params)
         return self.cmdctl.send_command_with_check(module_name, command_name, params)
    
    
-    def log_info(self, msg):
-        sys.stdout.write("[b10-cmdctl] %s\n" % str(msg))
-
 httpd = None
 httpd = None
 
 
 def signal_handler(signal, frame):
 def signal_handler(signal, frame):
@@ -607,15 +616,13 @@ if __name__ == '__main__':
         run(options.addr, options.port, options.idle_timeout, options.verbose)
         run(options.addr, options.port, options.idle_timeout, options.verbose)
         result = 0
         result = 0
     except isc.cc.SessionError as err:
     except isc.cc.SessionError as err:
-        sys.stderr.write("[b10-cmdctl] Error creating b10-cmdctl, "
-                         "is the command channel daemon running?\n")        
+        logger.fatal(CMDCTL_CC_SESSION_ERROR, err)
     except isc.cc.SessionTimeout:
     except isc.cc.SessionTimeout:
-        sys.stderr.write("[b10-cmdctl] Error creating b10-cmdctl, "
-                         "is the configuration manager running?\n")        
+        logger.fatal(CMDCTL_CC_SESSION_TIMEOUT)
     except KeyboardInterrupt:
     except KeyboardInterrupt:
-        sys.stderr.write("[b10-cmdctl] exit from Cmdctl\n")
+        logger.info(CMDCTL_STOPPED_BY_KEYBOARD)
     except CmdctlException as err:
     except CmdctlException as err:
-        sys.stderr.write("[b10-cmdctl] " + str(err) + "\n")
+        logger.fatal(CMDCTL_UNCAUGHT_EXCEPTION, err);
 
 
     if httpd:
     if httpd:
         httpd.shutdown()
         httpd.shutdown()

+ 65 - 0
src/bin/cmdctl/cmdctl_messages.mes

@@ -14,3 +14,68 @@
 
 
 # No namespace declaration - these constants go in the global namespace
 # No namespace declaration - these constants go in the global namespace
 # of the cmdctl_messages python module.
 # of the cmdctl_messages python module.
+
+% CMDCTL_BAD_CONFIG_DATA error in config data: %1
+There was an error reading the updated configuration data. The specific
+error is printed.
+
+% CMDCTL_BAD_PASSWORD Bad password for user: %1
+A login attempt was made to b10-cmdctl, but the password was wrong.
+Users can be managed with the tool b10-cmdctl-usermgr.
+
+% CMDCTL_CC_SESSION_ERROR error reading from cc channel: %1
+There was a problem reading from the command and control channel. The
+most likely cause is that the msgq daemon is not running.
+
+% CMDCTL_CC_SESSION_TIMEOUT timeout on cc channel
+A timeout occurred when waiting for essential data from the cc session.
+This usually occurs when b10-cfgmgr is not running or not responding.
+Since we are waiting for essential information, this is a fatal error,
+and the cmdctl daemon will now shut down.
+
+% CMDCTL_COMMAND_ERROR Error in command %1 to module %2: %3
+An error was encountered sending the given command to the given module.
+Either there was a communication problem with the module, or the module
+was not able to process the command, and sent back an error. The
+specific error is printed in the message.
+
+% CMDCTL_COMMAND_SENT Command '%1' to module '%2' was sent
+This debug message indicates that the given command is being sent to
+the given module.
+
+% CMDCTL_NO_SUCH_USER username not found in user database: %1
+A login attempt was made to b10-cmdctl, but the username was not known.
+Users can be added with the tool b10-cmdctl-usermgr.
+
+% CMDCTL_NO_USER_ENTRIES_READ failed to read user information, all users will be denied
+The b10-cmdctl daemon was unable to find any user data in the user
+database file. Either it was unable to read the file (in which case
+this message follows a message CMDCTL_USER_DATABASE_READ_ERROR
+containing a specific error), or the file was empty. Users can be added
+with the tool b10-cmdctl-usermgr.
+
+% CMDCTL_SEND_COMMAND Sending command %1 to module %2
+This debug message indicates that the given command is being sent to
+the given module.
+
+% CMDCTL_SSL_ERROR_USER_DENIED User denied because of SSL error: %1
+The user was denied because the SSL connection could not successfully
+be set up. The specific error is given in the log message. Possible
+causes may be that the ssl request itself was bad, or the key or
+certificate file had a problem.
+
+% CMDCTL_STOPPED_BY_KEYBOARD keyboard interrupt, shutting down
+There was a keyboard interrupt signal to stop the cmdctl daemon. The
+daemon will now shut down.
+
+% CMDCTL_UNCAUGHT_EXCEPTION uncaught exception: %1
+The b10-cdmctl daemon encountered an uncaught exception and
+will now shut down. This is indicative of a programming error and
+should not happen under normal circumstances. The exception message
+is printed.
+
+% CMDCTL_USER_DATABASE_READ_ERROR failed to read user database file %1: %2
+The b10-cmdctl daemon was unable to read the user database file. The
+file may be unreadable for the daemon, or it may be corrupted. In the
+latter case, it can be recreated with b10-cmdctl-usermgr. The specific
+error is printed in the log message.

+ 2 - 2
src/bin/cmdctl/tests/cmdctl_test.py

@@ -173,7 +173,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.server._user_infos['root'] = ['aa', 'aaa']
         self.handler.server._user_infos['root'] = ['aa', 'aaa']
         ret, msg = self.handler._check_user_name_and_pwd()
         ret, msg = self.handler._check_user_name_and_pwd()
         self.assertFalse(ret)
         self.assertFalse(ret)
-        self.assertEqual(msg, ['password doesn\'t match'])
+        self.assertEqual(msg, ['username or password error'])
 
 
     def test_check_user_name_and_pwd_2(self):
     def test_check_user_name_and_pwd_2(self):
         user_info = {'username':'root', 'password':'abc123'}
         user_info = {'username':'root', 'password':'abc123'}
@@ -214,7 +214,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
 
 
         ret, msg = self.handler._check_user_name_and_pwd()
         ret, msg = self.handler._check_user_name_and_pwd()
         self.assertFalse(ret)
         self.assertFalse(ret)
-        self.assertEqual(msg, ['user doesn\'t exist'])
+        self.assertEqual(msg, ['username or password error'])
 
 
     def test_do_POST(self):
     def test_do_POST(self):
         self.handler.headers = {}
         self.handler.headers = {}