2022-04-27 10:38:15

by Willy Tarreau

[permalink] [raw]
Subject: [PATCH net 0/7] insufficient TCP source port randomness

Hi,

In a not-yet published paper, Moshe Kol, Amit Klein, and Yossi Gilad
report being able to accurately identify a client by forcing it to emit
only 40 times more connections than the number of entries in the
table_perturb[] table, which is indexed by hashing the connection tuple.
The current 2^8 setting allows them to perform that attack with only 10k
connections, which is not hard to achieve in a few seconds.

Eric, Amit and I have been working on this for a few weeks now imagining,
testing and eliminating a number of approaches that Amit and his team were
still able to break or that were found to be too risky or too expensive,
and ended up with the simple improvements in this series that resists to
the attack, doesn't degrade the performance, and preserves a reliable port
selection algorithm to avoid connection failures, including the odd/even
port selection preference that allows bind() to always find a port quickly
even under strong connect() stress.

The approach relies on several factors:
- resalting the hash secret that's used to choose the table_perturb[]
entry every 10 seconds to eliminate slow attacks and force the
attacker to forget everything that was learned after this delay.
This already eliminates most of the problem because if a client
stays silent for more than 10 seconds there's no link between the
previous and the next patterns, and 10s isn't yet frequent enough
to cause too frequent repetition of a same port that may induce a
connection failure ;

- adding small random increments to the source port. Previously, a
random 0 or 1 was added every 16 ports. Now a random 0 to 7 is
added after each port. This means that with the default 32768-60999
range, a worst case rollover happens after 1764 connections, and
an average of 3137. This doesn't stop statistical attacks but
requires significantly more iterations of the same attack to
confirm a guess.

- increasing the table_perturb[] size from 2^8 to 2^16, which Amit
says will require 2.6 million connections to be attacked with the
changes above, making it pointless to get a fingerprint that will
only last 10 seconds. Due to the size, the table was made dynamic.

- a few minor improvements on the bits used from the hash, to eliminate
some unfortunate correlations that may possibly have been exploited
to design future attack models.

These changes were tested under the most extreme conditions, up to
1.1 million connections per second to one and a few targets, showing no
performance regression, and only 2 connection failures within 13 billion,
which is less than 2^-32 and perfectly within usual values.

The series is split into small reviewable changes and was already reviewed
by Amit and Eric.

Regards,
Willy

---
Eric Dumazet (1):
tcp: resalt the secret every 10 seconds

Willy Tarreau (6):
secure_seq: return the full 64-bit of the siphash
tcp: use different parts of the port_offset for index and offset
tcp: add small random increments to the source port
tcp: dynamically allocate the perturb table used by source ports
tcp: increase source port perturb table to 2^16
tcp: drop the hash_32() part from the index calculation

include/net/inet_hashtables.h | 2 +-
include/net/secure_seq.h | 2 +-
net/core/secure_seq.c | 14 ++++++++----
net/ipv4/inet_hashtables.c | 40 ++++++++++++++++++++++-------------
4 files changed, 37 insertions(+), 21 deletions(-)

--
2.17.5


2022-04-27 11:32:47

by Willy Tarreau

[permalink] [raw]
Subject: [PATCH net 3/7] tcp: resalt the secret every 10 seconds

From: Eric Dumazet <[email protected]>

In order to limit the ability for an observer to recognize the source
ports sequence used to contact a set of destinations, we should
periodically shuffle the secret. 10 seconds looks effective enough
without causing particular issues.

Cc: Moshe Kol <[email protected]>
Cc: Yossi Gilad <[email protected]>
Cc: Amit Klein <[email protected]>
Tested-by: Willy Tarreau <[email protected]>
Signed-off-by: Eric Dumazet <[email protected]>
---
net/core/secure_seq.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 2cdd43a63f64..200ab4686275 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -22,6 +22,8 @@
static siphash_aligned_key_t net_secret;
static siphash_aligned_key_t ts_secret;

+#define EPHEMERAL_PORT_SHUFFLE_PERIOD (10 * HZ)
+
static __always_inline void net_secret_init(void)
{
net_get_random_once(&net_secret, sizeof(net_secret));
@@ -101,10 +103,12 @@ u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
struct in6_addr saddr;
struct in6_addr daddr;
__be16 dport;
+ unsigned int timeseed;
} __aligned(SIPHASH_ALIGNMENT) combined = {
.saddr = *(struct in6_addr *)saddr,
.daddr = *(struct in6_addr *)daddr,
- .dport = dport
+ .dport = dport,
+ .timeseed = jiffies / EPHEMERAL_PORT_SHUFFLE_PERIOD,
};
net_secret_init();
return siphash(&combined, offsetofend(typeof(combined), dport),
@@ -145,8 +149,10 @@ EXPORT_SYMBOL_GPL(secure_tcp_seq);
u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
{
net_secret_init();
- return siphash_3u32((__force u32)saddr, (__force u32)daddr,
- (__force u16)dport, &net_secret);
+ return siphash_4u32((__force u32)saddr, (__force u32)daddr,
+ (__force u16)dport,
+ jiffies / EPHEMERAL_PORT_SHUFFLE_PERIOD,
+ &net_secret);
}
EXPORT_SYMBOL_GPL(secure_ipv4_port_ephemeral);
#endif
--
2.17.5

2022-04-27 16:28:26

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH net 3/7] tcp: resalt the secret every 10 seconds

On Wed, 27 Apr 2022 08:52:29 +0200
Willy Tarreau <[email protected]> wrote:

> From: Eric Dumazet <[email protected]>
>
> In order to limit the ability for an observer to recognize the source
> ports sequence used to contact a set of destinations, we should
> periodically shuffle the secret. 10 seconds looks effective enough
> without causing particular issues.
>
> Cc: Moshe Kol <[email protected]>
> Cc: Yossi Gilad <[email protected]>
> Cc: Amit Klein <[email protected]>
> Tested-by: Willy Tarreau <[email protected]>
> Signed-off-by: Eric Dumazet <[email protected]>
> ---
> net/core/secure_seq.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> index 2cdd43a63f64..200ab4686275 100644
> --- a/net/core/secure_seq.c
> +++ b/net/core/secure_seq.c
> @@ -22,6 +22,8 @@
> static siphash_aligned_key_t net_secret;
> static siphash_aligned_key_t ts_secret;
>

Rather than hard coding, why not have a sysctl knob for this?
That way the tinfoil types can set it smaller.

2022-04-27 16:57:32

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH net 3/7] tcp: resalt the secret every 10 seconds

Hi Stephen,

On Wed, Apr 27, 2022 at 08:56:21AM -0700, Stephen Hemminger wrote:
> On Wed, 27 Apr 2022 08:52:29 +0200
> Willy Tarreau <[email protected]> wrote:
>
> > From: Eric Dumazet <[email protected]>
> >
> > In order to limit the ability for an observer to recognize the source
> > ports sequence used to contact a set of destinations, we should
> > periodically shuffle the secret. 10 seconds looks effective enough
> > without causing particular issues.
> >
> > Cc: Moshe Kol <[email protected]>
> > Cc: Yossi Gilad <[email protected]>
> > Cc: Amit Klein <[email protected]>
> > Tested-by: Willy Tarreau <[email protected]>
> > Signed-off-by: Eric Dumazet <[email protected]>
> > ---
> > net/core/secure_seq.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> > index 2cdd43a63f64..200ab4686275 100644
> > --- a/net/core/secure_seq.c
> > +++ b/net/core/secure_seq.c
> > @@ -22,6 +22,8 @@
> > static siphash_aligned_key_t net_secret;
> > static siphash_aligned_key_t ts_secret;
> >
>
> Rather than hard coding, why not have a sysctl knob for this?
> That way the tinfoil types can set it smaller.

It's a legit question. First I think that there's no good value; before
it used to be infinite, and now we're trying to figure a reasonable value
that make the attack impractical without going too close to the risk of
occasionally failing to establish a connection. I'm really not convinced
that there's any benefit in fiddling with that, except for breaking one's
stack by resalting too often and complaining about stupid network issues
with ACK or RST being sent in response to a SYN.

And stupidly, dividing jiffies by a constant known at build time is
slightly cheaper than dividing by a variable. I know it's a detail but
we tried hard to limit the accumulation of details here :-/

Just my two cents,
Willy