-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[security] Predictable TXID can lead to response forgeries #1043
Comments
jsha
added a commit
to jsha/dns
that referenced
this issue
Dec 5, 2019
Thanks for filing this! I've sent a PR at #1044. Just for the public record: Let's Encrypt uses |
miekg
pushed a commit
that referenced
this issue
Dec 11, 2019
This was referenced Dec 11, 2019
aanm
pushed a commit
to cilium/dns
that referenced
this issue
Jul 29, 2022
* Use crypto/rand for random id generation. Fixes miekg#1043 and miekg#1037 * Panic on rare crypto/rand error. * Fixes in response to review.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The default
Id
function uses math/rand, meaning the outputs are predictable, and an attacker might be able to use this to forge responses without being on path.Seeding math/rand from crypto/rand is pointless, as the math/rand algorithm is invertible: given a sequence of outputs it's possible to reconstruct the
Rand
state and predict all future outputs. Exploitation might be a little slower because the outputs are just 16 bits, but it's likely to be possible.Unless 0x20 or DNSSEC are used, response verification relies only on source port and TXID. They are both short, but the combination usually makes it hard for an off-path attacker to win the race against the real answer. Without the TXID, the attacker has a very good chance of success at a Kaminsky Attack.
A couple example scenarios:
Since the performance cost seems negligible (#1037), I recommend doing the secure thing by default and just reading the 2 bytes from crypto/rand. If there are performance problems, just using a
bufio.Reader
should solve them, as most of the cost of crypto/rand is syscall overhead.Filing publicly as asked by @miekg.
The text was updated successfully, but these errors were encountered: