BIND 10 #772: Update xfrout to use ACL checking library

[BIND 10 Development]

#772: Update xfrout to use ACL checking library
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110802
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  xfrout                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  3.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:7 jinmei]:
 > '''general'''
 > - I'm not sure if we should reject incoming AXFR request by default.
 >   At least it's different from BIND 9's default.  It will also break
 >   current deployment of BIND 10 (although I think it's acceptable at
 >   the current maturity of BIND 10 as long as we warn it in a release
 >   note or something).

 I'm not sure about that either. I'm OK with any reasonable default, I just
 thought this one is the safest one. What do you think should be the
 default?

 > '''misc in xfrout.py.in'''
 > - parse_query_message(): shouldn't we skip performing ACL if TSIG
 >   check fails?  In fact, (although quite unlikely) if the fail is MAC

 Ah, right, I overlooked it. This should be now fixed and tested.

 > - update_config_data: just checking, is it okay to not to protect it
 >   by lock?  maybe we should add comment about that.

 To be honest, I have no idea here. XfrOut hits some kind of mental block
 in my mind when trying to understand what part of code does exactly what,
 what does run in which thread, etc. It wasn't in one before, though.

 I'm quite sure the update can happen only one at a time, since the CC loop
 runs only one, but it might be possible some other thread reads them while
 being updated. Should we open a ticket to investigate it?

 > - The trick employed in this function seems to be the best current
 >   practice, but I'm afraid it's fragile.  I wouldn't be surprised
 >   there's a system that is more strict about the AF matching, for
 >   example.  For longer term, I guess we should pass some auxiliary
 >   information (at least the AF, socktype and protocol, and possibly
 >   local/remote addresses) when we pass a socket FD from one process to
 >   another.

 Hmm, maybe. I wanted to pass the addresses when sending it, but I found
 out the current auth side has no tests and it would require changes in a
 lot of places to pass it around, so I decided this might be easier.

 Anyway, it should be tested, so I suggest we keep this simpler approach
 and let the test catch it, if the problem exists anywhere.

 > - As for this:
 > {{{
 >             # To make it work even on hosts without IPv6 support
 >             # (Any idea how to simulate this in test?)
 > }}}
 >   It seems we can override socket.has_ipv6.  So a test could tweak it
 >   before testing.  (but we might not bother to do this minor case -
 >   today's many system already supports IPv6, and if we implement the
 >   "longer term" solution in not far future, the problem will be gone
 >   anyway)

 Overriding it doesn't really test anything. Such system still has IPv6
 support, I'd like to test it with something that really doesn't have IPv6
 support. Maybe we have a buildbot without IPv6?

 > '''changelog'''
 > As discussed in 'general', if we keep rejecting requests by default,
 > we should note it more explicitly that it's a backward incompatible
 > change (and we should mark it with '*').

 OK, depends on what we decide to do about that.

 And with the addresses for logs, I think it might be nice thing to have,
 but I'd rather like to put it into a different task.

 Thank you

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