Patchwork [next] netfilter: conntrack: avoid large timeout for mid-stream pickup

login
register
mail settings
Submitter Florian Westphal
Date June 13, 2013, 3:31 p.m.
Message ID <1371137488-16428-1-git-send-email-fw@strlen.de>
Download mbox | patch
Permalink /patch/251094/
State Accepted
Delegated to: Pablo Neira
Headers show

Comments

Florian Westphal - June 13, 2013, 3:31 p.m.
When loose tracking is enabled (default), non-syn packets cause
creation of new conntracks in established state with default timeout for
established state (5 days).  This causes the table to fill up with UNREPLIED
when the 'new ack' packet happened to be the last-ack of a previous,
already timed-out connection.

Consider:

A 192.168.x.52792 > 10.184.y.80: F, 426:426(0) ack 9237 win 255
B 10.184.y.80 > 192.168.x.52792: ., ack 427 win 123
<61 second pause>
C 10.184.y.80 > 192.168.x.52792: F, 9237:9237(0) ack 427 win 123
D 192.168.x.52792 > 10.184.y.80: ., ack 9238 win 255

B moves conntrack to CLOSE_WAIT and will kill it after 60 second timeout,
C is ignored (FIN set), but last packet (D) causes new ct with 5-days timeout.

Use UNACK timeout (5 minutes) instead to get rid of these entries sooner
when in ESTABLISHED state without having seen traffic in both directions.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Warning; this is a bit different from the initial proposal in
 that it no longer bothers with old_state (the branch is taken
 for !SEEN_REPLY only).
Jozsef Kadlecsik - June 13, 2013, 5:21 p.m.
On Thu, 13 Jun 2013, Florian Westphal wrote:

> When loose tracking is enabled (default), non-syn packets cause
> creation of new conntracks in established state with default timeout for
> established state (5 days).  This causes the table to fill up with UNREPLIED
> when the 'new ack' packet happened to be the last-ack of a previous,
> already timed-out connection.
> 
> Consider:
> 
> A 192.168.x.52792 > 10.184.y.80: F, 426:426(0) ack 9237 win 255
> B 10.184.y.80 > 192.168.x.52792: ., ack 427 win 123
> <61 second pause>
> C 10.184.y.80 > 192.168.x.52792: F, 9237:9237(0) ack 427 win 123
> D 192.168.x.52792 > 10.184.y.80: ., ack 9238 win 255
> 
> B moves conntrack to CLOSE_WAIT and will kill it after 60 second timeout,
> C is ignored (FIN set), but last packet (D) causes new ct with 5-days timeout.
> 
> Use UNACK timeout (5 minutes) instead to get rid of these entries sooner
> when in ESTABLISHED state without having seen traffic in both directions.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

Best regards,
Jozsef

> ---
>  Warning; this is a bit different from the initial proposal in
>  that it no longer bothers with old_state (the branch is taken
>  for !SEEN_REPLY only).

It's all right and one condition is spared :-).

> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 4d4d8f1..7dcc376 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -1043,6 +1043,12 @@ static int tcp_packet(struct nf_conn *ct,
>  			nf_ct_kill_acct(ct, ctinfo, skb);
>  			return NF_ACCEPT;
>  		}
> +		/* ESTABLISHED without SEEN_REPLY, i.e. mid-connection
> +		 * pickup with loose=1. Avoid large ESTABLISHED timeout.
> +		 */
> +		if (new_state == TCP_CONNTRACK_ESTABLISHED &&
> +		    timeout > timeouts[TCP_CONNTRACK_UNACK])
> +			timeout = timeouts[TCP_CONNTRACK_UNACK];
>  	} else if (!test_bit(IPS_ASSURED_BIT, &ct->status)
>  		   && (old_state == TCP_CONNTRACK_SYN_RECV
>  		       || old_state == TCP_CONNTRACK_ESTABLISHED)
> -- 
> 1.7.8.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - June 20, 2013, 9:45 a.m.
On Thu, Jun 13, 2013 at 05:31:28PM +0200, Florian Westphal wrote:
> When loose tracking is enabled (default), non-syn packets cause
> creation of new conntracks in established state with default timeout for
> established state (5 days).  This causes the table to fill up with UNREPLIED
> when the 'new ack' packet happened to be the last-ack of a previous,
> already timed-out connection.
> 
> Consider:
> 
> A 192.168.x.52792 > 10.184.y.80: F, 426:426(0) ack 9237 win 255
> B 10.184.y.80 > 192.168.x.52792: ., ack 427 win 123
> <61 second pause>
> C 10.184.y.80 > 192.168.x.52792: F, 9237:9237(0) ack 427 win 123
> D 192.168.x.52792 > 10.184.y.80: ., ack 9238 win 255
> 
> B moves conntrack to CLOSE_WAIT and will kill it after 60 second timeout,
> C is ignored (FIN set), but last packet (D) causes new ct with 5-days timeout.
> 
> Use UNACK timeout (5 minutes) instead to get rid of these entries sooner
> when in ESTABLISHED state without having seen traffic in both directions.

Applied, thanks!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 4d4d8f1..7dcc376 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -1043,6 +1043,12 @@  static int tcp_packet(struct nf_conn *ct,
 			nf_ct_kill_acct(ct, ctinfo, skb);
 			return NF_ACCEPT;
 		}
+		/* ESTABLISHED without SEEN_REPLY, i.e. mid-connection
+		 * pickup with loose=1. Avoid large ESTABLISHED timeout.
+		 */
+		if (new_state == TCP_CONNTRACK_ESTABLISHED &&
+		    timeout > timeouts[TCP_CONNTRACK_UNACK])
+			timeout = timeouts[TCP_CONNTRACK_UNACK];
 	} else if (!test_bit(IPS_ASSURED_BIT, &ct->status)
 		   && (old_state == TCP_CONNTRACK_SYN_RECV
 		       || old_state == TCP_CONNTRACK_ESTABLISHED)