Skip to contentSkip to conversation

kill the fake conntrack

RFC PATCH In review Pablo Neira 3 participants 8 messages Jun 25, 2005 – Aug 28, 2005

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 kill the fake conntrack

Hi,

Well, the subject looks like good action film but it isn't :)

The patch attached kills the fake conntrack and propose a new logic to explicitely set connection as untracked. We set nfct to NULL and use a new flag called IP_CT_UNTRACKED for nfctinfo. I've slightely tested it here and works fine.

Comments welcome.

--
Pablo

include/linux/netfilter_ipv4/ip_conntrack.h +4 -1
Index: linux-2.6/include/linux/netfilter_ipv4/ip_conntrack.h
===================================================================
@@ -20,7 +20,10 @@
 	IP_CT_IS_REPLY,
 
 	/* Number of distinct IP_CT types (no NEW in reply dirn). */
-	IP_CT_NUMBER = IP_CT_IS_REPLY * 2 - 1
+	IP_CT_NUMBER = IP_CT_IS_REPLY * 2 - 1,
+
+	/* This packet belongs to a untracked connection */
+	IP_CT_UNTRACKED = ~0U
 };
 
 /* Bitset representing status of connection. */
net/ipv4/netfilter/ip_nat_core.c +0 -2
Index: linux-2.6/net/ipv4/netfilter/ip_nat_core.c
===================================================================
@@ -525,8 +525,6 @@
 	IP_NF_ASSERT(ip_conntrack_destroyed == NULL);
 	ip_conntrack_destroyed = &ip_nat_cleanup_conntrack;
 
-	/* Initialize fake conntrack so that NAT will skip it */
-	ip_conntrack_untracked.status |= IPS_NAT_DONE_MASK;
 	return 0;
 }
 
net/ipv4/netfilter/ip_conntrack_standalone.c +0 -1
Index: linux-2.6/net/ipv4/netfilter/ip_conntrack_standalone.c
===================================================================
@@ -986,7 +986,6 @@
 EXPORT_SYMBOL(ip_conntrack_htable_size);
 EXPORT_SYMBOL(ip_conntrack_lock);
 EXPORT_SYMBOL(ip_conntrack_hash);
-EXPORT_SYMBOL(ip_conntrack_untracked);
 EXPORT_SYMBOL_GPL(ip_conntrack_find_get);
 EXPORT_SYMBOL_GPL(ip_conntrack_put);
 #ifdef CONFIG_IP_NF_NAT_NEEDED
net/ipv4/netfilter/ipt_conntrack.c +1 -1
Index: linux-2.6/net/ipv4/netfilter/ipt_conntrack.c
===================================================================
@@ -35,7 +35,7 @@
 
 #define FWINV(bool,invflg) ((bool) ^ !!(sinfo->invflags & invflg))
 
-	if (ct == &ip_conntrack_untracked)
+	if (ctinfo == IP_CT_UNTRACKED)
 		statebit = IPT_CONNTRACK_STATE_UNTRACKED;
 	else if (ct)
  		statebit = IPT_CONNTRACK_STATE_BIT(ctinfo);
net/ipv4/netfilter/ipt_state.c +1 -1
Index: linux-2.6/net/ipv4/netfilter/ipt_state.c
===================================================================
@@ -30,7 +30,7 @@
 	enum ip_conntrack_info ctinfo;
 	unsigned int statebit;
 
-	if (skb->nfct == &ip_conntrack_untracked.ct_general)
+	if (skb->nfctinfo == IP_CT_UNTRACKED)
 		statebit = IPT_STATE_UNTRACKED;
 	else if (!ip_conntrack_get(skb, &ctinfo))
 		statebit = IPT_STATE_INVALID;
net/ipv4/netfilter/ip_conntrack_core.c +1 -8
Index: linux-2.6/net/ipv4/netfilter/ip_conntrack_core.c
===================================================================
@@ -71,7 +71,6 @@
 struct list_head *ip_conntrack_hash;
 static kmem_cache_t *ip_conntrack_cachep;
 static kmem_cache_t *ip_conntrack_expect_cachep;
-struct ip_conntrack ip_conntrack_untracked;
 unsigned int ip_ct_log_invalid;
 static LIST_HEAD(unconfirmed);
 static int ip_conntrack_vmalloc;
@@ -613,7 +612,7 @@
 	int ret;
 
 	/* Previously seen (loopback or untracked)?  Ignore. */
-	if ((*pskb)->nfct) {
+	if ((*pskb)->nfct || (*pskb)->nfctinfo == IP_CT_UNTRACKED) {
 		CONNTRACK_STAT_INC(ignore);
 		return NF_ACCEPT;
 	}
@@ -1203,12 +1202,6 @@
 	/* For use by ipt_REJECT */
 	ip_ct_attach = ip_conntrack_attach;
 
-	/* Set up fake conntrack:
-	    - to never be deleted, not in any hashes */
-	atomic_set(&ip_conntrack_untracked.ct_general.use, 1);
-	/*  - and look it like as a confirmed connection */
-	set_bit(IPS_CONFIRMED_BIT, &ip_conntrack_untracked.status);
-
 	return ret;
 
 err_free_conntrack_slab:
net/ipv4/netfilter/ip_nat_standalone.c +8 -0
Index: linux-2.6/net/ipv4/netfilter/ip_nat_standalone.c
===================================================================
@@ -81,6 +81,14 @@
 			return NF_DROP;
 
 	ct = ip_conntrack_get(*pskb, &ctinfo);
+	/* This is freak. The user is mixing non-tracked stuff together
+	 * with NAT, this is not possible since we lack of the necessary
+	 * information to perform any handling. Since we don't want to 
+	 * break stupid settings <SIGH>, just let this packet continue 
+	 * its travel through the network stack */
+	if (ctinfo == IP_CT_UNTRACKED)
+		return NF_ACCEPT;
+
 	/* Can't track?  It's not due to stress, or conntrack would
 	   have dropped it.  Hence it's the user's responsibilty to
 	   packet filter it out, or implement conntrack/NAT for that
net/ipv4/netfilter/ipt_NOTRACK.c +9 -4
Index: linux-2.6/net/ipv4/netfilter/ipt_NOTRACK.c
===================================================================
@@ -1,5 +1,8 @@
 /* This is a module which is used for setting up fake conntracks
  * on packets so that they are not seen by the conntrack/NAT code.
+ *
+ * 05/06/25: Added new untracked logic. 
+ * 	     Pablo Neira Ayuso <pablo at eurodev dot net>
  */
 #include <linux/module.h>
 #include <linux/skbuff.h>
@@ -16,16 +19,18 @@
        void *userinfo)
 {
 	/* Previously seen (loopback)? Ignore. */
-	if ((*pskb)->nfct != NULL)
+	if ((*pskb)->nfct != NULL || (*pskb)->nfctinfo == IP_CT_UNTRACKED)
 		return IPT_CONTINUE;
 
 	/* Attach fake conntrack entry. 
 	   If there is a real ct entry correspondig to this packet, 
 	   it'll hang aroun till timing out. We don't deal with it
 	   for performance reasons. JK */
-	(*pskb)->nfct = &ip_conntrack_untracked.ct_general;
-	(*pskb)->nfctinfo = IP_CT_NEW;
-	nf_conntrack_get((*pskb)->nfct);
+	   
+	/* This packet hasn't got any conntrack associated. To 
+	   explicitely mark it as untracked, set the nfctinfo flag. */
+	(*pskb)->nfct = NULL;
+	(*pskb)->nfctinfo = IP_CT_UNTRACKED;
 
 	return IPT_CONTINUE;
 }
#1 lore raw
Quoted 6 lines · Pablo Neira

The patch attached kills the fake conntrack and propose a new logic to explicitely set connection as untracked. We set nfct to NULL and use a new flag called IP_CT_UNTRACKED for nfctinfo. I've slightely tested it here and works fine.

Comments welcome.

What is the advantage of this patch? Its changing the assumption that skb->nfctinfo is only valid if skb->nfct is set, so you probably need to set nfctinfo to 0 in nf_reset as well. This alone is a reason not to do it IMO.

Regards
Patrick

#2 lore raw
Quoted 10 lines · Patrick McHardy
Quoted 6 lines · Pablo Neira

The patch attached kills the fake conntrack and propose a new logic to explicitely set connection as untracked. We set nfct to NULL and use a new flag called IP_CT_UNTRACKED for nfctinfo. I've slightely tested it here and works fine.

Comments welcome.

What is the advantage of this patch?

Whenever we work with conntracks, we always have to check if it's the fake or not previously. For example, a bad use of NOTRACK together with other targets, say ipt_CONNMARK, that doesn't currently check if it's working with the fake conntrack or not. Same thing for NAT handlings, I see that this can result in weirdness.

Setting nfct to NULL will make NAT targets, conntrack helpers and friends just ignore untracked packets, so we could forget about making sure that we aren't messing with the fake conntrack.

Quoted 4 lines

Its changing the assumption
that skb->nfctinfo is only valid if skb->nfct is set, so you probably need to set nfctinfo to 0 in nf_reset as well. This alone is a reason not to do it IMO.

why need to set nfctinfo to 0 in nf_reset? since skb->nfct is NULL nf_reset shouldn't be ever called. With the logic I'm proposing, this packet will be handle just like invalid ones, so it will be just ignored.

--
Pablo

#3 lore raw
Quoted 1 line · Pablo Neira

why need to set nfctinfo to 0 in nf_reset? since skb->nfct is NULL nf_reset shouldn't be ever called.

Well, this isn't true at all. nf_reset can be called even if skb->nfct is NULL.

Quoted 1 line

With the logic I'm proposing, this packet will be handle just like invalid ones, so it will be just ignored.

But I still support this this above, making untracked conntracks look ignore can simplify the logic AFAICS.

--
Pablo

#4 lore raw
In reply to Pablo Neira
Quoted 10 lines · Pablo Neira
Quoted 1 line · Patrick McHardy

What is the advantage of this patch?

Whenever we work with conntracks, we always have to check if it's the fake or not previously. For example, a bad use of NOTRACK together with other targets, say ipt_CONNMARK, that doesn't currently check if it's working with the fake conntrack or not. Same thing for NAT handlings, I see that this can result in weirdness.

Quoted 3 lines

Setting nfct to NULL will make NAT targets, conntrack helpers and friends just ignore untracked packets, so we could forget about making sure that we aren't messing with the fake conntrack.

Sounds reasonable - but couldn't we simply change ip_conntrack_get to return NULL for untracked connections?

Quoted 8 lines
Quoted 4 lines

Its changing the assumption
that skb->nfctinfo is only valid if skb->nfct is set, so you probably need to set nfctinfo to 0 in nf_reset as well. This alone is a reason not to do it IMO.

why need to set nfctinfo to 0 in nf_reset? since skb->nfct is NULL nf_reset shouldn't be ever called. With the logic I'm proposing, this packet will be handle just like invalid ones, so it will be just ignored.

nf_reset is called independant of the contents of skb->nfct. nfctinfo needs to be set to 0 so a packet isn't untracked afterwards anymore. Think of tunnel devices.

Regards
Patrick

#5 lore raw
Quoted 13 lines · Patrick McHardy
Quoted 11 lines · Pablo Neira
Quoted 1 line · Patrick McHardy

What is the advantage of this patch?

Whenever we work with conntracks, we always have to check if it's the fake or not previously. For example, a bad use of NOTRACK together with other targets, say ipt_CONNMARK, that doesn't currently check if it's working with the fake conntrack or not. Same thing for NAT handlings, I see that this can result in weirdness.

Quoted 7 lines
Quoted 3 lines

Setting nfct to NULL will make NAT targets, conntrack helpers and friends just ignore untracked packets, so we could forget about making sure that we aren't messing with the fake conntrack.

Sounds reasonable - but couldn't we simply change ip_conntrack_get to return NULL for untracked connections?

Then I won't be able to differenciate between invalid and untracked connections. So I'd need to return some extra info via ctinfo to achieve it, something in ip_conntrack_get like:

if (ct == &ip_conntrack_untracked)
ctinfo = IP_CT_UNTRACKED

Well, but we'll still need the fake conntrack.

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

Its changing the assumption
that skb->nfctinfo is only valid if skb->nfct is set, so you probably need to set nfctinfo to 0 in nf_reset as well. This alone is a reason not to do it IMO.

Quoted 8 lines
Quoted 3 lines

why need to set nfctinfo to 0 in nf_reset? since skb->nfct is NULL nf_reset shouldn't be ever called. With the logic I'm proposing, this packet will be handle just like invalid ones, so it will be just ignored.

nf_reset is called independant of the contents of skb->nfct. nfctinfo needs to be set to 0 so a packet isn't untracked afterwards anymore. Think of tunnel devices.

OK. Hm, do you still consider that bad adding a line to nf_reset to reset skb->nfctinfo? This is the only drawback I see at the moment.

--
Pablo

#6 lore raw
Quoted 6 lines · Pablo Neira
Quoted 3 lines

nf_reset is called independant of the contents of skb->nfct. nfctinfo needs to be set to 0 so a packet isn't untracked afterwards anymore. Think of tunnel devices.

OK. Hm, do you still consider that bad adding a line to nf_reset to reset skb->nfctinfo? This is the only drawback I see at the moment.

Its not about the line, I'm not sure about changing the fact that nfctinfo is only valid if nfct != NULL. Its too hot to think right now, please give me until tommorrow :)

Regards
Patrick

#7 lore raw
Quoted 11 lines · Patrick McHardy
Quoted 6 lines · Pablo Neira
Quoted 3 lines

nf_reset is called independant of the contents of skb->nfct. nfctinfo needs to be set to 0 so a packet isn't untracked afterwards anymore. Think of tunnel devices.

OK. Hm, do you still consider that bad adding a line to nf_reset to reset skb->nfctinfo? This is the only drawback I see at the moment.

Its not about the line, I'm not sure about changing the fact that nfctinfo is only valid if nfct != NULL. Its too hot to think right now, please give me until tommorrow :)

Looking through patchwork.netfilter.org, this thread never came to a conclusion. Any news on it?

Signature · 5 lines
- Harald Welte <[email protected]>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie