BIND 10 #1310: auth NSEC support: Handle WILDCARD_NXRRSET case

[BIND 10 Development]

#1310: auth NSEC support: Handle WILDCARD_NXRRSET case
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  kevin_tes
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111122
                  Component:         |            Resolution:
  b10-auth                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  3
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:         |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => kevin_tes


Comment:

 '''src/bin/auth/query.cc'''[[BR]]
 Indentation of addWildcardNxrrsetProof() uses tabs - it should use spaces.

 The first block of comments in addWildcardNxrrsetProof() do not apply to
 the immediately following statements, but to the block of statements after
 that - the one starting:
 {{{
 const ZoneFinder::FindResult fresult =
 }}}

 As far as I can determine, the first "if" block:
 {{{
 if (nsec->getRdataCount() == 0) {
 }}}
 is a check that the the first NSEC RR in 3.1.3.4 - the one that proves
 there are no RRsets of the STYPE requested at the name that matched
 <SNAME, SCLASS> via wildcard expansion - is present in the "nsec" RRset.
 The reason for this check should be commented.

 Adding the NSEC RRsets to the Authority section is done via the construct:
 {{{
 if names are equal {
    add fresult.rrset
 } else {
    add nsec
    add fresult.rrset
 }
 }}}
 The "add fresult rrset" is common to both branches of the "if" statement
 and so can be outside it.

 If the above change is made, then part of the processing in
 addWildcardNxrrsetProof() is identical to the processing in
 addWildcardProof().  It suggests that the two could be combined: add a
 second argument to addWildcardProof() that defaults to an empty
 ConstRRsetPtr.  The code would then:
 * add the standard wildcard NSEC record
 * if "nsec" is not null:
   * test if it contains any NSEC records; if not, thrown an exception
   * add to the Authority section if the name is different from the
 standard wildcard record


 '''src/bin/auth/query.h'''[[BR]]
 The header for addWildcardNxrrsetProof() should include Doxygen tags
 describing the arguments.

 Although not really part of this ticket, while addressing the above
 comment the opportunity should be taken to add Doxygen tags to the headers
 of the other "addXxx()" methods.

 The headers of these methods should be expanded to do a better description
 as to how the methods are used.  It took quite a bit of searching (and
 reading [http://tools.ietf.org/hrml/rfc4035 RFC 4035]) to find out how the
 logic worked (i.e. that addWildcardNxrrsetProof() depends on the fact that
 ZoneFinder::find() in this case - Wildcard No Data - returns the NSEC that
 covers the interval where the empty non-terminal lives, but not the NSEC
 for the wildcard demanded by [http://tools.ietf.org/hrml/rfc4035 RFC
 4035], thus requiring the addition of the wildcard NSEC).


 '''src/bin/auth/tests/query_unittest.cc'''[[BR]]
 There should be two tests: one for the case that returns two NSEC records,
 and one that returns a single NSEC record (to prove that the duplicate
 NSEC is not included in the Authority section).

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