From bcdad793791835f842a5a77d5abc27c609a151dc Mon Sep 17 00:00:00 2001 From: Wietse Z Venema Date: Fri, 23 Feb 2024 00:00:00 -0500 Subject: [PATCH] postfix-3.9-20240223-nonprod --- postfix/HISTORY | 5 +- postfix/src/dns/dns.h | 9 +-- postfix/src/dns/dns_lookup.c | 12 ++-- postfix/src/dns/dns_rr.c | 78 ++++++++------------- postfix/src/dns/test_dns_lookup.c | 3 +- postfix/src/global/mail_version.h | 2 +- postfix/src/posttls-finger/posttls-finger.c | 31 ++------ postfix/src/smtp/smtp_addr.c | 47 ++++--------- postfix/src/smtpd/smtpd_check.c | 12 ++-- 9 files changed, 69 insertions(+), 130 deletions(-) diff --git a/postfix/HISTORY b/postfix/HISTORY index 0539d07f0..01893b367 100644 --- a/postfix/HISTORY +++ b/postfix/HISTORY @@ -27921,13 +27921,12 @@ Apologies for any names omitted. postconf/postconf.h, conf/postfix-script, conf/post-install, postfix-install. -20240222 +20240223 Safety: drop and log over-size DNS responses resulting in more than 100 records. This prevents a tail recursion error in dns_rr_append(), reported by Toshifumi Sakaguchi, and - prevents DNS request multiplication in check_xx_yy_access. + limits DNS request multiplication in check_xx_yy_access. Files: dns/dns.h, dns/dns_lookup.c, dns/dns_rr.c, dns/test_dns_lookup.c, posttls-finger/posttls-finger.c, smtp/smtp_addr.c, smtpd/smtpd_check.c. - diff --git a/postfix/src/dns/dns.h b/postfix/src/dns/dns.h index 63360418c..a3507ffa1 100644 --- a/postfix/src/dns/dns.h +++ b/postfix/src/dns/dns.h @@ -26,7 +26,6 @@ #include #endif #include -#include /* INT_MAX */ /* * Name server compatibility. These undocumented macros appear in the file @@ -165,11 +164,14 @@ typedef struct DNS_RR { struct DNS_RR *next; /* linkage */ size_t data_len; /* actual data size */ char *data; /* a bunch of data */ - int len; /* list length or DNS_RR_DISCARDED */ + int len; /* list length */ + int flags; /* DNS_RR_FLAG_XX, see below */ /* Add new fields at the end, for ABI forward compatibility. */ } DNS_RR; -#define DNS_RR_DISCARDED INT_MAX /* sentinel */ +#define DNS_RR_FLAG_TRUNCATED (1<<0) + +#define DNS_RR_IS_TRUNCATED(rr) ((rr)->flags & DNS_RR_FLAG_TRUNCATED) /* * dns_strerror.c @@ -211,7 +213,6 @@ extern DNS_RR *dns_rr_create(const char *, const char *, extern void dns_rr_free(DNS_RR *); extern DNS_RR *dns_rr_copy(DNS_RR *); extern DNS_RR *dns_rr_append(DNS_RR *, DNS_RR *); -extern DNS_RR *dns_rr_append_discard(DNS_RR *, DNS_RR *); extern DNS_RR *dns_rr_sort(DNS_RR *, int (*) (DNS_RR *, DNS_RR *)); extern DNS_RR *dns_srv_rr_sort(DNS_RR *); extern int dns_rr_compare_pref_ipv6(DNS_RR *, DNS_RR *); diff --git a/postfix/src/dns/dns_lookup.c b/postfix/src/dns/dns_lookup.c index 60098f94c..08bd0319d 100644 --- a/postfix/src/dns/dns_lookup.c +++ b/postfix/src/dns/dns_lookup.c @@ -983,8 +983,8 @@ static int dns_get_answer(const char *orig_name, DNS_REPLY *reply, int type, &fixed)) == DNS_OK) { resource_found++; rr->dnssec_valid = *maybe_secure ? reply->dnssec_ad : 0; - *rrlist = dns_rr_append_discard(*rrlist, rr); - if (*rrlist && (*rrlist)->len == DNS_RR_DISCARDED) + *rrlist = dns_rr_append(*rrlist, rr); + if (DNS_RR_IS_TRUNCATED(*rrlist)) break; } else if (status == DNS_NULLMX || status == DNS_NULLSRV) { CORRUPT(status); /* TODO: use better name */ @@ -1217,8 +1217,8 @@ int dns_lookup_rl(const char *name, unsigned flags, DNS_RR **rrlist, status = dns_lookup_x(name, type, flags, rrlist ? &rr : (DNS_RR **) 0, fqdn, why, rcode, lflags); if (rrlist && rr) { - *rrlist = dns_rr_append_discard(*rrlist, rr); - if (*rrlist && (*rrlist)->len == DNS_RR_DISCARDED) + *rrlist = dns_rr_append(*rrlist, rr); + if (DNS_RR_IS_TRUNCATED(*rrlist)) break; } if (status == DNS_OK) { @@ -1272,8 +1272,8 @@ int dns_lookup_rv(const char *name, unsigned flags, DNS_RR **rrlist, status = dns_lookup_x(name, type, flags, rrlist ? &rr : (DNS_RR **) 0, fqdn, why, rcode, lflags); if (rrlist && rr) { - *rrlist = dns_rr_append_discard(*rrlist, rr); - if (*rrlist && (*rrlist)->len == DNS_RR_DISCARDED) + *rrlist = dns_rr_append(*rrlist, rr); + if (DNS_RR_IS_TRUNCATED(*rrlist)) break; } if (status == DNS_OK) { diff --git a/postfix/src/dns/dns_rr.c b/postfix/src/dns/dns_rr.c index 20a51f421..ed629d9d6 100644 --- a/postfix/src/dns/dns_rr.c +++ b/postfix/src/dns/dns_rr.c @@ -29,10 +29,6 @@ /* DNS_RR *list; /* DNS_RR *record; /* -/* DNS_RR *dns_rr_append_discard(list, record) -/* DNS_RR *list; -/* DNS_RR *record; -/* /* DNS_RR *dns_rr_sort(list, compar) /* DNS_RR *list /* int (*compar)(DNS_RR *, DNS_RR *); @@ -101,19 +97,22 @@ /* /* dns_rr_copy() makes a copy of a resource record. /* -/* dns_rr_append() appends a resource record to a (list of) resource -/* record(s). -/* A null input list is explicitly allowed. -/* -/* dns_rr_append_discard() appends of discards a resource -/* record list. When the existing list already contains -/* var_dns_rr_list_limit elements (default: 100), -/* dns_rr_append_discard() logs a warning, sets the existing -/* list length to the sentinel value DNS_RR_DISCARDED and -/* discards the new resource record(s) instead of appending -/* them. Once a list length is set to DNS_RR_DISCARDED, the -/* caller is expected to stop trying to append records to that -/* list. +/* dns_rr_append() appends an entire input resource record +/* list to an output list, or discards the entire input list. +/* Null arguments are explicitly allowed. When the output list +/* already contains var_dns_rr_list_limit or more elements +/* (default: 100), dns_rr_append() logs a warning, flags the +/* output list as truncated, and discards the entire input +/* list. Otherwise, dns_rr_append() appends the entire input +/* list to the output list. Once an output list is flagged +/* as truncated (test with DNS_RR_IS_TRUNCATED()), the caller +/* is expected to stop trying to append records to that list. +/* Note 1: the input list is either entirely discarded or +/* entirely appended; the output list may therefore be longer +/* than var_dns_rr_list_limit. Note 2: the 'truncated' flag +/* is transitive, i.e. when appending a input list that was +/* flagged as truncated to an output list, the output list +/* will also be flagged as truncated. /* /* dns_rr_sort() sorts a list of resource records into ascending /* order according to a user-specified criterion. The result is the @@ -208,6 +207,7 @@ DNS_RR *dns_rr_create(const char *qname, const char *rname, rr->data_len = data_len; rr->next = 0; rr->len = 1; + rr->flags = 0; return (rr); } @@ -245,45 +245,29 @@ DNS_RR *dns_rr_copy(DNS_RR *src) return (dst); } -/* dns_rr_append - append resource record to list */ +/* dns_rr_append - append resource record(s) to list, or discard */ DNS_RR *dns_rr_append(DNS_RR *list, DNS_RR *rr) { + + /* + * Note: rr is not length checked; when multiple lists are concatenated, + * the output length may be a small multiple of var_dns_rr_list_limit. + */ if (rr == 0) return (list); - if (list == 0) { - list = rr; - } else { + if (list == 0) + return (rr); + if (list->len < var_dns_rr_list_limit) { list->next = dns_rr_append(list->next, rr); - /* Non-sentinel lengths are o(100) an are always safe to add. */ - if (list->len == DNS_RR_DISCARDED || rr->len == DNS_RR_DISCARDED) - list->len = DNS_RR_DISCARDED; - else - list->len += rr->len; - } - return (list); -} - -/* dns_rr_append_discard - append resource record to list, or discard */ - -DNS_RR *dns_rr_append_discard(DNS_RR *list, DNS_RR *rr) -{ - if (rr == 0) - return (list); - if (list == 0) { - list = rr; - } else if (list->len < var_dns_rr_list_limit) { - list->next = dns_rr_append(list->next, rr); - /* Non-sentinel lengths are o(100) an are always safe to add. */ - if (rr->len == DNS_RR_DISCARDED) - list->len = DNS_RR_DISCARDED; - else - list->len += rr->len; + /* Lengths are o(100) an are always safe to add. */ + list->len += rr->len; + list->flags |= rr->flags; } else { - if (list->len != DNS_RR_DISCARDED) { + if ((list->flags & DNS_RR_FLAG_TRUNCATED) == 0) { msg_warn("dropping excess records for qname=%s qtype=%s", rr->qname, dns_strtype(rr->type)); - list->len = DNS_RR_DISCARDED; + list->flags |= DNS_RR_FLAG_TRUNCATED; } dns_rr_free(rr); } diff --git a/postfix/src/dns/test_dns_lookup.c b/postfix/src/dns/test_dns_lookup.c index 380425ba1..1399334e0 100644 --- a/postfix/src/dns/test_dns_lookup.c +++ b/postfix/src/dns/test_dns_lookup.c @@ -123,9 +123,10 @@ int main(int argc, char **argv) if (rr) { vstream_printf("%s: fqdn: %s\n", name, vstring_str(fqdn)); buf = vstring_alloc(100); + vstream_printf("%s: %d records\n", name, rr->len); print_rr(buf, rr); vstream_fflush(VSTREAM_OUT); - if (rr && rr->len == DNS_RR_DISCARDED) + if (DNS_RR_IS_TRUNCATED(rr)) msg_warn("one or more excess DNS_RR records were dropped"); dns_rr_free(rr); vstring_free(buf); diff --git a/postfix/src/global/mail_version.h b/postfix/src/global/mail_version.h index 6763a14eb..741704b87 100644 --- a/postfix/src/global/mail_version.h +++ b/postfix/src/global/mail_version.h @@ -20,7 +20,7 @@ * Patches change both the patchlevel and the release date. Snapshots have no * patchlevel; they change the release date only. */ -#define MAIL_RELEASE_DATE "20240222" +#define MAIL_RELEASE_DATE "20240223" #define MAIL_VERSION_NUMBER "3.9" #ifdef SNAPSHOT diff --git a/postfix/src/posttls-finger/posttls-finger.c b/postfix/src/posttls-finger/posttls-finger.c index 4dab73087..b474a4006 100644 --- a/postfix/src/posttls-finger/posttls-finger.c +++ b/postfix/src/posttls-finger/posttls-finger.c @@ -1207,7 +1207,7 @@ static DNS_RR *addr_one(STATE *state, DNS_RR *addr_list, const char *host, host, ((struct sockaddr *) (res0->ai_addr))->sa_family); addr->pref = pref; addr->port = port; - addr_list = dns_rr_append_discard(addr_list, addr); + addr_list = dns_rr_append(addr_list, addr); freeaddrinfo(res0); return (addr_list); } @@ -1227,7 +1227,7 @@ static DNS_RR *addr_one(STATE *state, DNS_RR *addr_list, const char *host, rr->pref = pref; rr->port = port; } - addr_list = dns_rr_append_discard(addr_list, addr); + addr_list = dns_rr_append(addr_list, addr); return (addr_list); default: dsb_status(why, "4.4.3"); @@ -1279,8 +1279,8 @@ static DNS_RR *addr_one(STATE *state, DNS_RR *addr_list, const char *host, if ((addr = dns_sa_to_rr(host, pref, res->ai_addr)) == 0) msg_fatal("host %s: conversion error for address family %d: %m", host, ((struct sockaddr *) (res0->ai_addr))->sa_family); - addr_list = dns_rr_append_discard(addr_list, addr); - if (addr_list->len == DNS_RR_DISCARDED) + addr_list = dns_rr_append(addr_list, addr); + if (DNS_RR_IS_TRUNCATED(addr_list)) break; } freeaddrinfo(res0); @@ -1319,7 +1319,7 @@ static DNS_RR *mx_addr_list(STATE *state, DNS_RR *mx_names) msg_panic("%s: bad resource type: %d", myname, rr->type); addr_list = addr_one(state, addr_list, (char *) rr->data, res_opt, rr->pref, rr->port); - if (addr_list && addr_list->len == DNS_RR_DISCARDED) + if (addr_list && DNS_RR_IS_TRUNCATED(addr_list)) break; } return (addr_list); @@ -1365,10 +1365,6 @@ static DNS_RR *domain_addr(STATE *state, char *domain) dsb_status(state->why, "5.4.3"); break; case DNS_OK: - if (mx_names && mx_names->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up MX records for domain %s", - var_dns_rr_list_limit, aname); mx_names = dns_rr_sort(mx_names, dns_rr_compare_pref_any); addr_list = mx_addr_list(state, mx_names); state->mx = dns_rr_copy(mx_names); @@ -1377,10 +1373,6 @@ static DNS_RR *domain_addr(STATE *state, char *domain) msg_warn("no MX host for %s has a valid address record", domain); break; } - if (addr_list->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up MX host addresses for domain %s", - var_dns_rr_list_limit, aname); #define COMPARE_ADDR(flags) \ ((flags & MISC_FLAG_PREF_IPV6) ? dns_rr_compare_pref_ipv6 : \ (flags & MISC_FLAG_PREF_IPV4) ? dns_rr_compare_pref_ipv4 : \ @@ -1445,10 +1437,6 @@ static DNS_RR *service_addr(STATE *state, const char *domain, dsb_status(state->why, "5.4.3"); break; case DNS_OK: - if (srv_names && srv_names->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up SRV records for domain %s", - var_dns_rr_list_limit, aname); /* Shuffle then sort the SRV rr records by priority and weight. */ srv_names = dns_srv_rr_sort(srv_names); addr_list = mx_addr_list(state, srv_names); @@ -1459,10 +1447,6 @@ static DNS_RR *service_addr(STATE *state, const char *domain, str_srv_qname); break; } - if (addr_list->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up SRV host addresses for domain %s", - var_dns_rr_list_limit, aname); /* TODO: sort by priority, weight, and address family preference. */ break; case DNS_NOTFOUND: @@ -1502,11 +1486,6 @@ static DNS_RR *host_addr(STATE *state, const char *host) #define PREF0 0 #define NOPORT 0 addr_list = addr_one(state, (DNS_RR *) 0, ahost, res_opt, PREF0, NOPORT); - if (addr_list && addr_list->len == DNS_RR_DISCARDED) { - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up addresses for host %s", - var_dns_rr_list_limit, ahost); - } if (addr_list && addr_list->next) { addr_list = dns_rr_shuffle(addr_list); if (inet_proto_info()->ai_family_list[1] != 0) diff --git a/postfix/src/smtp/smtp_addr.c b/postfix/src/smtp/smtp_addr.c index 6eb2bd61b..8c384fc37 100644 --- a/postfix/src/smtp/smtp_addr.c +++ b/postfix/src/smtp/smtp_addr.c @@ -181,8 +181,8 @@ static DNS_RR *smtp_addr_one(DNS_RR *addr_list, const char *host, int res_opt, "%d: %m", host, res0->ai_addr->sa_family); addr->pref = pref; addr->port = port; - addr_list = dns_rr_append_discard(addr_list, addr); - if (msg_verbose && addr_list->len != DNS_RR_DISCARDED) + addr_list = dns_rr_append(addr_list, addr); + if (msg_verbose && !DNS_RR_IS_TRUNCATED(addr_list)) msg_info("%s: using numerical host %s", myname, host); freeaddrinfo(res0); return (addr_list); @@ -206,7 +206,7 @@ static DNS_RR *smtp_addr_one(DNS_RR *addr_list, const char *host, int res_opt, rr->pref = pref; rr->port = port; } - addr_list = dns_rr_append_discard(addr_list, addr); + addr_list = dns_rr_append(addr_list, addr); return (addr_list); default: dsb_status(why, "4.4.3"); @@ -261,8 +261,8 @@ static DNS_RR *smtp_addr_one(DNS_RR *addr_list, const char *host, int res_opt, if ((addr = dns_sa_to_rr(host, pref, res->ai_addr)) == 0) msg_fatal("host %s: conversion error for address family " "%d: %m", host, res0->ai_addr->sa_family); - addr_list = dns_rr_append_discard(addr_list, addr); - if (addr_list->len == DNS_RR_DISCARDED) + addr_list = dns_rr_append(addr_list, addr); + if (DNS_RR_IS_TRUNCATED(addr_list)) break; if (msg_verbose) { MAI_HOSTADDR_STR hostaddr_str; @@ -329,7 +329,7 @@ static DNS_RR *smtp_addr_list(DNS_RR *mx_names, DSN_BUF *why) msg_panic("smtp_addr_list: bad resource type: %d", rr->type); addr_list = smtp_addr_one(addr_list, (char *) rr->data, res_opt, rr->pref, rr->port, why); - if (addr_list && addr_list->len == DNS_RR_DISCARDED) + if (addr_list && DNS_RR_IS_TRUNCATED(addr_list)) break; } return (addr_list); @@ -424,6 +424,13 @@ static DNS_RR *smtp_balance_inet_proto(DNS_RR *addr_list, int misc_flags, * relative list order is unchanged, but some elements are removed. */ + /* + * Ensure that dns_rr_append() won't interfere with the protocol + * balancing goals. + */ + if (addr_limit > var_dns_rr_list_limit) + addr_limit = var_dns_rr_list_limit; + /* * Count the number of IPv6 and IPv4 addresses. */ @@ -500,14 +507,6 @@ static DNS_RR *smtp_balance_inet_proto(DNS_RR *addr_list, int misc_flags, msg_panic("%s: unexpected record type: %s", myname, dns_strtype(rr->type)); } - - /* - * addr_list is the concatenation of multi-element lists, and may - * be longer than var_dns_rr_limit. When both the addr_list - * length and addr_limit are > var_dns_rr_limit, calling - * dns_rr_append_discard() below could discard inputs and violate - * an invariant of the balancing algorithm. Let's keep it simple. - */ if (*p > 0) { stripped_list = dns_rr_append(stripped_list, rr); *p -= 1; @@ -630,10 +629,6 @@ DNS_RR *smtp_domain_addr(const char *name, DNS_RR **mxrr, int misc_flags, addr_list = smtp_host_addr(aname, misc_flags, why); break; case DNS_OK: - if (mx_names && mx_names->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up MX records for domain %s", - var_dns_rr_list_limit, aname); mx_names = dns_rr_sort(mx_names, dns_rr_compare_pref_any); best_pref = (mx_names ? mx_names->pref : IMPOSSIBLE_PREFERENCE); addr_list = smtp_addr_list(mx_names, why); @@ -653,10 +648,6 @@ DNS_RR *smtp_domain_addr(const char *name, DNS_RR **mxrr, int misc_flags, msg_warn("no MX host for %s has a valid address record", name); break; } - if (addr_list->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up MX host addresses for domain %s", - var_dns_rr_list_limit, aname); best_found = (addr_list ? addr_list->pref : IMPOSSIBLE_PREFERENCE); if (msg_verbose) smtp_print_addr(name, addr_list); @@ -729,10 +720,6 @@ DNS_RR *smtp_host_addr(const char *host, int misc_flags, DSN_BUF *why) */ #define PREF0 0 addr_list = smtp_addr_one((DNS_RR *) 0, ahost, res_opt, PREF0, 0, why); - if (addr_list && addr_list->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up host addresses for %s", - var_dns_rr_list_limit, ahost); if (addr_list && (misc_flags & SMTP_MISC_FLAG_LOOP_DETECT) && smtp_find_self(addr_list) != 0) { @@ -821,10 +808,6 @@ DNS_RR *smtp_service_addr(const char *name, const char *service, DNS_RR **mxrr, dsb_status(why, "5.1.0"); break; case DNS_OK: - if (srv_names && srv_names->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up SRV records for domain %s", - var_dns_rr_list_limit, aname); /* Shuffle then sort the SRV rr records by priority and weight. */ srv_names = dns_srv_rr_sort(srv_names); best_pref = (srv_names ? srv_names->pref : IMPOSSIBLE_PREFERENCE); @@ -837,10 +820,6 @@ DNS_RR *smtp_service_addr(const char *name, const char *service, DNS_RR **mxrr, str_srv_qname); break; } - if (addr_list->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up SRV host addresses for domain %s", - var_dns_rr_list_limit, aname); /* Optional loop prevention, similar to smtp_domain_addr(). */ best_found = (addr_list ? addr_list->pref : IMPOSSIBLE_PREFERENCE); if (msg_verbose) diff --git a/postfix/src/smtpd/smtpd_check.c b/postfix/src/smtpd/smtpd_check.c index fce3857de..769f7c90b 100644 --- a/postfix/src/smtpd/smtpd_check.c +++ b/postfix/src/smtpd/smtpd_check.c @@ -3134,11 +3134,6 @@ static int check_server_access(SMTPD_STATE *state, const char *table, return (SMTPD_CHECK_DUNNO); } } - if (server_list && server_list->len == DNS_RR_DISCARDED) - msg_warn("DNS resource record limit (%d) exceeded" - "while looking up %s records for %s", - var_dns_rr_list_limit, dns_strtype(type), - domain && domain[1] ? domain : name); /* * No bare returns after this point or we have a memory leak. @@ -3180,9 +3175,10 @@ static int check_server_access(SMTPD_STATE *state, const char *table, for (res = res0; res != 0; res = res->ai_next) { server_addr_count += 1; if (server_addr_count > var_dns_rr_list_limit) { - msg_warn("Host address count limit (%d) exceeded" - "while looking up host addresses for %s", - var_dns_rr_list_limit, (char *) server->data); + msg_warn("%s: %s server address count limit (%d) exceeded" + " for %s %s -- ignoring the remainder", myname, + dns_strtype(type), var_dns_rr_list_limit, + reply_class, reply_name); freeaddrinfo(res0); CHECK_SERVER_RETURN(SMTPD_CHECK_DUNNO); }