[trac1069] introduce "RequestCheckCreator", the factory for the

default check of the DNS ACLs.  match logic is not implemented yet.

src/lib/acl/dns.cc

@@ -12,20 +12,85 @@
 // 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 {
+template <>
+bool
+IPCheck<dns::RequestContext>::matches(const dns::RequestContext&) const {
+    return (false);
+}
+
 namespace dns {
 
-Loader&
+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&
 getLoader() {
-    static Loader* loader(NULL);
+    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);
 }
 

src/lib/acl/dns.h

@@ -15,7 +15,15 @@
 #ifndef ACL_DNS_H
 #define ACL_DNS_H
 
-#include "loader.h"
+#include <string>
+#include <vector>
+
+#include <boost/shared_ptr.hpp>
+
+#include <cc/data.h>
+
+#include <acl/ip_check.h>
+#include <acl/loader.h>
 
 #include <asiolink/io_address.h>
 #include <dns/message.h>
@@ -64,13 +72,13 @@ struct RequestContext {
 };
 
 /// \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 +88,37 @@ 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& getLoader();
+
+// 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); }
+};
+}
 
 }
 }
 }
 
 #endif
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/acl/tests/dns_test.cc

@@ -12,10 +12,25 @@
 // 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 <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 <gtest/gtest.h>
 
+using namespace std;
+using namespace isc::data;
 using namespace isc::acl::dns;
+using isc::acl::LoaderError;
 
 namespace {
 
@@ -23,13 +38,86 @@ namespace {
 // time and the returned value can be used to anything. It is not much of a
 // test, but the getLoader is not much of a function.
 TEST(DNSACL, getLoader) {
-    Loader* l(&getLoader());
+    RequestLoader* l(&getLoader());
     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_NO_THROW(l->load(Element::fromJSON("[{\"action\": \"DROP\"}]")));
+
+    // Test check rules registered by default, i.e. RequestCheck
+    EXPECT_NO_THROW(l->load(Element::fromJSON("[{\"action\": \"DROP\","
+                                              "  \"from\": \"192.0.2.1\"}]")));
+}
+
+class RequestCheckCreatorTest : public ::testing::Test {
+protected:
+    internal::RequestCheckCreator creator_;
+
+    typedef boost::shared_ptr<const 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\""),
+                             getLoader());
+    const internal::RequestIPCheck& ipcheck_ =
+        dynamic_cast<const 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\""),
+                             getLoader());
+    const internal::RequestIPCheck& ipcheck_ =
+        dynamic_cast<const 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\""),
+                                 getLoader()), LoaderError);
+
+    // Invalid type of parameter
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("4"), getLoader()),
+                 isc::data::TypeError);
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("[]"), getLoader()),
+                 isc::data::TypeError);
+
+    // Syntax error for IPCheck
+    EXPECT_THROW(creator_.create("from", Element::fromJSON("\"bad\""),
+                                 getLoader()),
+                 isc::InvalidParameter);
+
+    // NULL pointer
+    EXPECT_THROW(creator_.create("from", ConstElementPtr(), getLoader()),
+                 LoaderError);
 }
 
 }