public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Siddhesh Poyarekar <siddhesh@gotplt.org>
To: Florian Weimer <fweimer@redhat.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH v2 01/13] resolv: Add tst-resolv-byaddr for testing reverse lookup
Date: Mon, 29 Aug 2022 15:56:17 -0400	[thread overview]
Message-ID: <06e5a416-17d4-87ad-32f4-c3b3a173783e@gotplt.org> (raw)
In-Reply-To: <1e53a7e6cd3fdeb9410581dc064c7d6d8b16186c.1661509943.git.fweimer@redhat.com>



On 2022-08-26 06:33, Florian Weimer via Libc-alpha wrote:
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> ---
>   resolv/Makefile                      |   2 +
>   resolv/tst-resolv-byaddr.c           | 326 +++++++++++++++++++++++++++
>   resolv/tst-resolv-maybe_insert_sig.h |  32 +++
>   3 files changed, 360 insertions(+)
>   create mode 100644 resolv/tst-resolv-byaddr.c
>   create mode 100644 resolv/tst-resolv-maybe_insert_sig.h

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

since it changed a bit :)

> 
> diff --git a/resolv/Makefile b/resolv/Makefile
> index 5b15321f9b..98b10d97a0 100644
> --- a/resolv/Makefile
> +++ b/resolv/Makefile
> @@ -91,6 +91,7 @@ tests += \
>     tst-res_hnok \
>     tst-resolv-basic \
>     tst-resolv-binary \
> +  tst-resolv-byaddr \
>     tst-resolv-edns \
>     tst-resolv-network \
>     tst-resolv-noaaaa \
> @@ -260,6 +261,7 @@ $(objpfx)tst-resolv-ai_idn-nolibidn2.out: \
>     $(gen-locales) $(objpfx)tst-no-libidn2.so
>   $(objpfx)tst-resolv-basic: $(objpfx)libresolv.so $(shared-thread-library)
>   $(objpfx)tst-resolv-binary: $(objpfx)libresolv.so $(shared-thread-library)
> +$(objpfx)tst-resolv-byaddr: $(objpfx)libresolv.so $(shared-thread-library)
>   $(objpfx)tst-resolv-edns: $(objpfx)libresolv.so $(shared-thread-library)
>   $(objpfx)tst-resolv-network: $(objpfx)libresolv.so $(shared-thread-library)
>   $(objpfx)tst-resolv-res_init: $(objpfx)libresolv.so
> diff --git a/resolv/tst-resolv-byaddr.c b/resolv/tst-resolv-byaddr.c
> new file mode 100644
> index 0000000000..6299e89837
> --- /dev/null
> +++ b/resolv/tst-resolv-byaddr.c
> @@ -0,0 +1,326 @@
> +/* Test reverse DNS lookup.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <arpa/inet.h>
> +#include <errno.h>
> +#include <netdb.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/check_nss.h>
> +#include <support/next_to_fault.h>
> +#include <support/resolv_test.h>
> +#include <support/support.h>
> +
> +#include "tst-resolv-maybe_insert_sig.h"
> +
> +/* QNAME format:
> +
> +   ADDRESSES.CNAMES...(lots of 0s)...8.b.d.0.1.0.0.2.ip6.arpa.
> +   CNAMES|ADDRESSES.2.0.192.in-addr-arpa.
> +
> +   For the IPv4 reverse lookup, the address count is in the lower
> +   bits.
> +
> +   CNAMES is the length of the CNAME chain, ADDRESSES is the number of
> +   addresses in the response.  The special value 15 means that there
> +   are no addresses, and the RCODE is NXDOMAIN.  */
> +static void
> +response (const struct resolv_response_context *ctx,
> +          struct resolv_response_builder *b,
> +          const char *qname, uint16_t qclass, uint16_t qtype)
> +{
> +  TEST_COMPARE (qclass, C_IN);
> +  TEST_COMPARE (qtype, T_PTR);
> +
> +  unsigned int addresses, cnames, bits;
> +  char *tail;
> +  if (strstr (qname, "ip6.arpa") != NULL
> +      && sscanf (qname, "%x.%x.%ms", &addresses, &cnames, &tail) == 3)
> +    TEST_COMPARE_STRING (tail, "\
> +0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa");
> +  else if (sscanf (qname, "%u.%ms", &bits, &tail) == 2)
> +    {
> +      TEST_COMPARE_STRING (tail, "2.0.192.in-addr.arpa");
> +      addresses = bits & 0x0f;
> +      cnames = bits >> 4;
> +    }
> +  else
> +    FAIL_EXIT1 ("invalid QNAME: %s", qname);
> +  free (tail);
> +
> +  int rcode;
> +  if (addresses == 15)
> +    {
> +      /* Special case: Use no addresses with NXDOMAIN response.  */
> +      rcode = ns_r_nxdomain;
> +      addresses = 0;
> +    }
> +  else
> +    rcode = 0;
> +
> +  struct resolv_response_flags flags = { .rcode = rcode };
> +  resolv_response_init (b, flags);
> +  resolv_response_add_question (b, qname, qclass, qtype);
> +  resolv_response_section (b, ns_s_an);
> +  maybe_insert_sig (b, qname);
> +
> +  /* Provide the requested number of CNAME records.  */
> +  char *previous_name = (char *) qname;
> +  for (int unique = 0; unique < cnames; ++unique)
> +    {
> +      resolv_response_open_record (b, previous_name, qclass, T_CNAME, 60);
> +      char *new_name = xasprintf ("%d.alias.example", unique);
> +      resolv_response_add_name (b, new_name);
> +      resolv_response_close_record (b);
> +
> +      maybe_insert_sig (b, qname);
> +
> +      if (previous_name != qname)
> +        free (previous_name);
> +      previous_name = new_name;
> +    }
> +
> +  for (int unique = 0; unique < addresses; ++unique)
> +    {
> +      resolv_response_open_record (b, previous_name, qclass, T_PTR, 60);
> +      char *ptr = xasprintf ("unique-%d.cnames-%u.addresses-%u.example",
> +                             unique, cnames, addresses);
> +      resolv_response_add_name (b, ptr);
> +      free (ptr);
> +      resolv_response_close_record (b);
> +    }
> +
> +  if (previous_name != qname)
> +    free (previous_name);
> +}
> +
> +/* Used to check that gethostbyaddr_r does not write past the buffer
> +   end.  */
> +static struct support_next_to_fault ntf;
> +
> +/* Perform a gethostbyaddr call and check the result.  */
> +static void
> +check_gethostbyaddr (const char *address, const char *expected)
> +{
> +  unsigned char bytes[16];
> +  unsigned int byteslen;
> +  int family;
> +  if (strchr (address, ':') != NULL)
> +    {
> +      family = AF_INET6;
> +      byteslen = 16;
> +    }
> +  else
> +    {
> +      family = AF_INET;
> +      byteslen = 4;
> +    }
> +  TEST_COMPARE (inet_pton (family, address, bytes), 1);
> +
> +  struct hostent *e = gethostbyaddr (bytes, byteslen, family);
> +  check_hostent (address, e, expected);
> +
> +  if (e == NULL)
> +    return;
> +
> +  /* Try gethostbyaddr_r with increasing sizes until success.  First
> +     compute a reasonable minimum buffer size, to avoid many pointless
> +     attempts.  */
> +  size_t minimum_size = strlen (e->h_name);
> +  for (int i = 0; e->h_addr_list[i] != NULL; ++i)
> +    minimum_size += e->h_length + sizeof (char *);
> +  for (int i = 0; e->h_aliases[i] != NULL; ++i)
> +    minimum_size += strlen (e->h_aliases[i]) + 1 + sizeof (char *);
> +
> +  /* Gradually increase the size until success.  */
> +  for (size_t size = minimum_size; size < ntf.length; ++size)
> +    {
> +      struct hostent result;
> +      int herrno;
> +      int ret = gethostbyaddr_r (bytes, byteslen, family, &result,
> +                                 ntf.buffer + ntf.length - size, size,
> +                                 &e, &herrno);
> +      if (ret == ERANGE)
> +        /* Retry with larger size.  */
> +        TEST_COMPARE (herrno, NETDB_INTERNAL);
> +      else if (ret == 0)
> +        {
> +         TEST_VERIFY (size > minimum_size);
> +         check_hostent (address, e, expected);
> +         return;
> +        }
> +      else
> +        FAIL_EXIT1 ("Unexpected gethostbyaddr_r failure: %d", ret);
> +    }
> +
> +  FAIL_EXIT1 ("gethostbyaddr_r always failed for: %s", address);
> +}
> +
> +/* Perform a getnameinfo call and check the result.  */
> +static void
> +check_getnameinfo (const char *address, const char *expected)
> +{
> +  struct sockaddr_in sin = { };
> +  struct sockaddr_in6 sin6 = { };
> +  void *sa;
> +  socklen_t salen;
> +  if (strchr (address, ':') != NULL)
> +    {
> +      sin6.sin6_family = AF_INET6;
> +      TEST_COMPARE (inet_pton (AF_INET6, address, &sin6.sin6_addr), 1);
> +      sin6.sin6_port = htons (80);
> +      sa = &sin6;
> +      salen = sizeof (sin6);
> +    }
> +  else
> +    {
> +      sin.sin_family = AF_INET;
> +      TEST_COMPARE (inet_pton (AF_INET, address, &sin.sin_addr), 1);
> +      sin.sin_port = htons (80);
> +      sa = &sin;
> +      salen = sizeof (sin);
> +    }
> +
> +  char host[64];
> +  char service[64];
> +  int ret = getnameinfo (sa, salen, host,
> +                         sizeof (host), service, sizeof (service),
> +                         NI_NAMEREQD | NI_NUMERICSERV);
> +  switch (ret)
> +    {
> +    case 0:
> +      TEST_COMPARE_STRING (host, expected);
> +      TEST_COMPARE_STRING (service, "80");
> +      break;
> +    case EAI_SYSTEM:
> +      TEST_COMPARE_STRING (strerror (errno), expected);
> +      break;
> +    default:
> +      TEST_COMPARE_STRING (gai_strerror (ret), expected);
> +    }
> +}
> +
> +static int
> +do_test (void)
> +{
> +  /* Some reasonably upper bound for the maximum response size.  */
> +  ntf = support_next_to_fault_allocate (4096);
> +
> +  struct resolv_test *obj = resolv_test_start
> +    ((struct resolv_redirect_config)
> +     {
> +       .response_callback = response
> +     });
> +
> +  for (int do_insert_sig = 0; do_insert_sig < 2; ++do_insert_sig)
> +    {
> +      insert_sig = do_insert_sig;
> +
> +      /* No PTR record, RCODE=0.  */
> +      check_gethostbyaddr ("192.0.2.0", "error: NO_RECOVERY\n");
> +      check_getnameinfo ("192.0.2.0", "Name or service not known");
> +      check_gethostbyaddr ("192.0.2.16", "error: NO_RECOVERY\n");
> +      check_getnameinfo ("192.0.2.16", "Name or service not known");
> +      check_gethostbyaddr ("192.0.2.32", "error: NO_RECOVERY\n");
> +      check_getnameinfo ("192.0.2.32", "Name or service not known");
> +      check_gethostbyaddr ("2001:db8::", "error: NO_RECOVERY\n");
> +      check_getnameinfo ("2001:db8::", "Name or service not known");
> +      check_gethostbyaddr ("2001:db8::10", "error: NO_RECOVERY\n");
> +      check_getnameinfo ("2001:db8::10", "Name or service not known");
> +      check_gethostbyaddr ("2001:db8::20", "error: NO_RECOVERY\n");
> +      check_getnameinfo ("2001:db8::20", "Name or service not known");
> +
> +      /* No PTR record, NXDOMAIN.  */
> +      check_gethostbyaddr ("192.0.2.15", "error: HOST_NOT_FOUND\n");
> +      check_getnameinfo ("192.0.2.15", "Name or service not known");
> +      check_gethostbyaddr ("192.0.2.31", "error: HOST_NOT_FOUND\n");
> +      check_getnameinfo ("192.0.2.31", "Name or service not known");
> +      check_gethostbyaddr ("192.0.2.47", "error: HOST_NOT_FOUND\n");
> +      check_getnameinfo ("192.0.2.47", "Name or service not known");
> +      check_gethostbyaddr ("2001:db8::f", "error: HOST_NOT_FOUND\n");
> +      check_getnameinfo ("2001:db8::f", "Name or service not known");
> +      check_gethostbyaddr ("2001:db8::1f", "error: HOST_NOT_FOUND\n");
> +      check_getnameinfo ("2001:db8::1f", "Name or service not known");
> +      check_gethostbyaddr ("2001:db8::2f", "error: HOST_NOT_FOUND\n");
> +      check_getnameinfo ("2001:db8::2f", "Name or service not known");
> +
> +      /* Actual response data.  Only the first PTR record is returned.  */
> +      check_gethostbyaddr ("192.0.2.1",
> +                           "name: unique-0.cnames-0.addresses-1.example\n"
> +                           "address: 192.0.2.1\n");
> +      check_getnameinfo ("192.0.2.1",
> +                         "unique-0.cnames-0.addresses-1.example");
> +      check_gethostbyaddr ("192.0.2.17",
> +                           "name: unique-0.cnames-1.addresses-1.example\n"
> +                           "address: 192.0.2.17\n");
> +      check_getnameinfo ("192.0.2.17",
> +                         "unique-0.cnames-1.addresses-1.example");
> +      check_gethostbyaddr ("192.0.2.18",
> +                           "name: unique-0.cnames-1.addresses-2.example\n"
> +                           "address: 192.0.2.18\n");
> +      check_getnameinfo ("192.0.2.18",
> +                         "unique-0.cnames-1.addresses-2.example");
> +      check_gethostbyaddr ("192.0.2.33",
> +                           "name: unique-0.cnames-2.addresses-1.example\n"
> +                           "address: 192.0.2.33\n");
> +      check_getnameinfo ("192.0.2.33",
> +                         "unique-0.cnames-2.addresses-1.example");
> +      check_gethostbyaddr ("192.0.2.34",
> +                           "name: unique-0.cnames-2.addresses-2.example\n"
> +                           "address: 192.0.2.34\n");
> +      check_getnameinfo ("192.0.2.34",
> +                         "unique-0.cnames-2.addresses-2.example");
> +
> +      /* Same for IPv6 addresses.  */
> +      check_gethostbyaddr ("2001:db8::1",
> +                           "name: unique-0.cnames-0.addresses-1.example\n"
> +                           "address: 2001:db8::1\n");
> +      check_getnameinfo ("2001:db8::1",
> +                         "unique-0.cnames-0.addresses-1.example");
> +      check_gethostbyaddr ("2001:db8::11",
> +                           "name: unique-0.cnames-1.addresses-1.example\n"
> +                           "address: 2001:db8::11\n");
> +      check_getnameinfo ("2001:db8::11",
> +                         "unique-0.cnames-1.addresses-1.example");
> +      check_gethostbyaddr ("2001:db8::12",
> +                           "name: unique-0.cnames-1.addresses-2.example\n"
> +                           "address: 2001:db8::12\n");
> +      check_getnameinfo ("2001:db8::12",
> +                         "unique-0.cnames-1.addresses-2.example");
> +      check_gethostbyaddr ("2001:db8::21",
> +                           "name: unique-0.cnames-2.addresses-1.example\n"
> +                           "address: 2001:db8::21\n");
> +      check_getnameinfo ("2001:db8::21",
> +                         "unique-0.cnames-2.addresses-1.example");
> +      check_gethostbyaddr ("2001:db8::22",
> +                           "name: unique-0.cnames-2.addresses-2.example\n"
> +                           "address: 2001:db8::22\n");
> +      check_getnameinfo ("2001:db8::22",
> +                         "unique-0.cnames-2.addresses-2.example");
> +    }
> +
> +  resolv_test_end (obj);
> +
> +  support_next_to_fault_free (&ntf);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/resolv/tst-resolv-maybe_insert_sig.h b/resolv/tst-resolv-maybe_insert_sig.h
> new file mode 100644
> index 0000000000..05725225af
> --- /dev/null
> +++ b/resolv/tst-resolv-maybe_insert_sig.h
> @@ -0,0 +1,32 @@
> +/* Code snippet for optionally inserting ignored SIG records in resolver tests.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* Set to true for an alternative pass that inserts (ignored) SIG
> +   records.  This does not alter the response, so this property is not
> +   encoded in the QNAME.  The variable needs to be volatile because
> +   leaf attributes tell GCC that the response function is not
> +   called.  */
> +static volatile bool insert_sig;
> +
> +static void
> +maybe_insert_sig (struct resolv_response_builder *b, const char *owner)
> +{
> +  resolv_response_open_record (b, owner, C_IN, T_SIG, 60);
> +  resolv_response_add_data (b, "", 1);
> +  resolv_response_close_record (b);
> +}

  reply	other threads:[~2022-08-29 19:56 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-26 10:33 [PATCH v2 00/13] nss_dns: Fix handling of non-host CNAMEs (bug 12154) Florian Weimer
2022-08-26 10:33 ` [PATCH v2 01/13] resolv: Add tst-resolv-byaddr for testing reverse lookup Florian Weimer
2022-08-29 19:56   ` Siddhesh Poyarekar [this message]
2022-08-26 10:33 ` [PATCH v2 02/13] resolv: Add tst-resolv-aliases Florian Weimer
2022-08-29 19:57   ` Siddhesh Poyarekar
2022-08-26 10:33 ` [PATCH v2 03/13] resolv: Add internal __res_binary_hnok function Florian Weimer
2022-08-26 10:33 ` [PATCH v2 04/13] resolv: Add the __ns_samebinaryname function Florian Weimer
2022-08-26 10:33 ` [PATCH v2 05/13] resolv: Add internal __ns_name_length_uncompressed function Florian Weimer
2022-08-26 10:33 ` [PATCH v2 06/13] resolv: Add DNS packet parsing helpers geared towards wire format Florian Weimer
2022-08-26 10:33 ` [PATCH v2 07/13] nss_dns: Split getanswer_ptr from getanswer_r Florian Weimer
2022-08-26 10:33 ` [PATCH v2 08/13] nss_dns: Rewrite _nss_dns_gethostbyaddr2_r and getanswer_ptr Florian Weimer
2022-08-26 10:33 ` [PATCH v2 09/13] nss_dns: Remove remnants of IPv6 address mapping Florian Weimer
2022-08-26 10:34 ` [PATCH v2 10/13] nss_dns: Rewrite getanswer_r to match getanswer_ptr (bug 12154, bug 29305) Florian Weimer
2022-08-26 10:34 ` [PATCH v2 11/13] nss_dns: In gaih_getanswer_slice, skip strange aliases (bug 12154) Florian Weimer
2022-08-26 10:34 ` [PATCH v2 12/13] resolv: Add new tst-resolv-invalid-cname Florian Weimer
2022-08-30 10:59   ` Szabolcs Nagy
2022-08-30 11:13     ` Florian Weimer
2022-08-26 10:34 ` [PATCH v2 13/13] nss_dns: Rewrite _nss_dns_gethostbyname4_r using current interfaces Florian Weimer
2022-08-29 20:01   ` Siddhesh Poyarekar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=06e5a416-17d4-87ad-32f4-c3b3a173783e@gotplt.org \
    --to=siddhesh@gotplt.org \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).