Re: [RFC] TCPOPTSTRIP target

[Patrick McHardy <kaber@trash.net>]

Jan Engelhardt wrote:
> Since I had nothing better to do, I did a cleanup :)


Great :) My main question is what the use case of this is.


> @@ -0,0 +1,155 @@
> +/*
> + * A module for stripping a specific TCP option from TCP packets.
> + *
> + * Copyright (C) 2007 Sven Schnelle <svens@bitebene.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
> +#include <net/ipv6.h>
> +#include <net/tcp.h>
> +#include <linux/netfilter/x_tables.h>
> +#include <linux/netfilter/xt_tcpudp.h>

tcpudp.h? Shouldn't be needed.

> +#include <linux/netfilter/xt_TCPOPTSTRIP.h>
> +
> +static void fast_csum(__sum16 *csum, const u_int8_t *optr,
> +                      const u_int8_t *nptr, const int offset)
> +{
> +	u_int8_t s[4];
> +
> +	if (offset & 1) {
> +		s[0] = s[2] = 0;
> +		s[1] = ~*optr;
> +		s[3] = *nptr;
> +	} else {
> +		s[1] = s[3] = 0;
> +		s[0] = ~*optr;
> +		s[2] = *nptr;
> +	}
> +
> +	*csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum)));
> +}


Please use the generic checksumming helpers.

> +
> +static unsigned int
> +tcpoptstrip_mangle_packet(struct sk_buff **pskb,
> +                          const struct xt_tcpoptstrip_info *info,
> +                          unsigned int tcphoff, unsigned int minlen)
> +{
> +	const u_int8_t newopt = TCPOPT_NOP;
> +	unsigned int optl, tcplen, i, j;
> +	struct tcphdr *tcph;
> +	u_int8_t *opt;
> +
> +	if (!skb_make_writable(pskb, (*pskb)->len))
> +		return NF_DROP;
> +
> +	tcplen = (*pskb)->len - tcphoff;
> +	tcph   = (struct tcphdr *)(skb_network_header(*pskb) + tcphoff);
> +
> +	if (tcplen != 4 * tcph->doff)
> +		return NF_DROP;


Seems to be copied from TCPMSS - but I don't see why you shouldn't allow
stripping options from packets with data.

> +
> +	opt = (u_int8_t *)tcph;
> +
> +	for (i = sizeof(struct tcphdr); i < 4 * tcph->doff; i += optl) {
> +		optl = optlen(opt, i);
> +
> +		if (optl + i > tcph->doff*4)
> +			break;
> +
> +		if (opt[i] == info->tcpoption) {
> +			for (j = 0; j < optl; j++)
> +				fast_csum(&tcph->check, opt + i + j,
> +				          &newopt, i + j);
> +			if (optl & 1)
> +				fast_csum(&tcph->check, &newopt,
> +				          &newopt, i + j);
> +			memset(opt+i, newopt, optl);


For TCPOPTSTRIP I would expect either real stripping or replacement
by TCPOPT_NOP. In which cases does replacement by something else
make sense?