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 stephen):

 > Michal:
 > The non-match, I guess, will not be used, for consistency. We probably
 will have a general not, so having a different syntax for not on IP seems
 strange.
 Fair enough.  I added the negation because I wasn't certain on this, and
 the code for it was trivial.  It's easy enough to remove it (or leave it
 in - if the "inverse" argument is not specified, it does a positive
 match).

 > Michal:
 > But I don't see a way to say „use this context and extract the source
 address“ for one version and „use the same context, but extract the
 destination address“.
 See discussion below.

 > Jinmei:
 > My understanding is that it's expected to be very Context dependent, and
 no generic template is provided (it's the Context class author's
 responsibility to give the implementation).
 The matches() method has not been written because as yet, the "Context"
 object holding the address is not specified.  I didn't want to guess at
 some structure then require the code do some conversion from its own
 representation to that structure when the data could have been directly
 extracted from the representation.

 > Michal:
 > Won't that be confusing, to have methods of single class scattered over
 multiple files?
 When the Context type has been decided, then we write the matches() code
 (and probably place it in the same file as the rest of the class).


 > Jinmei
 > 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
 I'm not sure that it's adding more characteristics. My argument for the
 dual-class approach is that at present the class has a single purpose -
 match an IP address.  It will do that in the absence of any context or
 knowledge of how the address will be used.  (So, as you suggest, it could
 also be used in filtering A/AAAA RDATA where the use of that data is not
 defined.)

 To extract source/destination IP information really requires some form of
 adapter to process the information before it gets to the matches() method
 rather than to have knowledge in the class itself.  This transformation is
 provided by the subclass.

 > Jinmei:
 > The problem is that it wouldn't be so obvious how to extract the IP
 address to match from the Context.
 That really summarises where we are at the moment.  Once we have
 "Context", the final elements of the code can be completed.

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