netdev - Re: [PATCH V4 net 0/2] ipv6: fix flowlabel issue for 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171114182454.hjj6hhlor33w5x5c@kernel.org>
Date:   Tue, 14 Nov 2017 10:24:54 -0800
From:   Shaohua Li <shli@...nel.org>
To:     Tom Herbert <tom@...bertland.com>
Cc:     David Miller <davem@...emloft.net>, Martin Lau <kafai@...com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH V4 net 0/2] ipv6: fix flowlabel issue for reset packet

On Wed, Nov 08, 2017 at 09:44:51AM -0800, Tom Herbert wrote:
> On Fri, Aug 18, 2017 at 3:27 PM, David Miller <davem@...emloft.net> wrote:
> > From: Martin KaFai Lau <kafai@...com>
> > Date: Fri, 18 Aug 2017 13:51:36 -0700
> >
> >> It seems like that middle box specifically drops TCP_RST if it
> >> does not know anything about this flow.  Since the flowlabel of the TCP_RST
> >> (sent in TW state) is always different, it always lands to a different middle
> >> box.  All of these TCP_RST cannot be delivered.
> >
> > This really is illegal behavior.  The flow label is not a flow _KEY_
> > by any definition whatsoever.
> >
> > Flow labels are an optimization, not a determinant for flow matching
> > particularly for proper TCP state processing.
> >
> > I'd rather you invest all of this energy getting that vendor to fix
> > their kit.
> >
> We're now seeing several router vendors recommending people to not use
> flow labels for ECMP hashing. This is precisely because when a flow
> label changes, network devices that maintain state (firewalls, NAT,
> load balancers) can't deal with packets being rerouted so connections
> are dropped. Unfortunately, the need for packets of a flow to always
> follow the same path has become an implicit requirement that I think
> we need follow at least as the default behavior.
> 
> Martin: is there any change you could resurrect these patches? In
> order to solve the general problem of making routing consistent, I
> believe we want to keep sk_tx_hash consistent for the connection from
> which a consistent flow label can be derived. To avoid the overhead of
> a hash field in sk_common, maybe we could initially set a connection
> hash to a five-tuple hash for a flow instead of a random value? So in
> TW state the consistent hash can be computed on the fly.

Hi Tom,
Do we really need to use the five-tupe hash? There are several places using
current random hash, which looks more lightweight. To fix issue, we only need
to make sure reset packet include the correct flowlabel. Like what my previous
patch did, we can set tw->tw_flowlabel in tcp_time_wait based on txhash and use
it reset packet. In this way we can use the random hash and not add extra field
in sock.

Thanks,
Shaohua