Re: [PATCH net] tcp: secure_seq: add back ports to TS offset

["Jörg Sommer" <joerg@jo-so.de>]

[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]

Eric Dumazet schrieb am Mo 02. Mär, 20:55 (+0000):
> This reverts 28ee1b746f49 ("secure_seq: downgrade to per-host timestamp offsets")
> 
> tcp_tw_recycle went away in 2017.
> 
> Zhouyan Deng reported off-path TCP source port leakage via
> SYN cookie side-channel that can be fixed in multiple ways.
> 
> One of them is to bring back TCP ports in TS offset randomization.
> 
> As a bonus, we perform a single siphash() computation
> to provide both an ISN and a TS offset.

This sounds great! I was questioning myself if the grace period for
tcp_tw_recycle isn't over.

> @@ -118,33 +99,30 @@ EXPORT_SYMBOL(secure_ipv6_port_ephemeral);
>  #endif
>  
>  #ifdef CONFIG_INET
> -u32 secure_tcp_ts_off(const struct net *net, __be32 saddr, __be32 daddr)
> -{
> -	if (READ_ONCE(net->ipv4.sysctl_tcp_timestamps) != 1)
> -		return 0;
> -
> -	ts_secret_init();
> -	return siphash_2u32((__force u32)saddr, (__force u32)daddr,
> -			    &ts_secret);
> -}
> -
>  /* secure_tcp_seq_and_tsoff(a, b, 0, d) == secure_ipv4_port_ephemeral(a, b, d),
>   * but fortunately, `sport' cannot be 0 in any circumstances. If this changes,
>   * it would be easy enough to have the former function use siphash_4u32, passing
>   * the arguments as separate u32.
>   */
> -u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
> -		   __be16 sport, __be16 dport)
> +union tcp_seq_and_ts_off
> +secure_tcp_seq_and_ts_off(const struct net *net, __be32 saddr, __be32 daddr,
> +			  __be16 sport, __be16 dport)
>  {
> -	u32 hash;
> +	u32 ports = (__force u32)sport << 16 | (__force u32)dport;
> +	union tcp_seq_and_ts_off st;
>  
>  	net_secret_init();
> -	hash = siphash_3u32((__force u32)saddr, (__force u32)daddr,
> -			    (__force u32)sport << 16 | (__force u32)dport,
> -			    &net_secret);
> -	return seq_scale(hash);
> +
> +	st.hash64 = siphash_3u32((__force u32)saddr, (__force u32)daddr,
> +				 ports, &net_secret);

Sorry, if this is a dump question, but does this make the ts_off unique per
connection or only per quadruple (saddr, sport, daddr, dport), i.e. the same
remote port gets the same ts_off. The documentation says ‘per connection’
and it might be helpful to say it gets the same ts_off if the addresses and
ports are the same.


Kind regards, Jörg

-- 
“Computer games don't affect kids. If Pacman would have affected us as
children, we would now run around in darkened rooms, munching yellow
pills and listening to repetetive music.”

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]