Mailing List Archive

On Aug 27 2007 14:08, Sebastian Classen wrote:
>
>The ROUTE targe seems to be finaly gone from pom-ng and we need a
>solution for the --tee function. So I deciced to use some parts from the
>ROUTE target, adaped the code to the current kernel and implmemented a
>new taget called TEE. The only function is packet duplication and those
>duplicate packets are routed the the given gateway (--gw option).
>
>We use this as a replacement for netwrk traffic taps, the syntax is:
>itpables -t mangle -A PREROUTING -p tcp --sport <some-port> -j TEE --gw
><IP.of.mirror.box>
>
>Is there any chance getting this into pom-ng?

Here is an updated version first that uses xt:

http://dev.computergmbh.de/wsvn/misc_kernel/xt_TEE/

it's not entirely POM-conformant, since, well, if a module is good
enough to go to POM, it is likely good enough to go into mainline too.


A few questions:

* Is --gw 0.0.0.0 even supported/meaningful? It seems not, so some
checks could be removed.
* Your route_gw() function returns something, but you do not really use the
result. Is this intended?




Jan
--

Am Montag, den 27.08.2007, 16:21 +0200 schrieb Jan Engelhardt:
> On Aug 27 2007 14:08, Sebastian Classen wrote:
> >
> >The ROUTE targe seems to be finaly gone from pom-ng and we need a
> >solution for the --tee function. So I deciced to use some parts from the
> >ROUTE target, adaped the code to the current kernel and implmemented a
> >new taget called TEE. The only function is packet duplication and those
> >duplicate packets are routed the the given gateway (--gw option).
> >
> >We use this as a replacement for netwrk traffic taps, the syntax is:
> >itpables -t mangle -A PREROUTING -p tcp --sport <some-port> -j TEE --gw
> ><IP.of.mirror.box>
> >
> >Is there any chance getting this into pom-ng?
>
> Here is an updated version first that uses xt:
>
> http://dev.computergmbh.de/wsvn/misc_kernel/xt_TEE/

Thanks for this one, will have a closer look at it tomorow ;)

> it's not entirely POM-conformant, since, well, if a module is good
> enough to go to POM, it is likely good enough to go into mainline too.
>
>
> A few questions:
>
> * Is --gw 0.0.0.0 even supported/meaningful? It seems not, so some
> checks could be removed.

No, theres no special meaning. I don't know which checks u mean.

> * Your route_gw() function returns something, but you do not really use the
> result. Is this intended?

You are right, the value is not used anywhere, that's a left over from
previous ROUTE target. Also the comment above route() does mention a no
longer used return code.

Greets
Sebastian.

--
Sebastian Claßen
Postmaster
----------------------------------------------------------------------
Telefon: + 49 (0) 211 53087 522
Telefax: + 49 (0) 211 5381573
E-Mail: sebastian.classen@freenet.ag
Website: www.freenet.de; www.mobilcom.de
----------------------------------------------------------------------
freenet AG
Willstätterstr. 13
40549 Düsseldorf
----------------------------------------------------------------------
Vorsitzender des Aufsichtsrates: Prof. Dr. Helmut Thoma
Vorstand: Eckhard Spoerr (Vors.), Axel Krieger, Stephan Esch, Eric
Berger
Sitz: Büdelsdorf
Amtsgericht Kiel HRB 7306 KI

Sebastian Claßen wrote:
> Am Montag, den 27.08.2007, 16:21 +0200 schrieb Jan Engelhardt:
>> On Aug 27 2007 14:08, Sebastian Classen wrote:
>>> The ROUTE targe seems to be finaly gone from pom-ng and we need a
>>> solution for the --tee function. So I deciced to use some parts from the
>>> ROUTE target, adaped the code to the current kernel and implmemented a
>>> new taget called TEE. The only function is packet duplication and those
>>> duplicate packets are routed the the given gateway (--gw option).
>>>
>>> We use this as a replacement for netwrk traffic taps, the syntax is:
>>> itpables -t mangle -A PREROUTING -p tcp --sport <some-port> -j TEE --gw
>>> <IP.of.mirror.box>
>>>
>>> Is there any chance getting this into pom-ng?
>> Here is an updated version first that uses xt:
>>
>> http://dev.computergmbh.de/wsvn/misc_kernel/xt_TEE/


Please don't post gzipped patches (Sebastian) or links if the
patch isn't excessively large. I don't look at these things,
and I suspect a lot of others neither.

On Aug 27 2007 19:55, Sebastian Claßen wrote:
>>
>> * Is --gw 0.0.0.0 even supported/meaningful? It seems not, so some
>> checks could be removed.
>
>No, theres no special meaning. I don't know which checks u mean.

if (tee_info->gw)
route_gw(...)
else
DEBUGP(...)

If gw is not allowed to be zero, then the else case is redundant.
Of course, gw=0 is valid I think, meaning the default gateway.


Jan
--

On Aug 29 2007 21:03, Patrick McHardy wrote:
> Please don't post gzipped patches (Sebastian) or links if the
> patch isn't excessively large. I don't look at these things,
> and I suspect a lot of others neither.

I hope you don't require unidiff patches in this stage. It's just
additionss anyway. Of course, if you'd like one, I create one,
for a git tree of your choice. Posted code below is for 2.6.22.

>>> Makefile.ladd <<<
obj-$(CONFIG_NETFILTER_XT_TARGET_TEE) += xt_TEE.o

>>> Kconfig.ladd <<<
config NETFILTER_XT_TARGET_TEE
tristate '"TEE" target support'
depends on NETFILTER_XTABLES && IP_NF_MANGLE
---help---
This option adds a "TEE" target, which enables you to duplicate
packets and route those duplicates to a different gateway.

The target has to be used inside the mangle table.

If you want to compile it as a module, say M here and read
Documentation/modules.txt. The module will be called xt_TEE.ko.
If unsure, say N.

>>> xt_TEE.h <<<
/* Header file for iptables ipt_TEE target
*
* (C) 2006 by Sebastian Classen <sebastian.classen@freenet.ag>
* based on ipt_ROUTE.h from Cédric de Launois <delaunois@info.ucl.ac.be>
*
* This software is distributed under GNU GPL v2, 1991
*/
#ifndef _XT_TEE_TARGET_H
#define _XT_TEE_TARGET_H

struct xt_TEE_info {
u_int32_t gw; /* IP address of gateway */
};

#endif /* _XT_TEE_TARGET_H */

>>> xt_TEE.c <<<
/*
* This implements the TEE target.
*
* Copyright (C) 2007 Sebastian.Classen <sebastian.classen@freenet.de>
* Jan Engelhardt <jengelh@computergmbh.de>, 2007
* based on ipt_ROUTE.c from Cédric de Launois <delaunois@info.ucl.ac.be>
*
* This software is distributed under GNU GPL v2, 1991
*/
#include <linux/ip.h>
#include <linux/module.h>
#include <linux/netfilter/x_tables.h>
#include <linux/route.h>
#include <linux/skbuff.h>
#include <net/checksum.h>
#include <net/icmp.h>
#include <net/ip.h>
#include <net/netfilter/nf_conntrack.h>
#include <net/route.h>
#include "xt_TEE.h"

static struct nf_conn tee_track;

/*
* Try to route the packet according to the routing keys specified in
* route_info. Keys are :
* - ifindex :
* 0 if no oif preferred,
* otherwise set to the index of the desired oif
* - route_info->gw :
* 0 if no gateway specified,
* otherwise set to the next host to which the pkt must be routed
* If success, skb->dev is the output device to which the packet must
* be sent and skb->dst is not NULL
*
* RETURN: -1 if an error occured
* 1 if the packet was succesfully routed to the
* destination desired
* 0 if the kernel routing table could not route the packet
* according to the keys specified
*/
static int route(struct sk_buff *skb,
const struct xt_TEE_info *info)
{
int err;
struct rtable *rt;
struct iphdr *iph = ip_hdr(skb);
struct flowi fl = {
.oif = 0,
.nl_u = {
.ip4_u = {
.daddr = iph->daddr,
.saddr = 0,
.tos = RT_TOS(iph->tos),
.scope = RT_SCOPE_UNIVERSE,
}
}
};

/* The destination address may be overloaded by the target */
if (info->gw != 0)
fl.fl4_dst = info->gw;

/* Trying to route the packet using the standard routing table. */
if ((err = ip_route_output_key(&rt, &fl)) != 0) {
if (net_ratelimit())
pr_debug(KBUILD_MODNAME
"could not route pkt (err: %d)", err);
return -1;
}

/* Drop old route. */
dst_release(skb->dst);
skb->dst = NULL;

/* Success if no oif specified or if the oif correspond to the
* one desired */
/* SC: allways the case, because we have no oif. */
skb->dst = &rt->u.dst;
skb->dev = skb->dst->dev;
skb->protocol = htons(ETH_P_IP);
return 1;
}

/* Stolen from ip_finish_output2
* PRE : skb->dev is set to the device we are leaving by
* skb->dst is not NULL
* POST: the packet is sent with the link layer header pushed
* the packet is destroyed
*/
static void ip_direct_send(struct sk_buff *skb)
{
struct dst_entry *dst = skb->dst;
struct net_device *dev = dst->dev;
int hh_len = LL_RESERVED_SPACE(dev);

/* Be paranoid, rather than too clever. */
if (unlikely(skb_headroom(skb) < hh_len && dev->hard_header)) {
struct sk_buff *skb2;

skb2 = skb_realloc_headroom(skb, LL_RESERVED_SPACE(dev));
if (skb2 == NULL) {
kfree_skb(skb);
return;
}
if (skb->sk)
skb_set_owner_w(skb2, skb->sk);
kfree_skb(skb);
skb = skb2;
}

if (dst->hh) {
neigh_hh_output(dst->hh, skb);
} else if (dst->neighbour) {
dst->neighbour->output(skb);
} else {
if (net_ratelimit())
pr_debug(KBUILD_MODNAME "no hdr & no neighbour cache!\n");
kfree_skb(skb);
}
}

static inline void route_gw(const struct xt_TEE_info *info, struct sk_buff *skb)
{
if (route(skb, info) != 1)
return;

ip_direct_send(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
xt_TEE_target(struct sk_buff **pskb, const struct net_device *in,
const struct net_device *out, unsigned int hooknum,
const struct xt_target *target, const void *targinfo)
{
const struct xt_TEE_info *info = targinfo;
struct sk_buff *skb = *pskb;

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;
}

/*
* If we are at PREROUTING or INPUT hook,
* the TTL is not decreased by the IP stack
*/
if (hooknum == NF_IP_PRE_ROUTING || hooknum == NF_IP_LOCAL_IN) {
struct iphdr *iph = ip_hdr(skb);

if (iph->ttl <= 1) {
struct rtable *rt;
struct flowi fl = {
.oif = 0,
.nl_u = {
.ip4_u = {
.daddr = iph->daddr,
.saddr = iph->saddr,
.tos = RT_TOS(iph->tos),
.scope = ((iph->tos & RTO_ONLINK) ?
RT_SCOPE_LINK :
RT_SCOPE_UNIVERSE)
}
}
};

if (ip_route_output_key(&rt, &fl))
return NF_DROP;

if (skb->dev == rt->u.dst.dev) {
/* Drop old route */
dst_release(skb->dst);
skb->dst = &rt->u.dst;

/*
* this will traverse the normal stack and
* thus call conntrack on the ICMP packet
*/
icmp_send(skb, ICMP_TIME_EXCEEDED,
ICMP_EXC_TTL, 0);
}

return NF_DROP;
}

/*
* If we are at INPUT the checksum must be recalculated since
* the length could change as the result of a defragmentation.
*/
if (hooknum == NF_IP_LOCAL_IN) {
--iph->ttl;
iph->check = 0;
iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
} else {
ip_decrease_ttl(iph);
}
}

/*
* Copy the *pskb, 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 independantly delivered to the TEE
* --gw.
*/
skb = skb_copy(*pskb, GFP_ATOMIC);
if (skb == NULL) {
if (net_ratelimit())
pr_debug(KBUILD_MODNAME "copy failed!\n");
return XT_CONTINUE;
}

/*
* 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.
*/
nf_conntrack_put(skb->nfct);
skb->nfct = &tee_track.ct_general;
skb->nfctinfo = IP_CT_NEW;
nf_conntrack_get(skb->nfct);

if (info->gw != 0)
route_gw(info, skb);
else if (net_ratelimit())
pr_debug(KBUILD_MODNAME "no parameter!\n");

return XT_CONTINUE;
}

static struct xt_target xt_TEE_reg __read_mostly = {
.name = "TEE",
.family = AF_INET,
.table = "mangle",
.hooks = (1 << NF_IP_PRE_ROUTING) | (1 << NF_IP_LOCAL_IN) |
(1 << NF_IP_FORWARD) | (1 << NF_IP_LOCAL_OUT) |
(1 << NF_IP_POST_ROUTING),
.target = xt_TEE_target,
.targetsize = sizeof(struct xt_TEE_info),
.me = THIS_MODULE,
};

static int __init xt_TEE_init(void)
{
/*
* Set up fake conntrack (stolen from raw.patch):
* - to never be deleted, not in any hashes
*/
atomic_set(&tee_track.ct_general.use, 1);

/* - and look it like as a confirmed connection */
set_bit(IPS_CONFIRMED_BIT, &tee_track.status);

/* Initialize fake conntrack so that NAT will skip it */
tee_track.status |= IPS_NAT_DONE_MASK;

return xt_register_target(&xt_TEE_reg);
}

static void __exit xt_TEE_exit(void)
{
xt_unregister_target(&xt_TEE_reg);
/* SC: shoud not we cleanup tee_track here? */
}

module_init(xt_TEE_init);
module_exit(xt_TEE_exit);
MODULE_AUTHOR("Sebastian Classen <sebastian.classen@freenet.ag>, Jan Engelhardt <jengelh@computergmbh.de>");
MODULE_DESCRIPTION("netfilter TEE target module");
MODULE_LICENSE("GPL");

Jan Engelhardt wrote:
> On Aug 29 2007 21:03, Patrick McHardy wrote:
>> Please don't post gzipped patches (Sebastian) or links if the
>> patch isn't excessively large. I don't look at these things,
>> and I suspect a lot of others neither.
>
> I hope you don't require unidiff patches in this stage. It's just
> additionss anyway. Of course, if you'd like one, I create one,
> for a git tree of your choice. Posted code below is for 2.6.22.


Thanks, just the file is fine.

> * RETURN: -1 if an error occured
> * 1 if the packet was succesfully routed to the
> * destination desired
> * 0 if the kernel routing table could not route the packet
> * according to the keys specified
> */
> static int route(struct sk_buff *skb,
> const struct xt_TEE_info *info)
...
> /* Trying to route the packet using the standard routing table. */
> if ((err = ip_route_output_key(&rt, &fl)) != 0) {
> if (net_ratelimit())
> pr_debug(KBUILD_MODNAME
> "could not route pkt (err: %d)", err);
> return -1;
> }

Comment does not match behaviour. Suggest to change comment, and make
function return 0/1 only.

> static inline void route_gw(const struct xt_TEE_info *info, struct sk_buff *skb)
> {
> if (route(skb, info) != 1)
> return;
> ip_direct_send(skb);
> }

Too small, and only called once, to warrant being a function.

> /*
> * If we are at PREROUTING or INPUT hook,
> * the TTL is not decreased by the IP stack
> */
> if (hooknum == NF_IP_PRE_ROUTING || hooknum == NF_IP_LOCAL_IN) {
...
> if (iph->ttl <= 1) {
...

I believe this case (the whole synthesizing an ICMP_TIME_EXCEEDED) is not
neccessary for TEE.

The code is working on the original skb. With ROUTE, the logic was that
the skb would be dropped soon due to exceeding TTL, and if we reroute
and send directly now, that wouldn't happen.

With TEE (as well as the previous ROUTE --tee option), the original
packet is not rerouted, and ICMP_TIME_EXCEEDED should be generated
for it as usual. Right?

That would leave two questions:

Should we skb_copy+route for the above quoted conditions, i.e. hook
PRE_ROUTING/LOCAL_IN and skb->ttl <= 1, although the tee'd packet will
be dropped soon?

And if we decide to tee the packet regardless of the ttl condition,
should the ttl be decremented after skb_copy on the teed packet,
or not?

My gut feeling is to remove all ttl handling, and let the next hop of
the teed packet handle ttl as if it were the original recipient,
instead of our tee pot.

best regards
Patrick

Am Donnerstag, den 30.08.2007, 09:00 +0200 schrieb Patrick Schaaf:
> > * RETURN: -1 if an error occured
> > * 1 if the packet was succesfully routed to the
> > * destination desired
> > * 0 if the kernel routing table could not route the packet
> > * according to the keys specified
> > */
> > static int route(struct sk_buff *skb,
> > const struct xt_TEE_info *info)
> ...
> > /* Trying to route the packet using the standard routing table. */
> > if ((err = ip_route_output_key(&rt, &fl)) != 0) {
> > if (net_ratelimit())
> > pr_debug(KBUILD_MODNAME
> > "could not route pkt (err: %d)", err);
> > return -1;
> > }
>
> Comment does not match behaviour. Suggest to change comment, and make
> function return 0/1 only.

OK, fixed.

> > static inline void route_gw(const struct xt_TEE_info *info, struct sk_buff *skb)
> > {
> > if (route(skb, info) != 1)
> > return;
> > ip_direct_send(skb);
> > }
>
> Too small, and only called once, to warrant being a function.

Function removed.

> > /*
> > * If we are at PREROUTING or INPUT hook,
> > * the TTL is not decreased by the IP stack
> > */
> > if (hooknum == NF_IP_PRE_ROUTING || hooknum == NF_IP_LOCAL_IN) {
> ...
> > if (iph->ttl <= 1) {
> ...
>
> I believe this case (the whole synthesizing an ICMP_TIME_EXCEEDED) is not
> neccessary for TEE.
>
> The code is working on the original skb. With ROUTE, the logic was that
> the skb would be dropped soon due to exceeding TTL, and if we reroute
> and send directly now, that wouldn't happen.
>
> With TEE (as well as the previous ROUTE --tee option), the original
> packet is not rerouted, and ICMP_TIME_EXCEEDED should be generated
> for it as usual. Right?
>
> That would leave two questions:
>
> Should we skb_copy+route for the above quoted conditions, i.e. hook
> PRE_ROUTING/LOCAL_IN and skb->ttl <= 1, although the tee'd packet will
> be dropped soon?
>
> And if we decide to tee the packet regardless of the ttl condition,
> should the ttl be decremented after skb_copy on the teed packet,
> or not?
>
> My gut feeling is to remove all ttl handling, and let the next hop of
> the teed packet handle ttl as if it were the original recipient,
> instead of our tee pot.
>

I would also suggest to remove TTL handling completly and already did
so. Find the new xt_TEE.c attached.

@Jan: Could you please add the new version to your SVN repository.
Thanks.

Greets
Sebastian.


--
Mit freundlichen Grüßen / Yours sincerely

Sebastian Claßen
Postmaster
----------------------------------------------------------------------
Telefon: + 49 (0) 211 53087 522
Telefax: + 49 (0) 211 5381573
E-Mail: sebastian.classen@freenet.ag
Website: www.freenet.de; www.mobilcom.de
----------------------------------------------------------------------
freenet AG
Willstätterstr. 13
40549 Düsseldorf
----------------------------------------------------------------------
Vorsitzender des Aufsichtsrates: Prof. Dr. Helmut Thoma
Vorstand: Eckhard Spoerr (Vors.), Axel Krieger, Stephan Esch, Eric Berger
Sitz: Büdelsdorf
Amtsgericht Kiel HRB 7306 KI

Am Donnerstag, den 13.09.2007, 09:37 +0200 schrieb Sebastian Classen:
> I would also suggest to remove TTL handling completly and already did
> so. Find the new xt_TEE.c attached.
>
> @Jan: Could you please add the new version to your SVN repository.
> Thanks.
>

Sorry, I forgot a pair of braces. Find corrected version attached.

Sebastian.

--
Mit freundlichen Grüßen / Yours sincerely

Sebastian Claßen
Postmaster
----------------------------------------------------------------------
Telefon: + 49 (0) 211 53087 522
Telefax: + 49 (0) 211 5381573
E-Mail: sebastian.classen@freenet.ag
Website: www.freenet.de; www.mobilcom.de
----------------------------------------------------------------------
freenet AG
Willstätterstr. 13
40549 Düsseldorf
----------------------------------------------------------------------
Vorsitzender des Aufsichtsrates: Prof. Dr. Helmut Thoma
Vorstand: Eckhard Spoerr (Vors.), Axel Krieger, Stephan Esch, Eric Berger
Sitz: Büdelsdorf
Amtsgericht Kiel HRB 7306 KI