Re: family inet[46]" and NULL for getaddrinfo() (was Re: "family inet[46]" and numeric address for getaddrinfo())

On 2025-12-13 14:02 +09, YASUOKA Masahiko <[email protected]> wrote:
> Hello,
>
> On Wed, 17 Sep 2025 12:00:09 +0900 (JST)
> YASUOKA Masahiko <[email protected]> wrote:
>> On Tue, 16 Sep 2025 13:27:36 +0200
>> Florian Obser <[email protected]> wrote:
>>> Coming back to an old email.
>>> 
>>> On 2025-03-07 09:43 +09, YASUOKA Masahiko <[email protected]> wrote:
>>>> Hello,
>>>>
>>>> sshd's config has the following default values,
>>>>
>>>>   #AddressFamily any
>>>>   #ListenAddress 0.0.0.0
>>>>   #ListenAddress ::
>>>>
>>>> If we configure "family" in /etc/resolv.conf, sshd will stop listening
>>>> 0.0.0.0 or :: following the "family" config.
>>>>
>>>>   - "family inet4" => stop listening on ::
>>>>   - "family inet6" => stop listening on 0.0.0.0
>>>>
>>>> This is because getaddrinfo() doesn't convert a numeric address if the
>>>> family is disabled and NI_NUMERIC_HOST is not specified.
>>>>
>>>> I don't think this is correct behavior.
>>> 
>>> I don't think this is correct behaviour either.
>
> Other than this problem, getaddrinfo can receive NULL for hostname.
> Then returning "::" and "0.0.0.0" is expected when AI_PASSIVE.  I
> suppose that "family" in "resolv.conf" must not affect this behavior
> in this case as well, but our resolver changes the result.  (Only
> "0.0.0.0" is returned when "family inet4" is configured).
>
> Also, because the default sshd_config doesn't have "ListenAddress"
> line, the previous diff hasn't fixed the original problem of sshd.
>
> The diff fixes the problem.  It prepares the constant variables for
> IPv6 but not for IPv4 because I avoided to use htonl() in the
> initializers.
>

I think (and I've also checked with deraadt at g2k26) that using htonl()
in the initializers is fine, it's a local variable.

I'd prefer to have the v4 case symmetric to the v6 case.

I agree the change makes sense and I also like the simplification of the
code.

> We can reproduce the problem in regress/lib/libc/getaddrinfo/ by
> modifying "/etc/resolv.conf".
>
> ok?
>
> Index: lib/libc/asr/getaddrinfo_async.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/asr/getaddrinfo_async.c,v
> diff -u -p -r1.64 getaddrinfo_async.c
> --- lib/libc/asr/getaddrinfo_async.c	17 Sep 2025 10:47:30 -0000	1.64
> +++ lib/libc/asr/getaddrinfo_async.c	13 Dec 2025 04:06:05 -0000
> @@ -42,7 +42,6 @@ struct match {
>  
>  static int getaddrinfo_async_run(struct asr_query *, struct asr_result *);
>  static int get_port(const char *, const char *, int);
> -static int iter_family(struct asr_query *, int);
>  static int addrinfo_add(struct asr_query *, const struct sockaddr *, const char *);
>  static int addrinfo_from_file(struct asr_query *, int,  FILE *);
>  static int addrinfo_from_pkt(struct asr_query *, char *, size_t);
> @@ -122,6 +121,16 @@ getaddrinfo_async_run(struct asr_query *
>  		struct sockaddr_in	sain;
>  		struct sockaddr_in6	sain6;
>  	} sa;
> +	const struct sockaddr_in6	sockaddr_in6_any = {
> +		.sin6_family = AF_INET6,
> +		.sin6_len = sizeof(struct sockaddr_in6),
> +		.sin6_addr = IN6ADDR_ANY_INIT
> +	};
> +	const struct sockaddr_in6	sockaddr_in6_lo = {
> +		.sin6_family = AF_INET6,
> +		.sin6_len = sizeof(struct sockaddr_in6),
> +		.sin6_addr = IN6ADDR_LOOPBACK_INIT
> +	};
>  
>      next:
>  	switch (as->as_state) {
> @@ -230,6 +239,8 @@ getaddrinfo_async_run(struct asr_query *
>  
>  		if (!(ai->ai_flags & AI_NUMERICHOST))
>  			is_localhost = _asr_is_localhost(as->as.ai.hostname);
> +		if (as->as.ai.hostname == NULL)
> +			is_localhost = (ai->ai_flags & AI_PASSIVE)? 0 : 1;
>  		/*
>  		 * If hostname is NULL, "localhost" or falls within the
>  		 * ".localhost." domain, use local address.
> @@ -242,27 +253,25 @@ getaddrinfo_async_run(struct asr_query *
>  		 * DNS server(s).
>  		 */
>  		if (as->as.ai.hostname == NULL || is_localhost) {
> -			for (family = iter_family(as, 1);
> -			    family != -1;
> -			    family = iter_family(as, 0)) {
> -				/*
> -				 * We could use statically built sockaddrs for
> -				 * those, rather than parsing over and over.
> -				 */
> -				if (family == PF_INET)
> -					str = (ai->ai_flags & AI_PASSIVE &&
> -					    !is_localhost) ? "0.0.0.0" :
> -					    "127.0.0.1";
> -				else /* PF_INET6 */
> -					str = (ai->ai_flags & AI_PASSIVE &&
> -					    !is_localhost) ? "::" : "::1";
> -				 /* This can't fail */
> -				_asr_sockaddr_from_str(&sa.sa, family, str);
> +			if (ai->ai_family == AF_UNSPEC ||
> +			    ai->ai_family == AF_INET) {
> +				memset(&sa, 0, sizeof(sa));
> +				sa.sain.sin_family = AF_INET;
> +				sa.sain.sin_len = sizeof(struct sockaddr_in);
> +				sa.sain.sin_addr.s_addr = htonl((is_localhost)?
> +				    INADDR_LOOPBACK : INADDR_ANY);
>  				if ((r = addrinfo_add(as, &sa.sa,
> -				    "localhost."))) {
> +				    "localhost.")))
> +					ar->ar_gai_errno = r;
> +			}
> +			if (ar->ar_gai_errno == 0 &&
> +			    (ai->ai_family == AF_UNSPEC ||
> +			    ai->ai_family == AF_INET6)) {
> +				if ((r = addrinfo_add(as,
> +				    (const struct sockaddr *)((is_localhost)?
> +				    &sockaddr_in6_lo : &sockaddr_in6_any),
> +				    "localhost.")))
>  					ar->ar_gai_errno = r;
> -					break;
> -				}
>  			}
>  			if (ar->ar_gai_errno == 0 && as->as_count == 0) {
>  				ar->ar_gai_errno = EAI_NODATA;
> @@ -503,28 +512,6 @@ get_port(const char *servname, const cha
>  	endservent_r(&sed);
>  
>  	return (port);
> -}
> -
> -/*
> - * Iterate over the address families that are to be queried. Use the
> - * list on the async context, unless a specific family was given in hints.
> - */
> -static int
> -iter_family(struct asr_query *as, int first)
> -{
> -	if (first) {
> -		as->as_family_idx = 0;
> -		if (as->as.ai.hints.ai_family != PF_UNSPEC)
> -			return as->as.ai.hints.ai_family;
> -		return AS_FAMILY(as);
> -	}
> -
> -	if (as->as.ai.hints.ai_family != PF_UNSPEC)
> -		return (-1);
> -
> -	as->as_family_idx++;
> -
> -	return AS_FAMILY(as);
>  }
>  
>  /*
>

-- 
In my defence, I have been left unsupervised.