Opened 9 years ago

Closed 5 years ago

Last modified 5 years ago

#920 closed enhancement (fixed)

extension to TSIG: support truncated signature

Reported by: jinmei Owned by: fdupont
Priority: medium Milestone: Kea0.9.1beta
Component: libdns++ Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 4.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description (last modified by jinmei)

This is a remaining task (for relatively minor cases) of #893 as noted in
http://bind10.isc.org/ticket/893#comment:3

We need to support truncated signature in TSIGContext::verify() (and
perhaps also in sign() - BIND 9 does something about this in its verify
implementation. we need to check). We then need to revert the short term
hack (and temporarily disabled tests) made in libcryptolink in #893 and
#954.

Subtickets

Change History (21)

comment:1 Changed 9 years ago by shane

  • Milestone changed from New Tasks to Year 3 Task Backlog

comment:2 Changed 9 years ago by jinmei

  • Milestone changed from Year 3 Task Backlog to Next-Sprint-Proposed

comment:3 Changed 9 years ago by jinmei

  • Owner changed from jinmei to UnAssigned
  • Status changed from new to assigned

comment:4 Changed 9 years ago by jinmei

  • Description modified (diff)

comment:5 Changed 9 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 4

comment:6 Changed 6 years ago by tomek

  • Milestone set to Remaining BIND10 tickets

comment:7 Changed 5 years ago by fdupont

  • Version set to git

#3454 added truncated HMAC support so this ticket can now be addressed.

comment:8 Changed 5 years ago by fdupont

  • Owner changed from UnAssigned to fdupont

comment:9 Changed 5 years ago by fdupont

Add missing rcode's.
Return BADTRUNC according to RFC 4635.
Added an unit test.

Note truncated HMAC are still not supported, they are just rejected by the policy (compliant but not user friendly).
I suggest to open a new ticket if/when we'd like to implement truncated HMAC's (extend key syntax, add a signature bit length in keys and TSIG contexts, etc).
BTW the usage is to code the bit length in the algorithm name, for instance hmac-sha512-256 for HMAC SHA512 truncated to 256 bits (used by some as an alternate to HMAC SHA256).

comment:10 Changed 5 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from assigned to reviewing

comment:11 Changed 5 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:12 follow-up: Changed 5 years ago by stephen

  • Owner changed from stephen to fdupont

Reviewed commit 1d4f578e9d4e31a0798f11423377f35491c077c6

src/lib/cryptolink/botan_hmac.cc
Copyright should be updated to 2014 (I missed that in the the review of #3454).

src/lib/dns/rdataclass.cc
Now that we are editing this directly, the copyright should be updated to 2014.

As we have deferred decisions on the DNS library and what to do with it, I would also update the source file as well - rdata/any_255/tsig_250.cc.

(I think we were a bit too keen to get rid of all the Python scripts. We still have the "gen-rdatacode.py.in" file in the repository, and if we were to restore a couple of files in the now-deleted "python" subdirectory, we would still be able to use it. Not during the build, but as a programmer aid to generate a new rdataclass.cc that is then checked into the repository.)

src/lib/dns/tests/tsig_unittest.cc
Copyright should be updated to 2014.

src/lib/dns/tests/tsigerror_unittest.cc
Copyright should be updated to 2014.

src/lib/dns/tsig.cc
Copyright should be updated to 2014.

I see that this does not support truncated HMAC. However, I thought the purpose of #3454 was to support it?

src/lib/dns/tsig.h
Copyright should be updated to 2014.

src/lib/dns/tsigerror.cc
Copyright should be updated to 2014.

In toRcode() and toText(): rather than check code_ against BAD_TRUNC_CODE, I suggest defining a symbol such as MAX_BAD_CODE in the CodeValue enum in the header file and checking against that. If we ever extend the TSIG error codes in the future, we update that value and won't need to alter the methods.

comment:13 in reply to: ↑ 12 Changed 5 years ago by fdupont

Replying to stephen:

Reviewed commit 1d4f578e9d4e31a0798f11423377f35491c077c6

src/lib/cryptolink/botan_hmac.cc
Copyright should be updated to 2014 (I missed that in the the review of #3454).

=> I asked in the next ticket (#3593) what exactly do about copyrights. It seems they must be fixed before review. BTW for botan_hmac.cc which has 2011, should I put 2011,2014 or 2011-2014?

As we have deferred decisions on the DNS library and what to do with it, I would also update the source file as well - rdata/any_255/tsig_250.cc.

=> I agree.

(I think we were a bit too keen to get rid of all the Python scripts. We still have the "gen-rdatacode.py.in" file in the repository, and if we were to restore a couple of files in the now-deleted "python" subdirectory, we would still be able to use it. Not during the build, but as a programmer aid to generate a new rdataclass.cc that is then checked into the repository.)

=> I have a copy of bundy for doing this kind of things...

I see that this does not support truncated HMAC. However, I thought the purpose of #3454 was to support it?

=> it is the purpose of #3593.

In toRcode() and toText(): rather than check code_ against BAD_TRUNC_CODE, I suggest defining a symbol such as MAX_BAD_CODE in the CodeValue enum in the header file and checking against that. If we ever extend the TSIG error codes in the future, we update that value and won't need to alter the methods.

=> it is a change in the tsigerror design, IMHO it should be done but in another ticket.

comment:14 Changed 5 years ago by fdupont

  • Owner changed from fdupont to stephen

comment:15 Changed 5 years ago by fdupont

Fixed Copyright years in the 3 tickets, updated tsig_250.cc from rdataclass.cc patch.

comment:16 Changed 5 years ago by stephen

BTW for botan_hmac.cc which has 2011, should I put 2011,2014 or 2011-2014?

I think "2011, 2014". I suspect that the hyphen is used only if changes have been made in all intervening years as well.

Fixed Copyright years in the 3 tickets, updated tsig_250.cc from rdataclass.cc patch.

Thanks.

All OK, this can be merged.

comment:17 Changed 5 years ago by fdupont

  • Owner changed from stephen to fdupont
  • Status changed from reviewing to accepted

comment:18 Changed 5 years ago by fdupont

  • Status changed from accepted to assigned

comment:19 Changed 5 years ago by fdupont

Merged to #3593 ticket.

comment:20 Changed 5 years ago by fdupont

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:21 Changed 5 years ago by tomek

  • Milestone changed from Remaining BIND10 tickets to Kea0.9.1
Note: See TracTickets for help on using tickets.