Validate DNS responses before accepting a resolved address - #101
Closed
Watson1978 wants to merge 1 commit into
Closed
Validate DNS responses before accepting a resolved address#101Watson1978 wants to merge 1 commit into
Watson1978 wants to merge 1 commit into
Conversation
DNSResolver built every query with the constant transaction ID 2 and authenticated the reply with nothing but `id == 2`, while on_readable read the datagram off a never-connected UDPSocket with `recvfrom_nonblock(...).first`, discarding the sender. Any host that could land a UDP packet on the resolver's ephemeral port could therefore choose the address a hostname resolved to, and Coolio::TCPSocket.connect would open the application's session to it. Give each query a random 16-bit transaction ID from SecureRandom, and accept a datagram only when it carries that ID and arrives from port 53 of an address this query was actually sent to. The check runs before the response is parsed. A datagram that fails it is ignored rather than turned into a failure, so a single spoofed packet can no longer deny the resolution either; the existing TIMEOUT/RETRIES budget still bounds the wait. Nameservers may be given as hostnames, so send_request resolves the nameserver to a numeric address and remembers it, caching only successful lookups. The constructor keeps accepting whatever it accepted before, an unresolvable nameserver still surfaces as SocketError from the same place, and one that is only transiently unresolvable recovers on the next retry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Watson1978
force-pushed
the
fix-dns-response-spoofing
branch
from
July 31, 2026 07:00
67b4bb4 to
aa9a50f
Compare
Collaborator
Author
|
Closing: this change is already on Note for release notes: the commit title of fa7ba96 only mentions the Valgrind CI fix, so this DNS response validation change is not visible from |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Coolio::DNSResolverbuilt every query with the constant transaction ID2and authenticated the reply with nothing butid == 2, whileon_readableread the datagram off a never-connectedUDPSocketwithrecvfrom_nonblock(...).first, discarding the sender address.Both of the usual anti-spoofing defences were therefore absent: the transaction ID contributed zero entropy, and no check tied the datagram to the nameserver that was queried. Any host that could land a UDP packet on the resolver's ephemeral port — an off-path attacker spraying the port range, or anything on the same LAN — could choose the address a hostname resolved to, and
Coolio::TCPSocket.connect(hostname, port)would open the application's session to it. A single unsolicited datagram could also deny the resolution outright, since anything that failed to parse becameon_failure+detach.Fix
SecureRandom, packed byrequest_messageand compared byresponse_address.solicited_response?) runs before the response is parsed, so the parser's attack surface shrinks as well.TIMEOUT/RETRIESbudget still bounds how long the resolver waits, and theErrno::ECONNREFUSEDpath still fails fast exactly as before.Nameservers may be given as hostnames (
DNSResolver.new(host, "ns.example.com")), whichUDPSocket#sendused to resolve on every send. To know the address a reply must come from,send_requestnow resolves the nameserver itself and remembers the numeric address, caching only successful lookups:SocketError, raised from the same place it was raised from before;/etc/resolv.confyields, and the only pathCoolio::TCPSocketuses — cost no lookup at all, where the previous code resolved on every retransmission.Retries rotate through
@nameservers, so a reply to an earlier retransmission is still accepted from any address already queried.Tests
spec/dns_spec.rbgains 16 examples covering the transaction ID being unpredictable, responses rejected for a wrong ID / unqueried source address / wrong source port / truncated datagram / arrival before the request was sent, a spoofed response being ignored rather than resolved or failed, and the hostname-nameserver behaviour above (queried at its numeric address, looked up once, not rejected at construction, surfacing asSocketError, recovering from a transient failure).rspec— 65 examples, 0 failures.Notes
UDPSocket#sendused to try each resolved address, whereas this change sends to the first. Reaching it needs a hostname nameserver, multiple A records, and a partially broken route.response_addressstill ignoresancount, so an A record placed in the authority or additional section is accepted. That is separate hardening, left for its own change.🤖 Generated with Claude Code