From: Alex Rousskov Date: Tue, 16 Jul 2024 01:38:59 +0000 (+0000) Subject: Enable EDNS for DNS A queries and reverse IPv4 lookups (#1864) X-Git-Tag: SQUID_7_0_1~88 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=51c82279ac5aa734c382bc31804e477bf76fe448;p=thirdparty%2Fsquid.git Enable EDNS for DNS A queries and reverse IPv4 lookups (#1864) This change brings Squid code in sync with existing dns_packet_max directive documentation and allows admins to enable a useful performance optimization for still very popular IPv4-related DNS queries. An enabled dns_packet_max feature may now break Squid compatibility with more buggy nameservers (that appeared to "work" before), but we should not penalize many Squid deployments (or complicate configuration and spend a lot of development time) to accommodate a few exceptional ones, at least until such heroic efforts are proven to be necessary. --- diff --git a/doc/release-notes/release-7.sgml.in b/doc/release-notes/release-7.sgml.in index 881ad8f141..35fc35d958 100644 --- a/doc/release-notes/release-7.sgml.in +++ b/doc/release-notes/release-7.sgml.in @@ -172,6 +172,17 @@ This section gives an account of those changes in three categories:

Removed the non_peers action. See the Cache Manager for details. + dns_packet_max +

Honor positive dns_packet_max values when sending DNS A queries + and PTR queries containing IPv4 addresses. Prior to this change, Squid did + not add EDNS extension (RFC 6891) to those DNS queries because 2010 tests + revealed compatibility problems with some DNS resolvers. We hope that those + problems are now sufficiently rare to enable this useful optimization for + all DNS queries, as originally intended. Squid still sends EDNS extension + with DNS AAAA queries and PTR queries containing IPv6 addresses (when + dns_packet_max is set to a positive value). Rare deployments that must use + buggy DNS resolvers should not set dns_packet_max. + access_log

Built-in common and combined logformats now always receive a dash character ("-") in the position of what used to be a diff --git a/src/dns/rfc3596.cc b/src/dns/rfc3596.cc index 997e5d94a2..37a10bf9e0 100644 --- a/src/dns/rfc3596.cc +++ b/src/dns/rfc3596.cc @@ -9,6 +9,7 @@ #include "squid.h" #include "dns/rfc2671.h" #include "dns/rfc3596.h" +#include "SquidConfig.h" #include "util.h" #if HAVE_UNISTD_H @@ -54,7 +55,7 @@ * Returns the size of the query */ ssize_t -rfc3596BuildHostQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, rfc1035_query * query, int qtype, ssize_t edns_sz) +rfc3596BuildHostQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, rfc1035_query * query, int qtype) { static rfc1035_message h; size_t offset = 0; @@ -64,7 +65,10 @@ rfc3596BuildHostQuery(const char *hostname, char *buf, size_t sz, unsigned short h.rd = 1; h.opcode = 0; /* QUERY */ h.qdcount = (unsigned int) 1; + + const auto edns_sz = Config.dns.packet_max; h.arcount = (edns_sz > 0 ? 1 : 0); + offset += rfc1035HeaderPack(buf + offset, sz - offset, &h); offset += rfc1035QuestionPack(buf + offset, sz - offset, @@ -93,9 +97,9 @@ rfc3596BuildHostQuery(const char *hostname, char *buf, size_t sz, unsigned short * \return the size of the query */ ssize_t -rfc3596BuildAQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, rfc1035_query * query, ssize_t edns_sz) +rfc3596BuildAQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, rfc1035_query * query) { - return rfc3596BuildHostQuery(hostname, buf, sz, qid, query, RFC1035_TYPE_A, edns_sz); + return rfc3596BuildHostQuery(hostname, buf, sz, qid, query, RFC1035_TYPE_A); } /** @@ -107,9 +111,9 @@ rfc3596BuildAQuery(const char *hostname, char *buf, size_t sz, unsigned short qi * \return the size of the query */ ssize_t -rfc3596BuildAAAAQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, rfc1035_query * query, ssize_t edns_sz) +rfc3596BuildAAAAQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, rfc1035_query * query) { - return rfc3596BuildHostQuery(hostname, buf, sz, qid, query, RFC1035_TYPE_AAAA, edns_sz); + return rfc3596BuildHostQuery(hostname, buf, sz, qid, query, RFC1035_TYPE_AAAA); } /** @@ -121,7 +125,7 @@ rfc3596BuildAAAAQuery(const char *hostname, char *buf, size_t sz, unsigned short * \return the size of the query */ ssize_t -rfc3596BuildPTRQuery4(const struct in_addr addr, char *buf, size_t sz, unsigned short qid, rfc1035_query * query, ssize_t edns_sz) +rfc3596BuildPTRQuery4(const struct in_addr addr, char *buf, size_t sz, unsigned short qid, rfc1035_query * query) { static char rev[RFC1035_MAXHOSTNAMESZ]; unsigned int i; @@ -133,11 +137,11 @@ rfc3596BuildPTRQuery4(const struct in_addr addr, char *buf, size_t sz, unsigned (i >> 16) & 255, (i >> 24) & 255); - return rfc3596BuildHostQuery(rev, buf, sz, qid, query, RFC1035_TYPE_PTR, edns_sz); + return rfc3596BuildHostQuery(rev, buf, sz, qid, query, RFC1035_TYPE_PTR); } ssize_t -rfc3596BuildPTRQuery6(const struct in6_addr addr, char *buf, size_t sz, unsigned short qid, rfc1035_query * query, ssize_t edns_sz) +rfc3596BuildPTRQuery6(const struct in6_addr addr, char *buf, size_t sz, unsigned short qid, rfc1035_query * query) { static char rev[RFC1035_MAXHOSTNAMESZ]; const uint8_t* r = addr.s6_addr; @@ -152,6 +156,6 @@ rfc3596BuildPTRQuery6(const struct in6_addr addr, char *buf, size_t sz, unsigned snprintf(p,10,"ip6.arpa."); - return rfc3596BuildHostQuery(rev, buf, sz, qid, query, RFC1035_TYPE_PTR, edns_sz); + return rfc3596BuildHostQuery(rev, buf, sz, qid, query, RFC1035_TYPE_PTR); } diff --git a/src/dns/rfc3596.h b/src/dns/rfc3596.h index 10e49e4708..3ebad86415 100644 --- a/src/dns/rfc3596.h +++ b/src/dns/rfc3596.h @@ -16,29 +16,25 @@ ssize_t rfc3596BuildAQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, - rfc1035_query * query, - ssize_t edns_sz); + rfc1035_query *); ssize_t rfc3596BuildAAAAQuery(const char *hostname, char *buf, size_t sz, unsigned short qid, - rfc1035_query * query, - ssize_t edns_sz); + rfc1035_query *); ssize_t rfc3596BuildPTRQuery4(const struct in_addr, char *buf, size_t sz, unsigned short qid, - rfc1035_query * query, - ssize_t edns_sz); + rfc1035_query *); ssize_t rfc3596BuildPTRQuery6(const struct in6_addr, char *buf, size_t sz, unsigned short qid, - rfc1035_query * query, - ssize_t edns_sz); + rfc1035_query *); /* RFC3596 library implements RFC1035 generic host interface */ ssize_t rfc3596BuildHostQuery(const char *hostname, @@ -46,8 +42,7 @@ ssize_t rfc3596BuildHostQuery(const char *hostname, size_t sz, unsigned short qid, rfc1035_query * query, - int qtype, - ssize_t edns_sz); + int qtype); /* RFC3596 section 2.1 defines new RR type AAAA as 28 */ #define RFC1035_TYPE_AAAA 28 diff --git a/src/dns_internal.cc b/src/dns_internal.cc index 1dcad7cb77..8b4d98e9c8 100644 --- a/src/dns_internal.cc +++ b/src/dns_internal.cc @@ -233,13 +233,6 @@ static hash_table *idns_lookup_hash = nullptr; /* * Notes on EDNS: * - * IPv4: - * EDNS as specified may be sent as an additional record for any request. - * early testing has revealed that it works on common devices, but cannot - * be reliably used on any A or PTR requet done for IPv4 addresses. - * - * As such the IPv4 packets are still hard-coded not to contain EDNS (0) - * * Squid design: * Squid is optimized to generate one packet and re-send it to all NS * due to this we cannot customize the EDNS size per NS. @@ -1296,8 +1289,7 @@ idnsGrokReply(const char *buf, size_t sz, int /*from_ns*/) // Build new query q->query_id = idnsQueryID(); debugs(78, 3, "idnsGrokReply: Trying A Query for " << q->name); - // see EDNS notes at top of file why this sends 0 - q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query, 0); + q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query); if (q->sz < 0) { /* problem with query data -- query not sent */ idnsCallback(q, "Internal error"); @@ -1740,7 +1732,7 @@ idnsSendSlaveAAAAQuery(idns_query *master) memcpy(q->orig, master->orig, sizeof(q->orig)); q->master = master; q->query_id = idnsQueryID(); - q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query, Config.dns.packet_max); + q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query); debugs(78, 3, "buf is " << q->sz << " bytes for " << q->name << ", id = 0x" << asHex(q->query_id)); @@ -1796,8 +1788,7 @@ idnsALookup(const char *name, IDNSCB * callback, void *data) debugs(78, 3, "idnsALookup: searchpath used for " << q->name); } - // see EDNS notes at top of file why this sends 0 - q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query, 0); + q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->query_id, &q->query); if (q->sz < 0) { /* problem with query data -- query not sent */ @@ -1829,12 +1820,11 @@ idnsPTRLookup(const Ip::Address &addr, IDNSCB * callback, void *data) if (addr.isIPv6()) { struct in6_addr addr6; addr.getInAddr(addr6); - q->sz = rfc3596BuildPTRQuery6(addr6, q->buf, sizeof(q->buf), q->query_id, &q->query, Config.dns.packet_max); + q->sz = rfc3596BuildPTRQuery6(addr6, q->buf, sizeof(q->buf), q->query_id, &q->query); } else { struct in_addr addr4; addr.getInAddr(addr4); - // see EDNS notes at top of file why this sends 0 - q->sz = rfc3596BuildPTRQuery4(addr4, q->buf, sizeof(q->buf), q->query_id, &q->query, 0); + q->sz = rfc3596BuildPTRQuery4(addr4, q->buf, sizeof(q->buf), q->query_id, &q->query); } if (q->sz < 0) {