netdev - Re: [PATCH 1/1] NFS: change the ip_map cache code to handle IPv6 addresses

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 09 Aug 2007 17:08:49 +0200
From:	Aurélien Charbon 
	<aurelien.charbon@....bull.net>
To:	chuck.lever@...cle.com
Cc:	Mailing list NFSv4 <nfsv4@...ux-nfs.org>,
	netdev ML <netdev@...r.kernel.org>, Neil Brown <neilb@...e.de>
Subject: Re: [PATCH 1/1] NFS: change the ip_map cache code to handle IPv6
 addresses

Thanks for these comments.

Chuck Lever wrote:

> Some naive questions:
>
> 1.  If IPv6 support is not configured into the kernel, how does an 
> IPv6-only cache work?

Yes it should work.
The INET6 structures are only used as containers for both type of address.
I have tested by unabling IPv6 options in the .config file.

>
> 2.  I seem to recall (only quite vaguely) that at some point the 
> server might need to use one of the stored addresses to, say, open a 
> socket to the client?  In that case, on a system with NICs configured 
> only with IPv4, is the cached IPv6 address properly converted back to 
> IPv4 somehow?

Yes. we can do that by testing the format of the address, as we know 
that an IPv4 address is mapped like that:
[00000000] [00000000] [0000FFFF] <32 bits of IPv4 addr>

> Can the cache code tell the difference between a cached IPv6 address 
> that was converted from IPv4 and one that was added to the cache as 
> IPv6?  Sorry I can't remember more clearly.

Yes. If the address starts with ::FFFF, we can infer that is a mapped 
IPv4 address.

>
> 3.  Would it be better to make the m_addr field a struct sockaddr, 
> store a whole address (with address family), and switch on the 
> sa_family field?

That is possible but we're able to retrieve the family of the address by 
testing its format (IPv4 mapped or not).
But I don't know if it is a better solution.

>
>
>> diff -u -r -N linux-2.6.23-rc1/fs/nfsd/export.c 
>> linux-2.6.23-rc1-IPv6-ip_map/fs/nfsd/export.c
>> --- linux-2.6.23-rc1/fs/nfsd/export.c    2007-08-08 
>> 17:52:58.000000000 +0200
>> +++ linux-2.6.23-rc1-IPv6-ip_map/fs/nfsd/export.c    2007-08-08 
>> 17:49:09.000000000 +0200
>> @@ -1558,6 +1558,7 @@
>>  {
>>      struct auth_domain    *dom;
>>      int            i, err;
>> +    struct in6_addr addr6;
>>  
>>      /* First, consistency check. */
>>      err = -EINVAL;
>> @@ -1576,9 +1577,14 @@
>>          goto out_unlock;
>>  
>>      /* Insert client into hashtable. */
>> -    for (i = 0; i < ncp->cl_naddr; i++)
>> -        auth_unix_add_addr(ncp->cl_addrlist[i], dom);
>> -
>> +    for (i = 0; i < ncp->cl_naddr; i++) {
>> +        /* Mapping address */
>> +            addr6.s6_addr32[0] = 0;
>> +            addr6.s6_addr32[1] = 0;
>> +            addr6.s6_addr32[2] = htonl(0xffff);
>> +            addr6.s6_addr32[3] = (uint32_t)ncp->cl_addrlist[i].s_addr;
>> +        auth_unix_add_addr(addr6, dom);
>> +    }
>>      auth_unix_forget_old(dom);
>>      auth_domain_put(dom);
>
>
> This converts IPv4 addresses to canonical IPv6 as it stores them.  
> What happens if a full-blown IPv6 address is encountered?  Likewise, 
> in nfsctl.c?

I have done this kind of convertion only for compilation an testing with 
only IPv4... For the moment.

>
>> @@ -112,12 +112,16 @@
>>      return (hash ^ (hash>>8)) & 0xff;
>>  }
>>  #endif
>> +static inline int hash_ip6(struct in6_addr ip)
>> +{
>> +        return (hash_ip(ip.s6_addr32[0]) ^ hash_ip(ip.s6_addr32[1]) 
>> ^ hash_ip(ip.s6_addr32[2]) ^ hash_ip(ip.s6_addr32[3])) ;
>> +}
>
>
> How have you tested the effectiveness of the new hash function?

I have not tested that point but I can easily imagine there are better 
solutions.
Perhaps we can keep the same function for an IPv4 address (only taking 
the 32 bits of IPv4 addr), and then design one for IPv6 addresses.

Do you have any suggestion on that ?

>> -    seq_printf(m, "%s %d.%d.%d.%d %s\n",
>> +    seq_printf(m, "%s %04x.%04x.%04x.%04x.%04x.%04x.%04x.%04x %s\n",
>>             im->m_class,
>> -           ntohl(addr.s_addr) >> 24 & 0xff,
>> -           ntohl(addr.s_addr) >> 16 & 0xff,
>> -           ntohl(addr.s_addr) >>  8 & 0xff,
>> -           ntohl(addr.s_addr) >>  0 & 0xff,
>> +           ntohl(addr.s6_addr32[3]) >> 16 & 0xffff,
>> +           ntohl(addr.s6_addr32[3]) & 0xffff,
>> +           ntohl(addr.s6_addr32[2]) >> 16 & 0xffff,
>> +           ntohl(addr.s6_addr32[2]) & 0xffff,
>> +           ntohl(addr.s6_addr32[1]) >> 16 & 0xffff,
>> +           ntohl(addr.s6_addr32[1]) & 0xffff,
>> +           ntohl(addr.s6_addr32[0]) >> 16 & 0xffff,
>> +           ntohl(addr.s6_addr32[0]) & 0xffff,
>>             dom
>>             );
>>      return 0;
>
>
> And I think here NIP6_FMT should be used, but you're not using colons 
> between the hex digits.  Was that intentional
> ?

No that's a mistake. Colons have to be used here.

Aurélien

-- 

********************************
       Aurelien Charbon
       Linux NFSv4 team
           Bull SAS
     Echirolles - France
http://nfsv4.bullopensource.org/
********************************

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html