mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2026-05-05 04:56:10 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: dns: always validate the source address in responses

Browse Source 
When we removed the use of connect() to reach DNS servers in 3.3 with
commit 2c7e05f80e ("MEDIUM: dns: don't call connect to dest socket for
AF_INET*"), we accidentally lost a check on the server's address in
responses, opening the possibility of spoofed DNS responses for someone
who knows both haproxy's IP:port and the transaction ID.

In practice, the impact is very low, because:
  - DNS servers IP addresses are almost always known, and often even among
    the widely used ones (1.1.1.1, 8.8.8.8 etc), and their port is 53.
  - all DNS "security" relies on the ignorance of the transaction ID and
    is possible source port, so if either the attacker is on-path and sees
    them, or it's off-path and has to guess them, but in any case it's
    trivial to spoof the known server in responses, with or without the
    check.

Regardless, let's not further weaken the protocol and do the check.

Thanks to Omkhar Arasaratnam for reporting this issue.

An interesting observation while testing this fix was that the code does
support UNIX dgram sockets (via connect()) but that since we don't bind
to a local UNIX socket to send requests, the server's recvfrom() doesn't
get any address and has nowhere to respond to. So in practice while the
code is designed to deal with UNIX sockets, these cannot work by design.

This fix must be backported to 3.2 where the commit above was backported.
This commit is contained in:
Willy Tarreau 2026-05-04 16:10:20 +02:00
parent 4153aae932
commit 4ef31cb5c2
1 changed files with 9 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/dns.c
View File

@ -219,6 +219,8 @@ ssize_t dns_recv_nameserver(struct dns_nameserver *ns, void *data, size_t size)
if (ns->dgram) {
struct dgram_conn *dgram = &ns->dgram->conn;
struct sockaddr_storage from = {0};
socklen_t fromlen = sizeof(from);
int fd;
HA_SPIN_LOCK(DNS_LOCK, &dgram->lock);
@ -228,7 +230,7 @@ ssize_t dns_recv_nameserver(struct dns_nameserver *ns, void *data, size_t size)
return -1;
}
if ((ret = recv(fd, data, size, 0)) < 0) {
if ((ret = recvfrom(fd, data, size, 0, (struct sockaddr *)&from, &fromlen)) < 0) {
if (errno == EAGAIN || errno == EWOULDBLOCK) {
fd_cant_recv(fd);
HA_SPIN_UNLOCK(DNS_LOCK, &dgram->lock);
@ -239,6 +241,12 @@ ssize_t dns_recv_nameserver(struct dns_nameserver *ns, void *data, size_t size)
HA_SPIN_UNLOCK(DNS_LOCK, &dgram->lock);
return -1;
}
if ((dgram->addr.to.ss_family == AF_INET || dgram->addr.to.ss_family == AF_INET6) &&
ipcmp(&from, &dgram->addr.to, 1) != 0) {
/* reply from unexpected source -- silently discard */
fd_want_recv(fd);
ret = 0;
}
HA_SPIN_UNLOCK(DNS_LOCK, &dgram->lock);
}
else if (ns->stream) {