Merge branch 'master' into trac1155

ChangeLog

@@ -1,3 +1,13 @@
+275.	[func]		jinmei
+	Added support for TSIG key matching in ACLs.  The xfrout ACL can
+	now refer to TSIG key names using the "key" attribute.  For
+	example, the following specifies an ACL that allows zone transfer
+	if and only if the request is signed with a TSIG of a key name
+	"key.example":
+	> config set Xfrout/query_acl[0] {"action": "ACCEPT", \
+	                                  "key": "key.example"}
+	(Trac #1104, git 9b2e89cabb6191db86f88ee717f7abc4171fa979)
+
 274.	[bug]		naokikambe
 	add unittests for functions xml_handler, xsd_handler and xsl_handler
 	respectively to make sure their behaviors are correct, regardless of

src/bin/bind10/run_bind10.sh.in

@@ -23,14 +23,14 @@ BIND10_PATH=@abs_top_builddir@/src/bin/bind10
 PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/resolver:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:@abs_top_builddir@/src/bin/dhcp6:$PATH
 export PATH
 
-PYTHONPATH=@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config
+PYTHONPATH=@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config:@abs_top_builddir@/src/lib/python/isc/acl/.libs:
 export PYTHONPATH
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # required by loadable python modules.
 SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
 if test $SET_ENV_LIBRARY_PATH = yes; then
-	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:$@ENV_LIBRARY_PATH@
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/cryptolink/.libs:@abs_top_builddir@/src/lib/cc/.libs:@abs_top_builddir@/src/lib/config/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/acl/.libs:@abs_top_builddir@/src/lib/util/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:$@ENV_LIBRARY_PATH@
 	export @ENV_LIBRARY_PATH@
 fi
 

src/bin/resolver/resolver.cc

@@ -520,7 +520,8 @@ ResolverImpl::processNormalQuery(const IOMessage& io_message,
     const Client client(io_message);
     const BasicAction query_action(
         getQueryACL().execute(acl::dns::RequestContext(
-                                  client.getRequestSourceIPAddress())));
+                                  client.getRequestSourceIPAddress(),
+                                  query_message->getTSIGRecord())));
     if (query_action == isc::acl::REJECT) {
         LOG_INFO(resolver_logger, RESOLVER_QUERY_REJECTED)
             .arg(question->getName()).arg(qtype).arg(qclass).arg(client);

src/bin/resolver/tests/resolver_config_unittest.cc

@@ -72,7 +72,8 @@ protected:
                                           IOSocket::getDummyUDPSocket(),
                                           *endpoint));
         client.reset(new Client(*query_message));
-        request.reset(new RequestContext(client->getRequestSourceIPAddress()));
+        request.reset(new RequestContext(client->getRequestSourceIPAddress(),
+                                         NULL));
         return (*request);
     }
     void invalidTest(const string &JSON, const string& name);

src/bin/xfrout/tests/xfrout_test.py.in

@@ -137,7 +137,8 @@ class TestXfroutSession(unittest.TestCase):
         self.assertEqual(rcode.to_text(), "NOTAUTH")
         self.assertTrue(self.xfrsess._tsig_ctx is not None)
         # NOERROR
-        self.xfrsess._tsig_key_ring.add(TSIG_KEY)
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.xfrsess._tsig_key_ring.add(TSIG_KEY))
         [rcode, msg] = self.xfrsess._parse_query_message(request_data)
         self.assertEqual(rcode.to_text(), "NOERROR")
         self.assertTrue(self.xfrsess._tsig_ctx is not None)
@@ -172,6 +173,54 @@ class TestXfroutSession(unittest.TestCase):
         self.assertEqual(rcode.to_text(), "NOTAUTH")
         self.assertTrue(self.xfrsess._tsig_ctx is not None)
 
+        # ACL using TSIG: successful case
+        self.xfrsess._acl = isc.acl.dns.REQUEST_LOADER.load([
+            {"key": "example.com", "action": "ACCEPT"}, {"action": "REJECT"}
+        ])
+        self.assertEqual(TSIGKeyRing.SUCCESS,
+                         self.xfrsess._tsig_key_ring.add(TSIG_KEY))
+        [rcode, msg] = self.xfrsess._parse_query_message(request_data)
+        self.assertEqual(rcode.to_text(), "NOERROR")
+
+        # ACL using TSIG: key name doesn't match; should be rejected
+        self.xfrsess._acl = isc.acl.dns.REQUEST_LOADER.load([
+            {"key": "example.org", "action": "ACCEPT"}, {"action": "REJECT"}
+        ])
+        [rcode, msg] = self.xfrsess._parse_query_message(request_data)
+        self.assertEqual(rcode.to_text(), "REFUSED")
+
+        # ACL using TSIG: no TSIG; should be rejected
+        self.xfrsess._acl = isc.acl.dns.REQUEST_LOADER.load([
+            {"key": "example.org", "action": "ACCEPT"}, {"action": "REJECT"}
+        ])
+        [rcode, msg] = self.xfrsess._parse_query_message(self.mdata)
+        self.assertEqual(rcode.to_text(), "REFUSED")
+
+        #
+        # ACL using IP + TSIG: both should match
+        #
+        self.xfrsess._acl = isc.acl.dns.REQUEST_LOADER.load([
+                {"ALL": [{"key": "example.com"}, {"from": "192.0.2.1"}],
+                 "action": "ACCEPT"},
+                {"action": "REJECT"}
+        ])
+        # both matches
+        self.xfrsess._remote = ('192.0.2.1', 12345)
+        [rcode, msg] = self.xfrsess._parse_query_message(request_data)
+        self.assertEqual(rcode.to_text(), "NOERROR")
+        # TSIG matches, but address doesn't
+        self.xfrsess._remote = ('192.0.2.2', 12345)
+        [rcode, msg] = self.xfrsess._parse_query_message(request_data)
+        self.assertEqual(rcode.to_text(), "REFUSED")
+        # Address matches, but TSIG doesn't (not included)
+        self.xfrsess._remote = ('192.0.2.1', 12345)
+        [rcode, msg] = self.xfrsess._parse_query_message(self.mdata)
+        self.assertEqual(rcode.to_text(), "REFUSED")
+        # Neither address nor TSIG matches
+        self.xfrsess._remote = ('192.0.2.2', 12345)
+        [rcode, msg] = self.xfrsess._parse_query_message(self.mdata)
+        self.assertEqual(rcode.to_text(), "REFUSED")
+
     def test_get_query_zone_name(self):
         msg = self.getmsg()
         self.assertEqual(self.xfrsess._get_query_zone_name(msg), "example.com.")

src/bin/xfrout/xfrout.py.in

@@ -147,7 +147,8 @@ class XfroutSession():
             if rcode == Rcode.NOERROR():
                 # ACL checks
                 acl_result = self._acl.execute(
-                    isc.acl.dns.RequestContext(self._remote))
+                    isc.acl.dns.RequestContext(self._remote,
+                                               msg.get_tsig_record()))
                 if acl_result == DROP:
                     logger.info(XFROUT_QUERY_DROPPED,
                                 self._get_query_zone_name(msg),

src/lib/acl/Makefile.am

@@ -19,7 +19,7 @@ libacl_la_LIBADD += $(top_builddir)/src/lib/util/libutil.la
 # DNS specialized one
 lib_LTLIBRARIES += libdnsacl.la
 
-libdnsacl_la_SOURCES = dns.h dns.cc
+libdnsacl_la_SOURCES = dns.h dns.cc dnsname_check.h
 
 libdnsacl_la_LIBADD = libacl.la
 libdnsacl_la_LIBADD += $(top_builddir)/src/lib/dns/libdns++.la

src/lib/acl/dns.cc

@@ -20,15 +20,20 @@
 
 #include <exceptions/exceptions.h>
 
+#include <dns/name.h>
+#include <dns/tsigrecord.h>
+
 #include <cc/data.h>
 
 #include <acl/dns.h>
 #include <acl/ip_check.h>
+#include <acl/dnsname_check.h>
 #include <acl/loader.h>
 #include <acl/logic_check.h>
 
 using namespace std;
 using boost::shared_ptr;
+using namespace isc::dns;
 using namespace isc::data;
 
 namespace isc {
@@ -39,9 +44,6 @@ namespace acl {
 /// 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(
@@ -53,6 +55,18 @@ IPCheck<dns::RequestContext>::matches(
 
 namespace dns {
 
+/// The specialization of \c NameCheck for access control with
+/// \c RequestContext.
+///
+/// It returns \c true if the request contains a TSIG record and its key
+/// (owner) name is equal to the name stored in the check; otherwise
+/// it returns \c false.
+template<>
+bool
+NameCheck<RequestContext>::matches(const RequestContext& request) const {
+    return (request.tsig != NULL && request.tsig->getName() == name_);
+}
+
 vector<string>
 internal::RequestCheckCreator::names() const {
     // Probably we should eventually build this vector in a more
@@ -60,6 +74,7 @@ internal::RequestCheckCreator::names() const {
     // everything.
     vector<string> supported_names;
     supported_names.push_back("from");
+    supported_names.push_back("key");
     return (supported_names);
 }
 
@@ -77,6 +92,10 @@ internal::RequestCheckCreator::create(const string& name,
     if (name == "from") {
         return (shared_ptr<internal::RequestIPCheck>(
                     new internal::RequestIPCheck(definition->stringValue())));
+    } else if (name == "key") {
+        return (shared_ptr<internal::RequestKeyCheck>(
+                    new internal::RequestKeyCheck(
+                        Name(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

src/lib/acl/dns.h

@@ -23,9 +23,13 @@
 #include <cc/data.h>
 
 #include <acl/ip_check.h>
+#include <acl/dnsname_check.h>
 #include <acl/loader.h>
 
 namespace isc {
+namespace dns {
+class TSIGRecord;
+}
 namespace acl {
 namespace dns {
 
@@ -53,9 +57,9 @@ namespace dns {
  * used only for a very short period as stated above.
  *
  * 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
+ * maintains the remote (source) IP address of the request and (optionally)
+ * the TSIG record included in the request.  We may add more parameters of
+ * the request as we see the need for them.  Possible additional parameters
  * 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).
  */
@@ -68,8 +72,12 @@ struct RequestContext {
     /// \exception None
     ///
     /// \parameter remote_address_param The remote IP address
-    explicit RequestContext(const IPAddress& remote_address_param) :
-        remote_address(remote_address_param)
+    /// \parameter tsig_param A valid pointer to the TSIG record included in
+    /// the request or NULL if the request doesn't contain a TSIG.
+    RequestContext(const IPAddress& remote_address_param,
+                   const isc::dns::TSIGRecord* tsig_param) :
+        remote_address(remote_address_param),
+        tsig(tsig_param)
     {}
 
     ///
@@ -83,6 +91,11 @@ struct RequestContext {
     //@{
     /// \brief The remote IP address (eg. the client's IP address).
     const IPAddress& remote_address;
+
+    /// \brief The TSIG record included in the request message, if any.
+    ///
+    /// If the request doesn't include a TSIG, this member will be NULL.
+    const isc::dns::TSIGRecord* const tsig;
     //@}
 };
 
@@ -114,6 +127,7 @@ namespace internal {
 
 // Shortcut typedef
 typedef isc::acl::IPCheck<RequestContext> RequestIPCheck;
+typedef isc::acl::dns::NameCheck<RequestContext> RequestKeyCheck;
 
 class RequestCheckCreator : public acl::Loader<RequestContext>::CheckCreator {
 public:

src/lib/acl/dnsname_check.h

@@ -0,0 +1,83 @@
+// 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 __DNSNAME_CHECK_H
+#define __DNSNAME_CHECK_H 1
+
+#include <dns/name.h>
+
+#include <acl/check.h>
+
+namespace isc {
+namespace acl {
+namespace dns {
+
+/// ACL check for DNS names
+///
+/// This class is intended to perform a match between a domain name
+/// specified in an ACL and a given name.  The primary usage of this class
+/// is an ACL match for TSIG keys, where an ACL would contain a list of
+/// acceptable key names and the \c match() method would compare the owner
+/// name of a TSIG record against the specified names.
+///
+/// This class could be used for other kinds of names such as the query name
+/// of normal DNS queries.
+///
+/// The class is templated on the type of a context structure passed to the
+/// matches() method, and a template specialisation for that method must be
+/// supplied for the class to be used.
+template <typename Context>
+class NameCheck : public Check<Context> {
+public:
+    /// The constructor
+    ///
+    /// \exception std::bad_alloc Resource allocation fails in copying the
+    /// name
+    ///
+    /// \param name The domain name to be matched in \c matches().
+    NameCheck(const isc::dns::Name& name) : name_(name) {}
+
+    /// Destructor
+    virtual ~NameCheck() {}
+
+    /// The check method
+    ///
+    /// Matches the passed argument to the condition stored here.  Different
+    /// specializations must be provided for different argument types, and the
+    /// program will fail to compile if a required specialisation is not
+    /// provided.
+    ///
+    /// \param context Information to be matched
+    virtual bool matches(const Context& context) const;
+
+    /// Returns the name specified on construction.
+    ///
+    /// This is mainly for testing purposes.
+    ///
+    /// \exception None
+    const isc::dns::Name& getName() const { return (name_); }
+
+private:
+    const isc::dns::Name name_;
+};
+
+} // namespace dns
+} // namespace acl
+} // namespace isc
+
+#endif // __DNSNAME_CHECK_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/acl/tests/Makefile.am

@@ -16,6 +16,7 @@ run_unittests_SOURCES += acl_test.cc
 run_unittests_SOURCES += check_test.cc
 run_unittests_SOURCES += dns_test.cc
 run_unittests_SOURCES += ip_check_unittest.cc
+run_unittests_SOURCES += dnsname_check_unittest.cc
 run_unittests_SOURCES += loader_test.cc
 run_unittests_SOURCES += logcheck.h
 run_unittests_SOURCES += creators.h

src/lib/acl/tests/dns_test.cc

@@ -23,6 +23,11 @@
 
 #include <exceptions/exceptions.h>
 
+#include <dns/name.h>
+#include <dns/tsigkey.h>
+#include <dns/tsigrecord.h>
+#include <dns/rdataclass.h>
+
 #include <cc/data.h>
 #include <acl/dns.h>
 #include <acl/loader.h>
@@ -35,6 +40,8 @@
 
 using namespace std;
 using boost::scoped_ptr;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
 using namespace isc::data;
 using namespace isc::acl;
 using namespace isc::acl::dns;
@@ -64,8 +71,10 @@ protected:
 };
 
 TEST_F(RequestCheckCreatorTest, names) {
-    ASSERT_EQ(1, creator_.names().size());
-    EXPECT_EQ("from", creator_.names()[0]);
+    const vector<string> names = creator_.names();
+    EXPECT_EQ(2, names.size());
+    EXPECT_TRUE(find(names.begin(), names.end(), "from") != names.end());
+    EXPECT_TRUE(find(names.begin(), names.end(), "key") != names.end());
 }
 
 TEST_F(RequestCheckCreatorTest, allowListAbbreviation) {
@@ -93,11 +102,11 @@ TEST_F(RequestCheckCreatorTest, createIPv6Check) {
     check_ = creator_.create("from",
                              Element::fromJSON("\"2001:db8::5300/120\""),
                              getRequestLoader());
-    const dns::internal::RequestIPCheck& ipcheck_ =
+    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());
+    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,
@@ -106,6 +115,14 @@ TEST_F(RequestCheckCreatorTest, createIPv6Check) {
                       expected_address));
 }
 
+TEST_F(RequestCheckCreatorTest, createTSIGKeyCheck) {
+    check_ = creator_.create("key", Element::fromJSON("\"key.example.com\""),
+                             getRequestLoader());
+    const dns::internal::RequestKeyCheck& keycheck =
+        dynamic_cast<const dns::internal::RequestKeyCheck&>(*check_);
+    EXPECT_EQ(Name("key.example.com"), keycheck.getName());
+}
+
 TEST_F(RequestCheckCreatorTest, badCreate) {
     // Invalid name
     EXPECT_THROW(creator_.create("bad", Element::fromJSON("\"192.0.2.1\""),
@@ -118,12 +135,23 @@ TEST_F(RequestCheckCreatorTest, badCreate) {
     EXPECT_THROW(creator_.create("from", Element::fromJSON("[]"),
                                  getRequestLoader()),
                  isc::data::TypeError);
+    EXPECT_THROW(creator_.create("key", Element::fromJSON("1"),
+                                 getRequestLoader()),
+                 isc::data::TypeError);
+    EXPECT_THROW(creator_.create("key", Element::fromJSON("{}"),
+                                 getRequestLoader()),
+                 isc::data::TypeError);
 
     // Syntax error for IPCheck
     EXPECT_THROW(creator_.create("from", Element::fromJSON("\"bad\""),
                                  getRequestLoader()),
                  isc::InvalidParameter);
 
+    // Syntax error for Name (key) Check
+    EXPECT_THROW(creator_.create("key", Element::fromJSON("\"bad..name\""),
+                                 getRequestLoader()),
+                 EmptyLabel);
+
     // NULL pointer
     EXPECT_THROW(creator_.create("from", ConstElementPtr(), getRequestLoader()),
                  LoaderError);
@@ -140,23 +168,43 @@ protected:
                                 getRequestLoader()));
     }
 
+    // A helper shortcut to create a single Name (key) check for the given
+    // name.
+    ConstRequestCheckPtr createKeyCheck(const string& key_name) {
+        return (creator_.create("key", Element::fromJSON(
+                                    string("\"") + key_name + 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() {
+    const dns::RequestContext& getRequest4(const TSIGRecord* tsig = NULL) {
         ipaddr.reset(new IPAddress(tests::getSockAddr("192.0.2.1")));
-        request.reset(new dns::RequestContext(*ipaddr));
+        request.reset(new dns::RequestContext(*ipaddr, tsig));
         return (*request);
     }
-    const dns::RequestContext& getRequest6() {
+    const dns::RequestContext& getRequest6(const TSIGRecord* tsig = NULL) {
         ipaddr.reset(new IPAddress(tests::getSockAddr("2001:db8::1")));
-        request.reset(new dns::RequestContext(*ipaddr));
+        request.reset(new dns::RequestContext(*ipaddr, tsig));
         return (*request);
     }
 
+    // create a one time TSIG Record for a specific test.  The only parameter
+    // of the record that matters is the key name; others are hardcoded with
+    // arbitrarily chosen values.
+    const TSIGRecord* getTSIGRecord(const string& key_name) {
+        tsig_rdata.reset(new any::TSIG(TSIGKey::HMACMD5_NAME(), 0, 0, 0, NULL,
+                                       0, 0, 0, NULL));
+        tsig.reset(new TSIGRecord(Name(key_name), *tsig_rdata));
+        return (tsig.get());
+    }
+
 private:
     scoped_ptr<IPAddress> ipaddr;
     scoped_ptr<dns::RequestContext> request;
+    scoped_ptr<any::TSIG> tsig_rdata;
+    scoped_ptr<TSIGRecord> tsig;
     dns::internal::RequestCheckCreator creator_;
 };
 
@@ -184,6 +232,24 @@ TEST_F(RequestCheckTest, checkIPv6) {
     EXPECT_FALSE(createIPCheck("32.1.13.184")->matches(getRequest6()));
 }
 
+TEST_F(RequestCheckTest, checkTSIGKey) {
+    EXPECT_TRUE(createKeyCheck("key.example.com")->matches(
+                    getRequest4(getTSIGRecord("key.example.com"))));
+    EXPECT_FALSE(createKeyCheck("key.example.com")->matches(
+                     getRequest4(getTSIGRecord("badkey.example.com"))));
+
+    // Same for IPv6 (which shouldn't matter)
+    EXPECT_TRUE(createKeyCheck("key.example.com")->matches(
+                    getRequest6(getTSIGRecord("key.example.com"))));
+    EXPECT_FALSE(createKeyCheck("key.example.com")->matches(
+                     getRequest6(getTSIGRecord("badkey.example.com"))));
+
+    // by default the test request doesn't have a TSIG key, which shouldn't
+    // match any key checks.
+    EXPECT_FALSE(createKeyCheck("key.example.com")->matches(getRequest4()));
+    EXPECT_FALSE(createKeyCheck("key.example.com")->matches(getRequest6()));
+}
+
 // The following tests test only the creators are registered, they are tested
 // elsewhere
 

src/lib/acl/tests/dnsname_check_unittest.cc

@@ -0,0 +1,59 @@
+// 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.
+
+#include <gtest/gtest.h>
+
+#include <dns/name.h>
+
+#include <acl/dnsname_check.h>
+
+using namespace isc::dns;
+using namespace isc::acl::dns;
+
+// Provide a specialization of the DNSNameCheck::matches() method.
+namespace isc  {
+namespace acl {
+namespace dns {
+template <>
+bool NameCheck<Name>::matches(const Name& name) const {
+    return (name_ == name);
+}
+} // namespace dns
+} // namespace acl
+} // namespace isc
+
+namespace {
+TEST(DNSNameCheck, construct) {
+    EXPECT_EQ(Name("example.com"),
+              NameCheck<Name>(Name("example.com")).getName());
+
+    // Construct the same check with an explicit trailing dot.  Should result
+    // in the same result.
+    EXPECT_EQ(Name("example.com"),
+              NameCheck<Name>(Name("example.com.")).getName());
+}
+
+TEST(DNSNameCheck, match) {
+    NameCheck<Name> check(Name("example.com"));
+    EXPECT_TRUE(check.matches(Name("example.com")));
+    EXPECT_FALSE(check.matches(Name("example.org")));
+
+    // comparison is case insensitive
+    EXPECT_TRUE(check.matches(Name("EXAMPLE.COM")));
+
+    // this is exact match.  so super/sub domains don't match
+    EXPECT_FALSE(check.matches(Name("com")));
+    EXPECT_FALSE(check.matches(Name("www.example.com")));
+}
+} // Unnamed namespace

src/lib/python/isc/acl/Makefile.am

@@ -4,10 +4,10 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
-python_PYTHON = __init__.py
+python_PYTHON = __init__.py dns.py
 pythondir = $(PYTHON_SITEPKG_DIR)/isc/acl
 
-pyexec_LTLIBRARIES = acl.la dns.la
+pyexec_LTLIBRARIES = acl.la _dns.la
 pyexecdir = $(PYTHON_SITEPKG_DIR)/isc/acl
 
 acl_la_SOURCES = acl.cc
@@ -15,14 +15,14 @@ acl_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 acl_la_LDFLAGS = $(PYTHON_LDFLAGS)
 acl_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
 
-dns_la_SOURCES = dns.h dns.cc dns_requestacl_python.h dns_requestacl_python.cc
-dns_la_SOURCES += dns_requestcontext_python.h dns_requestcontext_python.cc
-dns_la_SOURCES += dns_requestloader_python.h dns_requestloader_python.cc
-dns_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
-dns_la_LDFLAGS = $(PYTHON_LDFLAGS)
+_dns_la_SOURCES = dns.h dns.cc dns_requestacl_python.h dns_requestacl_python.cc
+_dns_la_SOURCES += dns_requestcontext_python.h dns_requestcontext_python.cc
+_dns_la_SOURCES += dns_requestloader_python.h dns_requestloader_python.cc
+_dns_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
+_dns_la_LDFLAGS = $(PYTHON_LDFLAGS)
 # Note: PYTHON_CXXFLAGS may have some -Wno... workaround, which must be
 # placed after -Wextra defined in AM_CXXFLAGS
-dns_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
+_dns_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
 
 # Python prefers .so, while some OSes (specifically MacOS) use a different
 # suffix for dynamic objects.  -module is necessary to work this around.
@@ -30,11 +30,11 @@ acl_la_LDFLAGS += -module
 acl_la_LIBADD = $(top_builddir)/src/lib/acl/libacl.la
 acl_la_LIBADD += $(PYTHON_LIB)
 
-dns_la_LDFLAGS += -module
-dns_la_LIBADD = $(top_builddir)/src/lib/acl/libdnsacl.la
-dns_la_LIBADD += $(PYTHON_LIB)
+_dns_la_LDFLAGS += -module
+_dns_la_LIBADD = $(top_builddir)/src/lib/acl/libdnsacl.la
+_dns_la_LIBADD += $(PYTHON_LIB)
 
-EXTRA_DIST = acl.py dns.py
+EXTRA_DIST = acl.py _dns.py
 EXTRA_DIST += acl_inc.cc
 EXTRA_DIST += dnsacl_inc.cc dns_requestacl_inc.cc dns_requestcontext_inc.cc
 EXTRA_DIST += dns_requestloader_inc.cc

src/lib/python/isc/acl/_dns.py

@@ -0,0 +1,29 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+# This file is not installed; The .so version will be installed into the right
+# place at installation time.
+# This helper script is only to find it in the .libs directory when we run
+# as a test or from the build directory.
+
+import os
+import sys
+
+for base in sys.path[:]:
+    bindingdir = os.path.join(base, 'isc/acl/.libs')
+    if os.path.exists(bindingdir):
+        sys.path.insert(0, bindingdir)
+
+from _dns import *

src/lib/python/isc/acl/dns.cc

@@ -52,7 +52,7 @@ PyMethodDef methods[] = {
 
 PyModuleDef dnsacl = {
     { PyObject_HEAD_INIT(NULL) NULL, 0, NULL},
-    "isc.acl.dns",
+    "isc.acl._dns",
     dnsacl_doc,
     -1,
     methods,
@@ -90,7 +90,7 @@ getACLException(const char* ex_name) {
 }
 
 PyMODINIT_FUNC
-PyInit_dns(void) {
+PyInit__dns(void) {
     PyObject* mod = PyModule_Create(&dnsacl);
     if (mod == NULL) {
         return (NULL);

src/lib/python/isc/acl/dns.py

@@ -13,21 +13,61 @@
 # NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-# This file is not installed. The log.so is installed into the right place.
-# It is only to find it in the .libs directory when we run as a test or
-# from the build directory.
-# But as nobody gives us the builddir explicitly (and we can't use generation
-# from .in file, as it would put us into the builddir and we wouldn't be found)
-# we guess from current directory. Any idea for something better? This should
-# be enough for the tests, but would it work for B10_FROM_SOURCE as well?
-# Should we look there? Or define something in bind10_config?
-
-import os
-import sys
-
-for base in sys.path[:]:
-    bindingdir = os.path.join(base, 'isc/acl/.libs')
-    if os.path.exists(bindingdir):
-        sys.path.insert(0, bindingdir)
-
-from dns import *
+"""\
+This module provides Python bindings for the C++ classes in the
+isc::acl::dns namespace.  Specifically, it defines Python interfaces of
+handling access control lists (ACLs) with DNS related contexts.
+The actual binding is implemented in an effectively hidden module,
+isc.acl._dns; this frontend module is in terms of implementation so that
+the C++ binding code doesn't have to deal with complicated operations
+that could be done in a more straightforward way in native Python.
+
+For further details of the actual module, see the documentation of the
+_dns module.
+"""
+
+import pydnspp
+
+import isc.acl._dns
+from isc.acl._dns import *
+
+class RequestACL(isc.acl._dns.RequestACL):
+    """A straightforward wrapper subclass of isc.acl._dns.RequestACL.
+
+    See the base class documentation for more implementation.
+    """
+    pass
+
+class RequestLoader(isc.acl._dns.RequestLoader):
+    """A straightforward wrapper subclass of isc.acl._dns.RequestLoader.
+
+    See the base class documentation for more implementation.
+    """
+    pass
+
+class RequestContext(isc.acl._dns.RequestContext):
+    """A straightforward wrapper subclass of isc.acl._dns.RequestContext.
+
+    See the base class documentation for more implementation.
+    """
+
+    def __init__(self, remote_address, tsig=None):
+        """Wrapper for the RequestContext constructor.
+
+        Internal implementation details that the users don't have to
+        worry about: To avoid dealing with pydnspp bindings in the C++ code,
+        this wrapper converts the TSIG record in its wire format in the form
+        of byte data, and has the binding re-construct the record from it.
+        """
+        tsig_wire = b''
+        if tsig is not None:
+            if not isinstance(tsig, pydnspp.TSIGRecord):
+                raise TypeError("tsig must be a TSIGRecord, not %s" %
+                                tsig.__class__.__name__)
+            tsig_wire = tsig.to_wire(tsig_wire)
+        isc.acl._dns.RequestContext.__init__(self, remote_address, tsig_wire)
+
+    def __str__(self):
+        """Wrap __str__() to convert the module name."""
+        s = isc.acl._dns.RequestContext.__str__(self)
+        return s.replace('<isc.acl._dns', '<isc.acl.dns')

src/lib/python/isc/acl/dns_requestacl_python.cc

@@ -114,7 +114,7 @@ namespace python {
 // Most of the functions are not actually implemented and NULL here.
 PyTypeObject requestacl_type = {
     PyVarObject_HEAD_INIT(NULL, 0)
-    "isc.acl.dns.RequestACL",
+    "isc.acl._dns.RequestACL",
     sizeof(s_RequestACL),                 // tp_basicsize
     0,                                  // tp_itemsize
     RequestACL_destroy,                // tp_dealloc
@@ -132,7 +132,7 @@ PyTypeObject requestacl_type = {
     NULL,                               // tp_getattro
     NULL,                               // tp_setattro
     NULL,                               // tp_as_buffer
-    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE, // tp_flags
     RequestACL_doc,
     NULL,                               // tp_traverse
     NULL,                               // tp_clear

src/lib/python/isc/acl/dns_requestcontext_inc.cc

@@ -5,18 +5,18 @@ DNS request to be checked.\n\
 This plays the role of ACL context for the RequestACL object.\n\
 \n\
 Based on the minimalist philosophy, the initial implementation only\n\
-maintains the remote (source) IP address of the request. The plan is\n\
-to add more parameters of the request. A scheduled next step is to\n\
-support the TSIG key (if it's included in the request). Other\n\
-possibilities are the local (destination) IP address, the remote and\n\
-local port numbers, various fields of the DNS request (e.g. a\n\
-particular header flag value).\n\
+maintains the remote (source) IP address of the request and\n\
+(optionally) the TSIG record included in the request. We may add more\n\
+parameters of the request as we see the need for them. Possible\n\
+additional parameters are the local (destination) IP address, the\n\
+remote and local port numbers, various fields of the DNS request (e.g.\n\
+a particular header flag value).\n\
 \n\
-RequestContext(remote_address)\n\
+RequestContext(remote_address, tsig)\n\
 \n\
     In this initial implementation, the constructor only takes a\n\
     remote IP address in the form of a socket address as used in the\n\
-    Python socket module.\n\
+    Python socket module, and optionally a pydnspp.TSIGRecord object.\n\
 \n\
     Exceptions:\n\
       isc.acl.ACLError Normally shouldn't happen, but still possible\n\
@@ -25,6 +25,9 @@ RequestContext(remote_address)\n\
 \n\
     Parameters:\n\
       remote_address The remote IP address\n\
+      tsig   The TSIG record included in the request message, if any.\n\
+             If the request doesn't include a TSIG, this will be None.\n\
+             If this parameter is omitted None will be assumed.\n\
 \n\
 ";
 } // unnamed namespace

src/lib/python/isc/acl/dns_requestcontext_python.cc

@@ -14,7 +14,7 @@
 
 // Enable this if you use s# variants with PyArg_ParseTuple(), see
 // http://docs.python.org/py3k/c-api/arg.html#strings-and-buffers
-//#define PY_SSIZE_T_CLEAN
+#define PY_SSIZE_T_CLEAN
 
 // Python.h needs to be placed at the head of the program file, see:
 // http://docs.python.org/py3k/extending/extending.html#a-simple-example
@@ -37,8 +37,16 @@
 
 #include <exceptions/exceptions.h>
 
+#include <util/buffer.h>
 #include <util/python/pycppwrapper_util.h>
 
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+#include <dns/rrttl.h>
+#include <dns/rdata.h>
+#include <dns/tsigrecord.h>
+
 #include <acl/dns.h>
 #include <acl/ip_check.h>
 
@@ -49,6 +57,8 @@ using namespace std;
 using boost::scoped_ptr;
 using boost::lexical_cast;
 using namespace isc;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
 using namespace isc::util::python;
 using namespace isc::acl::dns;
 using namespace isc::acl::dns::python;
@@ -59,11 +69,39 @@ namespace dns {
 namespace python {
 
 struct s_RequestContext::Data {
-    // The constructor.  Currently it only accepts the information of the
-    // request source address, and contains all necessary logic in the body
-    // of the constructor.  As it's extended we may have refactor it by
-    // introducing helper methods.
-    Data(const char* const remote_addr, const unsigned short remote_port) {
+    // The constructor.
+    Data(const char* const remote_addr, const unsigned short remote_port,
+         const char* tsig_data, const Py_ssize_t tsig_len)
+    {
+        createRemoteAddr(remote_addr, remote_port);
+        createTSIGRecord(tsig_data, tsig_len);
+    }
+
+    // A convenient type converter from sockaddr_storage to sockaddr
+    const struct sockaddr& getRemoteSockaddr() const {
+        const void* p = &remote_ss;
+        return (*static_cast<const struct sockaddr*>(p));
+    }
+
+    // The remote (source) IP address of the request.  Note that it needs
+    // a reference to remote_ss.  That's why the latter is stored within
+    // this structure.
+    scoped_ptr<IPAddress> remote_ipaddr;
+
+    // The effective length of remote_ss.  It's necessary for getnameinfo()
+    // called from sockaddrToText (__str__ backend).
+    socklen_t remote_salen;
+
+    // The TSIG record included in the request, if any.  If the request
+    // doesn't contain a TSIG, this will be NULL.
+    scoped_ptr<TSIGRecord> tsig_record;
+
+private:
+    // A helper method for the constructor that is responsible for constructing
+    // the remote address.
+    void createRemoteAddr(const char* const remote_addr,
+                          const unsigned short remote_port)
+    {
         struct addrinfo hints, *res;
         memset(&hints, 0, sizeof(hints));
         hints.ai_family = AF_UNSPEC;
@@ -85,20 +123,31 @@ struct s_RequestContext::Data {
         remote_ipaddr.reset(new IPAddress(getRemoteSockaddr()));
     }
 
-    // A convenient type converter from sockaddr_storage to sockaddr
-    const struct sockaddr& getRemoteSockaddr() const {
-        const void* p = &remote_ss;
-        return (*static_cast<const struct sockaddr*>(p));
-    }
-
-    // The remote (source) IP address the request.  Note that it needs
-    // a reference to remote_ss.  That's why the latter is stored within
-    // this structure.
-    scoped_ptr<IPAddress> remote_ipaddr;
+    // A helper method for the constructor that is responsible for constructing
+    // the request TSIG.
+    void createTSIGRecord(const char* tsig_data, const Py_ssize_t tsig_len) {
+        if (tsig_len == 0) {
+            return;
+        }
 
-    // The effective length of remote_ss.  It's necessary for getnameinf()
-    // called from sockaddrToText (__str__ backend).
-    socklen_t remote_salen;
+        // Re-construct the TSIG record from the passed binary.  This should
+        // normally succeed because we are generally expected to be called
+        // from the frontend .py, which converts a valid TSIGRecord in its
+        // wire format.  If some evil or buggy python program directly calls
+        // us with bogus data, validation in libdns++ will trigger an
+        // exception, which will be caught and converted to a Python exception
+        // in RequestContext_init().
+        isc::util::InputBuffer b(tsig_data, tsig_len);
+        const Name key_name(b);
+        const RRType tsig_type(b.readUint16());
+        const RRClass tsig_class(b.readUint16());
+        const RRTTL ttl(b.readUint32());
+        const size_t rdlen(b.readUint16());
+        const ConstRdataPtr rdata = createRdata(tsig_type, tsig_class, b,
+                                                rdlen);
+        tsig_record.reset(new TSIGRecord(key_name, tsig_class, ttl,
+                                         *rdata, 0));
+    }
 
 private:
     struct sockaddr_storage remote_ss;
@@ -145,31 +194,41 @@ RequestContext_init(PyObject* po_self, PyObject* args, PyObject*) {
     s_RequestContext* const self = static_cast<s_RequestContext*>(po_self);
 
     try {
-        // In this initial implementation, the constructor is simply: It
-        // takes a single parameter, which should be a Python socket address
-        // object.  For IPv4, it's ('address test', numeric_port); for IPv6,
+        // In this initial implementation, the constructor is simple: It
+        // takes two parameters.  The first parameter should be a Python
+        // socket address object.
+        // For IPv4, it's ('address test', numeric_port); for IPv6,
         // it's ('address text', num_port, num_flowid, num_zoneid).
+        // The second parameter is wire-format TSIG record in the form of
+        // Python byte data.  If the TSIG isn't included in the request,
+        // its length will be 0.
         // Below, we parse the argument in the most straightforward way.
         // As the constructor becomes more complicated, we should probably
         // make it more structural (for example, we should first retrieve
-        // the socket address as a PyObject, and parse it recursively)
+        // the python objects, and parse them recursively)
 
         const char* remote_addr;
         unsigned short remote_port;
         unsigned int remote_flowinfo; // IPv6 only, unused here
         unsigned int remote_zoneid; // IPv6 only, unused here
-
-        if (PyArg_ParseTuple(args, "(sH)", &remote_addr, &remote_port) ||
-            PyArg_ParseTuple(args, "(sHII)", &remote_addr, &remote_port,
-                             &remote_flowinfo, &remote_zoneid))
+        const char* tsig_data;
+        Py_ssize_t tsig_len;
+
+        if (PyArg_ParseTuple(args, "(sH)y#", &remote_addr, &remote_port,
+                             &tsig_data, &tsig_len) ||
+            PyArg_ParseTuple(args, "(sHII)y#", &remote_addr, &remote_port,
+                             &remote_flowinfo, &remote_zoneid,
+                             &tsig_data, &tsig_len))
         {
-            // We need to clear the error in case the first call to PareTuple
+            // We need to clear the error in case the first call to ParseTuple
             // fails.
             PyErr_Clear();
 
             auto_ptr<s_RequestContext::Data> dataptr(
-                new s_RequestContext::Data(remote_addr, remote_port));
-            self->cppobj = new RequestContext(*dataptr->remote_ipaddr);
+                new s_RequestContext::Data(remote_addr, remote_port,
+                                           tsig_data, tsig_len));
+            self->cppobj = new RequestContext(*dataptr->remote_ipaddr,
+                                              dataptr->tsig_record.get());
             self->data_ = dataptr.release();
             return (0);
         }
@@ -224,7 +283,11 @@ RequestContext_str(PyObject* po_self) {
         objss << "<" << requestcontext_type.tp_name << " object, "
               << "remote_addr="
               << sockaddrToText(self->data_->getRemoteSockaddr(),
-                                self->data_->remote_salen) << ">";
+                                self->data_->remote_salen);
+        if (self->data_->tsig_record) {
+            objss << ", key=" << self->data_->tsig_record->getName();
+        }
+        objss << ">";
         return (Py_BuildValue("s", objss.str().c_str()));
     } catch (const exception& ex) {
         const string ex_what =
@@ -248,7 +311,7 @@ namespace python {
 // Most of the functions are not actually implemented and NULL here.
 PyTypeObject requestcontext_type = {
     PyVarObject_HEAD_INIT(NULL, 0)
-    "isc.acl.dns.RequestContext",
+    "isc.acl._dns.RequestContext",
     sizeof(s_RequestContext),                 // tp_basicsize
     0,                                  // tp_itemsize
     RequestContext_destroy,             // tp_dealloc
@@ -266,7 +329,7 @@ PyTypeObject requestcontext_type = {
     NULL,                               // tp_getattro
     NULL,                               // tp_setattro
     NULL,                               // tp_as_buffer
-    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE, // tp_flags
     RequestContext_doc,
     NULL,                               // tp_traverse
     NULL,                               // tp_clear

src/lib/python/isc/acl/dns_requestloader_python.cc

@@ -171,7 +171,7 @@ namespace python {
 // Most of the functions are not actually implemented and NULL here.
 PyTypeObject requestloader_type = {
     PyVarObject_HEAD_INIT(NULL, 0)
-    "isc.acl.dns.RequestLoader",
+    "isc.acl._dns.RequestLoader",
     sizeof(s_RequestLoader),                 // tp_basicsize
     0,                                  // tp_itemsize
     RequestLoader_destroy,       // tp_dealloc
@@ -189,7 +189,7 @@ PyTypeObject requestloader_type = {
     NULL,                               // tp_getattro
     NULL,                               // tp_setattro
     NULL,                               // tp_as_buffer
-    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE, // tp_flags
     RequestLoader_doc,
     NULL,                               // tp_traverse
     NULL,                               // tp_clear

src/lib/python/isc/acl/tests/Makefile.am

@@ -19,7 +19,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
-	env PYTHONPATH=$(abs_top_builddir)/src/lib/isc/python/acl/.libs:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python \
+	env PYTHONPATH=$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/isc/python/acl/.libs:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python \
 	$(LIBRARY_PATH_PLACEHOLDER) \
 	$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
 	done

src/lib/python/isc/acl/tests/dns_test.py

@@ -15,6 +15,7 @@
 
 import unittest
 import socket
+from pydnspp import *
 from isc.acl.acl import LoaderError, Error, ACCEPT, REJECT, DROP
 from isc.acl.dns import *
 
@@ -39,12 +40,37 @@ def get_acl_json(prefix):
     json[0]["from"] = prefix
     return REQUEST_LOADER.load(json)
 
-def get_context(address):
+# The following two are similar to the previous two, but use a TSIG key name
+# instead of IP prefix.
+def get_tsig_acl(key):
+    return REQUEST_LOADER.load('[{"action": "ACCEPT", "key": "' + \
+                                   key + '"}]')
+
+def get_tsig_acl_json(key):
+    json = [{"action": "ACCEPT"}]
+    json[0]["key"] = key
+    return REQUEST_LOADER.load(json)
+
+# commonly used TSIG RDATA.  For the purpose of ACL checks only the key name
+# matters; other parrameters are simply borrowed from some other tests, which
+# can be anything for the purpose of the tests here.
+TSIG_RDATA = TSIG("hmac-md5.sig-alg.reg.int. 1302890362 " + \
+                      "300 16 2tra2tra2tra2tra2tra2g== " + \
+                      "11621 0 0")
+
+def get_context(address, key_name=None):
     '''This is a simple shortcut wrapper for creating a RequestContext
-    object with a given IP address.  Port number doesn't matter in the test
-    (as of the initial implementation), so it's fixed for simplicity.
+    object with a given IP address and optionally TSIG key  name.
+    Port number doesn't matter in the test (as of the initial implementation),
+    so it's fixed for simplicity.
+    If key_name is not None, it internally creates a (faked) TSIG record
+    and constructs a context with that key.  Note that only the key name
+    matters for the purpose of ACL checks.
     '''
-    return RequestContext(get_sockaddr(address, 53000))
+    tsig_record = None
+    if key_name is not None:
+        tsig_record = TSIGRecord(Name(key_name), TSIG_RDATA)
+    return RequestContext(get_sockaddr(address, 53000), tsig_record)
 
 # These are commonly used RequestContext object
 CONTEXT4 = get_context('192.0.2.1')
@@ -63,6 +89,21 @@ class RequestContextTest(unittest.TestCase):
                          RequestContext(('2001:db8::1234', 53006,
                                          0, 0)).__str__())
 
+        # Construct the context from IP address and a TSIG record.
+        tsig_record = TSIGRecord(Name("key.example.com"), TSIG_RDATA)
+        self.assertEqual('<isc.acl.dns.RequestContext object, ' + \
+                             'remote_addr=[192.0.2.1]:53001, ' + \
+                             'key=key.example.com.>',
+                         RequestContext(('192.0.2.1', 53001),
+                                        tsig_record).__str__())
+
+        # same with IPv6 address, just in case.
+        self.assertEqual('<isc.acl.dns.RequestContext object, ' + \
+                             'remote_addr=[2001:db8::1234]:53006, ' + \
+                             'key=key.example.com.>',
+                         RequestContext(('2001:db8::1234', 53006,
+                                         0, 0), tsig_record).__str__())
+
         # Unusual case: port number overflows (this constructor allows that,
         # although it should be rare anyway; the socket address should
         # normally come from the Python socket module.
@@ -89,7 +130,9 @@ class RequestContextTest(unittest.TestCase):
         # not a tuple
         self.assertRaises(TypeError, RequestContext, 1)
         # invalid number of parameters
-        self.assertRaises(TypeError, RequestContext, ('192.0.2.1', 53), 0)
+        self.assertRaises(TypeError, RequestContext, ('192.0.2.1', 53), 0, 1)
+        # type error for TSIG
+        self.assertRaises(TypeError, RequestContext, ('192.0.2.1', 53), tsig=1)
         # tuple is not in the form of sockaddr
         self.assertRaises(TypeError, RequestContext, (0, 53))
         self.assertRaises(TypeError, RequestContext, ('192.0.2.1', 'http'))
@@ -159,10 +202,22 @@ class RequestACLTest(unittest.TestCase):
         self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           [{"action": "ACCEPT", "from": []}])
         self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "ACCEPT", "key": 1}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "key": 1}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          '[{"action": "ACCEPT", "key": {}}]')
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "key": {}}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "ACCEPT", "from": "bad"}]')
         self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           [{"action": "ACCEPT", "from": "bad"}])
         self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "key": "bad..name"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
+                          [{"action": "ACCEPT", "key": "bad..name"}])
+        self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           '[{"action": "ACCEPT", "from": null}]')
         self.assertRaises(LoaderError, REQUEST_LOADER.load,
                           [{"action": "ACCEPT", "from": None}])
@@ -237,6 +292,28 @@ class RequestACLTest(unittest.TestCase):
         self.assertEqual(REJECT, get_acl('32.1.13.184').execute(CONTEXT6))
         self.assertEqual(REJECT, get_acl_json('32.1.13.184').execute(CONTEXT6))
 
+        # TSIG checks, derived from dns_test.cc
+        self.assertEqual(ACCEPT, get_tsig_acl('key.example.com').\
+                             execute(get_context('192.0.2.1',
+                                                 'key.example.com')))
+        self.assertEqual(REJECT, get_tsig_acl_json('key.example.com').\
+                             execute(get_context('192.0.2.1',
+                                                 'badkey.example.com')))
+        self.assertEqual(ACCEPT, get_tsig_acl('key.example.com').\
+                             execute(get_context('2001:db8::1',
+                                                 'key.example.com')))
+        self.assertEqual(REJECT, get_tsig_acl_json('key.example.com').\
+                             execute(get_context('2001:db8::1',
+                                                 'badkey.example.com')))
+        self.assertEqual(REJECT, get_tsig_acl('key.example.com').\
+                             execute(CONTEXT4))
+        self.assertEqual(REJECT, get_tsig_acl_json('key.example.com').\
+                             execute(CONTEXT4))
+        self.assertEqual(REJECT, get_tsig_acl('key.example.com').\
+                             execute(CONTEXT6))
+        self.assertEqual(REJECT, get_tsig_acl_json('key.example.com').\
+                             execute(CONTEXT6))
+
         # A bit more complicated example, derived from resolver_config_unittest
         acl = REQUEST_LOADER.load('[ {"action": "ACCEPT", ' +
                                   '     "from": "192.0.2.1"},' +