Re: xt_TARPIT (was: ipt_account / iptables 1.3.8)

[Patrick McHardy <kaber@trash.net>]

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");
>