BIND 10 #999: Integrate ACLs into b10-resolver

[BIND 10 Development]

#999: Integrate ACLs into b10-resolver
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110628
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  4.0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 I agree with putting the unification work into a followup ticket, but I
 think it should happen soon, because it will allow adding more types of
 checks easily and it already contains superset of the syntax your loader
 is able to load (for example it can have condition-less actions to modify
 the „default“ result, after merging two tickets currently in review, it
 will support shorter syntax).

 Anyway, the current code fails to compile for me:
 {{{
 libtool: link: g++ -Wall -Wextra -Wwrite-strings -Woverloaded-virtual
 -Wno-sign-compare -Werror -fPIC -g -O0 -o .libs/run_unittests
 run_unittests-run_unittests.o run_unittests-check_test.o run_unittests-
 acl_test.o run_unittests-loader_test.o run_unittests-dns_test.o
 run_unittests-ip_check_unittest.o -pthread  -L/usr/lib64 -lgtest
 ../../../../src/lib/util/.libs/libutil.so
 ../../../../src/lib/util/unittests/.libs/libutil_unittests.so
 /home/vorner/work/bind10/src/lib/util/io/.libs/libutil_io.so
 ../../../../src/lib/acl/.libs/libacl.so
 ../../../../src/lib/cc/.libs/libcc.so
 ../../../../src/lib/exceptions/.libs/libexceptions.so
 ../../../../src/lib/acl/.libs/libdnsacl.so -L/usr/lib
 /home/vorner/work/bind10/src/lib/acl/.libs/libacl.so
 /home/vorner/work/bind10/src/lib/cc/.libs/libcc.so
 /home/vorner/work/bind10/src/lib/dns/.libs/libdns++.so
 /home/vorner/work/bind10/src/lib/cryptolink/.libs/libcryptolink.so -lbotan
 -lbz2 -lcrypto /usr/lib64/libgmp.so -lpthread -lrt -lz
 ../../../../src/lib/log/.libs/liblog.so -L/opt/log4cplus/lib
 /opt/log4cplus/lib/liblog4cplus.so
 /home/vorner/work/bind10/src/lib/util/.libs/libutil.so
 /home/vorner/work/bind10/src/lib/exceptions/.libs/libexceptions.so
 -pthread -Wl,-rpath -Wl,/home/vorner/testing/bind10/lib -Wl,-rpath
 -Wl,/opt/log4cplus/lib -Wl,-rpath -Wl,/usr/lib
 run_unittests-ip_check_unittest.o: In function
 `isc::acl::IPCheck<(anonymous namespace)::GeneralAddress>::getAddress()
 const':
 /home/vorner/work/bind10/src/lib/acl/tests/../../../../src/lib/acl/ip_check.h:258:
 undefined reference to `isc::acl::IPCheck<(anonymous
 namespace)::GeneralAddress>::IPV4_SIZE'
 /home/vorner/work/bind10/src/lib/acl/tests/../../../../src/lib/acl/ip_check.h:258:
 undefined reference to `isc::acl::IPCheck<(anonymous
 namespace)::GeneralAddress>::IPV6_SIZE'
 run_unittests-ip_check_unittest.o: In function
 `isc::acl::IPCheck<(anonymous namespace)::GeneralAddress>::getMask()
 const':
 /home/vorner/work/bind10/src/lib/acl/tests/../../../../src/lib/acl/ip_check.h:264:
 undefined reference to `isc::acl::IPCheck<(anonymous
 namespace)::GeneralAddress>::IPV4_SIZE'
 /home/vorner/work/bind10/src/lib/acl/tests/../../../../src/lib/acl/ip_check.h:264:
 undefined reference to `isc::acl::IPCheck<(anonymous
 namespace)::GeneralAddress>::IPV6_SIZE'
 run_unittests-ip_check_unittest.o: In function
 `isc::acl::IPCheck<(anonymous namespace)::GeneralAddress>::setMask(int)':
 /home/vorner/work/bind10/src/lib/acl/tests/../../../../src/lib/acl/ip_check.h:362:
 undefined reference to `isc::acl::IPCheck<(anonymous
 namespace)::GeneralAddress>::IPV4_SIZE'
 /home/vorner/work/bind10/src/lib/acl/tests/../../../../src/lib/acl/ip_check.h:362:
 undefined reference to `isc::acl::IPCheck<(anonymous
 namespace)::GeneralAddress>::IPV6_SIZE'
 collect2: ld returned 1 exit status
 make[5]: *** [run_unittests] Error 1
 make[5]: Leaving directory `/home/vorner/work/bind10/src/lib/acl/tests'
 }}}

 And, finally, some comments about the code:
  * In the `io_endpoint_unittest`, you introduced shared pointers. Wouldn't
 `auto_ptr` be enough? But if you like shared pointers more, I think it is
 OK in test, there's no performance problem in it.
  * `EXPECT_THROW(IPAddress ipaddr(sa), isc::BadValue);` ‒ this doesn't
 need the (unused) variable ipaddr, it should be enough to have
 `EXPECT_THROW(IPAddress(sa), isc::BadValue);`
  * Should the `IPCheck<Client>::matches` really be in
 `server_common/client.{cc,h}`?
  * Should we test localhost addresses as well? (and in the following test)
 They are default, so to check that they are default at the right place.
 {{{#!C++
 TEST_F(ResolverConfig, defaultQueryACL) {
     // If no configuration is loaded, the default ACL should reject
 everything.
 }}}
  * The name `compoundQueryAcl` might be confusing, considering that we
 have compound ACL checks. This ACL contains multiple entries, but none of
 them is compound in that sense. Would be `longerAcl` or `multiEntryACL` be
 acceptable?
  * You enable debug logging in tests. But should we spam the outputs with
 the debug logging?

 And, do we plan to support checking the receiving IP address and the
 „from“ port in future (surely not in this ticket)?

-- 
Ticket URL: <http://bind10.isc.org/ticket/999#comment:12>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development