: Netfilter fixes

PATCH Applied Patrick McHardy 2 participants 5 messages Oct 11, 2007

Subsystems (2)

Networking netfilter

lore

Share & tools

lore

Patrick McHardy

author

2007-10-11 16:44 UTC #0 lore raw

cover : Netfilter fixes

Hi Dave,

following are two netfilter fixes, adding missing IPv6 module aliases to a few matches and targets and fixing TCP conntrack connection reopening. I'll also push the conntrack patch to -stable once it hits upstream.

Please apply. thanks.

 net/netfilter/nf_conntrack_proto_tcp.c |   35 ++++++++++++-------------------
 net/netfilter/xt_CLASSIFY.c            |    1 +
 net/netfilter/xt_CONNMARK.c            |    1 +
 net/netfilter/xt_NOTRACK.c             |    1 +
 net/netfilter/xt_connbytes.c           |    1 +
 net/netfilter/xt_connmark.c            |    1 +
 net/netfilter/xt_dccp.c                |    1 +
 net/netfilter/xt_sctp.c                |    1 +
 net/netfilter/xt_tcpmss.c              |    1 +
 9 files changed, 22 insertions(+), 21 deletions(-)

Jan Engelhardt (1):
[NETFILTER]: x_tables: add missing ip6t_modulename aliases

Jozsef Kadlecsik (1):
[NETFILTER]: nf_conntrack_tcp: fix connection reopening

Patrick McHardy

author

2007-10-11 16:44 UTC #1 lore raw

PATCH 1/2 : nf_conntrack_tcp: fix connection reopening

[NETFILTER]: nf_conntrack_tcp: fix connection reopening

With your description I could reproduce the bug and actually you were completely right: the code above is incorrect. Somehow I was able to misread RFC1122 and mixed the roles :-(:

When a connection is >>closed actively<<, it MUST linger in TIME-WAIT state for a time 2xMSL (Maximum Segment Lifetime). However, it MAY >>accept<< a new SYN from the remote TCP to reopen the connection directly from TIME-WAIT state, if it:
[...]

The fix is as follows: if the receiver initiated an active close, then the sender may reopen the connection - otherwise try to figure out if we hold a dead connection.

Signed-off-by Jozsef Kadlecsik <[email protected]>

Tested-by Krzysztof Piotr Oledzki <[email protected]>

Signed-off-by Patrick McHardy <[email protected]>

---
commit c24af9fc1daa4e809ad2c5032d6504e57474bd1a
tree 1c75a5d1c7494551731acc83cbdae099814f2f7c
parent d23410bf25ccf95fe91d0f8f56bf2e82fde702a6
author Jozsef Kadlecsik <[email protected]> Thu, 11 Oct 2007 06:04:04 +0200 committer Patrick McHardy <[email protected]> Thu, 11 Oct 2007 06:04:04 +0200

 net/netfilter/nf_conntrack_proto_tcp.c |   35 +++++++++++++-------------------
 1 files changed, 14 insertions(+), 21 deletions(-)

net/netfilter/nf_conntrack_proto_tcp.c +14 ~~-21~~

index df718e7..c707534 100644
@@ -831,6 +831,20 @@ static int tcp_packet(struct nf_conn *conntrack,
 	tuple = &conntrack->tuplehash[dir].tuple;
 
 	switch (new_state) {
+	case TCP_CONNTRACK_SYN_SENT:
+		if (old_state < TCP_CONNTRACK_TIME_WAIT)
+			break;
+		if (conntrack->proto.tcp.seen[!dir].flags &
+			IP_CT_TCP_FLAG_CLOSE_INIT) {
+			/* Attempt to reopen a closed connection.
+			* Delete this connection and look up again. */
+			write_unlock_bh(&tcp_lock);
+			if (del_timer(&conntrack->timeout))
+				conntrack->timeout.function((unsigned long)
+							    conntrack);
+			return -NF_REPEAT;
+		}
+		/* Fall through */
 	case TCP_CONNTRACK_IGNORE:
 		/* Ignored packets:
 		 *

@@ -879,27 +893,6 @@ static int tcp_packet(struct nf_conn *conntrack,
 			nf_log_packet(pf, 0, skb, NULL, NULL, NULL,
 				  "nf_ct_tcp: invalid state ");
 		return -NF_ACCEPT;
-	case TCP_CONNTRACK_SYN_SENT:
-		if (old_state < TCP_CONNTRACK_TIME_WAIT)
-			break;
-		if ((conntrack->proto.tcp.seen[dir].flags &
-			IP_CT_TCP_FLAG_CLOSE_INIT)
-		    || after(ntohl(th->seq),
-			     conntrack->proto.tcp.seen[dir].td_end)) {
-			/* Attempt to reopen a closed connection.
-			* Delete this connection and look up again. */
-			write_unlock_bh(&tcp_lock);
-			if (del_timer(&conntrack->timeout))
-				conntrack->timeout.function((unsigned long)
-							    conntrack);
-			return -NF_REPEAT;
-		} else {
-			write_unlock_bh(&tcp_lock);
-			if (LOG_INVALID(IPPROTO_TCP))
-				nf_log_packet(pf, 0, skb, NULL, NULL,
-					      NULL, "nf_ct_tcp: invalid SYN");
-			return -NF_ACCEPT;
-		}
 	case TCP_CONNTRACK_CLOSE:
 		if (index == TCP_RST_SET
 		    && ((test_bit(IPS_SEEN_REPLY_BIT, &conntrack->status)

David Miller

maintainer

2007-10-11 21:37 UTC #2 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening

From: Patrick McHardy <[email protected]>
Date: Thu, 11 Oct 2007 18:44:04 +0200 (MEST)

Quoted 12 lines

[NETFILTER]: nf_conntrack_tcp: fix connection reopening
With your description I could reproduce the bug and actually you were completely right: the code above is incorrect. Somehow I was able to misread RFC1122 and mixed the roles :-(:
When a connection is >>closed actively<<, it MUST linger in TIME-WAIT state for a time 2xMSL (Maximum Segment Lifetime). However, it MAY >>accept<< a new SYN from the remote TCP to reopen the connection directly from TIME-WAIT state, if it:
   [...]

Quoted 7 lines

The fix is as follows: if the receiver initiated an active close, then the sender may reopen the connection - otherwise try to figure out if we hold a dead connection.
Signed-off-by Jozsef Kadlecsik <[email protected]>
Tested-by Krzysztof Piotr Oledzki <[email protected]>
Signed-off-by Patrick McHardy <[email protected]>

Patch applied.

Patrick McHardy

author

2007-10-11 16:44 UTC #3 lore raw

PATCH 2/2 : x_tables: add missing ip6t_modulename aliases

In reply to Patrick McHardy

[NETFILTER]: x_tables: add missing ip6t_modulename aliases

The patch will add MODULE_ALIAS("ip6t_<modulename>") where missing, otherwise you will get

ip6tables: No chain/target/match by that name

when xt_<modulename> is not already loaded.

Signed-off-by Jan Engelhardt <[email protected]>

Signed-off-by Patrick McHardy <[email protected]>

---
commit 3045023bc5004d09b4cba553e9df6e3d0f3fa6d9
tree ff50a2197669da3b377e44ceecb91c0b5d9e004c
parent c24af9fc1daa4e809ad2c5032d6504e57474bd1a
author Jan Engelhardt <[email protected]> Thu, 11 Oct 2007 06:07:18 +0200 committer Patrick McHardy <[email protected]> Thu, 11 Oct 2007 06:07:18 +0200

 net/netfilter/xt_CLASSIFY.c  |    1 +
 net/netfilter/xt_CONNMARK.c  |    1 +
 net/netfilter/xt_NOTRACK.c   |    1 +
 net/netfilter/xt_connbytes.c |    1 +
 net/netfilter/xt_connmark.c  |    1 +
 net/netfilter/xt_dccp.c      |    1 +
 net/netfilter/xt_sctp.c      |    1 +
 net/netfilter/xt_tcpmss.c    |    1 +
 8 files changed, 8 insertions(+), 0 deletions(-)

net/netfilter/xt_CLASSIFY.c +1 -0

index 5194285..07a1b96 100644
@@ -24,6 +24,7 @@ MODULE_AUTHOR("Patrick McHardy <[email protected]>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("iptables qdisc classification target module");
 MODULE_ALIAS("ipt_CLASSIFY");
+MODULE_ALIAS("ip6t_CLASSIFY");
 
 static unsigned int
 target(struct sk_buff **pskb,

net/netfilter/xt_CONNMARK.c +1 -0

index 5a00c54..7043c27 100644
@@ -27,6 +27,7 @@ MODULE_AUTHOR("Henrik Nordstrom <[email protected]>");
 MODULE_DESCRIPTION("IP tables CONNMARK matching module");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ipt_CONNMARK");
+MODULE_ALIAS("ip6t_CONNMARK");
 
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_CONNMARK.h>

net/netfilter/xt_NOTRACK.c +1 -0

index b7d6312..fec1aef 100644
@@ -9,6 +9,7 @@
 
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ipt_NOTRACK");
+MODULE_ALIAS("ip6t_NOTRACK");
 
 static unsigned int
 target(struct sk_buff **pskb,

net/netfilter/xt_connbytes.c +1 -0

index dd4d79b..af79423 100644
@@ -14,6 +14,7 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Harald Welte <[email protected]>");
 MODULE_DESCRIPTION("iptables match for matching number of pkts/bytes per connection");
 MODULE_ALIAS("ipt_connbytes");
+MODULE_ALIAS("ip6t_connbytes");
 
 static bool
 match(const struct sk_buff *skb,

net/netfilter/xt_connmark.c +1 -0

index e73fa9b..1071fc5 100644
@@ -29,6 +29,7 @@ MODULE_AUTHOR("Henrik Nordstrom <[email protected]>");
 MODULE_DESCRIPTION("IP tables connmark match module");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ipt_connmark");
+MODULE_ALIAS("ip6t_connmark");
 
 static bool
 match(const struct sk_buff *skb,

net/netfilter/xt_dccp.c +1 -0

index 83224ec..c2b1b24 100644
@@ -24,6 +24,7 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Harald Welte <[email protected]>");
 MODULE_DESCRIPTION("Match for DCCP protocol packets");
 MODULE_ALIAS("ipt_dccp");
+MODULE_ALIAS("ip6t_dccp");
 
 #define DCCHECK(cond, option, flag, invflag) (!((flag) & (option)) \
 				  || (!!((invflag) & (option)) ^ (cond)))

net/netfilter/xt_sctp.c +1 -0

index c002153..f907770 100644
@@ -13,6 +13,7 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Kiran Kumar Immidi");
 MODULE_DESCRIPTION("Match for SCTP protocol packets");
 MODULE_ALIAS("ipt_sctp");
+MODULE_ALIAS("ip6t_sctp");
 
 #ifdef DEBUG_SCTP
 #define duprintf(format, args...) printk(format , ## args)

net/netfilter/xt_tcpmss.c +1 -0

index cd5f6d7..84d401b 100644
@@ -22,6 +22,7 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Marc Boucher <[email protected]>");
 MODULE_DESCRIPTION("iptables TCP MSS match module");
 MODULE_ALIAS("ipt_tcpmss");
+MODULE_ALIAS("ip6t_tcpmss");
 
 static bool
 match(const struct sk_buff *skb,

David Miller

maintainer Applied

2007-10-11 21:37 UTC #4 lore raw

Re: [NETFILTER 02/02]: x_tables: add missing ip6t_modulename aliases

From: Patrick McHardy <[email protected]>
Date: Thu, 11 Oct 2007 18:44:06 +0200 (MEST)

Quoted 11 lines

[NETFILTER]: x_tables: add missing ip6t_modulename aliases
The patch will add MODULE_ALIAS("ip6t_<modulename>") where missing, otherwise you will get
ip6tables: No chain/target/match by that name
when xt_<modulename> is not already loaded.
Signed-off-by Jan Engelhardt <[email protected]>
Signed-off-by Patrick McHardy <[email protected]>

Applied, thanks!