On Mon, Nov 07, 2022 at 07:08:46PM +0100, Stefano Brivio wrote:On Fri, 4 Nov 2022 19:43:33 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Well, except for the fact it's a 31.24 MHz / 32 ns clock. I assumed there was a good reason for that.It looks like tcp_seq_init() is supposed to advance the sequence number by one every 32ns. However we only right shift the ns part of the timespec not the seconds part, meaning that we'll advance by an extra 32 steps on each second. I don't know if that's exploitable in any way, but it doesn't appear to be the intent, nor what RFC 6528 suggests.Oh, oops, nice catch. Well, as long as the step, modulo 32 bits, is not 0, it's still arguably the 250 KHz / 4 µs period clock from RFC 793, so there's nopractical difference (other than the overflow period). See also the note in RFC 1948: More precisely, RFC 793 specifies that the 32-bit counter be incremented by 1 in the low-order position about every 4 microseconds. Instead, Berkeley-derived kernels increment it by a constant every second [...] I kind of wonder if adding a time non-linearity like the unintended one I added actually increases entropy.Right, I don't know.But indeed ~4 seconds overflow is also not intended, and we should just stick to RFC 6528.Well... 4s overflow, yes, but not I think a 4s period before repeating. Again, I don't know enough about this stuff to analyze whether that's important or not.-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibsonSigned-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tcp.c b/tcp.c index 59e03ff..941fafb 100644 --- a/tcp.c +++ b/tcp.c @@ -2027,8 +2027,8 @@ static void tcp_seq_init(const struct ctx *c, struct tcp_conn *conn, seq = siphash_36b((uint8_t *)&in, c->tcp.hash_secret); - ns = now->tv_sec * 1E9; - ns += now->tv_nsec >> 5; /* 32ns ticks, overflows 32 bits every 137s */ + /* 32ns ticks, overflows 32 bits every 137s */ + ns = (now->tv_sec * 1E9 + now->tv_nsec) >> 5; conn->seq_to_tap = seq + ns; }