Merge branch 'master' into trac2004

ChangeLog

@@ -1,3 +1,20 @@
+444.	[bug]		jinmei
+	libdatasrc: fixed ZoneFinder for database-based data sources so
+	that it handles type DS query correctly, i.e., treating it as
+	authoritative data even on a delegation point.
+	(Trac #1912, git 7130da883f823ce837c10cbf6e216a15e1996e5d)
+
+443.    [func]*		muks
+	The logger now uses a lockfile named `logger_lockfile' that is
+	created in the local state directory to mutually separate
+	individual logging operations from various processes. This is
+	done so that log messages from different processes don't mix
+	together in the middle of lines. The `logger_lockfile` is created
+	with file permission mode 0660. BIND 10's local state directory
+	should be writable and perhaps have g+s mode bit so that the
+	`logger_lockfile` can be opened by a group of processes.
+	(Trac #1704, git ad8d445dd0ba208107eb239405166c5c2070bd8b)
+
 442.	[func]		tomek
 	b10-dhcp4, b10-dhcp6: Both DHCP servers now accept -p parameter
 	that can be used to specify listening port number. This capability

Makefile.am

@@ -405,3 +405,5 @@ EXTRA_DIST += ext/coroutine/coroutine.h
 
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = dns++.pc
+
+CLEANFILES = $(abs_top_builddir)/logger_lockfile

configure.ac

@@ -1187,6 +1187,7 @@ AC_OUTPUT([doc/version.ent
            src/lib/log/tests/destination_test.sh
            src/lib/log/tests/init_logger_test.sh
            src/lib/log/tests/local_file_test.sh
+           src/lib/log/tests/logger_lock_test.sh
            src/lib/log/tests/severity_test.sh
            src/lib/log/tests/tempdir.h
            src/lib/util/python/mkpywrapper.py
@@ -1235,6 +1236,7 @@ AC_OUTPUT([doc/version.ent
            chmod +x src/lib/log/tests/destination_test.sh
            chmod +x src/lib/log/tests/init_logger_test.sh
            chmod +x src/lib/log/tests/local_file_test.sh
+           chmod +x src/lib/log/tests/logger_lock_test.sh
            chmod +x src/lib/log/tests/severity_test.sh
            chmod +x src/lib/util/python/mkpywrapper.py
            chmod +x src/lib/util/python/gen_wiredata.py

src/bin/auth/auth_srv.cc

@@ -390,8 +390,9 @@ private:
     AuthSrv* server_;
 };
 
-AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client,
-                 BaseSocketSessionForwarder& ddns_forwarder)
+AuthSrv::AuthSrv(const bool use_cache,
+                 isc::xfr::AbstractXfroutClient& xfrout_client,
+                 isc::util::io::BaseSocketSessionForwarder& ddns_forwarder)
 {
     impl_ = new AuthSrvImpl(use_cache, xfrout_client, ddns_forwarder);
     checkin_ = new ConfigChecker(this);

src/bin/bind10/bind10_src.py.in

@@ -64,6 +64,7 @@ import posix
 import copy
 
 from bind10_config import LIBEXECPATH
+import bind10_config
 import isc.cc
 import isc.util.process
 import isc.net.parse
@@ -1122,6 +1123,28 @@ def unlink_pid_file(pid_file):
         if error.errno is not errno.ENOENT:
             raise
 
+def remove_lock_files():
+    """
+    Remove various lock files which were created by code such as in the
+    logger. This function should be called after BIND 10 shutdown.
+    """
+
+    lockfiles = ["logger_lockfile"]
+
+    lpath = bind10_config.DATA_PATH
+    if "B10_FROM_BUILD" in os.environ:
+        lpath = os.environ["B10_FROM_BUILD"]
+    if "B10_FROM_SOURCE_LOCALSTATEDIR" in os.environ:
+        lpath = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"]
+    if "B10_LOCKFILE_DIR_FROM_BUILD" in os.environ:
+        lpath = os.environ["B10_LOCKFILE_DIR_FROM_BUILD"]
+
+    for f in lockfiles:
+        fname = lpath + '/' + f
+        if os.path.isfile(fname):
+            os.unlink(fname)
+
+    return
 
 def main():
     global options
@@ -1201,6 +1224,7 @@ def main():
     finally:
         # Clean up the filesystem
         unlink_pid_file(options.pid_file)
+        remove_lock_files()
         if boss_of_bind is not None:
             boss_of_bind.remove_socket_srv()
     sys.exit(boss_of_bind.exitcode)

src/bin/bind10/tests/Makefile.am

@@ -23,6 +23,7 @@ endif
 	chmod +x $(abs_builddir)/$$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_srcdir)/src/bin:$(abs_top_builddir)/src/bin/bind10:$(abs_top_builddir)/src/lib/util/io/.libs \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	BIND10_MSGQ_SOCKET_FILE=$(abs_top_builddir)/msgq_socket \
 		$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
 	done

src/bin/bind10/tests/bind10_test.py.in

@@ -1463,6 +1463,41 @@ class SocketSrvTest(unittest.TestCase):
         self.assertEqual({}, self.__boss._unix_sockets)
         self.assertTrue(sock.closed)
 
+class TestFunctions(unittest.TestCase):
+    def setUp(self):
+        self.lockfile_testpath = \
+            "@abs_top_builddir@/src/bin/bind10/tests/lockfile_test"
+        self.assertFalse(os.path.exists(self.lockfile_testpath))
+        os.mkdir(self.lockfile_testpath)
+        self.assertTrue(os.path.isdir(self.lockfile_testpath))
+
+    def tearDown(self):
+        os.rmdir(self.lockfile_testpath)
+        self.assertFalse(os.path.isdir(self.lockfile_testpath))
+        os.environ["B10_LOCKFILE_DIR_FROM_BUILD"] = "@abs_top_builddir@"
+
+    def test_remove_lock_files(self):
+        os.environ["B10_LOCKFILE_DIR_FROM_BUILD"] = self.lockfile_testpath
+
+        # create lockfiles for the testcase
+        lockfiles = ["logger_lockfile"]
+        for f in lockfiles:
+            fname = os.environ["B10_LOCKFILE_DIR_FROM_BUILD"] + '/' + f
+            self.assertFalse(os.path.exists(fname))
+            open(fname, "w").close()
+            self.assertTrue(os.path.isfile(fname))
+
+        # first call should clear up all the lockfiles
+        bind10_src.remove_lock_files()
+
+        # check if the lockfiles exist
+        for f in lockfiles:
+            fname = os.environ["B10_LOCKFILE_DIR_FROM_BUILD"] + '/' + f
+            self.assertFalse(os.path.isfile(fname))
+
+        # second call should not assert anyway
+        bind10_src.remove_lock_files()
+
 if __name__ == '__main__':
     # store os.environ for test_unchanged_environment
     original_os_environ = copy.deepcopy(os.environ)

src/bin/cmdctl/tests/Makefile.am

@@ -22,5 +22,6 @@ endif
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/bin/cmdctl \
 	CMDCTL_SPEC_PATH=$(abs_top_builddir)/src/bin/cmdctl \
 	CMDCTL_SRC_PATH=$(abs_top_srcdir)/src/bin/cmdctl \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/bin/dbutil/tests/Makefile.am

@@ -3,4 +3,5 @@ SUBDIRS = . testdata
 # Tests of the update script.
 
 check-local:
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(SHELL) $(abs_builddir)/dbutil_test.sh

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 + \
-        "src" + os.sep + "bin" + os.sep + "ddns" + os.sep + "ddns.spec"
+    SPECFILE_PATH = os.environ["B10_FROM_SOURCE"] + "/src/bin/ddns"
 else:
     PREFIX = "@prefix@"
     DATAROOTDIR = "@datarootdir@"
-    SPECFILE_LOCATION = "@datadir@" + os.sep + "@PACKAGE@" + os.sep + "ddns.spec"
-    SPECFILE_LOCATION = SPECFILE_LOCATION.replace("${datarootdir}", DATAROOTDIR)\
-        .replace("${prefix}", PREFIX)
+    SPECFILE_PATH = "@datadir@/@PACKAGE@".replace("${datarootdir}", DATAROOTDIR)
+    SPECFILE_PATH = SPECFILE_PATH.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"] + \
-            "/ddns_socket"
+        SOCKET_FILE_PATH = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"]
     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
-        # (once we have any configuration)
-        answer = create_answer(0)
-        return answer
+        try:
+            if 'zones' in new_config:
+                self._zone_config = \
+                    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),
-        # just drop the packet.
-        pass
+        return True
 
     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": {},
-        "named_set_item_spec": {
+        "item_default": [],
+        "list_item_spec": {
           "item_name": "entry",
           "item_type": "map",
           "item_optional": true,
           "item_default": {
-            "update_acl": [{"action": "ACCEPT", "from": "127.0.0.1"},
-                           {"action": "ACCEPT", "from": "::1"}]
+	    "origin": "",
+	    "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
-import isc
+from isc.ddns.session import *
+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
-        # return 'ok' for now.
-        new_config = {}
+        # Update with a simple zone configuration: including an accept-all ACL
+        new_config = { 'zones': [ { 'origin': TEST_ZONE_NAME_STR,
+                                    '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/bin/stats/tests/Makefile.am

@@ -24,6 +24,7 @@ endif
 	B10_FROM_SOURCE=$(abs_top_srcdir) \
 	BIND10_MSGQ_SOCKET_FILE=$(abs_top_builddir)/msgq_socket \
 	CONFIG_TESTDATA_PATH=$(abs_top_srcdir)/src/lib/config/tests/testdata \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done
 

src/lib/asiolink/io_endpoint.h

@@ -168,8 +168,8 @@ public:
 ///
 /// This method converts the address and port of the endpoint in the textual
 /// format that other BIND 10 modules would use in logging, i.e.,
-/// - For IPv6 address: [<address>]:port (e.g., [2001:db8::5300]:53)
-/// - For IPv4 address: <address>:port (e.g., 192.0.2.53:5300)
+/// - For IPv6 address: [&lt;address&gt;]:port (e.g., [2001:db8::5300]:53)
+/// - For IPv4 address: &lt;address&gt;:port (e.g., 192.0.2.53:5300)
 ///
 /// If it's neither IPv6 nor IPv4, it converts the endpoint into text in the
 /// same format as that for IPv4, although in practice such a case is not

src/lib/datasrc/database.cc

@@ -450,7 +450,8 @@ DatabaseClient::Finder::findDelegationPoint(const isc::dns::Name& name,
     const size_t remove_labels = name.getLabelCount() - origin_label_count;
 
     // Go through all superdomains from the origin down searching for nodes
-    // that indicate a delegation (.e. NS or DNAME).
+    // that indicate a delegation (.e. NS or DNAME).  Note that we only check
+    // pure superdomains; delegation on an exact match will be detected later.
     for (int i = remove_labels; i > 0; --i) {
         const Name superdomain(name.split(i));
 
@@ -810,12 +811,14 @@ DatabaseClient::Finder::findOnNameResult(const Name& name,
     const FoundIterator cni(found.second.find(RRType::CNAME()));
     const FoundIterator wti(found.second.find(type));
 
-    if (!is_origin && (options & FIND_GLUE_OK) == 0 &&
+    if (!is_origin && (options & FIND_GLUE_OK) == 0 && type != RRType::DS() &&
         nsi != found.second.end()) {
         // A NS RRset was found at the domain we were searching for.  As it is
         // not at the origin of the zone, it is a delegation and indicates that
         // this zone is not authoritative for the data. Just return the
-        // delegation information.
+        // delegation information, except:
+        // - when we are looking for glue records (FIND_GLUE_OK), or
+        // - when the query type is DS (which cancels the delegation)
         return (logAndCreateResult(name, wildname, type, DELEGATION,
                                    nsi->second,
                                    wild ? DATASRC_DATABASE_WILDCARD_NS :

src/lib/datasrc/memory_datasrc.cc

@@ -1747,7 +1747,7 @@ generateRRsetFromIterator(ZoneIterator* iterator, LoadCallback callback) {
 }
 
 void
-InMemoryZoneFinder::load(const string& filename) {
+InMemoryZoneFinder::load(const std::string& filename) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_LOAD).arg(getOrigin()).
         arg(filename);
 

src/lib/datasrc/tests/database_unittest.cc

@@ -142,9 +142,11 @@ const char* const TEST_RECORDS[][5] = {
     {"delegation.example.org.", "NS", "3600", "", "ns.example.com."},
     {"delegation.example.org.", "NS", "3600", "",
      "ns.delegation.example.org."},
-    {"delegation.example.org.", "DS", "3600", "", "1 RSAMD5 2 abcd"},
+    {"delegation.example.org.", "DS", "3600", "", "1 1 2 abcd"},
     {"delegation.example.org.", "RRSIG", "3600", "", "NS 5 3 3600 "
      "20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE"},
+    {"delegation.example.org.", "RRSIG", "3600", "", "DS 5 3 3600 "
+     "20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE"},
     {"ns.delegation.example.org.", "A", "3600", "", "192.0.2.1"},
     {"deep.below.delegation.example.org.", "A", "3600", "", "192.0.2.1"},
 
@@ -156,6 +158,16 @@ const char* const TEST_RECORDS[][5] = {
 
     {"below.dname.example.org.", "A", "3600", "", "192.0.2.1"},
 
+    // Insecure delegation (i.e., no DS at the delegation point)
+    {"insecdelegation.example.org.", "NS", "3600", "", "ns.example.com."},
+    {"insecdelegation.example.org.", "NSEC", "3600", "",
+     "dummy.example.org. NS NSEC"},
+    // and a DS under the zone cut. Such an RR shouldn't exist in a sane zone,
+    // but it could by error or some malicious attempt.  It shouldn't confuse
+    // the implementation)
+    {"child.insecdelegation.example.org.", "DS", "3600", "", "DS 5 3 3600 "
+     "20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE"},
+
     // Broken NS
     {"brokenns1.example.org.", "A", "3600", "", "192.0.2.1"},
     {"brokenns1.example.org.", "NS", "3600", "", "ns.example.com."},
@@ -2201,6 +2213,48 @@ TYPED_TEST(DatabaseClientTest, findDelegation) {
                  DataSourceError);
 }
 
+TYPED_TEST(DatabaseClientTest, findDS) {
+    // Type DS query is an exception to the general delegation case; the NS
+    // should be ignored and it should be treated just like normal
+    // authoritative data.
+
+    boost::shared_ptr<DatabaseClient::Finder> finder(this->getFinder());
+
+    // DS exists at the delegation point.  It should be returned with result
+    // code of SUCCESS.
+    this->expected_rdatas_.push_back("1 1 2 abcd"),
+    this->expected_sig_rdatas_.push_back("DS 5 3 3600 20000101000000 "
+                                         "20000201000000 12345 example.org. "
+                                         "FAKEFAKEFAKE");
+    doFindTest(*finder, Name("delegation.example.org."),
+               RRType::DS(), RRType::DS(), this->rrttl_, ZoneFinder::SUCCESS,
+               this->expected_rdatas_, this->expected_sig_rdatas_,
+               ZoneFinder::RESULT_DEFAULT);
+
+    // DS doesn't exist at the delegation point.  The result should be
+    // NXRRSET, and if DNSSEC is requested and the zone is NSEC-signed,
+    // the corresponding NSEC should be returned (normally with its RRSIG,
+    // but in this simplified test setup it's omitted in the test data).
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("dummy.example.org. NS NSEC");
+    this->expected_sig_rdatas_.clear();
+    doFindTest(*finder, Name("insecdelegation.example.org."),
+               RRType::DS(), RRType::NSEC(), this->rrttl_, ZoneFinder::NXRRSET,
+               this->expected_rdatas_, this->expected_sig_rdatas_,
+               ZoneFinder::RESULT_NSEC_SIGNED,
+               Name("insecdelegation.example.org."), ZoneFinder::FIND_DNSSEC);
+
+    // Some insane case: DS under a zone cut.  It's included in the DB, but
+    // shouldn't be visible via finder.
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("ns.example.com");
+    doFindTest(*finder, Name("child.insecdelegation.example.org"),
+               RRType::DS(), RRType::NS(), this->rrttl_,
+               ZoneFinder::DELEGATION, this->expected_rdatas_,
+               this->empty_rdatas_, ZoneFinder::RESULT_DEFAULT,
+               Name("insecdelegation.example.org."), ZoneFinder::FIND_DNSSEC);
+}
+
 TYPED_TEST(DatabaseClientTest, emptyDomain) {
     boost::shared_ptr<DatabaseClient::Finder> finder(this->getFinder());
 

src/lib/dns/labelsequence.h

@@ -101,7 +101,7 @@ public:
     /// \note No actual memory is changed, this operation merely updates the
     /// internal pointers based on the offsets in the Name object.
     ///
-    /// \exeption OutOfRange if i is greater than or equal to the number
+    /// \exception OutOfRange if i is greater than or equal to the number
     ///           of labels currently pointed to by this LabelSequence
     ///
     /// \param i The number of labels to remove.
@@ -112,7 +112,7 @@ public:
     /// \note No actual memory is changed, this operation merely updates the
     /// internal pointers based on the offsets in the Name object.
     ///
-    /// \exeption OutOfRange if i is greater than or equal to the number
+    /// \exception OutOfRange if i is greater than or equal to the number
     ///           of labels currently pointed to by this LabelSequence
     ///
     /// \param i The number of labels to remove.

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/rdata_python.cc

@@ -116,6 +116,7 @@ Rdata_init(PyObject* self_p, PyObject* args, PyObject*) {
             return (0);
         } else if (PyArg_ParseTuple(args, "O!O!y#", &rrtype_type, &rrtype,
                                     &rrclass_type, &rrclass, &data, &len)) {
+            PyErr_Clear();
             InputBuffer input_buffer(data, len);
             self->cppobj = createRdata(PyRRType_ToRRType(rrtype),
                                        PyRRClass_ToRRClass(rrclass),

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

@@ -309,6 +309,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/rdata.cc

@@ -119,7 +119,7 @@ Generic::Generic(isc::util::InputBuffer& buffer, size_t rdata_len) {
     impl_ = new GenericImpl(data);
 }
 
-Generic::Generic(const string& rdata_string) {
+Generic::Generic(const std::string& rdata_string) {
     istringstream iss(rdata_string);
     string unknown_mark;
     iss >> unknown_mark;

src/lib/dns/rdata/any_255/tsig_250.cc

@@ -74,25 +74,28 @@ struct TSIG::TSIGImpl {
 /// \code <Alg> <Time> <Fudge> <MACsize> [<MAC>] <OrigID> <Error> <OtherLen> [<OtherData>]
 /// \endcode
 /// where
-/// - <Alg> is a valid textual representation of domain name.
-/// - <Time> is an unsigned 48-bit decimal integer.
-/// - <MACSize>, <OrigID>, and <OtherLen> are an unsigned 16-bit decimal
+/// - &lt;Alg&gt; is a valid textual representation of domain name.
+/// - &lt;Time&gt; is an unsigned 48-bit decimal integer.
+/// - &lt;MACSize&gt;, &lt;OrigID&gt;, and &lt;OtherLen&gt; are an unsigned
+///   16-bit decimal
 ///   integer.
-/// - <Error> is an unsigned 16-bit decimal integer or a valid mnemonic for
-///   the Error field specified in RFC2845.  Currently, "BADSIG", "BADKEY",
+/// - &lt;Error&gt; is an unsigned 16-bit decimal integer or a valid mnemonic
+///   for the Error field specified in RFC2845.  Currently, "BADSIG", "BADKEY",
 ///   and "BADTIME" are supported (case sensitive).  In future versions
 ///   other representations that are compatible with the DNS RCODE will be
 ///   supported.
-/// - <MAC> and <OtherData> is a BASE-64 encoded string that does not contain
-///   space characters.
-///   When <MACSize> and <OtherLen> is 0, <MAC> and <OtherData> must not
-///   appear in \c tsgi_str, respectively.
-/// - The decoded data of <MAC> is <MACSize> bytes of binary stream.
-/// - The decoded data of <OtherData> is <OtherLen> bytes of binary stream.
+/// - &lt;MAC&gt; and &lt;OtherData&gt; is a BASE-64 encoded string that does
+///   not contain space characters.
+///   When &lt;MACSize&gt; and &lt;OtherLen&gt; is 0, &lt;MAC&gt; and
+///   &lt;OtherData&gt; must not appear in \c tsig_str, respectively.
+/// - The decoded data of &lt;MAC&gt; is &lt;MACSize&gt; bytes of binary
+///   stream.
+/// - The decoded data of &lt;OtherData&gt; is &lt;OtherLen&gt; bytes of
+///   binary stream.
 ///
 /// An example of valid string is:
 /// \code "hmac-sha256. 853804800 300 3 AAAA 2845 0 0" \endcode
-/// In this example <OtherData> is missing because <OtherLen> is 0.
+/// In this example &lt;OtherData&gt; is missing because &lt;OtherLen&gt; is 0.
 ///
 /// Note that RFC2845 does not define the standard presentation format
 /// of %TSIG RR, so the above syntax is implementation specific.
@@ -101,10 +104,10 @@ struct TSIG::TSIGImpl {
 ///
 /// <b>Exceptions</b>
 ///
-/// If <Alg> is not a valid domain name, a corresponding exception from
+/// If &lt;Alg&gt; is not a valid domain name, a corresponding exception from
 /// the \c Name class will be thrown;
-/// if <MAC> or <OtherData> is not validly encoded in BASE-64, an exception
-/// of class \c isc::BadValue will be thrown;
+/// if &lt;MAC&gt; or &lt;OtherData&gt; is not validly encoded in BASE-64, an
+/// exception of class \c isc::BadValue will be thrown;
 /// if %any of the other bullet points above is not met, an exception of
 /// class \c InvalidRdataText will be thrown.
 /// This constructor internally involves resource allocation, and if it fails

src/lib/dns/rdata/ch_3/a_1.cc

@@ -27,7 +27,7 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-A::A(const string&) {
+A::A(const std::string&) {
     // TBD
 }
 

src/lib/dns/rdata/generic/dlv_32769.cc

@@ -34,7 +34,7 @@ using namespace isc::dns::rdata::generic::detail;
 /// \brief Constructor from string.
 ///
 /// A copy of the implementation object is allocated and constructed.
-DLV::DLV(const string& ds_str) :
+DLV::DLV(const std::string& ds_str) :
     impl_(new DLVImpl(ds_str))
 {}
 

src/lib/dns/rdata/generic/dnskey_48.cc

@@ -51,7 +51,7 @@ struct DNSKEYImpl {
     const vector<uint8_t> keydata_;
 };
 
-DNSKEY::DNSKEY(const string& dnskey_str) :
+DNSKEY::DNSKEY(const std::string& dnskey_str) :
     impl_(NULL)
 {
     istringstream iss(dnskey_str);

src/lib/dns/rdata/generic/ds_43.cc

@@ -31,7 +31,7 @@ using namespace isc::dns::rdata::generic::detail;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-DS::DS(const string& ds_str) :
+DS::DS(const std::string& ds_str) :
     impl_(new DSImpl(ds_str))
 {}
 

src/lib/dns/rdata/generic/hinfo_13.cc

@@ -37,7 +37,7 @@ using namespace isc::dns::characterstr;
 // BEGIN_RDATA_NAMESPACE
 
 
-HINFO::HINFO(const string& hinfo_str) {
+HINFO::HINFO(const std::string& hinfo_str) {
     string::const_iterator input_iterator = hinfo_str.begin();
     cpu_ = getNextCharacterString(hinfo_str, input_iterator);
 

src/lib/dns/rdata/generic/nsec3_50.cc

@@ -64,7 +64,7 @@ struct NSEC3Impl {
     const vector<uint8_t> typebits_;
 };
 
-NSEC3::NSEC3(const string& nsec3_str) :
+NSEC3::NSEC3(const std::string& nsec3_str) :
     impl_(NULL)
 {
     istringstream iss(nsec3_str);

src/lib/dns/rdata/generic/nsec3param_51.cc

@@ -46,7 +46,7 @@ struct NSEC3PARAMImpl {
     const vector<uint8_t> salt_;
 };
 
-NSEC3PARAM::NSEC3PARAM(const string& nsec3param_str) :
+NSEC3PARAM::NSEC3PARAM(const std::string& nsec3param_str) :
     impl_(NULL)
 {
     istringstream iss(nsec3param_str);

src/lib/dns/rdata/generic/nsec_47.cc

@@ -49,7 +49,7 @@ struct NSECImpl {
     vector<uint8_t> typebits_;
 };
 
-NSEC::NSEC(const string& nsec_str) :
+NSEC::NSEC(const std::string& nsec_str) :
     impl_(NULL)
 {
     istringstream iss(nsec_str);

src/lib/dns/rdata/generic/opt_41.cc

@@ -27,7 +27,7 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-OPT::OPT(const string&) {
+OPT::OPT(const std::string&) {
     isc_throw(InvalidRdataText, "OPT RR cannot be constructed from text");
 }
 

src/lib/dns/rdata/generic/ptr_12.cc

@@ -28,7 +28,7 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-PTR::PTR(const string& type_str) :
+PTR::PTR(const std::string& type_str) :
     ptr_name_(type_str)
 {}
 

src/lib/dns/rdata/generic/rrsig_46.cc

@@ -72,7 +72,7 @@ struct RRSIGImpl {
     const vector<uint8_t> signature_;
 };
 
-RRSIG::RRSIG(const string& rrsig_str) :
+RRSIG::RRSIG(const std::string& rrsig_str) :
     impl_(NULL)
 {
     istringstream iss(rrsig_str);

src/lib/dns/rdata/generic/soa_6.cc

@@ -41,7 +41,7 @@ SOA::SOA(InputBuffer& buffer, size_t) :
     buffer.readData(numdata_, sizeof(numdata_));
 }
 
-SOA::SOA(const string& soastr) :
+SOA::SOA(const std::string& soastr) :
     mname_("."), rname_(".")    // quick hack workaround
 {
     istringstream iss(soastr);

src/lib/dns/rdata/generic/sshfp_44.cc

@@ -80,7 +80,7 @@ SSHFP::SSHFP(const std::string& sshfp_str)
     decodeHex(fingerprintbuf.str(), fingerprint_);
 }
 
-SSHFP::SSHFP(uint8_t algorithm, uint8_t fingerprint_type, const string& fingerprint)
+SSHFP::SSHFP(uint8_t algorithm, uint8_t fingerprint_type, const std::string& fingerprint)
 {
     if ((algorithm < 1) || (algorithm > 2)) {
       isc_throw(InvalidRdataText, "SSHFP algorithm number out of range");

src/lib/dns/rdata/hs_4/a_1.cc

@@ -27,7 +27,7 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-A::A(const string&) {
+A::A(const std::string&) {
     // TBD
 }
 

src/lib/dns/rdata/in_1/a_1.cc

@@ -34,7 +34,7 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-A::A(const string& addrstr) {
+A::A(const std::string& addrstr) {
     // RFC1035 states textual representation of IN/A RDATA is
     // "four decimal numbers separated by dots without any embedded spaces".
     // This is exactly what inet_pton() accepts for AF_INET.  In particular,

src/lib/dns/rdata/in_1/aaaa_28.cc

@@ -34,7 +34,7 @@ using namespace isc::util;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-AAAA::AAAA(const string& addrstr) {
+AAAA::AAAA(const std::string& addrstr) {
     if (inet_pton(AF_INET6, addrstr.c_str(), &addr_) != 1) {
         isc_throw(InvalidRdataText,
                   "IN/AAAA RDATA construction from text failed: "

src/lib/dns/rdata/in_1/dhcid_49.cc

@@ -47,7 +47,7 @@ using namespace isc::util;
 ///           < n octets >    Digest (length depends on digest type)
 /// If the data is less than 3 octets (i.e. it cannot contain id type code and
 /// digest type code), an exception of class \c InvalidRdataLength is thrown.
-DHCID::DHCID(const string& dhcid_str) {
+DHCID::DHCID(const std::string& dhcid_str) {
     istringstream iss(dhcid_str);
     stringbuf digestbuf;
 

src/lib/dns/rdata/in_1/srv_33.cc

@@ -52,22 +52,22 @@ struct SRVImpl {
 /// \code <Priority> <Weight> <Port> <Target>
 /// \endcode
 /// where
-/// - <Priority>, <Weight>, and <Port> are an unsigned 16-bit decimal
-///   integer.
-/// - <Target> is a valid textual representation of domain name.
+/// - &lt;Priority&gt;, &lt;Weight&gt;, and &lt;Port&gt; are an unsigned
+///   16-bit decimal integer.
+/// - &lt;Target&gt; is a valid textual representation of domain name.
 ///
 /// An example of valid string is:
 /// \code "1 5 1500 example.com." \endcode
 ///
 /// <b>Exceptions</b>
 ///
-/// If <Target> is not a valid domain name, a corresponding exception from
-/// the \c Name class will be thrown;
+/// If &lt;Target&gt; is not a valid domain name, a corresponding exception
+/// from the \c Name class will be thrown;
 /// if %any of the other bullet points above is not met, an exception of
 /// class \c InvalidRdataText will be thrown.
 /// This constructor internally involves resource allocation, and if it fails
 /// a corresponding standard exception will be thrown.
-SRV::SRV(const string& srv_str) :
+SRV::SRV(const std::string& srv_str) :
     impl_(NULL)
 {
     istringstream iss(srv_str);

src/lib/dns/rrclass.cc

@@ -30,7 +30,7 @@ using namespace isc::util;
 namespace isc {
 namespace dns {
 
-RRClass::RRClass(const string& classstr) {
+RRClass::RRClass(const std::string& classstr) {
     classcode_ = RRParamRegistry::getRegistry().textToClassCode(classstr);
 }
 

src/lib/dns/rrparamregistry-placeholder.cc

@@ -224,7 +224,7 @@ RRParamRegistry::getRegistry() {
 }
 
 void
-RRParamRegistry::add(const string& typecode_string, uint16_t typecode,
+RRParamRegistry::add(const std::string& typecode_string, uint16_t typecode,
                      RdataFactoryPtr rdata_factory)
 {
     bool type_added = false;
@@ -242,8 +242,8 @@ RRParamRegistry::add(const string& typecode_string, uint16_t typecode,
 }
 
 void
-RRParamRegistry::add(const string& typecode_string, uint16_t typecode,
-                     const string& classcode_string, uint16_t classcode,
+RRParamRegistry::add(const std::string& typecode_string, uint16_t typecode,
+                     const std::string& classcode_string, uint16_t classcode,
                      RdataFactoryPtr rdata_factory)
 {
     // Rollback logic on failure is complicated.  If adding the new type or
@@ -470,7 +470,7 @@ RRParamRegistry::codeToClassText(uint16_t code) const {
 
 RdataPtr
 RRParamRegistry::createRdata(const RRType& rrtype, const RRClass& rrclass,
-                             const string& rdata_string)
+                             const std::string& rdata_string)
 {
     // If the text indicates that it's rdata of an "unknown" type (beginning
     // with '\# n'), parse it that way. (TBD)

src/lib/dns/rrttl.cc

@@ -28,7 +28,7 @@ using namespace isc::util;
 namespace isc {
 namespace dns {
 
-RRTTL::RRTTL(const string& ttlstr) {
+RRTTL::RRTTL(const std::string& ttlstr) {
     // Some systems (at least gcc-4.4) flow negative values over into
     // unsigned integer, where older systems failed to parse. We want
     // that failure here, so we extract into int64 and check the value

src/lib/dns/rrtype.cc

@@ -31,7 +31,7 @@ using isc::dns::RRType;
 namespace isc {
 namespace dns {
 
-RRType::RRType(const string& typestr) {
+RRType::RRType(const std::string& typestr) {
     typecode_ = RRParamRegistry::getRegistry().textToTypeCode(typestr);
 }
 

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/log/Makefile.am

@@ -2,6 +2,7 @@ SUBDIRS = . compiler tests
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
+AM_CPPFLAGS += -DTOP_BUILDDIR=\"${abs_top_builddir}\"
 
 CLEANFILES = *.gcno *.gcda
 

src/lib/log/compiler/message.cc

@@ -58,14 +58,14 @@ static const char* VERSION = "1.0-0";
 /// \b Invocation<BR>
 /// The program is invoked with the command:
 ///
-/// <tt>message [-v | -h | -p | -d <dir> | <message-file>]</tt>
+/// <tt>message [-v | -h | -p | -d &lt;dir&gt; | <message-file>]</tt>
 ///
 /// It reads the message file and writes out two files of the same
 /// name in the current working directory (unless -d is used) but
 /// with extensions of .h and .cc, or .py if -p is used.
 ///
 /// -v causes it to print the version number and exit. -h prints a help
-/// message (and exits). -p sets the output to python. -d <dir> will make
+/// message (and exits). -p sets the output to python. -d &lt;dir&gt; will make
 /// it write the output file(s) to dir instead of current working
 /// directory
 
@@ -119,9 +119,9 @@ currentTime() {
 
 /// \brief Create Header Sentinel
 ///
-/// Given the name of a file, create an #ifdef sentinel name.  The name is
-/// __<name>_<ext>, where <name> is the name of the file, and <ext> is the
-/// extension less the leading period.  The sentinel will be upper-case.
+/// Given the name of a file, create an \#ifdef sentinel name.  The name is
+/// __<name>_<ext>, where &lt;name&gt; is the name of the file, and &lt;ext&gt;
+/// is the extension less the leading period.  The sentinel will be upper-case.
 ///
 /// \param file Filename object representing the file.
 ///

src/lib/log/logger.cc

@@ -179,6 +179,13 @@ Logger::fatal(const isc::log::MessageID& ident) {
     }
 }
 
+// Replace the interprocess synchronization object
+
+void
+Logger::setInterprocessSync(isc::util::InterprocessSync* sync) {
+    getLoggerPtr()->setInterprocessSync(sync);
+}
+
 // Comparison (testing only)
 
 bool

src/lib/log/logger.h

@@ -25,6 +25,7 @@
 #include <log/message_types.h>
 #include <log/log_formatter.h>
 
+#include <util/interprocess_sync.h>
 
 namespace isc {
 namespace log {
@@ -98,6 +99,17 @@ public:
     {}
 };
 
+/// \brief Bad Interprocess Sync
+///
+/// Exception thrown if a bad InterprocessSync object (such as NULL) is
+/// used.
+class BadInterprocessSync : public isc::Exception {
+public:
+    BadInterprocessSync(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what)
+    {}
+};
+
 /// \brief Logger Class
 ///
 /// This class is the main class used for logging.  Use comprises:
@@ -237,6 +249,17 @@ public:
     /// \param ident Message identification.
     Formatter fatal(const MessageID& ident);
 
+    /// \brief Replace the interprocess synchronization object
+    ///
+    /// If this method is called with NULL as the argument, it throws a
+    /// BadInterprocessSync exception.
+    ///
+    /// \param sync The logger uses this synchronization object for
+    /// synchronizing output of log messages. It should be deletable and
+    /// the ownership is transferred to the logger. If NULL is passed,
+    /// a BadInterprocessSync exception is thrown.
+    void setInterprocessSync(isc::util::InterprocessSync* sync);
+
     /// \brief Equality
     ///
     /// Check if two instances of this logger refer to the same stream.

src/lib/log/logger_impl.cc

@@ -32,12 +32,14 @@
 #include <log/message_types.h>
 
 #include <util/strutil.h>
+#include <util/interprocess_sync_file.h>
 
 // Note: as log4cplus and the BIND 10 logger have many concepts in common, and
 // thus many similar names, to disambiguate types we don't "use" the log4cplus
 // namespace: instead, all log4cplus types are explicitly qualified.
 
 using namespace std;
+using namespace isc::util;
 
 namespace isc {
 namespace log {
@@ -47,14 +49,17 @@ namespace log {
 // one compiler requires that all member variables be constructed before the
 // constructor is run, but log4cplus::Logger (the type of logger_) has no
 // default constructor.
-LoggerImpl::LoggerImpl(const string& name) : name_(expandLoggerName(name)),
-    logger_(log4cplus::Logger::getInstance(name_))
+LoggerImpl::LoggerImpl(const string& name) :
+    name_(expandLoggerName(name)),
+    logger_(log4cplus::Logger::getInstance(name_)),
+    sync_(new InterprocessSyncFile("logger"))
 {
 }
 
 // Destructor. (Here because of virtual declaration.)
 
 LoggerImpl::~LoggerImpl() {
+    delete sync_;
 }
 
 // Set the severity for logging.
@@ -102,8 +107,30 @@ LoggerImpl::lookupMessage(const MessageID& ident) {
                        MessageDictionary::globalDictionary().getText(ident)));
 }
 
+// Replace the interprocess synchronization object
+
+void
+LoggerImpl::setInterprocessSync(isc::util::InterprocessSync* sync) {
+    if (sync == NULL) {
+        isc_throw(BadInterprocessSync,
+                  "NULL was passed to setInterprocessSync()");
+    }
+
+    delete sync_;
+    sync_ = sync;
+}
+
 void
 LoggerImpl::outputRaw(const Severity& severity, const string& message) {
+    // Use an interprocess sync locker for mutual exclusion from other
+    // processes to avoid log messages getting interspersed.
+
+    InterprocessSyncLocker locker(*sync_);
+
+    if (!locker.lock()) {
+        LOG4CPLUS_ERROR(logger_, "Unable to lock logger lockfile");
+    }
+
     switch (severity) {
         case DEBUG:
             LOG4CPLUS_DEBUG(logger_, message);
@@ -124,6 +151,10 @@ LoggerImpl::outputRaw(const Severity& severity, const string& message) {
         case FATAL:
             LOG4CPLUS_FATAL(logger_, message);
     }
+
+    if (!locker.unlock()) {
+        LOG4CPLUS_ERROR(logger_, "Unable to unlock logger lockfile");
+    }
 }
 
 } // namespace log

src/lib/log/logger_impl.h

@@ -32,6 +32,8 @@
 #include <log/logger_level_impl.h>
 #include <log/message_types.h>
 
+#include <util/interprocess_sync.h>
+
 namespace isc {
 namespace log {
 
@@ -167,6 +169,17 @@ public:
     /// This gets you the unformatted text of message for given ID.
     std::string* lookupMessage(const MessageID& id);
 
+    /// \brief Replace the interprocess synchronization object
+    ///
+    /// If this method is called with NULL as the argument, it throws a
+    /// BadInterprocessSync exception.
+    ///
+    /// \param sync The logger uses this synchronization object for
+    /// synchronizing output of log messages. It should be deletable and
+    /// the ownership is transferred to the logger implementation.
+    /// If NULL is passed, a BadInterprocessSync exception is thrown.
+    void setInterprocessSync(isc::util::InterprocessSync* sync);
+
     /// \brief Equality
     ///
     /// Check if two instances of this logger refer to the same stream.
@@ -178,8 +191,9 @@ public:
     }
 
 private:
-    std::string         name_;              ///< Full name of this logger
-    log4cplus::Logger   logger_;            ///< Underlying log4cplus logger
+    std::string                  name_;   ///< Full name of this logger
+    log4cplus::Logger            logger_; ///< Underlying log4cplus logger
+    isc::util::InterprocessSync* sync_;
 };
 
 } // namespace log

src/lib/log/logger_manager.cc

@@ -28,6 +28,7 @@
 #include <log/message_initializer.h>
 #include <log/message_reader.h>
 #include <log/message_types.h>
+#include "util/interprocess_sync_null.h"
 
 using namespace std;
 
@@ -148,6 +149,13 @@ LoggerManager::readLocalMessageFile(const char* file) {
 
     MessageDictionary& dictionary = MessageDictionary::globalDictionary();
     MessageReader reader(&dictionary);
+
+    // Turn off use of any lock files. This is because this logger can
+    // be used by standalone programs which may not have write access to
+    // the local state directory (to create lock files). So we switch to
+    // using a null interprocess sync object here.
+    logger.setInterprocessSync(new isc::util::InterprocessSyncNull("logger"));
+
     try {
 
         logger.info(LOG_READING_LOCAL_FILE).arg(file);

src/lib/log/logger_unittest_support.cc

@@ -160,6 +160,9 @@ void initLogger(isc::log::Severity severity, int dbglevel) {
     // Set the local message file
     const char* localfile = getenv("B10_LOGGER_LOCALMSG");
 
+    // Set a directory for creating lockfiles when running tests
+    setenv("B10_LOCKFILE_DIR_FROM_BUILD", TOP_BUILDDIR, 1);
+
     // Initialize logging
     initLogger(root, isc::log::DEBUG, isc::log::MAX_DEBUG_LEVEL, localfile);
 

src/lib/log/message_dictionary.cc

@@ -29,7 +29,7 @@ MessageDictionary::~MessageDictionary() {
 // Add message and note if ID already exists
 
 bool
-MessageDictionary::add(const string& ident, const string& text) {
+MessageDictionary::add(const std::string& ident, const std::string& text) {
     Dictionary::iterator i = dictionary_.find(ident);
     bool not_found = (i == dictionary_.end());
     if (not_found) {
@@ -44,7 +44,7 @@ MessageDictionary::add(const string& ident, const string& text) {
 // Add message and note if ID does not already exist
 
 bool
-MessageDictionary::replace(const string& ident, const string& text) {
+MessageDictionary::replace(const std::string& ident, const std::string& text) {
     Dictionary::iterator i = dictionary_.find(ident);
     bool found = (i != dictionary_.end());
     if (found) {
@@ -87,7 +87,7 @@ MessageDictionary::load(const char* messages[]) {
 // output.
 
 const string&
-MessageDictionary::getText(const string& ident) const {
+MessageDictionary::getText(const std::string& ident) const {
     static const string empty("");
     Dictionary::const_iterator i = dictionary_.find(ident);
     if (i == dictionary_.end()) {

src/lib/log/message_exception.h

@@ -38,6 +38,9 @@ public:
 
     /// \brief Constructor
     ///
+    /// \param file Filename where the exception occurred.
+    /// \param line Line where exception occurred.
+    /// \param what Text description of the problem.
     /// \param id Message identification.
     /// \param lineno Line number on which error occurred (if > 0).
     MessageException(const char* file, size_t line, const char* what,
@@ -51,6 +54,9 @@ public:
 
     /// \brief Constructor
     ///
+    /// \param file Filename where the exception occurred.
+    /// \param line Line where exception occurred.
+    /// \param what Text description of the problem.
     /// \param id Message identification.
     /// \param arg1 First message argument.
     /// \param lineno Line number on which error occurred (if > 0).
@@ -66,6 +72,9 @@ public:
 
     /// \brief Constructor
     ///
+    /// \param file Filename where the exception occurred.
+    /// \param line Line where exception occurred.
+    /// \param what Text description of the problem.
     /// \param id Message identification.
     /// \param arg1 First message argument.
     /// \param arg2 Second message argument.

src/lib/log/tests/.gitignore

@@ -6,6 +6,10 @@
 /initializer_unittests_2
 /local_file_test.sh
 /logger_example
+/logger_lock_test
+/logger_lock_test.sh
+/log_test_messages.cc
+/log_test_messages.h
 /run_unittests
 /severity_test.sh
 /tempdir.h

src/lib/log/tests/Makefile.am

@@ -12,6 +12,13 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+EXTRA_DIST = log_test_messages.mes
+BUILT_SOURCES = log_test_messages.h log_test_messages.cc
+log_test_messages.h log_test_messages.cc: log_test_messages.mes
+	$(AM_V_GEN) $(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/log/tests/log_test_messages.mes
+
+CLEANFILES += log_test_messages.h log_test_messages.cc
+
 noinst_PROGRAMS = logger_example
 logger_example_SOURCES = logger_example.cc
 logger_example_CPPFLAGS = $(AM_CPPFLAGS)
@@ -30,6 +37,16 @@ init_logger_test_LDADD += $(top_builddir)/src/lib/util/libutil.la
 init_logger_test_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 init_logger_test_LDADD += $(AM_LDADD) $(LOG4CPLUS_LIBS)
 
+noinst_PROGRAMS += logger_lock_test
+logger_lock_test_SOURCES = logger_lock_test.cc
+nodist_logger_lock_test_SOURCES = log_test_messages.cc log_test_messages.h
+logger_lock_test_CPPFLAGS = $(AM_CPPFLAGS)
+logger_lock_test_LDFLAGS = $(AM_LDFLAGS)
+logger_lock_test_LDADD  = $(top_builddir)/src/lib/log/liblog.la
+logger_lock_test_LDADD += $(top_builddir)/src/lib/util/libutil.la
+logger_lock_test_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+logger_lock_test_LDADD += $(AM_LDADD) $(LOG4CPLUS_LIBS)
+
 if HAVE_GTEST
 TESTS =
 
@@ -62,6 +79,7 @@ run_unittests_SOURCES += logger_specification_unittest.cc
 run_unittests_SOURCES += message_dictionary_unittest.cc
 run_unittests_SOURCES += message_reader_unittest.cc
 run_unittests_SOURCES += output_option_unittest.cc
+nodist_run_unittests_SOURCES = log_test_messages.cc log_test_messages.h
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS)
 run_unittests_CXXFLAGS = $(AM_CXXFLAGS)
@@ -104,4 +122,5 @@ check-local:
 	$(SHELL) $(abs_builddir)/destination_test.sh
 	$(SHELL) $(abs_builddir)/init_logger_test.sh
 	$(SHELL) $(abs_builddir)/local_file_test.sh
+	$(SHELL) $(abs_builddir)/logger_lock_test.sh
 	$(SHELL) $(abs_builddir)/severity_test.sh

src/lib/log/tests/log_test_messages.mes

@@ -0,0 +1,26 @@
+# Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+# REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+# AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+# LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+# OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+# PERFORMANCE OF THIS SOFTWARE.
+
+# \brief Message Utility Message File
+#
+# This is the source of the set of messages generated by the message and
+# logging components.  The associated .h and .cc files are created by hand from
+# this file though and are not built during the build process; this is to avoid
+# the chicken-and-egg situation where we need the files to build the message
+# compiler, yet we need the compiler to build the files.
+
+$NAMESPACE isc::log
+
+% LOG_LOCK_TEST_MESSAGE this is a test message.
+This is a log message used in testing.

src/lib/log/tests/logger_example.cc

@@ -41,6 +41,7 @@
 
 // Include a set of message definitions.
 #include <log/log_messages.h>
+#include "util/interprocess_sync_null.h"
 
 using namespace isc::log;
 using namespace std;
@@ -280,10 +281,17 @@ int main(int argc, char** argv) {
         LoggerManager::readLocalMessageFile(argv[optind]);
     }
 
-    // Log a few messages to different loggers.
+    // Log a few messages to different loggers. Here, we switch to using
+    // null interprocess sync objects for the loggers below as the
+    // logger example can be used as a standalone program (which may not
+    // have write access to a local state directory to create
+    // lockfiles).
     isc::log::Logger logger_ex(ROOT_NAME);
+    logger_ex.setInterprocessSync(new isc::util::InterprocessSyncNull("logger"));
     isc::log::Logger logger_alpha("alpha");
+    logger_alpha.setInterprocessSync(new isc::util::InterprocessSyncNull("logger"));
     isc::log::Logger logger_beta("beta");
+    logger_beta.setInterprocessSync(new isc::util::InterprocessSyncNull("logger"));
 
     LOG_FATAL(logger_ex, LOG_WRITE_ERROR).arg("test1").arg("42");
     LOG_ERROR(logger_ex, LOG_READING_LOCAL_FILE).arg("dummy/file");

src/lib/log/tests/logger_lock_test.cc

@@ -0,0 +1,64 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <log/macros.h>
+#include <log/logger_support.h>
+#include <log/log_messages.h>
+#include "util/interprocess_sync.h"
+#include "log_test_messages.h"
+#include <iostream>
+
+using namespace std;
+using namespace isc::log;
+
+class MockLoggingSync : public isc::util::InterprocessSync {
+public:
+    /// \brief Constructor
+    MockLoggingSync(const std::string& component_name) :
+        InterprocessSync(component_name)
+    {}
+
+protected:
+    virtual bool lock() {
+        cout << "FIELD1 FIELD2 LOGGER_LOCK_TEST: LOCK\n";
+        return (true);
+    }
+
+    virtual bool tryLock() {
+        cout << "FIELD1 FIELD2 LOGGER_LOCK_TEST: TRYLOCK\n";
+        return (true);
+    }
+
+    virtual bool unlock() {
+        cout << "FIELD1 FIELD2 LOGGER_LOCK_TEST: UNLOCK\n";
+        return (true);
+    }
+};
+
+/// \brief Test logger lock sequence
+///
+/// A program used in testing the logger. It verifies that (1) an
+/// interprocess sync lock is first acquired by the logger, (2) the
+/// message is logged by the logger, and (3) the lock is released in
+/// that sequence.
+int
+main(int, char**) {
+    initLogger();
+    Logger logger("log");
+    logger.setInterprocessSync(new MockLoggingSync("log"));
+
+    LOG_INFO(logger, LOG_LOCK_TEST_MESSAGE);
+
+    return (0);
+}

src/lib/log/tests/logger_lock_test.sh.in

@@ -0,0 +1,46 @@
+#!/bin/sh
+# Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+# REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+# AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+# LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+# OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+# PERFORMANCE OF THIS SOFTWARE.
+
+# Checks that the locker interprocess sync locks are acquired and
+# released correctly.
+
+failcount=0
+tempfile=@abs_builddir@/logger_lock_test_tempfile_$$
+destfile=@abs_builddir@/logger_lock_test_destfile_$$
+
+passfail() {
+    if [ $1 -eq 0 ]; then
+        echo " pass"
+    else
+        echo " FAIL"
+        failcount=`expr $failcount + $1`
+    fi
+}
+
+echo -n  "Testing that logger acquires and releases locks correctly:"
+cat > $tempfile << .
+LOGGER_LOCK_TEST: LOCK
+INFO  [bind10.log] LOG_LOCK_TEST_MESSAGE this is a test message.
+LOGGER_LOCK_TEST: UNLOCK
+.
+rm -f $destfile
+B10_LOGGER_SEVERITY=INFO B10_LOGGER_DESTINATION=stdout ./logger_lock_test > $destfile
+cut -d' ' -f3- $destfile | diff $tempfile -
+passfail $?
+
+# Tidy up.
+rm -f $tempfile $destfile
+
+exit $failcount

src/lib/log/tests/logger_unittest.cc

@@ -23,6 +23,9 @@
 #include <log/logger_manager.h>
 #include <log/logger_name.h>
 #include <log/log_messages.h>
+#include "log/tests/log_test_messages.h"
+
+#include <util/interprocess_sync_file.h>
 
 using namespace isc;
 using namespace isc::log;
@@ -379,3 +382,66 @@ TEST_F(LoggerTest, LoggerNameLength) {
     }, ".*");
 #endif
 }
+
+TEST_F(LoggerTest, setInterprocessSync) {
+    // Create a logger
+    Logger logger("alpha");
+
+    EXPECT_THROW(logger.setInterprocessSync(NULL), BadInterprocessSync);
+}
+
+class MockSync : public isc::util::InterprocessSync {
+public:
+    /// \brief Constructor
+    MockSync(const std::string& component_name) :
+        InterprocessSync(component_name), was_locked_(false),
+        was_unlocked_(false)
+    {}
+
+    bool wasLocked() const {
+        return (was_locked_);
+    }
+
+    bool wasUnlocked() const {
+        return (was_unlocked_);
+    }
+
+protected:
+    bool lock() {
+        was_locked_ = true;
+        return (true);
+    }
+
+    bool tryLock() {
+        return (true);
+    }
+
+    bool unlock() {
+        was_unlocked_ = true;
+        return (true);
+    }
+
+private:
+    bool was_locked_;
+    bool was_unlocked_;
+};
+
+// Checks that the logger logs exclusively and other BIND 10 components
+// are locked out.
+
+TEST_F(LoggerTest, Lock) {
+    // Create a logger
+    Logger logger("alpha");
+
+    // Setup our own mock sync object so that we can intercept the lock
+    // call and check if a lock has been taken.
+    MockSync* sync = new MockSync("logger");
+    logger.setInterprocessSync(sync);
+
+    // Log a message and put things into play.
+    logger.setSeverity(isc::log::INFO, 100);
+    logger.info(LOG_LOCK_TEST_MESSAGE);
+
+    EXPECT_TRUE(sync->wasLocked());
+    EXPECT_TRUE(sync->wasUnlocked());
+}

src/lib/log/tests/run_initializer_unittests.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <stdlib.h>
 #include <gtest/gtest.h>
 #include <util/unittests/run_all.h>
 

src/lib/log/tests/run_unittests.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <stdlib.h>
 #include <gtest/gtest.h>
 #include <util/unittests/run_all.h>
 

src/lib/python/isc/bind10/tests/Makefile.am

@@ -23,6 +23,7 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_srcdir)/src/bin:$(abs_top_builddir)/src/bin/bind10:$(abs_top_builddir)/src/lib/util/io/.libs \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	BIND10_MSGQ_SOCKET_FILE=$(abs_top_builddir)/msgq_socket \
 		$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/python/isc/config/tests/Makefile.am

@@ -22,6 +22,7 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/python/isc/config \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \
 	CONFIG_TESTDATA_PATH=$(abs_top_srcdir)/src/lib/config/tests/testdata \
 	CONFIG_WR_TESTDATA_PATH=$(abs_top_builddir)/src/lib/config/tests/testdata \

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.
@@ -58,7 +69,7 @@ specified NAME.  Note that this prerequisite IS satisfied by
 empty nonterminals.
 
 % LIBDDNS_PREREQ_NOTZONE update client %1 for zone %2: prerequisite not in zone (%3)
-A DNS UPDATE prerequisite has a name that does not appear to be inside
+A DDNS UPDATE prerequisite has a name that does not appear to be inside
 the zone specified in the Zone section of the UPDATE message.
 The specific prerequisite is shown. A NOTZONE error response is sent to
 the client.
@@ -93,10 +104,57 @@ RRset exists (value independent).  At least one RR with a
 specified NAME and TYPE (in the zone and class specified by
 the Zone Section) must exist.
 
+% LIBDDNS_UPDATE_ADD_BAD_TYPE update client %1 for zone %2: update addition RR bad type: %3
+The Update section of a DDNS update message contains a statement
+that tries to add a record of an invalid type. Most likely the
+record has an RRType that is considered a 'meta' type, which
+cannot be zone content data. The specific record is shown.
+A FORMERR response is sent back to the client.
+
 % LIBDDNS_UPDATE_APPROVED update client %1 for zone %2 approved
 Debug message.  An update request was approved in terms of the zone's
 update ACL.
 
+% LIBDDNS_UPDATE_BAD_CLASS update client %1 for zone %2: bad class in update RR: %3
+The Update section of a DDNS update message contains an RRset with
+a bad class. The class of the update RRset must be either the same
+as the class in the Zone Section, ANY, or NONE.
+A FORMERR response is sent back to the client.
+
+% LIBDDNS_UPDATE_DATASRC_ERROR error in datasource during DDNS update: %1
+An error occured while committing the DDNS update changes to the
+datasource. The specific error is printed. A SERVFAIL response is sent
+back to the client.
+
+% LIBDDNS_UPDATE_DELETE_BAD_TYPE update client %1 for zone %2: update deletion RR bad type: %3
+The Update section of a DDNS update message contains a statement
+that tries to delete an rrset of an invalid type. Most likely the
+record has an RRType that is considered a 'meta' type, which
+cannot be zone content data. The specific record is shown.
+A FORMERR response is sent back to the client.
+
+% LIBDDNS_UPDATE_DELETE_NONZERO_TTL update client %1 for zone %2: update deletion RR has non-zero TTL: %3
+The Update section of a DDNS update message contains a 'delete rrset'
+statement with a non-zero TTL. This is not allowed by the protocol.
+A FORMERR response is sent back to the client.
+
+% LIBDDNS_UPDATE_DELETE_RRSET_NOT_EMPTY update client %1 for zone %2: update deletion RR contains data %3
+The Update section of a DDNS update message contains a 'delete rrset'
+statement with a non-empty RRset. This is not allowed by the protocol.
+A FORMERR response is sent back to the client.
+
+% LIBDDNS_UPDATE_DELETE_RR_BAD_TYPE update client %1 for zone %2: update deletion RR bad type: %3
+The Update section of a DDNS update message contains a statement
+that tries to delete one or more rrs of an invalid type. Most
+likely the records have an RRType that is considered a 'meta'
+type, which cannot be zone content data. The specific record is
+shown. A FORMERR response is sent back to the client.
+
+% LIBDDNS_UPDATE_DELETE_RR_NONZERO_TTL update client %1 for zone %2: update deletion RR has non-zero TTL: %3
+The Update section of a DDNS update message contains a 'delete rrs'
+statement with a non-zero TTL. This is not allowed by the protocol.
+A FORMERR response is sent back to the client.
+
 % LIBDDNS_UPDATE_DENIED update client %1 for zone %2 denied
 Informational message.  An update request was denied because it was
 rejected by the zone's update ACL.  When this library is used by
@@ -134,9 +192,23 @@ configuration of those clients to suppress the requests.  As specified
 in Section 3.1 of RFC2136, the receiving server will return a response
 with an RCODE of NOTAUTH.
 
-% LIBDDNS_UPDATE_PREREQUISITE_FAILED prerequisite failed in update update client %1 for zone %2: result code %3
+% LIBDDNS_UPDATE_NOTZONE update client %1 for zone %2: update RR out of zone %3
+A DDNS UPDATE record has a name that does not appear to be inside
+the zone specified in the Zone section of the UPDATE message.
+The specific update record is shown. A NOTZONE error response is
+sent to the client.
+
+% LIBDDNS_UPDATE_PREREQUISITE_FAILED prerequisite failed in update client %1 for zone %2: result code %3
 The handling of the prerequisite section (RFC2136 Section 3.2) found
 that one of the prerequisites was not satisfied. The result code
 should give more information on what prerequisite type failed.
 If the result code is FORMERR, the prerequisite section was not well-formed.
 An error response with the given result code is sent back to the client.
+
+% LIBDDNS_UPDATE_UNCAUGHT_EXCEPTION update client %1 for zone %2: uncaught exception while processing update section: %1
+An uncaught exception was encountered while processing the Update
+section of a DDNS message. The specific exception is shown in the log message.
+To make sure DDNS service is not interrupted, this problem is caught instead
+of reraised; The update is aborted, and a SERVFAIL is sent back to the client.
+This is most probably a bug in the DDNS code, but *could* be caused by
+the data source.

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

@@ -19,6 +19,8 @@ from isc.log import *
 from isc.ddns.logger import logger, ClientFormatter, ZoneFormatter,\
                             RRsetFormatter
 from isc.log_messages.libddns_messages import *
+from isc.datasrc import ZoneFinder
+import isc.xfrin.diff
 from isc.acl.acl import ACCEPT, REJECT, DROP
 import copy
 
@@ -59,6 +61,70 @@ class UpdateError(Exception):
         self.rcode = rcode
         self.nolog = nolog
 
+def foreach_rr(rrset):
+    '''
+    Generator that creates a new RRset with one RR from
+    the given RRset upon each iteration, usable in calls that
+    need to loop over an RRset and perform an action with each
+    of the individual RRs in it.
+    Example:
+    for rr in foreach_rr(rrset):
+        print(str(rr))
+    '''
+    for rdata in rrset.get_rdata():
+        rr = isc.dns.RRset(rrset.get_name(),
+                           rrset.get_class(),
+                           rrset.get_type(),
+                           rrset.get_ttl())
+        rr.add_rdata(rdata)
+        yield rr
+
+def convert_rrset_class(rrset, rrclass):
+    '''Returns a (new) rrset with the data from the given rrset,
+       but of the given class. Useful to convert from NONE and ANY to
+       a real class.
+       Note that the caller should be careful what to convert;
+       and DNS error that could happen during wire-format reading
+       could technically occur here, and is not caught by this helper.
+    '''
+    new_rrset = isc.dns.RRset(rrset.get_name(), rrclass,
+                              rrset.get_type(), rrset.get_ttl())
+    for rdata in rrset.get_rdata():
+        # Rdata class is nof modifiable, and must match rrset's
+        # class, so we need to to some ugly conversion here.
+        # And we cannot use to_text() (since the class may be unknown)
+        wire = rdata.to_wire(bytes())
+        new_rrset.add_rdata(isc.dns.Rdata(rrset.get_type(), rrclass, wire))
+    return new_rrset
+
+def collect_rrsets(collection, rrset):
+    '''
+    Helper function to collect similar rrsets.
+    Collect all rrsets with the same name, class, and type
+    collection is the currently collected list of RRsets,
+    rrset is the RRset to add;
+    if an RRset with the same name, class and type as the
+    given rrset exists in the collection, its rdata fields
+    are added to that RRset. Otherwise, the rrset is added
+    to the given collection.
+    TTL is ignored.
+    This method does not check rdata contents for duplicate
+    values.
+
+    The collection and its rrsets are modified in-place,
+    this method does not return anything.
+    '''
+    found = False
+    for existing_rrset in collection:
+        if existing_rrset.get_name() == rrset.get_name() and\
+           existing_rrset.get_class() == rrset.get_class() and\
+           existing_rrset.get_type() == rrset.get_type():
+            for rdata in rrset.get_rdata():
+                existing_rrset.add_rdata(rdata)
+            found = True
+    if not found:
+        collection.append(rrset)
+
 class UpdateSession:
     '''Protocol handling for a single dynamic update request.
 
@@ -89,6 +155,7 @@ class UpdateSession:
         self.__tsig = req_message.get_tsig_record()
         self.__client_addr = client_addr
         self.__zone_config = zone_config
+        self.__added_soa = None
 
     def get_message(self):
         '''Return the update message.
@@ -122,17 +189,18 @@ class UpdateSession:
 
         '''
         try:
-            datasrc_client, zname, zclass = self.__get_update_zone()
-            # conceptual code that would follow
-            prereq_result = self.__check_prerequisites(datasrc_client,
-                                                       zname, zclass)
+            self.__get_update_zone()
+            prereq_result = self.__check_prerequisites()
             if prereq_result != Rcode.NOERROR():
                 self.__make_response(prereq_result)
-                return UPDATE_ERROR, zname, zclass
-            self.__check_update_acl(zname, zclass)
-            # self.__do_update()
-            # self.__make_response(Rcode.NOERROR())
-            return UPDATE_SUCCESS, zname, zclass
+                return UPDATE_ERROR, self.__zname, self.__zclass
+            self.__check_update_acl(self.__zname, self.__zclass)
+            update_result = self.__do_update()
+            if update_result != Rcode.NOERROR():
+                self.__make_response(update_result)
+                return UPDATE_ERROR, self.__zname, self.__zclass
+            self.__make_response(Rcode.NOERROR())
+            return UPDATE_SUCCESS, self.__zname, self.__zclass
         except UpdateError as e:
             if not e.nolog:
                 logger.debug(logger.DBGLVL_TRACE_BASIC, LIBDDNS_UPDATE_ERROR,
@@ -145,16 +213,23 @@ 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.
 
         If the zone section is valid and the specified zone is found in
-        the configuration, it returns a tuple of:
-        - A matching data source that contains the specified zone
-        - The zone name as a Name object
-        - The zone class as an RRClass object
-
+        the configuration, sets private member variables for this session:
+        __datasrc_client: A matching data source that contains the specified
+                          zone
+        __zname: The zone name as a Name object
+        __zclass: The zone class as an RRClass object
+        __finder: A ZoneFinder for this zone
+        If this method raises an exception, these members are not set
         '''
         # Validation: the zone section must contain exactly one question,
         # and it must be of type SOA.
@@ -172,7 +247,11 @@ class UpdateSession:
         zclass = zrecord.get_class()
         zone_type, datasrc_client = self.__zone_config.find_zone(zname, zclass)
         if zone_type == isc.ddns.zone_config.ZONE_PRIMARY:
-            return datasrc_client, zname, zclass
+            _, self.__finder = datasrc_client.find_zone(zname)
+            self.__zname = zname
+            self.__zclass = zclass
+            self.__datasrc_client = datasrc_client
+            return
         elif zone_type == isc.ddns.zone_config.ZONE_SECONDARY:
             # We are a secondary server; since we don't yet support update
             # forwarding, we return 'not implemented'.
@@ -217,7 +296,7 @@ class UpdateSession:
         self.__message.clear_section(SECTION_ZONE)
         self.__message.set_rcode(rcode)
 
-    def __prereq_rrset_exists(self, datasrc_client, rrset):
+    def __prereq_rrset_exists(self, rrset):
         '''Check whether an rrset with the given name and type exists. Class,
            TTL, and Rdata (if any) of the given RRset are ignored.
            RFC2136 Section 2.4.1.
@@ -229,22 +308,22 @@ class UpdateSession:
            only return what the result code would be (and not read/copy
            any actual data).
         '''
-        _, finder = datasrc_client.find_zone(rrset.get_name())
-        result, _, _ = finder.find(rrset.get_name(), rrset.get_type(),
-                                   finder.NO_WILDCARD | finder.FIND_GLUE_OK)
-        return result == finder.SUCCESS
+        result, _, _ = self.__finder.find(rrset.get_name(), rrset.get_type(),
+                                          ZoneFinder.NO_WILDCARD |
+                                          ZoneFinder.FIND_GLUE_OK)
+        return result == ZoneFinder.SUCCESS
 
-    def __prereq_rrset_exists_value(self, datasrc_client, rrset):
+    def __prereq_rrset_exists_value(self, rrset):
         '''Check whether an rrset that matches name, type, and rdata(s) of the
            given rrset exists.
            RFC2136 Section 2.4.2
            Returns True if the prerequisite is satisfied, False otherwise.
         '''
-        _, finder = datasrc_client.find_zone(rrset.get_name())
-        result, found_rrset, _ = finder.find(rrset.get_name(), rrset.get_type(),
-                                             finder.NO_WILDCARD |
-                                             finder.FIND_GLUE_OK)
-        if result == finder.SUCCESS and\
+        result, found_rrset, _ = self.__finder.find(rrset.get_name(),
+                                                    rrset.get_type(),
+                                                    ZoneFinder.NO_WILDCARD |
+                                                    ZoneFinder.FIND_GLUE_OK)
+        if result == ZoneFinder.SUCCESS and\
            rrset.get_name() == found_rrset.get_name() and\
            rrset.get_type() == found_rrset.get_type():
             # We need to match all actual RRs, unfortunately there is no
@@ -262,15 +341,15 @@ class UpdateSession:
             return len(found_rdata) == 0
         return False
 
-    def __prereq_rrset_does_not_exist(self, datasrc_client, rrset):
+    def __prereq_rrset_does_not_exist(self, rrset):
         '''Check whether no rrsets with the same name and type as the given
            rrset exist.
            RFC2136 Section 2.4.3.
            Returns True if the prerequisite is satisfied, False otherwise.
         '''
-        return not self.__prereq_rrset_exists(datasrc_client, rrset)
+        return not self.__prereq_rrset_exists(rrset)
 
-    def __prereq_name_in_use(self, datasrc_client, rrset):
+    def __prereq_name_in_use(self, rrset):
         '''Check whether the name of the given RRset is in use (i.e. has
            1 or more RRs).
            RFC2136 Section 2.4.4
@@ -282,37 +361,45 @@ class UpdateSession:
            to only return what the result code would be (and not read/copy
            any actual data).
         '''
-        _, finder = datasrc_client.find_zone(rrset.get_name())
-        result, rrsets, flags = finder.find_all(rrset.get_name(),
-                                                finder.NO_WILDCARD |
-                                                finder.FIND_GLUE_OK)
-        if result == finder.SUCCESS and\
-           (flags & finder.RESULT_WILDCARD == 0):
+        result, rrsets, flags = self.__finder.find_all(rrset.get_name(),
+                                                       ZoneFinder.NO_WILDCARD |
+                                                       ZoneFinder.FIND_GLUE_OK)
+        if result == ZoneFinder.SUCCESS and\
+           (flags & ZoneFinder.RESULT_WILDCARD == 0):
             return True
         return False
 
-    def __prereq_name_not_in_use(self, datasrc_client, rrset):
+    def __prereq_name_not_in_use(self, rrset):
         '''Check whether the name of the given RRset is not in use (i.e. does
            not exist at all, or is an empty nonterminal.
            RFC2136 Section 2.4.5.
            Returns True if the prerequisite is satisfied, False otherwise.
         '''
-        return not self.__prereq_name_in_use(datasrc_client, rrset)
+        return not self.__prereq_name_in_use(rrset)
 
-    def __check_prerequisites(self, datasrc_client, zname, zclass):
+    def __check_in_zone(self, rrset):
+        '''Returns true if the name of the given rrset is equal to
+           or a subdomain of the zname from the Zone Section.'''
+        relation = rrset.get_name().compare(self.__zname).get_relation()
+        return relation == NameComparisonResult.SUBDOMAIN or\
+               relation == NameComparisonResult.EQUAL
+
+    def __check_prerequisites(self):
         '''Check the prerequisites section of the UPDATE Message.
            RFC2136 Section 2.4.
            Returns a dns Rcode signaling either no error (Rcode.NOERROR())
            or that one of the prerequisites failed (any other Rcode).
         '''
+
+        # Temporary array to store exact-match RRsets
+        exact_match_rrsets = []
+
         for rrset in self.__message.get_section(SECTION_PREREQUISITE):
             # First check if the name is in the zone
-            relation = rrset.get_name().compare(zname).get_relation()
-            if relation != NameComparisonResult.SUBDOMAIN and\
-               relation != NameComparisonResult.EQUAL:
+            if not self.__check_in_zone(rrset):
                 logger.info(LIBDDNS_PREREQ_NOTZONE,
                             ClientFormatter(self.__client_addr),
-                            ZoneFormatter(zname, zclass),
+                            ZoneFormatter(self.__zname, self.__zclass),
                             RRsetFormatter(rrset))
                 return Rcode.NOTZONE()
 
@@ -322,24 +409,23 @@ class UpdateSession:
                    rrset.get_rdata_count() != 0:
                     logger.info(LIBDDNS_PREREQ_FORMERR_ANY,
                                 ClientFormatter(self.__client_addr),
-                                ZoneFormatter(zname, zclass),
+                                ZoneFormatter(self.__zname, self.__zclass),
                                 RRsetFormatter(rrset))
                     return Rcode.FORMERR()
                 elif rrset.get_type() == RRType.ANY():
-                    if not self.__prereq_name_in_use(datasrc_client,
-                                                     rrset):
+                    if not self.__prereq_name_in_use(rrset):
                         rcode = Rcode.NXDOMAIN()
                         logger.info(LIBDDNS_PREREQ_NAME_IN_USE_FAILED,
                                     ClientFormatter(self.__client_addr),
-                                    ZoneFormatter(zname, zclass),
+                                    ZoneFormatter(self.__zname, self.__zclass),
                                     RRsetFormatter(rrset), rcode)
                         return rcode
                 else:
-                    if not self.__prereq_rrset_exists(datasrc_client, rrset):
+                    if not self.__prereq_rrset_exists(rrset):
                         rcode = Rcode.NXRRSET()
                         logger.info(LIBDDNS_PREREQ_RRSET_EXISTS_FAILED,
                                     ClientFormatter(self.__client_addr),
-                                    ZoneFormatter(zname, zclass),
+                                    ZoneFormatter(self.__zname, self.__zclass),
                                     RRsetFormatter(rrset), rcode)
                         return rcode
             elif rrset.get_class() == RRClass.NONE():
@@ -347,49 +433,367 @@ class UpdateSession:
                    rrset.get_rdata_count() != 0:
                     logger.info(LIBDDNS_PREREQ_FORMERR_NONE,
                                 ClientFormatter(self.__client_addr),
-                                ZoneFormatter(zname, zclass),
+                                ZoneFormatter(self.__zname, self.__zclass),
                                 RRsetFormatter(rrset))
                     return Rcode.FORMERR()
                 elif rrset.get_type() == RRType.ANY():
-                    if not self.__prereq_name_not_in_use(datasrc_client,
-                                                         rrset):
+                    if not self.__prereq_name_not_in_use(rrset):
                         rcode = Rcode.YXDOMAIN()
                         logger.info(LIBDDNS_PREREQ_NAME_NOT_IN_USE_FAILED,
                                     ClientFormatter(self.__client_addr),
-                                    ZoneFormatter(zname, zclass),
+                                    ZoneFormatter(self.__zname, self.__zclass),
                                     RRsetFormatter(rrset), rcode)
                         return rcode
                 else:
-                    if not self.__prereq_rrset_does_not_exist(datasrc_client,
-                                                              rrset):
+                    if not self.__prereq_rrset_does_not_exist(rrset):
                         rcode = Rcode.YXRRSET()
                         logger.info(LIBDDNS_PREREQ_RRSET_DOES_NOT_EXIST_FAILED,
                                     ClientFormatter(self.__client_addr),
-                                    ZoneFormatter(zname, zclass),
+                                    ZoneFormatter(self.__zname, self.__zclass),
                                     RRsetFormatter(rrset), rcode)
                         return rcode
-            elif rrset.get_class() == zclass:
+            elif rrset.get_class() == self.__zclass:
                 if rrset.get_ttl().get_value() != 0:
                     logger.info(LIBDDNS_PREREQ_FORMERR,
                                 ClientFormatter(self.__client_addr),
-                                ZoneFormatter(zname, zclass),
+                                ZoneFormatter(self.__zname, self.__zclass),
                                 RRsetFormatter(rrset))
                     return Rcode.FORMERR()
                 else:
-                    if not self.__prereq_rrset_exists_value(datasrc_client,
-                                                            rrset):
-                        rcode = Rcode.NXRRSET()
-                        logger.info(LIBDDNS_PREREQ_RRSET_EXISTS_VAL_FAILED,
-                                    ClientFormatter(self.__client_addr),
-                                    ZoneFormatter(zname, zclass),
-                                    RRsetFormatter(rrset), rcode)
-                        return rcode
+                    collect_rrsets(exact_match_rrsets, rrset)
             else:
                 logger.info(LIBDDNS_PREREQ_FORMERR_CLASS,
                             ClientFormatter(self.__client_addr),
-                            ZoneFormatter(zname, zclass),
+                            ZoneFormatter(self.__zname, self.__zclass),
                             RRsetFormatter(rrset))
                 return Rcode.FORMERR()
 
+        for collected_rrset in exact_match_rrsets:
+            if not self.__prereq_rrset_exists_value(collected_rrset):
+                rcode = Rcode.NXRRSET()
+                logger.info(LIBDDNS_PREREQ_RRSET_EXISTS_VAL_FAILED,
+                            ClientFormatter(self.__client_addr),
+                            ZoneFormatter(self.__zname, self.__zclass),
+                            RRsetFormatter(collected_rrset), rcode)
+                return rcode
+
         # All prerequisites are satisfied
         return Rcode.NOERROR()
+
+    def __set_soa_rrset(self, rrset):
+        '''Sets the given rrset to the member __added_soa (which
+           is used by __do_update for updating the SOA record'''
+        self.__added_soa = rrset
+
+    def __do_prescan(self):
+        '''Perform the prescan as defined in RFC2136 section 3.4.1.
+           This method has a side-effect; it sets self._new_soa if
+           it encounters the addition of a SOA record in the update
+           list (so serial can be checked by update later, etc.).
+           It puts the added SOA in self.__added_soa.
+        '''
+        for rrset in self.__message.get_section(SECTION_UPDATE):
+            if not self.__check_in_zone(rrset):
+                logger.info(LIBDDNS_UPDATE_NOTZONE,
+                            ClientFormatter(self.__client_addr),
+                            ZoneFormatter(self.__zname, self.__zclass),
+                            RRsetFormatter(rrset))
+                return Rcode.NOTZONE()
+            if rrset.get_class() == self.__zclass:
+                # In fact, all metatypes are in a specific range,
+                # so one check can test TKEY to ANY
+                # (some value check is needed anyway, since we do
+                # not have defined RRtypes for MAILA and MAILB)
+                if rrset.get_type().get_code() >=  249:
+                    logger.info(LIBDDNS_UPDATE_ADD_BAD_TYPE,
+                                ClientFormatter(self.__client_addr),
+                                ZoneFormatter(self.__zname, self.__zclass),
+                                RRsetFormatter(rrset))
+                    return Rcode.FORMERR()
+                if rrset.get_type() == RRType.SOA():
+                    # In case there's multiple soa records in the update
+                    # somehow, just take the last
+                    for rr in foreach_rr(rrset):
+                        self.__set_soa_rrset(rr)
+            elif rrset.get_class() == RRClass.ANY():
+                if rrset.get_ttl().get_value() != 0:
+                    logger.info(LIBDDNS_UPDATE_DELETE_NONZERO_TTL,
+                                ClientFormatter(self.__client_addr),
+                                ZoneFormatter(self.__zname, self.__zclass),
+                                RRsetFormatter(rrset))
+                    return Rcode.FORMERR()
+                if rrset.get_rdata_count() > 0:
+                    logger.info(LIBDDNS_UPDATE_DELETE_RRSET_NOT_EMPTY,
+                                ClientFormatter(self.__client_addr),
+                                ZoneFormatter(self.__zname, self.__zclass),
+                                RRsetFormatter(rrset))
+                    return Rcode.FORMERR()
+                if rrset.get_type().get_code() >= 249 and\
+                   rrset.get_type().get_code() <= 254:
+                    logger.info(LIBDDNS_UPDATE_DELETE_BAD_TYPE,
+                                ClientFormatter(self.__client_addr),
+                                ZoneFormatter(self.__zname, self.__zclass),
+                                RRsetFormatter(rrset))
+                    return Rcode.FORMERR()
+            elif rrset.get_class() == RRClass.NONE():
+                if rrset.get_ttl().get_value() != 0:
+                    logger.info(LIBDDNS_UPDATE_DELETE_RR_NONZERO_TTL,
+                                ClientFormatter(self.__client_addr),
+                                ZoneFormatter(self.__zname, self.__zclass),
+                                RRsetFormatter(rrset))
+                    return Rcode.FORMERR()
+                if rrset.get_type().get_code() >= 249:
+                    logger.info(LIBDDNS_UPDATE_DELETE_RR_BAD_TYPE,
+                                ClientFormatter(self.__client_addr),
+                                ZoneFormatter(self.__zname, self.__zclass),
+                                RRsetFormatter(rrset))
+                    return Rcode.FORMERR()
+            else:
+                logger.info(LIBDDNS_UPDATE_BAD_CLASS,
+                            ClientFormatter(self.__client_addr),
+                            ZoneFormatter(self.__zname, self.__zclass),
+                            RRsetFormatter(rrset))
+                return Rcode.FORMERR()
+        return Rcode.NOERROR()
+
+    def __do_update_add_single_rr(self, diff, rr, existing_rrset):
+        '''Helper for __do_update_add_rrs_to_rrset: only add the
+           rr if it is not present yet
+           (note that rr here should already be a single-rr rrset)
+        '''
+        if existing_rrset is None:
+            diff.add_data(rr)
+        else:
+            rr_rdata = rr.get_rdata()[0]
+            if not rr_rdata in existing_rrset.get_rdata():
+                diff.add_data(rr)
+
+    def __do_update_add_rrs_to_rrset(self, diff, rrset):
+        '''Add the rrs from the given rrset to the diff.
+           There is handling for a number of special cases mentioned
+           in RFC2136;
+           - If the addition is a CNAME, but existing data at its
+             name is not, the addition is ignored, and vice versa.
+           - If it is a CNAME, and existing data is too, it is
+             replaced (existing data is deleted)
+           An additional restriction is that SOA data is ignored as
+           well (it is handled separately by the __do_update method).
+
+           Note that in the (near) future, this method may have
+           addition special-cases processing.
+        '''
+        # For a number of cases, we may need to remove data in the zone
+        # (note; SOA is handled separately by __do_update, so that one
+        # is explicitely ignored here)
+        if rrset.get_type() == RRType.SOA():
+            return
+        result, orig_rrset, _ = self.__finder.find(rrset.get_name(),
+                                                   rrset.get_type(),
+                                                   ZoneFinder.NO_WILDCARD |
+                                                   ZoneFinder.FIND_GLUE_OK)
+        if result == self.__finder.CNAME:
+            # Ignore non-cname rrs that try to update CNAME records
+            # (if rrset itself is a CNAME, the finder result would be
+            # SUCCESS, see next case)
+            return
+        elif result == ZoneFinder.SUCCESS:
+            # if update is cname, and zone rr is not, ignore
+            if rrset.get_type() == RRType.CNAME():
+                # Remove original CNAME record (the new one
+                # is added below)
+                diff.delete_data(orig_rrset)
+            # We do not have WKS support at this time, but if there
+            # are special Update equality rules such as for WKS, and
+            # we do have support for the type, this is where the check
+            # (and potential delete) would go.
+        elif result == ZoneFinder.NXRRSET:
+            # There is data present, but not for this type.
+            # If this type is CNAME, ignore the update
+            if rrset.get_type() == RRType.CNAME():
+                return
+        for rr in foreach_rr(rrset):
+            self.__do_update_add_single_rr(diff, rr, orig_rrset)
+
+    def __do_update_delete_rrset(self, diff, rrset):
+        '''Deletes the rrset with the name and type of the given
+           rrset from the zone data (by putting all existing data
+           in the given diff as delete statements).
+           Special cases: if the delete statement is for the
+           zone's apex, and the type is either SOA or NS, it
+           is ignored.'''
+        result, to_delete, _ = self.__finder.find(rrset.get_name(),
+                                                  rrset.get_type(),
+                                                  ZoneFinder.NO_WILDCARD |
+                                                  ZoneFinder.FIND_GLUE_OK)
+        if result == ZoneFinder.SUCCESS:
+            if to_delete.get_name() == self.__zname and\
+               (to_delete.get_type() == RRType.SOA() or\
+                to_delete.get_type() == RRType.NS()):
+                # ignore
+                return
+            for rr in foreach_rr(to_delete):
+                diff.delete_data(rr)
+
+    def __ns_deleter_helper(self, diff, rrset):
+        '''Special case helper for deleting NS resource records
+           at the zone apex. In that scenario, the last NS record
+           may never be removed (and any action that would do so
+           should be ignored).
+        '''
+        # NOTE: This method is currently bad: it WILL delete all
+        # NS rrsets in a number of cases.
+        # We need an extension to our diff.py to handle this correctly
+        # (see ticket #2016)
+        # The related test is currently disabled. When this is fixed,
+        # enable that test again.
+        result, orig_rrset, _ = self.__finder.find(rrset.get_name(),
+                                                   rrset.get_type(),
+                                                   ZoneFinder.NO_WILDCARD |
+                                                   ZoneFinder.FIND_GLUE_OK)
+        # Even a real rrset comparison wouldn't help here...
+        # The goal is to make sure that after deletion of the
+        # given rrset, at least 1 NS record is left (at the apex).
+        # So we make a (shallow) copy of the existing rrset,
+        # and for each rdata in the to_delete set, we check if it wouldn't
+        # delete the last one. If it would, that specific one is ignored.
+        # If it would not, the rdata is removed from the temporary list
+        orig_rrset_rdata = copy.copy(orig_rrset.get_rdata())
+        for rdata in rrset.get_rdata():
+            if len(orig_rrset_rdata) == 1 and rdata == orig_rrset_rdata[0]:
+                # ignore
+                continue
+            else:
+                # create an individual RRset for deletion
+                to_delete = isc.dns.RRset(rrset.get_name(),
+                                          rrset.get_class(),
+                                          rrset.get_type(),
+                                          rrset.get_ttl())
+                to_delete.add_rdata(rdata)
+                orig_rrset_rdata.remove(rdata)
+                diff.delete_data(to_delete)
+
+    def __do_update_delete_name(self, diff, rrset):
+        '''Delete all data at the name of the given rrset,
+           by adding all data found by find_all as delete statements
+           to the given diff.
+           Special case: if the name is the zone's apex, SOA and
+           NS records are kept.
+        '''
+        result, rrsets, flags = self.__finder.find_all(rrset.get_name(),
+                                                       ZoneFinder.NO_WILDCARD |
+                                                       ZoneFinder.FIND_GLUE_OK)
+        if result == ZoneFinder.SUCCESS and\
+           (flags & ZoneFinder.RESULT_WILDCARD == 0):
+            for to_delete in rrsets:
+                # if name == self.__zname and type is soa or ns, don't delete!
+                if to_delete.get_name() == self.__zname and\
+                   (to_delete.get_type() == RRType.SOA() or
+                    to_delete.get_type() == RRType.NS()):
+                    continue
+                else:
+                    for rr in foreach_rr(to_delete):
+                        diff.delete_data(rr)
+
+    def __do_update_delete_rrs_from_rrset(self, diff, rrset):
+        '''Deletes all resource records in the given rrset from the
+           zone. Resource records that do not exist are ignored.
+           If the rrset if of type SOA, it is ignored.
+           Uses the __ns_deleter_helper if the rrset's name is the
+           zone's apex, and the type is NS.
+        '''
+        # Delete all rrs in the rrset, except if name=self.__zname and type=soa, or
+        # type = ns and there is only one left (...)
+
+        # The delete does not want class NONE, we would not have gotten here
+        # if it wasn't, but now is a good time to change it to the zclass.
+        to_delete = convert_rrset_class(rrset, self.__zclass)
+
+        if rrset.get_name() == self.__zname:
+            if rrset.get_type() == RRType.SOA():
+                # ignore
+                return
+            elif rrset.get_type() == RRType.NS():
+                # hmm. okay. annoying. There must be at least one left,
+                # delegate to helper method
+                self.__ns_deleter_helper(diff, to_delete)
+                return
+        for rr in foreach_rr(to_delete):
+            diff.delete_data(rr)
+
+    def __update_soa(self, diff):
+        '''Checks the member value __added_soa, and depending on
+           whether it has been set and what its value is, creates
+           a new SOA if necessary.
+           Then removes the original SOA and adds the new one,
+           by adding the needed operations to the given diff.'''
+        # Get the existing SOA
+        # if a new soa was specified, add that one, otherwise, do the
+        # serial magic and add the newly created one
+
+        # get it from DS and to increment and stuff
+        result, old_soa, _ = self.__finder.find(self.__zname, RRType.SOA(),
+                                                ZoneFinder.NO_WILDCARD |
+                                                ZoneFinder.FIND_GLUE_OK)
+
+        if self.__added_soa is not None:
+            new_soa = self.__added_soa
+            # serial check goes here
+        else:
+            new_soa = old_soa
+            # increment goes here
+
+        diff.delete_data(old_soa)
+        diff.add_data(new_soa)
+
+    def __do_update(self):
+        '''Scan, check, and execute the Update section in the
+           DDNS Update message.
+           Returns an Rcode to signal the result (NOERROR upon success,
+           any error result otherwise).
+        '''
+        # prescan
+        prescan_result = self.__do_prescan()
+        if prescan_result != Rcode.NOERROR():
+            return prescan_result
+
+        # update
+        try:
+            # create an ixfr-out-friendly diff structure to work on
+            diff = isc.xfrin.diff.Diff(self.__datasrc_client, self.__zname,
+                                       journaling=True, single_update_mode=True)
+
+            # Do special handling for SOA first
+            self.__update_soa(diff)
+
+            # Algorithm from RFC2136 Section 3.4
+            # Note that this works on full rrsets, not individual RRs.
+            # Some checks might be easier with individual RRs, but only if we
+            # would use the ZoneUpdater directly (so we can query the
+            # 'zone-as-it-would-be-so-far'. However, due to the current use
+            # of the Diff class, this is not the case, and therefore it
+            # is easier to work with full rrsets for the most parts
+            # (less lookups needed; conversion to individual rrs is
+            # the same effort whether it is done here or in the several
+            # do_update statements)
+            for rrset in self.__message.get_section(SECTION_UPDATE):
+                if rrset.get_class() == self.__zclass:
+                    self.__do_update_add_rrs_to_rrset(diff, rrset)
+                elif rrset.get_class() == RRClass.ANY():
+                    if rrset.get_type() == RRType.ANY():
+                        self.__do_update_delete_name(diff, rrset)
+                    else:
+                        self.__do_update_delete_rrset(diff, rrset)
+                elif rrset.get_class() == RRClass.NONE():
+                    self.__do_update_delete_rrs_from_rrset(diff, rrset)
+
+            diff.commit()
+            return Rcode.NOERROR()
+        except isc.datasrc.Error as dse:
+            logger.info(LIBDDNS_UPDATE_DATASRC_ERROR, dse)
+            return Rcode.SERVFAIL()
+        except Exception as uce:
+            logger.error(LIBDDNS_UPDATE_UNCAUGHT_EXCEPTION,
+                         ClientFormatter(self.__client_addr),
+                         ZoneFormatter(self.__zname, self.__zclass),
+                         uce)
+            return Rcode.SERVFAIL()

src/lib/python/isc/log/tests/Makefile.am

@@ -17,6 +17,7 @@ check-local:
 	chmod +x $(abs_builddir)/log_console.py
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/python/isc/log \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(abs_srcdir)/check_output.sh $(abs_builddir)/log_console.py $(abs_srcdir)/console.out
 if ENABLE_PYTHON_COVERAGE
 	touch $(abs_top_srcdir)/.coverage
@@ -28,6 +29,7 @@ endif
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/python/isc/log:$(abs_top_builddir)/src/lib/log/python/.libs \
 	B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done ; \
 	for pytest in $(PYTESTS_GEN) ; do \
@@ -36,5 +38,6 @@ endif
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/python/isc/log:$(abs_top_builddir)/src/lib/log/python/.libs \
 	B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
 	done

src/lib/python/isc/server_common/tests/Makefile.am

@@ -20,5 +20,6 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/dns/python/.libs \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/python/isc/xfrin/tests/Makefile.am

@@ -20,5 +20,6 @@ endif
 	echo Running test: $$pytest ; \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	PYTHONPATH=$(COMMON_PYTHON_PATH):$(abs_top_builddir)/src/lib/dns/python/.libs \
+	B10_LOCKFILE_DIR_FROM_BUILD=$(abs_top_builddir) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/testutils/socket_request.h

@@ -55,7 +55,7 @@ public:
     /// \param expect_port The port which is expected to be requested. If
     ///     the application requests a different port, it is considered
     ///     a failure.
-    /// \param expeted_app The share name for which all the requests should
+    /// \param expected_app The share name for which all the requests should
     ///     be made. This is not the usual app_name - the requestSocket does
     ///     not fall back to this value if its share_name is left empty, if
     ///     you want to check the code relies on the requestor to use the

src/lib/util/Makefile.am

@@ -4,6 +4,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/util -I$(top_builddir)/src/lib/util
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/exceptions -I$(top_builddir)/src/lib/exceptions
 AM_CPPFLAGS += $(BOOST_INCLUDES)
+AM_CPPFLAGS += -DLOCKFILE_DIR=\"${localstatedir}/${PACKAGE_NAME}\"
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
 lib_LTLIBRARIES = libutil.la
@@ -12,6 +13,9 @@ libutil_la_SOURCES += locks.h lru_list.h
 libutil_la_SOURCES += strutil.h strutil.cc
 libutil_la_SOURCES += buffer.h io_utilities.h
 libutil_la_SOURCES += time_utilities.h time_utilities.cc
+libutil_la_SOURCES += interprocess_sync.h
+libutil_la_SOURCES += interprocess_sync_file.h interprocess_sync_file.cc
+libutil_la_SOURCES += interprocess_sync_null.h interprocess_sync_null.cc
 libutil_la_SOURCES += range_utilities.h
 libutil_la_SOURCES += hash/sha1.h hash/sha1.cc
 libutil_la_SOURCES += encode/base16_from_binary.h

src/lib/util/buffer.h

@@ -206,8 +206,8 @@ public:
     /// If specified buffer is too short, it will be expanded
     /// using vector::resize() method.
     ///
-    /// @param Reference to a buffer (data will be stored there).
-    /// @param Size specified number of bytes to read in a vector.
+    /// @param data Reference to a buffer (data will be stored there).
+    /// @param len Size specified number of bytes to read in a vector.
     ///
     void readVector(std::vector<uint8_t>& data, size_t len) {
         if (position_ + len > len_) {

src/lib/util/interprocess_sync.h

@@ -0,0 +1,149 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __INTERPROCESS_SYNC_H__
+#define __INTERPROCESS_SYNC_H__
+
+#include <string>
+
+namespace isc {
+namespace util {
+
+class InterprocessSyncLocker; // forward declaration
+
+/// \brief Interprocess Sync Class
+///
+/// This class specifies an interface for mutual exclusion among
+/// co-operating processes. This is an abstract class and a real
+/// implementation such as InterprocessSyncFile should be used
+/// in code. Usage is as follows:
+///
+/// 1. Client instantiates a sync object of an implementation (such as
+/// InterprocessSyncFile).
+/// 2. Client then creates an automatic (stack) object of
+/// InterprocessSyncLocker around the sync object. Such an object
+/// destroys itself and releases any acquired lock when it goes out of extent.
+/// 3. Client calls lock() method on the InterprocessSyncLocker.
+/// 4. Client performs task that needs mutual exclusion.
+/// 5. Client frees lock with unlock(), or simply returns from the basic
+/// block which forms the scope for the InterprocessSyncLocker.
+///
+/// NOTE: All implementations of InterprocessSync should keep the
+/// is_locked_ member variable updated whenever their
+/// lock()/tryLock()/unlock() implementations are called.
+class InterprocessSync {
+  // InterprocessSyncLocker is the only code outside this class that
+  // should be allowed to call the lock(), tryLock() and unlock()
+  // methods.
+  friend class InterprocessSyncLocker;
+
+public:
+    /// \brief Constructor
+    ///
+    /// Creates an interprocess synchronization object
+    ///
+    /// \param task_name Name of the synchronization task. This has to be
+    /// identical among the various processes that need to be
+    /// synchronized for the same task.
+    InterprocessSync(const std::string& task_name) :
+        task_name_(task_name), is_locked_(false)
+    {}
+
+    /// \brief Destructor
+    virtual ~InterprocessSync() {}
+
+protected:
+    /// \brief Acquire the lock (blocks if something else has acquired a
+    /// lock on the same task name)
+    ///
+    /// \return Returns true if the lock was acquired, false otherwise.
+    virtual bool lock() = 0;
+
+    /// \brief Try to acquire a lock (doesn't block)
+    ///
+    /// \return Returns true if the lock was acquired, false otherwise.
+    virtual bool tryLock() = 0;
+
+    /// \brief Release the lock
+    ///
+    /// \return Returns true if the lock was released, false otherwise.
+    virtual bool unlock() = 0;
+
+    const std::string task_name_; ///< The task name
+    bool is_locked_;              ///< Is the lock taken?
+};
+
+/// \brief Interprocess Sync Locker Class
+///
+/// This class is used for making automatic stack objects to manage
+/// locks that are released automatically when the block is exited
+/// (RAII). It is meant to be used along with InterprocessSync objects. See
+/// the description of InterprocessSync.
+class InterprocessSyncLocker {
+public:
+    /// \brief Constructor
+    ///
+    /// Creates a lock manager around a interprocess synchronization object
+    ///
+    /// \param sync The sync object which has to be locked/unlocked by
+    /// this locker object.
+    InterprocessSyncLocker(InterprocessSync& sync) :
+        sync_(sync)
+    {}
+
+    /// \brief Destructor
+    ~InterprocessSyncLocker() {
+        if (isLocked())
+            unlock();
+    }
+
+    /// \brief Acquire the lock (blocks if something else has acquired a
+    /// lock on the same task name)
+    ///
+    /// \return Returns true if the lock was acquired, false otherwise.
+    bool lock() {
+        return (sync_.lock());
+    }
+
+    /// \brief Try to acquire a lock (doesn't block)
+    ///
+    /// \return Returns true if a new lock could be acquired, false
+    ///         otherwise.
+    bool tryLock() {
+        return (sync_.tryLock());
+    }
+
+    /// \brief Check if the lock is taken
+    ///
+    /// \return Returns true if a lock is currently acquired, false
+    ///         otherwise.
+    bool isLocked() const {
+        return (sync_.is_locked_);
+    }
+
+    /// \brief Release the lock
+    ///
+    /// \return Returns true if the lock was released, false otherwise.
+    bool unlock() {
+        return (sync_.unlock());
+    }
+
+protected:
+    InterprocessSync& sync_; ///< Ref to underlying sync object
+};
+
+} // namespace util
+} // namespace isc
+
+#endif // __INTERPROCESS_SYNC_H__

src/lib/util/interprocess_sync_file.cc

@@ -0,0 +1,130 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "interprocess_sync_file.h"
+
+#include <string>
+
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+namespace isc {
+namespace util {
+
+InterprocessSyncFile::~InterprocessSyncFile() {
+    if (fd_ != -1) {
+        // This will also release any applied locks.
+        close(fd_);
+        // The lockfile will continue to exist, and we must not delete
+        // it.
+    }
+}
+
+bool
+InterprocessSyncFile::do_lock(int cmd, short l_type) {
+    // Open lock file only when necessary (i.e., here). This is so that
+    // if a default InterprocessSync object is replaced with another
+    // implementation, it doesn't attempt any opens.
+    if (fd_ == -1) {
+        std::string lockfile_path = LOCKFILE_DIR;
+
+        const char* const env = getenv("B10_FROM_BUILD");
+        if (env != NULL) {
+            lockfile_path = env;
+        }
+
+        const char* const env2 = getenv("B10_FROM_BUILD_LOCALSTATEDIR");
+        if (env2 != NULL) {
+            lockfile_path = env2;
+        }
+
+        const char* const env3 = getenv("B10_LOCKFILE_DIR_FROM_BUILD");
+        if (env3 != NULL) {
+            lockfile_path = env3;
+        }
+
+        lockfile_path += "/" + task_name_ + "_lockfile";
+
+        // Open the lockfile in the constructor so it doesn't do the access
+        // checks every time a message is logged.
+        const mode_t mode = umask(0111);
+        fd_ = open(lockfile_path.c_str(), O_CREAT | O_RDWR, 0660);
+        umask(mode);
+
+        if (fd_ == -1) {
+            isc_throw(InterprocessSyncFileError,
+                      "Unable to use interprocess sync lockfile: " +
+                      lockfile_path);
+        }
+    }
+
+    struct flock lock;
+
+    memset(&lock, 0, sizeof (lock));
+    lock.l_type = l_type;
+    lock.l_whence = SEEK_SET;
+    lock.l_start = 0;
+    lock.l_len = 1;
+
+    return (fcntl(fd_, cmd, &lock) == 0);
+}
+
+bool
+InterprocessSyncFile::lock() {
+    if (is_locked_) {
+        return (true);
+    }
+
+    if (do_lock(F_SETLKW, F_WRLCK)) {
+        is_locked_ = true;
+        return (true);
+    }
+
+    return (false);
+}
+
+bool
+InterprocessSyncFile::tryLock() {
+    if (is_locked_) {
+        return (true);
+    }
+
+    if (do_lock(F_SETLK, F_WRLCK)) {
+        is_locked_ = true;
+        return (true);
+    }
+
+    return (false);
+}
+
+bool
+InterprocessSyncFile::unlock() {
+    if (!is_locked_) {
+        return (true);
+    }
+
+    if (do_lock(F_SETLKW, F_UNLCK)) {
+        is_locked_ = false;
+        return (true);
+    }
+
+    return (false);
+}
+
+} // namespace util
+} // namespace isc

src/lib/util/interprocess_sync_file.h

@@ -0,0 +1,91 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __INTERPROCESS_SYNC_FILE_H__
+#define __INTERPROCESS_SYNC_FILE_H__
+
+#include <util/interprocess_sync.h>
+#include <exceptions/exceptions.h>
+
+namespace isc {
+namespace util {
+
+/// \brief InterprocessSyncFileError
+///
+/// Exception that is thrown if it's not possible to open the
+/// lock file.
+class InterprocessSyncFileError : public Exception {
+public:
+    InterprocessSyncFileError(const char* file, size_t line,
+                              const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/// \brief File-based Interprocess Sync Class
+///
+/// This class specifies a concrete implementation for a file-based
+/// interprocess synchronization mechanism. Please see the
+/// InterprocessSync class documentation for usage.
+///
+/// An InterprocessSyncFileError exception may be thrown if there is an
+/// issue opening the lock file.
+///
+/// Lock files are created typically in the local state directory
+/// (var). They are typically named like "<task_name>_lockfile".
+/// This implementation opens lock files lazily (only when
+/// necessary). It also leaves the lock files lying around as multiple
+/// processes may have locks on them.
+class InterprocessSyncFile : public InterprocessSync {
+public:
+    /// \brief Constructor
+    ///
+    /// Creates a file-based interprocess synchronization object
+    ///
+    /// \param name Name of the synchronization task. This has to be
+    /// identical among the various processes that need to be
+    /// synchronized for the same task.
+    InterprocessSyncFile(const std::string& task_name) :
+        InterprocessSync(task_name), fd_(-1)
+    {}
+
+    /// \brief Destructor
+    virtual ~InterprocessSyncFile();
+
+protected:
+    /// \brief Acquire the lock (blocks if something else has acquired a
+    /// lock on the same task name)
+    ///
+    /// \return Returns true if the lock was acquired, false otherwise.
+    bool lock();
+
+    /// \brief Try to acquire a lock (doesn't block)
+    ///
+    /// \return Returns true if the lock was acquired, false otherwise.
+    bool tryLock();
+
+    /// \brief Release the lock
+    ///
+    /// \return Returns true if the lock was released, false otherwise.
+    bool unlock();
+
+private:
+    bool do_lock(int cmd, short l_type);
+
+    int fd_; ///< The descriptor for the open file
+};
+
+} // namespace util
+} // namespace isc
+
+#endif // __INTERPROCESS_SYNC_FILE_H__

src/lib/util/interprocess_sync_null.cc

@@ -0,0 +1,42 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "interprocess_sync_null.h"
+
+namespace isc {
+namespace util {
+
+InterprocessSyncNull::~InterprocessSyncNull() {
+}
+
+bool
+InterprocessSyncNull::lock() {
+    is_locked_ = true;
+    return (true);
+}
+
+bool
+InterprocessSyncNull::tryLock() {
+    is_locked_ = true;
+    return (true);
+}
+
+bool
+InterprocessSyncNull::unlock() {
+    is_locked_ = false;
+    return (true);
+}
+
+} // namespace util
+} // namespace isc

src/lib/util/interprocess_sync_null.h

@@ -0,0 +1,64 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __INTERPROCESS_SYNC_NULL_H__
+#define __INTERPROCESS_SYNC_NULL_H__
+
+#include <util/interprocess_sync.h>
+
+namespace isc {
+namespace util {
+
+/// \brief Null Interprocess Sync Class
+///
+/// This class specifies a concrete implementation for a null (no effect)
+/// interprocess synchronization mechanism. Please see the
+/// InterprocessSync class documentation for usage.
+class InterprocessSyncNull : public InterprocessSync {
+public:
+    /// \brief Constructor
+    ///
+    /// Creates a null interprocess synchronization object
+    ///
+    /// \param name Name of the synchronization task. This has to be
+    /// identical among the various processes that need to be
+    /// synchronized for the same task.
+    InterprocessSyncNull(const std::string& task_name) :
+        InterprocessSync(task_name)
+    {}
+
+    /// \brief Destructor
+    virtual ~InterprocessSyncNull();
+
+protected:
+    /// \brief Acquire the lock (never blocks)
+    ///
+    /// \return Always returns true
+    bool lock();
+
+    /// \brief Try to acquire a lock (doesn't block)
+    ///
+    /// \return Always returns true
+    bool tryLock();
+
+    /// \brief Release the lock
+    ///
+    /// \return Always returns true
+    bool unlock();
+};
+
+} // namespace util
+} // namespace isc
+
+#endif // __INTERPROCESS_SYNC_NULL_H__

src/lib/util/tests/Makefile.am

@@ -28,6 +28,8 @@ run_unittests_SOURCES += filename_unittest.cc
 run_unittests_SOURCES += hex_unittest.cc
 run_unittests_SOURCES += io_utilities_unittest.cc
 run_unittests_SOURCES += lru_list_unittest.cc
+run_unittests_SOURCES += interprocess_sync_file_unittest.cc
+run_unittests_SOURCES += interprocess_sync_null_unittest.cc
 run_unittests_SOURCES += qid_gen_unittest.cc
 run_unittests_SOURCES += random_number_generator_unittest.cc
 run_unittests_SOURCES += sha1_unittest.cc

src/lib/util/tests/interprocess_sync_file_unittest.cc

@@ -0,0 +1,174 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "util/interprocess_sync_file.h"
+#include <gtest/gtest.h>
+#include <unistd.h>
+
+using namespace std;
+
+namespace isc {
+namespace util {
+
+namespace {
+unsigned char
+parentReadLockedState (int fd) {
+  unsigned char locked = 0xff;
+
+  fd_set rfds;
+  FD_ZERO(&rfds);
+  FD_SET(fd, &rfds);
+
+  // We use select() here to wait for new data on the input end of
+  // the pipe. We wait for 5 seconds (an arbitrary value) for input
+  // data, and continue if no data is available. This is done so
+  // that read() is not blocked due to some issue in the child
+  // process (and the tests continue running).
+
+  struct timeval tv;
+  tv.tv_sec = 5;
+  tv.tv_usec = 0;
+
+  const int nfds = select(fd + 1, &rfds, NULL, NULL, &tv);
+  EXPECT_EQ(1, nfds);
+
+  if (nfds == 1) {
+      // Read status
+      ssize_t bytes_read = read(fd, &locked, sizeof(locked));
+      EXPECT_EQ(sizeof(locked), bytes_read);
+  }
+
+  return (locked);
+}
+
+TEST(InterprocessSyncFileTest, TestLock) {
+  InterprocessSyncFile sync("test");
+  InterprocessSyncLocker locker(sync);
+
+  EXPECT_FALSE(locker.isLocked());
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.isLocked());
+
+  int fds[2];
+
+  // Here, we check that a lock has been taken by forking and
+  // checking from the child that a lock exists. This has to be
+  // done from a separate process as we test by trying to lock the
+  // range again on the lock file. The lock attempt would pass if
+  // done from the same process for the granted range. The lock
+  // attempt must fail to pass our check.
+
+  EXPECT_EQ(0, pipe(fds));
+
+  if (fork() == 0) {
+      unsigned char locked = 0;
+      // Child writes to pipe
+      close(fds[0]);
+
+      InterprocessSyncFile sync2("test");
+      InterprocessSyncLocker locker2(sync2);
+
+      if (!locker2.tryLock()) {
+          EXPECT_FALSE(locker2.isLocked());
+          locked = 1;
+      } else {
+          EXPECT_TRUE(locker2.isLocked());
+      }
+
+      ssize_t bytes_written = write(fds[1], &locked, sizeof(locked));
+      EXPECT_EQ(sizeof(locked), bytes_written);
+
+      close(fds[1]);
+      exit(0);
+  } else {
+      // Parent reads from pipe
+      close(fds[1]);
+
+      const unsigned char locked = parentReadLockedState(fds[0]);
+
+      close(fds[0]);
+
+      EXPECT_EQ(1, locked);
+  }
+
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_FALSE(locker.isLocked());
+
+  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test_lockfile"));
+}
+
+TEST(InterprocessSyncFileTest, TestMultipleFilesDirect) {
+  InterprocessSyncFile sync("test1");
+  InterprocessSyncLocker locker(sync);
+
+  EXPECT_TRUE(locker.lock());
+
+  InterprocessSyncFile sync2("test2");
+  InterprocessSyncLocker locker2(sync2);
+  EXPECT_TRUE(locker2.lock());
+  EXPECT_TRUE(locker2.unlock());
+
+  EXPECT_TRUE(locker.unlock());
+
+  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test1_lockfile"));
+  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test2_lockfile"));
+}
+
+TEST(InterprocessSyncFileTest, TestMultipleFilesForked) {
+  InterprocessSyncFile sync("test1");
+  InterprocessSyncLocker locker(sync);
+
+  EXPECT_TRUE(locker.lock());
+
+  int fds[2];
+
+  EXPECT_EQ(0, pipe(fds));
+
+  if (fork() == 0) {
+      unsigned char locked = 0xff;
+      // Child writes to pipe
+      close(fds[0]);
+
+      InterprocessSyncFile sync2("test2");
+      InterprocessSyncLocker locker2(sync2);
+
+      if (locker2.tryLock()) {
+          locked = 0;
+      }
+
+      ssize_t bytes_written = write(fds[1], &locked, sizeof(locked));
+      EXPECT_EQ(sizeof(locked), bytes_written);
+
+      close(fds[1]);
+      exit(0);
+  } else {
+      // Parent reads from pipe
+      close(fds[1]);
+
+      const unsigned char locked = parentReadLockedState(fds[0]);
+
+      close(fds[0]);
+
+      EXPECT_EQ(0, locked);
+  }
+
+  EXPECT_TRUE(locker.unlock());
+
+  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test1_lockfile"));
+  EXPECT_EQ (0, unlink(TEST_DATA_TOPBUILDDIR "/test2_lockfile"));
+}
+}
+
+} // namespace util
+} // namespace isc

src/lib/util/tests/interprocess_sync_null_unittest.cc

@@ -0,0 +1,76 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "util/interprocess_sync_null.h"
+#include <gtest/gtest.h>
+
+using namespace std;
+
+namespace isc {
+namespace util {
+
+TEST(InterprocessSyncNullTest, TestNull) {
+  InterprocessSyncNull sync("test1");
+  InterprocessSyncLocker locker(sync);
+
+  // Check if the is_locked_ flag is set correctly during lock().
+  EXPECT_FALSE(locker.isLocked());
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.isLocked());
+
+  // lock() must always return true (this is called 4 times, just an
+  // arbitrary number)
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.lock());
+
+  // Check if the is_locked_ flag is set correctly during unlock().
+  EXPECT_TRUE(locker.isLocked());
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_FALSE(locker.isLocked());
+
+  // unlock() must always return true (this is called 4 times, just an
+  // arbitrary number)
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_TRUE(locker.unlock());
+
+  // Check if the is_locked_ flag is set correctly during tryLock().
+  EXPECT_FALSE(locker.isLocked());
+  EXPECT_TRUE(locker.tryLock());
+  EXPECT_TRUE(locker.isLocked());
+
+  // tryLock() must always return true (this is called 4 times, just an
+  // arbitrary number)
+  EXPECT_TRUE(locker.tryLock());
+  EXPECT_TRUE(locker.tryLock());
+  EXPECT_TRUE(locker.tryLock());
+  EXPECT_TRUE(locker.tryLock());
+
+  // Random order (should all return true)
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.tryLock());
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_TRUE(locker.lock());
+  EXPECT_TRUE(locker.tryLock());
+  EXPECT_TRUE(locker.unlock());
+  EXPECT_TRUE(locker.unlock());
+}
+
+} // namespace util
+} // namespace isc

src/lib/util/tests/run_unittests.cc

@@ -14,10 +14,12 @@
 
 #include <gtest/gtest.h>
 #include <util/unittests/run_all.h>
+#include <stdlib.h>
 
 int
 main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
 
+    setenv("B10_LOCKFILE_DIR_FROM_BUILD", TEST_DATA_TOPBUILDDIR, 1);
     return (isc::util::unittests::run_all());
 }