[master] Merge branch 'trac2013' with fixing a conflict.

src/bin/ddns/ddns.py.in

@@ -18,12 +18,18 @@
 
 import sys; sys.path.append ('@@PYTHONPATH@@')
 import isc
+from isc.acl.dns import REQUEST_LOADER
 import bind10_config
 from isc.dns import *
+import isc.ddns.session
+from isc.ddns.zone_config import ZoneConfig
+from isc.ddns.logger import ClientFormatter
 from isc.config.ccsession import *
 from isc.cc import SessionError, SessionTimeout
 import isc.util.process
 import isc.util.cio.socketsession
+import isc.server_common.tsig_keyring
+from isc.datasrc import DataSourceClient
 import select
 import errno
 
@@ -39,26 +45,37 @@ isc.log.init("b10-ddns")
 logger = isc.log.Logger("ddns")
 TRACE_BASIC = logger.DBGLVL_TRACE_BASIC
 
+# Well known path settings.  We need to define
+# SPECFILE_LOCATION: ddns configuration spec file
+# SOCKET_FILE: Unix domain socket file to communicate with b10-auth
+# AUTH_SPECFILE_LOCATION: b10-auth configuration spec file (tentatively
+#  necessarily for sqlite3-only-and-older-datasrc-API stuff).  This should be
+#  gone once we migrate to the new API and start using generalized config.
+#
 # If B10_FROM_SOURCE 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_LOCATION = os.environ["B10_FROM_SOURCE"] + os.sep + \
+    SPECFILE_PATH = os.environ["B10_FROM_SOURCE"] + "/src/bin/ddns"
-        "src" + os.sep + "bin" + os.sep + "ddns" + os.sep + "ddns.spec"
 else:
     PREFIX = "@prefix@"
     DATAROOTDIR = "@datarootdir@"
-    SPECFILE_LOCATION = "@datadir@" + os.sep + "@PACKAGE@" + os.sep + "ddns.spec"
+    SPECFILE_PATH = "@datadir@/@PACKAGE@".replace("${datarootdir}", DATAROOTDIR)
-    SPECFILE_LOCATION = SPECFILE_LOCATION.replace("${datarootdir}", DATAROOTDIR)\
+    SPECFILE_PATH = SPECFILE_PATH.replace("${prefix}", PREFIX)
-        .replace("${prefix}", PREFIX)
 
-SOCKET_FILE = bind10_config.DATA_PATH + '/ddns_socket'
 if "B10_FROM_BUILD" in os.environ:
+    AUTH_SPECFILE_PATH = os.environ["B10_FROM_BUILD"] + "/src/bin/auth"
     if "B10_FROM_SOURCE_LOCALSTATEDIR" in os.environ:
-        SOCKET_FILE = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"] + \
+        SOCKET_FILE_PATH = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"]
-            "/ddns_socket"
     else:
-        SOCKET_FILE = os.environ["B10_FROM_BUILD"] + "/ddns_socket"
+        SOCKET_FILE_PATH = os.environ["B10_FROM_BUILD"]
+else:
+    SOCKET_FILE_PATH = bind10_config.DATA_PATH
+    AUTH_SPECFILE_PATH = SPECFILE_PATH
+
+SPECFILE_LOCATION = SPECFILE_PATH + "/ddns.spec"
+SOCKET_FILE = SOCKET_FILE_PATH + '/ddns_socket'
+AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + '/auth.spec'
 
 isc.util.process.rename()
 
@@ -93,6 +110,42 @@ def clear_socket():
     if os.path.exists(SOCKET_FILE):
         os.remove(SOCKET_FILE)
 
+def get_datasrc_client(cc_session):
+    '''Return data source client for update requests.
+
+    This is supposed to have a very short lifetime and should soon be replaced
+    with generic data source configuration framework.  Based on that
+    observation we simply hardcode everything except the SQLite3 database file,
+    which will be retrieved from the auth server configuration (this behavior
+    will also be deprecated).  When something goes wrong with it this function
+    still returns a dummy client so that the caller doesn't have to bother
+    to handle the error (which would also have to be replaced anyway).
+    The caller will subsequently call its find_zone method via an update
+    session object, which will result in an exception, and then result in
+    a SERVFAIL response.
+
+    Once we are ready for introducing the general framework, the whole
+    function will simply be removed.
+
+    '''
+    HARDCODED_DATASRC_CLASS = RRClass.IN()
+    file, is_default = cc_session.get_remote_config_value("Auth",
+                                                          "database_file")
+    # See xfrout.py:get_db_file() for this trick:
+    if is_default and "B10_FROM_BUILD" in os.environ:
+        file = os.environ["B10_FROM_BUILD"] + "/bind10_zones.sqlite3"
+    datasrc_config = '{ "database_file": "' + file + '"}'
+    try:
+        return HARDCODED_DATASRC_CLASS, DataSourceClient('sqlite3',
+                                                         datasrc_config)
+    except isc.datasrc.Error as ex:
+        class DummyDataSourceClient:
+            def __init__(self, ex):
+                self.__ex = ex
+            def find_zone(self, zone_name):
+                raise isc.datasrc.Error(self.__ex)
+        return HARDCODED_DATASRC_CLASS, DummyDataSourceClient(ex)
+
 class DDNSServer:
     def __init__(self, cc_session=None):
         '''
@@ -110,8 +163,17 @@ class DDNSServer:
                                                   self.config_handler,
                                                   self.command_handler)
 
+        # Initialize configuration with defaults.  Right now 'zones' is the
+        # only configuration, so we simply directly set it here.
         self._config_data = self._cc.get_full_config()
+        self._zone_config = self.__update_zone_config(
+            self._cc.get_default_value('zones'))
         self._cc.start()
+
+        # Get necessary configurations from remote modules.
+        self._cc.add_remote_config(AUTH_SPECFILE_LOCATION)
+        isc.server_common.tsig_keyring.init_keyring(self._cc)
+
         self._shutdown = False
         # List of the session receivers where we get the requests
         self._socksession_receivers = {}
@@ -120,12 +182,52 @@ class DDNSServer:
         self._listen_socket.bind(SOCKET_FILE)
         self._listen_socket.listen(16)
 
+        # Create reusable resources
+        self.__request_msg = Message(Message.PARSE)
+        self.__response_renderer = MessageRenderer()
+
+        # The following attribute(s) are essentially private and constant,
+        # but defined as "protected" so that test code can customize them.
+        # They should not be overridden for any other purposes.
+        #
+        # DDNS Protocol handling class.
+        self._UpdateSessionClass = isc.ddns.session.UpdateSession
+
+    class SessionError(Exception):
+        '''Exception for internal errors in an update session.
+
+        This exception is expected to be caught within the server class,
+        only used for controling the code flow.
+
+        '''
+        pass
+
     def config_handler(self, new_config):
         '''Update config data.'''
-        # TODO: Handle exceptions and turn them to an error response
+        try:
-        # (once we have any configuration)
+            if 'zones' in new_config:
-        answer = create_answer(0)
+                self._zone_config = \
-        return answer
+                    self.__update_zone_config(new_config['zones'])
+            return create_answer(0)
+        except Exception as ex:
+            # We catch any exception here.  That includes any syntax error
+            # against the configuration spec.  The config interface is too
+            # complicated and it's not clear how much validation is performed
+            # there, so, while assuming it's unlikely to happen, we act
+            # proactively.
+            logger.error(DDNS_CONFIG_HANDLER_ERROR, ex)
+            return create_answer(1, "Failed to handle new configuration: " +
+                                 str(ex))
+
+    def __update_zone_config(self, new_zones_config):
+        '''Handle zones configuration update.'''
+        new_zones = {}
+        for zone_config in new_zones_config:
+            origin = Name(zone_config['origin'])
+            rrclass = RRClass(zone_config['class'])
+            update_acl = zone_config['update_acl']
+            new_zones[(origin, rrclass)] = REQUEST_LOADER.load(update_acl)
+        return new_zones
 
     def command_handler(self, cmd, args):
         '''
@@ -168,10 +270,10 @@ class DDNSServer:
         Accept another connection and create the session receiver.
         """
         try:
-            sock = self._listen_socket.accept()
+            (sock, remote_addr) = self._listen_socket.accept()
             fileno = sock.fileno()
             logger.debug(TRACE_BASIC, DDNS_NEW_CONN, fileno,
-                         sock.getpeername())
+                         remote_addr if remote_addr else '<anonymous address>')
             receiver = isc.util.cio.socketsession.SocketSessionReceiver(sock)
             self._socksession_receivers[fileno] = (sock, receiver)
         except (socket.error, isc.util.cio.socketsession.SocketSessionError) \
@@ -180,7 +282,30 @@ class DDNSServer:
             # continue with the rest
             logger.error(DDNS_ACCEPT_FAILURE, e)
 
-    def handle_request(self, request):
+    def __check_request_tsig(self, msg, req_data):
+        '''TSIG checker for update requests.
+
+        This is a helper method for handle_request() below.  It examines
+        the given update request message to see if it contains a TSIG RR,
+        and verifies the signature if it does.  It returs the TSIG context
+        used for the verification, or None if the request doesn't contain
+        a TSIG.  If the verification fails it simply raises an exception
+        as handle_request() assumes it should succeed.
+
+        '''
+        tsig_record = msg.get_tsig_record()
+        if tsig_record is None:
+            return None
+        tsig_ctx = TSIGContext(tsig_record.get_name(),
+                               tsig_record.get_rdata().get_algorithm(),
+                               isc.server_common.tsig_keyring.get_keyring())
+        tsig_error = tsig_ctx.verify(tsig_record, req_data)
+        if tsig_error != TSIGError.NOERROR:
+            raise SessionError("Failed to verify request's TSIG: " +
+                               str(tsig_error))
+        return tsig_ctx
+
+    def handle_request(self, req_session):
         """
         This is the place where the actual DDNS processing is done. Other
         methods are either subroutines of this method or methods doing the
@@ -190,12 +315,70 @@ class DDNSServer:
         It is called with the request being session as received from
         SocketSessionReceiver, i.e. tuple
         (socket, local_address, remote_address, data).
+
+        In general, this method doesn't propagate exceptions outside the
+        method.  Most of protocol or system errors will result in an error
+        response to the update client or dropping the update request.
+        The update session class should also ensure this.  Critical exceptions
+        such as memory allocation failure will be propagated, however, and
+        will subsequently terminate the server process.
+
+        Return: True if a response to the request is successfully sent;
+        False otherwise.  The return value wouldn't be useful for the server
+        itself; it's provided mainly for testing purposes.
+
         """
-        # TODO: Implement the magic
+        # give tuple elements intuitive names
+        (sock, local_addr, remote_addr, req_data) = req_session
+
+        # The session sender (b10-auth) should have made sure that this is
+        # a validly formed DNS message of OPCODE being UPDATE, and if it's
+        # TSIG signed, its key is known to the system and the signature is
+        # valid.  Messages that don't meet these should have been resopnded
+        # or dropped by the sender, so if such error is detected we treat it
+        # as an internal error and don't bother to respond.
+        try:
+            if sock.proto == socket.IPPROTO_TCP:
+                raise SessionError('TCP requests are not yet supported')
+            self.__request_msg.clear(Message.PARSE)
+            # specify PRESERVE_ORDER as we need to handle each RR separately.
+            self.__request_msg.from_wire(req_data, Message.PRESERVE_ORDER)
+            if self.__request_msg.get_opcode() != Opcode.UPDATE():
+                raise SessionError('Update request has unexpected opcode: ' +
+                                   str(self.__request_msg.get_opcode()))
+            tsig_ctx = self.__check_request_tsig(self.__request_msg, req_data)
+        except Exception as ex:
+            logger.error(DDNS_REQUEST_PARSE_FAIL, ex)
+            return False
+
+        # Let an update session object handle the request.  Note: things around
+        # ZoneConfig will soon be substantially revised.  For now we don't
+        # bother to generalize it.
+        datasrc_class, datasrc_client = get_datasrc_client(self._cc)
+        zone_cfg = ZoneConfig([], datasrc_class, datasrc_client,
+                              self._zone_config)
+        update_session = self._UpdateSessionClass(self.__request_msg,
+                                                  remote_addr, zone_cfg)
+        result, zname, zclass = update_session.handle()
+
+        # If the request should be dropped, we're done; otherwise, send the
+        # response generated by the session object.
+        if result == isc.ddns.session.UPDATE_DROP:
+            return False
+        msg = update_session.get_message()
+        self.__response_renderer.clear()
+        if tsig_ctx is not None:
+            msg.to_wire(self.__response_renderer, tsig_ctx)
+        else:
+            msg.to_wire(self.__response_renderer)
+        try:
+            sock.sendto(self.__response_renderer.get_data(), remote_addr)
+        except socket.error as ex:
+            logger.error(DDNS_RESPONSE_SOCKET_ERROR,
+                         ClientFormatter(remote_addr), ex)
+            return False
 
-        # TODO: Don't propagate most of the exceptions (like datasrc errors),
+        return True
-        # just drop the packet.
-        pass
 
     def handle_session(self, fileno):
         """

src/bin/ddns/ddns.spec

@@ -4,22 +4,36 @@
     "config_data": [
       {
         "item_name": "zones",
-        "item_type": "named_set",
+        "item_type": "list",
         "item_optional": false,
-        "item_default": {},
+        "item_default": [],
-        "named_set_item_spec": {
+        "list_item_spec": {
           "item_name": "entry",
           "item_type": "map",
           "item_optional": true,
           "item_default": {
-            "update_acl": [{"action": "ACCEPT", "from": "127.0.0.1"},
+	    "origin": "",
-                           {"action": "ACCEPT", "from": "::1"}]
+	    "class": "IN",
+            "update_acl": []
           },
           "map_item_spec": [
             {
+              "item_name": "origin",
+              "item_type": "string",
+              "item_optional": false,
+              "item_default": ""
+            },
+            {
+              "item_name": "class",
+              "item_type": "string",
+              "item_optional": false,
+              "item_default": "IN"
+            },
+            {
               "item_name": "update_acl",
               "item_type": "list",
               "item_optional": false,
+	      "item_default": [],
               "list_item_spec": {
                 "item_name": "acl_element",
                 "item_type": "any",

src/bin/ddns/ddns_messages.mes

@@ -38,6 +38,14 @@ configuration manager b10-cfgmgr is not running.
 The ddns process encountered an error when installing the configuration at
 startup time.  Details of the error are included in the log message.
 
+% DDNS_CONFIG_HANDLER_ERROR failed to update ddns configuration: %1
+An update to b10-ddns configuration was delivered but an error was
+found while applying them.  None of the delivered updates were applied
+to the running b10-ddns system, and the server will keep running with
+the existing configuration.  If this happened in the initial
+configuration setup, the server will be running with the default
+configurations.
+
 % DDNS_DROP_CONN dropping connection on file descriptor %1 because of error %2
 There was an error on a connection with the b10-auth server (or whatever
 connects to the ddns daemon). This might be OK, for example when the
@@ -62,6 +70,22 @@ coming from a b10-auth process.
 The ddns process received a shutdown command from the command channel
 and will now shut down.
 
+% DDNS_REQUEST_PARSE_FAIL failed to parse update request: %1
+b10-ddns received an update request via b10-auth, but the received
+data failed to pass minimum validation: it was either broken wire
+format data for a valid DNS message (e.g. it's shorter than the
+fixed-length header), or the opcode is not update, or TSIG is included
+in the request but it fails to validate.  Since b10-auth should have
+performed this level of checks, such an error shouldn't be detected at
+this stage and should rather be considered an internal bug.  This
+event is therefore logged at the error level, and the request is
+simply dropped.  Additional information of the error is also logged.
+
+% DDNS_RESPONSE_SOCKET_ERROR failed to send update response to %1: %2
+Network I/O error happens in sending an update request.  The
+client's address that caused the error and error details are also
+logged.
+
 % DDNS_RUNNING ddns server is running and listening for updates
 The ddns process has successfully started and is now ready to receive commands
 and updates.

src/bin/ddns/tests/Makefile.am

@@ -25,5 +25,6 @@ endif
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/bin/ddns:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/util/io/.libs \
 	TESTDATASRCDIR=$(abs_srcdir)/testdata/ \
+	TESTDATA_PATH=$(abs_top_srcdir)/src/lib/testutils/testdata \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/ddns/tests/ddns_test.py

@@ -15,28 +15,71 @@
 
 '''Tests for the DDNS module'''
 
-import unittest
+from isc.ddns.session import *
-import isc
+from isc.dns import *
+from isc.acl.acl import ACCEPT
+import isc.util.cio.socketsession
+from isc.datasrc import DataSourceClient
 import ddns
-import isc.config
-import select
 import errno
-import isc.util.cio.socketsession
+import os
+import select
+import shutil
 import socket
-import os.path
+import unittest
+
+# Some common test parameters
+TESTDATA_PATH = os.environ['TESTDATA_PATH'] + os.sep
+READ_ZONE_DB_FILE = TESTDATA_PATH + "rwtest.sqlite3" # original, to be copied
+TEST_ZONE_NAME = Name('example.org')
+TEST_ZONE_NAME_STR = TEST_ZONE_NAME.to_text()
+UPDATE_RRTYPE = RRType.SOA()
+TEST_QID = 5353                 # arbitrary chosen
+TEST_RRCLASS = RRClass.IN()
+TEST_RRCLASS_STR = TEST_RRCLASS.to_text()
+TEST_SERVER6 = ('2001:db8::53', 53, 0, 0)
+TEST_CLIENT6 = ('2001:db8::1', 53000, 0, 0)
+TEST_SERVER4 = ('192.0.2.53', 53)
+TEST_CLIENT4 = ('192.0.2.1', 53534)
+TEST_ZONE_RECORD = Question(TEST_ZONE_NAME, TEST_RRCLASS, UPDATE_RRTYPE)
+TEST_ACL_CONTEXT = isc.acl.dns.RequestContext(
+    socket.getaddrinfo("192.0.2.1", 1234, 0, socket.SOCK_DGRAM,
+                       socket.IPPROTO_UDP, socket.AI_NUMERICHOST)[0][4])
+# TSIG key for tests when needed.  The key name is TEST_ZONE_NAME.
+TEST_TSIG_KEY = TSIGKey("example.org:SFuWd/q99SzF8Yzd1QbB9g==")
+# TSIG keyring that contanins the test key
+TEST_TSIG_KEYRING = TSIGKeyRing()
+TEST_TSIG_KEYRING.add(TEST_TSIG_KEY)
+# Another TSIG key not in the keyring, making verification fail
+BAD_TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==")
 
 class FakeSocket:
     """
     A fake socket. It only provides a file number, peer name and accept method.
     """
     def __init__(self, fileno):
+        self.proto = socket.IPPROTO_UDP
         self.__fileno = fileno
+        self._sent_data = None
+        self._sent_addr = None
+        # customizable by tests; if set to True, sendto() will throw after
+        # recording the parameters.
+        self._raise_on_send = False
     def fileno(self):
         return self.__fileno
     def getpeername(self):
         return "fake_unix_socket"
     def accept(self):
-        return FakeSocket(self.__fileno + 1)
+        return FakeSocket(self.__fileno + 1), '/dummy/path'
+    def sendto(self, data, addr):
+        self._sent_data = data
+        self._sent_addr = addr
+        if self._raise_on_send:
+            raise socket.error('test socket failure')
+    def clear(self):
+        '''Clear internal instrumental data.'''
+        self._sent_data = None
+        self._sent_addr = None
 
 class FakeSessionReceiver:
     """
@@ -51,14 +94,67 @@ class FakeSessionReceiver:
         """
         return self._socket
 
+class FakeUpdateSession:
+    '''A fake update session, emulating isc.ddns.session.UpdateSession.
+
+    It provides the same interfaces as UpdateSession with skipping complicated
+    internal protocol processing and returning given faked results.  This
+    will help simplify test setups.
+
+    '''
+    def __init__(self, msg, client_addr, zone_config, faked_result):
+        '''Faked constructor.
+
+        It takes an additional faked_result parameter.  It will be used
+        as the result value of handle().  If its value is UPDATE_ERROR,
+        get_message() will create a response message whose Rcode is
+        REFUSED.
+
+        '''
+        self.__msg = msg
+        self.__faked_result = faked_result
+
+    def handle(self):
+        if self.__faked_result == UPDATE_SUCCESS:
+            return self.__faked_result, TEST_ZONE_NAME, TEST_RRCLASS
+        return self.__faked_result, None, None
+
+    def get_message(self):
+        self.__msg.make_response()
+        self.__msg.clear_section(SECTION_ZONE)
+        if self.__faked_result == UPDATE_SUCCESS:
+            self.__msg.set_rcode(Rcode.NOERROR())
+        else:
+            self.__msg.set_rcode(Rcode.REFUSED())
+        return self.__msg
+
+class FakeKeyringModule:
+    '''Fake the entire isc.server_common.tsig_keyring module.'''
+
+    def init_keyring(self, cc):
+        '''Set the instrumental attribute to True when called.
+
+        It can be used for a test that confirms TSIG key initialization is
+        surely performed.  This class doesn't use any CC session, so the
+        cc parameter will be ignored.
+
+        '''
+        self.initialized = True
+
+    def get_keyring(self):
+        '''Simply return the predefined TSIG keyring unconditionally.'''
+        return TEST_TSIG_KEYRING
+
 class MyCCSession(isc.config.ConfigData):
-    '''Fake session with minimal interface compliance'''
+    '''Fake session with minimal interface compliance.'''
     def __init__(self):
         module_spec = isc.config.module_spec_from_file(
             ddns.SPECFILE_LOCATION)
         isc.config.ConfigData.__init__(self, module_spec)
         self._started = False
         self._stopped = False
+        # Used as the return value of get_remote_config_value.  Customizable.
+        self.auth_db_file = READ_ZONE_DB_FILE
 
     def start(self):
         '''Called by DDNSServer initialization, but not used in tests'''
@@ -74,6 +170,13 @@ class MyCCSession(isc.config.ConfigData):
         """
         return FakeSocket(1)
 
+    def add_remote_config(self, spec_file_name):
+        pass
+
+    def get_remote_config_value(self, module_name, item):
+        if module_name == "Auth" and item == "database_file":
+            return self.auth_db_file, False
+
 class MyDDNSServer():
     '''Fake DDNS server used to test the main() function'''
     def __init__(self):
@@ -104,6 +207,8 @@ class TestDDNSServer(unittest.TestCase):
     def setUp(self):
         cc_session = MyCCSession()
         self.assertFalse(cc_session._started)
+        self.orig_tsig_keyring = isc.server_common.tsig_keyring
+        isc.server_common.tsig_keyring = FakeKeyringModule()
         self.ddns_server = ddns.DDNSServer(cc_session)
         self.__cc_session = cc_session
         self.assertTrue(cc_session._started)
@@ -118,6 +223,7 @@ class TestDDNSServer(unittest.TestCase):
         ddns.select.select = select.select
         ddns.isc.util.cio.socketsession.SocketSessionReceiver = \
             isc.util.cio.socketsession.SocketSessionReceiver
+        isc.server_common.tsig_keyring = self.orig_tsig_keyring
 
     def test_listen(self):
         '''
@@ -141,12 +247,92 @@ class TestDDNSServer(unittest.TestCase):
         ddns.clear_socket()
         self.assertFalse(os.path.exists(ddns.SOCKET_FILE))
 
+    def test_initial_config(self):
+        # right now, the only configuration is the zone configuration, whose
+        # default should be an empty map.
+        self.assertEqual({}, self.ddns_server._zone_config)
+
     def test_config_handler(self):
-        # Config handler does not do anything yet, but should at least
+        # Update with a simple zone configuration: including an accept-all ACL
-        # return 'ok' for now.
+        new_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
-        new_config = {}
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'ACCEPT'}] } ] }
         answer = self.ddns_server.config_handler(new_config)
         self.assertEqual((0, None), isc.config.parse_answer(answer))
+        acl = self.ddns_server._zone_config[(TEST_ZONE_NAME, TEST_RRCLASS)]
+        self.assertEqual(ACCEPT, acl.execute(TEST_ACL_CONTEXT))
+
+        # Slightly more complicated one: containing multiple ACLs
+        new_config = { 'zones': [ { 'origin': 'example.com',
+                                    'class': 'CH',
+                                    'update_acl': [{'action': 'REJECT',
+                                                    'from': '2001:db8::1'}] },
+                                  { 'origin': TEST_ZONE_NAME_STR,
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'ACCEPT'}] },
+                                  { 'origin': 'example.org',
+                                    'class': 'CH',
+                                    'update_acl': [{'action': 'DROP'}] } ] }
+        answer = self.ddns_server.config_handler(new_config)
+        self.assertEqual((0, None), isc.config.parse_answer(answer))
+        self.assertEqual(3, len(self.ddns_server._zone_config))
+        acl = self.ddns_server._zone_config[(TEST_ZONE_NAME, TEST_RRCLASS)]
+        self.assertEqual(ACCEPT, acl.execute(TEST_ACL_CONTEXT))
+
+        # empty zone config
+        new_config = { 'zones': [] }
+        answer = self.ddns_server.config_handler(new_config)
+        self.assertEqual((0, None), isc.config.parse_answer(answer))
+        self.assertEqual({}, self.ddns_server._zone_config)
+
+        # bad zone config data: bad name.  The previous config shouls be kept.
+        bad_config = { 'zones': [ { 'origin': 'bad..example',
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'ACCEPT'}] } ] }
+        answer = self.ddns_server.config_handler(bad_config)
+        self.assertEqual(1, isc.config.parse_answer(answer)[0])
+        self.assertEqual({}, self.ddns_server._zone_config)
+
+        # bad zone config data: bad class.
+        bad_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
+                                    'class': 'badclass',
+                                    'update_acl': [{'action': 'ACCEPT'}] } ] }
+        answer = self.ddns_server.config_handler(bad_config)
+        self.assertEqual(1, isc.config.parse_answer(answer)[0])
+        self.assertEqual({}, self.ddns_server._zone_config)
+
+        # bad zone config data: bad ACL.
+        bad_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'badaction'}]}]}
+        answer = self.ddns_server.config_handler(bad_config)
+        self.assertEqual(1, isc.config.parse_answer(answer)[0])
+        self.assertEqual({}, self.ddns_server._zone_config)
+
+        # the first zone cofig is valid, but not the second.  the first one
+        # shouldn't be installed.
+        bad_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'ACCEPT'}] },
+                                  { 'origin': 'bad..example',
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'ACCEPT'}] } ] }
+        answer = self.ddns_server.config_handler(bad_config)
+        self.assertEqual(1, isc.config.parse_answer(answer)[0])
+        self.assertEqual({}, self.ddns_server._zone_config)
+
+        # Half-broken case: 'origin, class' pair is duplicate.  For now we
+        # we accept it (the latter one will win)
+        dup_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'REJECT'}] },
+                                  { 'origin': TEST_ZONE_NAME_STR,
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'ACCEPT'}] } ] }
+        answer = self.ddns_server.config_handler(dup_config)
+        self.assertEqual((0, None), isc.config.parse_answer(answer))
+        acl = self.ddns_server._zone_config[(TEST_ZONE_NAME, TEST_RRCLASS)]
+        self.assertEqual(ACCEPT, acl.execute(TEST_ACL_CONTEXT))
 
     def test_shutdown_command(self):
         '''Test whether the shutdown command works'''
@@ -361,6 +547,186 @@ class TestDDNSServer(unittest.TestCase):
         self.__select_expected = ([1, 2], [], [], None)
         self.assertRaises(select.error, self.ddns_server.run)
 
+def create_msg(opcode=Opcode.UPDATE(), zones=[TEST_ZONE_RECORD], prereq=[],
+               tsigctx=None):
+    msg = Message(Message.RENDER)
+    msg.set_qid(TEST_QID)
+    msg.set_opcode(opcode)
+    msg.set_rcode(Rcode.NOERROR())
+    for z in zones:
+        msg.add_question(z)
+    for p in prereq:
+        msg.add_rrset(SECTION_PREREQUISITE, p)
+
+    renderer = MessageRenderer()
+    if tsigctx is not None:
+        msg.to_wire(renderer, tsigctx)
+    else:
+        msg.to_wire(renderer)
+
+    # re-read the created data in the parse mode
+    msg.clear(Message.PARSE)
+    msg.from_wire(renderer.get_data())
+
+    return renderer.get_data()
+
+
+class TestDDNSession(unittest.TestCase):
+    def setUp(self):
+        cc_session = MyCCSession()
+        self.assertFalse(cc_session._started)
+        self.orig_tsig_keyring = isc.server_common.tsig_keyring
+        isc.server_common.tsig_keyring = FakeKeyringModule()
+        self.server = ddns.DDNSServer(cc_session)
+        self.server._UpdateSessionClass = self.__fake_session_creator
+        self.__faked_result = UPDATE_SUCCESS # will be returned by fake session
+        self.__sock = FakeSocket(-1)
+
+    def tearDown(self):
+        self.assertTrue(isc.server_common.tsig_keyring.initialized)
+        isc.server_common.tsig_keyring = self.orig_tsig_keyring
+
+    def __fake_session_creator(self, req_message, client_addr, zone_config):
+        # remember the passed message for possible inspection later.
+        self.__req_message = req_message
+        return FakeUpdateSession(req_message, client_addr, zone_config,
+                                 self.__faked_result)
+
+    def check_update_response(self, resp_wire, expected_rcode=Rcode.NOERROR(),
+                              tsig_ctx=None):
+        '''Check if given wire data are valid form of update response.
+
+        In this implementation, zone/prerequisite/update sections should be
+        empty in responses.
+
+        If tsig_ctx (isc.dns.TSIGContext) is not None, the response should
+        be TSIG signed and the signature should be verifiable with the context
+        that has signed the corresponding request.
+
+        '''
+        msg = Message(Message.PARSE)
+        msg.from_wire(resp_wire)
+        if tsig_ctx is not None:
+            tsig_record = msg.get_tsig_record()
+            self.assertNotEqual(None, tsig_record)
+            self.assertEqual(TSIGError.NOERROR,
+                             tsig_ctx.verify(tsig_record, resp_wire))
+        self.assertEqual(Opcode.UPDATE(), msg.get_opcode())
+        self.assertEqual(expected_rcode, msg.get_rcode())
+        self.assertEqual(TEST_QID, msg.get_qid())
+        for section in [SECTION_ZONE, SECTION_PREREQUISITE, SECTION_UPDATE]:
+            self.assertEqual(0, msg.get_rr_count(section))
+
+    def check_session(self, result=UPDATE_SUCCESS, ipv6=True, tsig_key=None):
+        # reset test parameters
+        self.__sock.clear()
+        self.__faked_result = result
+
+        server_addr = TEST_SERVER6 if ipv6 else TEST_SERVER4
+        client_addr = TEST_CLIENT6 if ipv6 else TEST_CLIENT4
+        tsig = TSIGContext(tsig_key) if tsig_key is not None else None
+        rcode = Rcode.NOERROR() if result == UPDATE_SUCCESS else Rcode.REFUSED()
+        has_response = (result != UPDATE_DROP)
+
+        self.assertEqual(has_response,
+                         self.server.handle_request((self.__sock,
+                                                     server_addr, client_addr,
+                                                     create_msg(tsigctx=tsig))))
+        if has_response:
+            self.assertEqual(client_addr, self.__sock._sent_addr)
+            self.check_update_response(self.__sock._sent_data, rcode)
+        else:
+            self.assertEqual((None, None), (self.__sock._sent_addr,
+                                            self.__sock._sent_data))
+
+    def test_handle_request(self):
+        '''Basic request handling without any unexpected errors.'''
+        # Success, without TSIG
+        self.check_session()
+        # Update will be refused with a response.
+        self.check_session(UPDATE_ERROR, ipv6=False)
+        # Update will be refused and dropped
+        self.check_session(UPDATE_DROP)
+        # Success, with TSIG
+        self.check_session(ipv6=False, tsig_key=TEST_TSIG_KEY)
+        # Update will be refused with a response, with TSIG.
+        self.check_session(UPDATE_ERROR, tsig_key=TEST_TSIG_KEY)
+        # Update will be refused and dropped, with TSIG (doesn't matter though)
+        self.check_session(UPDATE_DROP, ipv6=False, tsig_key=TEST_TSIG_KEY)
+
+    def test_broken_request(self):
+        # Message data too short
+        s = self.__sock
+        self.assertFalse(self.server.handle_request((self.__sock, None,
+                                                     None, b'x' * 11)))
+        self.assertEqual((None, None), (s._sent_data, s._sent_addr))
+
+        # Opcode is not UPDATE
+        self.assertFalse(self.server.handle_request(
+                (self.__sock, None, None, create_msg(opcode=Opcode.QUERY()))))
+        self.assertEqual((None, None), (s._sent_data, s._sent_addr))
+
+        # TSIG verification error.  We use UPDATE_DROP to signal check_session
+        # that no response should be given.
+        self.check_session(result=UPDATE_DROP, ipv6=False,
+                           tsig_key=BAD_TSIG_KEY)
+
+    def test_socket_error(self):
+        # Have the faked socket raise an exception on sendto()
+        self.__sock._raise_on_send = True
+        # handle_request indicates the failure
+        self.assertFalse(self.server.handle_request((self.__sock, TEST_SERVER6,
+                                                     TEST_SERVER4,
+                                                     create_msg())))
+        # this check ensures sendto() was really attempted.
+        self.check_update_response(self.__sock._sent_data, Rcode.NOERROR())
+
+    def test_tcp_request(self):
+        # Right now TCP request is not supported.
+        s = self.__sock
+        s.proto = socket.IPPROTO_TCP
+        self.assertFalse(self.server.handle_request((s, TEST_SERVER6,
+                                                     TEST_SERVER4,
+                                                     create_msg())))
+        self.assertEqual((None, None), (s._sent_data, s._sent_addr))
+
+    def test_request_message(self):
+        '''Test if the request message stores RRs separately.'''
+        # Specify 'drop' so the passed message won't be modified.
+        self.__faked_result = UPDATE_DROP
+        # Put the same RR twice in the prerequisite section.  We should see
+        # them as separate RRs.
+        dummy_record = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.NS(),
+                             RRTTL(0))
+        dummy_record.add_rdata(Rdata(RRType.NS(), TEST_RRCLASS, "ns.example"))
+        self.server.handle_request((self.__sock, TEST_SERVER6, TEST_CLIENT6,
+                                    create_msg(prereq=[dummy_record,
+                                                       dummy_record])))
+        num_rrsets = len(self.__req_message.get_section(SECTION_PREREQUISITE))
+        self.assertEqual(2, num_rrsets)
+
+    def test_session_with_config(self):
+        '''Check a session with more relistic config setups
+
+        We don't have to explore various cases in detail in this test.
+        We're just checking if the expected configured objects are passed
+        to the session object.
+
+        '''
+
+        # reset the session class to the real one
+        self.server._UpdateSessionClass = isc.ddns.session.UpdateSession
+
+        # install all-drop ACL
+        new_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
+                                    'class': TEST_RRCLASS_STR,
+                                    'update_acl': [{'action': 'DROP'}] } ] }
+        answer = self.server.config_handler(new_config)
+        self.assertEqual((0, None), isc.config.parse_answer(answer))
+
+        # check the result
+        self.check_session(UPDATE_DROP)
+
 class TestMain(unittest.TestCase):
     def setUp(self):
         self._server = MyDDNSServer()
@@ -416,6 +782,38 @@ class TestMain(unittest.TestCase):
         self.assertRaises(BaseException, ddns.main, self._server)
         self.assertTrue(self._server.exception_raised)
 
+class TestConfig(unittest.TestCase):
+    '''Test some simple config related things that don't need server. '''
+    def setUp(self):
+        self.__ccsession = MyCCSession()
+
+    def test_file_path(self):
+        # Check some common paths
+        self.assertEqual(os.environ["B10_FROM_BUILD"] + "/ddns_socket",
+                         ddns.SOCKET_FILE)
+        self.assertEqual(os.environ["B10_FROM_SOURCE"] +
+                         "/src/bin/ddns/ddns.spec", ddns.SPECFILE_LOCATION)
+        self.assertEqual(os.environ["B10_FROM_BUILD"] +
+                         "/src/bin/auth/auth.spec",
+                         ddns.AUTH_SPECFILE_LOCATION)
+
+    def test_get_datasrc_client(self):
+        # The test sqlite DB should contain the example.org zone.
+        rrclass, datasrc_client = ddns.get_datasrc_client(self.__ccsession)
+        self.assertEqual(RRClass.IN(), rrclass)
+        self.assertEqual(DataSourceClient.SUCCESS,
+                         datasrc_client.find_zone(Name('example.org'))[0])
+
+    def test_get_datasrc_client_fail(self):
+        # DB file is in a non existent directory, and creatng the client
+        # will fail.  get_datasrc_client will return a dummy client, which
+        # will subsequently make find_zone() fail.
+        self.__ccsession.auth_db_file = './notexistentdir/somedb.sqlite3'
+        rrclass, datasrc_client = ddns.get_datasrc_client(self.__ccsession)
+        self.assertEqual(RRClass.IN(), rrclass)
+        self.assertRaises(isc.datasrc.Error,
+                          datasrc_client.find_zone, Name('example.org'))
+
 if __name__== "__main__":
     isc.log.resetUnitTestRootLogger()
     unittest.main()

src/lib/dns/message.cc

@@ -573,7 +573,11 @@ Message::clearSection(const Section section) {
     if (section >= MessageImpl::NUM_SECTIONS) {
         isc_throw(OutOfRange, "Invalid message section: " << section);
     }
-    impl_->rrsets_[section].clear();
+    if (section == Message::SECTION_QUESTION) {
+        impl_->questions_.clear();
+    } else {
+        impl_->rrsets_[section].clear();
+    }
     impl_->counts_[section] = 0;
 }
 

src/lib/dns/python/tests/message_python_test.py

@@ -295,6 +295,7 @@ class MessageTest(unittest.TestCase):
         self.assertEqual(1, self.r.get_rr_count(Message.SECTION_QUESTION))
         self.r.clear_section(Message.SECTION_QUESTION)
         self.assertEqual(0, self.r.get_rr_count(Message.SECTION_QUESTION))
+        self.assertEqual(0, len(self.r.get_question()))
 
     def test_clear_section(self):
         for section in [Message.SECTION_ANSWER, Message.SECTION_AUTHORITY,

src/lib/dns/tests/message_unittest.cc

@@ -406,6 +406,8 @@ TEST_F(MessageTest, clearQuestionSection) {
 
     message_render.clearSection(Message::SECTION_QUESTION);
     EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_QUESTION));
+    EXPECT_TRUE(message_render.beginQuestion() ==
+                message_render.endQuestion());
 }
 
 

src/lib/python/isc/ddns/libddns_messages.mes

@@ -15,6 +15,17 @@
 # No namespace declaration - these constants go in the global namespace
 # of the libddns_messages python module.
 
+% LIBDDNS_DATASRC_ERROR update client %1 failed due to data source error: %2
+An update attempt failed due to some error in the corresponding data
+source.  This is generally an unexpected event, but can still happen
+for various reasons such as DB lock contention or a failure of the
+backend DB server.  The cause of the error is also logged.  It's
+advisable to check the message, and, if necessary, take an appropriate
+action (e.g., restarting the DB server if it dies).  If this message
+is logged the data source isn't modified due to the
+corresponding update request.  When used by the b10-ddns, the server
+will return a response with an RCODE of SERVFAIL.
+
 % LIBDDNS_PREREQ_FORMERR update client %1 for zone %2: Format error in prerequisite (%3). Non-zero TTL.
 The prerequisite with the given name, class and type is not well-formed.
 The specific prerequisite is shown. In this case, it has a non-zero TTL value.

src/lib/python/isc/ddns/session.py

@@ -214,6 +214,11 @@ class UpdateSession:
                 return UPDATE_ERROR, None, None
             self.__message = None
             return UPDATE_DROP, None, None
+        except isc.datasrc.Error as e:
+            logger.error(LIBDDNS_DATASRC_ERROR,
+                         ClientFormatter(self.__client_addr, self.__tsig), e)
+            self.__make_response(Rcode.SERVFAIL())
+            return UPDATE_ERROR, None, None
 
     def __get_update_zone(self):
         '''Parse the zone section and find the zone to be updated.

src/lib/python/isc/ddns/tests/session_tests.py

@@ -194,6 +194,21 @@ class SessionTest(SessionTestBase):
         # zone class doesn't match
         self.check_notauth(Name('example.org'), RRClass.CH())
 
+    def test_update_datasrc_error(self):
+        # if the data source client raises an exception, it should result in
+        # a SERVFAIL.
+        class BadDataSourceClient:
+            def find_zone(self, name):
+                raise isc.datasrc.Error('faked exception')
+        msg = create_update_msg(zones=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                                RRType.SOA())])
+        session = UpdateSession(msg, TEST_CLIENT4,
+                                ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+                                           TEST_RRCLASS,
+                                           BadDataSourceClient()))
+        self.assertEqual(UPDATE_ERROR, session.handle()[0])
+        self.check_response(session.get_message(), Rcode.SERVFAIL())
+
     def test_foreach_rr_in_rrset(self):
         rrset = create_rrset("www.example.org", TEST_RRCLASS,
                              RRType.A(), 3600, [ "192.0.2.1" ])