BIND 10 #1104: support TSIG in DNS (Request) ACL

[BIND 10 Development]

#1104: support TSIG in DNS (Request) ACL
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110816
                  Component:         |            Resolution:
  xfrout                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:16 stephen]:
 > First off, I agree with the suggestion to close this ticket and open a
 new one.

 > > I don't think it a good idea to deceive the compiler with mutable in
 this case. When we set the "verified" flag of TSIGRecord, it actually
 changes externally observable behavior
 > Only if we are allowing for the possibility that the contents of the
 TSIG keyring may have changed between two calls to verify().

 It can also happen if we call verify() multiple times for the same
 record (with different data and/or TSIG context).  Admittedly these
 are an unlikely scenario in practice, but IMO we should not rely on
 likely common cases in designing an API.

 Also, while it was my own idea I now think it may not be that a good
 idea to have TSIGRecord a state of "validated", because such a state
 cannot be well defined with the record itself (we need data and
 context).

 > > IMO, if an application wants to handle TSIG, it should explicitly do
 it at an appropriate position of the code.
 > But there is a separation between ACLs and the application; the ACL is
 set by the user, the packet is checked against it, then the (rest of the)
 application is invoked.  If we restrict the TSIG check to just the name,
 we leave open the possibility that an incorrect packet may get through the
 check.

 RequestContext is constructed with TSIGRecord (or it may have to be
 TSIGContext according to the discussion of the previous paragraph),
 not a particular name.  So, it's impossible even for a naive/buggy
 implementation to accidentally bypass the ACL with TSIG validation if
 we add some kind of "validated-or-not" state to TSIGRecord or
 TSIGContext and have RequestContext check that on construction.

 That is, a good and complete implementation would do this:

 {{{
     tsig = message->getTSIGRecord();
     ctx.verify(tsig, data, data_len);
     acl.execute(RequestContext(from_addr, tsig));
 }}}

 A good bug incomplete (missing TSIG support) would do this:

 {{{
     acl.execute(RequestContext(from_addr, NULL));
 }}}

 A naive application would do this:

 {{{
     acl.execute(RequestContext(from_addr, message->getTSIGRecord()));
 }}}

 But even in the last case, the ACL check would identify the record has
 not been verified and avoid accidental match.

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