kill the fake conntrack
Subsystems (2)
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 ; }
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_UNTRACKEDfor 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
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_UNTRACKEDfor 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
thatskb->nfctinfois only valid ifskb->nfctis set, so you probably need to set nfctinfo to 0 innf_resetas 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
Quoted 1 line · Pablo Neira
why need to set nfctinfo to 0 in
nf_reset? sinceskb->nfctis NULLnf_resetshouldn'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
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
thatskb->nfctinfois only valid ifskb->nfctis set, so you probably need to set nfctinfo to 0 innf_resetas well. This alone is a reason not to do it IMO.why need to set nfctinfo to 0 in
nf_reset? sinceskb->nfctis NULLnf_resetshouldn'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
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_getto 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
thatskb->nfctinfois only valid ifskb->nfctis set, so you probably need to set nfctinfo to 0 innf_resetas 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? sinceskb->nfctis NULLnf_resetshouldn'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_resetis called independant of the contents ofskb->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
Quoted 6 lines · Pablo Neira
Quoted 3 lines
nf_resetis called independant of the contents ofskb->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_resetto resetskb->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
Quoted 11 lines · Patrick McHardy
Quoted 6 lines · Pablo Neira
Quoted 3 lines
nf_resetis called independant of the contents ofskb->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_resetto resetskb->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