[trac1069] added specialization for IPCheck<RequestContext> with tests

based on the server_common/Client counterparts.  Also move the helper
getSockAddr() function for the test program as it's shared by multiple
test implementations.

src/lib/acl/dns.cc

@@ -32,10 +32,22 @@ 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&) const {
-    return (false);
+IPCheck<dns::RequestContext>::matches(
+    const dns::RequestContext& request) const
+{
+    return (compare(request.remote_address.getData(),
+                    request.remote_address.getFamily()));
 }
 
 namespace dns {

src/lib/acl/dns.h

@@ -46,16 +46,28 @@ namespace dns {
  *     call the ACLs directly?
  */
 struct RequestContext {
+    explicit RequestContext(const IPAddress& remote_address_param) :
+        remote_address(remote_address_param)
+    {}
+
+    const IPAddress& remote_address;
+
+#ifdef notyet
     /// \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.
      *
@@ -69,6 +81,7 @@ struct RequestContext {
      * ACL.
      */
     std::string tsig_key_name;
+#endif
 };
 
 /// \brief DNS based check.
@@ -96,6 +109,7 @@ RequestLoader& getLoader();
 // applications directly, and to signal the intent, they are given inside
 // a separate namespace.
 namespace internal {
+
 // Shortcut typedef
 typedef isc::acl::IPCheck<RequestContext> RequestIPCheck;
 
@@ -111,11 +125,11 @@ public:
     /// 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
 

src/lib/acl/tests/dns_test.cc

@@ -16,6 +16,10 @@
 
 #include <algorithm>
 #include <vector>
+#include <string>
+
+#include <boost/scoped_ptr.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include <exceptions/exceptions.h>
 
@@ -25,11 +29,14 @@
 #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::dns;
+using namespace isc::acl;
 using isc::acl::LoaderError;
 
 namespace {
@@ -38,9 +45,9 @@ namespace {
 // 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) {
-    RequestLoader* l(&getLoader());
+    dns::RequestLoader* l(&dns::getLoader());
     ASSERT_TRUE(l != NULL);
-    EXPECT_EQ(l, &getLoader());
+    EXPECT_EQ(l, &dns::getLoader());
     EXPECT_NO_THROW(l->load(Element::fromJSON("[{\"action\": \"DROP\"}]")));
 
     // Test check rules registered by default, i.e. RequestCheck
@@ -50,9 +57,9 @@ TEST(DNSACL, getLoader) {
 
 class RequestCheckCreatorTest : public ::testing::Test {
 protected:
-    internal::RequestCheckCreator creator_;
+    dns::internal::RequestCheckCreator creator_;
 
-    typedef boost::shared_ptr<const RequestCheck> ConstRequestCheckPtr;
+    typedef boost::shared_ptr<const dns::RequestCheck> ConstRequestCheckPtr;
     ConstRequestCheckPtr check_;
 };
 
@@ -70,9 +77,9 @@ TEST_F(RequestCheckCreatorTest, allowListAbbreviation) {
 // are done in the tests for IPCheck.
 TEST_F(RequestCheckCreatorTest, createIPv4Check) {
     check_ = creator_.create("from", Element::fromJSON("\"192.0.2.1\""),
-                             getLoader());
-    const internal::RequestIPCheck& ipcheck_ =
-        dynamic_cast<const internal::RequestIPCheck&>(*check_);
+                             dns::getLoader());
+    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());
@@ -85,9 +92,9 @@ TEST_F(RequestCheckCreatorTest, createIPv4Check) {
 TEST_F(RequestCheckCreatorTest, createIPv6Check) {
     check_ = creator_.create("from",
                              Element::fromJSON("\"2001:db8::5300/120\""),
-                             getLoader());
-    const internal::RequestIPCheck& ipcheck_ =
-        dynamic_cast<const internal::RequestIPCheck&>(*check_);
+                             dns::getLoader());
+    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());
@@ -102,22 +109,79 @@ TEST_F(RequestCheckCreatorTest, createIPv6Check) {
 TEST_F(RequestCheckCreatorTest, badCreate) {
     // Invalid name
     EXPECT_THROW(creator_.create("bad", Element::fromJSON("\"192.0.2.1\""),
-                                 getLoader()), LoaderError);
+                                 dns::getLoader()), LoaderError);
 
     // Invalid type of parameter
-    EXPECT_THROW(creator_.create("from", Element::fromJSON("4"), getLoader()),
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("4"),
+                                 dns::getLoader()),
                  isc::data::TypeError);
-    EXPECT_THROW(creator_.create("from", Element::fromJSON("[]"), getLoader()),
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("[]"),
+                                 dns::getLoader()),
                  isc::data::TypeError);
 
     // Syntax error for IPCheck
     EXPECT_THROW(creator_.create("from", Element::fromJSON("\"bad\""),
-                                 getLoader()),
+                                 dns::getLoader()),
                  isc::InvalidParameter);
 
     // NULL pointer
-    EXPECT_THROW(creator_.create("from", ConstElementPtr(), getLoader()),
+    EXPECT_THROW(creator_.create("from", ConstElementPtr(), dns::getLoader()),
                  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("\"")),
+                                dns::getLoader()));
+    }
+
+    // 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/sockaddr.h

@@ -0,0 +1,66 @@
+// 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>
+
+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 return a dummy value that would make subsequent test
+    // fail.
+    static struct sockaddr sa_dummy;
+    sa_dummy.sa_family = AF_UNSPEC;
+    return (sa_dummy);
+}
+
+} // end of namespace "tests"
+} // end of namespace "acl"
+} // end of namespace "isc"
+
+#endif  // __ACL_TEST_SOCKADDR_H
+
+// Local Variables:
+// mode: c++
+// End: