From 56535551b34c75d16e7bea2f0adf8cdef225797d Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Mon, 22 Mar 2010 23:29:20 +0100 Subject: [PATCH 01/10] xt_TEE: use ip_send_check instead of open-coded logic --- extensions/xt_TEE.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index ce25663..ad90cfe 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -166,11 +166,8 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) * If we are in INPUT, the checksum must be recalculated since * the length could have changed as a result of defragmentation. */ - if (par->hooknum == NF_INET_LOCAL_IN) { - struct iphdr *iph = ip_hdr(skb); - iph->check = 0; - iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl); - } + if (par->hooknum == NF_INET_LOCAL_IN) + ip_send_check(ip_hdr(skb)); /* * Copy the skb, and route the copy. Will later return %XT_CONTINUE for From 346fc1a376297c2c60b65f94ce56725b9851331c Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Mon, 22 Mar 2010 23:16:51 +0100 Subject: [PATCH 02/10] xt_TEE: do rechecksumming in PREROUTING too --- doc/changelog.txt | 1 + extensions/xt_TEE.c | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/changelog.txt b/doc/changelog.txt index d109979..cca26fc 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -1,6 +1,7 @@ HEAD ==== +- TEE: do rechecksumming in PREROUTING too Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index ad90cfe..74ac709 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -163,10 +163,11 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) skb = *pskb; /* - * If we are in INPUT, the checksum must be recalculated since - * the length could have changed as a result of defragmentation. + * If we are in PREROUTING/INPUT, the checksum must be recalculated + * since the length could have changed as a result of defragmentation. */ - if (par->hooknum == NF_INET_LOCAL_IN) + if (par->hooknum == NF_INET_PRE_ROUTING || + par->hooknum == NF_INET_LOCAL_IN) ip_send_check(ip_hdr(skb)); /* From 937571bb9db8ba4e448d7aee33624869ae5cad5a Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Mon, 22 Mar 2010 23:15:42 +0100 Subject: [PATCH 03/10] xt_TEE: decrease TTL on cloned packet --- doc/changelog.txt | 1 + extensions/xt_TEE.c | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/doc/changelog.txt b/doc/changelog.txt index cca26fc..1c630d3 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -2,6 +2,7 @@ HEAD ==== - TEE: do rechecksumming in PREROUTING too +- TEE: decrease TTL on cloned packet Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index 74ac709..95c41f7 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -165,11 +165,17 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) /* * If we are in PREROUTING/INPUT, the checksum must be recalculated * since the length could have changed as a result of defragmentation. + * + * We also decrease the TTL to mitigate potential TEE loops + * between two hosts. */ if (par->hooknum == NF_INET_PRE_ROUTING || - par->hooknum == NF_INET_LOCAL_IN) - ip_send_check(ip_hdr(skb)); + par->hooknum == NF_INET_LOCAL_IN) { + struct iphdr *iph = ip_hdr(skb); + --iph->ttl; + ip_send_check(iph); + } /* * Copy the skb, and route the copy. Will later return %XT_CONTINUE for * the original skb, which should continue on its way as if nothing has @@ -276,6 +282,11 @@ tee_tg6(struct sk_buff **pskb, const struct xt_target_param *par) skb->nfctinfo = IP_CT_NEW; nf_conntrack_get(skb->nfct); #endif + if (par->hooknum == NF_INET_PRE_ROUTING || + par->hooknum == NF_INET_LOCAL_IN) { + struct ipv6hdr *iph = ipv6_hdr(skb); + --iph->hop_limit; + } if (tee_tg_route6(skb, info)) tee_tg_send(skb); From fd19a40dbece03e4fe477a9053312d10d8a86069 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Fri, 26 Mar 2010 23:28:13 +0100 Subject: [PATCH 04/10] xt_TEE: avoid making original packet writable There is not any real need to make the original packet writable, as it is not going to be modified anyway. --- extensions/xt_TEE.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index 95c41f7..b6aa69a 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -157,11 +157,15 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) return NF_DROP; } #endif - - if (!skb_make_writable(pskb, sizeof(struct iphdr))) - return NF_DROP; - skb = *pskb; - + /* + * Copy the skb, and route the copy. Will later return %XT_CONTINUE for + * the original skb, which should continue on its way as if nothing has + * happened. The copy should be independently delivered to the TEE + * --gateway. + */ + skb = skb_copy(skb, GFP_ATOMIC); + if (skb == NULL) + return XT_CONTINUE; /* * If we are in PREROUTING/INPUT, the checksum must be recalculated * since the length could have changed as a result of defragmentation. @@ -176,16 +180,6 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) --iph->ttl; ip_send_check(iph); } - /* - * Copy the skb, and route the copy. Will later return %XT_CONTINUE for - * the original skb, which should continue on its way as if nothing has - * happened. The copy should be independently delivered to the TEE - * --gateway. - */ - skb = skb_copy(skb, GFP_ATOMIC); - if (skb == NULL) - return XT_CONTINUE; - #ifdef WITH_CONNTRACK /* * Tell conntrack to forget this packet since it may get confused From ba356367182e055ffb1ba6c80651540f7dc7797e Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Fri, 26 Mar 2010 23:48:29 +0100 Subject: [PATCH 05/10] xt_TEE: set dont-fragment on cloned packets --- doc/changelog.txt | 1 + extensions/xt_TEE.c | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/doc/changelog.txt b/doc/changelog.txt index 1c630d3..a170667 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -3,6 +3,7 @@ HEAD ==== - TEE: do rechecksumming in PREROUTING too - TEE: decrease TTL on cloned packet +- TEE: set dont-fragment on cloned packets Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index b6aa69a..00cc3ad 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -145,6 +145,7 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) { const struct xt_tee_tginfo *info = par->targinfo; struct sk_buff *skb = *pskb; + struct iphdr *iph; #ifdef WITH_CONNTRACK if (skb->nfct == &tee_track.ct_general) { @@ -172,14 +173,17 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) * * We also decrease the TTL to mitigate potential TEE loops * between two hosts. + * + * Set %IP_DF so that the original source is notified of a potentially + * decreased MTU on the clone route. IPv6 does this too. */ + iph = ip_hdr(skb); + iph->frag_off |= htons(IP_DF); if (par->hooknum == NF_INET_PRE_ROUTING || - par->hooknum == NF_INET_LOCAL_IN) { - struct iphdr *iph = ip_hdr(skb); - + par->hooknum == NF_INET_LOCAL_IN) --iph->ttl; - ip_send_check(iph); - } + ip_send_check(iph); + #ifdef WITH_CONNTRACK /* * Tell conntrack to forget this packet since it may get confused From 7338a2a40005a5017fbe03674a6ce45e0c896e0e Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Sat, 27 Mar 2010 00:17:52 +0100 Subject: [PATCH 06/10] xt_TEE: free skb when route lookup failed --- doc/changelog.txt | 1 + extensions/xt_TEE.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/doc/changelog.txt b/doc/changelog.txt index a170667..f360f83 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -4,6 +4,7 @@ HEAD - TEE: do rechecksumming in PREROUTING too - TEE: decrease TTL on cloned packet - TEE: set dont-fragment on cloned packets +- TEE: free skb when route lookup failed Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index 00cc3ad..de624c1 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -76,6 +76,7 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info) if (net_ratelimit()) pr_debug(KBUILD_MODNAME ": could not route packet (%d)", err); + kfree_skb(skb); return false; } @@ -249,6 +250,7 @@ tee_tg_route6(struct sk_buff *skb, const struct xt_tee_tginfo *info) if (dst == NULL) { if (net_ratelimit()) printk(KERN_ERR "ip6_route_output failed for tee\n"); + kfree_skb(skb); return false; } From 295b6b6d73b1132f647f2ffcc73168d92f6aea06 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Sat, 27 Mar 2010 02:55:41 +0100 Subject: [PATCH 07/10] xt_TEE: do not limit use to mangle table --- doc/changelog.txt | 1 + extensions/xt_TEE.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/changelog.txt b/doc/changelog.txt index f360f83..9f40d1f 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -5,6 +5,7 @@ HEAD - TEE: decrease TTL on cloned packet - TEE: set dont-fragment on cloned packets - TEE: free skb when route lookup failed +- TEE: do not limit use to mangle table Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index de624c1..4614a48 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -308,7 +308,6 @@ static struct xt_target tee_tg_reg[] __read_mostly = { .name = "TEE", .revision = 0, .family = NFPROTO_IPV4, - .table = "mangle", .target = tee_tg4, .targetsize = sizeof(struct xt_tee_tginfo), .checkentry = tee_tg_check, @@ -319,7 +318,6 @@ static struct xt_target tee_tg_reg[] __read_mostly = { .name = "TEE", .revision = 0, .family = NFPROTO_IPV6, - .table = "mangle", .target = tee_tg6, .targetsize = sizeof(struct xt_tee_tginfo), .checkentry = tee_tg_check, From 987402dc612154c8efacca94900bd7a8b5c6f150 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Fri, 2 Apr 2010 16:59:13 +0200 Subject: [PATCH 08/10] xt_TEE: do not retain iif and mark on cloned packet Patrick McHardy explains in [1] that locally-generated packets (such as the clones xt_TEE will create) usually start with no iif and no mark value, and even if cloned packets are a little more special than locally-generated ones, let's do it that way. [1] http://marc.info/?l=netfilter-devel&m=127012289008156&w=2 --- doc/changelog.txt | 1 + extensions/xt_TEE.c | 13 ------------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/doc/changelog.txt b/doc/changelog.txt index 9f40d1f..556e4f0 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -6,6 +6,7 @@ HEAD - TEE: set dont-fragment on cloned packets - TEE: free skb when route lookup failed - TEE: do not limit use to mangle table +- TEE: do not retain iif and mark on cloned packet Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index 4614a48..af5173f 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -60,12 +60,6 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info) struct flowi fl; memset(&fl, 0, sizeof(fl)); - fl.iif = skb_ifindex(skb); -#if LINUX_VERSION_CODE <= KERNEL_VERSION(2, 6, 19) - fl.nl_u.ip4_u.fwmark = skb_nfmark(skb); -#else - fl.mark = skb_nfmark(skb); -#endif fl.nl_u.ip4_u.daddr = info->gw.ip; fl.nl_u.ip4_u.tos = RT_TOS(iph->tos); fl.nl_u.ip4_u.scope = RT_SCOPE_UNIVERSE; @@ -231,13 +225,6 @@ tee_tg_route6(struct sk_buff *skb, const struct xt_tee_tginfo *info) struct flowi fl; memset(&fl, 0, sizeof(fl)); - fl.iif = skb_ifindex(skb); - /* No mark in flowi before 2.6.19 */ -#if LINUX_VERSION_CODE == KERNEL_VERSION(2, 6, 19) - fl.nl_u.ip6_u.fwmark = skb_nfmark(skb); -#elif LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 20) - fl.mark = skb_nfmark(skb); -#endif fl.nl_u.ip6_u.daddr = info->gw.in6; fl.nl_u.ip6_u.flowlabel = ((iph->flow_lbl[0] & 0xF) << 16) | (iph->flow_lbl[1] << 8) | iph->flow_lbl[2]; From a17203e03681cb4878bf2aeae0d04526759b161d Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Fri, 2 Apr 2010 19:43:42 +0200 Subject: [PATCH 09/10] xt_TEE: remove old loop detection The loop detection does not work if the kernel is built without conntrack. In fact, since cloned packets are sent directly and do not pass through Xtables, there are no loops happening. --- extensions/xt_TEE.c | 35 ++++------------------------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index af5173f..b11dd1c 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -129,12 +129,6 @@ static void tee_tg_send(struct sk_buff *skb) } } -/* - * To detect and deter routed packet loopback when using the --tee option, we - * take a page out of the raw.patch book: on the copied skb, we set up a fake - * ->nfct entry, pointing to the local &route_tee_track. We skip routing - * packets when we see they already have that ->nfct. - */ static unsigned int tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) { @@ -142,17 +136,6 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) struct sk_buff *skb = *pskb; struct iphdr *iph; -#ifdef WITH_CONNTRACK - if (skb->nfct == &tee_track.ct_general) { - /* - * Loopback - a packet we already routed, is to be - * routed another time. Avoid that, now. - */ - if (net_ratelimit()) - pr_debug(KBUILD_MODNAME "loopback - DROP!\n"); - return NF_DROP; - } -#endif /* * Copy the skb, and route the copy. Will later return %XT_CONTINUE for * the original skb, which should continue on its way as if nothing has @@ -181,12 +164,9 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) #ifdef WITH_CONNTRACK /* - * Tell conntrack to forget this packet since it may get confused - * when a packet is leaving with dst address == our address. - * Good idea? Dunno. Need advice. - * - * NEW: mark the skb with our &tee_track, so we avoid looping - * on any already routed packet. + * Tell conntrack to forget this packet. It may have side effects to + * see the same packet twice, as for example, accounting the original + * connection for the cloned packet. */ nf_conntrack_put(skb->nfct); skb->nfct = &tee_track.ct_general; @@ -254,12 +234,6 @@ tee_tg6(struct sk_buff **pskb, const struct xt_target_param *par) const struct xt_tee_tginfo *info = par->targinfo; struct sk_buff *skb = *pskb; - /* Try silence. */ -#ifdef WITH_CONNTRACK - if (skb->nfct == &tee_track.ct_general) - return NF_DROP; -#endif - if ((skb = skb_copy(skb, GFP_ATOMIC)) == NULL) return XT_CONTINUE; @@ -317,8 +291,7 @@ static int __init tee_tg_init(void) { #ifdef WITH_CONNTRACK /* - * Set up fake conntrack (stolen from raw.patch): - * - to never be deleted, not in any hashes + * Set up fake conntrack - to never be deleted, not in any hashes */ atomic_set(&tee_track.ct_general.use, 1); From fb4c49d7945eb0ff4d0e14178a367fd17b36e121 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Mon, 5 Apr 2010 00:44:44 +0200 Subject: [PATCH 10/10] xt_TEE: new loop detection logic --- doc/changelog.txt | 1 + extensions/xt_TEE.c | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/doc/changelog.txt b/doc/changelog.txt index 556e4f0..56623ba 100644 --- a/doc/changelog.txt +++ b/doc/changelog.txt @@ -7,6 +7,7 @@ HEAD - TEE: free skb when route lookup failed - TEE: do not limit use to mangle table - TEE: do not retain iif and mark on cloned packet +- TEE: new loop detection logic Xtables-addons 1.24 (March 17 2010) diff --git a/extensions/xt_TEE.c b/extensions/xt_TEE.c index b11dd1c..d078d6e 100644 --- a/extensions/xt_TEE.c +++ b/extensions/xt_TEE.c @@ -33,6 +33,7 @@ static struct nf_conn tee_track; #include "compat_xtables.h" #include "xt_TEE.h" +static bool tee_active[NR_CPUS]; static const union nf_inet_addr tee_zero_address; /* @@ -135,7 +136,10 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) const struct xt_tee_tginfo *info = par->targinfo; struct sk_buff *skb = *pskb; struct iphdr *iph; + unsigned int cpu = smp_processor_id(); + if (tee_active[cpu]) + return XT_CONTINUE; /* * Copy the skb, and route the copy. Will later return %XT_CONTINUE for * the original skb, which should continue on its way as if nothing has @@ -190,9 +194,11 @@ tee_tg4(struct sk_buff **pskb, const struct xt_target_param *par) * Also on purpose, no fragmentation is done, to preserve the * packet as best as possible. */ - if (tee_tg_route4(skb, info)) + if (tee_tg_route4(skb, info)) { + tee_active[cpu] = true; tee_tg_send(skb); - + tee_active[cpu] = false; + } return XT_CONTINUE; } @@ -233,7 +239,10 @@ tee_tg6(struct sk_buff **pskb, const struct xt_target_param *par) { const struct xt_tee_tginfo *info = par->targinfo; struct sk_buff *skb = *pskb; + unsigned int cpu = smp_processor_id(); + if (tee_active[cpu]) + return XT_CONTINUE; if ((skb = skb_copy(skb, GFP_ATOMIC)) == NULL) return XT_CONTINUE; @@ -248,9 +257,11 @@ tee_tg6(struct sk_buff **pskb, const struct xt_target_param *par) struct ipv6hdr *iph = ipv6_hdr(skb); --iph->hop_limit; } - if (tee_tg_route6(skb, info)) + if (tee_tg_route6(skb, info)) { + tee_active[cpu] = true; tee_tg_send(skb); - + tee_active[cpu] = false; + } return XT_CONTINUE; } #endif /* WITH_IPV6 */