netdev - Re: [PATCH net v3 2/3] tcp: tighten acceptance of ACKs not matching a child socket

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]

Date:   Thu, 5 Dec 2019 19:45:48 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Guillaume Nault <gnault@...hat.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     netdev@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net v3 2/3] tcp: tighten acceptance of ACKs not matching a
 child socket



On 12/5/19 5:49 PM, Guillaume Nault wrote:
> When no synflood occurs, the synflood timestamp isn't updated.
> Therefore it can be so old that time_after32() can consider it to be
> in the future.
> 
> That's a problem for tcp_synq_no_recent_overflow() as it may report
> that a recent overflow occurred while, in fact, it's just that jiffies
> has grown past 'last_overflow' + TCP_SYNCOOKIE_VALID + 2^31.
> 
> Spurious detection of recent overflows lead to extra syncookie
> verification in cookie_v[46]_check(). At that point, the verification
> should fail and the packet dropped. But we should have dropped the
> packet earlier as we didn't even send a syncookie.
> 
> Let's refine tcp_synq_no_recent_overflow() to report a recent overflow
> only if jiffies is within the
> [last_overflow, last_overflow + TCP_SYNCOOKIE_VALID] interval. This
> way, no spurious recent overflow is reported when jiffies wraps and
> 'last_overflow' becomes in the future from the point of view of
> time_after32().
> 
> However, if jiffies wraps and enters the
> [last_overflow, last_overflow + TCP_SYNCOOKIE_VALID] interval (with
> 'last_overflow' being a stale synflood timestamp), then
> tcp_synq_no_recent_overflow() still erroneously reports an
> overflow. In such cases, we have to rely on syncookie verification
> to drop the packet. We unfortunately have no way to differentiate
> between a fresh and a stale syncookie timestamp.
> 
> In practice, using last_overflow as lower bound is problematic.
> If the synflood timestamp is concurrently updated between the time
> we read jiffies and the moment we store the timestamp in
> 'last_overflow', then 'now' becomes smaller than 'last_overflow' and
> tcp_synq_no_recent_overflow() returns true, potentially dropping a
> valid syncookie.
> 
> Reading jiffies after loading the timestamp could fix the problem,
> but that'd require a memory barrier. Let's just accommodate for
> potential timestamp growth instead and extend the interval using
> 'last_overflow - HZ' as lower bound.
> 
> Suggested-by: Eric Dumazet <eric.dumazet@...il.com>
> Signed-off-by: Guillaume Nault <gnault@...hat.com>

Signed-off-by: Eric Dumazet <edumazet@...gle.com>