Re: [musl] DNS answer buffer is too small

[Rich Felker <dalias@libc.org>]

[-- Attachment #1: Type: text/plain, Size: 3069 bytes --]

On Wed, Jul 05, 2023 at 12:18:11PM +1200, Hamish Forbes wrote:
> On Wed, 5 Jul 2023 at 04:06, Rich Felker <dalias@libc.org> wrote:
> >
> > On Tue, Jul 04, 2023 at 04:19:16PM +1200, Hamish Forbes wrote:
> > > On Tue, 4 Jul 2023 at 15:29, Rich Felker <dalias@libc.org> 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

[-- Attachment #2: 0001-dns-stub-resolver-increase-buffer-size-to-handle-cha.patch --]
[-- Type: text/plain, Size: 1966 bytes --]

From a4ecaf89a9b88df76e8bf9f28e1cc6cb89e4bfa8 Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
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