Skip to content
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

Closed
FiloSottile opened this issue Dec 5, 2019 · 1 comment · Fixed by #1044
Closed

[security] Predictable TXID can lead to response forgeries #1043

FiloSottile opened this issue Dec 5, 2019 · 1 comment · Fixed by #1044

Comments

@FiloSottile
Copy link
Contributor

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:

  • A DNS cache — the attacker makes a sequence of requests to get enough TXIDs to reverse the internal state of the RNG, then it causes a request to be issued by the cache to an upstream server, and knowing the TXID sends a spoofed answer to every port, poisoning the cache.
  • A prober with an API, like say Let's Encrypt — the attacker causes a number of requests to its own domain to observe TXIDs, then causes a lookup for a target domain, and spoofs the response.

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.

@jsha
Copy link
Contributor

jsha commented Dec 5, 2019

Thanks for filing this! I've sent a PR at #1044. Just for the public record: Let's Encrypt uses miekg/dns internally to communicate with our Unbound instances so I think this is not a vulnerability for us, but I do think it is worth fixing this issue by default for other software that uses this library.

miekg pushed a commit that referenced this issue Dec 11, 2019
* Use crypto/rand for random id generation.

Fixes #1043 and #1037

* Panic on rare crypto/rand error.

* Fixes in response to review.
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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants