Merge branch 'master' into trac1077

ChangeLog

@@ -1,8 +1,15 @@
-267.	[func]		stephen
+268.	[func]		stephen
 	Add environment variable to allow redirection of logging output during
 	unit tests.
 	(Trac #1071, git 05164f9d61006869233b498d248486b4307ea8b6)
 
+267.	[func]		tomek
+	Added a dummy module for DHCP6. This module does not actually
+	do anything at this point, and BIND 10 has no option for
+	starting it yet. It is included as a base for further
+	development.
+	(Trac #990, git 4a590df96a1b1d373e87f1f56edaceccb95f267d)
+
 266.	[func]		Multiple developers
         Convert various error messages, debugging and other output
         to the new logging interface, including for b10-resolver,

src/bin/auth/tests/command_unittest.cc

@@ -48,9 +48,9 @@ using namespace isc::datasrc;
 using namespace isc::config;
 
 namespace {
-class AuthConmmandTest : public ::testing::Test {
+class AuthCommandTest : public ::testing::Test {
 protected:
-    AuthConmmandTest() : server(false, xfrout), rcode(-1) {
+    AuthCommandTest() : server(false, xfrout), rcode(-1) {
         server.setStatisticsSession(&statistics_session);
     }
     void checkAnswer(const int expected_code) {
@@ -67,14 +67,14 @@ public:
     void stopServer();          // need to be public for boost::bind
 };
 
-TEST_F(AuthConmmandTest, unknownCommand) {
+TEST_F(AuthCommandTest, unknownCommand) {
     result = execAuthServerCommand(server, "no_such_command",
                                    ConstElementPtr());
     parseAnswer(rcode, result);
     EXPECT_EQ(1, rcode);
 }
 
-TEST_F(AuthConmmandTest, DISABLED_unexpectedException) {
+TEST_F(AuthCommandTest, DISABLED_unexpectedException) {
     // execAuthServerCommand() won't catch standard exceptions.
     // Skip this test for now: ModuleCCSession doesn't seem to validate
     // commands.
@@ -83,7 +83,7 @@ TEST_F(AuthConmmandTest, DISABLED_unexpectedException) {
                  runtime_error);
 }
 
-TEST_F(AuthConmmandTest, sendStatistics) {
+TEST_F(AuthCommandTest, sendStatistics) {
     result = execAuthServerCommand(server, "sendstats", ConstElementPtr());
     // Just check some message has been sent.  Detailed tests specific to
     // statistics are done in its own tests.
@@ -92,15 +92,15 @@ TEST_F(AuthConmmandTest, sendStatistics) {
 }
 
 void
-AuthConmmandTest::stopServer() {
+AuthCommandTest::stopServer() {
     result = execAuthServerCommand(server, "shutdown", ConstElementPtr());
     parseAnswer(rcode, result);
     assert(rcode == 0); // make sure the test stops when something is wrong
 }
 
-TEST_F(AuthConmmandTest, shutdown) {
+TEST_F(AuthCommandTest, shutdown) {
     isc::asiolink::IntervalTimer itimer(server.getIOService());
-    itimer.setup(boost::bind(&AuthConmmandTest::stopServer, this), 1);
+    itimer.setup(boost::bind(&AuthCommandTest::stopServer, this), 1);
     server.getIOService().run();
     EXPECT_EQ(0, rcode);
 }
@@ -165,7 +165,7 @@ newZoneChecks(AuthSrv& server) {
               find(Name("ns.test2.example"), RRType::AAAA()).code);
 }
 
-TEST_F(AuthConmmandTest, loadZone) {
+TEST_F(AuthCommandTest, loadZone) {
     configureZones(server);
 
     ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR
@@ -182,7 +182,7 @@ TEST_F(AuthConmmandTest, loadZone) {
     newZoneChecks(server);
 }
 
-TEST_F(AuthConmmandTest, loadBrokenZone) {
+TEST_F(AuthCommandTest, loadBrokenZone) {
     configureZones(server);
 
     ASSERT_EQ(0, system(INSTALL_PROG " " TEST_DATA_DIR
@@ -195,7 +195,7 @@ TEST_F(AuthConmmandTest, loadBrokenZone) {
     zoneChecks(server);     // zone shouldn't be replaced
 }
 
-TEST_F(AuthConmmandTest, loadUnreadableZone) {
+TEST_F(AuthCommandTest, loadUnreadableZone) {
     configureZones(server);
 
     // install the zone file as unreadable
@@ -209,7 +209,7 @@ TEST_F(AuthConmmandTest, loadUnreadableZone) {
     zoneChecks(server);     // zone shouldn't be replaced
 }
 
-TEST_F(AuthConmmandTest, loadZoneWithoutDataSrc) {
+TEST_F(AuthCommandTest, loadZoneWithoutDataSrc) {
     // try to execute load command without configuring the zone beforehand.
     // it should fail.
     result = execAuthServerCommand(server, "loadzone",
@@ -218,7 +218,7 @@ TEST_F(AuthConmmandTest, loadZoneWithoutDataSrc) {
     checkAnswer(1);
 }
 
-TEST_F(AuthConmmandTest, loadSqlite3DataSrc) {
+TEST_F(AuthCommandTest, loadSqlite3DataSrc) {
     // For sqlite3 data source we don't have to do anything (the data source
     // (re)loads itself automatically)
     result = execAuthServerCommand(server, "loadzone",
@@ -228,7 +228,7 @@ TEST_F(AuthConmmandTest, loadSqlite3DataSrc) {
     checkAnswer(0);
 }
 
-TEST_F(AuthConmmandTest, loadZoneInvalidParams) {
+TEST_F(AuthCommandTest, loadZoneInvalidParams) {
     configureZones(server);
 
     // null arg

src/bin/resolver/Makefile.am

@@ -60,6 +60,7 @@ b10_resolver_LDADD =  $(top_builddir)/src/lib/dns/libdns++.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/util/libutil.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/acl/libdnsacl.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/asiodns/libasiodns.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
@@ -68,7 +69,6 @@ b10_resolver_LDADD += $(top_builddir)/src/lib/log/liblog.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
-b10_resolver_LDADD += $(top_builddir)/src/lib/acl/libacl.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/resolve/libresolve.la
 b10_resolver_LDADD += $(top_builddir)/src/bin/auth/change_user.o
 b10_resolver_LDFLAGS = -pthread

src/bin/resolver/resolver.cc

@@ -26,7 +26,7 @@
 
 #include <exceptions/exceptions.h>
 
-#include <acl/acl.h>
+#include <acl/dns.h>
 #include <acl/loader.h>
 
 #include <asiodns/asiodns.h>
@@ -62,6 +62,7 @@ using boost::shared_ptr;
 using namespace isc;
 using namespace isc::util;
 using namespace isc::acl;
+using isc::acl::dns::RequestACL;
 using namespace isc::dns;
 using namespace isc::data;
 using namespace isc::config;
@@ -82,7 +83,9 @@ public:
         client_timeout_(4000),
         lookup_timeout_(30000),
         retries_(3),
-        query_acl_(new Resolver::ClientACL(REJECT)),
+        // we apply "reject all" (implicit default of the loader) ACL by
+        // default:
+        query_acl_(acl::dns::getRequestLoader().load(Element::fromJSON("[]"))),
         rec_query_(NULL)
     {}
 
@@ -160,11 +163,11 @@ public:
                                          OutputBufferPtr buffer,
                                          DNSServer* server);
 
-    const Resolver::ClientACL& getQueryACL() const {
+    const RequestACL& getQueryACL() const {
         return (*query_acl_);
     }
 
-    void setQueryACL(shared_ptr<const Resolver::ClientACL> new_acl) {
+    void setQueryACL(shared_ptr<const RequestACL> new_acl) {
         query_acl_ = new_acl;
     }
 
@@ -192,7 +195,7 @@ public:
 
 private:
     /// ACL on incoming queries
-    shared_ptr<const Resolver::ClientACL> query_acl_;
+    shared_ptr<const RequestACL> query_acl_;
 
     /// Object to handle upstream queries
     RecursiveQuery* rec_query_;
@@ -514,8 +517,10 @@ ResolverImpl::processNormalQuery(const IOMessage& io_message,
     const RRClass qclass = question->getClass();
 
     // Apply query ACL
-    Client client(io_message);
-    const BasicAction query_action(getQueryACL().execute(client));
+    const Client client(io_message);
+    const BasicAction query_action(
+        getQueryACL().execute(acl::dns::RequestContext(
+                                  client.getRequestSourceIPAddress())));
     if (query_action == isc::acl::REJECT) {
         LOG_INFO(resolver_logger, RESOLVER_QUERY_REJECTED)
             .arg(question->getName()).arg(qtype).arg(qclass).arg(client);
@@ -574,32 +579,6 @@ ResolverImpl::processNormalQuery(const IOMessage& io_message,
     return (RECURSION);
 }
 
-namespace {
-// This is a simplified ACL parser for the initial implementation with minimal
-// external dependency.  For a longer term we'll switch to a more generic
-// loader with allowing more complicated ACL syntax.
-shared_ptr<const Resolver::ClientACL>
-createQueryACL(isc::data::ConstElementPtr acl_config) {
-    if (!acl_config) {
-        return (shared_ptr<const Resolver::ClientACL>());
-    }
-
-    shared_ptr<Resolver::ClientACL> new_acl(
-        new Resolver::ClientACL(REJECT));
-    BOOST_FOREACH(ConstElementPtr rule, acl_config->listValue()) {
-        ConstElementPtr action = rule->get("action");
-        ConstElementPtr from = rule->get("from");
-        if (!action || !from) {
-            isc_throw(BadValue, "query ACL misses mandatory parameter");
-        }
-        new_acl->append(shared_ptr<IPCheck<Client> >(
-                            new IPCheck<Client>(from->stringValue())),
-                        defaultActionLoader(action));
-    }
-    return (new_acl);
-}
-}
-
 ConstElementPtr
 Resolver::updateConfig(ConstElementPtr config) {
     LOG_DEBUG(resolver_logger, RESOLVER_DBG_CONFIG, RESOLVER_CONFIG_UPDATED)
@@ -616,8 +595,10 @@ Resolver::updateConfig(ConstElementPtr config) {
         ConstElementPtr listenAddressesE(config->get("listen_on"));
         AddressList listenAddresses(parseAddresses(listenAddressesE,
                                                       "listen_on"));
-        shared_ptr<const ClientACL> query_acl(createQueryACL(
-                                                  config->get("query_acl")));
+        const ConstElementPtr query_acl_cfg(config->get("query_acl"));
+        const shared_ptr<const RequestACL> query_acl =
+            query_acl_cfg ? acl::dns::getRequestLoader().load(query_acl_cfg) :
+            shared_ptr<RequestACL>();
         bool set_timeouts(false);
         int qtimeout = impl_->query_timeout_;
         int ctimeout = impl_->client_timeout_;
@@ -777,13 +758,13 @@ Resolver::getListenAddresses() const {
     return (impl_->listen_);
 }
 
-const Resolver::ClientACL&
+const RequestACL&
 Resolver::getQueryACL() const {
     return (impl_->getQueryACL());
 }
 
 void
-Resolver::setQueryACL(shared_ptr<const ClientACL> new_acl) {
+Resolver::setQueryACL(shared_ptr<const RequestACL> new_acl) {
     if (!new_acl) {
         isc_throw(InvalidParameter, "NULL pointer is passed to setQueryACL");
     }

src/bin/resolver/resolver.h

@@ -21,10 +21,9 @@
 
 #include <boost/shared_ptr.hpp>
 
-#include <acl/acl.h>
-
 #include <cc/data.h>
 #include <config/ccsession.h>
+#include <acl/dns.h>
 #include <dns/message.h>
 #include <util/buffer.h>
 
@@ -41,12 +40,6 @@
 
 #include <resolve/resolver_interface.h>
 
-namespace isc {
-namespace server_common {
-class Client;
-}
-}
-
 class ResolverImpl;
 
 /**
@@ -246,13 +239,10 @@ public:
      */
     int getRetries() const;
 
-    // Shortcut typedef used for query ACL.
-    typedef isc::acl::ACL<isc::server_common::Client> ClientACL;
-
     /// Get the query ACL.
     ///
     /// \exception None
-    const ClientACL& getQueryACL() const;
+    const isc::acl::dns::RequestACL& getQueryACL() const;
 
     /// Set the new query ACL.
     ///
@@ -265,7 +255,8 @@ public:
     /// \exception InvalidParameter The given pointer is NULL
     ///
     /// \param new_acl The new ACL to replace the existing one.
-    void setQueryACL(boost::shared_ptr<const ClientACL> new_acl);
+    void setQueryACL(boost::shared_ptr<const isc::acl::dns::RequestACL>
+                     new_acl);
 
 private:
     ResolverImpl* impl_;

src/bin/resolver/tests/Makefile.am

@@ -39,6 +39,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
 run_unittests_LDADD += $(top_builddir)/src/lib/asiodns/libasiodns.la
 run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 run_unittests_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
+run_unittests_LDADD += $(top_builddir)/src/lib/acl/libdnsacl.la
 run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la

src/bin/resolver/tests/resolver_config_unittest.cc

@@ -43,6 +43,7 @@
 using namespace std;
 using boost::scoped_ptr;
 using namespace isc::acl;
+using isc::acl::dns::RequestContext;
 using namespace isc::data;
 using namespace isc::testutils;
 using namespace isc::asiodns;
@@ -57,19 +58,22 @@ protected:
     DNSService dnss;
     Resolver server;
     scoped_ptr<const IOEndpoint> endpoint;
-    scoped_ptr<const IOMessage> request;
+    scoped_ptr<const IOMessage> query_message;
     scoped_ptr<const Client> client;
+    scoped_ptr<const RequestContext> request;
     ResolverConfig() : dnss(ios, NULL, NULL, NULL) {
         server.setDNSService(dnss);
         server.setConfigured();
     }
-    const Client& createClient(const string& source_addr) {
+    const RequestContext& createRequest(const string& source_addr) {
         endpoint.reset(IOEndpoint::create(IPPROTO_UDP, IOAddress(source_addr),
                                           53210));
-        request.reset(new IOMessage(NULL, 0, IOSocket::getDummyUDPSocket(),
-                                    *endpoint));
-        client.reset(new Client(*request));
-        return (*client);
+        query_message.reset(new IOMessage(NULL, 0,
+                                          IOSocket::getDummyUDPSocket(),
+                                          *endpoint));
+        client.reset(new Client(*query_message));
+        request.reset(new RequestContext(client->getRequestSourceIPAddress()));
+        return (*request);
     }
     void invalidTest(const string &JSON, const string& name);
 };
@@ -100,14 +104,14 @@ TEST_F(ResolverConfig, forwardAddresses) {
 
 TEST_F(ResolverConfig, forwardAddressConfig) {
     // Try putting there some address
-    ElementPtr config(Element::fromJSON("{"
-        "\"forward_addresses\": ["
-        "   {"
-        "       \"address\": \"192.0.2.1\","
-        "       \"port\": 53"
-        "   }"
-        "]"
-        "}"));
+    ConstElementPtr config(Element::fromJSON("{"
+                                             "\"forward_addresses\": ["
+                                             " {"
+                                             "   \"address\": \"192.0.2.1\","
+                                             "   \"port\": 53"
+                                             " }"
+                                             "]"
+                                             "}"));
     ConstElementPtr result(server.updateConfig(config));
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
     EXPECT_TRUE(server.isForwarding());
@@ -127,14 +131,14 @@ TEST_F(ResolverConfig, forwardAddressConfig) {
 
 TEST_F(ResolverConfig, rootAddressConfig) {
     // Try putting there some address
-    ElementPtr config(Element::fromJSON("{"
-        "\"root_addresses\": ["
-        "   {"
-        "       \"address\": \"192.0.2.1\","
-        "       \"port\": 53"
-        "   }"
-        "]"
-        "}"));
+    ConstElementPtr config(Element::fromJSON("{"
+                                             "\"root_addresses\": ["
+                                             " {"
+                                             "    \"address\": \"192.0.2.1\","
+                                             "    \"port\": 53"
+                                             " }"
+                                             "]"
+                                             "}"));
     ConstElementPtr result(server.updateConfig(config));
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
     ASSERT_EQ(1, server.getRootAddresses().size());
@@ -210,12 +214,12 @@ TEST_F(ResolverConfig, timeouts) {
 }
 
 TEST_F(ResolverConfig, timeoutsConfig) {
-    ElementPtr config = Element::fromJSON("{"
-            "\"timeout_query\": 1000,"
-            "\"timeout_client\": 2000,"
-            "\"timeout_lookup\": 3000,"
-            "\"retries\": 4"
-            "}");
+    ConstElementPtr config = Element::fromJSON("{"
+                                               "\"timeout_query\": 1000,"
+                                               "\"timeout_client\": 2000,"
+                                               "\"timeout_lookup\": 3000,"
+                                               "\"retries\": 4"
+                                               "}");
     ConstElementPtr result(server.updateConfig(config));
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
     EXPECT_EQ(1000, server.getQueryTimeout());
@@ -253,51 +257,51 @@ TEST_F(ResolverConfig, invalidTimeoutsConfig) {
 
 TEST_F(ResolverConfig, defaultQueryACL) {
     // If no configuration is loaded, the default ACL should reject everything.
-    EXPECT_EQ(REJECT, server.getQueryACL().execute(createClient("192.0.2.1")));
+    EXPECT_EQ(REJECT, server.getQueryACL().execute(createRequest("192.0.2.1")));
     EXPECT_EQ(REJECT, server.getQueryACL().execute(
-                  createClient("2001:db8::1")));
+                  createRequest("2001:db8::1")));
 
     // The following would be allowed if the server had loaded the default
     // configuration from the spec file.  In this context it should not have
     // happened, and they should be rejected just like the above cases.
-    EXPECT_EQ(REJECT, server.getQueryACL().execute(createClient("127.0.0.1")));
-    EXPECT_EQ(REJECT, server.getQueryACL().execute(createClient("::1")));
+    EXPECT_EQ(REJECT, server.getQueryACL().execute(createRequest("127.0.0.1")));
+    EXPECT_EQ(REJECT, server.getQueryACL().execute(createRequest("::1")));
 }
 
 TEST_F(ResolverConfig, emptyQueryACL) {
     // Explicitly configured empty ACL should have the same effect.
-    ElementPtr config(Element::fromJSON("{ \"query_acl\": [] }"));
+    ConstElementPtr config(Element::fromJSON("{ \"query_acl\": [] }"));
     ConstElementPtr result(server.updateConfig(config));
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
-    EXPECT_EQ(REJECT, server.getQueryACL().execute(createClient("192.0.2.1")));
+    EXPECT_EQ(REJECT, server.getQueryACL().execute(createRequest("192.0.2.1")));
     EXPECT_EQ(REJECT, server.getQueryACL().execute(
-                  createClient("2001:db8::1")));
+                  createRequest("2001:db8::1")));
 }
 
 TEST_F(ResolverConfig, queryACLIPv4) {
     // A simple "accept" query for a specific IPv4 address
-    ElementPtr config(Element::fromJSON(
-                          "{ \"query_acl\": "
-                          "  [ {\"action\": \"ACCEPT\","
-                          "     \"from\": \"192.0.2.1\"} ] }"));
+    ConstElementPtr config(Element::fromJSON(
+                               "{ \"query_acl\": "
+                               "  [ {\"action\": \"ACCEPT\","
+                               "     \"from\": \"192.0.2.1\"} ] }"));
     ConstElementPtr result(server.updateConfig(config));
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
-    EXPECT_EQ(ACCEPT, server.getQueryACL().execute(createClient("192.0.2.1")));
+    EXPECT_EQ(ACCEPT, server.getQueryACL().execute(createRequest("192.0.2.1")));
     EXPECT_EQ(REJECT, server.getQueryACL().execute(
-                  createClient("2001:db8::1")));
+                  createRequest("2001:db8::1")));
 }
 
 TEST_F(ResolverConfig, queryACLIPv6) {
     // same for IPv6
-    ElementPtr config(Element::fromJSON(
-                          "{ \"query_acl\": "
-                          "  [ {\"action\": \"ACCEPT\","
-                          "     \"from\": \"2001:db8::1\"} ] }"));
+    ConstElementPtr config(Element::fromJSON(
+                               "{ \"query_acl\": "
+                               "  [ {\"action\": \"ACCEPT\","
+                               "     \"from\": \"2001:db8::1\"} ] }"));
     ConstElementPtr result(server.updateConfig(config));
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
-    EXPECT_EQ(REJECT, server.getQueryACL().execute(createClient("192.0.2.1")));
+    EXPECT_EQ(REJECT, server.getQueryACL().execute(createRequest("192.0.2.1")));
     EXPECT_EQ(ACCEPT, server.getQueryACL().execute(
-                  createClient("2001:db8::1")));
+                  createRequest("2001:db8::1")));
 }
 
 TEST_F(ResolverConfig, multiEntryACL) {
@@ -306,25 +310,26 @@ TEST_F(ResolverConfig, multiEntryACL) {
     // as it should have been tested in the underlying ACL module.  All we
     // have to do to check is a reasonably complicated ACL configuration is
     // loaded as expected.
-    ElementPtr config(Element::fromJSON(
-                          "{ \"query_acl\": "
-                          "  [ {\"action\": \"ACCEPT\","
-                          "     \"from\": \"192.0.2.1\"},"
-                          "    {\"action\": \"REJECT\","
-                          "     \"from\": \"192.0.2.0/24\"},"
-                          "    {\"action\": \"DROP\","
-                          "     \"from\": \"2001:db8::1\"},"
-                          "] }"));
+    ConstElementPtr config(Element::fromJSON(
+                               "{ \"query_acl\": "
+                               "  [ {\"action\": \"ACCEPT\","
+                               "     \"from\": \"192.0.2.1\"},"
+                               "    {\"action\": \"REJECT\","
+                               "     \"from\": \"192.0.2.0/24\"},"
+                               "    {\"action\": \"DROP\","
+                               "     \"from\": \"2001:db8::1\"},"
+                               "] }"));
     ConstElementPtr result(server.updateConfig(config));
     EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
-    EXPECT_EQ(ACCEPT, server.getQueryACL().execute(createClient("192.0.2.1")));
-    EXPECT_EQ(REJECT, server.getQueryACL().execute(createClient("192.0.2.2")));
+    EXPECT_EQ(ACCEPT, server.getQueryACL().execute(createRequest("192.0.2.1")));
+    EXPECT_EQ(REJECT, server.getQueryACL().execute(createRequest("192.0.2.2")));
     EXPECT_EQ(DROP, server.getQueryACL().execute(
-                  createClient("2001:db8::1")));
+                  createRequest("2001:db8::1")));
     EXPECT_EQ(REJECT, server.getQueryACL().execute(
-                  createClient("2001:db8::2"))); // match the default rule
+                  createRequest("2001:db8::2"))); // match the default rule
 }
 
+
 int
 getResultCode(ConstElementPtr result) {
     int rcode;
@@ -332,6 +337,22 @@ getResultCode(ConstElementPtr result) {
     return (rcode);
 }
 
+TEST_F(ResolverConfig, queryACLActionOnly) {
+    // "action only" rule will be accepted by the loader, which can
+    // effectively change the default action.
+    ConstElementPtr config(Element::fromJSON(
+                               "{ \"query_acl\": "
+                               "  [ {\"action\": \"ACCEPT\","
+                               "     \"from\": \"192.0.2.1\"},"
+                               "    {\"action\": \"DROP\"} ] }"));
+    EXPECT_EQ(0, getResultCode(server.updateConfig(config)));
+    EXPECT_EQ(ACCEPT, server.getQueryACL().execute(createRequest("192.0.2.1")));
+
+    // We reject non matching queries by default, but the last resort
+    // rule should have changed the action in that case to "DROP".
+    EXPECT_EQ(DROP, server.getQueryACL().execute(createRequest("192.0.2.2")));
+}
+
 TEST_F(ResolverConfig, badQueryACL) {
     // Most of these cases shouldn't happen in practice because the syntax
     // check should be performed before updateConfig().  But we check at
@@ -346,10 +367,6 @@ TEST_F(ResolverConfig, badQueryACL) {
                   server.updateConfig(
                       Element::fromJSON("{ \"query_acl\":"
                                         " [ {\"from\": \"192.0.2.1\"} ] }"))));
-    EXPECT_EQ(1, getResultCode(
-                  server.updateConfig(
-                      Element::fromJSON("{ \"query_acl\":"
-                                        " [ {\"action\": \"DROP\"} ] }"))));
     // invalid "action"
     EXPECT_EQ(1, getResultCode(
                   server.updateConfig(
@@ -361,7 +378,6 @@ TEST_F(ResolverConfig, badQueryACL) {
                       Element::fromJSON("{ \"query_acl\":"
                                         " [ {\"action\": \"BADACTION\","
                                         "    \"from\": \"192.0.2.1\"}]}"))));
-
     // invalid "from"
     EXPECT_EQ(1, getResultCode(
                   server.updateConfig(

src/bin/resolver/tests/resolver_unittest.cc

@@ -27,6 +27,7 @@
 using namespace std;
 using namespace isc::dns;
 using namespace isc::data;
+using isc::acl::dns::RequestACL;
 using namespace isc::testutils;
 using isc::UnitTestUtil;
 
@@ -156,8 +157,7 @@ TEST_F(ResolverTest, notifyFail) {
 TEST_F(ResolverTest, setQueryACL) {
     // valid cases are tested through other tests.  We only explicitly check
     // an invalid case: passing a NULL shared pointer.
-    EXPECT_THROW(server.setQueryACL(
-                     boost::shared_ptr<const Resolver::ClientACL>()),
+    EXPECT_THROW(server.setQueryACL(boost::shared_ptr<const RequestACL>()),
                  isc::InvalidParameter);
 }
 

src/lib/acl/dns.cc

@@ -12,20 +12,107 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include "dns.h"
+#include <memory>
+#include <string>
+#include <vector>
+
+#include <boost/shared_ptr.hpp>
+
+#include <exceptions/exceptions.h>
+
+#include <cc/data.h>
+
+#include <acl/dns.h>
+#include <acl/ip_check.h>
+#include <acl/loader.h>
+#include <acl/logic_check.h>
+
+using namespace std;
+using boost::shared_ptr;
+using namespace isc::data;
 
 namespace isc {
 namespace acl {
+
+/// The specialization of \c IPCheck for access control with \c RequestContext.
+///
+/// It returns \c true if the remote (source) IP address of the request
+/// matches the expression encapsulated in the \c IPCheck, and returns
+/// \c false if not.
+///
+/// \note The match logic is expected to be extended as we add
+/// more match parameters (at least there's a plan for TSIG key).
+template <>
+bool
+IPCheck<dns::RequestContext>::matches(
+    const dns::RequestContext& request) const
+{
+    return (compare(request.remote_address.getData(),
+                    request.remote_address.getFamily()));
+}
+
 namespace dns {
 
-Loader&
-getLoader() {
-    static Loader* loader(NULL);
+vector<string>
+internal::RequestCheckCreator::names() const {
+    // Probably we should eventually build this vector in a more
+    // sophisticated way.  For now, it's simple enough to hardcode
+    // everything.
+    vector<string> supported_names;
+    supported_names.push_back("from");
+    return (supported_names);
+}
+
+shared_ptr<RequestCheck>
+internal::RequestCheckCreator::create(const string& name,
+                                      ConstElementPtr definition,
+                                      // unused:
+                                      const acl::Loader<RequestContext>&)
+{
+    if (!definition) {
+        isc_throw(LoaderError,
+                  "NULL pointer is passed to RequestCheckCreator");
+    }
+
+    if (name == "from") {
+        return (shared_ptr<internal::RequestIPCheck>(
+                    new internal::RequestIPCheck(definition->stringValue())));
+    } else {
+        // This case shouldn't happen (normally) as it should have been
+        // rejected at the loader level.  But we explicitly catch the case
+        // and throw an exception for that.
+        isc_throw(LoaderError, "Invalid check name for RequestCheck: " <<
+                  name);
+    }
+}
+
+RequestLoader&
+getRequestLoader() {
+    static RequestLoader* loader(NULL);
     if (loader == NULL) {
-        loader = new Loader(REJECT);
-        // TODO: This is the place where we register default check creators
-        // like IP check, etc, once we have them.
+        // Creator registration may throw, so we first store the new loader
+        // in an auto pointer in order to provide the strong exception
+        // guarantee.
+        auto_ptr<RequestLoader> loader_ptr =
+            auto_ptr<RequestLoader>(new RequestLoader(REJECT));
+
+        // Register default check creator(s)
+        loader_ptr->registerCreator(shared_ptr<internal::RequestCheckCreator>(
+                                        new internal::RequestCheckCreator()));
+        loader_ptr->registerCreator(
+            shared_ptr<NotCreator<RequestContext> >(
+                new NotCreator<RequestContext>("NOT")));
+        loader_ptr->registerCreator(
+            shared_ptr<LogicCreator<AnyOfSpec, RequestContext> >(
+                new LogicCreator<AnyOfSpec, RequestContext>("ANY")));
+        loader_ptr->registerCreator(
+            shared_ptr<LogicCreator<AllOfSpec, RequestContext> >(
+                new LogicCreator<AllOfSpec, RequestContext>("ALL")));
+
+        // From this point there shouldn't be any exception thrown
+        loader = loader_ptr.release();
     }
+
     return (*loader);
 }
 

src/lib/acl/dns.h

@@ -13,12 +13,17 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #ifndef ACL_DNS_H
-#define ACL_DNS_H
+#define ACL_DNS_H 1
 
-#include "loader.h"
+#include <string>
+#include <vector>
 
-#include <asiolink/io_address.h>
-#include <dns/message.h>
+#include <boost/shared_ptr.hpp>
+
+#include <cc/data.h>
+
+#include <acl/ip_check.h>
+#include <acl/loader.h>
 
 namespace isc {
 namespace acl {
@@ -30,47 +35,65 @@ namespace dns {
  * This plays the role of Context of the generic template ACLs (in namespace
  * isc::acl).
  *
- * It is simple structure holding just the bunch of information. Therefore
- * the names don't end up with a slash, there are no methods so they can't be
- * confused with local variables.
+ * It is a simple structure holding just the bunch of information. Therefore
+ * the names don't end up with an underscore; there are no methods so they
+ * can't be confused with local variables.
+ *
+ * This structure is generally expected to be ephemeral and read-only: It
+ * would be constructed immediately before a particular ACL is checked
+ * and used only for the ACL match purposes.  Due to this nature, and since
+ * ACL processing is often performance sensitive (typically it's performed
+ * against all incoming packets), the construction is designed to be
+ * lightweight: it tries to avoid expensive data copies or dynamic memory
+ * allocation as much as possible.  Specifically, the constructor can
+ * take a pointer or reference to an object and keeps it as a reference
+ * (not making a local copy).  This also means the caller is responsible for
+ * keeping the passed parameters valid while this structure is used.
+ * This should generally be reasonable as this structure is expected to be
+ * used only for a very short period as stated above.
  *
- * \todo Do we want a constructor to set this in a shorter manner? So we can
- *     call the ACLs directly?
+ * Based on the minimalist philosophy, the initial implementation only
+ * maintains the remote (source) IP address of the request.  The plan is
+ * to add more parameters of the request.  A scheduled next step is to
+ * support the TSIG key (if it's included in the request).  Other possibilities
+ * are the local (destination) IP address, the remote and local port numbers,
+ * various fields of the DNS request (e.g. a particular header flag value).
  */
 struct RequestContext {
-    /// \brief The DNS message (payload).
-    isc::dns::ConstMessagePtr message;
-    /// \brief The remote IP address (eg. the client).
-    asiolink::IOAddress remote_address;
-    /// \brief The local IP address (ours, of the interface where we received).
-    asiolink::IOAddress local_address;
-    /// \brief The remote port.
-    uint16_t remote_port;
-    /// \brief The local port.
-    uint16_t local_port;
-    /**
-     * \brief Name of the TSIG key the message is signed with.
-     *
-     * This will be either the name of the TSIG key the message is signed with,
-     * or empty string, if the message is not signed. It is true we could get
-     * the information from the message itself, but because at the time when
-     * the ACL is checked, the signature has been verified already, so passing
-     * it around is probably cheaper.
-     *
-     * It is expected that messages with invalid signatures are handled before
-     * ACL.
-     */
-    std::string tsig_key_name;
+    /// The constructor
+    ///
+    /// This is a trivial constructor that perform straightforward
+    /// initialization of the member variables from the given parameters.
+    ///
+    /// \exception None
+    ///
+    /// \parameter remote_address_param The remote IP address
+    explicit RequestContext(const IPAddress& remote_address_param) :
+        remote_address(remote_address_param)
+    {}
+
+    ///
+    /// \name Parameter variables
+    ///
+    /// These member variables must be immutable so that the integrity of
+    /// the structure is kept throughout its lifetime.  The easiest way is
+    /// to declare the variable as const.  If it's not possible for a
+    /// particular variable, it must be defined as private and accessible
+    /// only via an accessor method.
+    //@{
+    /// \brief The remote IP address (eg. the client's IP address).
+    const IPAddress& remote_address;
+    //@}
 };
 
 /// \brief DNS based check.
-typedef acl::Check<RequestContext> Check;
+typedef acl::Check<RequestContext> RequestCheck;
 /// \brief DNS based compound check.
 typedef acl::CompoundCheck<RequestContext> CompoundCheck;
 /// \brief DNS based ACL.
-typedef acl::ACL<RequestContext> ACL;
+typedef acl::ACL<RequestContext> RequestACL;
 /// \brief DNS based ACL loader.
-typedef acl::Loader<RequestContext> Loader;
+typedef acl::Loader<RequestContext> RequestLoader;
 
 /**
  * \brief Loader singleton access function.
@@ -80,10 +103,38 @@ typedef acl::Loader<RequestContext> Loader;
  * one is enough, this one will have registered default checks and it
  * is known one, so any plugins can registrer additional checks as well.
  */
-Loader& getLoader();
+RequestLoader& getRequestLoader();
+
+// The following is essentially private to the implementation and could
+// be hidden in the implementation file.  But it's visible via this header
+// file for testing purposes.  They are not supposed to be used by normal
+// applications directly, and to signal the intent, they are given inside
+// a separate namespace.
+namespace internal {
+
+// Shortcut typedef
+typedef isc::acl::IPCheck<RequestContext> RequestIPCheck;
 
-}
-}
-}
+class RequestCheckCreator : public acl::Loader<RequestContext>::CheckCreator {
+public:
+    virtual std::vector<std::string> names() const;
+
+    virtual boost::shared_ptr<RequestCheck>
+    create(const std::string& name, isc::data::ConstElementPtr definition,
+           const acl::Loader<RequestContext>& loader);
+
+    /// Until we are sure how the various rules work for this case, we won't
+    /// allow unexpected special interpretation for list definitions.
+    virtual bool allowListAbbreviation() const { return (false); }
+};
+} // end of namespace "internal"
+
+} // end of namespace "dns"
+} // end of namespace "acl"
+} // end of namespace "isc"
 
 #endif
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/acl/loader.h

@@ -15,7 +15,8 @@
 #ifndef ACL_LOADER_H
 #define ACL_LOADER_H
 
-#include "acl.h"
+#include <exceptions/exceptions.h>
+#include <acl/acl.h>
 #include <cc/data.h>
 #include <boost/function.hpp>
 #include <boost/shared_ptr.hpp>
@@ -297,16 +298,28 @@ public:
      * \brief Load an ACL.
      *
      * This parses an ACL list, creates the checks and actions of each element
-     * and returns it. It may throw LoaderError if it isn't a list or the
-     * "action" key is missing in some element. Also, no exceptions from
-     * loadCheck (therefore from whatever creator is used) and from the
-     * actionLoader passed to constructor are not caught.
+     * and returns it.
+     *
+     * No exceptions from \c loadCheck (therefore from whatever creator is
+     * used) and from the actionLoader passed to constructor are caught.
+     *
+     * \exception InvalidParameter The given element is NULL (most likely a
+     * caller's bug)
+     * \exception LoaderError The given element isn't a list or the
+     * "action" key is missing in some element
      *
      * \param description The JSON list of ACL.
+     *
+     * \return The newly created ACL object
      */
     boost::shared_ptr<ACL<Context, Action> > load(const data::ConstElementPtr&
                                                   description) const
     {
+        if (!description) {
+            isc_throw(isc::InvalidParameter,
+                      "Null description is passed to ACL loader");
+        }
+
         // We first check it's a list, so we can use the list reference
         // (the list may be huge)
         if (description->getType() != data::Element::list) {
@@ -460,3 +473,7 @@ private:
 #include "logic_check.h"
 
 #endif
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/acl/logic_check.h

@@ -200,6 +200,86 @@ private:
     const std::string name_;
 };
 
+/**
+ * \brief The NOT operator for ACLs.
+ *
+ * This simply returns the negation of whatever returns the subexpression.
+ */
+template<typename Context>
+class NotOperator : public CompoundCheck<Context> {
+public:
+    /**
+     * \brief Constructor
+     *
+     * \param expr The subexpression to be negated by this NOT.
+     */
+    NotOperator(const boost::shared_ptr<Check<Context> >& expr) :
+        expr_(expr)
+    { }
+    /**
+     * \brief The list of subexpressions
+     *
+     * \return The vector will contain single value and it is the expression
+     *     passed by constructor.
+     */
+    virtual typename CompoundCheck<Context>::Checks getSubexpressions() const {
+        typename CompoundCheck<Context>::Checks result;
+        result.push_back(expr_.get());
+        return (result);
+    }
+    /// \brief The matching function
+    virtual bool matches(const Context& context) const {
+        return (!expr_->matches(context));
+    }
+private:
+    /// \brief The subexpression
+    const boost::shared_ptr<Check<Context> > expr_;
+};
+
+template<typename Context, typename Action = BasicAction>
+class NotCreator : public Loader<Context, Action>::CheckCreator {
+public:
+    /**
+     * \brief Constructor
+     *
+     * \param name The name of the NOT operator to be loaded as.
+     */
+    NotCreator(const std::string& name) :
+        name_(name)
+    { }
+    /**
+     * \brief List of the names this loads
+     *
+     * \return Single-value vector containing the name passed to the
+     *     constructor.
+     */
+    virtual std::vector<std::string> names() const {
+        std::vector<std::string> result;
+        result.push_back(name_);
+        return (result);
+    }
+    /// \brief Create the check.
+    virtual boost::shared_ptr<Check<Context> > create(const std::string&,
+                                                      data::ConstElementPtr
+                                                      definition,
+                                                      const Loader<Context,
+                                                      Action>& loader)
+    {
+        return (boost::shared_ptr<Check<Context> >(new NotOperator<Context>(
+                    loader.loadCheck(definition))));
+    }
+    /**
+     * \brief Or-abbreviated form.
+     *
+     * This returns false. In theory, the NOT operator could be used with
+     * the abbreviated form, but it would be confusing. Such syntax is
+     * therefore explicitly forbidden.
+     */
+    virtual bool allowListAbbreviation() const { return (false); }
+public:
+    const std::string name_;
+};
+
 }
 }
 

src/lib/acl/tests/Makefile.am

@@ -20,6 +20,7 @@ run_unittests_SOURCES += loader_test.cc
 run_unittests_SOURCES += logcheck.h
 run_unittests_SOURCES += creators.h
 run_unittests_SOURCES += logic_check_test.cc
+run_unittests_SOURCES += sockaddr.h
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)

src/lib/acl/tests/dns_test.cc

@@ -12,24 +12,194 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <stdint.h>
+
+#include <algorithm>
+#include <vector>
+#include <string>
+
+#include <boost/scoped_ptr.hpp>
+#include <boost/shared_ptr.hpp>
+
+#include <exceptions/exceptions.h>
+
+#include <cc/data.h>
 #include <acl/dns.h>
+#include <acl/loader.h>
+#include <acl/check.h>
+#include <acl/ip_check.h>
+
+#include "sockaddr.h"
+
 #include <gtest/gtest.h>
 
+using namespace std;
+using boost::scoped_ptr;
+using namespace isc::data;
+using namespace isc::acl;
 using namespace isc::acl::dns;
+using isc::acl::LoaderError;
 
 namespace {
 
-// Tests that the getLoader actually returns something, returns the same every
-// time and the returned value can be used to anything. It is not much of a
-// test, but the getLoader is not much of a function.
-TEST(DNSACL, getLoader) {
-    Loader* l(&getLoader());
+TEST(DNSACL, getRequestLoader) {
+    dns::RequestLoader* l(&getRequestLoader());
     ASSERT_TRUE(l != NULL);
-    EXPECT_EQ(l, &getLoader());
-    EXPECT_NO_THROW(l->load(isc::data::Element::fromJSON(
-        "[{\"action\": \"DROP\"}]")));
-    // TODO Test that the things we should register by default, like IP based
-    // check, are loaded.
+    EXPECT_EQ(l, &getRequestLoader());
+    EXPECT_NO_THROW(l->load(Element::fromJSON("[{\"action\": \"DROP\"}]")));
+
+    // Confirm it can load the ACl syntax acceptable to a default creator.
+    // Tests to see whether the loaded rules work correctly will be in
+    // other dedicated tests below.
+    EXPECT_NO_THROW(l->load(Element::fromJSON("[{\"action\": \"DROP\","
+                                              "  \"from\": \"192.0.2.1\"}]")));
+}
+
+class RequestCheckCreatorTest : public ::testing::Test {
+protected:
+    dns::internal::RequestCheckCreator creator_;
+
+    typedef boost::shared_ptr<const dns::RequestCheck> ConstRequestCheckPtr;
+    ConstRequestCheckPtr check_;
+};
+
+TEST_F(RequestCheckCreatorTest, names) {
+    ASSERT_EQ(1, creator_.names().size());
+    EXPECT_EQ("from", creator_.names()[0]);
+}
+
+TEST_F(RequestCheckCreatorTest, allowListAbbreviation) {
+    EXPECT_FALSE(creator_.allowListAbbreviation());
+}
+
+// The following two tests check the creator for the form of
+// 'from: "IP prefix"'.  We don't test many variants of prefixes, which
+// are done in the tests for IPCheck.
+TEST_F(RequestCheckCreatorTest, createIPv4Check) {
+    check_ = creator_.create("from", Element::fromJSON("\"192.0.2.1\""),
+                             getRequestLoader());
+    const dns::internal::RequestIPCheck& ipcheck_ =
+        dynamic_cast<const dns::internal::RequestIPCheck&>(*check_);
+    EXPECT_EQ(AF_INET, ipcheck_.getFamily());
+    EXPECT_EQ(32, ipcheck_.getPrefixlen());
+    const vector<uint8_t> check_address(ipcheck_.getAddress());
+    ASSERT_EQ(4, check_address.size());
+    const uint8_t expected_address[] = { 192, 0, 2, 1 };
+    EXPECT_TRUE(equal(check_address.begin(), check_address.end(),
+                      expected_address));
+}
+
+TEST_F(RequestCheckCreatorTest, createIPv6Check) {
+    check_ = creator_.create("from",
+                             Element::fromJSON("\"2001:db8::5300/120\""),
+                             getRequestLoader());
+    const dns::internal::RequestIPCheck& ipcheck_ =
+        dynamic_cast<const dns::internal::RequestIPCheck&>(*check_);
+    EXPECT_EQ(AF_INET6, ipcheck_.getFamily());
+    EXPECT_EQ(120, ipcheck_.getPrefixlen());
+    const vector<uint8_t> check_address(ipcheck_.getAddress());
+    ASSERT_EQ(16, check_address.size());
+    const uint8_t expected_address[] = { 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00,
+                                         0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                                         0x00, 0x00, 0x53, 0x00 };
+    EXPECT_TRUE(equal(check_address.begin(), check_address.end(),
+                      expected_address));
+}
+
+TEST_F(RequestCheckCreatorTest, badCreate) {
+    // Invalid name
+    EXPECT_THROW(creator_.create("bad", Element::fromJSON("\"192.0.2.1\""),
+                                 getRequestLoader()), LoaderError);
+
+    // Invalid type of parameter
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("4"),
+                                 getRequestLoader()),
+                 isc::data::TypeError);
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("[]"),
+                                 getRequestLoader()),
+                 isc::data::TypeError);
+
+    // Syntax error for IPCheck
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("\"bad\""),
+                                 getRequestLoader()),
+                 isc::InvalidParameter);
+
+    // NULL pointer
+    EXPECT_THROW(creator_.create("from", ConstElementPtr(), getRequestLoader()),
+                 LoaderError);
+}
+
+class RequestCheckTest : public ::testing::Test {
+protected:
+    typedef boost::shared_ptr<const dns::RequestCheck> ConstRequestCheckPtr;
+
+    // A helper shortcut to create a single IP check for the given prefix.
+    ConstRequestCheckPtr createIPCheck(const string& prefix) {
+        return (creator_.create("from", Element::fromJSON(
+                                    string("\"") + prefix + string("\"")),
+                                getRequestLoader()));
+    }
+
+    // create a one time request context for a specific test.  Note that
+    // getSockaddr() uses a static storage, so it cannot be called more than
+    // once in a single test.
+    const dns::RequestContext& getRequest4() {
+        ipaddr.reset(new IPAddress(tests::getSockAddr("192.0.2.1")));
+        request.reset(new dns::RequestContext(*ipaddr));
+        return (*request);
+    }
+    const dns::RequestContext& getRequest6() {
+        ipaddr.reset(new IPAddress(tests::getSockAddr("2001:db8::1")));
+        request.reset(new dns::RequestContext(*ipaddr));
+        return (*request);
+    }
+
+private:
+    scoped_ptr<IPAddress> ipaddr;
+    scoped_ptr<dns::RequestContext> request;
+    dns::internal::RequestCheckCreator creator_;
+};
+
+TEST_F(RequestCheckTest, checkIPv4) {
+    // Exact match
+    EXPECT_TRUE(createIPCheck("192.0.2.1")->matches(getRequest4()));
+    // Exact match (negative)
+    EXPECT_FALSE(createIPCheck("192.0.2.53")->matches(getRequest4()));
+    // Prefix match
+    EXPECT_TRUE(createIPCheck("192.0.2.0/24")->matches(getRequest4()));
+    // Prefix match (negative)
+    EXPECT_FALSE(createIPCheck("192.0.1.0/24")->matches(getRequest4()));
+    // Address family mismatch (the first 4 bytes of the IPv6 address has the
+    // same binary representation as the client's IPv4 address, which
+    // shouldn't confuse the match logic)
+    EXPECT_FALSE(createIPCheck("c000:0201::")->matches(getRequest4()));
+}
+
+TEST_F(RequestCheckTest, checkIPv6) {
+    // The following are a set of tests of the same concept as checkIPv4
+    EXPECT_TRUE(createIPCheck("2001:db8::1")->matches(getRequest6()));
+    EXPECT_FALSE(createIPCheck("2001:db8::53")->matches(getRequest6()));
+    EXPECT_TRUE(createIPCheck("2001:db8::/64")->matches(getRequest6()));
+    EXPECT_FALSE(createIPCheck("2001:db8:1::/64")->matches(getRequest6()));
+    EXPECT_FALSE(createIPCheck("32.1.13.184")->matches(getRequest6()));
+}
+
+// The following tests test only the creators are registered, they are tested
+// elsewhere
+
+TEST(DNSACL, notLoad) {
+    EXPECT_NO_THROW(getRequestLoader().loadCheck(isc::data::Element::fromJSON(
+        "{\"NOT\": {\"from\": \"192.0.2.1\"}}")));
+}
+
+TEST(DNSACL, allLoad) {
+    EXPECT_NO_THROW(getRequestLoader().loadCheck(isc::data::Element::fromJSON(
+        "{\"ALL\": [{\"from\": \"192.0.2.1\"}]}")));
+}
+
+TEST(DNSACL, anyLoad) {
+    EXPECT_NO_THROW(getRequestLoader().loadCheck(isc::data::Element::fromJSON(
+        "{\"ANY\": [{\"from\": \"192.0.2.1\"}]}")));
 }
 
 }

src/lib/acl/tests/ip_check_unittest.cc

@@ -14,12 +14,13 @@
 
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <netdb.h>
 #include <string.h>
 
 #include <gtest/gtest.h>
 #include <acl/ip_check.h>
 
+#include "sockaddr.h"
+
 using namespace isc::acl;
 using namespace isc::acl::internal;
 using namespace std;
@@ -159,32 +160,8 @@ TEST(IPFunctionCheck, SplitIPAddress) {
     EXPECT_THROW(splitIPAddress(" 1/ "), isc::InvalidParameter);
 }
 
-const struct sockaddr&
-getSockAddr(const char* const addr) {
-    struct addrinfo hints, *res;
-    memset(&hints, 0, sizeof(hints));
-    hints.ai_family = AF_UNSPEC;
-    hints.ai_socktype = SOCK_STREAM;
-    hints.ai_flags = AI_NUMERICHOST;
-
-    if (getaddrinfo(addr, NULL, &hints, &res) == 0) {
-        static struct sockaddr_storage ss;
-        void* ss_ptr = &ss;
-        memcpy(ss_ptr, res->ai_addr, res->ai_addrlen);
-        freeaddrinfo(res);
-        return (*static_cast<struct sockaddr*>(ss_ptr));
-    }
-
-    // We don't expect getaddrinfo to fail for our tests.  But if that
-    // ever happens we return a dummy value that would make subsequent test
-    // fail.
-    static struct sockaddr sa_dummy;
-    sa_dummy.sa_family = AF_UNSPEC;
-    return (sa_dummy);
-}
-
 TEST(IPAddress, constructIPv4) {
-    IPAddress ipaddr(getSockAddr("192.0.2.1"));
+    IPAddress ipaddr(tests::getSockAddr("192.0.2.1"));
     const char expected_data[4] = { 192, 0, 2, 1 };
     EXPECT_EQ(AF_INET, ipaddr.getFamily());
     EXPECT_EQ(4, ipaddr.getLength());
@@ -192,7 +169,7 @@ TEST(IPAddress, constructIPv4) {
 }
 
 TEST(IPAddress, constructIPv6) {
-    IPAddress ipaddr(getSockAddr("2001:db8:1234:abcd::53"));
+    IPAddress ipaddr(tests::getSockAddr("2001:db8:1234:abcd::53"));
     const char expected_data[16] = { 0x20, 0x01, 0x0d, 0xb8, 0x12, 0x34, 0xab,
                                      0xcd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                                      0x00, 0x53 };

src/lib/acl/tests/loader_test.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include "creators.h"
+#include <exceptions/exceptions.h>
 #include <acl/loader.h>
 #include <string>
 #include <gtest/gtest.h>
@@ -373,7 +374,10 @@ TEST_F(LoaderTest, ACLPropagate) {
                      Element::fromJSON(
                          "[{\"action\": \"ACCEPT\", \"throw\": 1}]")),
                  TestCreatorError);
+}
 
+TEST_F(LoaderTest, nullDescription) {
+    EXPECT_THROW(loader_.load(ConstElementPtr()), isc::InvalidParameter);
 }
 
 }

src/lib/acl/tests/logic_check_test.cc

@@ -93,6 +93,7 @@ public:
             LogicCreator<AllOfSpec, Log>("ALL")));
         loader_.registerCreator(CreatorPtr(new ThrowCreator));
         loader_.registerCreator(CreatorPtr(new LogCreator));
+        loader_.registerCreator(CreatorPtr(new NotCreator<Log>("NOT")));
     }
     // To mark which parts of the check did run
     Log log_;
@@ -242,4 +243,49 @@ TEST_F(LogicCreatorTest, nested) {
     log_.checkFirst(2);
 }
 
+void notTest(bool value) {
+    NotOperator<Log> notOp(shared_ptr<Check<Log> >(new ConstCheck(value, 0)));
+    Log log;
+    // It returns negated value
+    EXPECT_EQ(!value, notOp.matches(log));
+    // And runs the only one thing there
+    log.checkFirst(1);
+    // Check the getSubexpressions does sane things
+    ASSERT_EQ(1, notOp.getSubexpressions().size());
+    EXPECT_EQ(value, notOp.getSubexpressions()[0]->matches(log));
+}
+
+TEST(Not, trueValue) {
+    notTest(true);
+}
+
+TEST(Not, falseValue) {
+    notTest(false);
+}
+
+TEST_F(LogicCreatorTest, notInvalid) {
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"NOT\": null}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"NOT\": \"hello\"}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"NOT\": true}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"NOT\": 42}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"NOT\": []}")),
+                 LoaderError);
+    EXPECT_THROW(loader_.loadCheck(Element::fromJSON("{\"NOT\": [{"
+                                                     "\"logcheck\": [0, true]"
+                                                     "}]}")),
+                 LoaderError);
+}
+
+TEST_F(LogicCreatorTest, notValid) {
+    shared_ptr<NotOperator<Log> > notOp(load<NotOperator<Log> >("{\"NOT\":"
+                                                                "  {\"logcheck\":"
+                                                                "     [0, true]}}"));
+    EXPECT_FALSE(notOp->matches(log_));
+    log_.checkFirst(1);
+}
+
 }

src/lib/acl/tests/sockaddr.h

@@ -0,0 +1,69 @@
+// Copyright (C) 2011  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 __ACL_TEST_SOCKADDR_H
+#define __ACL_TEST_SOCKADDR_H 1
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <string.h>
+
+#include <exceptions/exceptions.h>
+
+namespace isc {
+namespace acl {
+namespace tests {
+
+// This is a helper function that returns a sockaddr for the given textual
+// IP address.  Note that "inline" is crucial because this function is defined
+// in a header file included in multiple .cc files.  Without inline it would
+// produce an external linkage and cause troubles at link time.
+//
+// Note that this function uses a static storage for the return value.
+// So if it's called more than once in a singe context (e.g., in the same
+// EXPECT_xx()), it's unlikely to work as expected.
+inline const struct sockaddr&
+getSockAddr(const char* const addr) {
+    struct addrinfo hints, *res;
+    memset(&hints, 0, sizeof(hints));
+    hints.ai_family = AF_UNSPEC;
+    hints.ai_socktype = SOCK_STREAM;
+    hints.ai_flags = AI_NUMERICHOST;
+
+    if (getaddrinfo(addr, NULL, &hints, &res) == 0) {
+        static struct sockaddr_storage ss;
+        void* ss_ptr = &ss;
+        memcpy(ss_ptr, res->ai_addr, res->ai_addrlen);
+        freeaddrinfo(res);
+        return (*static_cast<struct sockaddr*>(ss_ptr));
+    }
+
+    // We don't expect getaddrinfo to fail for our tests.  But if that
+    // ever happens we throw an exception to make sure the corresponding test
+    // fail (either due to a failure of *_NO_THROW or the uncaught exception).
+    isc_throw(Unexpected,
+              "failed to convert textual IP address to sockaddr for " <<
+              addr);
+}
+
+} // end of namespace "tests"
+} // end of namespace "acl"
+} // end of namespace "isc"
+
+#endif  // __ACL_TEST_SOCKADDR_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/cache/Makefile.am

@@ -31,5 +31,14 @@ libcache_la_SOURCES  += cache_entry_key.h cache_entry_key.cc
 libcache_la_SOURCES  += rrset_copy.h rrset_copy.cc
 libcache_la_SOURCES  += local_zone_data.h local_zone_data.cc
 libcache_la_SOURCES  += message_utility.h message_utility.cc
+libcache_la_SOURCES  += logger.h logger.cc
+nodist_libcache_la_SOURCES = cache_messages.cc cache_messages.h
 
-CLEANFILES = *.gcno *.gcda
+BUILT_SOURCES = cache_messages.cc cache_messages.h
+
+cache_messages.cc cache_messages.h: cache_messages.mes
+	$(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/cache/cache_messages.mes
+
+CLEANFILES = *.gcno *.gcda cache_messages.cc cache_messages.h
+
+EXTRA_DIST = cache_messages.mes

src/lib/cache/cache_messages.mes

@@ -0,0 +1,148 @@
+# Copyright (C) 2010  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.
+
+$NAMESPACE isc::cache
+
+% CACHE_ENTRY_MISSING_RRSET missing RRset to generate message for %1
+The cache tried to generate the complete answer message. It knows the structure
+of the message, but some of the RRsets to be put there are not in cache (they
+probably expired already). Therefore it pretends the message was not found.
+
+% CACHE_LOCALZONE_FOUND found entry with key %1 in local zone data
+Debug message, noting that the requested data was successfully found in the
+local zone data of the cache.
+
+% CACHE_LOCALZONE_UNKNOWN entry with key %1 not found in local zone data
+Debug message. The requested data was not found in the local zone data.
+
+% CACHE_LOCALZONE_UPDATE updating local zone element at key %1
+Debug message issued when there's update to the local zone section of cache.
+
+% CACHE_MESSAGES_DEINIT deinitialized message cache
+Debug message. It is issued when the server deinitializes the message cache.
+
+% CACHE_MESSAGES_EXPIRED found an expired message entry for %1 in the message cache
+Debug message. The requested data was found in the message cache, but it
+already expired. Therefore the cache removes the entry and pretends it found
+nothing.
+
+% CACHE_MESSAGES_FOUND found a message entry for %1 in the message cache
+Debug message. We found the whole message in the cache, so it can be returned
+to user without any other lookups.
+
+% CACHE_MESSAGES_INIT initialized message cache for %1 messages of class %2
+Debug message issued when a new message cache is issued. It lists the class
+of messages it can hold and the maximum size of the cache.
+
+% CACHE_MESSAGES_REMOVE removing old instance of %1/%2/%3 first
+Debug message. This may follow CACHE_MESSAGES_UPDATE and indicates that, while
+updating, the old instance is being removed prior of inserting a new one.
+
+% CACHE_MESSAGES_UNCACHEABLE not inserting uncacheable message %1/%2/%3
+Debug message, noting that the given message can not be cached. This is because
+there's no SOA record in the message. See RFC 2308 section 5 for more
+information.
+
+% CACHE_MESSAGES_UNKNOWN no entry for %1 found in the message cache
+Debug message. The message cache didn't find any entry for the given key.
+
+% CACHE_MESSAGES_UPDATE updating message entry %1/%2/%3
+Debug message issued when the message cache is being updated with a new
+message. Either the old instance is removed or, if none is found, new one
+is created.
+
+% CACHE_RESOLVER_DEEPEST looking up deepest NS for %1/%2
+Debug message. The resolver cache is looking up the deepest known nameserver,
+so the resolution doesn't have to start from the root.
+
+% CACHE_RESOLVER_INIT_INFO initializing resolver cache for class %1
+Debug message, the resolver cache is being created for this given class. The
+difference from CACHE_RESOLVER_INIT is only in different format of passed
+information, otherwise it does the same.
+
+% CACHE_RESOLVER_INIT initializing resolver cache for class %1
+Debug message. The resolver cache is being created for this given class.
+
+% CACHE_RESOLVER_LOCAL_MSG message for %1/%2 found in local zone data
+Debug message. The resolver cache found a complete message for the user query
+in the zone data.
+
+% CACHE_RESOLVER_LOCAL_RRSET RRset for %1/%2 found in local zone data
+Debug message. The resolver cache found a requested RRset in the local zone
+data.
+
+% CACHE_RESOLVER_LOOKUP_MSG looking up message in resolver cache for %1/%2
+Debug message. The resolver cache is trying to find a message to answer the
+user query.
+
+% CACHE_RESOLVER_LOOKUP_RRSET looking up RRset in resolver cache for %1/%2
+Debug message. The resolver cache is trying to find an RRset (which usually
+originates as internally from resolver).
+
+% CACHE_RESOLVER_NO_QUESTION answer message for %1/%2 has empty question section
+The cache tried to fill in found data into the response message. But it
+discovered the message contains no question section, which is invalid.
+This is likely a programmer error, please submit a bug report.
+
+% CACHE_RESOLVER_UNKNOWN_CLASS_MSG no cache for class %1
+Debug message. While trying to lookup a message in the resolver cache, it was
+discovered there's no cache for this class at all. Therefore no message is
+found.
+
+% CACHE_RESOLVER_UNKNOWN_CLASS_RRSET no cache for class %1
+Debug message. While trying to lookup an RRset in the resolver cache, it was
+discovered there's no cache for this class at all. Therefore no data is found.
+
+% CACHE_RESOLVER_UPDATE_MSG updating message for %1/%2/%3
+Debug message. The resolver is updating a message in the cache.
+
+% CACHE_RESOLVER_UPDATE_RRSET updating RRset for %1/%2/%3
+Debug message. The resolver is updating an RRset in the cache.
+
+% CACHE_RESOLVER_UPDATE_UNKNOWN_CLASS_MSG no cache for class %1
+Debug message. While trying to insert a message into the cache, it was
+discovered that there's no cache for the class of message. Therefore
+the message will not be cached.
+
+% CACHE_RESOLVER_UPDATE_UNKNOWN_CLASS_RRSET no cache for class %1
+Debug message. While trying to insert an RRset into the cache, it was
+discovered that there's no cache for the class of the RRset. Therefore
+the message will not be cached.
+
+% CACHE_RRSET_EXPIRED found expired RRset %1/%2/%3
+Debug message. The requested data was found in the RRset cache. However, it is
+expired, so the cache removed it and is going to pretend nothing was found.
+
+% CACHE_RRSET_INIT initializing RRset cache for %2 RRsets of class %1
+Debug message. The RRset cache to hold at most this many RRsets for the given
+class is being created.
+
+% CACHE_RRSET_LOOKUP looking up %1/%2/%3 in RRset cache
+Debug message. The resolver is trying to look up data in the RRset cache.
+
+% CACHE_RRSET_NOT_FOUND no RRset found for %1/%2/%3
+Debug message which can follow CACHE_RRSET_LOOKUP. This means the data is not
+in the cache.
+
+% CACHE_RRSET_REMOVE_OLD removing old RRset for %1/%2/%3 to make space for new one
+Debug message which can follow CACHE_RRSET_UPDATE. During the update, the cache
+removed an old instance of the RRset to replace it with the new one.
+
+% CACHE_RRSET_UNTRUSTED not replacing old RRset for %1/%2/%3, it has higher trust level
+Debug message which can follow CACHE_RRSET_UPDATE. The cache already holds the
+same RRset, but from more trusted source, so the old one is kept and new one
+ignored.
+
+% CACHE_RRSET_UPDATE updating RRset %1/%2/%3 in the cache
+Debug message. The RRset is updating its data with this given RRset.

src/lib/cache/local_zone_data.cc

@@ -16,6 +16,7 @@
 #include "local_zone_data.h"
 #include "cache_entry_key.h"
 #include "rrset_copy.h"
+#include "logger.h"
 
 using namespace std;
 using namespace isc::dns;
@@ -33,8 +34,10 @@ LocalZoneData::lookup(const isc::dns::Name& name,
     string key = genCacheEntryName(name, type);
     RRsetMapIterator iter = rrsets_map_.find(key);
     if (iter == rrsets_map_.end()) {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_LOCALZONE_UNKNOWN).arg(key);
         return (RRsetPtr());
     } else {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_LOCALZONE_FOUND).arg(key);
         return (iter->second);
     }
 }
@@ -43,6 +46,7 @@ void
 LocalZoneData::update(const isc::dns::RRset& rrset) {
     //TODO Do we really need to recreate the rrset again?
     string key = genCacheEntryName(rrset.getName(), rrset.getType());
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_LOCALZONE_UPDATE).arg(key);
     RRset* rrset_copy = new RRset(rrset.getName(), rrset.getClass(),
                                   rrset.getType(), rrset.getTTL());
 

src/lib/cache/logger.cc

@@ -0,0 +1,23 @@
+// Copyright (C) 2011  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 <cache/logger.h>
+
+namespace isc {
+namespace cache {
+
+isc::log::Logger logger("cache");
+
+}
+}

src/lib/cache/logger.h

@@ -0,0 +1,44 @@
+// Copyright (C) 2011  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_LOGGER_H
+#define __DATASRC_LOGGER_H
+
+#include <log/macros.h>
+#include <cache/cache_messages.h>
+
+/// \file logger.h
+/// \brief Cache library global logger
+///
+/// This holds the logger for the cache library. It is a private header
+/// and should not be included in any publicly used header, only in local
+/// cc files.
+
+namespace isc {
+namespace cache {
+
+/// \brief The logger for this library
+extern isc::log::Logger logger;
+
+enum {
+    /// \brief Trace basic operations
+    DBG_TRACE_BASIC = 10,
+    /// \brief Trace data operations
+    DBG_TRACE_DATA = 40,
+};
+
+}
+}
+
+#endif

src/lib/cache/message_cache.cc

@@ -1,6 +1,7 @@
 // Copyright (C) 2010  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.
 //
@@ -20,6 +21,7 @@
 #include "message_cache.h"
 #include "message_utility.h"
 #include "cache_entry_key.h"
+#include "logger.h"
 
 namespace isc {
 namespace cache {
@@ -39,11 +41,14 @@ MessageCache::MessageCache(const RRsetCachePtr& rrset_cache,
     message_lru_((3 * cache_size),
                   new HashDeleter<MessageEntry>(message_table_))
 {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, CACHE_MESSAGES_INIT).arg(cache_size).
+        arg(RRClass(message_class));
 }
 
 MessageCache::~MessageCache() {
     // Destroy all the message entries in the cache.
     message_lru_.clear();
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, CACHE_MESSAGES_DEINIT);
 }
 
 bool
@@ -57,26 +62,38 @@ MessageCache::lookup(const isc::dns::Name& qname,
     if(msg_entry) {
         // Check whether the message entry has expired.
        if (msg_entry->getExpireTime() > time(NULL)) {
+            LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_MESSAGES_FOUND).
+                arg(entry_name);
             message_lru_.touch(msg_entry);
             return (msg_entry->genMessage(time(NULL), response));
         } else {
             // message entry expires, remove it from hash table and lru list.
+            LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_MESSAGES_EXPIRED).
+                arg(entry_name);
             message_table_.remove(entry_key);
             message_lru_.remove(msg_entry);
             return (false);
        }
     }
 
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_MESSAGES_UNKNOWN).arg(entry_name);
     return (false);
 }
 
 bool
 MessageCache::update(const Message& msg) {
     if (!canMessageBeCached(msg)){
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_MESSAGES_UNCACHEABLE).
+            arg((*msg.beginQuestion())->getName()).
+            arg((*msg.beginQuestion())->getType()).
+            arg((*msg.beginQuestion())->getClass());
         return (false);
     }
 
     QuestionIterator iter = msg.beginQuestion();
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_MESSAGES_UPDATE).
+        arg((*iter)->getName()).arg((*iter)->getType()).
+        arg((*iter)->getClass());
     std::string entry_name = genCacheEntryName((*iter)->getName(),
                                                (*iter)->getType());
     HashKey entry_key = HashKey(entry_name, RRClass(message_class_));
@@ -88,6 +105,9 @@ MessageCache::update(const Message& msg) {
     // add the message entry, maybe there is one way to touch it once.
     MessageEntryPtr old_msg_entry = message_table_.get(entry_key);
     if (old_msg_entry) {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_MESSAGES_REMOVE).
+            arg((*iter)->getName()).arg((*iter)->getType()).
+            arg((*iter)->getClass());
         message_lru_.remove(old_msg_entry);
     }
 

src/lib/cache/message_cache.h

@@ -39,7 +39,7 @@ private:
     MessageCache& operator=(const MessageCache& source);
 public:
     /// \param rrset_cache The cache that stores the RRsets that the
-    ///        message entry will points to
+    ///        message entry will point to
     /// \param cache_size The size of message cache.
     /// \param message_class The class of the message cache
     /// \param negative_soa_cache The cache that stores the SOA record

src/lib/cache/message_entry.cc

@@ -20,6 +20,7 @@
 #include "message_entry.h"
 #include "message_utility.h"
 #include "rrset_cache.h"
+#include "logger.h"
 
 using namespace isc::dns;
 using namespace std;
@@ -64,7 +65,7 @@ static uint32_t MAX_UINT32 = numeric_limits<uint32_t>::max();
 // tunable.  Values of one to three hours have been found to work well
 // and would make sensible a default.  Values exceeding one day have
 // been found to be problematic. (sec 5, RFC2308)
-// The default value is 3 hourse (10800 seconds)
+// The default value is 3 hours (10800 seconds)
 // TODO:Give an option to let user configure
 static uint32_t MAX_NEGATIVE_CACHE_TTL = 10800;
 
@@ -142,6 +143,8 @@ MessageEntry::genMessage(const time_t& time_now,
         // has expired, if it is, return false.
         vector<RRsetEntryPtr> rrset_entry_vec;
         if (false == getRRsetEntries(rrset_entry_vec, time_now)) {
+            LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_ENTRY_MISSING_RRSET).
+                arg(entry_name_);
             return (false);
         }
 

src/lib/cache/resolver_cache.cc

@@ -17,6 +17,7 @@
 #include "resolver_cache.h"
 #include "dns/message.h"
 #include "rrset_cache.h"
+#include "logger.h"
 #include <string>
 #include <algorithm>
 
@@ -29,6 +30,7 @@ namespace cache {
 ResolverClassCache::ResolverClassCache(const RRClass& cache_class) :
     cache_class_(cache_class)
 {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, CACHE_RESOLVER_INIT).arg(cache_class);
     local_zone_data_ = LocalZoneDataPtr(new LocalZoneData(cache_class_.getCode()));
     rrsets_cache_ = RRsetCachePtr(new RRsetCache(RRSET_CACHE_DEFAULT_SIZE,
                                                  cache_class_.getCode()));
@@ -45,6 +47,8 @@ ResolverClassCache::ResolverClassCache(const RRClass& cache_class) :
 ResolverClassCache::ResolverClassCache(const CacheSizeInfo& cache_info) :
     cache_class_(cache_info.cclass)
 {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, CACHE_RESOLVER_INIT_INFO).
+        arg(cache_class_);
     uint16_t klass = cache_class_.getCode();
     // TODO We should find one way to load local zone data.
     local_zone_data_ = LocalZoneDataPtr(new LocalZoneData(klass));
@@ -69,8 +73,11 @@ ResolverClassCache::lookup(const isc::dns::Name& qname,
                       const isc::dns::RRType& qtype,
                       isc::dns::Message& response) const
 {
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_LOOKUP_MSG).
+        arg(qname).arg(qtype);
     // message response should has question section already.
     if (response.beginQuestion() == response.endQuestion()) {
+        LOG_ERROR(logger, CACHE_RESOLVER_NO_QUESTION).arg(qname).arg(qtype);
         isc_throw(MessageNoQuestionSection, "Message has no question section");
     }
 
@@ -79,6 +86,8 @@ ResolverClassCache::lookup(const isc::dns::Name& qname,
     // answer section.
     RRsetPtr rrset_ptr = local_zone_data_->lookup(qname, qtype);
     if (rrset_ptr) {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_LOCAL_MSG).
+            arg(qname).arg(qtype);
         response.addRRset(Message::SECTION_ANSWER, rrset_ptr);
         return (true);
     }
@@ -91,11 +100,15 @@ isc::dns::RRsetPtr
 ResolverClassCache::lookup(const isc::dns::Name& qname,
                const isc::dns::RRType& qtype) const
 {
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_LOOKUP_RRSET).
+        arg(qname).arg(qtype);
     // Algorithm:
     // 1. Search in local zone data first,
     // 2. Then do search in rrsets_cache_.
     RRsetPtr rrset_ptr = local_zone_data_->lookup(qname, qtype);
     if (rrset_ptr) {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_LOCAL_RRSET).
+            arg(qname).arg(qtype);
         return (rrset_ptr);
     } else {
         RRsetEntryPtr rrset_entry = rrsets_cache_->lookup(qname, qtype);
@@ -109,6 +122,10 @@ ResolverClassCache::lookup(const isc::dns::Name& qname,
 
 bool
 ResolverClassCache::update(const isc::dns::Message& msg) {
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_UPDATE_MSG).
+        arg((*msg.beginQuestion())->getName()).
+        arg((*msg.beginQuestion())->getType()).
+        arg((*msg.beginQuestion())->getClass());
     return (messages_cache_->update(msg));
 }
 
@@ -130,6 +147,9 @@ ResolverClassCache::updateRRsetCache(const isc::dns::ConstRRsetPtr& rrset_ptr,
 
 bool
 ResolverClassCache::update(const isc::dns::ConstRRsetPtr& rrset_ptr) {
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_UPDATE_RRSET).
+        arg(rrset_ptr->getName()).arg(rrset_ptr->getType()).
+        arg(rrset_ptr->getClass());
     // First update local zone, then update rrset cache.
     local_zone_data_->update((*rrset_ptr.get()));
     updateRRsetCache(rrset_ptr, rrsets_cache_);
@@ -166,6 +186,8 @@ ResolverCache::lookup(const isc::dns::Name& qname,
     if (cc) {
         return (cc->lookup(qname, qtype, response));
     } else {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_UNKNOWN_CLASS_MSG).
+            arg(qclass);
         return (false);
     }
 }
@@ -179,6 +201,8 @@ ResolverCache::lookup(const isc::dns::Name& qname,
     if (cc) {
         return (cc->lookup(qname, qtype));
     } else {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_UNKNOWN_CLASS_RRSET).
+            arg(qclass);
         return (RRsetPtr());
     }
 }
@@ -187,6 +211,8 @@ isc::dns::RRsetPtr
 ResolverCache::lookupDeepestNS(const isc::dns::Name& qname,
                                const isc::dns::RRClass& qclass) const
 {
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RESOLVER_DEEPEST).arg(qname).
+        arg(qclass);
     isc::dns::RRType qtype = RRType::NS();
     ResolverClassCache* cc = getClassCache(qclass);
     if (cc) {
@@ -213,6 +239,9 @@ ResolverCache::update(const isc::dns::Message& msg) {
     if (cc) {
         return (cc->update(msg));
     } else {
+        LOG_DEBUG(logger, DBG_TRACE_DATA,
+                  CACHE_RESOLVER_UPDATE_UNKNOWN_CLASS_MSG).
+            arg((*msg.beginQuestion())->getClass());
         return (false);
     }
 }
@@ -223,6 +252,9 @@ ResolverCache::update(const isc::dns::ConstRRsetPtr& rrset_ptr) {
     if (cc) {
         return (cc->update(rrset_ptr));
     } else {
+        LOG_DEBUG(logger, DBG_TRACE_DATA,
+                  CACHE_RESOLVER_UPDATE_UNKNOWN_CLASS_RRSET).
+            arg(rrset_ptr->getClass());
         return (false);
     }
 }

src/lib/cache/rrset_cache.cc

@@ -14,8 +14,9 @@
 
 #include <config.h>
 
-#include <string>
 #include "rrset_cache.h"
+#include "logger.h"
+#include <string>
 #include <nsas/nsas_entry_compare.h>
 #include <nsas/hash_table.h>
 #include <nsas/hash_deleter.h>
@@ -34,20 +35,28 @@ RRsetCache::RRsetCache(uint32_t cache_size,
     rrset_lru_((3 * cache_size),
                   new HashDeleter<RRsetEntry>(rrset_table_))
 {
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, CACHE_RRSET_INIT).arg(cache_size).
+        arg(RRClass(rrset_class));
 }
 
 RRsetEntryPtr
 RRsetCache::lookup(const isc::dns::Name& qname,
                    const isc::dns::RRType& qtype)
 {
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RRSET_LOOKUP).arg(qname).
+        arg(qtype).arg(RRClass(class_));
     const string entry_name = genCacheEntryName(qname, qtype);
-    RRsetEntryPtr entry_ptr = rrset_table_.get(HashKey(entry_name, RRClass(class_)));
+
+    RRsetEntryPtr entry_ptr = rrset_table_.get(HashKey(entry_name,
+                                                       RRClass(class_)));
     if (entry_ptr) {
         if (entry_ptr->getExpireTime() > time(NULL)) {
             // Only touch the non-expired rrset entries
             rrset_lru_.touch(entry_ptr);
             return (entry_ptr);
         } else {
+            LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RRSET_EXPIRED).arg(qname).
+                arg(qtype).arg(RRClass(class_));
             // the rrset entry has expired, so just remove it from
             // hash table and lru list.
             rrset_table_.remove(entry_ptr->hashKey());
@@ -55,19 +64,31 @@ RRsetCache::lookup(const isc::dns::Name& qname,
         }
     }
 
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RRSET_NOT_FOUND).arg(qname).
+        arg(qtype).arg(RRClass(class_));
     return (RRsetEntryPtr());
 }
 
 RRsetEntryPtr
-RRsetCache::update(const isc::dns::RRset& rrset, const RRsetTrustLevel& level) {
+RRsetCache::update(const isc::dns::RRset& rrset,
+                   const RRsetTrustLevel& level)
+{
+    LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RRSET_UPDATE).arg(rrset.getName()).
+        arg(rrset.getType()).arg(rrset.getClass());
     // TODO: If the RRset is an NS, we should update the NSAS as well
     // lookup first
     RRsetEntryPtr entry_ptr = lookup(rrset.getName(), rrset.getType());
     if (entry_ptr) {
         if (entry_ptr->getTrustLevel() > level) {
+            LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RRSET_UNTRUSTED).
+                arg(rrset.getName()).arg(rrset.getType()).
+                arg(rrset.getClass());
             // existed rrset entry is more authoritative, just return it
             return (entry_ptr);
         } else {
+            LOG_DEBUG(logger, DBG_TRACE_DATA, CACHE_RRSET_REMOVE_OLD).
+                arg(rrset.getName()).arg(rrset.getType()).
+                arg(rrset.getClass());
             // Remove the old rrset entry from the lru list.
             rrset_lru_.remove(entry_ptr);
         }

src/lib/cache/tests/run_unittests.cc

@@ -19,11 +19,15 @@
 
 #include <dns/tests/unittest_util.h>
 
+#include <log/logger_support.h>
+
 int
 main(int argc, char* argv[]) {
     ::testing::InitGoogleTest(&argc, argv);
     isc::UnitTestUtil::addDataPath(TEST_DATA_SRCDIR);
     isc::UnitTestUtil::addDataPath(TEST_DATA_BUILDDIR);
 
+    isc::log::initLogger();
+
     return (isc::util::unittests::run_all());
 }

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

@@ -23,6 +23,15 @@ log_la_LIBADD += $(PYTHON_LIB)
 # This is not installed, it helps locate the module during tests
 EXTRA_DIST = __init__.py
 
+# We're going to abuse install-data-local for a pre-install check.
+# This is to be considered a short term hack and is expected to be removed
+# in a near future version.
+install-data-local:
+	if test -d @pyexecdir@/isc/log; then \
+		echo "@pyexecdir@/isc/log is deprecated, and will confuse newer versions.  Please (re)move it by hand."; \
+		exit 1; \
+	fi
+
 pytest:
 	$(SHELL) tests/log_test
 

src/lib/server_common/client.cc

@@ -66,10 +66,3 @@ std::ostream&
 isc::server_common::operator<<(std::ostream& os, const Client& client) {
     return (os << client.toText());
 }
-
-template <>
-bool
-IPCheck<Client>::matches(const Client& client) const {
-    const IPAddress& request_src(client.getRequestSourceIPAddress());
-    return (compare(request_src.getData(), request_src.getFamily()));
-}

src/lib/server_common/client.h

@@ -145,17 +145,6 @@ private:
 /// parameter \c os after the insertion operation.
 std::ostream& operator<<(std::ostream& os, const Client& client);
 }
-
-namespace acl {
-/// The specialization of \c IPCheck for access control with \c Client.
-///
-/// It returns \c true if the source IP address of the client's request
-/// matches the expression encapsulated in the \c IPCheck, and returns
-/// \c false if not.
-template <>
-bool IPCheck<server_common::Client>::matches(
-    const server_common::Client& client) const;
-}
 }
 
 #endif  // __CLIENT_H

src/lib/server_common/tests/Makefile.am

@@ -41,6 +41,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD += $(top_builddir)/src/lib/acl/libacl.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la
+run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la

src/lib/server_common/tests/client_unittest.cc

@@ -89,30 +89,6 @@ TEST_F(ClientTest, constructIPv6) {
                         client6->getRequestSourceIPAddress().getData(), 16));
 }
 
-TEST_F(ClientTest, ACLCheckIPv4) {
-    // Exact match
-    EXPECT_TRUE(IPCheck<Client>("192.0.2.1").matches(*client4));
-    // Exact match (negative)
-    EXPECT_FALSE(IPCheck<Client>("192.0.2.53").matches(*client4));
-    // Prefix match
-    EXPECT_TRUE(IPCheck<Client>("192.0.2.0/24").matches(*client4));
-    // Prefix match (negative)
-    EXPECT_FALSE(IPCheck<Client>("192.0.1.0/24").matches(*client4));
-    // Address family mismatch (the first 4 bytes of the IPv6 address has the
-    // same binary representation as the client's IPv4 address, which
-    // shouldn't confuse the match logic)
-    EXPECT_FALSE(IPCheck<Client>("c000:0201::").matches(*client4));
-}
-
-TEST_F(ClientTest, ACLCheckIPv6) {
-    // The following are a set of tests of the same concept as ACLCheckIPv4
-    EXPECT_TRUE(IPCheck<Client>("2001:db8::1").matches(*client6));
-    EXPECT_FALSE(IPCheck<Client>("2001:db8::53").matches(*client6));
-    EXPECT_TRUE(IPCheck<Client>("2001:db8::/64").matches(*client6));
-    EXPECT_FALSE(IPCheck<Client>("2001:db8:1::/64").matches(*client6));
-    EXPECT_FALSE(IPCheck<Client>("32.1.13.184").matches(*client6));
-}
-
 TEST_F(ClientTest, toText) {
     EXPECT_EQ("192.0.2.1#53214", client4->toText());
     EXPECT_EQ("2001:db8::1#53216", client6->toText());