netdev - Re: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers

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+42goYHorJiP2ShhMj6CXCfPyn1r+obb0YsKXLeZFNW2PLw@mail.gmail.com>
Date:	Mon, 7 Mar 2016 15:42:59 -0800
From:	Jesse Gross <jesse@...nel.org>
To:	Alex Duyck <aduyck@...antis.com>
Cc:	David Miller <davem@...emloft.net>,
	Or Gerlitz <gerlitz.or@...il.com>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [net-next PATCH 3/4] vxlan: Enforce IP ID verification on outer headers

On Mon, Mar 7, 2016 at 3:06 PM, Alex Duyck <aduyck@...antis.com> wrote:
> On Mon, Mar 7, 2016 at 11:09 AM, David Miller <davem@...emloft.net> wrote:
>> From: Or Gerlitz <gerlitz.or@...il.com>
>> Date: Mon, 7 Mar 2016 20:05:20 +0200
>>
>>> On Mon, Mar 7, 2016 at 7:22 PM, Alexander Duyck <aduyck@...antis.com> wrote:
>>>> This change enforces the IP ID verification on outer headers.  As a result
>>>> if the DF flag is not set on the outer header we will force the flow to be
>>>> flushed in the event that the IP ID is out of sequence with the existing
>>>> flow.
>>>
>>> Can you please state the precise requirement for aggregation w.r.t IP
>>> IDs here? and point to where/how this is enforced, e.g for
>>> non-tunneled TCP GRO-ing?
>>
>> I also didn't see a nice "PATCH 0/4" posting explaining this series and
>> I'd really like to see that.
>
> Sorry about that.  I forgot to add the cover page when I sent this.
>
> The enforcement is coming from the IP and TCP layers.  If you take a
> look in inet_gro_receive we have the NAPI_GRO_CB(p)->flush_id value
> being populated based on the difference between the expected ID and
> the received one.  So for IPv4 we overwrite it, and for IPv6 we set it
> to 0.  The only consumer currently using it is TCP in tcp_gro_receive.
> The problem is with tunnels we lose the data for the outer when the
> inner overwrites it, as a result we can put whatever we want currently
> in the outer IP ID and it will be accepted.
>
> The patch set is based off of a conversation several of us had on the
> list about doing TSO for tunnels and the fact that the IP IDs for the
> outer header have to advance.  It makes it easier for me to validate
> that I am doing things properly if GRO doesn't destroy the IP ID data
> for the outer headers.

In net/ipv4/af_inet.c:inet_gro_receive() there is the following
comment above where NAPI_GRO_CB(p)->flush_id is set:

/* Save the IP ID check to be included later when we get to
* the transport layer so only the inner most IP ID is checked.
* This is because some GSO/TSO implementations do not
* correctly increment the IP ID for the outer hdrs.
*/

There was a long discussion about this a couple years ago and the
conclusion was that it is the inner IP ID is really the important one
in the case of encapsulation. Obviously, things like TCP/IP header
compression don't apply to the outer encapsulation header.