ports/65043: [patch] net/rsync: problems in client name lookup code

[Alex Vasylenko]

Wayne,

I'm changing the subject and keeping your text in an attempt to get this 
recorded in gnats.

Wayne Davison wrote:
> On Thu, Apr 01, 2004 at 11:31:00AM -0500, Alex Vasylenko wrote:
> 
>>	- detection of IPv6 via counting dots is plain silly and doesn't work;
> 
> 
> Thanks, I've tweaked your patch for this a little and checked it in.
> It's attached to this email.
> 
>>	- fails to set ss_len member of struct sockaddr_storage, which when
>>	  passed to getnameinfo causes the latter to fail;
> 
> 
> Not everyone has ss_len in sockaddr_storage, so this part of the patch
> doesn't work.  It looked like the only place you set the value was after
> calling client_sockaddr() (though you only affected one of the two
> places it was called).  

ss.ss_len is set correctly in the answer structure returned from 
getnameinfo, so no need to set it explicitly there. Setting ss_len 
inside "switch (answer->ai_family)" like you do is OK, as long as ss_len 
must be passed as a parameter to lookup_name.

> So, might it be better to set the value inside
> the client_sockaddr() routine?
> 
> Attached is my guess at how this might be fixed.  This looks odd,
> though, so I think this is probably not the right fix.  Can you (or
> anyone with access to a system that uses ss_len) let me know if this
> second patch is what we need, and if not, what the correct fix is.

I think your second patch is superfluous, as getpeername returns a 
correctly formatted sockaddr (with _len set, if it's there), so it 
should be sufficient to simply drop the "ss.ss_len = ss_len;" line from 
my original patch

> 
> ------------------------------------------------------------------------
> 
> --- clientname.c	1 Apr 2004 21:08:24 -0000	1.20
> +++ clientname.c	1 Apr 2004 21:36:31 -0000
> @@ -100,11 +100,7 @@ char *client_name(int fd)
>  	static char name_buf[100];
>  	static char port_buf[100];
>  	static int initialised;
> -	struct sockaddr_storage ss, *ssp;
> -	struct sockaddr_in sin;
> -#ifdef INET6
> -	struct sockaddr_in6 sin6;
> -#endif
> +	struct sockaddr_storage ss;
>  	socklen_t ss_len;
>  
>  	if (initialised)
> @@ -113,43 +109,45 @@ char *client_name(int fd)
>  	strcpy(name_buf, default_name);
>  	initialised = 1;
>  
> +	memset(&ss, 0, sizeof ss);
> +
>  	if (am_server) {	/* daemon over --rsh mode */
>  		char *addr = client_addr(fd);
> -#ifdef INET6
> -		int dots = 0;
> -		char *p;
> +		struct addrinfo hint, *answer;
> +		int err;
> +
> +		memset(&hint, 0, sizeof hint);
>  
> -		for (p = addr; *p && (dots <= 3); p++) {
> -			if (*p == '.')
> -				dots++;
> +		hint.ai_flags = AI_NUMERICHOST;
> +		hint.ai_socktype = SOCK_STREAM;
> +
> +		if ((err = getaddrinfo(addr, NULL, &hint, &answer)) != 0) {
> +			rprintf(FERROR, RSYNC_NAME ": malformed address %s: %s\n",
> +			        addr, gai_strerror(err));
> +			return name_buf;
>  		}
> -		if (dots > 3) {
> -			/* more than 4 parts to IP address, must be ipv6 */
> -			ssp = (struct sockaddr_storage *) &sin6;
> -			ss_len = sizeof sin6;
> -			memset(ssp, 0, ss_len);
> -			inet_pton(AF_INET6, addr, &sin6.sin6_addr);
> -			sin6.sin6_family = AF_INET6;
> -		} else
> +
> +		switch (answer->ai_family) {
> +		case AF_INET:
> +			ss_len = sizeof (struct sockaddr_in);
> +			memcpy(&ss, answer->ai_addr, ss_len);
> +			break;
> +#ifdef INET6
> +		case AF_INET6:
> +			ss_len = sizeof (struct sockaddr_in6);
> +			memcpy(&ss, answer->ai_addr, ss_len);
> +			break;
>  #endif
> -		{
> -			ssp = (struct sockaddr_storage *) &sin;
> -			ss_len = sizeof sin;
> -			memset(ssp, 0, ss_len);
> -			inet_pton(AF_INET, addr, &sin.sin_addr);
> -			sin.sin_family = AF_INET;
>  		}
> -
> +		freeaddrinfo(answer);
>  	} else {
>  		ss_len = sizeof ss;
> -		ssp = &ss;
> -
>  		client_sockaddr(fd, &ss, &ss_len);
>  	}
>  
> -	if (!lookup_name(fd, ssp, ss_len, name_buf, sizeof name_buf,
> +	if (!lookup_name(fd, &ss, ss_len, name_buf, sizeof name_buf,
>  			port_buf, sizeof port_buf))
> -		check_name(fd, ssp, name_buf);
> +		check_name(fd, &ss, name_buf);
>  
>  	return name_buf;
>  }
> 
> 
> ------------------------------------------------------------------------
> 
> --- clientname.c	1 Apr 2004 21:39:35 -0000	1.21
> +++ clientname.c	1 Apr 2004 22:00:22 -0000
> @@ -190,9 +190,6 @@ void client_sockaddr(int fd,
>  		memset(sin, 0, sizeof *sin);
>  		sin->sin_family = AF_INET;
>  		*ss_len = sizeof (struct sockaddr_in);
> -#if HAVE_SOCKADDR_IN_LEN
> -		sin->sin_len = *ss_len;
> -#endif
>  		sin->sin_port = sin6.sin6_port;
>  
>  		/* There is a macro to extract the mapped part
> @@ -201,6 +198,10 @@ void client_sockaddr(int fd,
>  		memcpy(&sin->sin_addr, &sin6.sin6_addr.s6_addr[12],
>  		    sizeof sin->sin_addr);
>  	}
> +#endif /* INET6 */
> +
> +#if HAVE_SOCKADDR_IN_LEN
> +	ss->ss_len = *ss_len;
>  #endif
>  }
>  

--
Alex.