Commit e114e1e8 authored Jul 01, 2020 by Eric Dumazet Committed by David S. Miller Jul 01, 2020

tcp: md5: do not send silly options in SYNCOOKIES

Whenever cookie_init_timestamp() has been used to encode
ECN,SACK,WSCALE options, we can not remove the TS option in the SYNACK.

Otherwise, tcp_synack_options() will still advertize options like WSCALE
that we can not deduce later when receiving the packet from the client
to complete 3WHS.

Note that modern linux TCP stacks wont use MD5+TS+SACK in a SYN packet,
but we can not know for sure that all TCP stacks have the same logic.

Before the fix a tcpdump would exhibit this wrong exchange :

10:12:15.464591 IP C > S: Flags [S], seq 4202415601, win 65535, options [nop,nop,md5 valid,mss 1400,sackOK,TS val 456965269 ecr 0,nop,wscale 8], length 0
10:12:15.464602 IP S > C: Flags [S.], seq 253516766, ack 4202415602, win 65535, options [nop,nop,md5 valid,mss 1400,nop,nop,sackOK,nop,wscale 8], length 0
10:12:15.464611 IP C > S: Flags [.], ack 1, win 256, options [nop,nop,md5 valid], length 0
10:12:15.464678 IP C > S: Flags [P.], seq 1:13, ack 1, win 256, options [nop,nop,md5 valid], length 12
10:12:15.464685 IP S > C: Flags [.], ack 13, win 65535, options [nop,nop,md5 valid], length 0

After this patch the exchange looks saner :

11:59:59.882990 IP C > S: Flags [S], seq 517075944, win 65535, options [nop,nop,md5 valid,mss 1400,sackOK,TS val 1751508483 ecr 0,nop,wscale 8], length 0
11:59:59.883002 IP S > C: Flags [S.], seq 1902939253, ack 517075945, win 65535, options [nop,nop,md5 valid,mss 1400,sackOK,TS val 1751508479 ecr 1751508483,nop,wscale 8], length 0
11:59:59.883012 IP C > S: Flags [.], ack 1, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508483 ecr 1751508479], length 0
11:59:59.883114 IP C > S: Flags [P.], seq 1:13, ack 1, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508483 ecr 1751508479], length 12
11:59:59.883122 IP S > C: Flags [.], ack 13, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508483 ecr 1751508483], length 0
11:59:59.883152 IP S > C: Flags [P.], seq 1:13, ack 13, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508484 ecr 1751508483], length 12
11:59:59.883170 IP C > S: Flags [.], ack 13, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508484 ecr 1751508484], length 0

Of course, no SACK block will ever be added later, but nothing should break.
Technically, we could remove the 4 nops included in MD5+TS options,
but again some stacks could break seeing not conventional alignment.

Fixes: 4957faad ("TCPCT part 1g: Responder Cookie => Initiator")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Florian Westphal <fw@strlen.de>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

parent 9ef845f8

net/ipv4/tcp_output.c

+5 −3

Original line number	Diff line number	Diff line
		@@ -700,7 +700,8 @@ static unsigned int tcp_synack_options(const struct sock *sk,
		unsigned int mss, struct sk_buff *skb,
		struct tcp_out_options *opts,
		const struct tcp_md5sig_key *md5,
		struct tcp_fastopen_cookie *foc)
		struct tcp_fastopen_cookie *foc,
		enum tcp_synack_type synack_type)
		{
		struct inet_request_sock *ireq = inet_rsk(req);
		unsigned int remaining = MAX_TCP_OPTION_SPACE;
		@@ -715,6 +716,7 @@ static unsigned int tcp_synack_options(const struct sock *sk,
		* rather than TS in order to fit in better with old,
		* buggy kernels, but that was deemed to be unnecessary.
		*/
		if (synack_type != TCP_SYNACK_COOKIE)
		ireq->tstamp_ok &= !ireq->sack_ok;
		}
		#endif
		@@ -3394,7 +3396,7 @@ struct sk_buff tcp_make_synack(const struct sock sk, struct dst_entry *dst,
		#endif
		skb_set_hash(skb, tcp_rsk(req)->txhash, PKT_HASH_TYPE_L4);
		tcp_header_size = tcp_synack_options(sk, req, mss, skb, &opts, md5,
		foc) + sizeof(*th);
		foc, synack_type) + sizeof(*th);

		skb_push(skb, tcp_header_size);
		skb_reset_transport_header(skb);

Admin message