xt_TARPIT (was: ipt_account / iptables 1.3.8)

xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Jan Engelhardt]

Hi,


in http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/028366.html
there was talk about a revamped xt_TARPIT.

Patrick McHardy wrote:
>Shouldn't be much work, maybe I'll look into this after finishing
>my conntrack hash patches if no one beats me to it.

Any progress? Because tarpit is in my series [kernel patch tree] (after 
connlimit), and I'd hate to do double effort if you already have it.


Thanks,
	Jan
-- 

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Patrick McHardy]

Jan Engelhardt wrote:
> in http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/028366.html
> there was talk about a revamped xt_TARPIT.
> 
> Patrick McHardy wrote:
> 
>>Shouldn't be much work, maybe I'll look into this after finishing
>>my conntrack hash patches if no one beats me to it.
> 
> 
> Any progress? Because tarpit is in my series [kernel patch tree] (after 
> connlimit), and I'd hate to do double effort if you already have it.


No, I couldn't come up with a good way to remove the xrlim abuse yet.

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Jan Engelhardt]

On Jul 9 2007 15:37, Patrick McHardy wrote:
>> in http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/028366.html
>> there was talk about a revamped xt_TARPIT.
>> 
>> Patrick McHardy wrote:
>> 
>>>Shouldn't be much work, maybe I'll look into this after finishing
>>>my conntrack hash patches if no one beats me to it.
>> 
>> Any progress? Because tarpit is in my series [kernel patch tree] (after 
>> connlimit), and I'd hate to do double effort if you already have it.
>
>No, I couldn't come up with a good way to remove the xrlim abuse yet.
>
from net/ipv4/icmp.c:
 *      Check transmit rate limitation for given message.
 *      The rate information is held in the destination cache now.
 *      This function is generic and could be used for other purposes
 *      too.

I suppose "other purposes" could mean TCP here.. ;-')



	Jan
-- 

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jul 9 2007 15:37, Patrick McHardy wrote:
> 
>>>in http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/028366.html
>>>there was talk about a revamped xt_TARPIT.
>>>
>>>Patrick McHardy wrote:
>>>
>>>
>>>>Shouldn't be much work, maybe I'll look into this after finishing
>>>>my conntrack hash patches if no one beats me to it.
>>>
>>>Any progress? Because tarpit is in my series [kernel patch tree] (after 
>>>connlimit), and I'd hate to do double effort if you already have it.
>>
>>No, I couldn't come up with a good way to remove the xrlim abuse yet.
>>
> 
> from net/ipv4/icmp.c:
>  *      Check transmit rate limitation for given message.
>  *      The rate information is held in the destination cache now.
>  *      This function is generic and could be used for other purposes
>  *      too.
> 
> I suppose "other purposes" could mean TCP here.. ;-')


I don't think so. Sending fake packets from an iptables target
shouldn't affect the behaviour of the network stack wrt. ICMP
errors.

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Jan Engelhardt]

On Jul 9 2007 16:15, Patrick McHardy wrote:
>>>
>>>No, I couldn't come up with a good way to remove the xrlim abuse yet.
>>>

If you ask me, just remove the xrlim call, and replace it by
some sort of 'last' timestamp, e.g.

time_t last;

static int target()
{
	if (!otcph->syn && otcph->ack)
		if (!(last <= now() + 5))
			return; /* rate limit */
	last = now();
}

As far as I can see, that xrlim call is there for at least one case:

tarpit sends ACKs with window=0, but client ignores the RFC-given delay for
window=0 packets.


	Jan
-- 

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jul 9 2007 16:15, Patrick McHardy wrote:
> 
>>>>No, I couldn't come up with a good way to remove the xrlim abuse yet.
>>>>
> 
> 
> If you ask me, just remove the xrlim call, and replace it by
> some sort of 'last' timestamp, e.g.
> 
> time_t last;
> 
> static int target()
> {
> 	if (!otcph->syn && otcph->ack)
> 		if (!(last <= now() + 5))
> 			return; /* rate limit */
> 	last = now();
> }

The difference is that this is a global ratelimit, while xrlim_allow
is a per dst_entry ratelimit.

> As far as I can see, that xrlim call is there for at least one case:
> 
> tarpit sends ACKs with window=0, but client ignores the RFC-given delay for
> window=0 packets.


I think its more to prevent flooding your upstream, especially with
asynchronous bandwidth. Same reason you would usually rate-limit
the REJECT target on a DSL line.

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Jan Engelhardt]

On Jul 9 2007 17:04, Patrick McHardy wrote:
>
>The difference is that this is a global ratelimit, while xrlim_allow
>is a per dst_entry ratelimit.
>
>> As far as I can see, that xrlim call is there for at least one case:
>> 
>> tarpit sends ACKs with window=0, but client ignores the RFC-given delay for
>> window=0 packets.
>
>I think its more to prevent flooding your upstream, especially with
>asynchronous bandwidth. Same reason you would usually rate-limit
>the REJECT target on a DSL line.
>
REJECT is for rejecting. People who use it _do_ want that the RST/ICMP
is sent out immediately; otherwise they could just use DROP IMO.

A user can combine REJECT with hashlimit if he desires so.
I do not have any objection to remove all ratelimiting in TARPIT,
and instead put the user in charge of combining TARPIT with hashlimit.



	Jan
-- 

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jul 9 2007 17:04, Patrick McHardy wrote:
> 
>>The difference is that this is a global ratelimit, while xrlim_allow
>>is a per dst_entry ratelimit.
>>
>>
>>>As far as I can see, that xrlim call is there for at least one case:
>>>
>>>tarpit sends ACKs with window=0, but client ignores the RFC-given delay for
>>>window=0 packets.
>>
>>I think its more to prevent flooding your upstream, especially with
>>asynchronous bandwidth. Same reason you would usually rate-limit
>>the REJECT target on a DSL line.
>>
> 
> REJECT is for rejecting. People who use it _do_ want that the RST/ICMP
> is sent out immediately; otherwise they could just use DROP IMO.


Speaking as "people", no. I want to reject packets since its a lot
nicer than silently dropping, but I don't want anyone to be able
to flood my upstream by sending lots of packets down my a lot faster
downstream to closed ports. So I ratelimit REJECTs using the limit
match.

> A user can combine REJECT with hashlimit if he desires so.
> I do not have any objection to remove all ratelimiting in TARPIT,
> and instead put the user in charge of combining TARPIT with hashlimit.


Yes, thats what I would prefer as well, but currently it only
ratelimits replies to ACKs, not to SYNs, so this would change
how it works. If that doesn't matter, I'm all in favour of this
change.

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Jan Engelhardt]

On Jul 9 2007 17:16, Patrick McHardy wrote:
>> A user can combine REJECT with hashlimit if he desires so.
>> I do not have any objection to remove all ratelimiting in TARPIT,
>> and instead put the user in charge of combining TARPIT with hashlimit.
>
>Yes, thats what I would prefer as well, but currently it only
>ratelimits replies to ACKs, not to SYNs, so this would change
>how it works. If that doesn't matter, I'm all in favour of this
>change.
>
Since it is not in-tree, it is not as much a "standard" as if it was in,
so I'd assume you are free to do so. I'd speculate that everyone who did
not read the code does not even know about the ratelimiting (unless it is
mentioned in some textfile AND the user read that too).
In fact, I did not know about it until I started to make TARPIT compile
with 2.6.22...


	Jan
-- 

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Jan Engelhardt]

>On Sun, Jul 08, 2007 at 16:59:30 +0200, Jan Engelhardt wrote:
>> Hi,
>> 
>> 
>> in http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/028366.html
>> there was talk about a revamped xt_TARPIT.
>> 
>> Patrick McHardy wrote:
>> >Shouldn't be much work, maybe I'll look into this after finishing
>> >my conntrack hash patches if no one beats me to it.
>> 
>> Any progress? Because tarpit is in my series [kernel patch tree] (after 
>> connlimit), and I'd hate to do double effort if you already have it.
>

Release early, release often, ...
so here is xt_TARPIT that sits in my tree. It is not perfect, but seems 
to work in a quick test.
Comments please, thanks!


---
 drivers/char/random.c     |    1 
 net/netfilter/Kconfig     |   17 ++
 net/netfilter/Makefile    |    1 
 net/netfilter/xt_TARPIT.c |  280 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 299 insertions(+)

Index: linux-2.6.22/drivers/char/random.c
===================================================================
--- linux-2.6.22.orig/drivers/char/random.c
+++ linux-2.6.22/drivers/char/random.c
@@ -1556,6 +1556,7 @@ __u32 secure_tcp_sequence_number(__be32 
 #endif
 	return seq;
 }
+EXPORT_SYMBOL(secure_tcp_sequence_number);
 
 /* Generate secure starting point for ephemeral IPV4 transport port search */
 u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
Index: linux-2.6.22/net/netfilter/Kconfig
===================================================================
--- linux-2.6.22.orig/net/netfilter/Kconfig
+++ linux-2.6.22/net/netfilter/Kconfig
@@ -383,6 +383,23 @@ config NETFILTER_XT_TARGET_CONNSECMARK
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config NETFILTER_XT_TARGET_TARPIT
+	tristate '"TARPIT" target support'
+	depends on NETFILTER_XTABLES
+	---help---
+	  Adds a TARPIT target to iptables, which captures and holds
+	  incoming TCP connections using no local per-connection resources.
+	  Connections are accepted, but immediately switched to the persist
+	  state (0 byte window), in which the remote side stops sending data
+	  and asks to continue every 60-240 seconds. Attempts to close the
+	  connection are ignored, forcing the remote side to time out the
+	  connection in 12-24 minutes.
+
+	  This offers similar functionality to LaBrea
+	  <http://www.hackbusters.net/LaBrea/>, but does not require dedicated
+	  hardware or IPs. Any TCP port that you would normally DROP or REJECT
+	  can instead become a tarpit.
+
 config NETFILTER_XT_TARGET_TCPMSS
 	tristate '"TCPMSS" target support'
 	depends on NETFILTER_XTABLES && (IPV6 || IPV6=n)
Index: linux-2.6.22/net/netfilter/Makefile
===================================================================
--- linux-2.6.22.orig/net/netfilter/Makefile
+++ linux-2.6.22/net/netfilter/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_NETFILTER_XT_TARGET_NFLOG) 
 obj-$(CONFIG_NETFILTER_XT_TARGET_NOTRACK) += xt_NOTRACK.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_TRACE) += xt_TRACE.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_SECMARK) += xt_SECMARK.o
+obj-$(CONFIG_NETFILTER_XT_TARGET_TARPIT) += xt_TARPIT.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_TCPMSS) += xt_TCPMSS.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_TPROXY) += xt_TPROXY.o
 obj-$(CONFIG_NETFILTER_XT_TARGET_CONNSECMARK) += xt_CONNSECMARK.o
Index: linux-2.6.22/net/netfilter/xt_TARPIT.c
===================================================================
--- /dev/null
+++ linux-2.6.22/net/netfilter/xt_TARPIT.c
@@ -0,0 +1,280 @@
+/*
+ * Kernel module to capture and hold incoming TCP connections using
+ * no local per-connection resources.
+ *
+ * Based on ipt_REJECT.c and offering functionality similar to
+ * LaBrea <http://www.hackbusters.net/LaBrea/>.
+ *
+ * Copyright (c) 2002 Aaron Hopkins <tools@die.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Goal:
+ * - Allow incoming TCP connections to be established.
+ * - Passing data should result in the connection being switched to the
+ *   persist state (0 byte window), in which the remote side stops sending
+ *   data and asks to continue every 60 seconds.
+ * - Attempts to shut down the connection should be ignored completely, so
+ *   the remote side ends up having to time it out.
+ *
+ * This means:
+ * - Reply to TCP SYN,!ACK,!RST,!FIN with SYN-ACK, window 5 bytes
+ * - Reply to TCP SYN,ACK,!RST,!FIN with RST to prevent spoofing
+ * - Reply to TCP !SYN,!RST,!FIN with ACK, window 0 bytes, rate-limited
+ */
+
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <net/ip.h>
+#include <net/tcp.h>
+#include <net/icmp.h>
+struct in_device;
+#include <net/route.h>
+#include <linux/random.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+
+#if 0
+#define DEBUGP printk
+#else
+#define DEBUGP(format, args...)
+#endif
+
+/* Stolen from ip_finish_output2 */
+static int ip_direct_send(struct sk_buff *skb)
+{
+	struct dst_entry *dst = skb->dst;
+
+        if (dst->hh != NULL)
+		return neigh_hh_output(dst->hh, skb);
+	else if (dst->neighbour != NULL)
+		return dst->neighbour->output(skb);
+
+	if (net_ratelimit())
+		printk(KERN_DEBUG "TARPIT ip_direct_send: no header cache and no neighbor!\n");
+
+	kfree_skb(skb);
+	return -EINVAL;
+}
+
+
+/* Send reply */
+static void tarpit_tcp(const struct sk_buff *oskb, struct rtable *ort,
+                       unsigned int local)
+{
+	struct sk_buff *nskb;
+	struct rtable *nrt;
+	struct tcphdr *otcph, *ntcph;
+	struct flowi fl = {};
+	unsigned int otcplen;
+	u_int16_t tmp;
+
+	const struct iphdr *oiph = ip_hdr(oskb);
+	struct iphdr *niph;
+
+	/* A truncated TCP header is not going to be useful */
+	if (oskb->len < ip_hdrlen(oskb) + sizeof(struct tcphdr))
+		return;
+
+	otcph   = (void *)oiph + ip_hdrlen(oskb);
+	otcplen = oskb->len - ip_hdrlen(oskb);
+
+	/* No replies for RST or FIN */
+	if (otcph->rst || otcph->fin)
+		return;
+
+	/* No reply to !SYN,!ACK.  Rate-limit replies to !SYN,ACKs */
+	if (!otcph->syn && (!otcph->ack || !xrlim_allow(&ort->u.dst, 1*HZ)))
+		return;
+
+	/* Check checksum. */
+	if (tcp_v4_check(otcplen, oiph->saddr, oiph->daddr,
+	    csum_partial((char *)otcph, otcplen, 0)) != 0)
+		return;
+
+	/*
+	 * Copy skb (even if skb is about to be dropped, we cannot just
+	 * clone it because there may be other things, such as tcpdump,
+	 * interested in it)
+	 */
+	nskb = skb_copy(oskb, GFP_ATOMIC);
+	if (nskb == NULL)
+		return;
+
+	niph = ip_hdr(nskb);
+
+	/* This packet will not be the same as the other: clear nf fields */
+	nf_conntrack_put(nskb->nfct);
+	nskb->nfct = NULL;
+#ifdef CONFIG_NETFILTER_DEBUG
+	nskb->nf_debug = 0;
+#endif
+
+	ntcph = (void *)niph + ip_hdrlen(nskb);
+
+	/* Truncate to length (no data) */
+	ntcph->doff = sizeof(struct tcphdr)/4;
+	skb_trim(nskb, ip_hdrlen(nskb) + sizeof(struct tcphdr));
+	niph->tot_len = htons(nskb->len);
+
+	/* Swap source and dest */
+	niph->daddr = xchg(&niph->saddr, niph->daddr);
+	tmp = ntcph->source;
+	ntcph->source = ntcph->dest;
+	ntcph->dest = tmp;
+
+	/* Use supplied sequence number or make a new one */
+	ntcph->seq = otcph->ack ? otcph->ack_seq
+		: htonl(secure_tcp_sequence_number(niph->saddr,
+						   niph->daddr,
+						   ntcph->source,
+						   ntcph->dest));
+
+	/* Our SYN-ACKs must have a >0 window */
+	ntcph->window = (otcph->syn && !otcph->ack) ? htons(5) : 0;
+
+	ntcph->urg_ptr = 0;
+
+	/* Reset flags */
+	((u_int8_t *)ntcph)[13] = 0;
+
+	if (otcph->syn && otcph->ack) {
+		ntcph->rst = 1;
+		ntcph->ack_seq = 0;
+	} else {
+		ntcph->syn = otcph->syn;
+		ntcph->ack = 1;
+		ntcph->ack_seq = htonl(ntohl(otcph->seq) + otcph->syn);
+	}
+
+	/* Adjust TCP checksum */
+	ntcph->check = 0;
+	ntcph->check = tcp_v4_check(sizeof(struct tcphdr),
+				   niph->saddr,
+				   niph->daddr,
+				   csum_partial((char *)ntcph,
+						sizeof(struct tcphdr), 0));
+
+	fl.nl_u.ip4_u.daddr = niph->daddr;
+	fl.nl_u.ip4_u.saddr = local ? niph->saddr : 0;
+	fl.nl_u.ip4_u.tos = RT_TOS(niph->tos) | RTO_CONN;
+	fl.oif = 0;
+
+	if (ip_route_output_key(&nrt, &fl))
+		goto free_nskb;
+
+	dst_release(nskb->dst);
+	nskb->dst = &nrt->u.dst;
+
+	/* Adjust IP TTL */
+	niph->ttl = dst_metric(nskb->dst, RTAX_HOPLIMIT);
+
+	/* Set DF, id = 0 */
+	niph->frag_off = htons(IP_DF);
+	niph->id = 0;
+
+	/* Adjust IP checksum */
+	niph->check = 0;
+	niph->check = ip_fast_csum((unsigned char *)niph, niph->ihl);
+
+	/* "Never happens" */
+	if (nskb->len > dst_mtu(nskb->dst))
+		goto free_nskb;
+
+	ip_direct_send(nskb);
+	return;
+
+ free_nskb:
+	kfree_skb(nskb);
+}
+
+static unsigned int xt_tarpit_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 sk_buff *skb = *pskb;
+	const struct iphdr *iph   = ip_hdr(skb);
+	struct rtable *rt         = (void *)skb->dst;
+
+	/* Do we have an input route cache entry? */
+	if (rt == NULL)
+		return NF_DROP;
+
+	/* No replies to physical multicast/broadcast */
+	if (skb->pkt_type != PACKET_HOST && skb->pkt_type != PACKET_OTHERHOST)
+		return NF_DROP;
+
+	/* Now check at the protocol level */
+	if (rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
+		return NF_DROP;
+
+	/*
+	 * Our naive response construction does not deal with IP
+	 * options, and probably should not try.
+	 */
+	if (iph->ihl * 4 != sizeof(struct iphdr))
+		return NF_DROP;
+
+	/* We are not interested in fragments */
+	if (iph->frag_off & htons(IP_OFFSET))
+		return NF_DROP;
+
+	tarpit_tcp(skb, rt, hooknum == NF_IP_LOCAL_IN);
+	return NF_DROP;
+}
+
+static bool xt_tarpit_check(const char *tablename, const void *entry,
+                            const struct xt_target *target, void *targinfo,
+                            unsigned int hook_mask)
+{
+	bool invalid;
+
+	if (strcmp(tablename, "raw") == 0 && hook_mask == NF_IP_PRE_ROUTING)
+		return true;
+	if (strcmp(tablename, "filter") != 0)
+		return false;
+	invalid = hook_mask & ~((1 << NF_IP_LOCAL_IN) | (1 << NF_IP_FORWARD));
+	return !invalid;
+}
+
+static struct xt_target xt_tarpit_reg = {
+	.name       = "TARPIT",
+	.family     = AF_INET,
+	.proto      = IPPROTO_TCP,
+	.target     = xt_tarpit_target,
+	.checkentry = xt_tarpit_check,
+	.me         = THIS_MODULE,
+};
+
+static int __init xt_tarpit_init(void)
+{
+	return xt_register_target(&xt_tarpit_reg);
+}
+
+static void __exit xt_tarpit_exit(void)
+{
+	xt_unregister_target(&xt_tarpit_reg);
+}
+
+module_init(xt_tarpit_init);
+module_exit(xt_tarpit_exit);
+MODULE_DESCRIPTION("netfilter xt_TARPIT target module");
+MODULE_AUTHOR("Jan Engelhardt <jengelh@gmx.de>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ipt_TARPIT");

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Patrick McHardy]

Jan Engelhardt wrote:

> Comments please, thanks!


> +EXPORT_SYMBOL(secure_tcp_sequence_number);


Seems unnecessary, we don't really care whether the sequence number is
secure or not.

> +++ linux-2.6.22/net/netfilter/xt_TARPIT.c

> +#include <linux/version.h>
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/ip.h>
> +#include <net/ip.h>
> +#include <net/tcp.h>
> +#include <net/icmp.h>
> +struct in_device;

#include <linux/inetdevice.h>

> +#include <net/route.h>
> +#include <linux/random.h>
> +#include <linux/netfilter_ipv4/ip_tables.h>
> +
> +#if 0
> +#define DEBUGP printk
> +#else
> +#define DEBUGP(format, args...)
> +#endif

pr_debug please

> +
> +/* Stolen from ip_finish_output2 */
> +static int ip_direct_send(struct sk_buff *skb)
> +{
> +	struct dst_entry *dst = skb->dst;
> +
> +        if (dst->hh != NULL)
> +		return neigh_hh_output(dst->hh, skb);
> +	else if (dst->neighbour != NULL)
> +		return dst->neighbour->output(skb);
> +
> +	if (net_ratelimit())
> +		printk(KERN_DEBUG "TARPIT ip_direct_send: no header cache and no neighbor!\n");
> +
> +	kfree_skb(skb);
> +	return -EINVAL;
> +}

This function must die, use dst_output.

> +
> +
> +/* Send reply */
> +static void tarpit_tcp(const struct sk_buff *oskb, struct rtable *ort,
> +                       unsigned int local)
> +{
> +	struct sk_buff *nskb;
> +	struct rtable *nrt;
> +	struct tcphdr *otcph, *ntcph;
> +	struct flowi fl = {};
> +	unsigned int otcplen;
> +	u_int16_t tmp;
> +
> +	const struct iphdr *oiph = ip_hdr(oskb);
> +	struct iphdr *niph;
> +
> +	/* A truncated TCP header is not going to be useful */
> +	if (oskb->len < ip_hdrlen(oskb) + sizeof(struct tcphdr))
> +		return;
> +

skb_header_pointer, might be in the non-linear area.

> +	otcph   = (void *)oiph + ip_hdrlen(oskb);
> +	otcplen = oskb->len - ip_hdrlen(oskb);
> +
> +	/* No replies for RST or FIN */
> +	if (otcph->rst || otcph->fin)
> +		return;
> +
> +	/* No reply to !SYN,!ACK.  Rate-limit replies to !SYN,ACKs */
> +	if (!otcph->syn && (!otcph->ack || !xrlim_allow(&ort->u.dst, 1*HZ)))


No xrlim_allow .. ok maybe keep it while working on the other issues,
then we can think of something.

> +		return;
> +
> +	/* Check checksum. */
> +	if (tcp_v4_check(otcplen, oiph->saddr, oiph->daddr,
> +	    csum_partial((char *)otcph, otcplen, 0)) != 0)
> +		return;


Please resync with ipt_REJECT

> +
> +	/*
> +	 * Copy skb (even if skb is about to be dropped, we cannot just
> +	 * clone it because there may be other things, such as tcpdump,
> +	 * interested in it)
> +	 */
> +	nskb = skb_copy(oskb, GFP_ATOMIC);
> +	if (nskb == NULL)
> +		return;
> +
> +	niph = ip_hdr(nskb);
> +
> +	/* This packet will not be the same as the other: clear nf fields */
> +	nf_conntrack_put(nskb->nfct);
> +	nskb->nfct = NULL;
> +#ifdef CONFIG_NETFILTER_DEBUG
> +	nskb->nf_debug = 0;
> +#endif


Please resync. There is no nf_debug for years, there is nf_reset,
secmark handling is missing, ...


> +
> +	ntcph = (void *)niph + ip_hdrlen(nskb);
> +
> +	/* Truncate to length (no data) */
> +	ntcph->doff = sizeof(struct tcphdr)/4;
> +	skb_trim(nskb, ip_hdrlen(nskb) + sizeof(struct tcphdr));
> +	niph->tot_len = htons(nskb->len);
> +
> +	/* Swap source and dest */
> +	niph->daddr = xchg(&niph->saddr, niph->daddr);
> +	tmp = ntcph->source;
> +	ntcph->source = ntcph->dest;
> +	ntcph->dest = tmp;
> +
> +	/* Use supplied sequence number or make a new one */
> +	ntcph->seq = otcph->ack ? otcph->ack_seq
> +		: htonl(secure_tcp_sequence_number(niph->saddr,
> +						   niph->daddr,
> +						   ntcph->source,
> +						   ntcph->dest));
> +
> +	/* Our SYN-ACKs must have a >0 window */
> +	ntcph->window = (otcph->syn && !otcph->ack) ? htons(5) : 0;
> +
> +	ntcph->urg_ptr = 0;
> +
> +	/* Reset flags */
> +	((u_int8_t *)ntcph)[13] = 0;
> +
> +	if (otcph->syn && otcph->ack) {
> +		ntcph->rst = 1;
> +		ntcph->ack_seq = 0;
> +	} else {
> +		ntcph->syn = otcph->syn;
> +		ntcph->ack = 1;
> +		ntcph->ack_seq = htonl(ntohl(otcph->seq) + otcph->syn);
> +	}
> +
> +	/* Adjust TCP checksum */
> +	ntcph->check = 0;
> +	ntcph->check = tcp_v4_check(sizeof(struct tcphdr),
> +				   niph->saddr,
> +				   niph->daddr,
> +				   csum_partial((char *)ntcph,
> +						sizeof(struct tcphdr), 0));
> +
> +	fl.nl_u.ip4_u.daddr = niph->daddr;
> +	fl.nl_u.ip4_u.saddr = local ? niph->saddr : 0;
> +	fl.nl_u.ip4_u.tos = RT_TOS(niph->tos) | RTO_CONN;
> +	fl.oif = 0;
> +
> +	if (ip_route_output_key(&nrt, &fl))
> +		goto free_nskb;

More resyncing .. does not support IPsec.

> +
> +	dst_release(nskb->dst);
> +	nskb->dst = &nrt->u.dst;
> +
> +	/* Adjust IP TTL */
> +	niph->ttl = dst_metric(nskb->dst, RTAX_HOPLIMIT);
> +
> +	/* Set DF, id = 0 */
> +	niph->frag_off = htons(IP_DF);
> +	niph->id = 0;
> +
> +	/* Adjust IP checksum */
> +	niph->check = 0;
> +	niph->check = ip_fast_csum((unsigned char *)niph, niph->ihl);
> +
> +	/* "Never happens" */
> +	if (nskb->len > dst_mtu(nskb->dst))
> +		goto free_nskb;
> +
> +	ip_direct_send(nskb);
> +	return;
> +
> + free_nskb:
> +	kfree_skb(nskb);
> +}
> +
> +static unsigned int xt_tarpit_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 sk_buff *skb = *pskb;
> +	const struct iphdr *iph   = ip_hdr(skb);
> +	struct rtable *rt         = (void *)skb->dst;
> +
> +	/* Do we have an input route cache entry? */
> +	if (rt == NULL)
> +		return NF_DROP;

DROP? Is that reasonable? How about restricting to != PREROUTING?

> +
> +	/* No replies to physical multicast/broadcast */
> +	if (skb->pkt_type != PACKET_HOST && skb->pkt_type != PACKET_OTHERHOST)
> +		return NF_DROP;

PACKET_OTHERHOST is filtered in ip_rcv.

> +
> +	/* Now check at the protocol level */
> +	if (rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
> +		return NF_DROP;
> +
> +	/*
> +	 * Our naive response construction does not deal with IP
> +	 * options, and probably should not try.
> +	 */
> +	if (iph->ihl * 4 != sizeof(struct iphdr))
> +		return NF_DROP;

ip_hdrlen()

> +
> +	/* We are not interested in fragments */
> +	if (iph->frag_off & htons(IP_OFFSET))
> +		return NF_DROP;
> +
> +	tarpit_tcp(skb, rt, hooknum == NF_IP_LOCAL_IN);
> +	return NF_DROP;
> +}
> +
> +static bool xt_tarpit_check(const char *tablename, const void *entry,
> +                            const struct xt_target *target, void *targinfo,
> +                            unsigned int hook_mask)
> +{
> +	bool invalid;
> +
> +	if (strcmp(tablename, "raw") == 0 && hook_mask == NF_IP_PRE_ROUTING)
> +		return true;
> +	if (strcmp(tablename, "filter") != 0)
> +		return false;
> +	invalid = hook_mask & ~((1 << NF_IP_LOCAL_IN) | (1 << NF_IP_FORWARD));

Use generic validation please. This logic also looks overly complicated.

> +	return !invalid;
> +}
> +
> +static struct xt_target xt_tarpit_reg = {
> +	.name       = "TARPIT",
> +	.family     = AF_INET,
> +	.proto      = IPPROTO_TCP,
> +	.target     = xt_tarpit_target,
> +	.checkentry = xt_tarpit_check,
> +	.me         = THIS_MODULE,
> +};
> +
> +static int __init xt_tarpit_init(void)
> +{
> +	return xt_register_target(&xt_tarpit_reg);
> +}
> +
> +static void __exit xt_tarpit_exit(void)
> +{
> +	xt_unregister_target(&xt_tarpit_reg);
> +}
> +
> +module_init(xt_tarpit_init);
> +module_exit(xt_tarpit_exit);
> +MODULE_DESCRIPTION("netfilter xt_TARPIT target module");
> +MODULE_AUTHOR("Jan Engelhardt <jengelh@gmx.de>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("ipt_TARPIT");
> 

Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Jozsef Kadlecsik]

Hi,

On Wed, 18 Jul 2007, Patrick McHardy wrote:

>> +	/* This packet will not be the same as the other: clear nf fields */
>> +	nf_conntrack_put(nskb->nfct);
>> +	nskb->nfct = NULL;

If the target is called from the raw table, please attach the fake untrack 
entry to the created packet so that we could use TARPIT and conntrack 
nicely.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
           H-1525 Budapest 114, POB. 49, Hungary

Re: xt_TARPIT

[Yasuyuki KOZAKAI]

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Date: Wed, 18 Jul 2007 17:02:45 +0200 (CEST)

> Hi,
> 
> On Wed, 18 Jul 2007, Patrick McHardy wrote:
> 
> >> +	/* This packet will not be the same as the other: clear nf fields */
> >> +	nf_conntrack_put(nskb->nfct);
> >> +	nskb->nfct = NULL;
> 
> If the target is called from the raw table, please attach the fake untrack 
> entry to the created packet so that we could use TARPIT and conntrack 
> nicely.

I'm not sure that we should make TARPIT usable in raw table, but anyway
why the fake untrack entry is necessary ? I think that the created packet
is better to pass through LOCAL_OUT hook so that nf_conntrack can attach an
appropriate entry. That is what REJECT does.

-- Yasuyuki Kozakai

Re: xt_TARPIT

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Date: Wed, 18 Jul 2007 17:02:45 +0200 (CEST)
>
>   
>> If the target is called from the raw table, please attach the fake untrack 
>> entry to the created packet so that we could use TARPIT and conntrack 
>> nicely.
>>     
>
> I'm not sure that we should make TARPIT usable in raw table, but anyway
> why the fake untrack entry is necessary ? I think that the created packet
> is better to pass through LOCAL_OUT hook so that nf_conntrack can attach an
> appropriate entry. That is what REJECT does.
>   


I think both cases are valid. The restriction of REJECT to filter
(and that means INPUT FORWARD OUTPUT) has only one technical justification,
the packet is guaranteed to have a dst_entry, which is used for some
simple checks that could also be done otherwise. In raw the original
packet can't have been NATed, so a valid conntrack reference is not
necessary to NAT the reply. Other than that I can think of no real
reason why REJECT or TARPIT packets must have a conntrack refererence.

I don't really like adding a notrack reference in the TARPIT target
though, I would prefer to use the one from the original packet (as in
REJECT) and for NOTRACK you would simply mark the original packet.

Re: xt_TARPIT

[Yasuyuki KOZAKAI]

From: Patrick McHardy <kaber@trash.net>

> > I'm not sure that we should make TARPIT usable in raw table, but anyway
> > why the fake untrack entry is necessary ? I think that the created packet
> > is better to pass through LOCAL_OUT hook so that nf_conntrack can attach an
> > appropriate entry. That is what REJECT does.
> >   
> 
> 
> I think both cases are valid. The restriction of REJECT to filter
> (and that means INPUT FORWARD OUTPUT) has only one technical justification,
> the packet is guaranteed to have a dst_entry, which is used for some
> simple checks that could also be done otherwise. In raw the original
> packet can't have been NATed, so a valid conntrack reference is not
> necessary to NAT the reply. Other than that I can think of no real
> reason why REJECT or TARPIT packets must have a conntrack refererence.
>
> I don't really like adding a notrack reference in the TARPIT target
> though, I would prefer to use the one from the original packet (as in
> REJECT) and for NOTRACK you would simply mark the original packet.

You're right. Yes, nf_ct_attach is also necessary. I need caffeine.

-- Yasuyuki Kozakai

Re: xt_TARPIT

[Jozsef Kadlecsik]

On Thu, 19 Jul 2007, Patrick McHardy wrote:

> I don't really like adding a notrack reference in the TARPIT target
> though, I would prefer to use the one from the original packet (as in
> REJECT) and for NOTRACK you would simply mark the original packet.

That's better, indeed. And later we'll be able to say

  <match evil source> -j NOTRACK -j TARPIT

:-)

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
           H-1525 Budapest 114, POB. 49, Hungary

Re: xt_TARPIT

[Jan Engelhardt]

On Jul 18 2007 15:04, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> +EXPORT_SYMBOL(secure_tcp_sequence_number);
>
>Seems unnecessary, we don't really care whether the sequence number is
>secure or not.

Will tcp->seq = 0 do?

>> +	/* Truncate to length (no data) */
>> +	ntcph->doff = sizeof(struct tcphdr)/4;
>> +	skb_trim(nskb, ip_hdrlen(nskb) + sizeof(struct tcphdr));
>> +	niph->tot_len = htons(nskb->len);
>> +
>> +	/* Swap source and dest */
>> +	niph->daddr = xchg(&niph->saddr, niph->daddr);

BTW, how come ipt_REJECT does not use xchg()?

>> +	/* Do we have an input route cache entry? */
>> +	if (rt == NULL)
>> +		return NF_DROP;
>
>DROP? Is that reasonable? How about restricting to != PREROUTING?

Yes, that is indeed better.

>> +static bool xt_tarpit_check(const char *tablename, const void *entry,
>> +                            const struct xt_target *target, void *targinfo,
>> +                            unsigned int hook_mask)
>> +{
>> +	bool invalid;
>> +
>> +	if (strcmp(tablename, "raw") == 0 && hook_mask == NF_IP_PRE_ROUTING)
>> +		return true;
>> +	if (strcmp(tablename, "filter") != 0)
>> +		return false;
>> +	invalid = hook_mask & ~((1 << NF_IP_LOCAL_IN) | (1 << NF_IP_FORWARD));
>
>Use generic validation please. This logic also looks overly complicated.

I suppose you mean

	.table = "filter"

with "generic validation". I cannot use that, because it can be used in
two tables. [ See below ]



On Jul 18 2007 17:02, Jozsef Kadlecsik wrote:
> On Wed, 18 Jul 2007, Patrick McHardy wrote:
>
>> > +	/* This packet will not be the same as the other: clear nf fields */
>> > +	nf_conntrack_put(nskb->nfct);
>> > +	nskb->nfct = NULL;
>
> If the target is called from the raw table, please attach the fake untrack
> entry to the created packet so that we could use TARPIT and conntrack nicely.

Indeed. Unfortunately, since I do not see a way to get the table name
in the xt_tarpit_target() function, I need to store a flag in a custom
struct xt_tarpit_state or so. Not a problem, but the ->targetsize
stuff needs to be done right. (If anyone could kindly look at it :))



On Jul 19 2007 09:38, Yasuyuki KOZAKAI wrote:
>Josef wrote:
>> If the target is called from the raw table, please attach the fake untrack 
>> entry to the created packet so that we could use TARPIT and conntrack 
>> nicely.
>
>I'm not sure that we should make TARPIT usable in raw table, but anyway
>why the fake untrack entry is necessary ? I think that the created packet
>is better to pass through LOCAL_OUT hook so that nf_conntrack can attach an
>appropriate entry. That is what REJECT does.

The original TARPIT advertised that TARPIT is cheap -- when connection
tracking is not used. With a notrack entry, we can live up to that even
when connection tracking is loaded.



On Jul 19 2007 02:44, Patrick McHardy wrote:
> Yasuyuki KOZAKAI wrote:
>> I'm not sure that we should make TARPIT usable in raw table, but anyway
>> why the fake untrack entry is necessary ? I think that the created packet
>> is better to pass through LOCAL_OUT hook so that nf_conntrack can attach an
>> appropriate entry. That is what REJECT does.
>
> I think both cases are valid. The restriction of REJECT to filter
> (and that means INPUT FORWARD OUTPUT) has only one technical justification,
> the packet is guaranteed to have a dst_entry, which is used for some
> simple checks that could also be done otherwise. In raw the original
> packet can't have been NATed, so a valid conntrack reference is not
> necessary to NAT the reply. Other than that I can think of no real
> reason why REJECT or TARPIT packets must have a conntrack refererence.
>
> I don't really like adding a notrack reference in the TARPIT target
> though, I would prefer to use the one from the original packet (as in
> REJECT) and for NOTRACK you would simply mark the original packet.


On Jul 19 2007 10:09, Yasuyuki KOZAKAI wrote:
>From: Patrick McHardy <kaber@trash.net>
>> I don't really like adding a notrack reference in the TARPIT target
>> though, I would prefer to use the one from the original packet (as in
>> REJECT) and for NOTRACK you would simply mark the original packet.
>
>You're right. Yes, nf_ct_attach is also necessary. I need caffeine.

So what's the final verdict? If TARPIT shall not imply NOTRACK,
then of course I will remove the checks for "raw", etc. Please vote.

But IMO, it is easier to just TARPIT a connection and imply NOTRACK
instead of trying to MARK and NOTRACK separately.

(And additionally, TARPIT should also attach the notrack ct to the _original_
direction, not only the reply.)




	Jan
-- 

Re: xt_TARPIT

[Patrick McHardy]

Jan Engelhardt wrote:
> 
> On Jul 18 2007 15:04, Patrick McHardy wrote:
> 
>>Jan Engelhardt wrote:
>>
>>>+EXPORT_SYMBOL(secure_tcp_sequence_number);
>>
>>Seems unnecessary, we don't really care whether the sequence number is
>>secure or not.
> 
> 
> Will tcp->seq = 0 do?


I don't know.

>>>+	/* Truncate to length (no data) */
>>>+	ntcph->doff = sizeof(struct tcphdr)/4;
>>>+	skb_trim(nskb, ip_hdrlen(nskb) + sizeof(struct tcphdr));
>>>+	niph->tot_len = htons(nskb->len);
>>>+
>>>+	/* Swap source and dest */
>>>+	niph->daddr = xchg(&niph->saddr, niph->daddr);
> 
> 
> BTW, how come ipt_REJECT does not use xchg()?


Because its not necessary, as in this case.

>>>+static bool xt_tarpit_check(const char *tablename, const void *entry,
>>>+                            const struct xt_target *target, void *targinfo,
>>>+                            unsigned int hook_mask)
>>>+{
>>>+	bool invalid;
>>>+
>>>+	if (strcmp(tablename, "raw") == 0 && hook_mask == NF_IP_PRE_ROUTING)
>>>+		return true;
>>>+	if (strcmp(tablename, "filter") != 0)
>>>+		return false;
>>>+	invalid = hook_mask & ~((1 << NF_IP_LOCAL_IN) | (1 << NF_IP_FORWARD));
>>
>>Use generic validation please. This logic also looks overly complicated.
> 
> 
> I suppose you mean
> 
> 	.table = "filter"
> 
> with "generic validation". I cannot use that, because it can be used in
> two tables. [ See below ]


The origin version was simply restricted to some hooks. Why
does the table matter?

>>>I don't really like adding a notrack reference in the TARPIT target
>>>though, I would prefer to use the one from the original packet (as in
>>>REJECT) and for NOTRACK you would simply mark the original packet.
>>
>>You're right. Yes, nf_ct_attach is also necessary. I need caffeine.
> 
> 
> So what's the final verdict? If TARPIT shall not imply NOTRACK,
> then of course I will remove the checks for "raw", etc. Please vote.


I think I already did :)

Re: xt_TARPIT

[Jan Engelhardt]

On Jul 30 2007 14:23, Patrick McHardy wrote:
>
>>>>+static bool xt_tarpit_check(const char *tablename, const void *entry,
>>>>+                            const struct xt_target *target, void *targinfo,
>>>>+                            unsigned int hook_mask)
>>>>+{
>>>>+	bool invalid;
>>>>+
>>>>+	if (strcmp(tablename, "raw") == 0 && hook_mask == NF_IP_PRE_ROUTING)
>>>>+		return true;
>>>>+	if (strcmp(tablename, "filter") != 0)
>>>>+		return false;
>>>>+	invalid = hook_mask & ~((1 << NF_IP_LOCAL_IN) | (1 << NF_IP_FORWARD));
>>>
>>>Use generic validation please. This logic also looks overly complicated.
>> 
>> 
>> I suppose you mean
>> 
>> 	.table = "filter"
>> 
>> with "generic validation". I cannot use that, because it can be used in
>> two tables. [ See below ]
>
>The origin version was simply restricted to some hooks. Why
>does the table matter?

The original version limited it to filter-INPUT and filter-FORWARD.
But if raw-PREROUTING was to be added, the generic validation
things cannot be used, because two tables cannot be represented with it.
Anyway, since you say no to implicit notracking, the issue is gone.


	Jan
--