netdev - Re: [PATCH] veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.

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] [day] [month] [year] [list]

Date:	Tue, 22 Dec 2015 15:16:12 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	vijayp@...ayp.ca
Cc:	xiyou.wangcong@...il.com, ej@...njones.ca,
	nicolas.dichtel@...nd.com, phil@....cc,
	makita.toshiaki@....ntt.co.jp, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] veth: don’t modify ip_summed;
 doing so treats packets with bad checksums as good.

From: Vijay Pandurangan <vijayp@...ayp.ca>
Date: Fri, 18 Dec 2015 14:34:59 -0500

> Packets that arrive from real hardware devices have ip_summed ==
> CHECKSUM_UNNECESSARY if the hardware verified the checksums, or
> CHECKSUM_NONE if the packet is bad or it was unable to verify it. The
> current version of veth will replace CHECKSUM_NONE with
> CHECKSUM_UNNECESSARY, which causes corrupt packets routed from hardware to
> a veth device to be delivered to the application. This caused applications
> at Twitter to receive corrupt data when network hardware was corrupting
> packets.
> 
> We believe this was added as an optimization to skip computing and
> verifying checksums for communication between containers. However, locally
> generated packets have ip_summed == CHECKSUM_PARTIAL, so the code as
> written does nothing for them. As far as we can tell, after removing this
> code, these packets are transmitted from one stack to another unmodified
> (tcpdump shows invalid checksums on both sides, as expected), and they are
> delivered correctly to applications. We didn’t test every possible network
> configuration, but we tried a few common ones such as bridging containers,
> using NAT between the host and a container, and routing from hardware
> devices to containers. We have effectively deployed this in production at
> Twitter (by disabling RX checksum offloading on veth devices).
> 
> This code dates back to the first version of the driver, commit
> <e314dbdc1c0dc6a548ecf> ("[NET]: Virtual ethernet device driver"), so I
> suspect this bug occurred mostly because the driver API has evolved
> significantly since then. Commit <0b7967503dc97864f283a> ("net/veth: Fix
> packet checksumming") (in December 2010) fixed this for packets that get
> created locally and sent to hardware devices, by not changing
> CHECKSUM_PARTIAL. However, the same issue still occurs for packets coming
> in from hardware devices.
> 
> Co-authored-by: Evan Jones <ej@...njones.ca>
> Signed-off-by: Evan Jones <ej@...njones.ca>
> Cc: Nicolas Dichtel <nicolas.dichtel@...nd.com>
> Cc: Phil Sutter <phil@....cc>
> Cc: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
> Cc: netdev@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Vijay Pandurangan <vijayp@...ayp.ca>

Applied and queued up for -stable, thanks.