netdev - Re: [RFC net 1/2] net: set skb hash for IP6 TCP reset packet

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]

Message-ID: <20170718185905.d3erulmh3ns6ddfs@kernel.org>
Date:   Tue, 18 Jul 2017 11:59:05 -0700
From:   Shaohua Li <shli@...nel.org>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, Kernel-team@...com,
        Florent Fourcot <flo@...rcot.fr>
Subject: Re: [RFC net 1/2] net: set skb hash for IP6 TCP reset packet

On Mon, Jul 17, 2017 at 09:02:57PM -0700, Eric Dumazet wrote:
> On Mon, 2017-07-17 at 14:53 -0700, Shaohua Li wrote:
> > On Mon, Jul 17, 2017 at 01:51:51AM -0700, Eric Dumazet wrote:
> > > On Thu, 2017-07-13 at 10:56 -0700, Shaohua Li wrote:
> > > > From: Shaohua Li <shli@...com>
> > > > 
> > > > Please see below tcpdump output:
> > > 
> > > > The tcp reset packet has a different flowlabel, which causes our router
> > > > doesn't correctly close tcp connection.
> > > 
> > > This looks a bug in your router, because (IPv6 only) flowlabel is not
> > > part of the tuple identifying a TCP flow.
> > 
> > Actually it's for load balance between several routers.
> 
> What happens then when flowlabel changes as I described ?
> 
> See commit 3acf3ec3f4b0 ("tcp: Change txhash on every SYN and RTO
> retransmit")

Frankly I have no idea. People in the team do think this is a problem in some
corner cases. Didn't get any report yet though.
 
> > > 
> > > >   The reason is the normal packet
> > > > gets the skb->hash from sk->sk_txhash, which is generated randomly.
> > > > ip6_make_flowlabel then uses the hash to create a flowlabel. The reset
> > > > packet doesn't get assigned a hash, so the flowlabel is calculated with
> > > > flowi6.
> > > > 
> > > > The solution is to save the hash value for timeout sock and use it for
> > > > reset packet.
> > > 
> > > I am a bit unsure why we need to add yet another field in TCP timewait
> > > structure, since :
> > > 
> > > 1) flowlabel can vary during a TCP flow lifetime.
> > > 2) flowlabel is different unde synflood (each syncookie gets a random
> > > flowlabel), and if 3rd packet comes back from the client to finish 3WHS,
> > > the flowlabel will again be different from the one that SYNACK used.
> > 
> > Is it acceptable we reuse tw_flowlabel as Florent Fourcot suggested? It makes
> > no sense to change flowlabel for no reason.
> 
> Sure, if you can find a way to keep storage as small as possible.
> 
> Current size is dangerously approaching 256 bytes, so we might soon use
> one additional cache line (64 bytes)

Will send a new patch.

Thanks,
Shaohua