Re: [RFC 1/3] tcp: randomize tcp timestamp offsets for each connection

[Florian Westphal <fw@strlen.de>]

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2016-08-18 at 14:48 +0200, Florian Westphal wrote:
> > commit ceaa1fef65a7c2e ("tcp: adding a per-socket timestamp offset")
> > added the main infrastructure that is needed for per-connection
> > randomization, in particular writing/reading the on-wire tcp header
> > format takes the offset into account so rest of stack can use normal
> > tcp_time_stamp (jiffies).

[..]

> > +secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
> > +			     __be16 sport, __be16 dport)
> >  {
> >  	u32 secret[MD5_MESSAGE_BYTES / 4];
> >  	u32 hash[MD5_DIGEST_WORDS];
> > +	struct secure_tcp_seq seq;
> >  	u32 i;
> >  
> >  	net_secret_init();
> > @@ -58,7 +60,9 @@ __u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
> >  
> >  	md5_transform(hash, secret);
> >  
> > -	return seq_scale(hash[0]);
> > +	seq.seq = seq_scale(hash[0]);
> > +	seq.tsoff = hash[1];
> > +	return seq;
> >  }
> 
> 
> I am not a super fan of this "struct secure_tcp_seq" being returned by
> functions. This adds unnecessary overhead.
>
> I would instead add a "u32 *ts_off" parameter, as you already did for
> tcp_v4_init_sequence()
>
> Patch on top of yours :
[..]

Looks great, I squashed it into my working branch.

Wrt. making randomization optional:

Would you go for another sysctl or should I just change
secure_tcpvX_sequence_number to check for tcp_timestamps == 2 mode?

	*tsoff  = sysctl_tcp_timestamps == 2 ? hash[1] : 0;

Could also use a static key but I don't think its worth it vs. md5 cost.
What do you think?

Thanks,
Florian