Re: [PATCH] net: ip/tnl: Set iph->id only when don't fragment is not set

[Oliver Herms <oliver.peter.herms@gmail.com>]

On 23.11.19 18:53, Eric Dumazet wrote:
> Official sources for this assertion please, so that we can double check if you
> implemented the proper avoidance ?
From RFC 6864 Section 4.1:
"The IPv4 ID field MUST NOT be used for purposes other than
      fragmentation and reassembly."

>>  net/ipv4/ip_tunnel_core.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
>> index 1452a97914a0..8636c1e0e7b7 100644
>> --- a/net/ipv4/ip_tunnel_core.c
>> +++ b/net/ipv4/ip_tunnel_core.c
>> @@ -73,7 +73,9 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
>>  	iph->daddr	=	dst;
>>  	iph->saddr	=	src;
>>  	iph->ttl	=	ttl;
>> -	__ip_select_ident(net, iph, skb_shinfo(skb)->gso_segs ?: 1);
>> +
>> +	if (unlikely((iph->frag_off & htons(IP_DF)) == false))
> 
> This unlikely() seems wrong to me.
> 
> You do not know what are the odds of IP_DF being set or not.
Right. I'll send a corrected patch.

>> +		__ip_select_ident(net, iph, skb_shinfo(skb)->gso_segs ?: 1);
> So we are going to send 2 bytes with garbage if we do not call __ip_select_ident()
> 
> This would cause various security threats, since the garbage might reveal a secret.
So we should set it to zero then.