On Sun, Dec 20, 2020 at 10:48:01AM +0100, Florian Obser wrote:
> On Thu, Dec 17, 2020 at 12:15:16PM +0100, Otto Moerbeek wrote:
> > Hi,
>
> >
> > as noted on misc dig does not like to talk to local link addresses.
> > This fixes that case. While investigating I also found another bug:
>
> Thanks for looking into this. Looks like I got distracted while
> ripping out isc_sockaddr and did not fully clean it up. Probably
> because I found another isc_indirection to delete :/
>
> I'd rather like to get rid of isc_sockaddr_fromin* completely, see
> diff at the end.
>
> > selecting v6 or v4 addresses only from resolv.conf via the -4 or -6
> > command line argument does not work as expected.
>
> Nice catch.
>
> >
> > This fixes both. I did not test binding to a src address with this.
> > This is likely as broken as it was before.
>
> My diff fixes that, too.
>
> I still need to keep isc_sockaddr_fromin* because it's used for
> +subnet i.e. ecs. Which is broken, too. I'm having a look now.
>
> >
> > -Otto
> >
>
> > Index: lib/lwres/lwconfig.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/dig/lib/lwres/lwconfig.c,v
> > retrieving revision 1.6
> > diff -u -p -r1.6 lwconfig.c
> > --- lib/lwres/lwconfig.c 25 Feb 2020 05:00:43 -0000 1.6
> > +++ lib/lwres/lwconfig.c 17 Dec 2020 11:06:49 -0000
> > @@ -231,7 +231,7 @@ lwres_conf_parsenameserver(lwres_conf_t
> >
> > res = lwres_create_addr(word, &address, 1);
> > use_ipv4 = confdata->flags & LWRES_USEIPV4;
> > - use_ipv6 = confdata->flags & LWRES_USEIPV4;
> > + use_ipv6 = confdata->flags & LWRES_USEIPV6;
> > if (res == LWRES_R_SUCCESS &&
> > ((address.family == LWRES_ADDRTYPE_V4 && use_ipv4) ||
> > (address.family == LWRES_ADDRTYPE_V6 && use_ipv6))) {
> >
>
> OK florian for this
Committed
>
> OK for this version for the rest?
A few nits inline. With those either addressed or ignored, OK,
-Otto
>
>
> diff --git dig.c dig.c
> index a0988a0567b..6b142a03239 100644
> --- dig.c
> +++ dig.c
> @@ -17,7 +17,10 @@
> /* $Id: dig.c,v 1.18 2020/09/15 11:47:42 florian Exp $ */
>
> /*! \file */
> -#include <sys/cdefs.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +
> +#include <netdb.h>
>
> #include <errno.h>
> #include <stdlib.h>
> @@ -1275,10 +1278,7 @@ dash_option(char *option, char *next, dig_lookup_t
> **lookup,
> dns_rdatatype_t rdtype;
> dns_rdataclass_t rdclass;
> char textname[MXNAME];
> - struct in_addr in4;
> - struct in6_addr in6;
> - in_port_t srcport;
> - char *hash, *cmd;
> + char *cmd;
> uint32_t num;
> const char *errstr;
>
> @@ -1346,28 +1346,39 @@ dash_option(char *option, char *next, dig_lookup_t
> **lookup,
> if (value == NULL)
> goto invalid_option;
> switch (opt) {
> - case 'b':
> + case 'b': {
> + struct addrinfo *ai = NULL, hints;
> + int error;
> + char *hash;
> +
> + memset(&hints, 0, sizeof(hints));
> + hints.ai_flags = AI_NUMERICHOST;
> + hints.ai_socktype = SOCK_STREAM;
It does not realy matter for the rsult, but SOCK_DGRAM feels more
natural for DNS.
> +
> hash = strchr(value, '#');
> if (hash != NULL) {
> - num = strtonum(hash + 1, 0, MAXPORT, &errstr);
> - if (errstr != NULL)
> - fatal("port number is %s: '%s'", errstr,
> - hash + 1);
> - srcport = num;
> *hash = '\0';
> + error = getaddrinfo(value, hash + 1, &hints, &ai);
> + *hash = '#';
> } else
> - srcport = 0;
> - if (have_ipv6 && inet_pton(AF_INET6, value, &in6) == 1)
> - isc_sockaddr_fromin6(&bind_address, &in6, srcport);
> - else if (have_ipv4 && inet_pton(AF_INET, value, &in4) == 1)
> - isc_sockaddr_fromin(&bind_address, &in4, srcport);
> - else
> + error = getaddrinfo(value, NULL, &hints, &ai);
> +
> + if (error)
> + fatal("invalid address %s: %s", value,
> + gai_strerror(error));
> + if (ai == NULL || ai->ai_addrlen > sizeof(bind_address))
> + fatal("invalid address %s", value);
> + if (!have_ipv4 && ai->ai_family == AF_INET)
> + fatal("invalid address %s", value);
Please be more specific, like "%s: wrong address family"
> + if (!have_ipv6 && ai->ai_family == AF_INET6)
> fatal("invalid address %s", value);
Same here
>
> - if (hash != NULL)
> - *hash = '#';
> + memset(&bind_address, 0, sizeof(bind_address));
> + memcpy(&bind_address, ai->ai_addr, ai->ai_addrlen);
> +
> specified_source = 1;
> return (value_from_next);
> + }
> case 'c':
> if ((*lookup)->rdclassset) {
> fprintf(stderr, ";; Warning, extra class option\n");
> diff --git dighost.c dighost.c
> index 52afde3d7d3..3974843600d 100644
> --- dighost.c
> +++ dighost.c
> @@ -498,6 +498,7 @@ get_addresses(const char *hostname, in_port_t dstport,
> {
> struct addrinfo *ai = NULL, *tmpai, hints;
> int result, i;
> + char dport[sizeof("65535")];
>
> REQUIRE(hostname != NULL);
> REQUIRE(addrs != NULL);
> @@ -515,7 +516,8 @@ get_addresses(const char *hostname, in_port_t dstport,
> }
> hints.ai_socktype = SOCK_STREAM;
>
> - result = getaddrinfo(hostname, NULL, &hints, &ai);
> + snprintf(dport, sizeof(dport), "%d", dstport);
> + result = getaddrinfo(hostname, dport, &hints, &ai);
> switch (result) {
> case 0:
> break;
> @@ -532,16 +534,10 @@ get_addresses(const char *hostname, in_port_t dstport,
> if (tmpai->ai_family != AF_INET &&
> tmpai->ai_family != AF_INET6)
> continue;
> - if (tmpai->ai_family == AF_INET) {
> - struct sockaddr_in *sin;
> - sin = (struct sockaddr_in *)tmpai->ai_addr;
> - isc_sockaddr_fromin(&addrs[i], &sin->sin_addr, dstport);
> - } else {
> - struct sockaddr_in6 *sin6;
> - sin6 = (struct sockaddr_in6 *)tmpai->ai_addr;
> - isc_sockaddr_fromin6(&addrs[i], &sin6->sin6_addr,
> - dstport);
> - }
> + if (tmpai->ai_addrlen > sizeof(addrs[i]))
> + continue;
> + memset(&addrs[i], 0, sizeof(addrs[i]));
> + memcpy(&addrs[i], tmpai->ai_addr, tmpai->ai_addrlen);
> i++;
>
> }
>
>
> --
> I'm not entirely sure you are real.
