BIND 10 #1431: NSEC3: closest provable encloser proof

[BIND 10 Development]

#1431: NSEC3: closest provable encloser proof
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20120124
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  6
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:  NSEC3  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:17 jinmei]:
 > As for the explicit Hints class, on thinking about it further, I guess
 > we might (eventually) do something like this:
 >
 > [...]
 >
 > This way we can also avoid the need for down cast (dynamic_cast) and
 > can make the interface type-safer.

 This would indeed make the code slightly cleaner design-wise. However, I
 fear that it would be harder to use (as the caller would need to
 distinguish if a NULL was returned and if not, call the method on hints
 instead of the data source). Also, the code inside the data source would
 get duplicated and we would need to unify it, adding another layer the
 reader would need to get through. I'm not sure if avoiding one dynamic
 cast is worth this much.

 > Would something like this make sense?  If so, what I'd propose for
 > this ticket is:
 >
 > - introduce the flag field to FindResult and update the find()
 >   interface so that if the zone is signed with NSEC/NSEC3 the
 >   corresponding flags are set, and same for wildcard.
 > - deprecate WILDCARD_xxx and have the caller refer to this flag
 > - remove the idea of returning NSEC3 RRset from find() and allowing
 >   the caller to use it for findNSEC3() for now.  The idea of
 >   FindContext will be big enough (and non urgent), so it's probably
 >   better to postpone it.

 ACK, this direction looks good, no matter if we do it the context or hints
 way.

 > > Also, I'd have few code-level details:
 > >
 > > Indentation:
 > > {{{
 > > +        /// TBD
 > > +       virtual FindNSEC3Result
 > > +        findNSEC3(const isc::dns::Name& name, bool recursive,
 > > +                  const isc::dns::ConstRRsetPtr known_encloser);
 > > +
 > > }}}

 Actually, one indentation problem was left (there were tabs for some
 reason), and I fixed it.

 Replying to [comment:19 jinmei]:
 > Replying to [comment:17 jinmei]:
 >
 > > Would something like this make sense?  If so, what I'd propose for
 > > this ticket is:
 > >
 > > - introduce the flag field to FindResult and update the find()
 > >   interface so that if the zone is signed with NSEC/NSEC3 the
 > >   corresponding flags are set, and same for wildcard.
 > > - deprecate WILDCARD_xxx and have the caller refer to this flag
 > > - remove the idea of returning NSEC3 RRset from find() and allowing
 > >   the caller to use it for findNSEC3() for now.  The idea of
 > >   FindContext will be big enough (and non urgent), so it's probably
 > >   better to postpone it.
 >
 > I've updated the branch toward this direction, but not completely
 > deprecated WILDCARD_xxx as it would require rather big changes to the
 > existing database data source implementation.  For now, I introduced a
 > quick hack wrapper in the auth Query code to convert the old format to
 > the new one.
 >
 > If this approach is okay I'll create a separate ticket for the
 > subsequent refactoring.

 The wrapper looks kludge to me, but provided the ticket is handled soon
 enough (possibly in the next sprint), I think it is OK. The ticket should
 be relatively small, the only place that used WILDCARD_xxx is the database
 datasource and the compiler should be able to find all the occurrences if
 the definitions are removed.

 The code itself looks good enough to merge, so please do. We can discuss
 the inclusion of hints or whatever in the result later and outside of this
 ticket, possibly on the next non-planning call? Or the mailing list?

 Thank you

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