From f04cf8506a05eefda4294550b36cd02bc55b9703 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sun, 24 Nov 2024 23:06:22 +0000 Subject: [PATCH] Simplify EDNS0 packet size handling. In the post 2020 flag-day world, we limit UDP packets to 1232 bytes which can go anywhere, so the dodgy code to try and determine the functional maxmimum packet size on the path from upstream servers is obsolete. --- src/config.h | 1 - src/dnsmasq.h | 10 ++++---- src/edns0.c | 27 ++++++++-------------- src/forward.c | 64 +++++++++++++++------------------------------------ src/network.c | 4 ---- 5 files changed, 31 insertions(+), 75 deletions(-) diff --git a/src/config.h b/src/config.h index a1684a2..5964c23 100644 --- a/src/config.h +++ b/src/config.h @@ -20,7 +20,6 @@ #define TCP_MAX_QUERIES 100 /* Maximum number of queries per incoming TCP connection */ #define TCP_BACKLOG 32 /* kernel backlog limit for TCP connections */ #define EDNS_PKTSZ 1232 /* default max EDNS.0 UDP packet from from /dnsflagday.net/2020 */ -#define SAFE_PKTSZ 1232 /* "go anywhere" UDP packet size, see https://dnsflagday.net/2020/ */ #define KEYBLOCK_LEN 40 /* choose to minimise fragmentation when storing DNSSEC keys */ #define DNSSEC_LIMIT_WORK 40 /* Max number of queries to validate one question */ #define DNSSEC_LIMIT_SIG_FAIL 20 /* Number of signature that can fail to validate in one answer */ diff --git a/src/dnsmasq.h b/src/dnsmasq.h index 02fbd53..11daae4 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -594,8 +594,7 @@ struct server { char interface[IF_NAMESIZE+1]; unsigned int ifindex; /* corresponding to interface, above */ struct serverfd *sfd; - int tcpfd, edns_pktsz; - time_t pktsz_reduced; + int tcpfd; unsigned int queries, failed_queries, nxdomain_replies, retrys; unsigned int query_latency, mma_latency; time_t forwardtime; @@ -769,9 +768,8 @@ struct dyndir { #define FREC_AD_QUESTION 32 #define FREC_DO_QUESTION 64 #define FREC_ADDED_PHEADER 128 -#define FREC_TEST_PKTSZ 256 -#define FREC_HAS_PHEADER 512 -#define FREC_GONE_TO_TCP 1024 +#define FREC_HAS_PHEADER 256 +#define FREC_GONE_TO_TCP 512 struct frec { struct frec_src { @@ -1864,7 +1862,7 @@ void from_wire(char *name); unsigned char *find_pseudoheader(struct dns_header *header, size_t plen, size_t *len, unsigned char **p, int *is_sign, int *is_last); size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *limit, - unsigned short udp_sz, int optno, unsigned char *opt, size_t optlen, int set_do, int replace); + int optno, unsigned char *opt, size_t optlen, int set_do, int replace); size_t add_do_bit(struct dns_header *header, size_t plen, unsigned char *limit); size_t add_edns0_config(struct dns_header *header, size_t plen, unsigned char *limit, union mysockaddr *source, time_t now, int *cacheable); diff --git a/src/edns0.c b/src/edns0.c index 89ae2a7..fc19f31 100644 --- a/src/edns0.c +++ b/src/edns0.c @@ -99,11 +99,9 @@ unsigned char *find_pseudoheader(struct dns_header *header, size_t plen, size_t /* replace == 0 ->don't replace existing option replace == 1 ->replace existing or add option replace == 2 ->relpace existing option only. - - udp_sz == 0 -> leave unchanged in existing EDNS0 or set to deamon->edns_pksz in a new one. */ size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *limit, - unsigned short udp_sz, int optno, unsigned char *opt, size_t optlen, int set_do, int replace) + int optno, unsigned char *opt, size_t optlen, int set_do, int replace) { unsigned char *lenp, *datap, *p, *udp_len, *buff = NULL; int rdlen = 0, is_sign, is_last; @@ -122,11 +120,7 @@ size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *l p = udp_len; - if (udp_sz == 0) - GETSHORT(udp_sz, p); - else - PUTSHORT(udp_sz, p); - + PUTSHORT(daemon->edns_pktsz, p); GETSHORT(rcode, p); GETSHORT(flags, p); @@ -208,12 +202,9 @@ size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *l return plen; /* bad packet */ } - if (udp_sz == 0) - udp_sz = daemon->edns_pktsz; - *p++ = 0; /* empty name */ PUTSHORT(T_OPT, p); - PUTSHORT(udp_sz, p); /* max packet length, 512 if not given in EDNS0 header */ + PUTSHORT(daemon->edns_pktsz, p); /* max packet length, 512 if not given in EDNS0 header */ PUTSHORT(rcode, p); /* extended RCODE and version */ PUTSHORT(flags, p); /* DO flag */ lenp = p; @@ -258,7 +249,7 @@ size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *l size_t add_do_bit(struct dns_header *header, size_t plen, unsigned char *limit) { - return add_pseudoheader(header, plen, (unsigned char *)limit, 0, 0, NULL, 0, 1, 0); + return add_pseudoheader(header, plen, (unsigned char *)limit, 0, NULL, 0, 1, 0); } static unsigned char char64(unsigned char c) @@ -303,7 +294,7 @@ static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned ch replace = 2; if (replace != 0 || maclen == 6) - plen = add_pseudoheader(header, plen, limit, 0, EDNS0_OPTION_NOMDEVICEID, (unsigned char *)encode, strlen(encode), 0, replace); + plen = add_pseudoheader(header, plen, limit, EDNS0_OPTION_NOMDEVICEID, (unsigned char *)encode, strlen(encode), 0, replace); return plen; } @@ -328,7 +319,7 @@ static size_t add_mac(struct dns_header *header, size_t plen, unsigned char *lim replace = 2; if (replace != 0 || maclen != 0) - plen = add_pseudoheader(header, plen, limit, 0, EDNS0_OPTION_MAC, mac, maclen, 0, replace); + plen = add_pseudoheader(header, plen, limit, EDNS0_OPTION_MAC, mac, maclen, 0, replace); return plen; } @@ -428,7 +419,7 @@ static size_t add_source_addr(struct dns_header *header, size_t plen, unsigned c else return plen; - return add_pseudoheader(header, plen, (unsigned char *)limit, 0, EDNS0_OPTION_CLIENT_SUBNET, (unsigned char *)&opt, len, 0, replace); + return add_pseudoheader(header, plen, (unsigned char *)limit, EDNS0_OPTION_CLIENT_SUBNET, (unsigned char *)&opt, len, 0, replace); } int check_source(struct dns_header *header, size_t plen, unsigned char *pseudoheader, union mysockaddr *peer) @@ -528,7 +519,7 @@ static size_t add_umbrella_opt(struct dns_header *header, size_t plen, unsigned PUTLONG(daemon->umbrella_asset, u); } - return add_pseudoheader(header, plen, (unsigned char *)limit, 0, EDNS0_OPTION_UMBRELLA, (unsigned char *)&opt, u - (u8 *)&opt, 0, 1); + return add_pseudoheader(header, plen, (unsigned char *)limit, EDNS0_OPTION_UMBRELLA, (unsigned char *)&opt, u - (u8 *)&opt, 0, 1); } /* Set *check_subnet if we add a client subnet option, which needs to checked @@ -543,7 +534,7 @@ size_t add_edns0_config(struct dns_header *header, size_t plen, unsigned char *l plen = add_dns_client(header, plen, limit, source, now, cacheable); if (daemon->dns_client_id) - plen = add_pseudoheader(header, plen, limit, 0, EDNS0_OPTION_NOMCPEID, + plen = add_pseudoheader(header, plen, limit, EDNS0_OPTION_NOMCPEID, (unsigned char *)daemon->dns_client_id, strlen(daemon->dns_client_id), 0, 1); if (option_bool(OPT_UMBRELLA)) diff --git a/src/forward.c b/src/forward.c index 7795cd5..c2d6e48 100644 --- a/src/forward.c +++ b/src/forward.c @@ -456,19 +456,16 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, } } } - - /* If we didn't get an answer advertising a maximal packet in EDNS, - fall back to 1280, which should work everywhere on IPv6. - If that generates an answer, it will become the new default - for this server */ - forward->flags |= FREC_TEST_PKTSZ; } if (forward->forwardall) start = first; forwarded = 0; - + + /* Advertise the size of UDP reply we can accept. */ + plen = add_pseudoheader(header, plen, (unsigned char *)(header + daemon->edns_pktsz), 0, NULL, 0, 0, 0); + /* check for send errors here (no route to host) if we fail to send to all nameservers, send back an error packet straight away (helps modem users when offline) */ @@ -486,10 +483,6 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, if (option_bool(OPT_CONNTRACK)) set_outgoing_mark(forward, fd); #endif - - /* send EDNS0 packet size to the maximum size we believe the link allows at this time. */ - plen = add_pseudoheader(header, plen, (unsigned char *)(header + daemon->edns_pktsz), srv->edns_pktsz, 0, NULL, 0, 0, 0); - if (retry_send(sendto(fd, (char *)header, plen, 0, &srv->addr.sa, sa_len(&srv->addr)))) @@ -553,9 +546,9 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, u16 swap = htons((u16)ede); if (ede != EDE_UNSET) - plen = add_pseudoheader(header, plen, (unsigned char *)(header + replylimit), daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); + plen = add_pseudoheader(header, plen, (unsigned char *)(header + replylimit), EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); else - plen = add_pseudoheader(header, plen, (unsigned char *)(header + replylimit), daemon->edns_pktsz, 0, NULL, 0, do_bit, 0); + plen = add_pseudoheader(header, plen, (unsigned char *)(header + replylimit), 0, NULL, 0, do_bit, 0); } #if defined(HAVE_CONNTRACK) && defined(HAVE_UBUS) @@ -828,7 +821,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server if (pheader && ede != EDE_UNSET) { u16 swap = htons((u16)ede); - n = add_pseudoheader(header, n, limit, daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 1); + n = add_pseudoheader(header, n, limit, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 1); } if (RCODE(header) == NXDOMAIN) @@ -971,13 +964,12 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, struct randfd_list *rfds = NULL; struct frec *new = NULL; struct blockdata *newstash = NULL; - unsigned char *p; - + /* Make sure we don't expire and free the orig frec during the allocation of a new one: third arg of get_new_frec() does that. */ if ((serverind = dnssec_server(forward->sentto, daemon->keyname, NULL, NULL)) != -1 && (server = daemon->serverarray[serverind]) && - (nn = dnssec_generate_query(header, ((unsigned char *) header) + server->edns_pktsz, + (nn = dnssec_generate_query(header, ((unsigned char *) header) + daemon->edns_pktsz, daemon->keyname, forward->class, get_id(), STAT_ISEQUAL(status, STAT_NEED_KEY) ? T_DNSKEY : T_DS)) && (fd = allocate_rfd(&rfds, server)) != -1 && @@ -1023,10 +1015,6 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header, set_outgoing_mark(orig, fd); #endif - /* Maximum packet size depends on the server */ - if (find_pseudoheader(header, nn, NULL, &p, NULL, NULL)) - PUTSHORT(server->edns_pktsz, p); - server_send(server, fd, header, nn, 0); server->queries++; #ifdef HAVE_DUMPFILE @@ -1168,10 +1156,6 @@ void reply_query(int fd, time_t now) else if (daemon->serverarray[first]->last_server == c) daemon->serverarray[first]->last_server = -1; - /* If sufficient time has elapsed, try and expand UDP buffer size again. */ - if (difftime(now, server->pktsz_reduced) > UDP_TEST_TIME) - server->edns_pktsz = daemon->edns_pktsz; - /* log_query gets called indirectly all over the place, so pass these in global variables - sorry. */ daemon->log_display_id = forward->frec_src.log_id; @@ -1214,18 +1198,6 @@ void reply_query(int fd, time_t now) if (forward->forwardall && (--forward->forwardall > 1) && RCODE(header) == REFUSED) return; - /* We tried resending to this server with a smaller maximum size and got an answer. - Make that permanent. To avoid reduxing the packet size for a single dropped packet, - only do this when we get a truncated answer, or one that fits the safe size. */ - if (server->edns_pktsz > SAFE_PKTSZ && (forward->flags & FREC_TEST_PKTSZ) && - ((header->hb3 & HB3_TC) || n <= SAFE_PKTSZ)) - { - server->edns_pktsz = SAFE_PKTSZ; - server->pktsz_reduced = now; - (void)prettyprint_addr(&server->addr, daemon->addrbuff); - my_syslog(LOG_WARNING, _("reducing DNS packet size for nameserver %s to %d"), daemon->addrbuff, SAFE_PKTSZ); - } - forward->sentto = server; /* We have a good answer, and will now validate it or return it. @@ -1776,7 +1748,7 @@ void receive_query(struct listener *listen, time_t now) m = answer_disallowed(header, (size_t)n, (u32)mark, is_single_query ? daemon->namebuff : NULL); if (have_pseudoheader && m != 0) - m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, daemon->edns_pktsz, + m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); if (m >= 1) @@ -1797,7 +1769,7 @@ void receive_query(struct listener *listen, time_t now) if (m >= 1) { if (have_pseudoheader) - m = add_pseudoheader(header, m, ((unsigned char *) header) + udp_size, daemon->edns_pktsz, + m = add_pseudoheader(header, m, ((unsigned char *) header) + udp_size, 0, NULL, 0, do_bit, 0); #ifdef HAVE_DUMPFILE @@ -1843,11 +1815,11 @@ void receive_query(struct listener *listen, time_t now) { u16 swap = htons(ede); - m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, daemon->edns_pktsz, + m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); } else - m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, daemon->edns_pktsz, + m = add_pseudoheader(header, m, ((unsigned char *) header) + daemon->edns_pktsz, 0, NULL, 0, do_bit, 0); } @@ -2403,7 +2375,7 @@ unsigned char *tcp_request(int confd, time_t now, m = answer_disallowed(header, size, (u32)mark, is_single_query ? daemon->namebuff : NULL); if (have_pseudoheader && m != 0) - m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, + m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); } #endif @@ -2412,7 +2384,7 @@ unsigned char *tcp_request(int confd, time_t now, { m = answer_auth(header, ((char *) header) + 65536, (size_t)size, now, &peer_addr, local_auth); if (m >= 1 && have_pseudoheader) - m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, + m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, 0, NULL, 0, do_bit, 0); } #endif @@ -2438,7 +2410,7 @@ unsigned char *tcp_request(int confd, time_t now, dst_addr_4, netmask, now, ad_reqd, do_bit, &stale, &filtered); if (m >= 1 && have_pseudoheader) - m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, + m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, 0, NULL, 0, do_bit, 0); } /* Do this by steam now we're not in the select() loop */ @@ -2596,9 +2568,9 @@ unsigned char *tcp_request(int confd, time_t now, u16 swap = htons((u16)ede); if (ede != EDE_UNSET) - m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); + m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0); else - m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, 0, NULL, 0, do_bit, 0); + m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, 0, NULL, 0, do_bit, 0); } check_log_writer(1); diff --git a/src/network.c b/src/network.c index b061833..0e93c4d 100644 --- a/src/network.c +++ b/src/network.c @@ -1587,10 +1587,6 @@ void check_servers(int no_loop_check) for (count = 0, serv = daemon->servers; serv; serv = serv->next) { - /* Init edns_pktsz for newly created server records. */ - if (serv->edns_pktsz == 0) - serv->edns_pktsz = daemon->edns_pktsz; - #ifdef HAVE_DNSSEC if (option_bool(OPT_DNSSEC_VALID)) {