Re: PROBLEM: nf_conntrack tcp SYN reuse results in incorrect window scaling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



It would help if included a patch that would compile..

>From d5eb5c9bb7e7b8312282827bd304b1d5ae329842 Mon Sep 17 00:00:00 2001
From: Ryan Schaefer <ryanschf@xxxxxxxxxx>
Date: Fri, 19 Jan 2024 19:51:45 +0000
Subject: [PATCH] netfilter: conntrack: correct window scaling with
 retransmitted SYN

commit c7aab4f17021 ("netfilter: nf_conntrack_tcp: re-init for syn packets
only") introduces a bug where SYNs in ORIGINAL direction on reused 5-tuple
result in incorrect window scale negotiation. This commit merged the SYN
re-initialization and simultaneous open or SYN retransmits cases. Merging
this block added the logic in tcp_init_sender() that performed window scale
negotiation to the retransmitted syn case. Previously. this would only
result in updating the sender's scale and flags. After the merge the
additional logic results in improperly clearing the scale in ORIGINAL
direction before any packets in the REPLY direction are received. This
results in packets incorrectly being marked invalid for being
out-of-window.

This can be reproduced with the following trace:

Packet Sequence:
> Flags [S], seq 1687765604, win 62727, options [.. wscale 7], length 0
> Flags [S], seq 1944817196, win 62727, options [.. wscale 7], length 0

In order to fix the issue, only evaluate window negotiation for packets
in the REPLY direction. This was tested with simultaneous open, fast
open, and the above reproduction.

Fixes: c7aab4f17021 ("netfilter: nf_conntrack_tcp: re-init for syn packets only")
Signed-off-by: Ryan Schaefer <ryanschf@xxxxxxxxxx>
---
 net/netfilter/nf_conntrack_proto_tcp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 656631083177..d84bec388b08 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -457,7 +457,8 @@ static void tcp_init_sender(struct ip_ct_tcp_state *sender,
 			    const struct sk_buff *skb,
 			    unsigned int dataoff,
 			    const struct tcphdr *tcph,
-			    u32 end, u32 win)
+			    u32 end, u32 win,
+			    enum ip_conntrack_dir dir)
 {
 	/* SYN-ACK in reply to a SYN
 	 * or SYN from reply direction in simultaneous open.
@@ -471,7 +472,8 @@ static void tcp_init_sender(struct ip_ct_tcp_state *sender,
 	 * Both sides must send the Window Scale option
 	 * to enable window scaling in either direction.
 	 */
-	if (!(sender->flags & IP_CT_TCP_FLAG_WINDOW_SCALE &&
+	if (dir == IP_CT_DIR_REPLY &&
+	    !(sender->flags & IP_CT_TCP_FLAG_WINDOW_SCALE &&
 	      receiver->flags & IP_CT_TCP_FLAG_WINDOW_SCALE)) {
 		sender->td_scale = 0;
 		receiver->td_scale = 0;
@@ -542,7 +544,7 @@ tcp_in_window(struct nf_conn *ct, enum ip_conntrack_dir dir,
 		if (tcph->syn) {
 			tcp_init_sender(sender, receiver,
 					skb, dataoff, tcph,
-					end, win);
+					end, win, dir);
 			if (!tcph->ack)
 				/* Simultaneous open */
 				return NFCT_TCP_ACCEPT;
@@ -585,7 +587,7 @@ tcp_in_window(struct nf_conn *ct, enum ip_conntrack_dir dir,
 		 */
 		tcp_init_sender(sender, receiver,
 				skb, dataoff, tcph,
-				end, win);
+				end, win, dir);
 
 		if (dir == IP_CT_DIR_REPLY && !tcph->ack)
 			return NFCT_TCP_ACCEPT;
-- 
2.40.1



On Fri, 2024-01-19 at 17:11 -0800, Schaefer, Ryan wrote:
> PROBLEM: nf_conntrack tcp SYN reuse results in incorrect window
> scaling
> 
> There is a bug in netfilter conntrack tcp behavior. Commit
> c7aab4f17021
> ("netfilter: nf_conntrack_tcp: re-init for syn packets only") changed
> behavior that broke connection establishment when there is a new
> connection attempt on a connection currently in SYN_SENT state. The
> result is future packets are incorrectly considered out of window and
> can be dropped. This can be reproduced by enabling firewall rules for
> tcp and attempting to connect to a server that does not respond to
> SYNs.
> 
> Keywords: conntrack, nf_conntrack_tcp_proto
> Kernel Version: 6.172
> Kernel Version without bug: 5.15
> 
> Machine 1: // force connection tracking of port 9000
> # sudo sysctl net.netfilter.nf_log.2=nf_log_ipv4
> # sudo sysctl -w net.netfilter.nf_conntrack_log_invalid=6
> # sudo iptables -A OUTPUT -p tcp --dport 9000 -j ACCEPT
> # sudo iptables -A INPUT -p tcp -m tcp --sport 9000 -m state --state
> ESTABLISHED -j ACCEPT
> # sudo iptables -P INPUT DROP; iptables -P OUTPUT DROP;
> 
> Machine 2: // disable port 9000 on server
> # sudo iptables -P INPUT DROP;
> # socat socat OPEN:/dev/zero TCP4-LISTEN:9000,reuseaddr,fork
> 
> Machine 1: // perform first connection attempt. ctrl+c after some
> seconds
> # socat OPEN:/dev/zero TCP4:10.0.85.65:9000,sourceport=55560
> 
> Machine 2: // allow next connection attempt
> # sudo iptables -A INPUT -p tcp -m tcp --dport 9000 -j ACCEPT
> 
> Machine 1: reattempt connection
> # socat OPEN:/dev/zero TCP4:10.0.85.65:9000,sourceport=55560
> # dmesg | grep 9000 | grep " ACK " | tail -n1
> [16690.645068] nf_ct_proto_6: SEQ is over upper bound 2624537083
> (over
> the window of the receiver) IN=eth0 OUT=
> MAC=0e:6c:20:4d:61:0a:0e:ff:68:7e:1b:a6:08:00 SRC=10.0.85.65
> DST=10.0.83.30 LEN=52 TOS=0x00 PREC=0x00 TTL=255 ID=11781 DF
> PROTO=TCP
> SPT=9000 DPT=55560 SEQ=2653233300 ACK=2726773975 WINDOW=24555
> RES=0x00
> ACK URGP=0 OPT (0101080A47F2131D2ABC62AA)
> 
> The repeat socat attempt will result in re-using the same 5-tuple
> with
> the greater SEQ number and trigger the bug. This was reproduced with
> additional prints in tcp_init_sender() showing that the state was
> incorrectly reset during the SYN retransmit.
> 
> The following patch below addresses the issue.
> 
> From 92d9690066be1250d8aac3215930cb222510e17b Mon Sep 17 00:00:00
> 2001
> From: Ryan Schaefer <ryanschf@xxxxxxxxxx>
> Date: Fri, 19 Jan 2024 19:51:45 +0000
> Subject: [PATCH] netfilter: conntrack: correct window scaling with
>  retransmitted SYN
> 
> commit c7aab4f17021 ("netfilter: nf_conntrack_tcp: re-init for syn
> packets
> only") introduces a bug where SYNs in ORIGINAL direction on reused 5-
> tuple
> result in incorrect window scale negotiation. This commit merged the
> SYN
> re-initialization and simultaneous open or SYN retransmits cases.
> Merging
> this block added the logic in tcp_init_sender() that performed window
> scale
> negotiation to the retransmitted syn case. Previously. this would
> only
> result in updating the sender's scale and flags. After the merge the
> additional logic results in improperly clearing the scale in ORIGINAL
> direction before any packets in the REPLY direction are received.
> This
> results in packets incorrectly being marked invalid for being
> out-of-window.
> 
> This can be reproduced with the following trace:
> 
> Packet Sequence:
> > Flags [S], seq 1687765604, win 62727, options [.. wscale 7], length
> > 0
> > Flags [S], seq 1944817196, win 62727, options [.. wscale 7], length
> > 0
> 
> In order to fix the issue, only evaluate window negotiation for
> packets
> in the REPLY direction. This was tested with simultaneous open, fast
> open, and the above reproduction.
> 
> Fixes: c7aab4f17021 ("netfilter: nf_conntrack_tcp: re-init for syn
> packets only")
> Signed-off-by: Ryan Schaefer <ryanschf@xxxxxxxxxx>
> ---
>  net/netfilter/nf_conntrack_proto_tcp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
> b/net/netfilter/nf_conntrack_proto_tcp.c
> index e573be5afde7..3c2c70ae0b66 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -457,7 +457,8 @@ static void tcp_init_sender(struct
> ip_ct_tcp_state
> *sender,
>  			    const struct sk_buff *skb,
>  			    unsigned int dataoff,
>  			    const struct tcphdr *tcph,
> -			    u32 end, u32 win)
> +			    u32 end, u32 win,
> +			    enum ip_conntrack_dir dir)
>  {
>  	/* SYN-ACK in reply to a SYN
>  	 * or SYN from reply direction in simultaneous open.
> @@ -471,7 +472,8 @@ static void tcp_init_sender(struct
> ip_ct_tcp_state
> *sender,
>  	 * Both sides must send the Window Scale option
>  	 * to enable window scaling in either direction.
>  	 */
> -	if (!(sender->flags & IP_CT_TCP_FLAG_WINDOW_SCALE &&
> +	if (dir == IP_CT_DIR_REPLY &&
> +	    !(sender->flags & IP_CT_TCP_FLAG_WINDOW_SCALE &&
>  	      receiver->flags & IP_CT_TCP_FLAG_WINDOW_SCALE)) {
>  		sender->td_scale = 0;
>  		receiver->td_scale = 0;




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux