search

NLnetLabs / domain

A DNS library for Rust.
https://nlnetlabs.nl/projects/domain/about/
BSD 3-Clause "New" or "Revised" License
332 stars 56 forks source link

Fix TSIG signing in the multiple response case. #345

Closed ximon18 closed 3 days ago

ximon18 commented 3 weeks ago

When transferring large signed responses, e.g. AXFR of a large zone, that doesn't fit in a single response message, the TSIG signatures generated by the domain tsig module code are rejected by dig, BIND and NSD as invalid. This PR makes the issue I was seeing go away.

Example error from BIND 9:

04-Jul-2024 13:31:29.747 tsig key 'sec1-key': signature failed to verify(2)

Example successes with this PR:

DIG 9:

$ dig @127.0.0.1 -p 8053 -y hmac-sha256:sec1-key:****...****= AXFR .
...
;; Query time: 272 msec
;; SERVER: 127.0.0.1#8053(127.0.0.1) (TCP)
;; WHEN: Thu Jul 04 22:22:30 CEST 2024
;; XFR size: 10022 records (messages 21, bytes 403443)

BIND 9:

04-Jul-2024 22:23:27.554 zone ./IN: zone transfer finished: success
04-Jul-2024 22:23:27.554 zone ./IN: transferred serial 2019090500: TSIG 'sec1-key'
04-Jul-2024 22:23:27.554 zone_settimer: zone ./IN: enter
04-Jul-2024 22:23:27.554 transfer of './IN' from 127.0.0.1#8053: Transfer status: success
04-Jul-2024 22:23:27.554 transfer of './IN' from 127.0.0.1#8053: Transfer completed: 21 messages, 10036 records, 403928 bytes, 0.045 secs (8976177 bytes/sec) (serial 2019090500)

NSD 4.8.0:

[2024-07-04 22:21:23.945] nsd[3529944]: info: zone . received update to serial 2019090500 at 2024-07-04T22:21:23 from 127.0.0.1@8053 TSIG verified with key sec1-key of 402689 bytes in 0.016605 seconds

I've marked this as a DRAFT PR because it doesn't completely handle the TSIG RR doesn't fit case nor does it add unit tests or update the interop test to actually fail (e.g. by using dig instead of drill as the latter doesn't seem to actually verify the TSIG signatures), and this was only a first quick attempt to fix the issue.

Tested manually with BIND/dig 9.18.26 and NSD 4.8.0.

ximon18 commented 2 weeks ago

So, fixing that broke the interop::tsig_client_sequence_nsd test. I've pushed a hacky "fix" which makes both signing of server response sequences be acceptable to NSD and checking of client sequences produced by NSD be acceptable to domain.

partim commented 1 week ago

Feels to me the fix should be to call self.context.apply_signature(mac.as_ref()) in ServerSequence::answer_with_fudge before returning?

(The issue of not being able to push the TSIG record non-withstanding. Perhaps that should be a separate PR?)

partim commented 3 days ago

Obsoleted by #356