netdev - Re: [PATCH] [Syncookies:] Add support for TCP-options via timestamps.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 31 Mar 2008 11:19:46 -0700
From:	Glenn Griffin <ggriffin.kernel@...il.com>
To:	Florian Westphal <fw@...len.de>
Cc:	netdev@...r.kernel.org, Glenn Griffin <ggriffin.kernel@...il.com>
Subject: Re: [PATCH] [Syncookies:] Add support for TCP-options via
	timestamps.

> The downside is that the timestamp sent in the packet after the synack
> will increase by several seconds.

I didn't see any huge downsides quickly scanning over it.  This could
cause a difference of upto ~500 jiffies from what we will send on the
first packet sent after receiving an ack.  If you were connecting to
another linux peer I don't believe this would cause any huge issues, and
the rtt calculations would quickly recover once actual data
communications took place.  The problem is the interpretation of our
timestamp is dependant on the remote systems logic and while smart peers
could quickly recover from the relatively small discrepancy during the
handshake it's hard to determine what effect it will have on other
hosts.

Considering this only matters when we are already in the midst of a DOS
attack, and tcp timestamps would otherwise be disabled it seems like an
okay tradeoff to me.

> +static u32 options_encode(u32 options)
> +{
> +	u32 ts, ts_now = tcp_time_stamp;
> +
> +	if (unlikely(options > ts_now)) { /* recent overflow */
> +		options |= ~(TSMASK);
> +		return options;
> +	}
> +	ts = ts_now & ~TSMASK;
> +	ts |= options;
> +	if (ts > ts_now) { /* try to fix up ... */
> +		ts >>= TSBITS;
> +		ts--;
> +		ts <<= TSBITS;
> +		ts |= options;
> +	}
> +        return ts;
> +}

I may be missing something obvious, but I'm failing to see where the
initial if(options > ts_now) does anything different from the more
generic logic below it.  In order for ts_now to be smaller than options
it would need to have all bits above TSBITS off.  If that's the case
then 

ts >>= TSBITS;
ts--;
ts <<= TSBITS;

will flip all those bits on just as you do with

options |= ~(TSMASK)

Also the 'return ts' line is indented using spaces rather than tabs.

--Glenn Griffin
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html