From 79670d3c32ccb37fe06f25d8192943b58606a32a Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Fri, 17 Nov 2023 16:55:17 +0000 Subject: [PATCH] Lookups: Fix dnsdb lookup of multi-chunk TXT. Bug 3054 Broken=by: f6b1f8e7d642 --- doc/doc-txt/ChangeLog | 6 ++++- src/src/dns.c | 4 +++- src/src/lookups/dnsdb.c | 45 +++++++++++++++--------------------- test/dnszones-src/db.test.ex | 1 + test/scripts/2200-dnsdb/2200 | 1 + test/src/fakens.c | 1 + test/stdout/2200 | 1 + 7 files changed, 31 insertions(+), 28 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 9d23e8db2..1663f8552 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -24,13 +24,17 @@ JH/04 Bug 3039: Fix handling of of an empty log_reject_target, with a connection_reject log_selector, under tls_on_connect. Previously with this combination, when the connect ACL rejected, a spurious paniclog entry was made. -JH/04 Fix TLS resumption for TLS-on-connect. This was broken by the advent + +JH/05 Fix TLS resumption for TLS-on-connect. This was broken by the advent of loadbalancer-detection for resumption, in 4.96 - which tries to use the EHLO response. SMTPS does not have one at the time it is starting TLS. Change the default for the smtp transport host_name_extract option to be a static string, for TLS-on-connect cases; meaning that resumption will always be attempted (unless deliberately overriden). +JH/06 Bug 3054: Fix dnsdb lookup for a TXT record with multiple chunks. This + was broken by hardening introduced for Bug 3033. + Exim version 4.97 diff --git a/src/src/dns.c b/src/src/dns.c index a652dcd31..74c5a58c5 100644 --- a/src/src/dns.c +++ b/src/src/dns.c @@ -431,7 +431,9 @@ namelen = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, dnss->aptr, if (namelen < 0) goto null_return; /* Move the pointer past the name and fill in the rest of the data structure -from the following bytes. */ +from the following bytes. We seem to be assuming here that the RR blob passed +to us by the resolver library is the same as that defined for an RR by RFC 1035 +section 3.2.1 */ TRACE trace = "R-name"; if (dnss_inc_aptr(dnsa, dnss, namelen)) goto null_return; diff --git a/src/src/lookups/dnsdb.c b/src/src/lookups/dnsdb.c index ff51dec23..8288896ca 100644 --- a/src/src/lookups/dnsdb.c +++ b/src/src/lookups/dnsdb.c @@ -387,38 +387,31 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0))) } /* Other kinds of record just have one piece of data each, but there may be - several of them, of course. */ + several of them, of course. TXT & SPF can have data in multiple chunks. */ if (yield->ptr) yield = string_catn(yield, outsep, 1); if (type == T_TXT || type == T_SPF) - { - if (!outsep2) /* output only the first item of data */ + for (unsigned data_offset = 0; data_offset + 1 < rr->size; ) { - uschar n = (rr->data)[0]; - /* size byte + data bytes must not excced the RRs length */ - if (n + 1 <= rr->size) - yield = string_catn(yield, US (rr->data+1), n); + uschar chunk_len = (rr->data)[data_offset]; + int remain; + + if (outsep2 && *outsep2 && data_offset != 0) + yield = string_catn(yield, outsep2, 1); + + /* Apparently there are resolvers that do not check RRs before passing + them on, and glibc fails to do so. So every application must... + Check for chunk len exceeding RR */ + + remain = rr->size - ++data_offset; + if (chunk_len > remain) + chunk_len = remain; + yield = string_catn(yield, US ((rr->data) + data_offset), chunk_len); + data_offset += chunk_len; + + if (!outsep2) break; /* output only the first chunk of the RR */ } - else - for (unsigned data_offset = 0; data_offset < rr->size; ) - { - uschar chunk_len = (rr->data)[data_offset]; - int remain = rr->size - data_offset; - - /* Apparently there are resolvers that do not check RRs before passing - them on, and glibc fails to do so. So every application must... - Check for chunk len exceeding RR */ - - if (chunk_len > remain) - chunk_len = remain; - - if (*outsep2 && data_offset != 0) - yield = string_catn(yield, outsep2, 1); - yield = string_catn(yield, US ((rr->data) + ++data_offset), --chunk_len); - data_offset += chunk_len; - } - } else if (type == T_TLSA) if (rr->size < 3) continue; diff --git a/test/dnszones-src/db.test.ex b/test/dnszones-src/db.test.ex index 6ff1a6af4..94ac98608 100644 --- a/test/dnszones-src/db.test.ex +++ b/test/dnszones-src/db.test.ex @@ -24,6 +24,7 @@ test.ex. SOA exim.test.ex. hostmaster.exim.test.ex 1430683638 1200 120 6 test.ex. TXT "A TXT record for test.ex." s/lash TXT "A TXT record for s/lash.test.ex." +long TXT "This is a max-length chunk 789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234" "A short chunk" "A final chunk" cname CNAME test.ex. diff --git a/test/scripts/2200-dnsdb/2200 b/test/scripts/2200-dnsdb/2200 index 96c8dc7ec..4b89a7338 100644 --- a/test/scripts/2200-dnsdb/2200 +++ b/test/scripts/2200-dnsdb/2200 @@ -4,6 +4,7 @@ exim -be test.ex ${lookup dnsdb{test.ex}{$value}fail} s/lash.test.ex ${lookup dnsdb{s/lash.test.ex}{$value}fail} txt=test.ex ${lookup dnsdb{txt=test.ex}{$value}fail} +txt=long.test.ex ${lookup dnsdb{>\n; txt=long.test.ex}{$value}fail} a=black-1.test.ex ${lookup dnsdb{a=black-1.test.ex}{$value}fail} xxx=test.ex ${lookup dnsdb{xxx=test.ex}{$value}fail} a=localhost.test.ex ${lookup dnsdb{a=localhost.test.ex}{$value}fail} diff --git a/test/src/fakens.c b/test/src/fakens.c index 2c82c5a82..0de4e60c2 100644 --- a/test/src/fakens.c +++ b/test/src/fakens.c @@ -541,6 +541,7 @@ while (fgets(CS buffer, sizeof(buffer), f) != NULL) while (*p != 0 && *p != '"') *pk++ = *p++; *pp = pk - pp - 1; break; + /*XXX need a way of doing multi-chunk TXT RRs */ case ns_t_tlsa: pk = bytefield(&p, pk); /* usage */ diff --git a/test/stdout/2200 b/test/stdout/2200 index 766d433ef..8d02a903e 100644 --- a/test/stdout/2200 +++ b/test/stdout/2200 @@ -1,6 +1,7 @@ > test.ex A TXT record for test.ex. > s/lash.test.ex A TXT record for s/lash.test.ex. > txt=test.ex A TXT record for test.ex. +> txt=long.test.ex This is a max-length chunk 789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234A short chunkA final chunk > a=black-1.test.ex V4NET.11.12.13 > Failed: lookup of "xxx=test.ex" gave DEFER: unsupported DNS record type > a=localhost.test.ex 127.0.0.1 -- 2.30.2