From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 20576 invoked from network); 5 Jul 2023 03:41:27 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 5 Jul 2023 03:41:27 -0000 Received: (qmail 1119 invoked by uid 550); 5 Jul 2023 03:41:24 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 1079 invoked from network); 5 Jul 2023 03:41:23 -0000 Date: Tue, 4 Jul 2023 23:41:12 -0400 From: Rich Felker To: Hamish Forbes Cc: musl@lists.openwall.com Message-ID: <20230705034111.GD4163@brightrain.aerifal.cx> References: <20230704032918.GB4163@brightrain.aerifal.cx> <20230704160700.GC4163@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="xdWF/UuCWMRSqXrg" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] DNS answer buffer is too small --xdWF/UuCWMRSqXrg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jul 05, 2023 at 12:18:11PM +1200, Hamish Forbes wrote: > On Wed, 5 Jul 2023 at 04:06, Rich Felker wrote: > > > > On Tue, Jul 04, 2023 at 04:19:16PM +1200, Hamish Forbes wrote: > > > On Tue, 4 Jul 2023 at 15:29, Rich Felker wrote: > > It's as safe as it was before, and it's always been the intended > > behavior. Aside from the CNAME chain issue which wasn't even realized > > at the time, use of TCP for getaddrinfo is not about getting more > > answers than fit in the UDP response size. It's about handling the > > case where the recursive server returns a truncated response with zero > > answer records instead of the max number that fit. It turned out this > > could also occur with a single CNAME where both the queried name and > > the CNAME target take up nearly the full 255 length. > > > > As for why not to care about more results, getaddrinfo does not > > provide a precise view of DNS space. It takes a hostname and gives you > > a set of addresses you can use to attempt to connect to that host (or > > bind if that name is your own, etc.). There's very little utility in > > timing out more than 47 times then continuing to try more addresses > > rather than just failing. "Our name resolves to 100 addresses and you > > have to try all of them to find the one that works" is not a viable > > configuration. (A lot of software does not even iterate and try > > fallbacks at all, but only attempts to use the first one, typically > > round-robin rotated by the nameserver.) > > > > Anyway, if there are objections to this behavior, it's a completely > > separate issue from handling long CNAME chains. > > Ah yeah, ok that makes sense. > I wasn't thinking about it as "we just need any address". > No objections to that from me! > > > From my reading of your links, and > > > > https://groups.google.com/g/comp.protocols.dns.bind/c/rXici9NvIqI > > > > I don't think max-recursion-depth is related to CNAMEs. It's the depth > > of delegation recursion. The max CNAME chain length is separate, and > > in unbound terminology is the number of "restarts". Unbound's limit as > > you've found is 11. BIND's is supposedly hard-coded at 16. > > > > Assuming the recursive server uses pointers properly, max size of a > > length-N CNAME chain is (N+1)*(255+epsilon). This comes out to a > > little over 4k for the BIND limit, and that's assuming max-length > > names with no further redundancy. I would expect the real-world need > > is considerably lower than this, and that the Unbound default limit on > > chain length also suffices in practice (or it wouldn't be the default > > for a widely used recursive server). So, for example, using a 4k > > buffer (adding a little over 3k to what we have now, which already had > > enough for one CNAME) should solve the problem entirely. > > > > Does this sound like an okay fix to you? > > Sounds good to me! Great. Writing up the commit message, I figured it's not much larger to go with supporting the full 16, and easier to justify. Proposed patch attached. Rich --xdWF/UuCWMRSqXrg Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0001-dns-stub-resolver-increase-buffer-size-to-handle-cha.patch" >From a4ecaf89a9b88df76e8bf9f28e1cc6cb89e4bfa8 Mon Sep 17 00:00:00 2001 From: Rich Felker Date: Tue, 4 Jul 2023 23:11:17 -0400 Subject: [PATCH] dns stub resolver: increase buffer size to handle chained CNAMEs in the event of chained CNAMEs, the answer to a query will contain the entire CNAME chain, not just one CNAME record. previously, the answer buffer size had been chosen to admit a maximal-length CNAME, but only one. a moderate-length chain could fill the available 768 bytes leaving no room for an actual address answering the query. while the DNS RFCs do not specify any limit on the length of a CNAME chain, or any reasonable behavior is the chain exceeds the entire 64k possible message size, actual recursive servers have to impose a limit, and a such, for all practical purposes, chains longer than this limit are not usable. it turns out BIND has a hard-coded limit of 16, and Unbound has a default limit of 11. assuming the recursive server makes use of "compression" (pointers), each maximal-length CNAME record takes at most 268 bytes, and thus any chain up to length 16 fits in at most 4288 bytes. this patch increases the answer buffer size to preserve the original intent of having 512 bytes available for address answers, plus space needed for a maximal CNAME chain, for a total of 4800 bytes. the resulting size of 9600 bytes for two queries (A+AAAA) is still well within what is reasonable to place in automatic storage. --- src/network/lookup_name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/lookup_name.c b/src/network/lookup_name.c index 4281482e..35218185 100644 --- a/src/network/lookup_name.c +++ b/src/network/lookup_name.c @@ -109,7 +109,7 @@ struct dpc_ctx { #define RR_CNAME 5 #define RR_AAAA 28 -#define ABUF_SIZE 768 +#define ABUF_SIZE 4800 static int dns_parse_callback(void *c, int rr, const void *data, int len, const void *packet, int plen) { -- 2.21.0 --xdWF/UuCWMRSqXrg--