BIND 10 #998: IP based ACL check

[BIND 10 Development]

#998: IP based ACL check
-------------------------------------+-------------------------------------
                   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:  5.0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:9 stephen]:

 > > Third: maybe this is premature at this point, but I guess Ip(vX)Check
 will need to contain the information of "which address it is", such as the
 source address or destination address.
 [...]
 > The class as implemented here is only a single ACL match (or non-match -
 the capability to check for a non-match was trivial, which is why I added
 it to this class).  The ACL you are suggesting in the example is really a
 combined ACL:
 > {{{
 > allow (source matches 2001:db8:1::/64) AND (dest matches
 2001:db8:2::/64)
 > }}}
 > I would see then implementation as being a class containing two IPCheck
 classes, one for source and one for destination. It would be the container
 class that knows the structure of the packet and distinguishes between
 source and destination address.
 >
 > However, consider the possible ACL:
 > {{{
 > allow (source matches 2001:db8:1::/64) AND (protocol is TCP)
 > }}}
 > (I don't know whether we would want to allow access based on protocol,
 but as it is a possibility so I think we should consider it.)  In this
 case a more general class allowing the AND between two ACLs is called for.

 Do you mean something like the one I mentioned in my response to Michal?
 http://bind10.isc.org/ticket/998?replyto=9#comment:11

 If so, I actually meant that type of concept in my original comment
 (the above response of yours seems to indicate I was suggesting a
 single IPCheck class to support the compound "(source..) AND
 (dest...)" rule.  If so, no that's not what I meant).

 >  Applying this to the IP check problem, an alternative approach to a
 single class checking both source and destination addresses could be to
 derive SourceIPCheck and DestinationIPCheck classes from IPCheck and
 combine them using the AND operator.  (All the derived classes would do is
 provide a matches() method to pass the source/destination IP address from
 the packet to the parent IPCheck::matches())

 Functionality wise, these two options wouldn't be so different
 - store the source/dest information in a single IPCheck class and
   introduce IPCheck::getAddressType() method to return that
   information
 - define SourceIPCheck and DestinationIPCheck classes as you mentioned
   above

 Design wise, I'd generally be careful about adding more
 characteristics by creating more and more specialized subclasses
 because that approach would easily cause an explosion of specialized
 classes.  This is what I was afraid about in my comment for #977:
 http://bind10.isc.org/ticket/977#comment:5

 For example, hypothetically assume we still separated classes for IPv4
 and IPv6.  Then this approach would easily result in having 4 classes:
 SourceIPv4Check, SourceIPv6Check, DestinationIPv4Check,
 DestinationIPv6Check.  It can quite easily lead to an unmanageable
 level.

 Of course, the other approach could result in a monster, monolithic
 class, which should also be avoided.  So it's a matter of balance,
 and, in this specific case, I'd keep a single class approach.

 > > extend the compare() method (or something equivalent to that if
 addressing the first point affects this part of interface) so that it can
 take different types of addresses
 > Let's discuss this further.  As noted above, I think that this can be
 implemented by the planned logical operator ACLs.

 Hmm, maybe I confused you here...in this context by "different types
 of address" I meant IPv4 or IPv6, not source or destination.  You seem
 to interpret it as meaning source vs destination?

 > Thanks for the comprehensive review.

 You're welcome, but actually it was not "comprehensive" yet:-)

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