BIND 10 master, updated. 4c0d2595196da373ca70a52663b7ec13842c940d [master] Merge branch 'trac1069' with fixing conflicts in src/bin/resolver/Makefile.am
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Jul 5 17:51:44 UTC 2011
The branch, master has been updated
via 4c0d2595196da373ca70a52663b7ec13842c940d (commit)
via 803a215c662c5a692c3b057fc7c0bae6c91b3587 (commit)
via 18dcf1d0ec44f4ddf701d5872f6d5e493d3c4fdb (commit)
via ac06e0bbad2fd39f8cc77fac06fc397be14f92c2 (commit)
via a6de7efe8fbf314c5182744d462699283464d9f0 (commit)
via 3bd81bcaed4c9c2ca6c6ed5fab00f350be5c2eef (commit)
via d2dc7a3ef911a5ab83527753f351bc99440d60fe (commit)
via f0445f392c1e2c99acfe9117ad36eef0811bd68b (commit)
via aedaf51b32a4b31d697b18ecb914af3889d13c2c (commit)
via ae5aa5618516d4f894bdf5d2aefed76742069644 (commit)
via 0caa1d89d3e60a80ab7517d3691a149093e32be6 (commit)
via e3b0557e225ad3e7a6b7d192b8820666d7b81d0a (commit)
via 27bb28f8bbde1dfc79030b0129a1c0405a8ffc38 (commit)
from baf3d8783ad1bc05bbe4db507325e9bfcd8d9be9 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 4c0d2595196da373ca70a52663b7ec13842c940d
Merge: baf3d87 803a215
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Jul 5 10:51:31 2011 -0700
[master] Merge branch 'trac1069'
with fixing conflicts in src/bin/resolver/Makefile.am
-----------------------------------------------------------------------
Summary of changes:
src/bin/resolver/Makefile.am | 2 +-
src/bin/resolver/resolver.cc | 55 ++-----
src/bin/resolver/resolver.h | 17 +--
src/bin/resolver/tests/Makefile.am | 1 +
src/bin/resolver/tests/resolver_config_unittest.cc | 146 +++++++++--------
src/bin/resolver/tests/resolver_unittest.cc | 4 +-
src/lib/acl/dns.cc | 91 ++++++++++-
src/lib/acl/dns.h | 129 ++++++++++-----
src/lib/acl/loader.h | 27 +++-
src/lib/acl/tests/dns_test.cc | 172 ++++++++++++++++++--
src/lib/acl/tests/ip_check_unittest.cc | 31 +---
src/lib/acl/tests/loader_test.cc | 4 +
src/lib/acl/tests/sockaddr.h | 68 ++++++++
src/lib/server_common/client.cc | 7 -
src/lib/server_common/client.h | 11 --
src/lib/server_common/tests/client_unittest.cc | 24 ---
16 files changed, 541 insertions(+), 248 deletions(-)
create mode 100644 src/lib/acl/tests/sockaddr.h
-----------------------------------------------------------------------
diff --git a/src/bin/resolver/Makefile.am b/src/bin/resolver/Makefile.am
index 2cbf140..3f5f049 100644
--- a/src/bin/resolver/Makefile.am
+++ b/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
diff --git a/src/bin/resolver/resolver.cc b/src/bin/resolver/resolver.cc
index be254b7..a3a7a69 100644
--- a/src/bin/resolver/resolver.cc
+++ b/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");
}
diff --git a/src/bin/resolver/resolver.h b/src/bin/resolver/resolver.h
index 9c78126..4b9c773 100644
--- a/src/bin/resolver/resolver.h
+++ b/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_;
diff --git a/src/bin/resolver/tests/Makefile.am b/src/bin/resolver/tests/Makefile.am
index c519617..97a2ba6 100644
--- a/src/bin/resolver/tests/Makefile.am
+++ b/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
diff --git a/src/bin/resolver/tests/resolver_config_unittest.cc b/src/bin/resolver/tests/resolver_config_unittest.cc
index 9006301..698e535 100644
--- a/src/bin/resolver/tests/resolver_config_unittest.cc
+++ b/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(
diff --git a/src/bin/resolver/tests/resolver_unittest.cc b/src/bin/resolver/tests/resolver_unittest.cc
index 9bcc261..71474dd 100644
--- a/src/bin/resolver/tests/resolver_unittest.cc
+++ b/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);
}
diff --git a/src/lib/acl/dns.cc b/src/lib/acl/dns.cc
index 16f1bf5..0d0b398 100644
--- a/src/lib/acl/dns.cc
+++ b/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);
}
diff --git a/src/lib/acl/dns.h b/src/lib/acl/dns.h
index 6f36e51..118e5fd 100644
--- a/src/lib/acl/dns.h
+++ b/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:
diff --git a/src/lib/acl/loader.h b/src/lib/acl/loader.h
index a2aa2f8..c86373e 100644
--- a/src/lib/acl/loader.h
+++ b/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:
diff --git a/src/lib/acl/tests/dns_test.cc b/src/lib/acl/tests/dns_test.cc
index e5e0f3a..87fd8a1 100644
--- a/src/lib/acl/tests/dns_test.cc
+++ b/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()));
}
}
diff --git a/src/lib/acl/tests/ip_check_unittest.cc b/src/lib/acl/tests/ip_check_unittest.cc
index fb24978..8b8c498 100644
--- a/src/lib/acl/tests/ip_check_unittest.cc
+++ b/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 };
diff --git a/src/lib/acl/tests/loader_test.cc b/src/lib/acl/tests/loader_test.cc
index 4415081..1705c0a 100644
--- a/src/lib/acl/tests/loader_test.cc
+++ b/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);
}
}
diff --git a/src/lib/acl/tests/sockaddr.h b/src/lib/acl/tests/sockaddr.h
new file mode 100644
index 0000000..f004f7d
--- /dev/null
+++ b/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:
diff --git a/src/lib/server_common/client.cc b/src/lib/server_common/client.cc
index 31dee88..e6383d6 100644
--- a/src/lib/server_common/client.cc
+++ b/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()));
-}
diff --git a/src/lib/server_common/client.h b/src/lib/server_common/client.h
index 148e069..1c5928a 100644
--- a/src/lib/server_common/client.h
+++ b/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
diff --git a/src/lib/server_common/tests/client_unittest.cc b/src/lib/server_common/tests/client_unittest.cc
index 34a90a2..287a926 100644
--- a/src/lib/server_common/tests/client_unittest.cc
+++ b/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());
More information about the bind10-changes
mailing list