netdev - Re: [PATCH] gre: copy ToS/DiffServ bits to outer IP header

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:	Tue, 14 Jul 2009 16:21:51 +0200
From:	Andreas Jaggi <aj@...n.ch>
To:	Wojtek Sawasciuk <voyo@...ip.pl>
Cc:	netdev@...r.kernel.org, kuznet@....inr.ac.ru, kaber@...sh.net,
	davem@...emloft.net
Subject: Re: [PATCH] gre: copy ToS/DiffServ bits to outer IP header

On Tue, Jul 14, 2009 at 01:39:48PM +0100, Wojtek Sawasciuk wrote:
> I think I have similar problems to yours, and I spotted your email on  
> the lists when googling :)
> Andreas, sorry but I think your patch is unnecessary, at last in this form.
> From long time there is possibility to create tunnel with tos options,  
> you can use "inherit" or any other value to set it to encapsulated 
> packets.
>
> ip tun add mode gre name tun1 remote 1.2.3.5 local 10.0.0.234 tos inherit
> or
> ip tun add mode gre name tun2 remote 1.2.3.6 local 10.0.0.234 tos 2
>
> and its working, *almost* ok.
> only problem is when you set tos value with bit 1 set in it, regarding  
> this fragment of code from ip_gre.c , in ipgre_tunnel_xmit function:
>
>        tos = tiph->tos;
>        if (tos&1) {
>                if (skb->protocol == htons(ETH_P_IP))
>                        tos = old_iph->tos;
>                tos &= ~1;
>        }
>
> (ie. set tos to 2 - it will replicate it to 2, set tos to 3 - it will  
> change it to 2).
>
> IHMO it should be replaced to use maybe MBZ bit (look on RFC1349) ,  
> instead of bit 1 , to pass on value of tos , from tunnel parameter to IP  
> header.
> and this should be sufficient to make it work properly.

You're right, my patch in this form is not necessary.

But as you point out, there is a bug in the handling of the ToS/DiffServ
bits in GRE.
iproute2 and the Linux GRE driver use a tos value of 1 to indicate an
'inherit'/'copy-tos' behavior (and then clear this bit before using the
tos value in the IP header of the encapsulation GRE packet).

This was fine for RFC1349 where this bit is defined as MBZ/must-be-zero.
But with RFC3168, this bit (and the other previously unused one in the tos field)
are used for the ECN bits (ECT + CE). Therefore a tos value of 1 is 'legal' and
conflicts with the 'inherit'/'copy-tos' indication of iproute2 and the
Linux GRE driver.

This creates the problem you described, where IP packets who have a tos
value of 1 (with the ECT bit set to 1) are handled with the 'inherit'/'copy-tos'
mechanism and the resulting GRE IP header has a tos value of 0! (the ECT bit
set to 0 instead of 1)
--
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