[trac999] added a simplified version of query ACL configuration

src/bin/resolver/resolver.cc

@@ -20,12 +20,16 @@
 #include <vector>
 #include <cassert>
 
+#include <boost/shared_ptr.hpp>
+#include <boost/lexical_cast.hpp>
+#include <boost/foreach.hpp>
+
+#include <acl/acl.h>
+#include <acl/loader.h>
+
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
 
-#include <boost/foreach.hpp>
-#include <boost/lexical_cast.hpp>
-
 #include <config/ccsession.h>
 
 #include <exceptions/exceptions.h>
@@ -41,6 +45,8 @@
 #include <dns/rrttl.h>
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
+
+#include <server_common/client.h>
 #include <server_common/portconfig.h>
 
 #include <resolve/recursive_query.h>
@@ -49,14 +55,17 @@
 #include "resolver_log.h"
 
 using namespace std;
+using namespace boost;
 
 using namespace isc;
 using namespace isc::util;
+using namespace isc::acl;
 using namespace isc::dns;
 using namespace isc::data;
 using namespace isc::config;
 using namespace isc::asiodns;
 using namespace isc::asiolink;
+using namespace isc::server_common;
 using namespace isc::server_common::portconfig;
 
 class ResolverImpl {
@@ -71,6 +80,7 @@ public:
         client_timeout_(4000),
         lookup_timeout_(30000),
         retries_(3),
+        query_acl_(new Resolver::ClientACL(REJECT)),
         rec_query_(NULL)
     {}
 
@@ -167,6 +177,9 @@ public:
     /// Number of retries after timeout
     unsigned retries_;
 
+    /// TBD
+    shared_ptr<const Resolver::ClientACL> query_acl_;
+
 private:
 
     /// Object to handle upstream queries
@@ -530,6 +543,32 @@ ResolverImpl::processNormalQuery(ConstMessagePtr query_message,
     }
 }
 
+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_CONFIGUPD)
@@ -546,6 +585,8 @@ 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")));
         bool set_timeouts(false);
         int qtimeout = impl_->query_timeout_;
         int ctimeout = impl_->client_timeout_;
@@ -622,6 +663,9 @@ Resolver::updateConfig(ConstElementPtr config) {
             setTimeouts(qtimeout, ctimeout, ltimeout, retries);
             need_query_restart = true;
         }
+        if (query_acl) {
+            setQueryACL(query_acl);
+        }
 
         if (need_query_restart) {
             impl_->queryShutdown();
@@ -707,3 +751,13 @@ AddressList
 Resolver::getListenAddresses() const {
     return (impl_->listen_);
 }
+
+const Resolver::ClientACL&
+Resolver::getQueryACL() const {
+    return (*impl_->query_acl_);
+}
+
+void
+Resolver::setQueryACL(shared_ptr<const ClientACL> new_acl) {
+    impl_->query_acl_ = new_acl;
+}

src/bin/resolver/resolver.h

@@ -19,6 +19,10 @@
 #include <vector>
 #include <utility>
 
+#include <boost/shared_ptr.hpp>
+
+#include <acl/acl.h>
+
 #include <cc/data.h>
 #include <config/ccsession.h>
 #include <dns/message.h>
@@ -37,6 +41,12 @@
 
 #include <resolve/resolver_interface.h>
 
+namespace isc {
+namespace server_common {
+class Client;
+}
+}
+
 class ResolverImpl;
 
 /**
@@ -236,6 +246,12 @@ public:
      */
     int getRetries() const;
 
+    typedef isc::acl::ACL<isc::server_common::Client> ClientACL;
+
+    const ClientACL& getQueryACL() const;
+
+    void setQueryACL(boost::shared_ptr<const ClientACL> new_acl);
+
 private:
     ResolverImpl* impl_;
     isc::asiodns::DNSService* dnss_;

src/bin/resolver/resolver.spec.pre.in

@@ -113,6 +113,39 @@
             }
           ]
         }
+      },
+      {
+        "item_name": "query_acl",
+	"item_type": "list",
+	"item_optional": false,
+	"item_default": [
+	  {
+	    "action": "ACCEPT",
+	    "from": "127.0.0.1"
+	  },
+	  {
+	    "action": "ACCEPT",
+	    "from": "::1"
+	  }
+	],
+	"list_item_spec": {
+	  "item_name": "rule",
+	  "item_type": "map",
+	  "item_optional": false,
+	  "item_default": {},
+	},
+	"map_item_spec": [
+	  {
+	    "item_name": "action",
+	    "item_type": "string",
+	    "item_optional": false
+	  },
+	  {
+	    "item_name": "from",
+	    "item_type": "string",
+	    "item_optional": false
+	  }
+	]
       }
     ],
     "commands": [

src/bin/resolver/tests/Makefile.am

@@ -47,6 +47,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 run_unittests_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 run_unittests_LDADD += $(top_builddir)/src/lib/resolve/libresolve.la
+run_unittests_LDADD += $(top_builddir)/src/lib/acl/libacl.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 
 # Note the ordering matters: -Wno-... must follow -Wextra (defined in

src/bin/resolver/tests/resolver_config_unittest.cc

@@ -16,12 +16,23 @@
 
 #include <string>
 
+#include <boost/scoped_ptr.hpp>
+
 #include <gtest/gtest.h>
 
 #include <cc/data.h>
 
+#include <config/ccsession.h>
+
 #include <asiodns/asiodns.h>
 #include <asiolink/asiolink.h>
+#include <asiolink/io_address.h>
+#include <asiolink/io_socket.h>
+#include <asiolink/io_message.h>
+
+#include <acl/acl.h>
+
+#include <server_common/client.h>
 
 #include <resolver/resolver.h>
 
@@ -30,25 +41,37 @@
 #include <testutils/portconfig.h>
 
 using namespace std;
+using boost::scoped_ptr;
+using namespace isc::acl;
 using namespace isc::data;
 using namespace isc::testutils;
 using namespace isc::asiodns;
 using namespace isc::asiolink;
+using namespace isc::server_common;
 using isc::UnitTestUtil;
 
 namespace {
 class ResolverConfig : public ::testing::Test {
-    public:
-        IOService ios;
-        DNSService dnss;
-        Resolver server;
-        ResolverConfig() :
-            dnss(ios, NULL, NULL, NULL)
-        {
-            server.setDNSService(dnss);
-            server.setConfigured();
-        }
-        void invalidTest(const string &JSON, const string& name);
+protected:
+    IOService ios;
+    DNSService dnss;
+    Resolver server;
+    scoped_ptr<const IOEndpoint> endpoint;
+    scoped_ptr<const IOMessage> request;
+    scoped_ptr<const Client> client;
+    ResolverConfig() : dnss(ios, NULL, NULL, NULL) {
+        server.setDNSService(dnss);
+        server.setConfigured();
+    }
+    const Client& createClient(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);
+    }
+    void invalidTest(const string &JSON, const string& name);
 };
 
 TEST_F(ResolverConfig, forwardAddresses) {
@@ -228,4 +251,122 @@ TEST_F(ResolverConfig, invalidTimeoutsConfig) {
         "}", "Negative number of retries");
 }
 
+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(
+                  createClient("2001:db8::1")));
+}
+
+TEST_F(ResolverConfig, emptyQueryACL) {
+    // Explicitly configured empty ACL should have the same effect.
+    ElementPtr 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(
+                  createClient("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 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("2001:db8::1")));
+}
+
+TEST_F(ResolverConfig, queryACLIPv6) {
+    // same for IPv6
+    ElementPtr 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(ACCEPT, server.getQueryACL().execute(
+                  createClient("2001:db8::1")));
+}
+
+TEST_F(ResolverConfig, compoundQueryACL) {
+    // A bit more complicated one: mixture of IPv4 and IPv6 with 3 rules
+    // in total.  We shouldn't have to check so many variations of rules
+    // 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 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(DROP, server.getQueryACL().execute(
+                  createClient("2001:db8::1")));
+    EXPECT_EQ(REJECT, server.getQueryACL().execute(
+                  createClient("2001:db8::2"))); // match the default rule
+}
+
+int
+getResultCode(ConstElementPtr result) {
+    int rcode;
+    isc::config::parseAnswer(rcode, result);
+    return (rcode);
+}
+
+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
+    // least the server code won't crash even if an unexpected input is given.
+
+    // ACL must be a list
+    EXPECT_EQ(1, getResultCode(
+                  server.updateConfig(
+                      Element::fromJSON("{ \"query_acl\": 1 }"))));
+    // Each rule must have "action" and "from"
+    EXPECT_EQ(1, getResultCode(
+                  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(
+                      Element::fromJSON("{ \"query_acl\":"
+                                        " [ {\"action\": 1,"
+                                        "    \"from\": \"192.0.2.1\"}]}"))));
+    EXPECT_EQ(1, getResultCode(
+                  server.updateConfig(
+                      Element::fromJSON("{ \"query_acl\":"
+                                        " [ {\"action\": \"BADACTION\","
+                                        "    \"from\": \"192.0.2.1\"}]}"))));
+
+    // invalid "from"
+    EXPECT_EQ(1, getResultCode(
+                  server.updateConfig(
+                      Element::fromJSON("{ \"query_acl\":"
+                                        " [ {\"action\": \"ACCEPT\","
+                                        "    \"from\": 53}]}"))));
+    EXPECT_EQ(1, getResultCode(
+                  server.updateConfig(
+                      Element::fromJSON("{ \"query_acl\":"
+                                        " [ {\"action\": \"ACCEPT\","
+                                        "    \"from\": \"1922.0.2.1\"}]}"))));
+}
+
 }

src/bin/resolver/tests/response_scrubber_unittest.cc

@@ -68,6 +68,12 @@ public:
         return address_.getFamily();
     }
 
+    // This is completely dummy and unused.  Define it just for build.
+    virtual const struct sockaddr& getSockAddr() const {
+        static struct sockaddr sa;
+        return (sa);
+    }
+
 private:
     IOAddress   address_;        // Address of endpoint
     uint16_t    port_;          // Port number of endpoint

src/lib/server_common/client.h

@@ -17,6 +17,8 @@
 
 #include <boost/noncopyable.hpp>
 
+#include <acl/ip_check.h>
+
 #include <asiolink/io_message.h>
 
 namespace isc {

src/lib/server_common/tests/client_unittest.cc

@@ -34,14 +34,6 @@ using namespace isc::server_common;
 
 namespace {
 
-// copied from auth_srv_unittest.cc.  should share it.
-class DummyUnknownSocket : public IOSocket {
-public:
-    DummyUnknownSocket() {}
-    virtual int getNative() const { return (0); }
-    virtual int getProtocol() const { return (IPPROTO_IP); }
-};
-
 class ClientTest : public ::testing::Test {
 protected:
     ClientTest() {
@@ -55,7 +47,6 @@ protected:
                                      *endpoint6));
         client4.reset(new Client(*request4));
         client6.reset(new Client(*request6));
-        
     }
     scoped_ptr<const IOEndpoint> endpoint4;
     scoped_ptr<const IOEndpoint> endpoint6;