Merge branch 'master' into trac2023

ChangeLog

@@ -1,3 +1,20 @@
+448.	[func]		team
+	b10-ddns is now functional and handles dynamic update requests
+	per RFC 2136.  See BIND 10 guide for configuration and operation
+	details.
+	(Multiple Trac tickets)
+
+447.	[bug]		jinmei
+	Fixed a bug in b10-xfrout where a helper thread could fall into
+	an infinite loop if b10-auth stops while the thread is waiting for
+	forwarded requests from b10-auth.
+	(Trac #988 and #1833, git 95a03bbefb559615f3f6e529d408b749964d390a)
+
+446.	[bug]		muks
+	A number of warnings reported by Python about unclosed file and
+	socket objects were fixed. Some related code was also made safer.
+	(Trac #1828, git 464682a2180c672f1ed12d8a56fd0a5ab3eb96ed)
+
 445.	[bug]*		jinmei
 	The pre-install check for older SQLite3 DB now refers to the DB
 	file with the prefix of DESTDIR.  This ensures that 'make install'

Makefile.am

@@ -16,6 +16,26 @@ DISTCHECK_CONFIGURE_FLAGS = --disable-install-configurations
 # Use same --with-gtest flag if set
 DISTCHECK_CONFIGURE_FLAGS += $(DISTCHECK_GTEST_CONFIGURE_FLAG)
 
+dist_doc_DATA = AUTHORS COPYING ChangeLog README
+
+.PHONY: check-valgrind check-valgrind-suppress
+
+check-valgrind:
+if HAVE_VALGRIND
+	@VALGRIND_COMMAND="$(VALGRIND) -q --gen-suppressions=all --track-origins=yes --num-callers=48 --leak-check=full --fullpath-after=" \
+	make -C $(abs_top_builddir) check
+else
+	@echo "*** Valgrind is required for check-valgrind ***"; exit 1;
+endif
+
+check-valgrind-suppress:
+if HAVE_VALGRIND
+	@VALGRIND_COMMAND="$(VALGRIND) -q --gen-suppressions=all --error-exitcode=1 --suppressions=$(abs_top_srcdir)/src/valgrind-suppressions --suppressions=$(abs_top_srcdir)/src/valgrind-suppressions.revisit --num-callers=48 --leak-check=full --fullpath-after=" \
+	make -C $(abs_top_builddir) check
+else
+	@echo "*** Valgrind is required for check-valgrind-suppress ***"; exit 1;
+endif
+
 clean-cpp-coverage:
 	@if [ $(USE_LCOV) = yes ] ; then \
 		$(LCOV) --directory . --zerocounters; \

configure.ac

@@ -982,6 +982,15 @@ AC_ARG_ENABLE(logger-checks, [AC_HELP_STRING([--enable-logger-checks],
 AM_CONDITIONAL(ENABLE_LOGGER_CHECKS, test x$enable_logger_checks != xno)
 AM_COND_IF([ENABLE_LOGGER_CHECKS], [AC_DEFINE([ENABLE_LOGGER_CHECKS], [1], [Check logger messages?])])
 
+# Check for valgrind
+AC_PATH_PROG(VALGRIND, valgrind, no)
+AM_CONDITIONAL(HAVE_VALGRIND, test "x$VALGRIND" != "xno")
+
+found_valgrind="not found"
+if test "x$VALGRIND" != "xno"; then
+   found_valgrind="found"
+fi
+
 AC_CONFIG_FILES([Makefile
                  doc/Makefile
                  doc/guide/Makefile
@@ -1292,6 +1301,7 @@ Features:
 
 Developer:
   Google Tests:  $gtest_path
+  Valgrind: $found_valgrind
   C++ Code Coverage: $USE_LCOV
   Python Code Coverage: $USE_PYCOVERAGE
   Logger checks: $enable_logger_checks

doc/guide/Makefile.am

@@ -1,6 +1,7 @@
-EXTRA_DIST = bind10-guide.css
-EXTRA_DIST += bind10-guide.xml bind10-guide.html bind10-guide.txt
-EXTRA_DIST += bind10-messages.xml bind10-messages.html
+dist_doc_DATA = bind10-guide.txt
+dist_html_DATA = bind10-guide.css bind10-guide.html bind10-messages.html
+
+EXTRA_DIST = bind10-guide.xml bind10-messages.xml
 
 # This is not a "man" manual, but reuse this for now for docbook.
 if ENABLE_MAN

doc/guide/bind10-guide.xml

@@ -87,9 +87,10 @@
     <section>
       <title>Supported Platforms</title>
       <para>
-  BIND 10 builds have been tested on Debian GNU/Linux 5 and unstable,
-  Ubuntu 9.10, NetBSD 5, Solaris 10, FreeBSD 7 and 8, CentOS
-  Linux 5.3, and MacOS 10.6.
+  BIND 10 builds have been tested on (in no particular order)
+  Debian GNU/Linux 5 and unstable, Ubuntu 9.10, NetBSD 5,
+  Solaris 10 and 11, FreeBSD 7 and 8, CentOS Linux 5.3,
+  MacOS 10.6 and 10.7, and OpenBSD 5.1.
 
   It has been tested on Sparc, i386, and amd64 hardware
   platforms.
@@ -127,11 +128,13 @@
       </para>
 
       <para>
-        The <command>b10-xfrin</command>, <command>b10-xfrout</command>,
-        and <command>b10-zonemgr</command> components require the
-        libpython3 library and the Python _sqlite3.so module
-        (which is included with Python).
-        The Python module needs to be built for the corresponding Python 3.
+        The <command>b10-ddns</command>, <command>b10-xfrin</command>,
+        <command>b10-xfrout</command>, and <command>b10-zonemgr</command>
+        components require the libpython3 library and the Python
+        _sqlite3.so module (which is included with Python).
+        The <command>b10-stats-httpd</command> component uses the
+        Python pyexpat.so module.
+        The Python modules need to be built for the corresponding Python 3.
       </para>
 <!-- TODO: this will change ... -->
 
@@ -196,6 +199,16 @@
 
           <listitem>
             <simpara>
+              <command>b10-ddns</command> &mdash;
+              Dynamic DNS update service.
+              This process is used to handle incoming DNS update
+              requests to allow granted clients to update zones
+	      for which BIND 10 is serving as a primary server.
+            </simpara>
+          </listitem>
+
+          <listitem>
+            <simpara>
               <command>b10-msgq</command> &mdash;
               Message bus daemon.
               This process coordinates communication between all of the other
@@ -1831,7 +1844,6 @@ what if a NOTIFY is sent?
 
   <chapter id="xfrout">
     <title>Outbound Zone Transfers</title>
-
     <para>
       The <command>b10-xfrout</command> process is started by
       <command>bind10</command>.
@@ -1907,6 +1919,325 @@ what is XfroutClient xfr_client??
 
   </chapter>
 
+  <chapter id="ddns">
+    <title>Dynamic DNS Update</title>
+
+    <para>
+      BIND 10 supports the server side of the Dynamic DNS Update
+      (DDNS) protocol as defined in RFC 2136.
+      This service is provided by the <command>b10-ddns</command>
+      component, which is started by the <command>bind10</command>
+      process if configured so.
+    </para>
+
+    <para>
+      When the <command>b10-auth</command> authoritative DNS server
+      receives an UPDATE request, it internally forwards the request
+      to <command>b10-ddns</command>, which handles the rest of
+      request processing.
+      When the processing is completed <command>b10-ddns</command>
+      will send a response to the client with the RCODE set to the
+      value as specified in RFC 2136 (NOERROR for successful update,
+      REFUSED if rejected due to ACL check, etc).
+      If the zone has been changed as a result, it will internally
+      notify <command>b10-xfrout</command> so that other secondary
+      servers will be notified via the DNS notify protocol.
+      In addition, if <command>b10-auth</command> serves the updated
+      zone from its in-memory cache (as described in
+      <xref linkend="in-memory-datasource-with-sqlite3-backend" />),
+      <command>b10-ddns</command> will also
+      notify <command>b10-auth</command> so that <command>b10-auth</command>
+      will re-cache the updated zone content.
+    </para>
+
+    <para>
+      The <command>b10-ddns</command> component supports requests over
+      both UDP and TCP, and both IPv6 and IPv4; for TCP requests,
+      however, it terminates the TCP connection immediately after
+      each single request has been processed.  Clients cannot reuse the
+      same TCP connection for multiple requests. (This is a current
+      implementation limitation of <command>b10-ddns</command>.
+      While RFC 2136 doesn't specify anything about such reuse of TCP
+      connection, there is no reason for disallowing it as RFC 1035
+      generally allows multiple requests sent over a single TCP
+      connection.  BIND 9 supports such reuse.)
+    </para>
+
+    <para>
+      As of this writing <command>b10-ddns</command> does not support
+      update forwarding for secondary zones.
+      If it receives an update request for a secondary zone, it will
+      immediately return a response with an RCODE of NOTIMP.
+      <note><simpara>
+	  For feature completeness update forwarding should be
+	  eventually supported.  But right now it's considered a lower
+	  priority task and there is no specific plan of implementing
+	  this feature.
+	  <!-- See Trac #2063 -->
+      </simpara></note>
+    </para>
+
+    <section>
+      <title>Enabling Dynamic Update</title>
+      <para>
+        First off, it must be made sure that a few components on which
+        <command>b10-ddns</command> depends are configured to run,
+        which are <command>b10-auth</command>
+        and <command>b10-zonemgr</command>.
+        In addition, <command>b10-xfrout</command> should also be
+        configured to run; otherwise the notification after an update
+        (see above) will fail with a timeout, suspending the DDNS
+        service while <command>b10-ddns</command> waits for the
+        response (see the description of the <ulink
+        url="bind10-messages.html#DDNS_UPDATE_NOTIFY_FAIL">DDNS_UPDATE_NOTIFY_FAIL</ulink>
+        log message for further details).
+        If BIND 10 is already configured to provide authoritative DNS
+        service they should normally be configured to run already.
+      </para>
+
+      <para>
+        Second, for the obvious reason dynamic update requires that the
+        underlying data source storing the zone data be writable.
+        In the current implementation this means the zone must be stored
+        in an SQLite3-based data source.
+        Also, right now, the <command>b10-ddns</command> component
+        configures itself with the data source referring to the
+        <quote>database_file</quote> configuration parameter of
+        <command>b10-auth</command>.
+        So this information must be configured correctly before starting
+        <command>b10-ddns</command>.
+
+        <note><simpara>
+            The way to configure data sources is now being revised.
+            Configuration on the data source for DDNS will be very
+            likely to be changed in a backward incompatible manner in
+            a near future version.
+        </simpara></note>
+      </para>
+
+      <para>
+	In general, if something goes wrong regarding the dependency
+	described above, <command>b10-ddns</command> will log the
+	related event at the warning or error level.
+	It's advisable to check the log message when you first enable
+	DDNS or if it doesn't work as you expect to see if there's any
+	warning or error log message.
+      </para>
+
+      <para>
+        Next, to enable the DDNS service, <command>b10-ddns</command>
+        needs to be explicitly configured to run.
+        It can be done by using the <command>bindctl</command>
+        utility.  For example:
+      <screen>
+&gt; <userinput>config add Boss/components b10-ddns</userinput>
+&gt; <userinput>config set Boss/components/b10-ddns/address DDNS</userinput>
+&gt; <userinput>config set Boss/components/b10-ddns/kind dispensable</userinput>
+&gt; <userinput>config commit</userinput>
+</screen>
+      <note><simpara>
+	  In theory "kind" could be omitted because "dispensable" is its
+	  default.  But there's some peculiar behavior (which should
+	  be a bug and should be fixed eventually; see Trac ticket
+	  #2064) with bindctl and you'll still need to specify that explicitly.
+	  Likewise, "address" may look unnecessary because
+	  <command>b10-ddns</command> would start and work without
+	  specifying it.  But for it to shutdown gracefully this
+	  parameter should also be specified.
+      </simpara></note>
+      </para>
+    </section>
+
+    <section>
+      <title>Access Control</title>
+      <para>
+        By default <command>b10-ddns</command> rejects any update
+        requests from any clients by returning a response with an RCODE
+        of REFUSED.
+        To allow updates to take effect, an access control rule
+        (called update ACL) with a policy allowing updates must explicitly be
+        configured.
+        Update ACL must be configured per zone basis in the
+        <quote>zones</quote> configuration parameter of
+        <command>b10-ddns</command>.
+        This is a list of per-zone configuration regarding DDNS.
+        Each list element consists of the following parameters:
+        <variablelist>
+          <varlistentry>
+            <term>origin</term>
+            <listitem>
+              <simpara>The zone's origin name</simpara>
+            </listitem>
+          </varlistentry>
+          <varlistentry>
+            <term>class</term>
+            <listitem>
+              <simpara>The RR class of the zone
+                (normally <quote>IN</quote>, and in that case
+		can be omitted in configuration)</simpara>
+            </listitem>
+          </varlistentry>
+          <varlistentry>
+            <term>update_acl</term>
+            <listitem>
+              <simpara>List of access control rules (ACL) for the zone</simpara>
+            </listitem>
+          </varlistentry>
+        </variablelist>
+        The syntax of the ACL is the same as ACLs for other
+        components.
+        Specific examples are given below.
+      </para>
+
+      <para>
+        In general, an update ACL rule that allows an update request
+        should be configured with a TSIG key.
+        This is an example update ACL that allows updates to the zone
+        named <quote>example.org</quote> of RR class <quote>IN</quote>
+        from clients that send requests signed with a TSIG whose
+        key name is "key.example.org" (and refuses all others):
+      <screen>
+&gt; <userinput>config add DDNS/zones</userinput>
+&gt; <userinput>config set DDNS/zones[0]/origin example.org</userinput>
+&gt; <userinput>config set DDNS/zones[0]/class IN</userinput>
+(Note: "class" can be omitted)
+&gt; <userinput>config add DDNS/zones[0]/update_acl {"action": "ACCEPT", "key": "key.example.org"}</userinput>
+&gt; <userinput>config commit</userinput>
+</screen>
+      The TSIG key must be configured system wide
+      (see <xref linkend="xfrout"/>.)
+      </para>
+
+      <para>
+        Multiple rules can be specified in the ACL, and an ACL rule
+        can consist of multiple constraints, such as a combination of
+        IP address and TSIG.
+        The following configuration sequence will add a new rule to
+        the ACL created in the above example.  This additional rule
+	allows update requests sent from a client
+        using TSIG key name of "key.example" (different from the
+        key used in the previous example) and has an IPv6 address of ::1.
+      <screen>
+&gt; <userinput>config add DDNS/zones[0]/update_acl {"action": "ACCEPT", "from": "::1", "key": "key.example"}</userinput>
+&gt; <userinput>config show DDNS/zones[0]/update_acl</userinput>
+DDNS/zones[0]/update_acl[0]     {"action": "ACCEPT", "key": "key.example.org"} any (modified)
+DDNS/zones[0]/update_acl[1]     {"action": "ACCEPT", "from": "::1", "key": "key.example"} any (modified)
+&gt; <userinput>config commit</userinput>
+</screen>
+      (Note the "add" in the first line.  Before this sequence, we
+      have had only entry in zones[0]/update_acl.  The "add" command
+      with a value (rule) adds a new entry and sets it to the given rule.
+      Due to a limitation of the current implementation, it doesn't
+      work if you first try to just add a new entry and then set it to
+      a given rule).
+      </para>
+
+      <note><simpara>
+          The <command>b10-ddns</command> component accepts an ACL
+          rule that just allows updates from a specific IP address
+          (i.e., without requiring TSIG), but this is highly
+          discouraged (remember that requests can be made over UDP and
+          spoofing the source address of a UDP packet is often pretty
+          easy).
+          Unless you know what you are doing and that you can accept
+          its consequence, any update ACL rule that allows updates
+          should have a TSIG key in its constraints.
+      </simpara></note>
+
+      <para>
+	The ACL rules will be checked in the listed order, and the
+	first matching one will apply.
+	If none of the rules matches, the default rule will apply,
+	which is rejecting any requests in the case of
+	<command>b10-ddns</command>.
+      </para>
+
+      <para>
+	Other actions than "ACCEPT", namely "REJECT" and "DROP", can be
+	used, too.
+	See <xref linkend="resolverserver"/> about their effects.
+      </para>
+
+      <para>
+        Currently update ACL can only control updates per zone basis;
+        it's not possible to specify access control with higher
+        granularity such as for particular domain names or specific
+        types of RRs.
+	<!-- See Trac ticket #2065 -->
+      </para>
+
+      <note><simpara>
+        Contrary to what RFC 2136 (literally) specifies,
+        <command>b10-ddns</command> checks the update ACL before
+        checking the prerequisites of the update request.
+        This is a deliberate implementation decision.
+        This counter intuitive specification has been repeatedly
+        discussed among implementers and in the IETF, and it is now
+        widely agreed that it does not make sense to strictly follow
+        that part of RFC.
+        One known specific bad result of following the RFC is that it
+        could leak information about which name or record exists or does not
+        exist in the zone as a result of prerequisite checks even if a
+        zone is somehow configured to reject normal queries from
+        arbitrary clients.
+        There have been other troubles that could have been avoided if
+        the ACL could be checked before the prerequisite check.
+      </simpara></note>
+    </section>
+
+    <section>
+      <title>Miscellaneous Operational Issues</title>
+      <para>
+        Unlike BIND 9, BIND 10 currently does not support automatic
+        resigning of DNSSEC-signed zone when it's updated via DDNS.
+        It could be possible to resign the updated zone afterwards
+        or make sure the update request also updates related DNSSEC
+        records, but that will be pretty error-prone operation.
+        In general, it's not advisable to allow DDNS for a signed zone
+        at this moment.
+      </para>
+
+      <para>
+        Also unlike BIND 9, it's currently not possible
+        to <quote>freeze</quote> a zone temporarily in order to
+        suspend DDNS while you manually update the zone.
+        If you need to make manual updates to a dynamic zone,
+        you'll need to temporarily reject any updates to the zone via
+        the update ACLs.
+      </para>
+
+      <para>
+        Dynamic updates are only applicable to primary zones.
+        In order to avoid updating secondary zones via DDNS requests,
+        <command>b10-ddns</command> refers to the
+        <quote>secondary_zones</quote> configuration of
+        <command>b10-zonemgr</command>.  Zones listed in
+        <quote>secondary_zones</quote> will never be updated via DDNS
+        regardless of the update ACL configuration;
+	<command>b10-ddns</command> will return a response with an
+	RCODE of NOTAUTH as specified in RFC 2136.
+        If you have a "conceptual" secondary zone whose content is a
+        copy of some external source but is not updated via the
+        standard zone transfers and therefore not listed in
+        <quote>secondary_zones</quote>, be careful not to allow DDNS
+        for the zone; it would be quite likely to lead to inconsistent
+        state between different servers.
+        Normally this should not be a problem because the default
+        update ACL rejects any update requests, but you may want to
+        take an extra care about the configuration if you have such
+        type of secondary zones.
+      </para>
+      <para>
+        The difference of two versions of a zone, before and after a
+        DDNS transaction, is automatically recorded in the underlying
+        data source, and can be retrieved in the form of outbound
+        IXFR.
+        This is done automatically; it does not require specific
+        configuration to make this possible.
+      </para>
+    </section>
+  </chapter>
+
   <chapter id="resolverserver">
     <title>Recursive Name Server</title>
 

src/Makefile.am

@@ -1 +1,6 @@
 SUBDIRS = lib bin
+
+EXTRA_DIST = \
+	cppcheck-suppress.lst		\
+	valgrind-suppressions		\
+	valgrind-suppressions.revisit

src/bin/auth/tests/Makefile.am

@@ -19,6 +19,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 # Do not define global tests, use check-local so
 # environment can be set (needed for dynamic loading)
 TESTS =

src/bin/bind10/bind10_messages.mes

@@ -160,6 +160,11 @@ The boss module is sending a SIGKILL signal to the given process.
 % BIND10_SEND_SIGTERM sending SIGTERM to %1 (PID %2)
 The boss module is sending a SIGTERM signal to the given process.
 
+% BIND10_SETGID setting GID to %1
+The boss switches the process group ID to the given value.  This happens
+when BIND 10 starts with the -u option, and the group ID will be set to
+that of the specified user.
+
 % BIND10_SETUID setting UID to %1
 The boss switches the user it runs as to the given UID.
 

src/bin/bind10/bind10_src.py.in

@@ -169,8 +169,8 @@ class BoB:
     
     def __init__(self, msgq_socket_file=None, data_path=None,
                  config_filename=None, clear_config=False, nocache=False,
-                 verbose=False, nokill=False, setuid=None, username=None,
-                 cmdctl_port=None, wait_time=10):
+                 verbose=False, nokill=False, setuid=None, setgid=None,
+                 username=None, cmdctl_port=None, wait_time=10):
         """
             Initialize the Boss of BIND. This is a singleton (only one can run).
         
@@ -208,6 +208,7 @@ class BoB:
         self.components_to_restart = []
         self.runnable = False
         self.uid = setuid
+        self.gid = setgid
         self.username = username
         self.verbose = verbose
         self.nokill = nokill
@@ -1156,12 +1157,14 @@ def main():
 
     # Check user ID.
     setuid = None
+    setgid = None
     username = None
     if options.user:
         # Try getting information about the user, assuming UID passed.
         try:
             pw_ent = pwd.getpwuid(int(options.user))
             setuid = pw_ent.pw_uid
+            setgid = pw_ent.pw_gid
             username = pw_ent.pw_name
         except ValueError:
             pass
@@ -1175,6 +1178,7 @@ def main():
         try:
             pw_ent = pwd.getpwnam(options.user)
             setuid = pw_ent.pw_uid
+            setgid = pw_ent.pw_gid
             username = pw_ent.pw_name
         except KeyError:
             pass
@@ -1205,7 +1209,7 @@ def main():
         boss_of_bind = BoB(options.msgq_socket_file, options.data_path,
                            options.config_file, options.clear_config,
                            options.nocache, options.verbose, options.nokill,
-                           setuid, username, options.cmdctl_port,
+                           setuid, setgid, username, options.cmdctl_port,
                            options.wait_time)
         startup_result = boss_of_bind.startup()
         if startup_result:

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

@@ -1055,22 +1055,29 @@ class TestPIDFile(unittest.TestCase):
         # dump PID to the file, and confirm the content is correct
         dump_pid(self.pid_file)
         my_pid = os.getpid()
-        self.assertEqual(my_pid, int(open(self.pid_file, "r").read()))
+        with open(self.pid_file, "r") as f:
+            self.assertEqual(my_pid, int(f.read()))
 
     def test_dump_pid(self):
         self.check_pid_file()
 
         # make sure any existing content will be removed
-        open(self.pid_file, "w").write('dummy data\n')
+        with open(self.pid_file, "w") as f:
+            f.write('dummy data\n')
         self.check_pid_file()
 
     def test_unlink_pid_file_notexist(self):
         dummy_data = 'dummy_data\n'
-        open(self.pid_file, "w").write(dummy_data)
+
+        with open(self.pid_file, "w") as f:
+            f.write(dummy_data)
+
         unlink_pid_file("no_such_pid_file")
+
         # the file specified for unlink_pid_file doesn't exist,
         # and the original content of the file should be intact.
-        self.assertEqual(dummy_data, open(self.pid_file, "r").read())
+        with open(self.pid_file, "r") as f:
+            self.assertEqual(dummy_data, f.read())
 
     def test_dump_pid_with_none(self):
         # Check the behavior of dump_pid() and unlink_pid_file() with None.
@@ -1079,9 +1086,14 @@ class TestPIDFile(unittest.TestCase):
         self.assertFalse(os.path.exists(self.pid_file))
 
         dummy_data = 'dummy_data\n'
-        open(self.pid_file, "w").write(dummy_data)
+
+        with open(self.pid_file, "w") as f:
+            f.write(dummy_data)
+
         unlink_pid_file(None)
-        self.assertEqual(dummy_data, open(self.pid_file, "r").read())
+
+        with open(self.pid_file, "r") as f:
+            self.assertEqual(dummy_data, f.read())
 
     def test_dump_pid_failure(self):
         # the attempt to open file will fail, which should result in exception.

src/bin/bindctl/tests/bindctl_test.py

@@ -425,6 +425,12 @@ class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
 
 class TestBindCmdInterpreter(unittest.TestCase):
 
+    def setUp(self):
+        self.old_stdout = sys.stdout
+
+    def tearDown(self):
+        sys.stdout = self.old_stdout
+
     def _create_invalid_csv_file(self, csvfilename):
         import csv
         csvfile = open(csvfilename, 'w')
@@ -447,19 +453,17 @@ class TestBindCmdInterpreter(unittest.TestCase):
         self.assertEqual(new_csv_dir, custom_cmd.csv_file_dir)
 
     def test_get_saved_user_info(self):
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
-        cmd = bindcmd.BindCmdInterpreter()
-        users = cmd._get_saved_user_info('/notexist', 'csv_file.csv')
-        self.assertEqual([], users)
-
-        csvfilename = 'csv_file.csv'
-        self._create_invalid_csv_file(csvfilename)
-        users = cmd._get_saved_user_info('./', csvfilename)
-        self.assertEqual([], users)
-        os.remove(csvfilename)
-        sys.stdout = old_stdout
-
+        with open(os.devnull, 'w') as f:
+            sys.stdout = f
+            cmd = bindcmd.BindCmdInterpreter()
+            users = cmd._get_saved_user_info('/notexist', 'csv_file.csv')
+            self.assertEqual([], users)
+
+            csvfilename = 'csv_file.csv'
+            self._create_invalid_csv_file(csvfilename)
+            users = cmd._get_saved_user_info('./', csvfilename)
+            self.assertEqual([], users)
+            os.remove(csvfilename)
 
 class TestCommandLineOptions(unittest.TestCase):
     def setUp(self):

src/bin/cmdctl/tests/cmdctl_test.py

@@ -84,6 +84,7 @@ class TestSecureHTTPRequestHandler(unittest.TestCase):
         self.handler.rfile = open("check.tmp", 'w+b')
 
     def tearDown(self):
+        sys.stdout.close()
         sys.stdout = self.old_stdout
         self.handler.rfile.close()
         os.remove('check.tmp')
@@ -306,6 +307,7 @@ class TestCommandControl(unittest.TestCase):
         self.cmdctl = MyCommandControl(None, True)
    
     def tearDown(self):
+        sys.stdout.close()
         sys.stdout = self.old_stdout
 
     def _check_config(self, cmdctl):
@@ -427,6 +429,9 @@ class TestSecureHTTPServer(unittest.TestCase):
                                          MyCommandControl, verbose=True)
 
     def tearDown(self):
+        # both sys.stdout and sys.stderr are the same, so closing one is
+        # sufficient
+        sys.stdout.close()
         sys.stdout = self.old_stdout
         sys.stderr = self.old_stderr
 

src/bin/ddns/b10-ddns.8

@@ -1,7 +1,7 @@
 '\" t
 .\"     Title: b10-ddns
 .\"    Author: [FIXME: author] [see http://docbook.sf.net/el/author]
-.\" Generator: DocBook XSL Stylesheets v1.75.2 <http://docbook.sf.net/>
+.\" Generator: DocBook XSL Stylesheets v1.76.1 <http://docbook.sf.net/>
 .\"      Date: February 28, 2012
 .\"    Manual: BIND10
 .\"    Source: BIND10
@@ -9,6 +9,15 @@
 .\"
 .TH "B10\-DDNS" "8" "February 28, 2012" "BIND10" "BIND10"
 .\" -----------------------------------------------------------------
+.\" * Define some portability stuff
+.\" -----------------------------------------------------------------
+.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.\" http://bugs.debian.org/507673
+.\" http://lists.gnu.org/archive/html/groff/2009-02/msg00013.html
+.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.ie \n(.g .ds Aq \(aq
+.el       .ds Aq '
+.\" -----------------------------------------------------------------
 .\" * set default formatting
 .\" -----------------------------------------------------------------
 .\" disable hyphenation
@@ -33,29 +42,29 @@ boss process\&. When the
 \fBb10\-auth\fR
 DNS server receives a DDNS update,
 \fBb10\-ddns\fR
-updates the zone in the BIND 10 zone data store\&.
-.if n \{\
-.sp
-.\}
-.RS 4
-.it 1 an-trap
-.nr an-no-space-flag 1
-.nr an-break-flag 1
-.br
-.ps +1
-\fBNote\fR
-.ps -1
-.br
+updates the zone in the BIND 10 zone data source\&.
 .PP
-Currently installed is a dummy component\&. It does not provide any functionality\&. It is a skeleton implementation that will be expanded later\&.
-.sp .5v
-.RE
+When the
+\fBb10\-auth\fR
+authoritative DNS server receives an UPDATE request, it internally forwards the request to
+\fBb10\-ddns\fR, which handles the rest of request processing\&. When the process is completed
+\fBb10\-ddns\fR
+will send a response to the client with the processing result\&. If the zone has been changed as a result, it will internally notify
+\fBb10\-auth\fR
+and
+\fBb10\-xfrout\fR
+so the new version of zone will be served, and other secondary servers will be notified via the DNS notify protocol\&.
 .PP
 This daemon communicates with BIND 10 over a
 \fBb10-msgq\fR(8)
 C\-Channel connection\&. If this connection is not established,
 \fBb10\-ddns\fR
-will exit\&.
+will exit\&. The
+\fBb10\-ddns\fR
+daemon depends on some other BIND 10 components:
+\fBb10-auth\fR(8)
+and
+\fBb10-zonemgr\fR(8)\&.
 .PP
 
 \fBb10\-ddns\fR
@@ -75,7 +84,13 @@ The configurable settings are:
 .PP
 
 \fIzones\fR
-The zones option is a named set of zones that can be updated with DDNS\&. Each entry has one element called update_acl, which is a list of access control rules that define update permissions\&. By default this is empty; DDNS must be explicitely enabled per zone\&.
+The zones option is a list of configuration items for specific zones that can be updated with DDNS\&. Each entry is a map that can contain the following items:
+\fIorigin\fR
+is a textual domain name of the zone;
+\fIclass\fR
+(text) is the RR class of the zone;
+\fIupdate_acl\fR
+is a list of ACL that controls permission for updates\&. See the BIND 10 Guide for configuration details\&. Note that not listing a zone in this list does not directly mean update requests for the zone are rejected, but the end result is the same because the default ACL for updates is to deny all requests\&.
 .PP
 The module commands are:
 .PP
@@ -91,6 +106,8 @@ argument to select the process ID to stop\&. (Note that the BIND 10 boss process
 \fBb10-auth\fR(8),
 \fBb10-cfgmgr\fR(8),
 \fBb10-msgq\fR(8),
+\fBb10-xfrout\fR(8),
+\fBb10-zonemgr\fR(8),
 \fBbind10\fR(8),
 BIND 10 Guide\&.
 .SH "HISTORY"

src/bin/ddns/b10-ddns.xml

@@ -20,7 +20,7 @@
 <refentry>
 
   <refentryinfo>
-    <date>February 28, 2012</date>
+    <date>June 18, 2012</date>
   </refentryinfo>
 
   <refmeta>
@@ -58,23 +58,33 @@
       Normally it is started by the
       <citerefentry><refentrytitle>bind10</refentrytitle><manvolnum>8</manvolnum></citerefentry>
       boss process.
-      When the <command>b10-auth</command> DNS server receives
-      a DDNS update, <command>b10-ddns</command> updates the zone
-      in the BIND 10 zone data store.
     </para>
 
-    <note><para>
-      Currently installed is a dummy component. It does not provide
-      any functionality. It is a skeleton implementation that
-      will be expanded later.
-<!-- TODO: #1458 -->
-    </para></note>
+    <para>
+      When the <command>b10-auth</command> authoritative DNS server
+      receives an UPDATE request, it internally forwards the request
+      to <command>b10-ddns</command>, which handles the rest of
+      request processing.
+      When the processing is completed <command>b10-ddns</command>
+      will send a response to the client with the RCODE set to the
+      value as specified in RFC 2136.
+      If the zone has been changed as a result, it will internally
+      notify <command>b10-auth</command> and
+      <command>b10-xfrout</command> so the new version of the zone will
+      be served, and other secondary servers will be notified via the
+      DNS notify protocol.
+    </para>
 
     <para>
       This daemon communicates with BIND 10 over a
       <citerefentry><refentrytitle>b10-msgq</refentrytitle><manvolnum>8</manvolnum></citerefentry>
       C-Channel connection.  If this connection is not established,
       <command>b10-ddns</command> will exit.
+      The <command>b10-ddns</command> daemon also depends on some other
+      BIND 10 components (either directly or indirectly):
+      <citerefentry><refentrytitle>b10-auth</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
+      <citerefentry><refentrytitle>b10-xfrout</refentrytitle><manvolnum>8</manvolnum></citerefentry>, and
+      <citerefentry><refentrytitle>b10-zonemgr</refentrytitle><manvolnum>8</manvolnum></citerefentry>.
     </para>
 
     <para>
@@ -92,13 +102,24 @@
 
       <varlistentry>
         <term>
+          <option>-h</option>,
+          <option>--help</option>
+        </term>
+        <listitem>
+          <para>
+            Print the command line arguments and exit.
+          </para>
+        </listitem>
+      </varlistentry>
+      <varlistentry>
+        <term>
           <option>-v</option>,
           <option>--verbose</option>
         </term>
         <listitem>
           <para>
             This value is ignored at this moment, but is provided for
-            compatibility with the bind10 Boss process
+            compatibility with the bind10 Boss process.
           </para>
         </listitem>
       </varlistentry>
@@ -112,10 +133,18 @@
     </para>
     <para>
       <varname>zones</varname>
-      The zones option is a named set of zones that can be updated with
-      DDNS. Each entry has one element called update_acl, which is
-      a list of access control rules that define update permissions.
-      By default this is empty; DDNS must be explicitely enabled per zone.
+      The zones option is a list of configuration items for specific
+      zones that can be updated with DDNS. Each entry is a map that
+      can contain the following items:
+      <varname>origin</varname> is a textual domain name of the zone;
+      <varname>class</varname> (text) is the RR class of the zone;
+      <varname>update_acl</varname> is an ACL that controls
+      permission for updates.
+      See the BIND 10 Guide for configuration details.
+      Note that not listing a zone in this list does not directly
+      mean update requests for the zone are rejected, but the end
+      result is the same because the default ACL for updates is to
+      deny all requests.
     </para>
 
     <para>
@@ -145,6 +174,12 @@
         <refentrytitle>b10-msgq</refentrytitle><manvolnum>8</manvolnum>
       </citerefentry>,
       <citerefentry>
+        <refentrytitle>b10-xfrout</refentrytitle><manvolnum>8</manvolnum>
+      </citerefentry>,
+      <citerefentry>
+        <refentrytitle>b10-zonemgr</refentrytitle><manvolnum>8</manvolnum>
+      </citerefentry>,
+      <citerefentry>
         <refentrytitle>bind10</refentrytitle><manvolnum>8</manvolnum>
       </citerefentry>,
       <citetitle>BIND 10 Guide</citetitle>.
@@ -156,6 +191,7 @@
     <para>
       The <command>b10-ddns</command> daemon was first implemented
       in December 2011 for the ISC BIND 10 project.
+      The first functional version was released in June 2012.
     </para>
   </refsect1>
 </refentry><!--

src/bin/ddns/ddns.py.in

@@ -378,10 +378,13 @@ class DDNSServer:
         Perform any cleanup that is necessary when shutting down the server.
         Do NOT call this to initialize shutdown, use trigger_shutdown().
 
-        Currently, it only causes the ModuleCCSession to send a message that
-        this module is stopping.
         '''
+        # tell the ModuleCCSession to send a message that this module is
+        # stopping.
         self._cc.send_stopping()
+        # make sure any open socket is explicitly closed, per Python
+        # convention.
+        self._listen_socket.close()
 
     def accept(self):
         """
@@ -612,7 +615,7 @@ class DDNSServer:
         Get and process all commands sent from cfgmgr or other modules.
         This loops waiting for events until self.shutdown() has been called.
         '''
-        logger.info(DDNS_RUNNING)
+        logger.info(DDNS_STARTED)
         cc_fileno = self._cc.get_socket().fileno()
         listen_fileno = self._listen_socket.fileno()
         while not self._shutdown:

src/bin/ddns/ddns_messages.mes

@@ -153,10 +153,6 @@ case, there may not be able to do anything to fix it at the server
 side, but the administrator may want to check the general reachability
 with the client address.
 
-% 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.
-
 % DDNS_SECONDARY_ZONES_UPDATE updated secondary zone list (%1 zones are listed)
 b10-ddns has successfully updated the internal copy of secondary zones
 obtained from b10-zonemgr, based on a latest update to zonemgr's
@@ -192,6 +188,10 @@ The ddns process is shutting down. It will no longer listen for new commands
 or updates. Any command or update that is being addressed at this moment will
 be completed, after which the process will exit.
 
+% DDNS_STARTED ddns server is running and listening for updates
+The ddns process has successfully started and is now ready to receive commands
+and updates.
+
 % DDNS_STOPPED ddns server has stopped
 The ddns process has successfully stopped and is no longer listening for or
 handling commands or updates, and will now exit.
@@ -214,16 +214,31 @@ notify messages to secondary servers.
 
 % DDNS_UPDATE_NOTIFY_FAIL failed to notify %1 of updates to %2: %3
 b10-ddns has made updates to a zone based on an update request and
-tried to notify an external module of the updates, but the
-notification fails.  Severity of this effect depends on the type of
-the module.  If it's b10-xfrout, this means DNS notify messages won't
-be sent to secondary servers of the zone.  It's suboptimal, but not
-necessarily critical as the secondary servers will try to check the
-zone's status periodically.  If it's b10-auth and the notification was
-needed to have it reload the corresponding zone, it's more serious
-because b10-auth won't be able to serve the new version of the zone
-unless some explicit recovery action is taken.  So the administrator
-needs to examine this message and takes an appropriate action.  In
-either case, this notification is generally expected to succeed; so
-the fact it fails itself means there's something wrong in the BIND 10
-system, and it would be advisable to check other log messages.
+tried to notify an external component of the updates, but the
+notification fails.  One possible cause of this is that the external
+component is not really running and it times out in waiting for the
+response, although it will be less likely to happen in practice
+because these components will normally be configured to run when the
+server provides the authoritative DNS service; ddns is rather optional
+among them.  If this happens, however, it will suspend b10-ddns for a
+few seconds during which it cannot handle new requests (some may be
+delayed, some may be dropped, depending on the volume of the incoming
+requests).  This is obviously bad, and if this error happens due to
+this reason, the administrator should make sure the component in
+question should be configured to run.  For a longer term, b10-ddns
+should be more robust about this case such as by making this
+notification asynchronously and/or detecting the existence of the
+external components to avoid hopeless notification in the first place.
+Severity of this error for the receiving components depends on the
+type of the component.  If it's b10-xfrout, this means DNS notify
+messages won't be sent to secondary servers of the zone.  It's
+suboptimal, but not necessarily critical as the secondary servers will
+try to check the zone's status periodically.  If it's b10-auth and the
+notification was needed to have it reload the corresponding zone, it's
+more serious because b10-auth won't be able to serve the new version
+of the zone unless some explicit recovery action is taken.  So the
+administrator needs to examine this message and takes an appropriate
+action.  In either case, this notification is generally expected to
+succeed; so the fact it fails itself means there's something wrong in
+the BIND 10 system, and it would be advisable to check other log
+messages.

src/bin/ddns/tests/ddns_test.py

@@ -342,6 +342,10 @@ class TestDDNSServer(unittest.TestCase):
         self.__select_answer = None
         self.__select_exception = None
         self.__hook_called = False
+        # Because we overwrite the _listen_socket, close any existing
+        # socket object.
+        if self.ddns_server._listen_socket is not None:
+            self.ddns_server._listen_socket.close()
         self.ddns_server._listen_socket = FakeSocket(2)
         ddns.select.select = self.__select
 
@@ -380,6 +384,9 @@ class TestDDNSServer(unittest.TestCase):
         # Now make sure the clear_socket really works
         ddns.clear_socket()
         self.assertFalse(os.path.exists(ddns.SOCKET_FILE))
+        # Let ddns object complete any necessary cleanup (not part of the test,
+        # but for suppressing any warnings from the Python interpreter)
+        ddnss.shutdown_cleanup()
 
     def test_initial_config(self):
         # right now, the only configuration is the zone configuration, whose

src/bin/dhcp4/tests/Makefile.am

@@ -38,6 +38,9 @@ if USE_STATIC_LINK
 AM_LDFLAGS = -static
 endif
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 

src/bin/dhcp6/tests/Makefile.am

@@ -34,6 +34,9 @@ if USE_STATIC_LINK
 AM_LDFLAGS = -static
 endif
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 

src/bin/msgq/msgq.py.in

@@ -177,6 +177,7 @@ class MsgQ:
             # (note this is a catch-all, but we reraise it)
             if os.path.exists(self.socket_file):
                 os.remove(self.socket_file)
+            self.listen_socket.close()
             raise e
 
         if self.poller:

src/bin/msgq/tests/msgq_test.py

@@ -156,6 +156,12 @@ class SendNonblock(unittest.TestCase):
         except socket.error:
             pass
 
+        # Explicitly close temporary socket pair as the Python
+        # interpreter expects it.  It may not be 100% exception safe,
+        # but since this is only for tests we prefer brevity.
+        read.close()
+        write.close()
+
     def test_infinite_sendmsg(self):
         """
         Tries sending messages (and not reading them) until it either times
@@ -218,6 +224,12 @@ class SendNonblock(unittest.TestCase):
                     os.kill(queue_pid, signal.SIGTERM)
         self.terminate_check(run)
 
+        # Explicitly close temporary socket pair as the Python
+        # interpreter expects it.  It may not be 100% exception safe,
+        # but since this is only for tests we prefer brevity.
+        queue.close()
+        out.close()
+
     def test_small_sends(self):
         """
         Tests sending small data many times.

src/bin/resolver/tests/Makefile.am

@@ -14,6 +14,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/bin/sockcreator/tests/Makefile.am

@@ -8,6 +8,9 @@ if USE_STATIC_LINK
 AM_LDFLAGS = -static
 endif
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/bin/tests/process_rename_test.py.in

@@ -25,7 +25,8 @@ class TestRename(unittest.TestCase):
     def __scan(self, directory, script, fun):
         # Scan one script if it contains call to the renaming function
         filename = os.path.join(directory, script)
-        data = ''.join(open(filename).readlines())
+        with open(filename) as f:
+            data = ''.join(f.readlines())
         prettyname = 'src' + filename[filename.rfind('../') + 2:]
         self.assertTrue(fun.search(data),
             "Didn't find a call to isc.util.process.rename in " + prettyname)
@@ -53,8 +54,8 @@ class TestRename(unittest.TestCase):
         # Find all Makefile and extract names of scripts
         for (d, _, fs) in os.walk('@top_builddir@'):
             if 'Makefile' in fs:
-                makefile = ''.join(open(os.path.join(d,
-                    "Makefile")).readlines())
+                with open(os.path.join(d, "Makefile")) as f:
+                    makefile = ''.join(f.readlines())
                 for (var, _) in lines.findall(re.sub(excluded_lines, '',
                                                      makefile)):
                     for (script, _) in scripts.findall(var):

src/bin/xfrin/tests/xfrin_test.py

@@ -2127,7 +2127,8 @@ class TestXfrin(unittest.TestCase):
         self.assertFalse(self.xfr._module_cc.stopped);
         self.xfr.shutdown()
         self.assertTrue(self.xfr._module_cc.stopped);
-        sys.stderr= self.stderr_backup
+        sys.stderr.close()
+        sys.stderr = self.stderr_backup
 
     def _do_parse_zone_name_class(self):
         return self.xfr._parse_zone_name_and_class(self.args)

src/bin/xfrout/tests/xfrout_test.py.in

@@ -60,6 +60,9 @@ class MySocket():
         self.sendqueue.extend(data);
         return len(data)
 
+    def fileno(self):
+        return 42               # simply return a constant dummy value
+
     def readsent(self):
         if len(self.sendqueue) >= 2:
             size = 2 + struct.unpack("!H", self.sendqueue[:2])[0]
@@ -1155,6 +1158,15 @@ class TestUnixSockServer(unittest.TestCase):
     def setUp(self):
         self.write_sock, self.read_sock = socket.socketpair()
         self.unix = MyUnixSockServer()
+        # Some test below modify these module-wide attributes.  We'll need
+        # to restore them at the end of each test, so we remember them here.
+        self.__select_bak = xfrout.select.select
+        self.__recv_fd_back = xfrout.recv_fd
+
+    def tearDown(self):
+        # Restore possibly faked module-wide attributes.
+        xfrout.select.select = self.__select_bak
+        xfrout.recv_fd = self.__recv_fd_back
 
     def test_tsig_keyring(self):
         """
@@ -1201,6 +1213,7 @@ class TestUnixSockServer(unittest.TestCase):
         self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
                           ('127.0.0.1', 12345)),
                          self.unix._guess_remote(sock.fileno()))
+        sock.close()
         if socket.has_ipv6:
             # Don't check IPv6 address on hosts not supporting them
             sock = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM)
@@ -1208,6 +1221,7 @@ class TestUnixSockServer(unittest.TestCase):
             self.assertEqual((socket.AF_INET6, socket.SOCK_STREAM,
                               ('::1', 12345, 0, 0)),
                              self.unix._guess_remote(sock.fileno()))
+            sock.close()
             # Try when pretending there's no IPv6 support
             # (No need to pretend when there's really no IPv6)
             xfrout.socket.has_ipv6 = False
@@ -1216,6 +1230,7 @@ class TestUnixSockServer(unittest.TestCase):
             self.assertEqual((socket.AF_INET, socket.SOCK_STREAM,
                               ('127.0.0.1', 12345)),
                              self.unix._guess_remote(sock.fileno()))
+            sock.close()
             # Return it back
             xfrout.socket.has_ipv6 = True
 
@@ -1375,19 +1390,13 @@ class TestUnixSockServer(unittest.TestCase):
         self._remove_file(sock_file)
         self.assertFalse(self.unix._sock_file_in_use(sock_file))
         self._start_unix_sock_server(sock_file)
-
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
         self.assertTrue(self.unix._sock_file_in_use(sock_file))
-        sys.stdout = old_stdout
 
     def test_remove_unused_sock_file_in_use(self):
         sock_file = 'temp.sock.file'
         self._remove_file(sock_file)
         self.assertFalse(self.unix._sock_file_in_use(sock_file))
         self._start_unix_sock_server(sock_file)
-        old_stdout = sys.stdout
-        sys.stdout = open(os.devnull, 'w')
         try:
             self.unix._remove_unused_sock_file(sock_file)
         except SystemExit:
@@ -1396,8 +1405,6 @@ class TestUnixSockServer(unittest.TestCase):
             # This should never happen
             self.assertTrue(False)
 
-        sys.stdout = old_stdout
-
     def test_remove_unused_sock_file_dir(self):
         import tempfile
         dir_name = tempfile.mkdtemp()
@@ -1411,9 +1418,46 @@ class TestUnixSockServer(unittest.TestCase):
             # This should never happen
             self.assertTrue(False)
 
+        sys.stdout.close()
         sys.stdout = old_stdout
         os.rmdir(dir_name)
 
+    def __fake_select(self, r, w, e):
+        '''select emulator used in select_loop_fail test.'''
+        # This simplified faked function assumes to be called at most once,
+        # and in that case just return a pre-configured "readable" sockets.
+        if self.__select_count > 0:
+            raise RuntimeError('select called unexpected number of times')
+        self.__select_count += 1
+        return (self.__select_return_redable, [], [])
+
+    def test_select_loop_fail(self):
+        '''Check failure events in the main loop.'''
+        # setup faked select() environments
+        self.unix._read_sock = MySocket(socket.AF_INET6, socket.SOCK_STREAM)
+        xfrout.select.select = self.__fake_select
+        self.__select_return_redable = [MySocket(socket.AF_INET6,
+                                                 socket.SOCK_STREAM)]
+
+        # Check that loop terminates if recv_fd() fails.
+        for ret_code in [-1, FD_SYSTEM_ERROR]:
+            # fake recv_fd so it returns the faked failure code.
+            xfrout.recv_fd = lambda fileno: ret_code
+
+            # reset the counter, go to the loop.
+            self.__select_count = 0
+            self.unix._select_loop(self.__select_return_redable[0])
+            # select should have been called exactly once.
+            self.assertEqual(1, self.__select_count)
+
+        # Next, we test the case where recf_fd succeeds but receiving the
+        # request fails.
+        self.__select_count = 0
+        xfrout.recv_fd = lambda fileno: 1
+        self.unix._receive_query_message = lambda fd: None
+        self.unix._select_loop(self.__select_return_redable[0])
+        self.assertEqual(1, self.__select_count)
+
 class TestInitialization(unittest.TestCase):
     def setEnv(self, name, value):
         if value is None:

src/bin/xfrout/xfrout.py.in

@@ -678,30 +678,40 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
         except socket.error:
             logger.error(XFROUT_FETCH_REQUEST_ERROR)
             return
+        self._select_loop(request)
+
+    def _select_loop(self, request_sock):
+        '''Main loop for a single session between xfrout and auth.
+
+        This is a dedicated subroutine of handle_request(), but is defined
+        as a separate "protected" method for the convenience of tests.
+        '''
 
         # Check self._shutdown_event to ensure the real shutdown comes.
         # Linux could trigger a spurious readable event on the _read_sock
         # due to a bug, so we need perform a double check.
         while not self._shutdown_event.is_set(): # Check if xfrout is shutdown
             try:
-                (rlist, wlist, xlist) = select.select([self._read_sock, request], [], [])
+                (rlist, wlist, xlist) = select.select([self._read_sock,
+                                                       request_sock], [], [])
             except select.error as e:
                 if e.args[0] == errno.EINTR:
                     (rlist, wlist, xlist) = ([], [], [])
                     continue
                 else:
-                    logger.error(XFROUT_SOCKET_SELECT_ERROR, str(e))
+                    logger.error(XFROUT_SOCKET_SELECT_ERROR, e)
                     break
 
-            # self.server._shutdown_event will be set by now, if it is not a false
-            # alarm
+            # self.server._shutdown_event will be set by now, if it is not a
+            # false alarm
             if self._read_sock in rlist:
                 continue
 
             try:
-                self.process_request(request)
+                if not self.process_request(request_sock):
+                    break
             except Exception as pre:
-                logger.error(XFROUT_PROCESS_REQUEST_ERROR, str(pre))
+                logger.error(XFROUT_PROCESS_REQUEST_ERROR, pre)
                 break
 
     def _handle_request_noblock(self):
@@ -713,26 +723,33 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
 
     def process_request(self, request):
         """Receive socket fd and query message from auth, then
-        start a new thread to process the request."""
+        start a new thread to process the request.
+
+        Return: True if everything is okay; otherwise False, in which case
+        the calling thread will terminate.
+
+        """
         sock_fd = recv_fd(request.fileno())
         if sock_fd < 0:
-            # This may happen when one xfrout process try to connect to
-            # xfrout unix socket server, to check whether there is another
-            # xfrout running.
-            if sock_fd == FD_SYSTEM_ERROR:
-                logger.error(XFROUT_RECEIVE_FILE_DESCRIPTOR_ERROR)
-            return
+            logger.warn(XFROUT_RECEIVE_FILE_DESCRIPTOR_ERROR)
+            return False
 
-        # receive request msg
+        # receive request msg.  If it fails we simply terminate the thread;
+        # it might be possible to recover from this state, but it's more likely
+        # that auth and xfrout are in inconsistent states.  So it will make
+        # more sense to restart in a new session.
         request_data = self._receive_query_message(request)
-        if not request_data:
-            return
+        if request_data is None:
+            # The specific exception type doesn't matter so we use session
+            # error.
+            raise XfroutSessionError('Failed to get complete xfr request')
 
         t = threading.Thread(target=self.finish_request,
-                             args = (sock_fd, request_data))
+                             args=(sock_fd, request_data))
         if self.daemon_threads:
             t.daemon = True
         t.start()
+        return True
 
     def _guess_remote(self, sock_fd):
         """Guess remote address and port of the socket.
@@ -747,12 +764,15 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
         # to care about the SOCK_STREAM parameter at all (which it really is,
         # except for testing)
         if socket.has_ipv6:
-            sock = socket.fromfd(sock_fd, socket.AF_INET6, socket.SOCK_STREAM)
+            sock_domain = socket.AF_INET6
         else:
             # To make it work even on hosts without IPv6 support
             # (Any idea how to simulate this in test?)
-            sock = socket.fromfd(sock_fd, socket.AF_INET, socket.SOCK_STREAM)
+            sock_domain = socket.AF_INET
+
+        sock = socket.fromfd(sock_fd, sock_domain, socket.SOCK_STREAM)
         peer = sock.getpeername()
+        sock.close()
 
         # Identify the correct socket family.  Due to the above "trick",
         # we cannot simply use sock.family.
@@ -761,6 +781,7 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
             socket.inet_pton(socket.AF_INET6, peer[0])
         except socket.error:
             family = socket.AF_INET
+
         return (family, socket.SOCK_STREAM, peer)
 
     def finish_request(self, sock_fd, request_data):
@@ -805,8 +826,10 @@ class UnixSockServer(socketserver_mixin.NoPollMixIn,
             sock = socket.socket(socket.AF_UNIX)
             sock.connect(sock_file)
         except socket.error as err:
+            sock.close()
             return False
         else:
+            sock.close()
             return True
 
     def shutdown(self):

src/bin/xfrout/xfrout_messages.mes

@@ -115,11 +115,15 @@ In general, this should only occur for unexpected problems like
 memory allocation failures, as the query should already have been
 parsed by the b10-auth daemon, before it was passed here.
 
-% XFROUT_PROCESS_REQUEST_ERROR error processing transfer request: %2
-There was an error processing a transfer request. The error is included
-in the log message, but at this point no specific information other
-than that could be given. This points to incomplete exception handling
-in the code.
+% XFROUT_PROCESS_REQUEST_ERROR error processing transfer request: %1
+There was an error in receiving a transfer request from b10-auth.
+This is generally an unexpected event, but is possible when, for
+example, b10-auth terminates in the middle of forwarding the request.
+When this happens it's unlikely to be recoverable with the same
+communication session with b10-auth, so b10-xfrout drops it and
+waits for a new session.  In any case, this error indicates that
+there's something very wrong in the system, so it's advisable to check
+the over all status of the BIND 10 system.
 
 % XFROUT_QUERY_DROPPED %1 client %2: request to transfer %3 dropped
 The xfrout process silently dropped a request to transfer zone to
@@ -149,9 +153,16 @@ and will now shut down.
 
 % XFROUT_RECEIVE_FILE_DESCRIPTOR_ERROR error receiving the file descriptor for an XFR connection
 There was an error receiving the file descriptor for the transfer
-request. Normally, the request is received by b10-auth, and passed on
-to the xfrout daemon, so it can answer directly. However, there was a
-problem receiving this file descriptor. The request will be ignored.
+request from b10-auth.  There can be several reasons for this, but
+the most likely cause is that b10-auth terminates for some reason
+(maybe it's a bug of b10-auth, maybe it's an intentional restart by
+the administrator), so depending on how this happens it may or may not
+be a serious error.  But in any case this is not expected to happen
+frequently, and it's advisable to figure out how this happened if
+this message is logged.  Even if this error happens xfrout will reset
+its internal state and will keep receiving further requests.  So
+if it's just a temporary restart of b10-auth the administrator does
+not have to do anything.
 
 % XFROUT_REMOVE_OLD_UNIX_SOCKET_FILE_ERROR error removing unix socket file %1: %2
 The unix socket file xfrout needs for contact with the auth daemon

src/bin/zonemgr/tests/zonemgr_test.py

@@ -112,6 +112,7 @@ class TestZonemgrRefresh(unittest.TestCase):
     def tearDown(self):
         if os.path.exists(TEST_SQLITE3_DBFILE):
             os.unlink(TEST_SQLITE3_DBFILE)
+        sys.stderr.close()
         sys.stderr = self.stderr_backup
 
     def test_random_jitter(self):

src/bin/zonemgr/zonemgr.py.in

@@ -427,6 +427,8 @@ class ZonemgrRefresh:
         self._thread.join()
         # Wipe out what we do not need
         self._thread = None
+        self._read_sock.close()
+        self._write_sock.close()
         self._read_sock = None
         self._write_sock = None
 

src/lib/acl/tests/Makefile.am

@@ -8,6 +8,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/asiodns/tests/Makefile.am

@@ -12,6 +12,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/asiolink/tests/Makefile.am

@@ -18,6 +18,9 @@ AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/bench/tests/Makefile.am

@@ -5,6 +5,9 @@ AM_CXXFLAGS = $(B10_CXXFLAGS)
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/cache/tests/Makefile.am

@@ -28,6 +28,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/cc/tests/Makefile.am

@@ -16,6 +16,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/config/tests/Makefile.am

@@ -14,6 +14,9 @@ CLEANFILES = *.gcno *.gcda
 noinst_LTLIBRARIES = libfake_session.la
 libfake_session_la_SOURCES = fake_session.h fake_session.cc
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/cryptolink/tests/Makefile.am

@@ -10,6 +10,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/datasrc/Makefile.am

@@ -35,6 +35,7 @@ libdatasrc_la_SOURCES += logger.h logger.cc
 libdatasrc_la_SOURCES += client.h iterator.h
 libdatasrc_la_SOURCES += database.h database.cc
 libdatasrc_la_SOURCES += factory.h factory.cc
+libdatasrc_la_SOURCES += client_list.h client_list.cc
 nodist_libdatasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 libdatasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
 

src/lib/datasrc/client_list.cc

@@ -0,0 +1,162 @@
+// 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 "client_list.h"
+#include "client.h"
+#include "factory.h"
+
+#include <memory>
+#include <boost/foreach.hpp>
+
+using namespace isc::data;
+using namespace std;
+
+namespace isc {
+namespace datasrc {
+
+void
+ConfigurableClientList::configure(const Element& config, bool) {
+    // TODO: Implement the cache
+    // TODO: Implement recycling from the old configuration.
+    size_t i(0); // Outside of the try to be able to access it in the catch
+    try {
+        vector<DataSourceInfo> new_data_sources;
+        for (; i < config.size(); ++i) {
+            // Extract the parameters
+            const ConstElementPtr dconf(config.get(i));
+            const ConstElementPtr typeElem(dconf->get("type"));
+            if (typeElem == ConstElementPtr()) {
+                isc_throw(ConfigurationError, "Missing the type option in "
+                          "data source no " << i);
+            }
+            const string type(typeElem->stringValue());
+            ConstElementPtr paramConf(dconf->get("params"));
+            if (paramConf == ConstElementPtr()) {
+                paramConf.reset(new NullElement());
+            }
+            // TODO: Special-case the master files type.
+            // Ask the factory to create the data source for us
+            const DataSourcePair ds(this->getDataSourceClient(type,
+                                                              paramConf));
+            // And put it into the vector
+            new_data_sources.push_back(DataSourceInfo(ds.first, ds.second));
+        }
+        // If everything is OK up until now, we have the new configuration
+        // ready. So just put it there and let the old one die when we exit
+        // the scope.
+        data_sources_.swap(new_data_sources);
+    } catch (const TypeError& te) {
+        isc_throw(ConfigurationError, "Malformed configuration at data source "
+                  "no. " << i << ": " << te.what());
+    }
+}
+
+ClientList::FindResult
+ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
+                            bool) const
+{
+    // Nothing found yet.
+    //
+    // We have this class as a temporary storage, as the FindResult can't be
+    // assigned.
+    struct MutableResult {
+        MutableResult() :
+            datasrc_client(NULL),
+            matched_labels(0),
+            matched(false)
+        {}
+        DataSourceClient* datasrc_client;
+        ZoneFinderPtr finder;
+        uint8_t matched_labels;
+        bool matched;
+        operator FindResult() const {
+            // Conversion to the right result. If we return this, there was
+            // a partial match at best.
+            return (FindResult(datasrc_client, finder, false));
+        }
+    } candidate;
+
+    BOOST_FOREACH(const DataSourceInfo& info, data_sources_) {
+        // TODO: Once we have support for the caches, consider them too here
+        // somehow. This would probably get replaced by a function, that
+        // checks if there's a cache available, if it is, checks the loaded
+        // zones and zones expected to be in the real data source. If it is
+        // the cached one, provide the cached one. If it is in the external
+        // data source, use the datasource and don't provide the finder yet.
+        const DataSourceClient::FindResult result(
+            info.data_src_client_->findZone(name));
+        switch (result.code) {
+            case result::SUCCESS:
+                // If we found an exact match, we have no hope to getting
+                // a better one. Stop right here.
+
+                // TODO: In case we have only the datasource and not the finder
+                // and the need_updater parameter is true, get the zone there.
+                return (FindResult(info.data_src_client_, result.zone_finder,
+                                   true));
+            case result::PARTIALMATCH:
+                if (!want_exact_match) {
+                    // In case we have a partial match, check if it is better
+                    // than what we have. If so, replace it.
+                    //
+                    // We don't need the labels at the first partial match,
+                    // we have nothing to compare with. So we don't get it
+                    // (as a performance) and hope we will not need it at all.
+                    const uint8_t labels(candidate.matched ?
+                        result.zone_finder->getOrigin().getLabelCount() : 0);
+                    if (candidate.matched && candidate.matched_labels == 0) {
+                        // But if the hope turns out to be false, we need to
+                        // compute it for the first match anyway.
+                        candidate.matched_labels = candidate.finder->
+                            getOrigin().getLabelCount();
+                    }
+                    if (labels > candidate.matched_labels ||
+                        !candidate.matched) {
+                        // This one is strictly better. Replace it.
+                        candidate.datasrc_client = info.data_src_client_;
+                        candidate.finder = result.zone_finder;
+                        candidate.matched_labels = labels;
+                        candidate.matched = true;
+                    }
+                }
+                break;
+            default:
+                // Nothing found, nothing to do.
+                break;
+        }
+    }
+
+    // TODO: In case we have only the datasource and not the finder
+    // and the need_updater parameter is true, get the zone there.
+
+    // Return the partial match we have. In case we didn't want a partial
+    // match, this surely contains the original empty result.
+    return (candidate);
+}
+
+// NOTE: This function is not tested, it would be complicated. However, the
+// purpose of the function is to provide a very thin wrapper to be able to
+// replace the call to DataSourceClientContainer constructor in tests.
+ConfigurableClientList::DataSourcePair
+ConfigurableClientList::getDataSourceClient(const string& type,
+                                            const ConstElementPtr&
+                                            configuration)
+{
+    DataSourceClientContainerPtr
+        container(new DataSourceClientContainer(type, configuration));
+    return (DataSourcePair(&container->getInstance(), container));
+}
+
+}
+}

src/lib/datasrc/client_list.h

@@ -0,0 +1,289 @@
+// 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 DATASRC_CONTAINER_H
+#define DATASRC_CONTAINER_H
+
+#include <dns/name.h>
+#include <cc/data.h>
+#include <exceptions/exceptions.h>
+
+#include <vector>
+#include <boost/shared_ptr.hpp>
+#include <boost/noncopyable.hpp>
+
+namespace isc {
+namespace datasrc {
+
+class ZoneFinder;
+typedef boost::shared_ptr<ZoneFinder> ZoneFinderPtr;
+class DataSourceClient;
+typedef boost::shared_ptr<DataSourceClient> DataSourceClientPtr;
+class DataSourceClientContainer;
+typedef boost::shared_ptr<DataSourceClientContainer>
+    DataSourceClientContainerPtr;
+
+/// \brief The list of data source clients.
+///
+/// The purpose of this class is to hold several data source clients and search
+/// through them to find one containing a zone best matching a request.
+///
+/// All the data source clients should be for the same class. If you need
+/// to handle multiple classes, you need to create multiple separate lists.
+///
+/// This is an abstract base class. It is not expected we would use multiple
+/// implementation inside the servers (but it is not forbidden either), we
+/// have it to allow easy testing. It is possible to create a mock-up class
+/// instead of creating a full-blown configuration. The real implementation
+/// is the ConfigurableClientList.
+class ClientList : public boost::noncopyable {
+protected:
+    /// \brief Constructor.
+    ///
+    /// It is protected to prevent accidental creation of the abstract base
+    /// class.
+    ClientList() {}
+public:
+    /// \brief Virtual destructor
+    virtual ~ClientList() {}
+    /// \brief Structure holding the (compound) result of find.
+    ///
+    /// As this is read-only structure, we don't bother to create accessors.
+    /// Instead, all the member variables are defined as const and can be
+    /// accessed directly.
+    struct FindResult {
+        /// \brief Constructor.
+        ///
+        /// It simply fills in the member variables according to the
+        /// parameters. See the member descriptions for their meaning.
+        FindResult(DataSourceClient* dsrc_client, const ZoneFinderPtr& finder,
+                   bool exact_match) :
+            dsrc_client_(dsrc_client),
+            finder_(finder),
+            exact_match_(exact_match)
+        {}
+
+        /// \brief Negative answer constructor.
+        ///
+        /// This conscructs a result for negative answer. Both pointers are
+        /// NULL, and exact_match_ is false.
+        FindResult() :
+            dsrc_client_(NULL),
+            exact_match_(false)
+        {}
+
+        /// \brief Comparison operator.
+        ///
+        /// It is needed for tests and it might be of some use elsewhere
+        /// too.
+        bool operator ==(const FindResult& other) const {
+        return (dsrc_client_ == other.dsrc_client_ &&
+                finder_ == other.finder_ &&
+                exact_match_ == other.exact_match_);
+        }
+
+        /// \brief The found data source client.
+        ///
+        /// The client of the data source containing the best matching zone.
+        /// If no such data source exists, this is NULL pointer.
+        ///
+        /// Note that the pointer is valid only as long the ClientList which
+        /// returned the pointer is alive and was not reconfigured. The
+        /// ownership is preserved within the ClientList.
+        DataSourceClient* const dsrc_client_;
+
+        /// \brief The finder for the requested zone.
+        ///
+        /// This is the finder corresponding to the best matching zone.
+        /// This may be NULL even in case the datasrc_ is something
+        /// else, depending on the find options.
+        ///
+        /// \see find
+        const ZoneFinderPtr finder_;
+
+        /// \brief If the result is an exact match.
+        const bool exact_match_;
+    };
+
+    /// \brief Search for a zone through the data sources.
+    ///
+    /// This searches the contained data source clients for a one that best
+    /// matches the zone name.
+    ///
+    /// There are two expected usage scenarios. One is answering queries. In
+    /// this case, the zone finder is needed and the best matching superzone
+    /// of the searched name is needed. Therefore, the call would look like:
+    ///
+    /// \code FindResult result(list->find(queried_name));
+    ///   FindResult result(list->find(queried_name));
+    ///   if (result.datasrc_) {
+    ///       createTheAnswer(result.finder_);
+    ///   } else {
+    ///       createNotAuthAnswer();
+    /// } \endcode
+    ///
+    /// The other scenario is manipulating zone data (XfrOut, XfrIn, DDNS,
+    /// ...). In this case, the finder itself is not so important. However,
+    /// we need an exact match (if we want to manipulate zone data, we must
+    /// know exactly, which zone we are about to manipulate). Then the call
+    ///
+    /// \code FindResult result(list->find(zone_name, true, false));
+    ///   FindResult result(list->find(zone_name, true, false));
+    ///   if (result.datasrc_) {
+    ///       ZoneUpdaterPtr updater(result.datasrc_->getUpdater(zone_name);
+    ///       ...
+    /// } \endcode
+    ///
+    /// \param zone The name of the zone to look for.
+    /// \param want_exact_match If it is true, it returns only exact matches.
+    ///     If the best possible match is partial, a negative result is
+    ///     returned instead. It is possible the caller could check it and
+    ///     act accordingly if the result would be partial match, but with this
+    ///     set to true, the find might be actually faster under some
+    ///     circumstances.
+    /// \param want_finder If this is false, the finder_ member of FindResult
+    ///     might be NULL even if the corresponding data source is found. This
+    ///     is because of performance, in some cases the finder is a side
+    ///     result of the searching algorithm (therefore asking for it again
+    ///     would be a waste), but under other circumstances it is not, so
+    ///     providing it when it is not needed would also be wasteful.
+    ///
+    ///     Other things are never the side effect of searching, therefore the
+    ///     caller can get them explicitly (the updater, journal reader and
+    ///     iterator).
+    /// \return A FindResult describing the data source and zone with the
+    ///     longest match against the zone parameter.
+    virtual FindResult find(const dns::Name& zone,
+                            bool want_exact_match = false,
+                            bool want_finder = true) const = 0;
+};
+
+/// \brief Shared pointer to the list.
+typedef boost::shared_ptr<ClientList> ClientListPtr;
+/// \brief Shared const pointer to the list.
+typedef boost::shared_ptr<const ClientList> ConstClientListPtr;
+
+/// \Concrete implementation of the ClientList, which is constructed based on
+///     configuration.
+///
+/// This is the implementation which is expected to be used in the servers.
+/// However, it is expected most of the code will use it as the ClientList,
+/// only the creation is expected to be direct.
+///
+/// While it is possible to inherit this class, it is not expected to be
+/// inherited except for tests.
+class ConfigurableClientList : public ClientList {
+public:
+    /// \brief Exception thrown when there's an error in configuration.
+    class ConfigurationError : public Exception {
+    public:
+        ConfigurationError(const char* file, size_t line, const char* what) :
+            Exception(file, line, what)
+        {}
+    };
+
+    /// \brief Sets the configuration.
+    ///
+    /// This fills the ClientList with data source clients corresponding to the
+    /// configuration. The data source clients are newly created or recycled
+    /// from previous configuration.
+    ///
+    /// If any error is detected, an exception is thrown and the current
+    /// configuration is preserved.
+    ///
+    /// \param configuration The JSON element describing the configuration to
+    ///     use.
+    /// \param allow_cache If it is true, the 'cache' option of the
+    ///     configuration is used and some zones are cached into an In-Memory
+    ///     data source according to it. If it is false, it is ignored and
+    ///     no In-Memory data sources are created.
+    /// \throw DataSourceError if there's a problem creating a data source
+    ///     client.
+    /// \throw ConfigurationError if the configuration is invalid in some
+    ///     sense.
+    void configure(const data::Element& configuration, bool allow_cache);
+
+    /// \brief Implementation of the ClientList::find.
+    virtual FindResult find(const dns::Name& zone,
+                            bool want_exact_match = false,
+                            bool want_finder = true) const;
+
+    /// \brief This holds one data source client and corresponding information.
+    ///
+    /// \todo The content yet to be defined.
+    struct DataSourceInfo {
+        /// \brief Default constructor.
+        ///
+        /// Don't use directly. It is here so the structure can live in
+        /// a vector.
+        DataSourceInfo() :
+            data_src_client_(NULL)
+        {}
+        DataSourceInfo(DataSourceClient* data_src_client,
+                       const DataSourceClientContainerPtr& container) :
+            data_src_client_(data_src_client),
+            container_(container)
+        {}
+        DataSourceClient* data_src_client_;
+        DataSourceClientContainerPtr container_;
+    };
+
+    /// \brief The collection of data sources.
+    typedef std::vector<DataSourceInfo> DataSources;
+protected:
+    /// \brief The data sources held here.
+    ///
+    /// All our data sources are stored here. It is protected to let the
+    /// tests in. You should consider it private if you ever want to
+    /// derive this class (which is not really recommended anyway).
+    DataSources data_sources_;
+
+    /// \brief Convenience type alias.
+    ///
+    /// \see getDataSource
+    typedef std::pair<DataSourceClient*, DataSourceClientContainerPtr>
+        DataSourcePair;
+
+    /// \brief Create a data source client of given type and configuration.
+    ///
+    /// This is a thin wrapper around the DataSourceClientContainer
+    /// constructor. The function is here to make it possible for tests
+    /// to replace the DataSourceClientContainer with something else.
+    /// Also, derived classes could want to create the data source clients
+    /// in a different way, though inheriting this class is not recommended.
+    ///
+    /// The parameters are the same as of the constructor.
+    /// \return Pair containing both the data source client and the container.
+    ///     The container might be NULL in the derived class, it is
+    ///     only stored so the data source client is properly destroyed when
+    ///     not needed. However, in such case, it is the caller's
+    ///     responsibility to ensure the data source client is deleted when
+    ///     needed.
+    virtual DataSourcePair getDataSourceClient(const std::string& type,
+                                               const data::ConstElementPtr&
+                                               configuration);
+public:
+    /// \brief Access to the data source clients.
+    ///
+    /// It can be used to examine the loaded list of data sources clients
+    /// directly. It is not known if it is of any use other than testing, but
+    /// it might be, so it is just made public (there's no real reason to
+    /// hide it).
+    const DataSources& getDataSources() const { return (data_sources_); }
+};
+
+} // namespace datasrc
+} // namespace isc
+
+#endif // DATASRC_CONTAINER_H

src/lib/datasrc/tests/Makefile.am

@@ -17,6 +17,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 noinst_PROGRAMS =
 if HAVE_GTEST
@@ -59,6 +62,7 @@ run_unittests_SOURCES += memory_datasrc_unittest.cc
 run_unittests_SOURCES += rbnode_rrset_unittest.cc
 run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
+run_unittests_SOURCES += client_list_unittest.cc
 
 # We need the actual module implementation in the tests (they are not part
 # of libdatasrc)

src/lib/datasrc/tests/client_list_unittest.cc

@@ -0,0 +1,475 @@
+// 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 <datasrc/client_list.h>
+#include <datasrc/client.h>
+#include <datasrc/data_source.h>
+
+#include <dns/rrclass.h>
+
+#include <gtest/gtest.h>
+
+#include <set>
+
+using namespace isc::datasrc;
+using namespace isc::data;
+using namespace isc::dns;
+using namespace boost;
+using namespace std;
+
+namespace {
+
+// A test data source. It pretends it has some zones.
+class MockDataSourceClient : public DataSourceClient {
+public:
+    class Finder : public ZoneFinder {
+    public:
+        Finder(const Name& origin) :
+            origin_(origin)
+        {}
+        Name getOrigin() const { return (origin_); }
+        // The rest is not to be called, so just have them
+        RRClass getClass() const {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        shared_ptr<Context> find(const Name&, const RRType&,
+                                 const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        shared_ptr<Context> findAll(const Name&,
+                                    vector<ConstRRsetPtr>&,
+                                    const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        FindNSEC3Result findNSEC3(const Name&, bool) {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        Name findPreviousName(const Name&) const {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+    private:
+        Name origin_;
+    };
+    // Constructor from a list of zones.
+    MockDataSourceClient(const char* zone_names[]) {
+        for (const char** zone(zone_names); *zone; ++zone) {
+            zones.insert(Name(*zone));
+        }
+    }
+    // Constructor from configuration. The list of zones will be empty, but
+    // it will keep the configuration inside for further inspection.
+    MockDataSourceClient(const string& type,
+                         const ConstElementPtr& configuration) :
+        type_(type),
+        configuration_(configuration)
+    {}
+    virtual FindResult findZone(const Name& name) const {
+        if (zones.empty()) {
+            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
+        set<Name>::const_iterator it(zones.upper_bound(name));
+        if (it == zones.begin()) {
+            return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
+        --it;
+        NameComparisonResult compar(it->compare(name));
+        const ZoneFinderPtr finder(new Finder(*it));
+        switch (compar.getRelation()) {
+            case NameComparisonResult::EQUAL:
+                return (FindResult(result::SUCCESS, finder));
+            case NameComparisonResult::SUPERDOMAIN:
+                return (FindResult(result::PARTIALMATCH, finder));
+            default:
+                return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
+    }
+    // These methods are not used. They just need to be there to have
+    // complete vtable.
+    virtual ZoneUpdaterPtr getUpdater(const Name&, bool, bool) const {
+        isc_throw(isc::NotImplemented, "Not implemented");
+    }
+    virtual pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
+        getJournalReader(const Name&, uint32_t, uint32_t) const
+    {
+        isc_throw(isc::NotImplemented, "Not implemented");
+    }
+    const string type_;
+    const ConstElementPtr configuration_;
+private:
+    set<Name> zones;
+};
+
+
+// The test version is the same as the normal version. We, however, add
+// some methods to dig directly in the internals, for the tests.
+class TestedList : public ConfigurableClientList {
+public:
+    DataSources& getDataSources() { return (data_sources_); }
+    // Overwrite the list's method to get a data source with given type
+    // and configuration. We mock the data source and don't create the
+    // container. This is just to avoid some complexity in the tests.
+    virtual DataSourcePair getDataSourceClient(const string& type,
+                                               const ConstElementPtr&
+                                               configuration)
+    {
+        if (type == "error") {
+            isc_throw(DataSourceError, "The error data source type");
+        }
+        shared_ptr<MockDataSourceClient>
+            ds(new MockDataSourceClient(type, configuration));
+        // Make sure it is deleted when the test list is deleted.
+        to_delete_.push_back(ds);
+        return (DataSourcePair(ds.get(), DataSourceClientContainerPtr()));
+    }
+private:
+    // Hold list of data sources created internally, so they are preserved
+    // until the end of the test and then deleted.
+    vector<shared_ptr<MockDataSourceClient> > to_delete_;
+};
+
+const char* ds_zones[][3] = {
+    {
+        "example.org.",
+        "example.com.",
+        NULL
+    },
+    {
+        "sub.example.org.",
+        NULL, NULL
+    },
+    {
+        NULL, NULL, NULL
+    },
+    {
+        "sub.example.org.",
+        NULL, NULL
+    }
+};
+
+const size_t ds_count = (sizeof(ds_zones) / sizeof(*ds_zones));
+
+class ListTest : public ::testing::Test {
+public:
+    ListTest() :
+        // The empty list corresponds to a list with no elements inside
+        list_(new TestedList()),
+        config_elem_(Element::fromJSON("["
+            "{"
+            "   \"type\": \"test_type\","
+            "   \"cache\": \"off\","
+            "   \"params\": {}"
+            "}]"))
+    {
+        for (size_t i(0); i < ds_count; ++ i) {
+            shared_ptr<MockDataSourceClient>
+                ds(new MockDataSourceClient(ds_zones[i]));
+            ds_.push_back(ds);
+            ds_info_.push_back(ConfigurableClientList::DataSourceInfo(ds.get(),
+                DataSourceClientContainerPtr()));
+        }
+    }
+    // Check the positive result is as we expect it.
+    void positiveResult(const ClientList::FindResult& result,
+                        const shared_ptr<MockDataSourceClient>& dsrc,
+                        const Name& name, bool exact,
+                        const char* test)
+    {
+        SCOPED_TRACE(test);
+        EXPECT_EQ(dsrc.get(), result.dsrc_client_);
+        ASSERT_NE(ZoneFinderPtr(), result.finder_);
+        EXPECT_EQ(name, result.finder_->getOrigin());
+        EXPECT_EQ(exact, result.exact_match_);
+    }
+    // Configure the list with multiple data sources, according to
+    // some configuration. It uses the index as parameter, to be able to
+    // loop through the configurations.
+    void multiConfiguration(size_t index) {
+        list_->getDataSources().clear();
+        switch (index) {
+            case 2:
+                list_->getDataSources().push_back(ds_info_[2]);
+                // The ds_[2] is empty. We just check that it doesn't confuse
+                // us. Fall through to the case 0.
+            case 0:
+                list_->getDataSources().push_back(ds_info_[0]);
+                list_->getDataSources().push_back(ds_info_[1]);
+                break;
+            case 1:
+                // The other order
+                list_->getDataSources().push_back(ds_info_[1]);
+                list_->getDataSources().push_back(ds_info_[0]);
+                break;
+            case 3:
+                list_->getDataSources().push_back(ds_info_[1]);
+                list_->getDataSources().push_back(ds_info_[0]);
+                // It is the same as ds_[1], but we take from the first one.
+                // The first one to match is the correct one.
+                list_->getDataSources().push_back(ds_info_[3]);
+                break;
+            default:
+                FAIL() << "Unknown configuration index " << index;
+        }
+    }
+    void checkDS(size_t index, const string& type, const string& params) const
+    {
+        ASSERT_GT(list_->getDataSources().size(), index);
+        MockDataSourceClient* ds(dynamic_cast<MockDataSourceClient*>(
+            list_->getDataSources()[index].data_src_client_));
+
+        // Comparing with NULL does not work
+        ASSERT_NE(ds, static_cast<const MockDataSourceClient*>(NULL));
+        EXPECT_EQ(type, ds->type_);
+        EXPECT_TRUE(Element::fromJSON(params)->equals(*ds->configuration_));
+    }
+    shared_ptr<TestedList> list_;
+    const ClientList::FindResult negativeResult_;
+    vector<shared_ptr<MockDataSourceClient> > ds_;
+    vector<ConfigurableClientList::DataSourceInfo> ds_info_;
+    const ConstElementPtr config_elem_;
+};
+
+// Test the test itself
+TEST_F(ListTest, selfTest) {
+    EXPECT_EQ(result::SUCCESS, ds_[0]->findZone(Name("example.org")).code);
+    EXPECT_EQ(result::PARTIALMATCH,
+              ds_[0]->findZone(Name("sub.example.org")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("org")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[1]->findZone(Name("example.org")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("aaa")).code);
+    EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("zzz")).code);
+}
+
+// Test the list we create with empty configuration is, in fact, empty
+TEST_F(ListTest, emptyList) {
+    EXPECT_TRUE(list_->getDataSources().empty());
+}
+
+// Check the values returned by a find on an empty list. It should be
+// a negative answer (nothing found) no matter if we want an exact or inexact
+// match.
+TEST_F(ListTest, emptySearch) {
+    // No matter what we try, we don't get an answer.
+
+    // Note: we don't have operator<< for the result class, so we cannot use
+    // EXPECT_EQ.  Same for other similar cases.
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), false,
+                                               false));
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), false,
+                                               true));
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), true,
+                                               false));
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org"), true,
+                                               true));
+}
+
+// Put a single data source inside the list and check it can find an
+// exact match if there's one.
+TEST_F(ListTest, singleDSExactMatch) {
+    list_->getDataSources().push_back(ds_info_[0]);
+    // This zone is not there
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("org."), true));
+    // But this one is, so check it.
+    positiveResult(list_->find(Name("example.org"), true), ds_[0],
+                   Name("example.org"), true, "Exact match");
+    // When asking for a sub zone of a zone there, we get nothing
+    // (we want exact match, this would be partial one)
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("sub.example.org."),
+                                               true));
+}
+
+// When asking for a partial match, we get all that the exact one, but more.
+TEST_F(ListTest, singleDSBestMatch) {
+    list_->getDataSources().push_back(ds_info_[0]);
+    // This zone is not there
+    EXPECT_TRUE(negativeResult_ == list_->find(Name("org.")));
+    // But this one is, so check it.
+    positiveResult(list_->find(Name("example.org")), ds_[0],
+                   Name("example.org"), true, "Exact match");
+    // When asking for a sub zone of a zone there, we get the parent
+    // one.
+    positiveResult(list_->find(Name("sub.example.org.")), ds_[0],
+                   Name("example.org"), false, "Subdomain match");
+}
+
+const char* const test_names[] = {
+    "Sub second",
+    "Sub first",
+    "With empty",
+    "With a duplicity"
+};
+
+TEST_F(ListTest, multiExactMatch) {
+    // Run through all the multi-configurations
+    for (size_t i(0); i < sizeof(test_names) / sizeof(*test_names); ++i) {
+        SCOPED_TRACE(test_names[i]);
+        multiConfiguration(i);
+        // Something that is nowhere there
+        EXPECT_TRUE(negativeResult_ == list_->find(Name("org."), true));
+        // This one is there exactly.
+        positiveResult(list_->find(Name("example.org"), true), ds_[0],
+                       Name("example.org"), true, "Exact match");
+        // This one too, but in a different data source.
+        positiveResult(list_->find(Name("sub.example.org."), true), ds_[1],
+                       Name("sub.example.org"), true, "Subdomain match");
+        // But this one is in neither data source.
+        EXPECT_TRUE(negativeResult_ ==
+                    list_->find(Name("sub.example.com."), true));
+    }
+}
+
+TEST_F(ListTest, multiBestMatch) {
+    // Run through all the multi-configurations
+    for (size_t i(0); i < 4; ++ i) {
+        SCOPED_TRACE(test_names[i]);
+        multiConfiguration(i);
+        // Something that is nowhere there
+        EXPECT_TRUE(negativeResult_ == list_->find(Name("org.")));
+        // This one is there exactly.
+        positiveResult(list_->find(Name("example.org")), ds_[0],
+                       Name("example.org"), true, "Exact match");
+        // This one too, but in a different data source.
+        positiveResult(list_->find(Name("sub.example.org.")), ds_[1],
+                       Name("sub.example.org"), true, "Subdomain match");
+        // But this one is in neither data source. But it is a subdomain
+        // of one of the zones in the first data source.
+        positiveResult(list_->find(Name("sub.example.com.")), ds_[0],
+                       Name("example.com."), false, "Subdomain in com");
+    }
+}
+
+// Check the configuration is empty when the list is empty
+TEST_F(ListTest, configureEmpty) {
+    ConstElementPtr elem(new ListElement);
+    list_->configure(*elem, true);
+    EXPECT_TRUE(list_->getDataSources().empty());
+}
+
+// Check we can get multiple data sources and they are in the right order.
+TEST_F(ListTest, configureMulti) {
+    ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"type1\","
+        "   \"cache\": \"off\","
+        "   \"params\": {}"
+        "},"
+        "{"
+        "   \"type\": \"type2\","
+        "   \"cache\": \"off\","
+        "   \"params\": {}"
+        "}]"
+    ));
+    list_->configure(*elem, true);
+    EXPECT_EQ(2, list_->getDataSources().size());
+    checkDS(0, "type1", "{}");
+    checkDS(1, "type2", "{}");
+}
+
+// Check we can pass whatever we want to the params
+TEST_F(ListTest, configureParams) {
+    const char* params[] = {
+        "true",
+        "false",
+        "null",
+        "\"hello\"",
+        "42",
+        "[]",
+        "{}",
+        NULL
+    };
+    for (const char** param(params); *param; ++param) {
+        SCOPED_TRACE(*param);
+        ConstElementPtr elem(Element::fromJSON(string("["
+            "{"
+            "   \"type\": \"t\","
+            "   \"cache\": \"off\","
+            "   \"params\": ") + *param +
+            "}]"));
+        list_->configure(*elem, true);
+        EXPECT_EQ(1, list_->getDataSources().size());
+        checkDS(0, "t", *param);
+    }
+}
+
+TEST_F(ListTest, wrongConfig) {
+    const char* configs[] = {
+        // A lot of stuff missing from there
+        "[{\"type\": \"test_type\", \"params\": 13}, {}]",
+        // Some bad types completely
+        "{}",
+        "true",
+        "42",
+        "null",
+        "[{\"type\": \"test_type\", \"params\": 13}, true]",
+        "[{\"type\": \"test_type\", \"params\": 13}, []]",
+        "[{\"type\": \"test_type\", \"params\": 13}, 42]",
+        // Bad type of type
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": 42}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": true}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": null}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": []}]",
+        "[{\"type\": \"test_type\", \"params\": 13}, {\"type\": {}}]",
+        // TODO: Once cache is supported, add some invalid cache values
+        NULL
+    };
+    // Put something inside to see it survives the exception
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+    for (const char** config(configs); *config; ++config) {
+        SCOPED_TRACE(*config);
+        ConstElementPtr elem(Element::fromJSON(*config));
+        EXPECT_THROW(list_->configure(*elem, true),
+                     ConfigurableClientList::ConfigurationError);
+        // Still untouched
+        checkDS(0, "test_type", "{}");
+        EXPECT_EQ(1, list_->getDataSources().size());
+    }
+}
+
+// The param thing defaults to null. Cache is not used yet.
+TEST_F(ListTest, defaults) {
+    ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"type1\""
+        "}]"));
+    list_->configure(*elem, true);
+    EXPECT_EQ(1, list_->getDataSources().size());
+    checkDS(0, "type1", "null");
+}
+
+// Check we can call the configure multiple times, to change the configuration
+TEST_F(ListTest, reconfigure) {
+    ConstElementPtr empty(new ListElement);
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+    list_->configure(*empty, true);
+    EXPECT_TRUE(list_->getDataSources().empty());
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+}
+
+// Make sure the data source error exception from the factory is propagated
+TEST_F(ListTest, dataSrcError) {
+    ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"error\""
+        "}]"));
+    list_->configure(*config_elem_, true);
+    checkDS(0, "test_type", "{}");
+    EXPECT_THROW(list_->configure(*elem, true), DataSourceError);
+    checkDS(0, "test_type", "{}");
+}
+
+}

src/lib/dhcp/tests/Makefile.am

@@ -19,6 +19,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += libdhcp++_unittests

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

@@ -28,14 +28,14 @@ def read_wire_data(filename):
     data = bytes()
     for path in testdata_path.split(":"):
         try:
-            file = open(path + os.sep + filename, "r")
-            for line in file:
-                line = line.strip()
-                if line == "" or line.startswith("#"):
-                    pass
-                else:
-                    cur_data = bytes.fromhex(line)
-                    data += cur_data
+            with open(path + os.sep + filename, "r") as f:
+                for line in f:
+                    line = line.strip()
+                    if line == "" or line.startswith("#"):
+                        pass
+                    else:
+                        cur_data = bytes.fromhex(line)
+                        data += cur_data
 
             return data
         except IOError:

src/lib/dns/tests/Makefile.am

@@ -14,6 +14,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/exceptions/tests/Makefile.am

@@ -9,6 +9,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/log/tests/Makefile.am

@@ -47,6 +47,9 @@ 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)
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 if HAVE_GTEST
 TESTS =
 

src/lib/nsas/tests/Makefile.am

@@ -25,6 +25,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+	libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/python/isc/bind10/special_component.py

@@ -37,6 +37,7 @@ class SockCreator(BaseComponent):
         BaseComponent.__init__(self, boss, kind)
         self.__creator = None
         self.__uid = boss.uid
+        self.__gid = boss.gid
 
     def _start_internal(self):
         self._boss.curproc = 'b10-sockcreator'
@@ -45,6 +46,9 @@ class SockCreator(BaseComponent):
         self._boss.register_process(self.pid(), self)
         self._boss.set_creator(self.__creator)
         self._boss.log_started(self.pid())
+        if self.__gid is not None:
+            logger.info(BIND10_SETGID, self.__gid)
+            posix.setgid(self.__gid)
         if self.__uid is not None:
             logger.info(BIND10_SETUID, self.__uid)
             posix.setuid(self.__uid)

src/lib/python/isc/bind10/tests/component_test.py

@@ -104,6 +104,8 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.__stop_process_params = None
         self.__start_simple_params = None
         # Pretending to be boss
+        self.gid = None
+        self.__gid_set = None
         self.uid = None
         self.__uid_set = None
 
@@ -609,6 +611,9 @@ class ComponentTests(BossUtils, unittest.TestCase):
         self.assertTrue(process.killed)
         self.assertFalse(process.terminated)
 
+    def setgid(self, gid):
+        self.__gid_set = gid
+
     def setuid(self, uid):
         self.__uid_set = uid
 
@@ -637,7 +642,9 @@ class ComponentTests(BossUtils, unittest.TestCase):
         """
         component = isc.bind10.special_component.SockCreator(None, self,
                                                              'needed', None)
+        orig_setgid = isc.bind10.special_component.posix.setgid
         orig_setuid = isc.bind10.special_component.posix.setuid
+        isc.bind10.special_component.posix.setgid = self.setgid
         isc.bind10.special_component.posix.setuid = self.setuid
         orig_creator = \
             isc.bind10.special_component.isc.bind10.sockcreator.Creator
@@ -645,18 +652,22 @@ class ComponentTests(BossUtils, unittest.TestCase):
         isc.bind10.special_component.isc.bind10.sockcreator.Creator = \
             lambda path: self.FakeCreator()
         component.start()
-        # No uid set in boss, nothing called.
+        # No gid/uid set in boss, nothing called.
+        self.assertIsNone(self.__gid_set)
         self.assertIsNone(self.__uid_set)
         # Doesn't do anything, but doesn't crash
         component.stop()
         component.kill()
         component.kill(True)
+        self.gid = 4200
         self.uid = 42
         component = isc.bind10.special_component.SockCreator(None, self,
                                                              'needed', None)
         component.start()
         # This time, it get's called
+        self.assertEqual(4200, self.__gid_set)
         self.assertEqual(42, self.__uid_set)
+        isc.bind10.special_component.posix.setgid = orig_setgid
         isc.bind10.special_component.posix.setuid = orig_setuid
         isc.bind10.special_component.isc.bind10.sockcreator.Creator = \
             orig_creator

src/lib/python/isc/bind10/tests/sockcreator_test.py

@@ -303,6 +303,7 @@ class WrapTests(unittest.TestCase):
 
         # Transfer the descriptor
         send_fd(t1.fileno(), p1.fileno())
+        p1.close()
         p1 = socket.fromfd(t2.read_fd(), socket.AF_UNIX, socket.SOCK_STREAM)
 
         # Now, pass some data trough the socket
@@ -318,6 +319,14 @@ class WrapTests(unittest.TestCase):
         data = t1.recv(1)
         self.assertEqual(b'C', data)
 
+        # Explicitly close temporary socket pair as the Python
+        # interpreter expects it.  It may not be 100% exception safe,
+        # but since this is only for tests we prefer brevity.
+        p1.close()
+        p2.close()
+        t1.close()
+        t2.close()
+
 if __name__ == '__main__':
     isc.log.init("bind10") # FIXME Should this be needed?
     isc.log.resetUnitTestRootLogger()

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

@@ -46,8 +46,8 @@ class TestModuleSpec(unittest.TestCase):
         self.spec1(dd)
 
     def test_open_file_obj(self):
-        file1 = open(self.spec_file("spec1.spec"))
-        dd = isc.config.module_spec_from_file(file1)
+        with open(self.spec_file("spec1.spec")) as file1:
+            dd = isc.config.module_spec_from_file(file1)
         self.spec1(dd)
 
     def test_open_bad_file_obj(self):
@@ -89,8 +89,8 @@ class TestModuleSpec(unittest.TestCase):
 
     def validate_data(self, specfile_name, datafile_name):
         dd = self.read_spec_file(specfile_name);
-        data_file = open(self.spec_file(datafile_name))
-        data_str = data_file.read()
+        with open(self.spec_file(datafile_name)) as data_file:
+            data_str = data_file.read()
         data = isc.cc.data.parse_value_str(data_str)
         return dd.validate_config(True, data)
         
@@ -109,8 +109,8 @@ class TestModuleSpec(unittest.TestCase):
 
     def validate_command_params(self, specfile_name, datafile_name, cmd_name):
         dd = self.read_spec_file(specfile_name);
-        data_file = open(self.spec_file(datafile_name))
-        data_str = data_file.read()
+        with open(self.spec_file(datafile_name)) as data_file:
+            data_str = data_file.read()
         params = isc.cc.data.parse_value_str(data_str)
         return dd.validate_command(cmd_name, params)
 
@@ -131,8 +131,8 @@ class TestModuleSpec(unittest.TestCase):
     def test_statistics_validation(self):
         def _validate_stat(specfile_name, datafile_name):
             dd = self.read_spec_file(specfile_name);
-            data_file = open(self.spec_file(datafile_name))
-            data_str = data_file.read()
+            with open(self.spec_file(datafile_name)) as data_file:
+                data_str = data_file.read()
             data = isc.cc.data.parse_value_str(data_str)
             return dd.validate_statistics(True, data, [])
         self.assertFalse(self.read_spec_file("spec1.spec").validate_statistics(True, None, None));

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

@@ -242,12 +242,17 @@ class UpdateSession:
         '''
         try:
             self._get_update_zone()
+            # Contrary to what RFC2136 specifies, we do ACL checks before
+            # prerequisites. It's now generally considered to be a bad
+            # idea, and actually does harm such as information
+            # leak. It should make more sense to prevent any security issues
+            # by performing ACL check as early as possible.
+            self.__check_update_acl(self.__zname, self.__zclass)
             self._create_diff()
             prereq_result = self.__check_prerequisites()
             if prereq_result != Rcode.NOERROR():
                 self.__make_response(prereq_result)
                 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)

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

@@ -657,12 +657,12 @@ class SessionTest(SessionTestBase):
         self.assertEqual(str(expected_soa),
                          str(session._UpdateSession__added_soa))
 
-    def check_full_handle_result(self, expected, updates):
+    def check_full_handle_result(self, expected, updates, prerequisites=[]):
         '''Helper method for checking the result of a full handle;
            creates an update session, and fills it with the list of rrsets
            from 'updates'. Then checks if __handle()
            results in a response with rcode 'expected'.'''
-        msg = create_update_msg([TEST_ZONE_RECORD], [], updates)
+        msg = create_update_msg([TEST_ZONE_RECORD], prerequisites, updates)
         zconfig = ZoneConfig(set(), TEST_RRCLASS, self._datasrc_client,
                              self._acl_map)
         session = UpdateSession(msg, TEST_CLIENT4, zconfig)
@@ -902,6 +902,21 @@ class SessionTest(SessionTestBase):
                                 [ b'\x00\x0a\x04mail\x07example\x03org\x00' ])
         self.rrset_update_del_rrset_mx = rrset_update_del_rrset_mx
 
+    def test_acl_before_prereq(self):
+        name_in_use_no = create_rrset("foo.example.org", RRClass.ANY(),
+                                      RRType.ANY(), 0)
+
+        # Test a prerequisite that would fail
+        self.check_full_handle_result(Rcode.NXDOMAIN(), [], [ name_in_use_no ])
+
+        # Change ACL so that it would be denied
+        self._acl_map = {(TEST_ZONE_NAME, TEST_RRCLASS):
+                             REQUEST_LOADER.load([{"action": "REJECT"}])}
+
+        # The prerequisite should now not be reached; it should fail on the
+        # ACL
+        self.check_full_handle_result(Rcode.REFUSED(), [], [ name_in_use_no ])
+
     def test_prescan(self):
         '''Test whether the prescan succeeds on data that is ok, and whether
            if notices the SOA if present'''

src/lib/python/isc/util/cio/tests/socketsession_test.py

@@ -22,6 +22,8 @@ TESTDATA_OBJDIR = os.getenv("TESTDATAOBJDIR")
 TEST_UNIX_FILE = TESTDATA_OBJDIR + '/ssessiontest.unix'
 TEST_DATA = b'BIND10 test'
 TEST_PORT = 53535
+TEST_PORT2 = 53536
+TEST_PORT3 = 53537
 
 class TestForwarder(unittest.TestCase):
     '''In general, this is a straightforward port of the C++ counterpart.
@@ -31,12 +33,15 @@ class TestForwarder(unittest.TestCase):
     '''
 
     def setUp(self):
+        self.listen_sock = None
         self.forwarder = SocketSessionForwarder(TEST_UNIX_FILE)
         if os.path.exists(TEST_UNIX_FILE):
             os.unlink(TEST_UNIX_FILE)
         self.large_text = b'a' * 65535
 
     def tearDown(self):
+        if self.listen_sock is not None:
+            self.listen_sock.close()
         if os.path.exists(TEST_UNIX_FILE):
             os.unlink(TEST_UNIX_FILE)
 
@@ -172,15 +177,22 @@ class TestForwarder(unittest.TestCase):
             sock.settimeout(10)
             self.assertEqual(TEST_DATA, sock.recvfrom(len(TEST_DATA))[0])
         else:
-            server_sock.close()
             self.assertEqual(len(TEST_DATA), passed_sock.send(TEST_DATA))
             client_sock.setblocking(True)
             client_sock.settimeout(10)
             self.assertEqual(TEST_DATA, client_sock.recv(len(TEST_DATA)))
+            server_sock.close()
+            client_sock.close()
+
+        passed_sock.close()
+        sock.close()
 
     def test_push_and_pop(self):
-        # This is a straightforward port of C++ pushAndPop test.
+        # This is a straightforward port of C++ pushAndPop test.  See the
+        # C++ version why we use multiple ports for "local".
         local6 = ('::1', TEST_PORT, 0, 0)
+        local6_alt = ('::1', TEST_PORT2, 0, 0)
+        local6_alt2 = ('::1', TEST_PORT3, 0, 0)
         remote6 = ('2001:db8::1', 5300, 0, 0)
         self.check_push_and_pop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP,
                                 local6, remote6, TEST_DATA, True)
@@ -188,6 +200,7 @@ class TestForwarder(unittest.TestCase):
                                 local6, remote6, TEST_DATA, False)
 
         local4 = ('127.0.0.1', TEST_PORT)
+        local4_alt = ('127.0.0.1', TEST_PORT2)
         remote4 = ('192.0.2.2', 5300)
         self.check_push_and_pop(AF_INET, SOCK_DGRAM, IPPROTO_UDP,
                                 local4, remote4, TEST_DATA, False)
@@ -195,11 +208,11 @@ class TestForwarder(unittest.TestCase):
                                 local4, remote4, TEST_DATA, False)
 
         self.check_push_and_pop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP,
-                                local6, remote6, self.large_text, False)
+                                local6_alt, remote6, self.large_text, False)
         self.check_push_and_pop(AF_INET6, SOCK_STREAM, IPPROTO_TCP,
                                 local6, remote6, self.large_text, False)
         self.check_push_and_pop(AF_INET, SOCK_DGRAM, IPPROTO_UDP,
-                                local4, remote4, self.large_text, False)
+                                local4_alt, remote4, self.large_text, False)
         self.check_push_and_pop(AF_INET, SOCK_STREAM, IPPROTO_TCP,
                                 local4, remote4, self.large_text, False)
 
@@ -207,7 +220,7 @@ class TestForwarder(unittest.TestCase):
         # scope (zone) ID
         scope6 = ('fe80::1', TEST_PORT, 0, 1)
         self.check_push_and_pop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP,
-                                local6, scope6, TEST_DATA, False)
+                                local6_alt2, scope6, TEST_DATA, False)
 
     def test_push_too_fast(self):
         # A straightforward port of C++ pushTooFast test.
@@ -235,6 +248,7 @@ class TestForwarder(unittest.TestCase):
         receiver = SocketSessionReceiver(accept_sock)
         s.close()
         self.assertRaises(SocketSessionError, receiver.pop)
+        accept_sock.close()
 
 class TestReceiver(unittest.TestCase):
     # We only check a couple of failure cases on construction.  Valid cases

src/lib/resolve/tests/Makefile.am

@@ -8,6 +8,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/server_common/tests/Makefile.am

@@ -22,6 +22,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/statistics/tests/Makefile.am

@@ -15,6 +15,9 @@ AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/util/tests/Makefile.am

@@ -15,6 +15,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/lib/util/tests/socketsession_unittest.cc

@@ -53,6 +53,7 @@ namespace {
 
 const char* const TEST_UNIX_FILE = TEST_DATA_TOPBUILDDIR "/test.unix";
 const char* const TEST_PORT = "53535";
+const char* const TEST_PORT2 = "53536"; // use this in case we need 2 ports
 const char TEST_DATA[] = "BIND10 test";
 
 // A simple helper structure to automatically close test sockets on return
@@ -540,8 +541,12 @@ ForwardTest::checkPushAndPop(int family, int type, int protocol,
 }
 
 TEST_F(ForwardTest, pushAndPop) {
-    // Pass a UDP/IPv6 session.
+    // Pass a UDP/IPv6 session.  We use different ports for different UDP
+    // tests because Solaris 11 seems to prohibit reusing the same port for
+    // some short period once the socket FD is forwarded, even if the sockets
+    // are closed.  See Trac #2028.
     const SockAddrInfo sai_local6(getSockAddr("::1", TEST_PORT));
+    const SockAddrInfo sai_local6_alt(getSockAddr("::1", TEST_PORT2));
     const SockAddrInfo sai_remote6(getSockAddr("2001:db8::1", "5300"));
     {
         SCOPED_TRACE("Passing UDP/IPv6 session");
@@ -559,6 +564,7 @@ TEST_F(ForwardTest, pushAndPop) {
     // receiver, which should be usable for multiple attempts of passing,
     // regardless of family of the passed session
     const SockAddrInfo sai_local4(getSockAddr("127.0.0.1", TEST_PORT));
+    const SockAddrInfo sai_local4_alt(getSockAddr("127.0.0.1", TEST_PORT2));
     const SockAddrInfo sai_remote4(getSockAddr("192.0.2.2", "5300"));
     {
         SCOPED_TRACE("Passing UDP/IPv4 session");
@@ -575,7 +581,7 @@ TEST_F(ForwardTest, pushAndPop) {
     // Also try large data
     {
         SCOPED_TRACE("Passing UDP/IPv6 session with large data");
-        checkPushAndPop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, sai_local6,
+        checkPushAndPop(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, sai_local6_alt,
                         sai_remote6, large_text_.c_str(), large_text_.length(),
                         false);
     }
@@ -587,7 +593,7 @@ TEST_F(ForwardTest, pushAndPop) {
     }
     {
         SCOPED_TRACE("Passing UDP/IPv4 session with large data");
-        checkPushAndPop(AF_INET, SOCK_DGRAM, IPPROTO_UDP, sai_local4,
+        checkPushAndPop(AF_INET, SOCK_DGRAM, IPPROTO_UDP, sai_local4_alt,
                         sai_remote4, large_text_.c_str(), large_text_.length(),
                         false);
     }

src/lib/xfr/tests/Makefile.am

@@ -8,6 +8,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

src/valgrind-suppressions

@@ -0,0 +1,11 @@
+# Valgrind suppressions file. Place permanent suppressions that we never
+# want to reconsider again into this file. For temporary suppressions
+# that we want to revisit in the future, use
+# valgrind-suppressions.revisit.
+#
+# Don't add any "obj:" lines in suppressions as these are likely
+# site-specific. Use "..." instead to match these. Look at the other
+# suppressions as examples.
+#
+# In case you want to make sense of the following symbols, demangle them
+# with a command like: c++filt < valgrind-suppressions

src/valgrind-suppressions.revisit

@@ -0,0 +1,17 @@
+# Place temporary suppressions that we want to revisit in the future
+# into this file. For permanent suppressions that we don't want to look
+# at again, use valgrind-suppressions.
+#
+# Don't add any "obj:" lines in suppressions as these are likely
+# site-specific. Use "..." instead to match these. Look at the other
+# suppressions as examples.
+#
+# In case you want to make sense of the following symbols, demangle them
+# with a command like: c++filt < valgrind-suppressions.revisit
+
+############################################################################
+#### beginning of suppressions for existing issues that we want to fix. ####
+
+
+####### end of suppressions for existing issues that we want to fix. #######
+############################################################################

tests/lettuce/configurations/ddns/ddns.config.orig

@@ -0,0 +1,78 @@
+{
+    "version": 2,
+    "Logging": {
+        "loggers": [
+            {
+                "debuglevel": 99,
+                "severity": "DEBUG",
+                "name": "*"
+            }
+        ]
+    },
+    "Zonemgr": {
+        "secondary_zones": [
+            {
+                "class": "IN",
+                "name": "secondary.org"
+            }
+        ]
+    },
+    "Auth": {
+        "database_file": "data/ddns/example.org.sqlite3",
+        "listen_on": [
+            {
+                "port": 47806,
+                "address":
+                "127.0.0.1"
+            }
+        ]
+    },
+    "Boss": {
+        "components": {
+            "b10-xfrout": {
+                "kind": "dispensable",
+                "address": "Xfrout"
+            },
+            "b10-zonemgr": {
+                "kind": "dispensable",
+                "address": "ZoneMgr"
+            },
+            "b10-ddns": {
+                "kind": "dispensable",
+                "address": "DDNS"
+            },
+            "b10-auth": {
+                "kind": "needed",
+                "special": "auth"
+            },
+            "b10-cmdctl": {
+                "kind": "needed",
+                "special": "cmdctl"
+            }
+        }
+    },
+    "DDNS": {
+        "zones": [
+            {
+                "origin": "example.org.",
+                "update_acl": [
+                    {
+                        "action": "ACCEPT",
+                        "from": "127.0.0.1"
+                    }
+                ],
+                "class": "IN"
+            },
+            {
+                "origin": "secondary.org.",
+                "update_acl": [
+                    {
+                        "action": "ACCEPT",
+                        "from": "127.0.0.1"
+                    }
+                ],
+                "class": "IN"
+            }
+        ]
+    }
+}

tests/lettuce/configurations/ddns/noddns.config.orig

@@ -0,0 +1,43 @@
+{
+    "version": 2,
+    "Logging": {
+        "loggers": [
+            {
+                "severity": "DEBUG",
+                "name": "*",
+                "debuglevel": 99
+            }
+        ]
+    },
+    "DDNS": {"zones": []},
+    "Auth": {
+        "database_file": "data/ddns/example.org.sqlite3",
+        "listen_on": [
+            {
+                "port": 47806,
+                "address": "127.0.0.1"
+            }
+        ],
+        "datasources": [
+            {
+                "type": "memory",
+                "class": "IN",
+                "zones": [
+                    {
+                        "origin": "example.org",
+                        "filetype": "sqlite3",
+                        "file": "data/ddns/example.org.sqlite3"
+                    }
+                ]
+            }
+        ]
+    },
+    "Boss": {
+        "components": {
+            "b10-xfrout": {"kind": "dispensable"},
+            "b10-auth": {"kind": "needed", "special": "auth"},
+            "b10-zonemgr": {"kind": "dispensable", "address": "ZoneMgr" },
+            "b10-cmdctl": {"kind": "needed", "special": "cmdctl"}
+        }
+    }
+}

tests/lettuce/features/ddns_system.feature

@@ -0,0 +1,144 @@
+Feature: DDNS System
+    A number of BIND10-specific DDNS tests, that do not fall under the
+    'compliance' category; specific ACL checks, module checks, etc.
+
+    Scenario: Module tests
+        # The given config has b10-ddns disabled
+        Given I have bind10 running with configuration ddns/noddns.config
+        And wait for bind10 stderr message BIND10_STARTED_CC
+        And wait for bind10 stderr message AUTH_SERVER_STARTED
+
+        # Sanity check
+        bind10 module DDNS should not be running
+
+        # Test 1
+        When I use DDNS to set the SOA serial to 1235
+        # Note: test spec says refused here, system returns SERVFAIL
+        #The DDNS response should be REFUSED
+        The DDNS response should be SERVFAIL
+        And the SOA serial for example.org should be 1234
+
+        # Test 2
+        When I configure bind10 to run DDNS
+        And wait for new bind10 stderr message DDNS_STARTED
+        bind10 module DDNS should be running
+
+        # Test 3
+        When I use DDNS to set the SOA serial to 1236
+        The DDNS response should be REFUSED
+        And the SOA serial for example.org should be 1234
+
+        # Test 4
+        When I send bind10 the following commands
+        """
+        config add DDNS/zones
+        config set DDNS/zones[0]/origin example.org
+        config add DDNS/zones[0]/update_acl {"action": "ACCEPT", "from": "127.0.0.1"}
+        config commit
+        """
+
+        # Test 5
+        When I use DDNS to set the SOA serial to 1237
+        # also check if Auth server reloaded
+        And wait for new bind10 stderr message AUTH_LOAD_ZONE
+        The DDNS response should be SUCCESS
+        And the SOA serial for example.org should be 1237
+
+        # Test 6
+        When I send bind10 the command DDNS shutdown
+        And wait for new bind10 stderr message DDNS_STOPPED
+
+        # Test 7
+        # BoB should restart it
+        And wait for new bind10 stderr message DDNS_STARTED
+
+        # Test 8
+        # Known issue: after shutdown, first new attempt results in SERVFAIL
+        When I use DDNS to set the SOA serial to 1238
+        The DDNS response should be SERVFAIL
+        And the SOA serial for example.org should be 1237
+
+        When I use DDNS to set the SOA serial to 1238
+        And wait for new bind10 stderr message AUTH_LOAD_ZONE
+        The DDNS response should be SUCCESS
+        And the SOA serial for example.org should be 1238
+
+        # Test 9
+        When I send bind10 the command Auth shutdown
+        And wait for new bind10 stderr message AUTH_SHUTDOWN
+        # BoB should restart it automatically
+        And wait for new bind10 stderr message AUTH_SERVER_STARTED
+
+        # Test 10
+        When I use DDNS to set the SOA serial to 1239
+        And wait for new bind10 stderr message AUTH_LOAD_ZONE
+        The DDNS response should be SUCCESS
+        And the SOA serial for example.org should be 1239
+
+        # Test 11
+        When I configure BIND10 to stop running DDNS
+        And wait for new bind10 stderr message DDNS_STOPPED
+
+        bind10 module DDNS should not be running
+
+        # Test 12
+        When I use DDNS to set the SOA serial to 1240
+        # should this be REFUSED again?
+        The DDNS response should be SERVFAIL
+        And the SOA serial for example.org should be 1239
+
+    Scenario: ACL
+        Given I have bind10 running with configuration ddns/ddns.config
+        And wait for bind10 stderr message BIND10_STARTED_CC
+        And wait for bind10 stderr message AUTH_SERVER_STARTED
+        And wait for bind10 stderr message DDNS_STARTED
+
+        # Sanity check
+        A query for new1.example.org should have rcode NXDOMAIN
+        A query for new2.example.org should have rcode NXDOMAIN
+        A query for new3.example.org should have rcode NXDOMAIN
+        The SOA serial for example.org should be 1234
+
+        # Test 1
+        When I use DDNS to add a record new1.example.org. 3600 IN A 192.0.2.1
+        The DDNS response should be SUCCESS
+        A query for new1.example.org should have rcode NOERROR
+        The SOA serial for example.org should be 1235
+
+        # Test 2
+        When I set DDNS ACL 0 for 127.0.0.1 to REJECT
+        Then use DDNS to add a record new2.example.org. 3600 IN A 192.0.2.2
+        The DDNS response should be REFUSED
+        A query for new2.example.org should have rcode NXDOMAIN
+        The SOA serial for example.org should be 1235
+
+        # Test 3
+        When I set DDNS ACL 0 for 127.0.0.1 to ACCEPT
+        Then use DDNS to add a record new3.example.org. 3600 IN A 192.0.2.3
+        The DDNS response should be SUCCESS
+        A query for new3.example.org should have rcode NOERROR
+        The SOA serial for example.org should be 1236
+
+    #Scenario: DDNS and Xfrout
+    ## Unfortunately, Xfrout can only notify to inzone slaves, and hence only
+    ## to port 53, which we do not want to use for Lettuce tests (for various
+    ## reasons). So for now this test is only an outline, the configs
+    ## themselves have not been set up yet
+    #    When I start bind10 with configuration ddns/primary.config as primary
+    #    And wait for primary stderr message AUTH_SERVER_STARTED
+    #    And wait for primary stderr message XFROUT_STARTED
+    #    And wait for primary stderr message DDNS_STARTED
+
+    #    And I start bind10 with configuration example2.org.config with cmdctl port 47804 as secondary
+    #    And wait for secondary stderr message AUTH_SERVER_STARTED
+    #    And wait for secondary stderr message XFRIN_STARTED
+
+    #    # Sanity check
+    #    The SOA serial for example.org should be 1234
+    #    The SOA serial for example.org at 127.0.0.1:47807 should be 1234
+
+    #    When I use DDNS to set the SOA serial to 1235
+    #    The DDNS response should be SUCCESS
+
+    #    The SOA serial for example.org should be 1235
+    #    The SOA serial for example.org at 127.0.0.1:47807 should be 1235

tests/lettuce/features/example.feature

@@ -182,6 +182,9 @@ Feature: Example feature
 
         A query for www.example.org to 127.0.0.1:47806 should have rcode NOERROR
         A query for www.example.org to [::1]:47807 should have rcode NOERROR
+        The SOA serial for example.org should be 1234
+        The SOA serial for example.org at 127.0.0.1:47806 should be 1234
+        The SOA serial for example.org at ::1:47807 should be 1234
 
         Then set bind10 configuration Auth/database_file to data/empty_db.sqlite3
         And wait for bind10_one stderr message DATASRC_SQLITE_OPEN

tests/lettuce/features/terrain/bind10_control.py

@@ -52,7 +52,7 @@ def start_bind10(step, config_file, cmdctl_port, msgq_sockfile, process_name):
     It will also fail if there is a running process with the given process_name
     already.
     """
-    args = [ 'bind10', '-v' ]
+    args = [ 'bind10', '-n', '-v' ]
     if config_file is not None:
         args.append('-p')
         args.append("configurations/")
@@ -334,3 +334,32 @@ def module_is_running(step, name, not_str):
         not_str = ""
     step.given('send bind10 the command help')
     step.given('last bindctl output should' + not_str + ' contain ' + name + ' exactly')
+
+@step('Configure BIND10 to run DDNS')
+def configure_ddns_on(step):
+    """
+    Convenience compound step to enable the b10-ddns module.
+    """
+    step.behave_as("""
+    When I send bind10 the following commands
+        \"\"\"
+        config add Boss/components b10-ddns
+        config set Boss/components/b10-ddns/kind dispensable
+        config set Boss/components/b10-ddns/address DDNS
+        config commit
+        \"\"\"
+    """)
+
+@step('Configure BIND10 to stop running DDNS')
+def configure_ddns_off(step):
+    """
+    Convenience compound step to disable the b10-ddns module.
+    """
+    step.behave_as("""
+    When I send bind10 the following commands
+        \"\"\"
+        config remove Boss/components b10-ddns
+        config commit
+        \"\"\"
+    """)
+

tests/lettuce/features/terrain/nsupdate.py

@@ -0,0 +1,168 @@
+# Copyright (C) 2012  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+from lettuce import *
+import subprocess
+import re
+
+def run_nsupdate(commands):
+    """Run nsupdate.
+       Parameters:
+       commands: a sequence of strings which will be sent.
+       update_address: adress to send the update to
+       update_port: port to send the update to
+       zone: zone to update
+
+       Appends 'send' and 'quit' as final commands.
+
+       nsupdate's stdout and stderr streams are stored (as one multiline string
+       in world.last_nsupdate_stdout/stderr.
+
+       The return code is stored in world.last_nsupdate_returncode
+       (it is not checked here, since a number of tests intentionally
+       result in a non-zero return code).
+    """
+    commands.append('send')
+    commands.append('quit')
+    args = ['nsupdate' ]
+    nsupdate = subprocess.Popen(args, 1, None, subprocess.PIPE,
+                                subprocess.PIPE, subprocess.PIPE)
+    for line in commands:
+        nsupdate.stdin.write(line + "\n")
+    (stdout, stderr) = nsupdate.communicate()
+    world.last_nsupdate_returncode = nsupdate.returncode
+    world.last_nsupdate_stdout = stdout
+    world.last_nsupdate_stderr = stderr
+
+@step('send a DDNS update for (\S+) with the following commands:')
+def send_multiple_commands(step, zone):
+    """
+    Run nsupdate, and send it the given multiline set of commands.
+    A send and quit command is always appended.
+
+    This is the most 'raw' wrapper around the nsupdate call; every
+    command except the final send needs to be specified. Intended
+    for those tests that have unique properties.
+    """
+    commands = step.multiline.split("\n")
+    run_nsupdate(commands, zone)
+
+@step('DDNS response should be ([A-Z]+)')
+def check_ddns_response(step, response):
+    """
+    Checks the result of the last call to nsupdate.
+
+    If the given response argument is SUCCESS, it simply checks whether
+    the return code from nsupdate is 0 (there is no output in that case).
+    If not, it checks whether it is not 0, and if the given response string
+    matches a line 'update failed: <response>' in the stderr output of
+    nsupdate.
+
+    Prints exit code, stdout and stderr streams of nsupdate if it fails.
+    """
+    # For success, nsupdate is silent, only check result code 0
+    if response == "SUCCESS":
+        assert 0 == world.last_nsupdate_returncode,\
+               "nsupdate exit code: " + str(world.last_nsupdate_returncode) +\
+               "\nstdout:\n" + str(world.last_nsupdate_stdout) +\
+               "stderr:\n" + str(world.last_nsupdate_stderr)
+    else:
+        found = False
+        for line in world.last_nsupdate_stderr.split('\n'):
+            if line == "update failed: " + response:
+                found = True
+        assert found and (0 != world.last_nsupdate_returncode),\
+               "Response " + response + " not found in nsupdate output\n" +\
+               "nsupdate exit code: " + str(world.last_nsupdate_returncode) +\
+               "\nstdout:\n" + str(world.last_nsupdate_stdout) +\
+               "stderr:\n" + str(world.last_nsupdate_stderr)
+
+
+# Individual steps to create a DDNS update packet through nsupdate
+@step('Prepare a DDNS update(?: for (\S+))?(?: to (\S+)(?: port ([0-9]+)))?')
+def prepare_update(step, zone, server, port):
+    """
+    Prepares an nsupdate command that sets up an update to a server
+    for a zone. The update is not sent yet, but the commands
+    are stored in world.nsupdate_commands.
+    """
+    commands = []
+    if server is not None:
+        commands.append("server " + server)
+    else:
+        commands.append("server 127.0.0.1")
+    if port is not None:
+        commands[0] = commands[0] + " " + port
+    else:
+        commands[0] = commands[0] + " 47806"
+    if zone is not None:
+        commands.append("zone " + zone)
+    world.nsupdate_commands = commands
+
+@step('Add to the DDNS update: (.*)')
+def add_line_to_ddns_update(step, line):
+    """
+    Adds a single line to the prepared nsupdate. It is not sent yet.
+    The line must conform to nsupdate syntax.
+    """
+    world.nsupdate_commands.append(line)
+
+@step('Add the following lines to the DDNS update:')
+def add_lines_to_ddns_update(step, line):
+    """
+    Adds multiple lines to the prepared nsupdate. It is not sent yet.
+    The lines must conform to nsupdate syntax.
+    """
+    world.nsupdate_commands.extend(step.multiline.split('\n'))
+
+@step('Send the DDNS update')
+def run_ddns_update(step):
+    """
+    Runs the prepared nsupdate, see run_nsupdate() for more information.
+    """
+    run_nsupdate(world.nsupdate_commands)
+
+@step('use DDNS to set the SOA SERIAL to ([0-9]+)')
+def set_serial_to(step, new_serial):
+    """
+    Convenience compound step; prepare an update for example.org,
+    which sets the SERIAL to the given value, and send it.
+    It makes no other changes, and has hardcoded values for the other
+    SOA rdata fields.
+    """
+    step.given('Prepare a DDNS update')
+    step.given('add to the DDNS update: update add example.org 3600 IN SOA ns1.example.org. admin.example.org. ' + new_serial + ' 3600 1800 2419200 7200')
+    step.given('Send the DDNS update')
+
+@step('use DDNS to add a record (.*)')
+def add_record(step, new_record):
+    """
+    Convenience compound step; prepare an update for example.org,
+    which adds one record, then send it.
+    Apart from the update addition, the update will not contain anything else.
+    """
+    step.given('Prepare a DDNS update')
+    step.given('add to the DDNS update: update add ' + new_record)
+    step.given('Send the DDNS update')
+
+@step('set DDNS ACL ([0-9]+) for ([0-9.]+) to ([A-Z]+)')
+def set_ddns_acl_to(step, nr, address, action):
+    """
+    Convenience step to update a single ACL for DDNS.
+    Replaces the ACL at the given index for the given
+    address, to the given action
+    """
+    step.given('set bind10 configuration DDNS/zones[' + nr + ']/update_acl to [{"action": "' + action + '", "from": "' + address + '"}]')
+    step.given('last bindctl output should not contain Error')

tests/lettuce/features/terrain/querying.py

@@ -240,8 +240,8 @@ def query(step, dnssec, query_name, qtype, qclass, addr, port, rcode):
         "Expected: " + rcode + ", got " + query_result.rcode
     world.last_query_result = query_result
 
-@step('The SOA serial for ([\w.]+) should be ([0-9]+)')
-def query_soa(step, query_name, serial):
+@step('The SOA serial for ([\S.]+) (?:at (\S+)(?::([0-9]+)) )?should be ([0-9]+)')
+def query_soa(step, query_name, address, port, serial=None):
     """
     Convenience function to check the SOA SERIAL value of the given zone at
     the nameserver at the default address (127.0.0.1:47806).
@@ -251,7 +251,11 @@ def query_soa(step, query_name, serial):
     If the rcode is not NOERROR, or the answer section does not contain the
     SOA record, this step fails.
     """
-    query_result = QueryResult(query_name, "SOA", "IN", "127.0.0.1", "47806")
+    if address is None:
+        address = "127.0.0.1"
+    if port is None:
+        port = "47806"
+    query_result = QueryResult(query_name, "SOA", "IN", address, port)
     assert "NOERROR" == query_result.rcode,\
         "Got " + query_result.rcode + ", expected NOERROR"
     assert len(query_result.answer_section) == 1,\

tests/lettuce/features/terrain/terrain.py

@@ -53,8 +53,14 @@ copylist = [
      "configurations/resolver/resolver_basic.config"],
     ["configurations/multi_instance/multi_auth.config.orig",
      "configurations/multi_instance/multi_auth.config"],
+    ["configurations/ddns/ddns.config.orig",
+     "configurations/ddns/ddns.config"],
+    ["configurations/ddns/noddns.config.orig",
+     "configurations/ddns/noddns.config"],
     ["data/inmem-xfrin.sqlite3.orig",
-     "data/inmem-xfrin.sqlite3"]
+     "data/inmem-xfrin.sqlite3"],
+    ["data/ddns/example.org.sqlite3.orig",
+     "data/ddns/example.org.sqlite3"]
 ]
 
 # This is a list of files that, if present, will be removed before a scenario
@@ -258,7 +264,7 @@ class RunningProcesses:
         Initialize with no running processes.
         """
         self.processes = {}
-    
+
     def add_process(self, step, process_name, args):
         """
         Start a process with the given arguments, and store it under the given
@@ -297,14 +303,14 @@ class RunningProcesses:
             "Process " + name + " unknown"
         self.processes[process_name].stop_process()
         del self.processes[process_name]
-        
+
     def stop_all_processes(self):
         """
         Stop all running processes.
         """
         for process in self.processes.values():
             process.stop_process()
-    
+
     def keep_files(self):
         """
         Keep the redirection files for stdout/stderr output of all processes

tests/tools/badpacket/tests/Makefile.am

@@ -10,6 +10,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests

tests/tools/perfdhcp/tests/Makefile.am

@@ -10,6 +10,9 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 
+TESTS_ENVIRONMENT = \
+        libtool --mode=execute $(VALGRIND_COMMAND)
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests