From: Michal 'vorner' Vaner Date: Thu, 7 Jul 2011 14:18:57 +0000 (+0200) Subject: Merge branch 'work/not' X-Git-Tag: perftcpdns_before_epoll~263^2~1^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7203d4203cddbb6bf930586e2f3fba183ca12140;p=thirdparty%2Fkea.git Merge branch 'work/not' Conflicts: src/lib/acl/dns.cc --- 7203d4203cddbb6bf930586e2f3fba183ca12140 diff --cc src/lib/acl/dns.cc index 0d0b3988f1,4dae297f50..cb948ebb4b --- a/src/lib/acl/dns.cc +++ b/src/lib/acl/dns.cc @@@ -12,97 -12,33 +12,107 @@@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include "dns.h" -#include "logic_check.h" +#include +#include +#include +#include + +#include + +#include + +#include +#include +#include ++#include + +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::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 +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 supported_names; + supported_names.push_back("from"); + return (supported_names); +} + +shared_ptr +internal::RequestCheckCreator::create(const string& name, + ConstElementPtr definition, + // unused: + const acl::Loader&) +{ + if (!definition) { + isc_throw(LoaderError, + "NULL pointer is passed to RequestCheckCreator"); + } + + if (name == "from") { + return (shared_ptr( + 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) { - // TODO: - // There's some trick with auto_ptr in the branch when IP check - // is added here. That one is better, bring it in on merge. - loader = new Loader(REJECT); - loader->registerCreator( + // 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 loader_ptr = + auto_ptr(new RequestLoader(REJECT)); + + // Register default check creator(s) + loader_ptr->registerCreator(shared_ptr( + new internal::RequestCheckCreator())); ++ loader_ptr->registerCreator( + shared_ptr >( + new NotCreator("NOT"))); - loader->registerCreator( ++ loader_ptr->registerCreator( + shared_ptr >( + new LogicCreator("ANY"))); - loader->registerCreator( ++ loader_ptr->registerCreator( + shared_ptr >( + new LogicCreator("ALL"))); + + // From this point there shouldn't be any exception thrown + loader = loader_ptr.release(); } + return (*loader); } diff --cc src/lib/acl/tests/dns_test.cc index 87fd8a1569,b2f967843e..3a42af0355 --- a/src/lib/acl/tests/dns_test.cc +++ b/src/lib/acl/tests/dns_test.cc @@@ -42,146 -19,37 +42,164 @@@ 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 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(*check_); + EXPECT_EQ(AF_INET, ipcheck_.getFamily()); + EXPECT_EQ(32, ipcheck_.getPrefixlen()); + const vector 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(*check_); + EXPECT_EQ(AF_INET6, ipcheck_.getFamily()); + EXPECT_EQ(120, ipcheck_.getPrefixlen()); + const vector 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 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 ipaddr; + scoped_ptr request; + dns::internal::RequestCheckCreator creator_; +}; + +TEST_F(RequestCheckTest, checkIPv4) { + // Exact match + EXPECT_TRUE(createIPCheck("192.0.2.1")->matches(getRequest4())); + // Exact match (negative) + EXPECT_FALSE(createIPCheck("192.0.2.53")->matches(getRequest4())); + // Prefix match + EXPECT_TRUE(createIPCheck("192.0.2.0/24")->matches(getRequest4())); + // Prefix match (negative) + EXPECT_FALSE(createIPCheck("192.0.1.0/24")->matches(getRequest4())); + // Address family mismatch (the first 4 bytes of the IPv6 address has the + // same binary representation as the client's IPv4 address, which + // shouldn't confuse the match logic) + EXPECT_FALSE(createIPCheck("c000:0201::")->matches(getRequest4())); +} + +TEST_F(RequestCheckTest, checkIPv6) { + // The following are a set of tests of the same concept as checkIPv4 + EXPECT_TRUE(createIPCheck("2001:db8::1")->matches(getRequest6())); + EXPECT_FALSE(createIPCheck("2001:db8::53")->matches(getRequest6())); + EXPECT_TRUE(createIPCheck("2001:db8::/64")->matches(getRequest6())); + EXPECT_FALSE(createIPCheck("2001:db8:1::/64")->matches(getRequest6())); + EXPECT_FALSE(createIPCheck("32.1.13.184")->matches(getRequest6())); } + // The following tests test only the creators are registered, they are tested + // elsewhere + -// TODO: Enable the tests when we merge the IP check branch here (is it called -// "from" in the branch?) -TEST(DNSACL, DISABLED_notLoad) { - EXPECT_NO_THROW(getLoader().loadCheck(isc::data::Element::fromJSON( ++TEST(DNSACL, notLoad) { ++ EXPECT_NO_THROW(getRequestLoader().loadCheck(isc::data::Element::fromJSON( + "{\"NOT\": {\"from\": \"192.0.2.1\"}}"))); + } + -TEST(DNSACL, DISABLED_allLoad) { - EXPECT_NO_THROW(getLoader().loadCheck(isc::data::Element::fromJSON( ++TEST(DNSACL, allLoad) { ++ EXPECT_NO_THROW(getRequestLoader().loadCheck(isc::data::Element::fromJSON( + "{\"ALL\": [{\"from\": \"192.0.2.1\"}]}"))); + } + -TEST(DNSACL, DISABLED_anyLoad) { - EXPECT_NO_THROW(getLoader().loadCheck(isc::data::Element::fromJSON( ++TEST(DNSACL, anyLoad) { ++ EXPECT_NO_THROW(getRequestLoader().loadCheck(isc::data::Element::fromJSON( + "{\"ANY\": [{\"from\": \"192.0.2.1\"}]}"))); + } + } diff --cc src/lib/server_common/tests/Makefile.am index 5bc6586b50,caf8070610..d7e113af1c --- a/src/lib/server_common/tests/Makefile.am +++ b/src/lib/server_common/tests/Makefile.am @@@ -38,9 -38,9 +38,10 @@@ run_unittests_LDADD += $(top_builddir)/ run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la run_unittests_LDADD += $(top_builddir)/src/lib/asiodns/libasiodns.la run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la +run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la run_unittests_LDADD += $(top_builddir)/src/lib/acl/libacl.la run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la + run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la