netdev - Re: [PATCH] IPv6: Implement RFC 4429 Optimistic Duplicate Address Detection

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]

Message-ID: <45BA0E1C.80408@hp.com>
Date:	Fri, 26 Jan 2007 09:20:12 -0500
From:	Vlad Yasevich <vladislav.yasevich@...com>
To:	YOSHIFUJI Hideaki / 吉藤英明 
	<yoshfuji@...ux-ipv6.org>
Cc:	nhorman@...driver.com, sri@...ibm.com, davem@...emloft.net,
	kuznet@....inr.ac.ru, pekkas@...core.fi, jmorris@...ei.org,
	kaber@...eworks.de, netdev@...r.kernel.org
Subject: Re: [PATCH] IPv6: Implement RFC 4429 Optimistic Duplicate Address
 Detection

YOSHIFUJI Hideaki / 吉藤英明 wrote:
> In article <20070125194500.GB8891@...reliant.homelinux.net> (at Thu, 25 Jan 2007 14:45:00 -0500), Neil Horman <nhorman@...driver.com> says:
> 
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 2a7e461..46f91ee 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -830,7 +830,8 @@ retry:
>>  	ift = !max_addresses ||
>>  	      ipv6_count_addresses(idev) < max_addresses ? 
>>  		ipv6_add_addr(idev, &addr, tmp_plen,
>> -			      ipv6_addr_type(&addr)&IPV6_ADDR_SCOPE_MASK, IFA_F_TEMPORARY) : NULL;
>> +			      ipv6_addr_type(&addr)&IPV6_ADDR_SCOPE_MASK, 
>> +			      IFA_F_TEMPORARY|IFA_F_OPTIMISTIC) : NULL;
>>  	if (!ift || IS_ERR(ift)) {
>>  		in6_ifa_put(ifp);
>>  		in6_dev_put(idev);
> 
> If optimistic_dad is disabled, flags should be IFA_F_TEMPORARY,
> not IFA_F_TEMPORARY|IFA_F_OPTIMISTIC.
> 
> Another idea is to use IFA_F_OPTIMISTIC not
> IFA_F_OPTIMISTIC|IFA_F_TENTATIVE until the DAD has been finished.
> 
>> @@ -1027,15 +1029,17 @@ int ipv6_dev_get_saddr(struct net_device *daddr_dev,
> :
>> +			/* Rule 3: Avoid deprecated and optimistic address */
>>  			if (hiscore.rule < 3) {
>>  				if (ipv6_saddr_preferred(hiscore.addr_type) ||
>> -				    !(ifa_result->flags & IFA_F_DEPRECATED))
>> +				    ((!(ifa_result->flags & IFA_F_DEPRECATED)) && 
>> +				    (!(ifa_result->flags & IFA_F_OPTIMISTIC))))
>>  					hiscore.attrs |= IPV6_SADDR_SCORE_PREFERRED;
>>  				hiscore.rule++;
> 
>                                     ((ifa_result->flags & (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC)) == 0)
> 
>>  			}
>>  			if (ipv6_saddr_preferred(score.addr_type) ||
>> -			    !(ifa->flags & IFA_F_DEPRECATED)) {
>> +			    ((!(ifa->flags & IFA_F_DEPRECATED)) &&
>> +			    (!(ifa_result->flags & IFA_F_OPTIMISTIC)))) {
>>  				score.attrs |= IPV6_SADDR_SCORE_PREFERRED;
>>  				if (!(hiscore.attrs & IPV6_SADDR_SCORE_PREFERRED)) {
>>  					score.rule = 3;
> 
> ditto.
> 
>> @@ -2123,7 +2133,8 @@ static void addrconf_add_linklocal(struct inet6_dev *idev, struct in6_addr *addr
>>  {
>>  	struct inet6_ifaddr * ifp;
>>  
>> -	ifp = ipv6_add_addr(idev, addr, 64, IFA_LINK, IFA_F_PERMANENT);
>> +	ifp = ipv6_add_addr(idev, addr, 64, IFA_LINK, 
>> +		            IFA_F_PERMANENT|IFA_F_OPTIMISTIC);
>>  	if (!IS_ERR(ifp)) {
>>  		addrconf_dad_start(ifp, 0);
>>  		in6_ifa_put(ifp);
> 
> Please do not always put IFA_F_OPTIMISTIC.
> 
>>  
>> +	/*
>> +	 * Optimistic nodes need to joing the anycast address
>> +	 * right away
>> +	 */
>> +	if (ifp->flags & IFA_F_OPTIMISTIC)
>> +		addrconf_join_anycast(ifp);
>> +
>>  	if (ifp->prefix_len != 128 && (ifp->flags&IFA_F_PERMANENT))
>>  		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev, 0,
>>  					flags);
> 
> Should we join anycast even if the node is a host (not a router)?!

"A router SHOULD NOT configure an Optimistic Address. "
Section 3.3

I think this check is missing from the implementation.


>> @@ -622,9 +637,20 @@ void ndisc_send_rs(struct net_device *dev, struct in6_addr *saddr,
>> +	/*
>> +	 * Check the source address.  If its OPTIMISTIC
>> +	 * and addr_len is non-zero (implying the sllao option)
>> +	 * then don't send the RS (RFC 4429, section 2.2)
>> +	 */
>> +	ifp = ipv6_get_ifaddr(saddr, dev, 1);
>> +
>> +	if ((!ifp) || ((ifp->flags & IFA_F_OPTIMISTIC) && dev->addr_len))
>> +		return;
>> +
>>  	ndisc_flow_init(&fl, NDISC_ROUTER_SOLICITATION, saddr, daddr,
>>  			dev->ifindex);
>>  
> 
> I disagree. Please send RS in other way.
> Choose another address, or send it without SLLAO.
> 

Agree.  Sending without SLLAO is a "MAY" option in the draft and would
yield better operation.

-vlad
-
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