Browse Source

finaly sync for merge

git-svn-id: svn://bind10.isc.org/svn/bind10/experiments/python-binding@2360 e5f2f494-b856-4b98-b285-d166d9295462
Jelte Jansen 15 years ago
parent
commit
26266087e3

+ 19 - 0
ChangeLog

@@ -1,3 +1,22 @@
+  68.  [func]	    zhanglikun
+	Add options -c(--certificate-chain) to bindctl. Override class
+	HTTPSConnection to support server certificate validation.
+	Add support to cmdctl.spec file, now there are three configurable 
+	items for cmdctl: 'key_file', 'cert_file' and 'accounts_file', 
+	all of them can be changed in runtime.
+	(Trac #127, svn r2357)
+
+  67.  [func]	    zhanglikun
+	Make bindctl's command parser only do minimal check. Parameter 
+	value can be a sequence of non-space characters, or a string 
+	surrounded by quotation	marks(these marks can be a part of the 
+	value string in escaped form). Make	error message be more 
+	friendly.(if there is some error in parameter's value, the 
+	parameter name will be provided). Refactor function login_to_cmdctl() 
+	in class BindCmdInterpreter: avoid using Exception to catch all 
+	exceptions.
+	(Trac #220, svn r2356)
+
   66.  [bug]        each
 	Check for duplicate RRsets before inserting data into a message
 	section; this, among other things, will prevent multiple copies

+ 1 - 0
configure.ac

@@ -413,6 +413,7 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            src/bin/cmdctl/cmdctl.py
            src/bin/cmdctl/run_b10-cmdctl.sh
            src/bin/cmdctl/tests/cmdctl_test
+           src/bin/cmdctl/cmdctl.spec.pre
            src/bin/xfrin/tests/xfrin_test
            src/bin/xfrin/xfrin.py
            src/bin/xfrin/xfrin.spec.pre

+ 1 - 1
src/bin/bind10/bind10.py.in

@@ -387,7 +387,7 @@ class BoB:
     def stop_all_processes(self):
         """Stop all processes."""
         cmd = { "command": ['shutdown']}
-        self.cc_session.group_sendmsg(cmd, 'Boss', 'Cmd-Ctrld')
+        self.cc_session.group_sendmsg(cmd, 'Boss', 'Cmdctl')
         self.cc_session.group_sendmsg(cmd, "Boss", "ConfigManager")
         self.cc_session.group_sendmsg(cmd, "Boss", "Auth")
         self.cc_session.group_sendmsg(cmd, "Boss", "Xfrout")

+ 0 - 13
src/bin/bindctl/Makefile.am

@@ -9,8 +9,6 @@ python_PYTHON = __init__.py bindcmd.py cmdparse.py exception.py moduleinfo.py my
 pythondir = $(pyexecdir)/bindctl
 
 bindctldir = $(DESTDIR)$(pkgdatadir)
-bindctl_DATA = bindctl.pem
-EXTRA_DIST += bindctl.pem
 
 CLEANFILES = bindctl
 
@@ -26,14 +24,3 @@ bindctl: bindctl-source.py
 	       -e "s|@@SYSCONFDIR@@|@sysconfdir@|" \
 	       -e "s|@@LIBEXECDIR@@|$(pkglibexecdir)|" bindctl-source.py >$@
 	chmod a+x $@
-
-if INSTALL_CONFIGURATIONS
-
-# TODO: permissions handled later
-install-data-local:
-	$(mkinstalldirs) $(DESTDIR)/@sysconfdir@/@PACKAGE@   
-	if test ! -f $(DESTDIR)$(sysconfdir)/@PACKAGE@/bindctl.pem; then	\
-	  $(INSTALL_DATA) $(srcdir)/bindctl.pem $(DESTDIR)$(sysconfdir)/@PACKAGE@/ ;	\
-	fi
-
-endif

+ 1 - 0
src/bin/bindctl/TODO

@@ -18,3 +18,4 @@ functions should be updated first.):
    If the default user is saved in file, its password shouldn't be saved in plaintext.
 7. Need to think of what exactly to do with responses received to commands
    (currently it simply print the map)
+8. Remove bindctl-source.py.in(replace with bindctl-source.py) when merging to trunk.

+ 130 - 79
src/bin/bindctl/bindcmd.py

@@ -36,6 +36,9 @@ import getpass
 from hashlib import sha1
 import csv
 import ast
+import pwd
+import getpass
+import traceback
 
 try:
     from collections import OrderedDict
@@ -49,6 +52,8 @@ try:
 except ImportError:
     my_readline = sys.stdin.readline
 
+CSV_FILE_NAME = 'default_user.csv'
+FAIL_TO_CONNECT_WITH_CMDCTL = "Fail to connect with b10-cmdctl module, is it running?"
 CONFIG_MODULE_NAME = 'config'
 CONST_BINDCTL_HELP = """
 usage: <module name> <command name> [param1 = value1 [, param2 = value2]]
@@ -58,10 +63,34 @@ Type \"<module_name> help\" for help on the specific module.
 Type \"<module_name> <command_name> help\" for help on the specific command.
 \nAvailable module names: """
 
+class ValidatedHTTPSConnection(http.client.HTTPSConnection):
+    '''Overrides HTTPSConnection to support certification 
+    validation. '''
+    def __init__(self, host, ca_certs):
+        http.client.HTTPSConnection.__init__(self, host)
+        self.ca_certs = ca_certs
+
+    def connect(self):
+        ''' Overrides the connect() so that we do 
+        certificate validation. '''
+        sock = socket.create_connection((self.host, self.port),
+                                        self.timeout)
+        if self._tunnel_host:
+            self.sock = sock
+            self._tunnel()
+       
+        req_cert = ssl.CERT_NONE
+        if self.ca_certs:
+            req_cert = ssl.CERT_REQUIRED
+        self.sock = ssl.wrap_socket(sock, self.key_file,
+                                    self.cert_file,
+                                    cert_reqs=req_cert,
+                                    ca_certs=self.ca_certs)
+
 class BindCmdInterpreter(Cmd):
     """simple bindctl example."""    
 
-    def __init__(self, server_port = 'localhost:8080', pem_file = "bindctl.pem"):
+    def __init__(self, server_port = 'localhost:8080', pem_file = None):
         Cmd.__init__(self)
         self.location = ""
         self.prompt_end = '> '
@@ -70,19 +99,10 @@ class BindCmdInterpreter(Cmd):
         self.modules = OrderedDict()
         self.add_module_info(ModuleInfo("help", desc = "Get help for bindctl"))
         self.server_port = server_port
-        self.pem_file = pem_file
-        self._connect_to_cmd_ctrld()
+        self.conn = ValidatedHTTPSConnection(self.server_port,
+                                             ca_certs=pem_file)
         self.session_id = self._get_session_id()
 
-    def _connect_to_cmd_ctrld(self):
-        '''Connect to cmdctl in SSL context. '''
-        try:
-            self.conn = http.client.HTTPSConnection(self.server_port,
-                          cert_file=self.pem_file)
-        except  Exception as e:
-            print(e, "can't connect to %s, please make sure cmd-ctrld is running" %
-                  self.server_port)
-
     def _get_session_id(self):
         '''Generate one session id for the connection. '''
         rand = os.urandom(16)
@@ -93,17 +113,59 @@ class BindCmdInterpreter(Cmd):
         return digest
     
     def run(self):
-        '''Parse commands inputted from user and send them to cmdctl. '''
+        '''Parse commands from user and send them to cmdctl. '''
         try:
             if not self.login_to_cmdctl():
-                return False
+                return 
 
-            # Get all module information from cmd-ctrld
-            self.config_data = isc.config.UIModuleCCSession(self)
-            self._update_commands()
             self.cmdloop()
+        except FailToLogin as err:
+            print(err)
+            print(FAIL_TO_CONNECT_WITH_CMDCTL)
+            traceback.print_exc()
         except KeyboardInterrupt:
-            return True
+            print('\nExit from bindctl')
+
+    def _get_saved_user_info(self, dir, file_name):
+        ''' Read all the available username and password pairs saved in 
+        file(path is "dir + file_name"), Return value is one list of elements
+        ['name', 'password'], If get information failed, empty list will be 
+        returned.'''
+        if (not dir) or (not os.path.exists(dir)):
+            return []
+
+        try:
+            csvfile = None
+            users = []
+            csvfile = open(dir + file_name)
+            users_info = csv.reader(csvfile)
+            for row in users_info:
+                users.append([row[0], row[1]])
+        except (IOError, IndexError) as e:
+            pass
+        finally:
+            if csvfile:
+                csvfile.close()
+            return users
+
+    def _save_user_info(self, username, passwd, dir, file_name):
+        ''' Save username and password in file "dir + file_name"
+        If it's saved properly, return True, or else return False. '''
+        try:
+            if not os.path.exists(dir):
+                os.mkdir(dir, 0o700)
+
+            csvfilepath = dir + file_name 
+            csvfile = open(csvfilepath, 'w')
+            os.chmod(csvfilepath, 0o600)
+            writer = csv.writer(csvfile)
+            writer.writerow([username, passwd])
+            csvfile.close()
+        except Exception as e:
+            print(e, "\nCannot write %s%s; default user is not stored" % (dir, file_name))
+            return False
+
+        return True
 
     def login_to_cmdctl(self):
         '''Login to cmdctl with the username and password inputted 
@@ -112,33 +174,21 @@ class BindCmdInterpreter(Cmd):
         time, username and password saved in 'default_user.csv' will be
         used first.
         '''
-        csvfile = None
-        bsuccess = False
-        try:
-            cvsfilepath = ""
-            if ('HOME' in os.environ):
-                cvsfilepath = os.environ['HOME']
-                cvsfilepath += os.sep + '.bind10' + os.sep
-            cvsfilepath += 'default_user.csv'
-            csvfile = open(cvsfilepath)
-            users = csv.reader(csvfile)
-            for row in users:
-                param = {'username': row[0], 'password' : row[1]}
+        csv_file_dir = pwd.getpwnam(getpass.getuser()).pw_dir
+        csv_file_dir += os.sep + '.bind10' + os.sep
+        users = self._get_saved_user_info(csv_file_dir, CSV_FILE_NAME)
+        for row in users:
+            param = {'username': row[0], 'password' : row[1]}
+            try:
                 response = self.send_POST('/login', param)
                 data = response.read().decode()
-                if response.status == http.client.OK:
-                    print(data + ' login as ' + row[0] )
-                    bsuccess = True
-                    break
-        except IOError as e:
-            pass
-        except Exception as e:
-            print(e)
-        finally:
-            if csvfile:
-                csvfile.close()
-            if bsuccess:
-                return True
+            except socket.error:
+                traceback.print_exc()
+                raise FailToLogin()
+
+            if response.status == http.client.OK:
+                print(data + ' login as ' + row[0] )
+                return True 
 
         count = 0
         print("[TEMP MESSAGE]: username :root  password :bind10")
@@ -151,34 +201,18 @@ class BindCmdInterpreter(Cmd):
             username = input("Username:")
             passwd = getpass.getpass()
             param = {'username': username, 'password' : passwd}
-            response = self.send_POST('/login', param)
-            data = response.read().decode()
-            print(data)
-            
+            try:
+                response = self.send_POST('/login', param)
+                data = response.read().decode()
+                print(data)
+            except socket.error as e:
+                traceback.print_exc()
+                raise FailToLogin()
+
             if response.status == http.client.OK:
-                cvsfilepath = ""
-                try:
-                    if ('HOME' in os.environ):
-                        cvsfilepath = os.environ['HOME']
-                        cvsfilepath += os.sep + '.bind10' + os.sep
-                        if not os.path.exists(cvsfilepath):
-                                os.mkdir(cvsfilepath, 0o700)
-                    else:
-                        print("Cannot determine location of $HOME. Not storing default user")
-                        return True
-                    cvsfilepath += 'default_user.csv'
-                    csvfile = open(cvsfilepath, 'w')
-                    os.chmod(cvsfilepath, 0o600)
-                    writer = csv.writer(csvfile)
-                    writer.writerow([username, passwd])
-                    csvfile.close()
-                except Exception as e:
-                    # just not store it
-                    print("Cannot write ~/.bind10/default_user.csv; default user is not stored")
-                    print(e)
+                self._save_user_info(username, passwd, csv_file_dir, CSV_FILE_NAME)
                 return True
 
-
     def _update_commands(self):
         '''Update the commands of all modules. '''
         for module_name in self.config_data.get_config_item_list():
@@ -211,7 +245,19 @@ class BindCmdInterpreter(Cmd):
         headers = {"cookie" : self.session_id}
         self.conn.request('POST', url, param, headers)
         return self.conn.getresponse()
-        
+
+    def _update_all_modules_info(self):
+        ''' Get all modules' information from cmdctl, including
+        specification file and configuration data. This function
+        should be called before interpreting command line or complete-key
+        is entered. This may not be the best way to keep bindctl
+        and cmdctl share same modules information, but it works.'''
+        self.config_data = isc.config.UIModuleCCSession(self)
+        self._update_commands()
+
+    def precmd(self, line):
+        self._update_all_modules_info()
+        return line 
 
     def postcmd(self, stop, line):
         '''Update the prompt after every command'''
@@ -308,8 +354,11 @@ class BindCmdInterpreter(Cmd):
         if cmd.module != CONFIG_MODULE_NAME:
             for param_name in cmd.params:
                 param_spec = command_info.get_param_with_name(param_name).param_spec
-                cmd.params[param_name] = isc.config.config_data.convert_type(param_spec, cmd.params[param_name])
-
+                try:
+                    cmd.params[param_name] = isc.config.config_data.convert_type(param_spec, cmd.params[param_name])
+                except isc.cc.data.DataTypeError as e:
+                    raise isc.cc.data.DataTypeError('Invalid parameter value for \"%s\", the type should be \"%s\" \n' 
+                                                     % (param_name, param_spec['item_type']) + str(e))
     
     def _handle_cmd(self, cmd):
         '''Handle a command entered by the user'''
@@ -356,6 +405,7 @@ class BindCmdInterpreter(Cmd):
 
     def complete(self, text, state):
         if 0 == state:
+            self._update_all_modules_info()
             text = text.strip()
             hints = []
             cur_line = my_readline()
@@ -441,13 +491,14 @@ class BindCmdInterpreter(Cmd):
             cmd = BindCmdParse(line)
             self._validate_cmd(cmd)
             self._handle_cmd(cmd)
-        except BindCtlException as e:
-            print("Error! ", e)
-            self._print_correct_usage(e)
-        except isc.cc.data.DataTypeError as e:
-            print("Error! ", e)
-            self._print_correct_usage(e)
-            
+        except (IOError, http.client.HTTPException) as err:
+            print('Error!', err)
+            print(FAIL_TO_CONNECT_WITH_CMDCTL)
+        except BindCtlException as err:
+            print("Error! ", err)
+            self._print_correct_usage(err)
+        except isc.cc.data.DataTypeError as err:
+            print("Error! ", err)
             
     def _print_correct_usage(self, ept):        
         if isinstance(ept, CmdUnknownModuleSyntaxError):
@@ -556,7 +607,7 @@ class BindCmdInterpreter(Cmd):
         if (len(cmd.params) != 0):
             cmd_params = json.dumps(cmd.params)
 
-        print("send the message to cmd-ctrld")        
+        print("send the command to cmd-ctrld")        
         reply = self.send_POST(url, cmd.params)
         data = reply.read().decode()
         print("received reply:", data)

+ 8 - 12
src/bin/bindctl/bindctl-source.py.in

@@ -97,13 +97,16 @@ def check_addr(option, opt_str, value, parser):
 
 def set_bindctl_options(parser):
     parser.add_option('-p', '--port', dest = 'port', type = 'int',
-            action = 'callback', callback=check_port,
-            default = '8080', help = 'port for cmdctl of bind10')
+                      action = 'callback', callback=check_port,
+                      default = '8080', help = 'port for cmdctl of bind10')
 
     parser.add_option('-a', '--address', dest = 'addr', type = 'string',
-            action = 'callback', callback=check_addr,
-            default = '127.0.0.1', help = 'IP address for cmdctl of bind10')
+                      action = 'callback', callback=check_addr,
+                      default = '127.0.0.1', help = 'IP address for cmdctl of bind10')
 
+    parser.add_option('-c', '--certificate-chain', dest = 'cert_chain', 
+                      type = 'string', action = 'store',
+                      help = 'PEM formatted server certificate validation chain file')
 
 if __name__ == '__main__':
     try:
@@ -111,14 +114,7 @@ if __name__ == '__main__':
         set_bindctl_options(parser)
         (options, args) = parser.parse_args()
         server_addr = options.addr + ':' + str(options.port)
-        # If B10_FROM_SOURCE is set in the environment, we use PEM file
-        # from a directory relative to that, otherwise we use the one
-        # installed on the system
-        if "B10_FROM_SOURCE" in os.environ:
-            SYSCONF_PATH = os.environ["B10_FROM_SOURCE"] + "/src/bin/bindctl"
-        else:
-            SYSCONF_PATH = "@@SYSCONFDIR@@/@PACKAGE@"
-        tool = BindCmdInterpreter(server_addr, pem_file = SYSCONF_PATH + "/bindctl.pem")
+        tool = BindCmdInterpreter(server_addr, pem_file=options.cert_chain)
         prepare_config_commands(tool)
         tool.run()
     except Exception as e:

+ 0 - 36
src/bin/bindctl/bindctl.pem

@@ -1,36 +0,0 @@
------BEGIN RSA PRIVATE KEY-----
-MIICXAIBAAKBgQDpICWxJGKMvUhLFPbf5n8ZWogqjYcQqqoHqHVRHYjyiey6FZdt
-ZkY2s1gYh0G0NXtimlIgic+vEcFe7vdmyKntW7DYDaqAj0KrED7RKAj8324jNbSJ
-HtLP4evvJep3vsoNtTvNuceQJ46vukxyxgg3DuC9kVqPuD8CZ1Rq4ATyiwIDAQAB
-AoGBAOJlOtV+DUq6Y2Ou91VXRiU8GzKgAQP5iWgoe84Ljbxkn4XThBxVD2j94Fbp
-u7AjpDCMx6cbzpoo9w6XqaGizAmAehIfTE3eFYs74N/FM09Wg2OSDyxMY0jgyECU
-A4ukjlPwcGDbmgbmlY3i+FVHp+zCgtZEsMC1IAosMac1BoX5AkEA/lrXWaVtH8bo
-mut3GBaXvubZMdaUr0BUd5a9q+tt4dQcKG1kFqgCNKhNhBIcpiMVcz+jGmOuopNA
-8dnUGqv3FQJBAOqiJ54ZvOTWNDpJIe02wIXRxRmc1xhHFCqYP23KxBVrAcTYB19J
-lesov/hEbnGLCbKS/naZJ1zrTImUPNRLqx8CQCzDtA7U7GWhTiKluioFH+O7IRKC
-X1yQh80cPHlbT9VkzSfYSLssCmdWD35k6aHbntTPqFbmoD+AhveJjKi9BxkCQDwX
-1c+/RcrSNcQr0N2hZUOgyztZGRnlsnuKTMyA3yGhK23P6mt0PEpjQG+Ej0jTVGOB
-FF0pspQwy4R9C+tPif8CQH36NNlXBfVNmT7kDtyLmaE6pID0vY9duX56BJbU1R0x
-SQ8/LcfJagk6gvp08OyYCPA+WZ7u/bas9R/nMTCLivc=
------END RSA PRIVATE KEY-----
------BEGIN CERTIFICATE-----
-MIIDhzCCAvCgAwIBAgIJALwngNFik7ONMA0GCSqGSIb3DQEBBQUAMIGKMQswCQYD
-VQQGEwJjbjEQMA4GA1UECBMHYmVpamluZzEQMA4GA1UEBxMHYmVpamluZzEOMAwG
-A1UEChMFY25uaWMxDjAMBgNVBAsTBWNubmljMRMwEQYDVQQDEwp6aGFuZ2xpa3Vu
-MSIwIAYJKoZIhvcNAQkBFhN6aGFuZ2xpa3VuQGNubmljLmNuMB4XDTEwMDEwNzEy
-NDcxOFoXDTExMDEwNzEyNDcxOFowgYoxCzAJBgNVBAYTAmNuMRAwDgYDVQQIEwdi
-ZWlqaW5nMRAwDgYDVQQHEwdiZWlqaW5nMQ4wDAYDVQQKEwVjbm5pYzEOMAwGA1UE
-CxMFY25uaWMxEzARBgNVBAMTCnpoYW5nbGlrdW4xIjAgBgkqhkiG9w0BCQEWE3po
-YW5nbGlrdW5AY25uaWMuY24wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAOkg
-JbEkYoy9SEsU9t/mfxlaiCqNhxCqqgeodVEdiPKJ7LoVl21mRjazWBiHQbQ1e2Ka
-UiCJz68RwV7u92bIqe1bsNgNqoCPQqsQPtEoCPzfbiM1tIke0s/h6+8l6ne+yg21
-O825x5Anjq+6THLGCDcO4L2RWo+4PwJnVGrgBPKLAgMBAAGjgfIwge8wHQYDVR0O
-BBYEFJKM/O0ViGlwtb3JEci/DLTO/7DaMIG/BgNVHSMEgbcwgbSAFJKM/O0ViGlw
-tb3JEci/DLTO/7DaoYGQpIGNMIGKMQswCQYDVQQGEwJjbjEQMA4GA1UECBMHYmVp
-amluZzEQMA4GA1UEBxMHYmVpamluZzEOMAwGA1UEChMFY25uaWMxDjAMBgNVBAsT
-BWNubmljMRMwEQYDVQQDEwp6aGFuZ2xpa3VuMSIwIAYJKoZIhvcNAQkBFhN6aGFu
-Z2xpa3VuQGNubmljLmNuggkAvCeA0WKTs40wDAYDVR0TBAUwAwEB/zANBgkqhkiG
-9w0BAQUFAAOBgQBh5N6isMAQAFFD+pbfpppjQlO4vUNcEdzPdeuBFaf9CsX5ZdxV
-jmn1ZuGm6kRzqUPwPSxvCIAY0wuSu1g7YREPAZ3XBVwcg6262iGOA6n7E+nv5PLz
-EuZ1oUg+IfykUIoflKH6xZB4MyPL+EgkMT+i9BrngaXHXF8tEO30YppMiA==
------END CERTIFICATE-----

+ 8 - 5
src/bin/bindctl/cmdparse.py

@@ -24,15 +24,19 @@ except ImportError:
     from bindctl.mycollections import OrderedDict
 
 param_name_str = "^\s*(?P<param_name>[\w]+)\s*=\s*"
-param_value_str = "(?P<param_value>[\w\.:/-]+)"
-param_value_with_quota_str = "[\"\'](?P<param_value>[\w\.:, /-]+)[\"\']"
+
+# The value string can be a sequence without space or comma 
+# characters, or a string surroundedby quotation marks(such marks
+# can be part of string in an escaped form)
+#param_value_str  = "(?P<param_value>[\"\'].+?(?<!\\\)[\"\']|[^\'\"][^, ]+)"
+param_value_str  = "(?P<param_value>[^\'\" ][^, ]+)"
+param_value_with_quota_str  = "[\"\'](?P<param_value>.+?)(?<!\\\)[\"\']"
 next_params_str = "(?P<blank>\s*)(?P<comma>,?)(?P<next_params>.*)$"
 
 PARAM_WITH_QUOTA_PATTERN = re.compile(param_name_str + 
-                                      param_value_with_quota_str +
+                                      param_value_with_quota_str + 
                                       next_params_str)
 PARAM_PATTERN = re.compile(param_name_str + param_value_str + next_params_str)
-                           
 # Used for module and command name
 NAME_PATTERN = re.compile("^\s*(?P<name>[\w]+)(?P<blank>\s*)(?P<others>.*)$")
 
@@ -98,7 +102,6 @@ class BindCmdParse:
                 
             groups = PARAM_PATTERN.match(param_text) or \
                      PARAM_WITH_QUOTA_PATTERN.match(param_text)
-            
             if not groups:
                 # ok, fill in the params in the order entered
                 params = re.findall("([^\" ]+|\".*\")", param_text)

+ 7 - 0
src/bin/bindctl/exception.py

@@ -115,3 +115,10 @@ class CmdMissParamSyntaxError(CmdSyntaxError):
     def __str__(self):
         return str("Parameter '%s' is missed for command '%s' of module '%s'" % 
                    (self.param, self.command, self.module))
+
+
+class FailToLogin(BindCtlException):
+    def __str__(self):
+        return "Fail to login to cmdctl"
+
+

+ 48 - 4
src/bin/bindctl/tests/bindctl_test.py

@@ -16,6 +16,7 @@
 
 import unittest
 import isc.cc.data
+import os
 from bindctl import cmdparse
 from bindctl import bindcmd
 from bindctl.moduleinfo import *
@@ -50,12 +51,31 @@ class TestCmdLex(unittest.TestCase):
             assert cmd.params["zone_name"] == "cnnic.cn"
             assert cmd.params["file"] == "cnnic.cn.file"
             assert cmd.params["master"] == '1.1.1.1'
+
+    def testCommandWithParamters_2(self):
+        '''Test whether the parameters in key=value can be parsed properly.'''
+        cmd = cmdparse.BindCmdParse('zone cmd name = 1:34::2')
+        self.assertEqual(cmd.params['name'], '1:34::2')
+
+        cmd = cmdparse.BindCmdParse('zone cmd name = 1\"\'34**&2 value=44\"\'\"')
+        self.assertEqual(cmd.params['name'], '1\"\'34**&2')
+        self.assertEqual(cmd.params['value'], '44\"\'\"')
+
+        cmd = cmdparse.BindCmdParse('zone cmd name = 1\"\'34**&2 ,value=  44\"\'\"')
+        self.assertEqual(cmd.params['name'], '1\"\'34**&2')
+        self.assertEqual(cmd.params['value'], '44\"\'\"')
             
+        cmd = cmdparse.BindCmdParse('zone cmd name =  1\'34**&2value=44\"\'\" value = \"==============\'')
+        self.assertEqual(cmd.params['name'], '1\'34**&2value=44\"\'\"')
+        self.assertEqual(cmd.params['value'], '==============')
+
+        cmd = cmdparse.BindCmdParse('zone cmd name =    \"1234, 567890 \" value ==&*/')
+        self.assertEqual(cmd.params['name'], '1234, 567890 ')
+        self.assertEqual(cmd.params['value'], '=&*/')
             
     def testCommandWithListParam(self):
-            cmd = cmdparse.BindCmdParse("zone set zone_name='cnnic.cn', master='1.1.1.1, 2.2.2.2'")
-            assert cmd.params["master"] == '1.1.1.1, 2.2.2.2'            
-        
+        cmd = cmdparse.BindCmdParse("zone set zone_name='cnnic.cn', master='1.1.1.1, 2.2.2.2'")
+        assert cmd.params["master"] == '1.1.1.1, 2.2.2.2'            
         
     def testCommandWithHelpParam(self):
         cmd = cmdparse.BindCmdParse("zone add help")
@@ -217,8 +237,32 @@ class TestNameSequence(unittest.TestCase):
             assert self.random_names[i] == cmd_names[i+1]
             assert self.random_names[i] == module_names[i+1]
             i = i + 1
-        
     
+class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
+    def __init__(self):
+        pass
+
+class TestBindCmdInterpreter(unittest.TestCase):
+
+    def _create_invalid_csv_file(self, csvfilename):
+        import csv
+        csvfile = open(csvfilename, 'w')
+        writer = csv.writer(csvfile)
+        writer.writerow(['name1'])
+        writer.writerow(['name2'])
+        csvfile.close()
+
+    def test_get_saved_user_info(self):
+        cmd = FakeBindCmdInterpreter()
+        users = cmd._get_saved_user_info('/notexist', 'cvs_file.cvs')
+        self.assertEqual([], users)
+        
+        csvfilename = 'csv_file.csv'
+        self._create_invalid_csv_file(csvfilename)
+        users = cmd._get_saved_user_info('./', csvfilename)
+        self.assertEqual([], users)
+        os.remove(csvfilename)
+
 if __name__== "__main__":
     unittest.main()
     

+ 4 - 2
src/bin/cmdctl/Makefile.am

@@ -17,9 +17,8 @@ b10_cmdctl_DATA = $(CMDCTL_CONFIGURATIONS)
 b10_cmdctl_DATA += cmdctl.spec
  
 EXTRA_DIST = $(CMDCTL_CONFIGURATIONS)
-EXTRA_DIST += cmdctl.spec
 
-CLEANFILES=	b10-cmdctl cmdctl.pyc
+CLEANFILES=	b10-cmdctl cmdctl.pyc cmdctl.spec
 
 man_MANS = b10-cmdctl.8
 EXTRA_DIST += $(man_MANS) b10-cmdctl.xml
@@ -31,6 +30,9 @@ b10-cmdctl.8: b10-cmdctl.xml
 
 endif
 
+cmdctl.spec: cmdctl.spec.pre
+	$(SED) -e "s|@@SYSCONFDIR@@|$(sysconfdir)|" cmdctl.spec.pre >$@
+
 # TODO: does this need $$(DESTDIR) also?
 # this is done here since configure.ac AC_OUTPUT doesn't expand exec_prefix
 b10-cmdctl: cmdctl.py

+ 3 - 5
src/bin/cmdctl/TODO

@@ -1,8 +1,6 @@
-. Refine code for b10-cmdctl.
-. Add value type check according module specification.
 . Add return code for RESTful API document of b10-cmdctl.
-. Add more unit tests for b10-cmdctl.
 . Update man page for b10-cmdctl?
+. Add check for the content of key/certificate file
+  (when cmdctl starts or is configured by bindctl).
+. Use only one msgq/session to communicate with other modules?
 
-. Add id to each command, so the receiver knows if the response is what it wants.
-. Make cmdctl can be configured through bindctl.(after id is added)

+ 268 - 136
src/bin/cmdctl/cmdctl.py.in

@@ -41,6 +41,7 @@ import csv
 import random
 import time
 import signal
+from isc.config import ccsession
 from optparse import OptionParser, OptionValueError
 from hashlib import sha1
 try:
@@ -50,29 +51,27 @@ except ImportError:
 
 __version__ = 'BIND10'
 URL_PATTERN = re.compile('/([\w]+)(?:/([\w]+))?/?')
+CONFIG_DATA_URL = 'config_data'
+MODULE_SPEC_URL = 'module_spec'
 
-# If B10_FROM_SOURCE is set in the environment, we use data files
+
+# If B10_FROM_BUILD is set in the environment, we use data files
 # from a directory relative to that, otherwise we use the ones
 # installed on the system
-if "B10_FROM_SOURCE" in os.environ:
-    SPECFILE_PATH = os.environ["B10_FROM_SOURCE"] + "/src/bin/cmdctl"
-    SYSCONF_PATH = os.environ["B10_FROM_SOURCE"] + "/src/bin/cmdctl"
+if "B10_FROM_BUILD" in os.environ:
+    SPECFILE_PATH = os.environ["B10_FROM_BUILD"] + "/src/bin/cmdctl"
 else:
     PREFIX = "@prefix@"
     DATAROOTDIR = "@datarootdir@"
     SPECFILE_PATH = "@datadir@/@PACKAGE@".replace("${datarootdir}", DATAROOTDIR).replace("${prefix}", PREFIX)
-    SYSCONF_PATH = "@sysconfdir@/@PACKAGE@".replace("${prefix}", PREFIX)
 SPECFILE_LOCATION = SPECFILE_PATH + os.sep + "cmdctl.spec"
-USER_INFO_FILE = SYSCONF_PATH + os.sep + "cmdctl-accounts.csv"
-PRIVATE_KEY_FILE = SYSCONF_PATH + os.sep + "cmdctl-keyfile.pem"
-CERTIFICATE_FILE = SYSCONF_PATH + os.sep + "cmdctl-certfile.pem"
-        
+
+class CmdctlException(Exception):
+    pass
+       
 class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
     '''https connection request handler.
-    Currently only GET and POST are supported.
-
-    '''
-
+    Currently only GET and POST are supported.  '''
     def do_GET(self):
         '''The client should send its session id in header with 
         the name 'cookie'
@@ -167,13 +166,13 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
         user_name = user_info.get('username')
         if not user_name:
             return False, ["need user name"]
-        if not self.server.user_infos.get(user_name):
+        if not self.server.get_user_info(user_name):
             return False, ["user doesn't exist"]
 
         user_pwd = user_info.get('password')
         if not user_pwd:
             return False, ["need password"]
-        local_info = self.server.user_infos.get(user_name)
+        local_info = self.server.get_user_info(user_name)
         pwd_hashval = sha1((user_pwd + local_info[1]).encode())
         if pwd_hashval.hexdigest() != local_info[0]:
             return False, ["password doesn't match"] 
@@ -197,9 +196,6 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
                 pass
 
         rcode, reply = self.server.send_command_to_module(mod, cmd, param)
-        if self.server._verbose:
-            print('[b10-cmdctl] Finish send message \'%s\' to module %s' % (cmd, mod))
-
         ret = http.client.OK
         if rcode != 0:
             ret = http.client.BAD_REQUEST
@@ -214,65 +210,167 @@ class CommandControl():
     '''Get all modules' config data/specification from configmanager.
     receive command from client and resend it to proper module.
     '''
-
-    def __init__(self, verbose = False):
+    def __init__(self, httpserver, verbose = False):
+        ''' httpserver: the http server which use the object of
+        CommandControl to communicate with other modules. '''
         self._verbose = verbose
-        self.cc = isc.cc.Session()
-        self.cc.group_subscribe('Cmd-Ctrld')
-        self.module_spec = self.get_module_specification()
-        self.config_data = self.get_config_data()
+        self._httpserver = httpserver
+        self._lock = threading.Lock()
+        self._setup_session()
+        self.modules_spec = self._get_modules_specification()
+        self._config_data = self._get_config_data_from_config_manager()
+        self._serving = True
+        self._start_msg_handle_thread()
+
+    def _setup_session(self):
+        '''Setup the session for receving the commands
+        sent from other modules. There are two sessions 
+        for cmdctl, one(self.module_cc) is used for receiving 
+        commands sent from other modules, another one (self._cc) 
+        is used to send the command from Bindctl or other tools 
+        to proper modules.''' 
+        self._cc = isc.cc.Session()
+        self._module_cc = isc.config.ModuleCCSession(SPECFILE_LOCATION,
+                                              self.config_handler,
+                                              self.command_handler)
+        self._module_name = self._module_cc.get_module_spec().get_module_name()
+        self._cmdctl_config_data = self._module_cc.get_full_config()
+        self._module_cc.start()
+    
+    def _accounts_file_check(self, filepath):
+        ''' Check whether the accounts file is valid, each row
+        should be a list with 3 items.'''
+        csvfile = None
+        errstr = None
+        try:
+            csvfile = open(filepath)
+            reader = csv.reader(csvfile)
+            for row in reader:
+                a = (row[0], row[1], row[2])
+        except (IOError, IndexError) as e:
+            errstr = 'Invalid accounts file: ' + str(e)
+        finally:
+            if csvfile:
+                csvfile.close()
 
+        return errstr
+
+    def _config_data_check(self, new_config):
+        ''' Check whether the new config data is valid or
+        not. '''
+        errstr = None
+        for key in new_config:
+            if key == 'version':
+                continue
+            elif key in ['key_file', 'cert_file']:
+                #TODO, only check whether the file exist,
+                # further check need to be done: eg. whether
+                # the private/certificate is valid.
+                path = new_config[key]
+                if not os.path.exists(path):
+                    errstr = "the file doesn't exist: " + path
+            elif key == 'accounts_file':
+                errstr = self._accounts_file_check(new_config[key])
+            else:
+                errstr = 'unknown config item: ' + key
+            
+            if errstr != None:
+                self.log_info('Fail to apply config data, ' + errstr) 
+                return ccsession.create_answer(1, errstr)
+
+        return ccsession.create_answer(0)
+
+    def config_handler(self, new_config):
+        answer = self._config_data_check(new_config)
+        rcode, val = ccsession.parse_answer(answer)
+        if rcode != 0:
+            return answer
+
+        with self._lock:
+            for key in new_config:
+                if key in self._cmdctl_config_data:
+                    self._cmdctl_config_data[key] = new_config[key]
+        return answer
+
+    def command_handler(self, command, args):
+        answer = ccsession.create_answer(0)
+        if command == ccsession.COMMAND_MODULE_SPECIFICATION_UPDATE:
+            with self._lock:
+                self.modules_spec[args[0]] = args[1]
+
+        elif command == ccsession.COMMAND_SHUTDOWN:
+            #When cmdctl get 'shutdown' command from boss, 
+            #shutdown the outer httpserver.
+            self._httpserver.shutdown()
+            self._serving = False
+
+        elif command == 'print_settings':
+            answer = ccsession.create_answer(0, self._cmdctl_config_data)
+        else:
+            answer = ccsession.create_answer(1, 'unknown command: ' + command)
+
+        return answer
+
+    def _start_msg_handle_thread(self):
+        ''' Start one thread to handle received message from msgq.'''
+        td = threading.Thread(target=self._handle_msg_from_msgq)
+        td.daemon = True
+        td.start()
+
+    def _handle_msg_from_msgq(self):
+        '''Process all the received commands with module session. '''
+        while self._serving:
+            self._module_cc.check_command() 
+ 
     def _parse_command_result(self, rcode, reply):
         '''Ignore the error reason when command rcode isn't 0, '''
         if rcode != 0:
             return {}
         return reply
 
-    def get_config_data(self):
+    def _get_config_data_from_config_manager(self):
         '''Get config data for all modules from configmanager '''
-        rcode, reply = self.send_command('ConfigManager', isc.config.ccsession.COMMAND_GET_CONFIG)
+        rcode, reply = self.send_command('ConfigManager', ccsession.COMMAND_GET_CONFIG)
         return self._parse_command_result(rcode, reply)
 
-
-    def update_config_data(self, module_name, command_name):
+    def _update_config_data(self, module_name, command_name):
         '''Get lastest config data for all modules from configmanager '''
-        if module_name == 'ConfigManager' and command_name == isc.config.ccsession.COMMAND_SET_CONFIG:
-            self.config_data = self.get_config_data()
+        if module_name == 'ConfigManager' and command_name == ccsession.COMMAND_SET_CONFIG:
+            data = self._get_config_data_from_config_manager()
+            with self._lock:
+                self._config_data = data
 
-    def get_module_specification(self):
-        rcode, reply = self.send_command('ConfigManager', isc.config.ccsession.COMMAND_GET_MODULE_SPEC)
+    def get_config_data(self):
+        with self._lock:
+            data = self._config_data
+        return data
+
+    def get_modules_spec(self):
+        with self._lock:
+            spec = self.modules_spec
+        return spec
+
+    def _get_modules_specification(self):
+        '''Get all the modules' specification files. '''
+        rcode, reply = self.send_command('ConfigManager', ccsession.COMMAND_GET_MODULE_SPEC)
         return self._parse_command_result(rcode, reply)
 
-    def handle_recv_msg(self):
-        '''Handle received message, if 'shutdown' is received, return False'''
-        (message, env) = self.cc.group_recvmsg(True)
-        command, arg = isc.config.ccsession.parse_command(message)
-        while command:
-            if command == isc.config.ccsession.COMMAND_MODULE_SPECIFICATION_UPDATE:
-                self.module_spec[arg[0]] = arg[1]
-            elif command == isc.config.ccsession.COMMAND_SHUTDOWN:
-                return False
-            (message, env) = self.cc.group_recvmsg(True)
-            command, arg = isc.config.ccsession.parse_command(message)
-        
-        return True
-    
     def send_command_with_check(self, module_name, command_name, params = None):
         '''Before send the command to modules, check if module_name, command_name
         parameters are legal according the spec file of the module.
-        Return rcode, dict.
+        Return rcode, dict. TODO, the rcode should be defined properly.
         rcode = 0: dict is the correct returned value.
         rcode > 0: dict is : { 'error' : 'error reason' }
         '''
-
         # core module ConfigManager does not have a specification file
         if module_name == 'ConfigManager':
             return self.send_command(module_name, command_name, params)
 
-        if module_name not in self.module_spec.keys():
+        specs = self.get_modules_spec()
+        if module_name not in specs.keys():
             return 1, {'error' : 'unknown module'}
        
-        spec_obj = isc.config.module_spec.ModuleSpec(self.module_spec[module_name], False)
+        spec_obj = isc.config.module_spec.ModuleSpec(specs[module_name], False)
         errors = []
         if not spec_obj.validate_command(command_name, params, errors):
             return 1, {'error': errors[0]}
@@ -281,123 +379,157 @@ class CommandControl():
 
     def send_command(self, module_name, command_name, params = None):
         '''Send the command from bindctl to proper module. '''
-        
-        errstr = 'no error'
+        errstr = 'unknown error'
         if self._verbose:
-            self.log_info('[b10-cmdctl] send command \'%s\' to %s\n' %(command_name, module_name))
-        try:
-            msg = isc.config.ccsession.create_command(command_name, params)
-            seq = self.cc.group_sendmsg(msg, module_name)
+            self.log_info("Begin send command '%s' to module '%s'" %(command_name, module_name))
+
+        if module_name == self._module_name:
+            # Process the command sent to cmdctl directly. 
+            answer = self.command_handler(command_name, params)
+        else:
+            msg = ccsession.create_command(command_name, params)
+            seq = self._cc.group_sendmsg(msg, module_name)
             #TODO, it may be blocked, msqg need to add a new interface waiting in timeout.
-            answer, env = self.cc.group_recvmsg(False, seq)
-            if answer:
-                try:
-                    rcode, arg = isc.config.ccsession.parse_answer(answer)
-                    if rcode == 0:
-                        self.update_config_data(module_name, command_name)
-                        if arg != None:
-                            return rcode, arg
-                        else:
-                            return rcode, {}
+            answer, env = self._cc.group_recvmsg(False, seq)
+
+        if self._verbose:
+            self.log_info("Finish send command '%s' to module '%s'" % (command_name, module_name))
+
+        if answer:
+            try:
+                rcode, arg = ccsession.parse_answer(answer)
+                if rcode == 0:
+                    self._update_config_data(module_name, command_name)
+                    if arg != None:
+                        return rcode, arg
                     else:
-                        # todo: exception
-                        errstr = str(answer['result'][1])
-                except isc.config.ccsession.ModuleCCSessionError as mcse:
-                    errstr = str("Error in ccsession answer:") + str(mcse)
-                    self.log_info(answer)
-        except Exception as e:
-            errstr = str(e)
-            self.log_info('\'%s\':[b10-cmdctl] fail send command \'%s\' to %s\n' % (e, command_name, module_name))
+                        return rcode, {}
+                else:
+                    # TODO: exception
+                    errstr = str(answer['result'][1])
+            except ccsession.ModuleCCSessionError as mcse:
+                errstr = str("Error in ccsession answer:") + str(mcse)
+                self.log_info(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):
+        ''' If running in source code tree, use keyfile, certificate
+        and user accounts file in source code. '''
+        if "B10_FROM_SOURCE" in os.environ:
+            sysconf_path = os.environ["B10_FROM_SOURCE"] + "/src/bin/cmdctl/"
+            accountsfile  = sysconf_path + "cmdctl-accounts.csv"
+            keyfile = sysconf_path + "cmdctl-keyfile.pem"
+            certfile = sysconf_path + "cmdctl-certfile.pem"
+            return (keyfile, certfile, accountsfile)
+
+        with self._lock:
+            keyfile = self._cmdctl_config_data.get('key_file')
+            certfile = self._cmdctl_config_data.get('cert_file')
+            accountsfile = self._cmdctl_config_data.get('accounts_file')
+
+        return (keyfile, certfile, accountsfile)
+
 class SecureHTTPServer(socketserver.ThreadingMixIn, http.server.HTTPServer):
     '''Make the server address can be reused.'''
     allow_reuse_address = True
 
-    def __init__(self, server_address, RequestHandlerClass, idle_timeout = 1200, verbose = False):
+    def __init__(self, server_address, RequestHandlerClass, 
+                 CommandControlClass,
+                 idle_timeout = 1200, verbose = False):
         '''idle_timeout: the max idle time for login'''
         http.server.HTTPServer.__init__(self, server_address, RequestHandlerClass)
         self.user_sessions = {}
         self.idle_timeout = idle_timeout
-        self.cmdctrl = CommandControl()
-        self.__is_shut_down = threading.Event()
-        self.__serving = False
+        self.cmdctl = CommandControlClass(self, verbose)
         self._verbose = verbose
-        self.user_infos = {}
-        self._read_user_info()
+        self._lock = threading.Lock()
+        self._user_infos = {}
+        self._accounts_file = None
+
+    def _create_user_info(self, accounts_file):
+        '''Read all user's name and its' salt, hashed password 
+        from accounts file.'''
+        if (self._accounts_file == accounts_file) and (len(self._user_infos) > 0): 
+            return
+
+        with self._lock:
+            self._user_infos = {}
+            csvfile = None
+            try:
+                csvfile = open(accounts_file)
+                reader = csv.reader(csvfile)
+                for row in reader:
+                    self._user_infos[row[0]] = [row[1], row[2]]
+            except (IOError, IndexError) as e:
+                self.log_info("Fail to read user database, %s" % e)                
+            finally:
+                if csvfile:
+                    csvfile.close()
+
+        self._accounts_file = accounts_file
+        if len(self._user_infos) == 0:
+            self.log_info("Fail to get user information, will deny any user")                
+         
+    def get_user_info(self, username):
+        '''Get user's salt and hashed string. If the user
+        doesn't exist, return None, or else, the list 
+        [salt, hashed password] will be returned.'''
+        with self._lock:
+            info = self._user_infos.get(username)
+        return info
 
-    def _read_user_info(self):
-        '''Read all user's name and its' password from csv file.'''
-        csvfile = None
-        try:
-            csvfile = open(USER_INFO_FILE)
-            reader = csv.reader(csvfile)
-            for row in reader:
-                self.user_infos[row[0]] = [row[1], row[2]]
-        except Exception as e:
-            self.log_info('[b10-cmdctl] Fail to read user information :\'%s\'\n' % e)                
-        finally:
-            if csvfile:
-                csvfile.close()
-        
     def save_user_session_id(self, session_id):
-        # Record user's id and login time.
+        ''' Record user's id and login time. '''
         self.user_sessions[session_id] = time.time()
         
-    def get_request(self):
-        '''Get client request socket and wrap it in SSL context. '''
-        newsocket, fromaddr = self.socket.accept()
+    def _check_key_and_cert(self, key, cert):
+        # TODO, check the content of key/certificate file 
+        if not os.path.exists(key):
+            raise CmdctlException("key file '%s' doesn't exist " % key)
+
+        if not os.path.exists(cert):
+            raise CmdctlException("certificate file '%s' doesn't exist " % cert)
+
+    def _wrap_socket_in_ssl_context(self, sock, key, cert):
         try:
-            connstream = ssl.wrap_socket(newsocket,
-                                     server_side = True,
-                                     certfile = CERTIFICATE_FILE,
-                                     keyfile = PRIVATE_KEY_FILE,
-                                     ssl_version = ssl.PROTOCOL_SSLv23)
-            return (connstream, fromaddr)
-        except ssl.SSLError as e :
-            self.log_info('[b10-cmdctl] deny client\'s invalid connection:\'%s\'\n' % e)
-            self.close_request(newsocket)
+            self._check_key_and_cert(key, cert)
+            ssl_sock = ssl.wrap_socket(sock,
+                                      server_side = True,
+                                      certfile = cert,
+                                      keyfile = key,
+                                      ssl_version = ssl.PROTOCOL_SSLv23)
+            return ssl_sock 
+        except (ssl.SSLError, CmdctlException) as err :
+            self.log_info("Deny client's connection because %s" % str(err))
+            self.close_request(sock)
             # raise socket error to finish the request
             raise socket.error
-            
+
+    def get_request(self):
+        '''Get client request socket and wrap it in SSL context. '''
+        key, cert, account_file = self.cmdctl.get_cmdctl_config_data()
+        self._create_user_info(account_file)
+        newsocket, fromaddr = self.socket.accept()
+        ssl_sock = self._wrap_socket_in_ssl_context(newsocket, key, cert)
+        return (ssl_sock, fromaddr)
 
     def get_reply_data_for_GET(self, id, module):
         '''Currently only support the following three url GET request '''
         rcode, reply = http.client.NO_CONTENT, []        
         if not module:
-            if id == 'config_data':
-               rcode, reply = http.client.OK, self.cmdctrl.config_data
-            elif id == 'module_spec':
-                rcode, reply = http.client.OK, self.cmdctrl.module_spec
+            if id == CONFIG_DATA_URL:
+                rcode, reply = http.client.OK, self.cmdctl.get_config_data()
+            elif id == MODULE_SPEC_URL:
+                rcode, reply = http.client.OK, self.cmdctl.get_modules_spec()
         
         return rcode, reply 
 
-        
-    def serve_forever(self, poll_interval = 0.5):
-        '''Start cmdctl as one tcp server. '''
-        self.__serving = True
-        self.__is_shut_down.clear()
-        while self.__serving:
-            if not self.cmdctrl.handle_recv_msg():
-                break
-
-            r, w, e = select.select([self], [], [], poll_interval)
-            if r:
-                self._handle_request_noblock()
-
-        self.__is_shut_down.set()
-    
-    def shutdown(self):
-        self.__serving = False
-        self.__is_shut_down.wait()
-
-
     def send_command_to_module(self, module_name, command_name, params):
-        return self.cmdctrl.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))
@@ -417,7 +549,8 @@ def run(addr = 'localhost', port = 8080, idle_timeout = 1200, verbose = False):
     ''' Start cmdctl as one https server. '''
     if verbose:
         sys.stdout.write("[b10-cmdctl] starting on %s port:%d\n" %(addr, port))
-    httpd = SecureHTTPServer((addr, port), SecureHTTPRequestHandler, idle_timeout, verbose)
+    httpd = SecureHTTPServer((addr, port), SecureHTTPRequestHandler, 
+                             CommandControl, idle_timeout, verbose)
     httpd.serve_forever()
 
 def check_port(option, opt_str, value, parser):
@@ -453,13 +586,12 @@ def set_cmd_options(parser):
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true", default=False,
             help="display more about what is going on")
 
-
 if __name__ == '__main__':
     try:
+        set_signal_handler()
         parser = OptionParser(version = __version__)
         set_cmd_options(parser)
         (options, args) = parser.parse_args()
-        set_signal_handler()
         run(options.addr, options.port, options.idle_timeout, options.verbose)
     except isc.cc.SessionError as se:
         sys.stderr.write("[b10-cmdctl] Error creating b10-cmdctl, "

+ 0 - 50
src/bin/cmdctl/cmdctl.spec

@@ -1,50 +0,0 @@
-{
-  "module_spec": {
-    "module_name": "Cmdctl",
-    "module_description": "Interface for command and control",
-    "config_data": [
-      {
-        "item_name": "key_file",
-        "item_type": "string",
-        "item_optional": False,
-        "item_default": 'cmdctl-keyfile.pem'
-      },
-      {
-        "item_name": "cert_file",
-        "item_type": "string",
-        "item_optional": False,
-        "item_default": 'cmdctl-certfile.pem'
-      },
-      {
-        "item_name": "accounts_file",
-        "item_type": "string",
-        "item_optional": False,
-        "item_default": 'cmdctl-accounts.csv'
-      }
-    ],
-    "commands": [
-      {
-        "command_name": "print_message",
-        "command_description": "Print the given message to stdout",
-        "command_args": [ {
-          "item_name": "message",
-          "item_type": "string",
-          "item_optional": False,
-          "item_default": ""
-        } ]
-      },
-      {
-        "command_name": "print_settings",
-        "command_description": "Print some_string and some_int to stdout",
-        "command_args": []
-      },
-      {
-        "command_name": "shutdown",
-        "command_description": "Shut down cmdctl",
-        "command_args": []
-      }
-    ]
-  }
-}
-
-

+ 2 - 0
src/bin/cmdctl/tests/Makefile.am

@@ -8,5 +8,7 @@ check-local:
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/cmdctl \
+	CMDCTL_SPEC_PATH=$(abs_top_builddir)/src/bin/cmdctl \
+	CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \
 	$(PYCOVERAGE) $(abs_srcdir)/$$pytest ; \
 	done

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

@@ -18,10 +18,10 @@
 PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
 export PYTHON_EXEC
 
-BINDCTL_TEST_PATH=@abs_top_srcdir@/src/bin/cmdctl/tests
+CMDCTL_TEST_PATH=@abs_top_srcdir@/src/bin/cmdctl/tests
 PYTHONPATH=@abs_top_srcdir@/src/bin/cmdctl
 export PYTHONPATH
 
-cd ${BINDCTL_TEST_PATH}
+cd ${CMDCTL_TEST_PATH}
 exec ${PYTHON_EXEC} -O cmdctl_test.py $*
 

+ 185 - 20
src/bin/cmdctl/tests/cmdctl_test.py

@@ -16,8 +16,17 @@
 
 import unittest
 import socket
+import tempfile
 from cmdctl import *
 
+SPEC_FILE_PATH = '..' + os.sep
+if 'CMDCTL_SPEC_PATH' in os.environ:
+    SPEC_FILE_PATH = os.environ['CMDCTL_SPEC_PATH'] + os.sep
+
+SRC_FILE_PATH = '..' + os.sep
+if 'CMDCTL_SRC_PATH' in os.environ:
+    SRC_FILE_PATH = os.environ['CMDCTL_SRC_PATH'] + os.sep
+
 # Rewrite the class for unittest.
 class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
     def __init__(self):
@@ -42,17 +51,20 @@ class MySecureHTTPRequestHandler(SecureHTTPRequestHandler):
         os.remove('tmp.file')
     
 
-class MySecureHTTPServer(SecureHTTPServer):
+class FakeSecureHTTPServer(SecureHTTPServer):
     def __init__(self):
         self.user_sessions = {}
+        self.cmdctl = FakeCommandControlForTestRequestHandler()
+        self._verbose = True 
+        self._user_infos = {}
         self.idle_timeout = 1200
-        self.cmdctrl = MyCommandControl()
-        self._verbose = False
+        self._lock = threading.Lock()
 
-class MyCommandControl(CommandControl):
+class FakeCommandControlForTestRequestHandler(CommandControl):
     def __init__(self):
-        self.config_data = {}
-        self.module_spec = {}
+        self._config_data = {}
+        self.modules_spec = {}
+        self._lock = threading.Lock()
 
     def send_command(self, mod, cmd, param):
         return 0, {}
@@ -60,14 +72,17 @@ class MyCommandControl(CommandControl):
 
 class TestSecureHTTPRequestHandler(unittest.TestCase):
     def setUp(self):
+        self.old_stdout = sys.stdout
+        sys.stdout = open(os.devnull, 'w')
         self.handler = MySecureHTTPRequestHandler()
-        self.handler.server = MySecureHTTPServer()
+        self.handler.server = FakeSecureHTTPServer()
         self.handler.server.user_sessions = {}
-        self.handler.server.user_infos = {}
+        self.handler.server._user_infos = {}
         self.handler.headers = {}
         self.handler.rfile = open("check.tmp", 'w+b')
 
     def tearDown(self):
+        sys.stdout = self.old_stdout
         self.handler.rfile.close()
         os.remove('check.tmp')
 
@@ -145,7 +160,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
     def test_check_user_name_and_pwd(self):
         self.handler.headers = {}
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['invalid username or password'])
 
     def test_check_user_name_and_pwd_1(self):
@@ -154,9 +169,9 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.headers['Content-Length'] = len
         self.handler.rfile.seek(0, 0)
 
-        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()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['password doesn\'t match'])
 
     def test_check_user_name_and_pwd_2(self):
@@ -166,7 +181,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.rfile.seek(0, 0)
 
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['invalid username or password'])
 
     def test_check_user_name_and_pwd_3(self):
@@ -176,7 +191,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.rfile.seek(0, 0)
 
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['need user name'])
 
     def test_check_user_name_and_pwd_4(self):
@@ -185,9 +200,9 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.headers['Content-Length'] = len
         self.handler.rfile.seek(0, 0)
 
-        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()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['need password'])
 
     def test_check_user_name_and_pwd_5(self):
@@ -197,7 +212,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.rfile.seek(0, 0)
 
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['user doesn\'t exist'])
 
     def test_do_POST(self):
@@ -247,8 +262,8 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
 
         self.handler.rfile.seek(0, 0)
         self.handler.path = '/module/command'
-        self.handler.server.cmdctrl.module_spec = {}
-        self.handler.server.cmdctrl.module_spec['module'] = self._gen_module_spec()
+        self.handler.server.cmdctl.modules_spec = {}
+        self.handler.server.cmdctl.modules_spec['module'] = self._gen_module_spec()
         rcode, reply = self.handler._handle_post_request()
         self.assertEqual(http.client.OK, rcode)
 
@@ -259,10 +274,160 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
 
         self.handler.rfile.seek(0, 0)
         self.handler.path = '/module/command'
-        self.handler.server.cmdctrl.module_spec = {}
-        self.handler.server.cmdctrl.module_spec['module'] = self._gen_module_spec()
+        self.handler.server.cmdctl.modules_spec = {}
+        self.handler.server.cmdctl.modules_spec['module'] = self._gen_module_spec()
         rcode, reply = self.handler._handle_post_request()
         self.assertEqual(http.client.BAD_REQUEST, rcode)
 
+class MyCommandControl(CommandControl):
+    def _get_modules_specification(self):
+        return {}
+
+    def _get_config_data_from_config_manager(self):
+        return {}
+
+    def _setup_session(self):
+        spec_file = SPEC_FILE_PATH + 'cmdctl.spec'
+        module_spec = isc.config.module_spec_from_file(spec_file)
+        config = isc.config.config_data.ConfigData(module_spec)
+        self._module_name = 'Cmdctl'
+        self._cmdctl_config_data = config.get_full_config()
+
+    def _handle_msg_from_msgq(self): 
+        pass
+
+class TestCommandControl(unittest.TestCase):
+
+    def setUp(self):
+        self.old_stdout = sys.stdout
+        sys.stdout = open(os.devnull, 'w')
+        self.cmdctl = MyCommandControl(None, True)
+   
+    def tearDown(self):
+        sys.stdout = self.old_stdout
+
+    def _check_config(self, cmdctl):
+        key, cert, account = cmdctl.get_cmdctl_config_data()
+        self.assertIsNotNone(key)
+        self.assertIsNotNone(cert)
+        self.assertIsNotNone(account)
+
+    def test_get_cmdctl_config_data(self):
+        old_env = os.environ
+        if 'B10_FROM_SOURCE' in os.environ:
+            del os.environ['B10_FROM_SOURCE']
+        self.cmdctl.get_cmdctl_config_data() 
+        self._check_config(self.cmdctl)
+        os.environ = old_env
+
+        old_env = os.environ
+        os.environ['B10_FROM_SOURCE'] = '../'
+        self._check_config(self.cmdctl)
+        os.environ = old_env
+    
+    def test_parse_command_result(self):
+        self.assertEqual({}, self.cmdctl._parse_command_result(1, {'error' : 1}))
+        self.assertEqual({'a': 1}, self.cmdctl._parse_command_result(0, {'a' : 1}))
+
+    def _check_answer(self, answer, rcode_, msg_):
+        rcode, msg = ccsession.parse_answer(answer)
+        self.assertEqual(rcode, rcode_)
+        self.assertEqual(msg, msg_)
+
+    def test_command_handler(self):
+        answer = self.cmdctl.command_handler('unknown-command', None)
+        self._check_answer(answer, 1, 'unknown command: unknown-command')
+
+        answer = self.cmdctl.command_handler('print_settings', None)
+        rcode, msg = ccsession.parse_answer(answer)
+        self.assertEqual(rcode, 0)
+        self.assertTrue(msg != None)
+
+    def test_check_config_handler(self):
+        answer = self.cmdctl.config_handler({'non-exist': 123})
+        self._check_answer(answer, 1, 'unknown config item: non-exist')
+
+        old_env = os.environ
+        os.environ['B10_FROM_SOURCE'] = '../'
+        self._check_config(self.cmdctl)
+        os.environ = old_env
+
+        answer = self.cmdctl.config_handler({'key_file': '/user/non-exist_folder'})
+        self._check_answer(answer, 1, "the file doesn't exist: /user/non-exist_folder")
+
+        answer = self.cmdctl.config_handler({'cert_file': '/user/non-exist_folder'})
+        self._check_answer(answer, 1, "the file doesn't exist: /user/non-exist_folder")
+
+        answer = self.cmdctl.config_handler({'accounts_file': '/user/non-exist_folder'})
+        self._check_answer(answer, 1, 
+                "Invalid accounts file: [Errno 2] No such file or directory: '/user/non-exist_folder'")
+
+        # Test with invalid accounts file
+        file_name = 'tmp.account.file'
+        temp_file = open(file_name, 'w')
+        writer = csv.writer(temp_file)
+        writer.writerow(['a', 'b'])
+        temp_file.close()
+        answer = self.cmdctl.config_handler({'accounts_file': file_name})
+        self._check_answer(answer, 1, "Invalid accounts file: list index out of range")
+        os.remove(file_name)
+    
+    def test_send_command(self):
+        rcode, value = self.cmdctl.send_command('Cmdctl', 'print_settings', None)
+        self.assertEqual(rcode, 0)
+
+class MySecureHTTPServer(SecureHTTPServer):
+    def server_bind(self):
+        pass
+
+class TestSecureHTTPServer(unittest.TestCase):
+    def setUp(self):
+        self.old_stdout = sys.stdout
+        sys.stdout = open(os.devnull, 'w')
+        self.server = MySecureHTTPServer(('localhost', 8080), 
+                                         MySecureHTTPRequestHandler,
+                                         MyCommandControl, verbose=True)
+
+    def tearDown(self):
+        sys.stdout = self.old_stdout
+
+    def test_create_user_info(self):
+        self.server._create_user_info('/local/not-exist')
+        self.assertEqual(0, len(self.server._user_infos))
+
+        self.server._create_user_info(SRC_FILE_PATH + 'cmdctl-accounts.csv')
+        self.assertEqual(1, len(self.server._user_infos))
+        self.assertTrue('root' in self.server._user_infos)
+
+    def test_check_key_and_cert(self):
+        self.assertRaises(CmdctlException, self.server._check_key_and_cert,
+                         '/local/not-exist', 'cmdctl-keyfile.pem')
+
+        self.server._check_key_and_cert(SRC_FILE_PATH + 'cmdctl-keyfile.pem',
+                                        SRC_FILE_PATH + 'cmdctl-certfile.pem')
+
+    def test_wrap_sock_in_ssl_context(self):
+        sock = socket.socket()
+        self.assertRaises(socket.error, 
+                          self.server._wrap_socket_in_ssl_context,
+                          sock, 
+                          '../cmdctl-keyfile',
+                          '../cmdctl-certfile')
+
+        sock1 = socket.socket()
+        self.server._wrap_socket_in_ssl_context(sock1, 
+                          SRC_FILE_PATH + 'cmdctl-keyfile.pem',
+                          SRC_FILE_PATH + 'cmdctl-certfile.pem')
+
+class TestFuncNotInClass(unittest.TestCase):
+    def test_check_port(self):
+        self.assertRaises(OptionValueError, check_port, None, 'port', -1, None)        
+        self.assertRaises(OptionValueError, check_port, None, 'port', 65536, None)        
+        self.assertRaises(OptionValueError, check_addr, None, 'ipstr', 'a.b.d', None)        
+        self.assertRaises(OptionValueError, check_addr, None, 'ipstr', '1::0:a.b', None)        
+
+
 if __name__== "__main__":
     unittest.main()
+
+

+ 2 - 0
src/bin/xfrout/xfrout.py.in

@@ -181,6 +181,8 @@ class XfroutSession(BaseRequestHandler):
         zone_name = self._get_query_zone_name(msg)
         rcode_ = self._check_xfrout_available(zone_name)
         if rcode_ != Rcode.NOERROR():
+            self._log.log_message("info", "transfer of '%s/IN' failed: %s",
+                                  zone_name, rcode_.to_text())
             return self. _reply_query_with_error_rcode(msg, sock, rcode_)
 
         try:

+ 1 - 1
src/lib/python/isc/config/cfgmgr.py

@@ -347,7 +347,7 @@ class ConfigManager:
         # passes both specification and commands at once
         spec_update = ccsession.create_command(ccsession.COMMAND_MODULE_SPECIFICATION_UPDATE,
                                                [ spec.get_module_name(), spec.get_full_spec() ])
-        self.cc.group_sendmsg(spec_update, "Cmd-Ctrld")
+        self.cc.group_sendmsg(spec_update, "Cmdctl")
         return ccsession.create_answer(0)
 
     def handle_msg(self, msg):

+ 2 - 2
src/lib/python/isc/config/tests/cfgmgr_test.py

@@ -271,9 +271,9 @@ class TestConfigManager(unittest.TestCase):
         # the name here is actually wrong (and hardcoded), but needed in the current version
         # TODO: fix that
         #self.assertEqual({'specification_update': [ self.name, self.spec ] },
-        #                 self.fake_session.get_message("Cmd-Ctrld", None))
+        #                 self.fake_session.get_message("Cmdctl", None))
         #self.assertEqual({'commands_update': [ self.name, self.commands ] },
-        #                 self.fake_session.get_message("Cmd-Ctrld", None))
+        #                 self.fake_session.get_message("Cmdctl", None))
 
         self._handle_msg_helper({ "command": 
                                   ["shutdown"]