[master] Merge branch 'trac1069'

with fixing conflicts in src/bin/resolver/Makefile.am

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<const 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,97 @@
 // 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>
+
+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()));
+
+        // 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/tests/dns_test.cc

@@ -12,24 +12,176 @@
 // 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()));
 }
 
 }

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/sockaddr.h

@@ -0,0 +1,68 @@
+// 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 <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/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/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());