Merge branch 'trac811_new'

src/bin/xfrin/tests/xfrin_test.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2009  Internet Systems Consortium.
+# Copyright (C) 2009-2011  Internet Systems Consortium.
 #
 # Permission to use, copy, modify, and distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -20,8 +20,10 @@ from xfrin import *
 #
 # Commonly used (mostly constant) test parameters
 #
-TEST_ZONE_NAME = "example.com"
+TEST_ZONE_NAME_STR = "example.com."
+TEST_ZONE_NAME = Name(TEST_ZONE_NAME_STR)
 TEST_RRCLASS = RRClass.IN()
+TEST_RRCLASS_STR = 'IN'
 TEST_DB_FILE = 'db_file'
 TEST_MASTER_IPV4_ADDRESS = '127.0.0.1'
 TEST_MASTER_IPV4_ADDRINFO = (socket.AF_INET, socket.SOCK_STREAM,
@@ -40,12 +42,12 @@ TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==")
 soa_rdata = Rdata(RRType.SOA(), TEST_RRCLASS,
                   'master.example.com. admin.example.com ' +
                   '1234 3600 1800 2419200 7200')
-soa_rrset = RRset(Name(TEST_ZONE_NAME), TEST_RRCLASS, RRType.SOA(),
+soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(),
                   RRTTL(3600))
 soa_rrset.add_rdata(soa_rdata)
-example_axfr_question = Question(Name(TEST_ZONE_NAME), TEST_RRCLASS,
+example_axfr_question = Question(TEST_ZONE_NAME, TEST_RRCLASS,
                                  RRType.AXFR())
-example_soa_question = Question(Name(TEST_ZONE_NAME), TEST_RRCLASS,
+example_soa_question = Question(TEST_ZONE_NAME, TEST_RRCLASS,
                                  RRType.SOA())
 default_questions = [example_axfr_question]
 default_answers = [soa_rrset]
@@ -60,6 +62,13 @@ def strip_mutable_tsig_data(data):
     # Time Signed.
     return data[0:-32] + data[-26:-22] + data[-6:]
 
+class MockCC():
+    def get_default_value(self, identifier):
+        if identifier == "zones/master_port":
+            return TEST_MASTER_PORT
+        if identifier == "zones/class":
+            return TEST_RRCLASS_STR
+
 class MockXfrin(Xfrin):
     # This is a class attribute of a callable object that specifies a non
     # default behavior triggered in _cc_check_command().  Specific test methods
@@ -69,7 +78,8 @@ class MockXfrin(Xfrin):
     check_command_hook = None
 
     def _cc_setup(self):
-        self._tsig_key_str = None
+        self._tsig_key = None
+        self._module_cc = MockCC()
         pass
 
     def _get_db_file(self):
@@ -80,6 +90,16 @@ class MockXfrin(Xfrin):
         if MockXfrin.check_command_hook:
             MockXfrin.check_command_hook()
 
+    def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo,
+                    tsig_key, check_soa=True):
+        # store some of the arguments for verification, then call this
+        # method in the superclass
+        self.xfrin_started_master_addr = master_addrinfo[2][0]
+        self.xfrin_started_master_port = master_addrinfo[2][1]
+        return Xfrin.xfrin_start(self, zone_name, rrclass, db_file,
+                                 master_addrinfo, tsig_key,
+                                 check_soa)
+
 class MockXfrinConnection(XfrinConnection):
     def __init__(self, sock_map, zone_name, rrclass, db_file, shutdown_event,
                  master_addr):
@@ -450,7 +470,8 @@ class TestXfrin(unittest.TestCase):
         sys.stderr = open(os.devnull, 'w')
         self.xfr = MockXfrin()
         self.args = {}
-        self.args['zone_name'] = TEST_ZONE_NAME
+        self.args['zone_name'] = TEST_ZONE_NAME_STR
+        self.args['class'] = TEST_RRCLASS_STR
         self.args['port'] = TEST_MASTER_PORT
         self.args['master'] = TEST_MASTER_IPV4_ADDRESS
         self.args['db_file'] = TEST_DB_FILE
@@ -464,7 +485,8 @@ class TestXfrin(unittest.TestCase):
         return self.xfr._parse_zone_name_and_class(self.args)
 
     def _do_parse_master_port(self):
-        return self.xfr._parse_master_and_port(self.args)
+        name, rrclass = self._do_parse_zone_name_class()
+        return self.xfr._parse_master_and_port(self.args, name, rrclass)
 
     def test_parse_cmd_params(self):
         name, rrclass = self._do_parse_zone_name_class()
@@ -492,7 +514,7 @@ class TestXfrin(unittest.TestCase):
 
     def test_parse_cmd_params_bogusclass(self):
         self.args['zone_class'] = 'XXX'
-        self.assertRaises(XfrinException, self._do_parse_zone_name_class)
+        self.assertRaises(XfrinZoneInfoException, self._do_parse_zone_name_class)
 
     def test_parse_cmd_params_nozone(self):
         # zone name is mandatory.
@@ -502,8 +524,7 @@ class TestXfrin(unittest.TestCase):
     def test_parse_cmd_params_nomaster(self):
         # master address is mandatory.
         del self.args['master']
-        master_addrinfo = self._do_parse_master_port()
-        self.assertEqual(master_addrinfo[2][0], DEFAULT_MASTER)
+        self.assertRaises(XfrinException, self._do_parse_master_port)
 
     def test_parse_cmd_params_bad_ip4(self):
         self.args['master'] = '3.3.3.3.3'
@@ -533,6 +554,77 @@ class TestXfrin(unittest.TestCase):
     def test_command_handler_retransfer(self):
         self.assertEqual(self.xfr.command_handler("retransfer",
                                                   self.args)['result'][0], 0)
+        self.assertEqual(self.args['master'], self.xfr.xfrin_started_master_addr)
+        self.assertEqual(int(self.args['port']), self.xfr.xfrin_started_master_port)
+
+    def test_command_handler_retransfer_short_command1(self):
+        # try it when only specifying the zone name (of unknown zone)
+        # this should fail because master address is not specified.
+        short_args = {}
+        short_args['zone_name'] = TEST_ZONE_NAME_STR
+        self.assertEqual(self.xfr.command_handler("retransfer",
+                                                  short_args)['result'][0], 1)
+
+    def test_command_handler_retransfer_short_command2(self):
+        # try it when only specifying the zone name (of known zone)
+        short_args = {}
+        short_args['zone_name'] = TEST_ZONE_NAME_STR
+
+        zones = { 'zones': [
+                  { 'name': TEST_ZONE_NAME_STR,
+                    'master_addr': TEST_MASTER_IPV4_ADDRESS,
+                    'master_port': TEST_MASTER_PORT
+                  }
+                ]}
+        self.xfr.config_handler(zones)
+        self.assertEqual(self.xfr.command_handler("retransfer",
+                                                  short_args)['result'][0], 0)
+        self.assertEqual(TEST_MASTER_IPV4_ADDRESS,
+                         self.xfr.xfrin_started_master_addr)
+        self.assertEqual(int(TEST_MASTER_PORT),
+                         self.xfr.xfrin_started_master_port)
+
+    def test_command_handler_retransfer_short_command3(self):
+        # try it when only specifying the zone name (of known zone)
+        short_args = {}
+        # test it without the trailing root dot
+        short_args['zone_name'] = TEST_ZONE_NAME_STR[:-1]
+
+        zones = { 'zones': [
+                  { 'name': TEST_ZONE_NAME_STR,
+                    'master_addr': TEST_MASTER_IPV4_ADDRESS,
+                    'master_port': TEST_MASTER_PORT
+                  }
+                ]}
+        self.xfr.config_handler(zones)
+        self.assertEqual(self.xfr.command_handler("retransfer",
+                                                  short_args)['result'][0], 0)
+        self.assertEqual(TEST_MASTER_IPV4_ADDRESS,
+                         self.xfr.xfrin_started_master_addr)
+        self.assertEqual(int(TEST_MASTER_PORT),
+                         self.xfr.xfrin_started_master_port)
+
+    def test_command_handler_retransfer_short_command4(self):
+        # try it when only specifying the zone name (of known zone, with
+        # different case)
+        short_args = {}
+
+        # swap the case of the zone name in our command
+        short_args['zone_name'] = TEST_ZONE_NAME_STR.swapcase()
+
+        zones = { 'zones': [
+                  { 'name': TEST_ZONE_NAME_STR,
+                    'master_addr': TEST_MASTER_IPV4_ADDRESS,
+                    'master_port': TEST_MASTER_PORT
+                  }
+                ]}
+        self.xfr.config_handler(zones)
+        self.assertEqual(self.xfr.command_handler("retransfer",
+                                                  short_args)['result'][0], 0)
+        self.assertEqual(TEST_MASTER_IPV4_ADDRESS,
+                         self.xfr.xfrin_started_master_addr)
+        self.assertEqual(int(TEST_MASTER_PORT),
+                         self.xfr.xfrin_started_master_port)
 
     def test_command_handler_retransfer_badcommand(self):
         self.args['master'] = 'invalid'
@@ -540,13 +632,15 @@ class TestXfrin(unittest.TestCase):
                                                   self.args)['result'][0], 1)
 
     def test_command_handler_retransfer_quota(self):
+        self.args['master'] = TEST_MASTER_IPV4_ADDRESS
+
         for i in range(self.xfr._max_transfers_in - 1):
-            self.xfr.recorder.increment(str(i) + TEST_ZONE_NAME)
+            self.xfr.recorder.increment(Name(str(i) + TEST_ZONE_NAME_STR))
         # there can be one more outstanding transfer.
         self.assertEqual(self.xfr.command_handler("retransfer",
                                                   self.args)['result'][0], 0)
         # make sure the # xfrs would excceed the quota
-        self.xfr.recorder.increment(str(self.xfr._max_transfers_in) + TEST_ZONE_NAME)
+        self.xfr.recorder.increment(Name(str(self.xfr._max_transfers_in) + TEST_ZONE_NAME_STR))
         # this one should fail
         self.assertEqual(self.xfr.command_handler("retransfer",
                                                   self.args)['result'][0], 1)
@@ -570,14 +664,43 @@ class TestXfrin(unittest.TestCase):
         self.args['master'] = TEST_MASTER_IPV6_ADDRESS
         self.assertEqual(self.xfr.command_handler("refresh",
                                                   self.args)['result'][0], 0)
+        self.assertEqual(TEST_MASTER_IPV6_ADDRESS,
+                         self.xfr.xfrin_started_master_addr)
+        self.assertEqual(int(TEST_MASTER_PORT),
+                         self.xfr.xfrin_started_master_port)
 
     def test_command_handler_notify(self):
         # at this level, refresh is no different than retransfer.
         self.args['master'] = TEST_MASTER_IPV6_ADDRESS
-        # ...but right now we disable the feature due to security concerns.
+        # ...but the zone is unknown so this would return an error
+        self.assertEqual(self.xfr.command_handler("notify",
+                                                  self.args)['result'][0], 1)
+
+    def test_command_handler_notify_known_zone(self):
+        # try it with a known zone
+        self.args['master'] = TEST_MASTER_IPV6_ADDRESS
+
+        # but use a different address in the actual command
+        zones = { 'zones': [
+                  { 'name': TEST_ZONE_NAME_STR,
+                    'master_addr': TEST_MASTER_IPV4_ADDRESS,
+                    'master_port': TEST_MASTER_PORT
+                  }
+                ]}
+        self.xfr.config_handler(zones)
         self.assertEqual(self.xfr.command_handler("notify",
                                                   self.args)['result'][0], 0)
 
+        # and see if we used the address from the command, and not from
+        # the config
+        # This is actually NOT the address given in the command, which
+        # would at this point not make sense, see the TODO in
+        # xfrin.py.in Xfrin.command_handler())
+        self.assertEqual(TEST_MASTER_IPV4_ADDRESS,
+                         self.xfr.xfrin_started_master_addr)
+        self.assertEqual(int(TEST_MASTER_PORT),
+                         self.xfr.xfrin_started_master_port)
+
     def test_command_handler_unknown(self):
         self.assertEqual(self.xfr.command_handler("xxx", None)['result'][0], 1)
 
@@ -586,20 +709,145 @@ class TestXfrin(unittest.TestCase):
         self.assertEqual(self.xfr.config_handler({'transfers_in': 3})['result'][0], 0)
         self.assertEqual(self.xfr._max_transfers_in, 3)
 
-    def test_command_handler_masters(self):
-        master_info = {'master_addr': '1.1.1.1', 'master_port':53}
-        self.assertEqual(self.xfr.config_handler(master_info)['result'][0], 0)
-
-        master_info = {'master_addr': '1111.1.1.1', 'master_port':53 }
-        self.assertEqual(self.xfr.config_handler(master_info)['result'][0], 1)
-
-        master_info = {'master_addr': '2.2.2.2', 'master_port':530000 }
-        self.assertEqual(self.xfr.config_handler(master_info)['result'][0], 1)
-
-        master_info = {'master_addr': '2.2.2.2', 'master_port':53 } 
-        self.xfr.config_handler(master_info)
-        self.assertEqual(self.xfr._master_addr, '2.2.2.2')
-        self.assertEqual(self.xfr._master_port, 53)
+    def _check_zones_config(self, config_given):
+        if 'transfers_in' in config_given:
+            self.assertEqual(config_given['transfers_in'],
+                             self.xfr._max_transfers_in)
+        for zone_config in config_given['zones']:
+            zone_name = zone_config['name']
+            zone_info = self.xfr._get_zone_info(Name(zone_name), RRClass.IN())
+            self.assertEqual(str(zone_info.master_addr), zone_config['master_addr'])
+            self.assertEqual(zone_info.master_port, zone_config['master_port'])
+            if 'tsig_key' in zone_config:
+                self.assertEqual(zone_info.tsig_key.to_text(), TSIGKey(zone_config['tsig_key']).to_text())
+            else:
+                self.assertIsNone(zone_info.tsig_key)
+
+    def test_command_handler_zones(self):
+        config1 = { 'transfers_in': 3,
+                   'zones': [
+                   { 'name': 'test.example.',
+                    'master_addr': '192.0.2.1',
+                    'master_port': 53
+                   }
+                 ]}
+        self.assertEqual(self.xfr.config_handler(config1)['result'][0], 0)
+        self._check_zones_config(config1)
+
+        config2 = { 'transfers_in': 4,
+                   'zones': [
+                   { 'name': 'test.example.',
+                    'master_addr': '192.0.2.2',
+                    'master_port': 53,
+                    'tsig_key': "example.com:SFuWd/q99SzF8Yzd1QbB9g=="
+                   }
+                 ]}
+        self.assertEqual(self.xfr.config_handler(config2)['result'][0], 0)
+        self._check_zones_config(config2)
+
+        # test that configuring the zone multiple times fails
+        zones = { 'transfers_in': 5,
+                  'zones': [
+                  { 'name': 'test.example.',
+                    'master_addr': '192.0.2.1',
+                    'master_port': 53
+                  },
+                  { 'name': 'test.example.',
+                    'master_addr': '192.0.2.2',
+                    'master_port': 53
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
+
+        zones = { 'zones': [
+                  { 'name': 'test.example.',
+                    'master_addr': '192.0.2.3',
+                    'master_port': 53,
+                    'class': 'BADCLASS'
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        self._check_zones_config(config2)
+
+        zones = { 'zones': [
+                  { 'master_addr': '192.0.2.4',
+                    'master_port': 53
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
+
+        zones = { 'zones': [
+                  { 'name': 'bad..zone.',
+                    'master_addr': '192.0.2.5',
+                    'master_port': 53
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
+
+        zones = { 'zones': [
+                  { 'name': '',
+                    'master_addr': '192.0.2.6',
+                    'master_port': 53
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
+
+        zones = { 'zones': [
+                  { 'name': 'test.example',
+                    'master_addr': 'badaddress',
+                    'master_port': 53
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
+
+        zones = { 'zones': [
+                  { 'name': 'test.example',
+                    'master_addr': '192.0.2.7',
+                    'master_port': 'bad_port'
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
+
+        zones = { 'zones': [
+                  { 'name': 'test.example',
+                    'master_addr': '192.0.2.7',
+                    'master_port': 53,
+                    # using a bad TSIG key spec
+                    'tsig_key': "bad..example.com:SFuWd/q99SzF8Yzd1QbB9g=="
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
+
+        # let's also add a zone that is correct too, and make sure
+        # that the new config is not partially taken
+        zones = { 'zones': [
+                  { 'name': 'test.example.',
+                    'master_addr': '192.0.2.8',
+                    'master_port': 53
+                  },
+                  { 'name': 'test2.example.',
+                    'master_addr': '192.0.2.9',
+                    'master_port': 53,
+                    'tsig_key': 'badkey'
+                  }
+                ]}
+        self.assertEqual(self.xfr.config_handler(zones)['result'][0], 1)
+        # since this has failed, we should still have the previous config
+        self._check_zones_config(config2)
 
 
 def raise_interrupt():

src/bin/xfrin/xfrin.py.in

@@ -1,6 +1,6 @@
 #!@PYTHON@
 
-# Copyright (C) 2010  Internet Systems Consortium.
+# Copyright (C) 2009-2011  Internet Systems Consortium.
 #
 # Permission to use, copy, modify, and distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -56,26 +56,66 @@ XFROUT_MODULE_NAME = 'Xfrout'
 ZONE_MANAGER_MODULE_NAME = 'Zonemgr'
 REFRESH_FROM_ZONEMGR = 'refresh_from_zonemgr'
 ZONE_XFRIN_FAILED = 'zone_xfrin_failed'
+
+# These two default are currently hard-coded. For config this isn't
+# necessary, but we need these defaults for optional command arguments
+# (TODO: have similar support to get default values for command
+# arguments as we do for config options)
+DEFAULT_MASTER_PORT = 53
+DEFAULT_ZONE_CLASS = RRClass.IN()
+
 __version__ = 'BIND10'
 # define xfrin rcode
 XFRIN_OK = 0
 XFRIN_FAIL = 1
 
-DEFAULT_MASTER_PORT = '53'
-DEFAULT_MASTER = '127.0.0.1'
-
 def log_error(msg):
     sys.stderr.write("[b10-xfrin] %s\n" % str(msg))
 
 class XfrinException(Exception):
     pass
 
+class XfrinZoneInfoException(Exception):
+    """This exception is raised if there is an error in the given
+       configuration (part), or when a command does not have a required
+       argument or has bad arguments, for instance when the zone's master
+       address is not a valid IP address, when the zone does not
+       have a name, or when multiple settings are given for the same
+       zone."""
+    pass
+
+def _check_zone_name(zone_name_str):
+    """Checks if the given zone name is a valid domain name, and returns
+    it as a Name object. Raises an XfrinException if it is not."""
+    try:
+        # In the _zones dict, part of the key is the zone name,
+        # but due to a limitation in the Name class, we
+        # cannot directly use it as a dict key, and we use to_text()
+        #
+        # Downcase the name here for that reason.
+        return Name(zone_name_str, True)
+    except (EmptyLabel, TooLongLabel, BadLabelType, BadEscape,
+            TooLongName, IncompleteName) as ne:
+        raise XfrinZoneInfoException("bad zone name: " + zone_name_str + " (" + str(ne) + ")")
+
+def _check_zone_class(zone_class_str):
+    """If the given argument is a string: checks if the given class is
+       a valid one, and returns an RRClass object if so.
+       Raises XfrinZoneInfoException if not.
+       If it is None, this function returns the default RRClass.IN()"""
+    if zone_class_str is None:
+        return DEFAULT_ZONE_CLASS
+    try:
+        return RRClass(zone_class_str)
+    except InvalidRRClass as irce:
+        raise XfrinZoneInfoException("bad zone class: " + zone_class_str + " (" + str(irce) + ")")
+
 class XfrinConnection(asyncore.dispatcher):
     '''Do xfrin in this class. '''
 
     def __init__(self,
                  sock_map, zone_name, rrclass, db_file, shutdown_event,
-                 master_addrinfo, tsig_key_str = None, verbose = False,
+                 master_addrinfo, tsig_key = None, verbose = False,
                  idle_timeout = 60):
         ''' idle_timeout: max idle time for read data from socket.
             db_file: specify the data source file.
@@ -95,8 +135,8 @@ class XfrinConnection(asyncore.dispatcher):
         self._verbose = verbose
         self._master_address = master_addrinfo[2]
         self._tsig_ctx = None
-        if tsig_key_str is not None:
-            self._tsig_ctx = TSIGContext(TSIGKey(tsig_key_str))
+        if tsig_key is not None:
+            self._tsig_ctx = TSIGContext(tsig_key)
 
     def connect_to_master(self):
         '''Connect to master in TCP.'''
@@ -333,12 +373,12 @@ class XfrinConnection(asyncore.dispatcher):
 
 def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
                   shutdown_event, master_addrinfo, check_soa, verbose,
-                  tsig_key_str):
+                  tsig_key):
     xfrin_recorder.increment(zone_name)
     sock_map = {}
     conn = XfrinConnection(sock_map, zone_name, rrclass, db_file,
                            shutdown_event, master_addrinfo,
-                           tsig_key_str, verbose)
+                           tsig_key, verbose)
     ret = XFRIN_FAIL
     if conn.connect_to_master():
         ret = conn.do_xfrin(check_soa)
@@ -378,12 +418,100 @@ class XfrinRecorder:
         self._lock.release()
         return ret
 
+class ZoneInfo:
+    def __init__(self, config_data, module_cc):
+        """Creates a zone_info with the config data element as
+           specified by the 'zones' list in xfrin.spec. Module_cc is
+           needed to get the defaults from the specification"""
+        self._module_cc = module_cc
+        self.set_name(config_data.get('name'))
+        self.set_master_addr(config_data.get('master_addr'))
+
+        self.set_master_port(config_data.get('master_port'))
+        self.set_zone_class(config_data.get('class'))
+        self.set_tsig_key(config_data.get('tsig_key'))
+
+    def set_name(self, name_str):
+        """Set the name for this zone given a name string.
+           Raises XfrinZoneInfoException if name_str is None or if it
+           cannot be parsed."""
+        if name_str is None:
+            raise XfrinZoneInfoException("Configuration zones list "
+                                         "element does not contain "
+                                         "'name' attribute")
+        else:
+            self.name = _check_zone_name(name_str)
+
+    def set_master_addr(self, master_addr_str):
+        """Set the master address for this zone given an IP address
+           string. Raises XfrinZoneInfoException if master_addr_str is
+           None or if it cannot be parsed."""
+        if master_addr_str is None:
+            raise XfrinZoneInfoException("master address missing from config data")
+        else:
+            try:
+                self.master_addr = isc.net.parse.addr_parse(master_addr_str)
+            except ValueError:
+                errmsg = "bad format for zone's master: " + master_addr_str
+                log_error(errmsg)
+                raise XfrinZoneInfoException(errmsg)
+
+    def set_master_port(self, master_port_str):
+        """Set the master port given a port number string. If
+           master_port_str is None, the default from the specification
+           for this module will be used. Raises XfrinZoneInfoException if
+           the string contains an invalid port number"""
+        if master_port_str is None:
+            self.master_port = self._module_cc.get_default_value("zones/master_port")
+        else:
+            try:
+                self.master_port = isc.net.parse.port_parse(master_port_str)
+            except ValueError:
+                errmsg = "bad format for zone's master port: " + master_port_str
+                log_error(errmsg)
+                raise XfrinZoneInfoException(errmsg)
+
+    def set_zone_class(self, zone_class_str):
+        """Set the zone class given an RR class str (e.g. "IN"). If
+           zone_class_str is None, it will default to what is specified
+           in the specification file for this module. Raises
+           XfrinZoneInfoException if the string cannot be parsed."""
+        # TODO: remove _str
+        self.class_str = zone_class_str or self._module_cc.get_default_value("zones/class")
+        if zone_class_str == None:
+            #TODO rrclass->zone_class
+            self.rrclass = RRClass(self._module_cc.get_default_value("zones/class"))
+        else:
+            try:
+                self.rrclass = RRClass(zone_class_str)
+            except InvalidRRClass:
+                errmsg = "invalid zone class: " + zone_class_str
+                log_error(errmsg)
+                raise XfrinZoneInfoException(errmsg)
+
+    def set_tsig_key(self, tsig_key_str):
+        """Set the tsig_key for this zone, given a TSIG key string
+           representation. If tsig_key_str is None, no TSIG key will
+           be set. Raises XfrinZoneInfoException if tsig_key_str cannot
+           be parsed."""
+        if tsig_key_str is None:
+            self.tsig_key = None
+        else:
+            try:
+                self.tsig_key = TSIGKey(tsig_key_str)
+            except InvalidParameter as ipe:
+                errmsg = "bad TSIG key string: " + tsig_key_str
+                log_error(errmsg)
+                raise XfrinZoneInfoException(errmsg)
+
+    def get_master_addr_info(self):
+        return (self.master_addr.family, socket.SOCK_STREAM,
+                (str(self.master_addr), self.master_port))
+
 class Xfrin:
     def __init__(self, verbose = False):
         self._max_transfers_in = 10
-        #TODO, this is the temp way to set the zone's master.
-        self._master_addr = DEFAULT_MASTER
-        self._master_port = DEFAULT_MASTER_PORT
+        self._zones = {}
         self._cc_setup()
         self.recorder = XfrinRecorder()
         self._shutdown_event = threading.Event()
@@ -402,10 +530,7 @@ class Xfrin:
                                               self.command_handler)
         self._module_cc.start()
         config_data = self._module_cc.get_full_config()
-        self._max_transfers_in = config_data.get("transfers_in")
-        self._master_addr = config_data.get('master_addr') or self._master_addr
-        self._master_port = config_data.get('master_port') or self._master_port
-        self._tsig_key_str = config_data.get('tsig_key') or None
+        self.config_handler(config_data)
 
     def _cc_check_command(self):
         '''This is a straightforward wrapper for cc.check_command,
@@ -413,22 +538,42 @@ class Xfrin:
         of unit tests.'''
         self._module_cc.check_command(False)
 
+    def _get_zone_info(self, name, rrclass):
+        """Returns the ZoneInfo object containing the configured data
+           for the given zone name. If the zone name did not have any
+           data, returns None"""
+        return self._zones.get((name.to_text(), rrclass.to_text()))
+
+    def _add_zone_info(self, zone_info):
+        """Add the zone info. Raises a XfrinZoneInfoException if a zone
+           with the same name and class is already configured"""
+        key = (zone_info.name.to_text(), zone_info.class_str)
+        if key in self._zones:
+            raise XfrinZoneInfoException("zone " + str(key) +
+                                       " configured multiple times")
+        self._zones[key] = zone_info
+
+    def _clear_zone_info(self):
+        self._zones = {}
+
     def config_handler(self, new_config):
+        # backup all config data (should there be a problem in the new
+        # data)
+        old_max_transfers_in = self._max_transfers_in
+        old_zones = self._zones
+
         self._max_transfers_in = new_config.get("transfers_in") or self._max_transfers_in
-        self._tsig_key_str = new_config.get('tsig_key') or None
-        if ('master_addr' in new_config) or ('master_port' in new_config):
-            # User should change the port and address together.
-            try:
-                addr = new_config.get('master_addr') or self._master_addr
-                port = new_config.get('master_port') or self._master_port
-                isc.net.parse.addr_parse(addr)
-                isc.net.parse.port_parse(port)
-                self._master_addr = addr
-                self._master_port = port
-            except ValueError:
-                errmsg = "bad format for zone's master: " + str(new_config)
-                log_error(errmsg)
-                return create_answer(1, errmsg)
+
+        if 'zones' in new_config:
+            self._clear_zone_info()
+            for zone_config in new_config.get('zones'):
+                try:
+                    zone_info = ZoneInfo(zone_config, self._module_cc)
+                    self._add_zone_info(zone_info)
+                except XfrinZoneInfoException as xce:
+                    self._zones = old_zones
+                    self._max_transfers_in = old_max_transfers_in
+                    return create_answer(1, str(xce))
 
         return create_answer(0)
 
@@ -453,28 +598,43 @@ class Xfrin:
                 # notify command maybe has the parameters which
                 # specify the notifyfrom address and port, according the RFC1996, zone
                 # transfer should starts first from the notifyfrom, but now, let 'TODO' it.
+                # (using the value now, while we can only set one master address, would be
+                # a security hole. Once we add the ability to have multiple master addresses,
+                # we should check if it matches one of them, and then use it.)
                 (zone_name, rrclass) = self._parse_zone_name_and_class(args)
-                (master_addr) = build_addr_info(self._master_addr, self._master_port)
-                ret = self.xfrin_start(zone_name,
-                                       rrclass,
-                                       self._get_db_file(),
-                                       master_addr,
-                                       self._tsig_key_str,
-                                       True)
-                answer = create_answer(ret[0], ret[1])
+                zone_info = self._get_zone_info(zone_name, rrclass)
+                if zone_info is None:
+                    # TODO what to do? no info known about zone. defaults?
+                    errmsg = "Got notification to retransfer unknown zone " + zone_name.to_text()
+                    log_error(errmsg)
+                    answer = create_answer(1, errmsg)
+                else:
+                    master_addr = zone_info.get_master_addr_info()
+                    ret = self.xfrin_start(zone_name,
+                                           rrclass,
+                                           self._get_db_file(),
+                                           master_addr,
+                                           zone_info.tsig_key,
+                                           True)
+                    answer = create_answer(ret[0], ret[1])
 
             elif command == 'retransfer' or command == 'refresh':
                 # Xfrin receives the retransfer/refresh from cmdctl(sent by bindctl).
                 # If the command has specified master address, do transfer from the
                 # master address, or else do transfer from the configured masters.
                 (zone_name, rrclass) = self._parse_zone_name_and_class(args)
-                master_addr = self._parse_master_and_port(args)
+                master_addr = self._parse_master_and_port(args, zone_name,
+                                                          rrclass)
+                zone_info = self._get_zone_info(zone_name, rrclass)
+                tsig_key = None
+                if zone_info:
+                    tsig_key = zone_info.tsig_key
                 db_file = args.get('db_file') or self._get_db_file()
                 ret = self.xfrin_start(zone_name,
                                        rrclass,
                                        db_file,
                                        master_addr,
-                                       self._tsig_key_str,
+                                       tsig_key,
                                        (False if command == 'retransfer' else True))
                 answer = create_answer(ret[0], ret[1])
 
@@ -486,25 +646,51 @@ class Xfrin:
         return answer
 
     def _parse_zone_name_and_class(self, args):
-        zone_name = args.get('zone_name')
-        if not zone_name:
+        zone_name_str = args.get('zone_name')
+        if zone_name_str is None:
             raise XfrinException('zone name should be provided')
 
-        rrclass = args.get('zone_class')
-        if not rrclass:
-            rrclass = RRClass.IN()
+        return (_check_zone_name(zone_name_str), _check_zone_class(args.get('zone_class')))
+
+    def _parse_master_and_port(self, args, zone_name, zone_class):
+        """
+        Return tuple (family, socktype, sockaddr) for address and port in given
+        args dict.
+        IPv4 and IPv6 are the only supported addresses now, so sockaddr will be
+        (address, port). The socktype is socket.SOCK_STREAM for now.
+        """
+        # check if we have configured info about this zone, in case
+        # port or master are not specified
+        zone_info = self._get_zone_info(zone_name, zone_class)
+
+        addr_str = args.get('master')
+        if addr_str is None:
+            if zone_info is not None:
+                addr = zone_info.master_addr
+            else:
+                raise XfrinException("Master address not given or "
+                                     "configured for " + zone_name.to_text())
         else:
             try:
-                rrclass = RRClass(rrclass)
-            except InvalidRRClass as e:
-                raise XfrinException('invalid RRClass: ' + rrclass)
-
-        return zone_name, rrclass
+                addr = isc.net.parse.addr_parse(addr_str)
+            except ValueError as err:
+                raise XfrinException("failed to resolve master address %s: %s" %
+                                     (addr_str, str(err)))
+
+        port_str = args.get('port')
+        if port_str is None:
+            if zone_info is not None:
+                port = zone_info.master_port
+            else:
+                port = DEFAULT_MASTER_PORT
+        else:
+            try:
+                port = isc.net.parse.port_parse(port_str)
+            except ValueError as err:
+                raise XfrinException("failed to parse port=%s: %s" %
+                                     (port_str, str(err)))
 
-    def _parse_master_and_port(self, args):
-        port = args.get('port') or self._master_port
-        master = args.get('master') or self._master_addr
-        return build_addr_info(master, port)
+        return (addr.family, socket.SOCK_STREAM, (str(addr), port))
 
     def _get_db_file(self):
         #TODO, the db file path should be got in auth server's configuration
@@ -567,7 +753,7 @@ class Xfrin:
         while not self._shutdown_event.is_set():
             self._cc_check_command()
 
-    def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, tsig_key_str,
+    def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, tsig_key,
                     check_soa = True):
         if "pydnspp" not in sys.modules:
             return (1, "xfrin failed, can't load dns message python library: 'pydnspp'")
@@ -582,12 +768,13 @@ class Xfrin:
         xfrin_thread = threading.Thread(target = process_xfrin,
                                         args = (self,
                                                 self.recorder,
-                                                zone_name, rrclass,
+                                                zone_name.to_text(),
+                                                rrclass,
                                                 db_file,
                                                 self._shutdown_event,
                                                 master_addrinfo, check_soa,
                                                 self._verbose,
-                                                tsig_key_str))
+                                                tsig_key))
 
         xfrin_thread.start()
         return (0, 'zone xfrin is started')
@@ -604,20 +791,6 @@ def set_signal_handler():
     signal.signal(signal.SIGTERM, signal_handler)
     signal.signal(signal.SIGINT, signal_handler)
 
-def build_addr_info(addrstr, portstr):
-    """
-    Return tuple (family, socktype, sockaddr) for given address and port.
-    IPv4 and IPv6 are the only supported addresses now, so sockaddr will be
-    (address, port). The socktype is socket.SOCK_STREAM for now.
-    """
-    try:
-        port = isc.net.parse.port_parse(portstr)
-        addr = isc.net.parse.addr_parse(addrstr)
-        return (addr.family, socket.SOCK_STREAM, (addrstr, port))
-    except ValueError as err:
-        raise XfrinException("failed to resolve master address/port=%s/%s: %s" %
-                             (addrstr, portstr, str(err)))
-
 def set_cmd_options(parser):
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
             help="display more about what is going on")

src/bin/xfrin/xfrin.spec

@@ -9,21 +9,43 @@
         "item_optional": false,
         "item_default": 10
       },
-      {
-        "item_name": "master_addr",
-        "item_type": "string",
-        "item_optional": false,
-        "item_default": ""
-      },
-      { "item_name": "master_port",
-        "item_type": "integer",
+      { "item_name": "zones",
+        "item_type": "list",
         "item_optional": false,
-        "item_default": 53
-      },
-      { "item_name": "tsig_key",
-        "item_type": "string",
-        "item_optional": true,
-        "item_default": ""
+        "item_default": [],
+        "list_item_spec":
+        { "item_type": "map",
+          "item_name": "zone_info",
+          "item_optional": false,
+          "item_default": {},
+          "map_item_spec": [
+          { "item_name": "name",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": ""
+          },
+          { "item_name": "class",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": "IN"
+          },
+          {
+            "item_name": "master_addr",
+            "item_type": "string",
+            "item_optional": false,
+            "item_default": ""
+          },
+          { "item_name": "master_port",
+            "item_type": "integer",
+            "item_optional": false,
+            "item_default": 53
+          },
+          { "item_name": "tsig_key",
+            "item_type": "string",
+            "item_optional": true
+          }
+          ]
+        }
       }
     ],
     "commands": [

src/lib/python/isc/config/config_data.py

@@ -213,6 +213,15 @@ class ConfigData:
             return spec['item_default'], True
         return None, False
 
+    def get_default_value(self, identifier):
+        """Returns the default from the specification, or None if there
+           is no default"""
+        spec = find_spec_part(self.specification.get_config_spec(), identifier)
+        if spec and 'item_default' in spec:
+            return spec['item_default']
+        else:
+            return None
+
     def get_module_spec(self):
         """Returns the ModuleSpec object associated with this ConfigData"""
         return self.specification

src/lib/python/isc/config/tests/config_data_test.py

@@ -237,6 +237,27 @@ class TestConfigData(unittest.TestCase):
         self.assertEqual(None, value)
         self.assertEqual(False, default)
 
+    def test_get_default_value(self):
+        self.assertEqual(1, self.cd.get_default_value("item1"))
+        self.assertEqual('default', self.cd.get_default_value("item6/value1"))
+        self.assertEqual(None, self.cd.get_default_value("item6/value2"))
+
+        # set some local values to something else, and see if we
+        # still get the default
+        self.cd.set_local_config({"item1": 2, "item6": { "value1": "asdf" } })
+
+        self.assertEqual((2, False), self.cd.get_value("item1"))
+        self.assertEqual(1, self.cd.get_default_value("item1"))
+        self.assertEqual(('asdf', False), self.cd.get_value("item6/value1"))
+        self.assertEqual('default', self.cd.get_default_value("item6/value1"))
+
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          self.cd.get_default_value,
+                          "does_not_exist/value1")
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          self.cd.get_default_value,
+                          "item6/doesnotexist")
+
     def test_set_local_config(self):
         self.cd.set_local_config({"item1": 2})
         value, default = self.cd.get_value("item1")