BIND 10 #893: TSIG verify, part 2

BIND 10 Development do-not-reply at isc.org
Fri May 6 20:09:48 UTC 2011


#893: TSIG verify, part 2
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  accepted
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110517
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  DNSPacket API                      |           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      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Branch trac893 is ready for review.

 Unfortunately the size of diff is pretty large.  Hopefully the
 following notes help reduce the review load (or discuss how to
 breakdown the entire review into sub-reviews).

 - The first commit (34c2d5b) just imports dependencies and should be
   ignored.
 - changes to TSIGKey (a91fc73, df8ef3f) are independent but the main
   task relies on it.  If appropriate this set could be reviewed
   separately.
 - there's a small change to TSIGError (cabd3b1): this is a completely
   independent change and could be reviewed separately (although it's
   quite trivial)
 - there's also a small change to TSIGRecord (cabd3b1).  See the added
   comments about the intent.  This is also an independent change.
 - the main changes are for tsig.{h, cc} and its tests.  The sign()
   class is essentially unmodified, but due to refactoring there are
   changes on it, too.  The main thing to be reviewed is the verify()
   method (and its helper methods).  Many tests have been added, and in
   terms of the amount of diff the added tests occupies a significant
   part.  Hopefully the test scenarios are quite trivial.
 - I also had to make (short term) changes to cryptolink (a64f778,
   b734c55).  See the todo things below.

 As noted in tsig.h, there are two remaining features that have not
 been implemented:
 - truncated signatures (use 12-byte sig for HMAC-MD5, etc)
 - support the case of DNS messages over TCP some of which skip
   including TSIGs.

 As far as I know we don't need these immediately (especially the
 latter one - I don't know of an implementation at the sender side that
 supports and enables this protocol feature).  Since the branch is
 already quite big, I propose deferring these features to a later
 separate tasks (after the next release).  But to drop the support for
 truncated signatures, I needed to make changes in cryptolink so that
 it's more strict about the sig length and disable some tests for the
 truncation scenarios.  These should be re-enabled when we add support
 for truncation sigs in TSIG.

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


More information about the bind10-tickets mailing list