| Message ID | 20220427065233.2075-4-w@1wt.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | insufficient TCP source port randomness | expand |
On Wed, 27 Apr 2022 08:52:29 +0200 Willy Tarreau <w@1wt.eu> wrote: > From: Eric Dumazet <edumazet@google.com> > > 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 <moshe.kol@mail.huji.ac.il> > Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il> > Cc: Amit Klein <aksecurity@gmail.com> > Tested-by: Willy Tarreau <w@1wt.eu> > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > 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.
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 <w@1wt.eu> wrote: > > > From: Eric Dumazet <edumazet@google.com> > > > > 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 <moshe.kol@mail.huji.ac.il> > > Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il> > > Cc: Amit Klein <aksecurity@gmail.com> > > Tested-by: Willy Tarreau <w@1wt.eu> > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > --- > > 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
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