netdev - Re: [PATCH net-next] lwt: fix rx checksum setting for lwt devices tunneling over ipv6

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: <CAEh+42jyFVjM4A5HzJQ94xW+XZOTG5iBEFu2VHKCJU0sRwNgWA@mail.gmail.com>
Date:	Tue, 16 Feb 2016 10:22:38 -0800
From:	Jesse Gross <jesse@...nel.org>
To:	Jiri Benc <jbenc@...hat.com>
Cc:	Paolo Abeni <pabeni@...hat.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Pravin Shelar <pshelar@...ira.com>, Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH net-next] lwt: fix rx checksum setting for lwt devices
 tunneling over ipv6

On Thu, Feb 11, 2016 at 2:41 AM, Jiri Benc <jbenc@...hat.com> wrote:
> On Wed, 10 Feb 2016 16:47:21 +0100, Paolo Abeni wrote:
>> --- a/drivers/net/geneve.c
>> +++ b/drivers/net/geneve.c
>> @@ -1441,7 +1441,8 @@ struct net_device *geneve_dev_create_fb(struct net *net, const char *name,
>>               return dev;
>>
>>       err = geneve_configure(net, dev, &geneve_remote_unspec,
>> -                            0, 0, 0, htons(dst_port), true, 0);
>> +                            0, 0, 0, htons(dst_port), true,
>> +                            GENEVE_F_UDP_ZERO_CSUM6_RX);
>>       if (err) {
>>               free_netdev(dev);
>>               return ERR_PTR(err);
>> diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
>> index 1605691..d933cb8 100644
>> --- a/net/openvswitch/vport-vxlan.c
>> +++ b/net/openvswitch/vport-vxlan.c
>> @@ -90,7 +90,7 @@ static struct vport *vxlan_tnl_create(const struct vport_parms *parms)
>>       int err;
>>       struct vxlan_config conf = {
>>               .no_share = true,
>> -             .flags = VXLAN_F_COLLECT_METADATA,
>> +             .flags = VXLAN_F_COLLECT_METADATA | VXLAN_F_UDP_ZERO_CSUM6_RX,
>
> I'm afraid this looks wrong, we should not default to zero UDP checksum
> over IPv6. See RFC 2460, section 8.1:
>
>       o  Unlike IPv4, when UDP packets are originated by an IPv6 node,
>          the UDP checksum is not optional.  That is, whenever
>          originating a UDP packet, an IPv6 node must compute a UDP
>          checksum over the packet and the pseudo-header, and, if that
>          computation yields a result of zero, it must be changed to hex
>          FFFF for placement in the UDP header.  IPv6 receivers must
>          discard UDP packets containing a zero checksum, and should log
>          the error.
>
> One may argue that with tunneling, the situation is different but
> that's the reason why we have the IPv6 checksum flag and why it has
> opposite meaning to the IPv4 one. We shouldn't default to non-RFC
> behavior by default.

There's a bigger problem here, not really related to lightweight tunnels or OVS.

The VXLAN RFC says (referring to the UDP checksum and not specific to IPv4/v6):
"It SHOULD be transmitted as zero. When a packet is received with a
UDP checksum of zero, it MUST be accepted for decapsulation."

We can debate whether this is correct or whether it conflicts with RFC
2460 but this is what essentially everyone is going to implement. With
the default settings of the flags in IPv6, we are violating both
statements. With the second one in particular, the result is that
Linux will not be able to communicate with any non-Linux VXLAN
endpoint over IPv6 with default settings.