[PATCH xauth] Look for FamilyLocal if inet or inet6 address is loopback

[walter harms]

Am 07.08.2013 17:00, schrieb Egbert Eich:
> On Tue, Jul 30, 2013 at 11:01:02AM +0200, Egbert Eich wrote:
>> libxcb uses FamilyLocal authorization if the host name or IP in the
>> display string is from the loopback device. This patch adds the same
>> behavior to xauth.
>> This fixes a long standing problem that for ssh tunneled connections
>> a display variable of the form: localhost:<N>.<M> leads to correct
>> authorization when an X client is started but "xauth list $DISPLAY"
>> returns nothing.
> 
> Likewise, does anyone care to comment on this patch?
> It will change the behavior of xauth slightly - it will more
> closely match what libxcb does and therefore its results will
> resemble how Xclients behave more closely.
> 
> Cheers,
> 	Egbert.
> 

I see you can fix a known problem, so why not to go forward ?


>>
>> Signed-off-by: Egbert Eich <eich at freedesktop.org>
>> ---
>>  gethost.c | 40 +++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/gethost.c b/gethost.c
>> index 10f6078..7c4b600 100644
>> --- a/gethost.c
>> +++ b/gethost.c
>> @@ -224,16 +224,36 @@ struct addrlist *get_address_info (
>>  	for (ai = firstai; ai != NULL; ai = ai->ai_next) {
>>  	    struct addrlist *duplicate;
>>  
>> +	    len = 0;
>>  	    if (ai->ai_family == AF_INET) {
>>  		struct sockaddr_in *sin = (struct sockaddr_in *)ai->ai_addr;
>>  		src = &(sin->sin_addr);
>> -		len = sizeof(sin->sin_addr);
>> -		family = FamilyInternet;
>> +		if (*(in_addr_t *) src == htonl(INADDR_LOOPBACK)) {
>> +		    family = FamilyLocal;
>> +		    if (get_local_hostname (buf, sizeof buf)) {
>> +			src = buf;
>> +			len = strlen (buf);
>> +		    } else
>> +			src = NULL;
>> +		} else {
>> +		    len = sizeof(sin->sin_addr);
>> +		    family = FamilyInternet;
>> +		}

IMHO you could do that with less indent/effort like:

src = &(sin->sin_addr);
len = sizeof(sin->sin_addr);
family = FamilyInternet;
if (*(in_addr_t *) src == htonl(INADDR_LOOPBACK)) {
	family = FamilyLocal;
	src = NULL;   /* should len=0  ? */
	if (get_local_hostname (buf, sizeof buf)) {
		src = buf;
		len = strlen (buf);
	}
}

>>  	    } else if (ai->ai_family == AF_INET6) {
>>  		struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)ai->ai_addr;
>>  		src = &(sin6->sin6_addr);
>> -		len = sizeof(sin6->sin6_addr);
>> -		family = FamilyInternet6;
>> +		if (IN6_IS_ADDR_V4MAPPED((struct sockaddr_in6 *)src)
>> +		    || IN6_IS_ADDR_LOOPBACK((struct sockaddr_in6 *)src)) {
>> +		    family = FamilyLocal;
>> +		    if (get_local_hostname (buf, sizeof buf)) {
>> +			src = buf;
>> +			len = strlen (buf);
>> +		    } else
>> +			src = NULL;
>> +		} else {
>> +		    len = sizeof(sin6->sin6_addr);
>> +		    family = FamilyInternet6;
>> +		}
>>  	    }
>>  
>>  	    for(duplicate = retval; duplicate != NULL; duplicate = duplicate->next) {
>> @@ -272,7 +292,17 @@ struct addrlist *get_address_info (
>>  #else
>>  	if (!get_inet_address (host, &hostinetaddr)) return NULL;
>>  	src = (char *) &hostinetaddr;
>> -	len = 4; /* sizeof inaddr.sin_addr, would fail on Cray */
>> +	if (*(in_addr_t *) src == htonl(INADDR_LOOPBACK)) {
>> +	    family = FamilyLocal;
>> +	    if (get_local_hostname (buf, sizeof buf)) {
>> +		src = buf;
>> +		len = strlen (buf);
>> +	    } else {
>> +		len = 0;
>> +		src = NULL;
>> +	    }

maybe using a small wrapper for get_local_hostname() ?

void xget_local_host(*src,*len)
{
	*len = 0;
	*src = NULL;
	if (get_local_hostname (buf, sizeof buf)) {
		src = buf;
		len = strlen (buf);
        }
}


>> +	} else
>> +	    len = 4; /* sizeof inaddr.sin_addr, would fail on Cray */

	we have removed all #Ifdef CRAY so we can remove this also

>>  	break;
>>  #endif /* IPv6 */
>>  #else
>> -- 
>> 1.8.1.4
>

just my 2 cents,
re,
 wh