From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2178) id EB4163858293; Wed, 21 Sep 2022 18:01:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EB4163858293 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1663783299; bh=K1SqbsKg977PXy+2vXHIDYDt2nkctitYwhojJ7KlVTo=; h=From:To:Subject:Date:From; b=qu1cfdJ7S2alD2tucIQLATvP3hQMfXujzk/Oif2ryZftHddrre2ru8Z9lQ/GW01kq cPO5TpGu3A174lYB7JgEOmcuxeEfdOHXTiyp0jo8EdhtNlgX9SAI9pvOskTT6m15RV yMJZ4OB6+E0gmXNhW3aukfgJviSqzphr/0kKqZPA= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Florian Weimer To: glibc-cvs@sourceware.org Subject: [glibc/release/2.34/master] nss_dns: Rewrite _nss_dns_gethostbyname4_r using current interfaces X-Act-Checkin: glibc X-Git-Author: Florian Weimer X-Git-Refname: refs/heads/release/2.34/master X-Git-Oldrev: 480c820493add16e8dda6f3189d834223e1f4f39 X-Git-Newrev: 2def56a3498e3f89c866b7ef79649ee5a8a291c6 Message-Id: <20220921180139.EB4163858293@sourceware.org> Date: Wed, 21 Sep 2022 18:01:39 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=2def56a3498e3f89c866b7ef79649ee5a8a291c6 commit 2def56a3498e3f89c866b7ef79649ee5a8a291c6 Author: Florian Weimer Date: Tue Sep 20 12:38:22 2022 +0200 nss_dns: Rewrite _nss_dns_gethostbyname4_r using current interfaces Introduce struct alloc_buffer to this function, and use it and struct ns_rr_cursor in gaih_getanswer_slice. Adjust gaih_getanswer and gaih_getanswer_noaaaa accordingly. Reviewed-by: Siddhesh Poyarekar (cherry picked from commit 1d495912a746e2a1ffb780c9a81fd234ec2464e8) (conflict in resolv/nss_dns/dns-host.c due to missing noaaaa support) Diff: --- resolv/nss_dns/dns-host.c | 412 +++++++++++++++++----------------------------- 1 file changed, 150 insertions(+), 262 deletions(-) diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c index 2d8b839568..780d3d1c83 100644 --- a/resolv/nss_dns/dns-host.c +++ b/resolv/nss_dns/dns-host.c @@ -101,13 +101,6 @@ #endif #define MAXHOSTNAMELEN 256 -/* We need this time later. */ -typedef union querybuf -{ - HEADER hdr; - u_char buf[MAXPACKET]; -} querybuf; - /* For historic reasons, pointers to IP addresses are char *, so use a single list type for addresses and host names. */ #define DYNARRAY_STRUCT ptrlist @@ -126,11 +119,12 @@ static enum nss_status getanswer_ptr (unsigned char *packet, size_t packetlen, char **hnamep, int *errnop, int *h_errnop, int32_t *ttlp); -static enum nss_status gaih_getanswer (const querybuf *answer1, int anslen1, - const querybuf *answer2, int anslen2, - const char *qname, +static enum nss_status gaih_getanswer (unsigned char *packet1, + size_t packet1len, + unsigned char *packet2, + size_t packet2len, + struct alloc_buffer *abuf, struct gaih_addrtuple **pat, - char *buffer, size_t buflen, int *errnop, int *h_errnop, int32_t *ttlp); @@ -401,27 +395,23 @@ _nss_dns_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat, name = cp; } - union - { - querybuf *buf; - u_char *ptr; - } host_buffer; - querybuf *orig_host_buffer; - host_buffer.buf = orig_host_buffer = (querybuf *) alloca (2048); + unsigned char dns_packet_buffer[2048]; + unsigned char *alt_dns_packet_buffer = dns_packet_buffer; u_char *ans2p = NULL; int nans2p = 0; int resplen2 = 0; int ans2p_malloced = 0; + struct alloc_buffer abuf = alloc_buffer_create (buffer, buflen); int olderr = errno; int n = __res_context_search (ctx, name, C_IN, T_QUERY_A_AND_AAAA, - host_buffer.buf->buf, 2048, &host_buffer.ptr, - &ans2p, &nans2p, &resplen2, &ans2p_malloced); + dns_packet_buffer, sizeof (dns_packet_buffer), + &alt_dns_packet_buffer, &ans2p, &nans2p, + &resplen2, &ans2p_malloced); if (n >= 0) { - status = gaih_getanswer (host_buffer.buf, n, (const querybuf *) ans2p, - resplen2, name, pat, buffer, buflen, - errnop, herrnop, ttlp); + status = gaih_getanswer (alt_dns_packet_buffer, n, ans2p, resplen2, + &abuf, pat, errnop, herrnop, ttlp); } else { @@ -452,12 +442,20 @@ _nss_dns_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat, __set_errno (olderr); } + /* Implement the buffer resizing protocol. */ + if (alloc_buffer_has_failed (&abuf)) + { + *errnop = ERANGE; + *herrnop = NETDB_INTERNAL; + status = NSS_STATUS_TRYAGAIN; + } + /* Check whether ans2p was separately allocated. */ if (ans2p_malloced) free (ans2p); - if (host_buffer.buf != orig_host_buffer) - free (host_buffer.buf); + if (alt_dns_packet_buffer != dns_packet_buffer) + free (alt_dns_packet_buffer); __resolv_context_put (ctx); return status; @@ -871,259 +869,152 @@ getanswer_ptr (unsigned char *packet, size_t packetlen, return NSS_STATUS_TRYAGAIN; } +/* Parses DNS data found in PACKETLEN bytes at PACKET in struct + gaih_addrtuple address tuples. The new address tuples are linked + from **TAILP, with backing store allocated from ABUF, and *TAILP is + updated to point where the next tuple pointer should be stored. If + TTLP is not null, *TTLP is updated to reflect the minimum TTL. If + STORE_CANON is true, the canonical name is stored as part of the + first address tuple being written. */ static enum nss_status -gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname, - struct gaih_addrtuple ***patp, - char **bufferp, size_t *buflenp, - int *errnop, int *h_errnop, int32_t *ttlp, int *firstp) +gaih_getanswer_slice (unsigned char *packet, size_t packetlen, + struct alloc_buffer *abuf, + struct gaih_addrtuple ***tailp, + int *errnop, int *h_errnop, int32_t *ttlp, + bool store_canon) { - char *buffer = *bufferp; - size_t buflen = *buflenp; - - struct gaih_addrtuple **pat = *patp; - const HEADER *hp = &answer->hdr; - int ancount = ntohs (hp->ancount); - int qdcount = ntohs (hp->qdcount); - const u_char *cp = answer->buf + HFIXEDSZ; - const u_char *end_of_message = answer->buf + anslen; - if (__glibc_unlikely (qdcount != 1)) - { - *h_errnop = NO_RECOVERY; - return NSS_STATUS_UNAVAIL; - } - - u_char packtmp[NS_MAXCDNAME]; - int n = __ns_name_unpack (answer->buf, end_of_message, cp, - packtmp, sizeof packtmp); - /* We unpack the name to check it for validity. But we do not need - it later. */ - if (n != -1 && __ns_name_ntop (packtmp, buffer, buflen) == -1) - { - if (__glibc_unlikely (errno == EMSGSIZE)) - { - too_small: - *errnop = ERANGE; - *h_errnop = NETDB_INTERNAL; - return NSS_STATUS_TRYAGAIN; - } - - n = -1; - } - - if (__glibc_unlikely (n < 0)) - { - *errnop = errno; - *h_errnop = NO_RECOVERY; - return NSS_STATUS_UNAVAIL; - } - if (__glibc_unlikely (__libc_res_hnok (buffer) == 0)) + struct ns_rr_cursor c; + if (!__ns_rr_cursor_init (&c, packet, packetlen)) { - errno = EBADMSG; - *errnop = EBADMSG; + /* This should not happen because __res_context_query already + perfroms response validation. */ *h_errnop = NO_RECOVERY; return NSS_STATUS_UNAVAIL; } - cp += n + QFIXEDSZ; + bool haveanswer = false; /* Set to true if at least one address. */ + uint16_t qtype = ns_rr_cursor_qtype (&c); + int ancount = ns_rr_cursor_ancount (&c); + const unsigned char *expected_name = ns_rr_cursor_qname (&c); + /* expected_name may be updated to point into this buffer. */ + unsigned char name_buffer[NS_MAXCDNAME]; - int haveanswer = 0; - int had_error = 0; - char *canon = NULL; - char *h_name = NULL; - int h_namelen = 0; + /* This is a pointer to a possibly-compressed name in the packet. + Eventually it is equivalent to the canonical name. If needed, it + is uncompressed and translated to text form when the first + address tuple is encountered. */ + const unsigned char *compressed_alias_name = expected_name; - if (ancount == 0) + if (ancount == 0 || !__res_binary_hnok (compressed_alias_name)) { *h_errnop = HOST_NOT_FOUND; return NSS_STATUS_NOTFOUND; } - while (ancount-- > 0 && cp < end_of_message && had_error == 0) + for (; ancount > -0; --ancount) { - n = __ns_name_unpack (answer->buf, end_of_message, cp, - packtmp, sizeof packtmp); - if (n != -1 && - (h_namelen = __ns_name_ntop (packtmp, buffer, buflen)) == -1) - { - if (__glibc_unlikely (errno == EMSGSIZE)) - goto too_small; - - n = -1; - } - if (__glibc_unlikely (n < 0)) - { - ++had_error; - continue; - } - if (*firstp && canon == NULL && __libc_res_hnok (buffer)) - { - h_name = buffer; - buffer += h_namelen; - buflen -= h_namelen; - } - - cp += n; /* name */ - - if (__glibc_unlikely (cp + 10 > end_of_message)) - { - ++had_error; - continue; - } - - uint16_t type; - NS_GET16 (type, cp); - uint16_t class; - NS_GET16 (class, cp); - int32_t ttl; - NS_GET32 (ttl, cp); - NS_GET16 (n, cp); /* RDATA length. */ - - if (end_of_message - cp < n) + struct ns_rr_wire rr; + if (!__ns_rr_cursor_next (&c, &rr)) { - /* RDATA extends beyond the end of the packet. */ - ++had_error; - continue; + *h_errnop = NO_RECOVERY; + return NSS_STATUS_UNAVAIL; } - if (class != C_IN) - { - cp += n; - continue; - } + /* Update TTL for known record types. */ + if ((rr.rtype == T_CNAME || rr.rtype == qtype) + && ttlp != NULL && *ttlp > rr.ttl) + *ttlp = rr.ttl; - if (type == T_CNAME) + if (rr.rtype == T_CNAME) { - char tbuf[MAXDNAME]; - - /* A CNAME could also have a TTL entry. */ - if (ttlp != NULL && ttl < *ttlp) - *ttlp = ttl; - - n = __libc_dn_expand (answer->buf, end_of_message, cp, - tbuf, sizeof tbuf); - if (__glibc_unlikely (n < 0)) - { - ++had_error; - continue; - } - cp += n; - - if (*firstp && __libc_res_hnok (tbuf)) + /* NB: No check for owner name match, based on historic + precedent. Record the CNAME target as the new expected + name. */ + int n = __ns_name_unpack (c.begin, c.end, rr.rdata, + name_buffer, sizeof (name_buffer)); + if (n < 0) { - /* Reclaim buffer space. */ - if (h_name + h_namelen == buffer) - { - buffer = h_name; - buflen += h_namelen; - } - - n = strlen (tbuf) + 1; - if (__glibc_unlikely (n > buflen)) - goto too_small; - if (__glibc_unlikely (n >= MAXHOSTNAMELEN)) - { - ++had_error; - continue; - } - - canon = buffer; - buffer = __mempcpy (buffer, tbuf, n); - buflen -= n; - h_namelen = 0; + *h_errnop = NO_RECOVERY; + return NSS_STATUS_UNAVAIL; } - continue; + expected_name = name_buffer; + if (store_canon && __res_binary_hnok (name_buffer)) + /* This name can be used as a canonical name. Do not + translate to text form here to conserve buffer space. + Point to the compressed name because name_buffer can be + overwritten with an unusable name later. */ + compressed_alias_name = rr.rdata; } - - /* Stop parsing if we encounter a record with incorrect RDATA - length. */ - if (type == T_A || type == T_AAAA) + else if (rr.rtype == qtype + && __ns_samebinaryname (rr.rname, expected_name) + && rr.rdlength == rrtype_to_rdata_length (qtype)) { - if (n != rrtype_to_rdata_length (type)) + struct gaih_addrtuple *ntup + = alloc_buffer_alloc (abuf, struct gaih_addrtuple); + /* Delay error reporting to the callers (they implement the + ERANGE buffer resizing handshake). */ + if (ntup != NULL) { - ++had_error; - continue; + ntup->next = NULL; + if (store_canon && compressed_alias_name != NULL) + { + /* This assumes that all the CNAME records come + first. Use MAXHOSTNAMELEN instead of + NS_MAXCDNAME for additional length checking. + However, these checks are not expected to fail + because all size NS_MAXCDNAME names should into + the hname buffer because no escaping is + needed. */ + char unsigned nbuf[NS_MAXCDNAME]; + char hname[MAXHOSTNAMELEN + 1]; + if (__ns_name_unpack (c.begin, c.end, + compressed_alias_name, + nbuf, sizeof (nbuf)) >= 0 + && __ns_name_ntop (nbuf, hname, sizeof (hname)) >= 0) + /* Space checking is performed by the callers. */ + ntup->name = alloc_buffer_copy_string (abuf, hname); + store_canon = false; + } + else + ntup->name = NULL; + if (rr.rdlength == 4) + ntup->family = AF_INET; + else + ntup->family = AF_INET6; + memcpy (ntup->addr, rr.rdata, rr.rdlength); + ntup->scopeid = 0; + + /* Link in the new tuple, and update the tail pointer to + point to its next field. */ + **tailp = ntup; + *tailp = &ntup->next; + + haveanswer = true; } } - else - { - /* Skip unknown records. */ - cp += n; - continue; - } - - assert (type == T_A || type == T_AAAA); - if (*pat == NULL) - { - uintptr_t pad = (-(uintptr_t) buffer - % __alignof__ (struct gaih_addrtuple)); - buffer += pad; - buflen = buflen > pad ? buflen - pad : 0; - - if (__glibc_unlikely (buflen < sizeof (struct gaih_addrtuple))) - goto too_small; - - *pat = (struct gaih_addrtuple *) buffer; - buffer += sizeof (struct gaih_addrtuple); - buflen -= sizeof (struct gaih_addrtuple); - } - - (*pat)->name = NULL; - (*pat)->next = NULL; - - if (*firstp) - { - /* We compose a single hostent out of the entire chain of - entries, so the TTL of the hostent is essentially the lowest - TTL in the chain. */ - if (ttlp != NULL && ttl < *ttlp) - *ttlp = ttl; - - (*pat)->name = canon ?: h_name; - - *firstp = 0; - } - - (*pat)->family = type == T_A ? AF_INET : AF_INET6; - memcpy ((*pat)->addr, cp, n); - cp += n; - (*pat)->scopeid = 0; - - pat = &((*pat)->next); - - haveanswer = 1; } if (haveanswer) { - *patp = pat; - *bufferp = buffer; - *buflenp = buflen; - *h_errnop = NETDB_SUCCESS; return NSS_STATUS_SUCCESS; } - - /* Special case here: if the resolver sent a result but it only - contains a CNAME while we are looking for a T_A or T_AAAA record, - we fail with NOTFOUND instead of TRYAGAIN. */ - if (canon != NULL) + else { + /* Special case here: if the resolver sent a result but it only + contains a CNAME while we are looking for a T_A or T_AAAA + record, we fail with NOTFOUND. */ *h_errnop = HOST_NOT_FOUND; return NSS_STATUS_NOTFOUND; } - - *h_errnop = NETDB_INTERNAL; - return NSS_STATUS_TRYAGAIN; } static enum nss_status -gaih_getanswer (const querybuf *answer1, int anslen1, const querybuf *answer2, - int anslen2, const char *qname, - struct gaih_addrtuple **pat, char *buffer, size_t buflen, +gaih_getanswer (unsigned char *packet1, size_t packet1len, + unsigned char *packet2, size_t packet2len, + struct alloc_buffer *abuf, struct gaih_addrtuple **pat, int *errnop, int *h_errnop, int32_t *ttlp) { - int first = 1; - enum nss_status status = NSS_STATUS_NOTFOUND; /* Combining the NSS status of two distinct queries requires some @@ -1135,7 +1026,10 @@ gaih_getanswer (const querybuf *answer1, int anslen1, const querybuf *answer2, between TRYAGAIN (recoverable) and TRYAGAIN' (not-recoverable). A recoverable TRYAGAIN is almost always due to buffer size issues and returns ERANGE in errno and the caller is expected to retry - with a larger buffer. + with a larger buffer. (The caller, _nss_dns_gethostbyname4_r, + ignores the return status if it detects that the result buffer + has been exhausted and generates a TRYAGAIN failure with an + ERANGE code.) Lastly, you may be tempted to make significant changes to the conditions in this code to bring about symmetry between responses. @@ -1215,36 +1109,30 @@ gaih_getanswer (const querybuf *answer1, int anslen1, const querybuf *answer2, is a recoverable error we now return TRYAGIN even if the first response was SUCCESS. */ - if (anslen1 > 0) - status = gaih_getanswer_slice(answer1, anslen1, qname, - &pat, &buffer, &buflen, - errnop, h_errnop, ttlp, - &first); - - if ((status == NSS_STATUS_SUCCESS || status == NSS_STATUS_NOTFOUND - || (status == NSS_STATUS_TRYAGAIN - /* We want to look at the second answer in case of an - NSS_STATUS_TRYAGAIN only if the error is non-recoverable, i.e. - *h_errnop is NO_RECOVERY. If not, and if the failure was due to - an insufficient buffer (ERANGE), then we need to drop the results - and pass on the NSS_STATUS_TRYAGAIN to the caller so that it can - repeat the query with a larger buffer. */ - && (*errnop != ERANGE || *h_errnop == NO_RECOVERY))) - && answer2 != NULL && anslen2 > 0) + if (packet1len > 0) + { + status = gaih_getanswer_slice (packet1, packet1len, + abuf, &pat, errnop, h_errnop, ttlp, true); + if (alloc_buffer_has_failed (abuf)) + /* Do not try parsing the second packet if a larger result + buffer is needed. The caller implements the resizing + protocol because *abuf has been exhausted. */ + return NSS_STATUS_TRYAGAIN; /* Ignored by the caller. */ + } + + if ((status == NSS_STATUS_SUCCESS || status == NSS_STATUS_NOTFOUND) + && packet2 != NULL && packet2len > 0) { - enum nss_status status2 = gaih_getanswer_slice(answer2, anslen2, qname, - &pat, &buffer, &buflen, - errnop, h_errnop, ttlp, - &first); + enum nss_status status2 + = gaih_getanswer_slice (packet2, packet2len, + abuf, &pat, errnop, h_errnop, ttlp, + /* Success means that data with a + canonical name has already been + stored. Do not store the name again. */ + status != NSS_STATUS_SUCCESS); /* Use the second response status in some cases. */ if (status != NSS_STATUS_SUCCESS && status2 != NSS_STATUS_NOTFOUND) status = status2; - /* Do not return a truncated second response (unless it was - unavoidable e.g. unrecoverable TRYAGAIN). */ - if (status == NSS_STATUS_SUCCESS - && (status2 == NSS_STATUS_TRYAGAIN - && *errnop == ERANGE && *h_errnop != NO_RECOVERY)) - status = NSS_STATUS_TRYAGAIN; } return status;