BIND 10 #1573: auth::Query needs to return DS for secure delegation

[BIND 10 Development]

#1573: auth::Query needs to return DS for secure delegation
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120207
  critical                           |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  4
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 First, I made a couple of trivial editorial fixes.

 '''query.cc'''

 - I'd suggest rename parameter name for `ZoneFinder` from "zone" to
   "finder", partly for consistency, and partly because it's more
   appropriate in terms of what it is.
 - If DS isn't found (and the zone is signed with NSEC) we need to add
   NSEC (maybe it was not clear from the ticket description, if so,
   sorry about that).  I cannot find normative text about this in
   RFC4035 (it only provides an example in an appendix), but that's how
   BIND 9 works, and apparently so do all root servers.  Note also that
   the header file documentation would also have to be updated.
 - We should also probably handle a pathological case where find(DS)
   results in neither SUCCESS or NXRRSET (return SERVFAIL?)

 '''unittest'''

 - This comment doesn't seem to be very correct.  At least the answer
   section is empty, and I don't think anything is omitted in this
   case:
 {{{#!c++
     // find match rrset, omit additional data which has already been
 provided
     // in the answer section from the additional.
 }}}
 - We also need to test the case when DS isn't found (insecure
   delegation)
 - We also need to test the pathological case where find(DS) results in
   neither SUCCESS or NXRRSET.
 - As for the check for the additional section, we could at least
   reduce the amount of hardcoding a bit:
 {{{#!c++
                   (string("glue.delegation.example.com. 3600 IN RRSIG ") +
                    getCommonRRSIGText("A") + "\n" +
                    string(ns_addrs_txt) +
                    string("glue.delegation.example.com. 3600 IN RRSIG ") +
                    getCommonRRSIGText("AAAA") + "\n" +
                    string("noglue.example.com. 3600 IN RRSIG ") +
                    getCommonRRSIGText("A")).c_str());
 }}}
  (there's one tricky point, though: we need to place ns_addrs_txt
   between the glue A RRSIG and AAAA RRSIG; otherwise masterLoad()
   would be confused.  However, I believe #1614 will solve this issue
   too)
 - Not really related to this branch, but I noticed one oddity in the
   additional section: glue RRs should normally not have RRSIGs.  But
   it's probably okay for the Query class to include them if it happens
   to be passed such odd RRSIGs.

 '''other'''

 maybe we need a changelog for this?

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