fix TsigGenerate for non-0 TSIG error or non-empty other data #1138
Conversation
| @@ -131,15 +131,11 @@ func TsigGenerate(m *Msg, secret, requestMAC string, timersOnly bool) ([]byte, s | |||
| return nil, "", ErrKeyAlg | |||
| } | |||
| h.Write(buf) | |||
| // Copy all TSIG fields expect MAC and its size, which are filled using the computed digest. | |||
| *t = *rr | |||
There was a problem hiding this comment.
we could alternatively copy the error and other data (plus its length) fields separately below. I chose this way as it seemed more concise and less error prone (it's clear no fields are forgotten and all fields are an exact copy of the original TSIG RR), but I don't mind taking the alternative approach.
| t.MAC = hex.EncodeToString(h.Sum(nil)) | ||
| t.MACSize = uint16(len(t.MAC) / 2) // Size is half! | ||
|
|
||
| t.Hdr = RR_Header{Name: rr.Hdr.Name, Rrtype: TypeTSIG, Class: ClassANY, Ttl: 0} |
There was a problem hiding this comment.
Note: technically we can't always assume TTL is 0 here; the TSIG in the passed message could have a non-0 value (RFC2845 states it's 0 but doesn't prohibit using a different value). The fix in this PR preserves the passed TTL.
| t.Fudge = rr.Fudge | ||
| t.TimeSigned = rr.TimeSigned | ||
| t.Algorithm = rr.Algorithm | ||
| t.OrigId = m.Id |
There was a problem hiding this comment.
this is also not always correct and should be rr.OrigId. The fix in this PR makes it correct, too.
@tmthrgd thank you for the prompt review and approval. Just to let you know I've just noticed one minor typo in a comment line and corrected it. |
|
[ Quoting <notifications@github.com> in "Re: [miekg/dns] fix TsigGenerate fo..." ]
@tmthrgd approved this pull request.
Not sure of the RFC semantics, but seems reasonable and code looks fine. LGTM.
/merge
|
…1138) Automatically submitted.
The current implementation of
TsigGenerateignores the TSIG error and other data fields of the TSIG RR in the passed DNS message for generating the signed binary-format message (it uses these fields correctly for computing the MAC). So the resulting message would fail to be verified at the recipient. This PR is a straightforward fix to this problem.