netdev - RE: [PATCH -next v3 1/3] syncookies: avoid magic values and document which-bit-is-what-option

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]

Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1C9E44B1@AcuExch.aculab.com>
Date:	Mon, 3 Nov 2014 14:24:39 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Florian Westphal' <fw@...len.de>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	Daniel Borkmann <dborkman@...hat.com>
Subject: RE: [PATCH -next v3 1/3] syncookies: avoid magic values and
 document which-bit-is-what-option

From: Florian Westphal
> Was a bit more difficult to read than needed due to magic shifts;
> add defines and document the used encoding scheme.
> 
> Joint work with Daniel Borkmann.
> 
> Signed-off-by: Daniel Borkmann <dborkman@...hat.com>
> Signed-off-by: Florian Westphal <fw@...len.de>
> ---
>  This patch was not part of earlier versions of the set.
> 
>  net/ipv4/syncookies.c | 50 +++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 35 insertions(+), 15 deletions(-)
> 
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 4ac7bca..c3792c0 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -19,10 +19,6 @@
>  #include <net/tcp.h>
>  #include <net/route.h>
> 
> -/* Timestamps: lowest bits store TCP options */
> -#define TSBITS 6
> -#define TSMASK (((__u32)1 << TSBITS) - 1)
> -
>  extern int sysctl_tcp_syncookies;
> 
>  static u32 syncookie_secret[2][16-4+SHA_DIGEST_WORDS] __read_mostly;
> @@ -30,6 +26,30 @@ static u32 syncookie_secret[2][16-4+SHA_DIGEST_WORDS] __read_mostly;
>  #define COOKIEBITS 24	/* Upper bits store count */
>  #define COOKIEMASK (((__u32)1 << COOKIEBITS) - 1)
> 
> +/* TCP Timestamp: 6 lowest bits of timestamp sent in the cookie SYN-ACK
> + * stores TCP options:
> + *
> + * MSB                               LSB
> + * | 31 ...   6 |  5  |  4   | 3 2 1 0 |
> + * |  Timestamp | ECN | SACK | WScale  |
> + *
> + * When we receive a valid cookie-ACK, we look at the echoed tsval (if
> + * any) to figure out which TCP options we should use for the rebuilt
> + * connection.
> + *
> + * A WScale setting of '0xf' (which is an invalid scaling value)
> + * means that original syn did not include the TCP window scaling option.
> + */
> +#define TS_OPT_WSCALE_MASK	0xf
> +#define TS_OPT_SACK		BIT(4)
> +#define TS_OPT_ECN		BIT(5)
> +/* There is no TS_OPT_TIMESTAMP:
> + * if ACK contains timestamp option, we already know it was
> + * requested/supported by the syn/synack exchange.
> + */
> +#define TSBITS	6
> +#define TSMASK	(((__u32)1 << TSBITS) - 1)

Personally I'd define all the values as hex constants instead of mixing
and matching the defines.

So probably just:
#define TS_OPT_WSCALE_MASK	0x0f
#define TS_OPT_SACK		0x10
#define TS_OPT_ECN		0x20
#define TSMASK                0x3f

	David





--
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