Message ID | 20130610035948.GA2742@gmail.com |
---|---|
State | Not Applicable |
Headers | show |
On Sun, Jun 09, 2013 at 11:59:48PM -0400, Phil Oester wrote: > In commit bc6bcb59 ("netfilter: xt_TCPOPTSTRIP: fix possible mangling beyond > packet boundary"), a check for short TCP header or malformed packet was added. > This check is unnecessary, as these packets are already handled in the tcp_error > function of nf_conntrack_proto_tcp.c (see /* Not whole TCP header or malformed > packet */). We cannot assume nf_conntrack is loaded. We have to support stateless setups as well. > In addition, there was an error in the check which was added (len > is being calculated incorrectly). In my testing, ALL packets are being dropped > by the TCPOPTSTRIP target at present. Revert the unnecessary/incorrect checks. Then, we have to fix the wrong calculation. I cannot reproduce this here. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > We cannot assume nf_conntrack is loaded. We have to support stateless > setups as well. > > > In addition, there was an error in the check which was added (len > > is being calculated incorrectly). In my testing, ALL packets are being dropped > > by the TCPOPTSTRIP target at present. Revert the unnecessary/incorrect checks. > > Then, we have to fix the wrong calculation. I cannot reproduce this > here. Its most likely due to tcp_hdr() use, it only works for local packets. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/xt_TCPOPTSTRIP.c b/net/netfilter/xt_TCPOPTSTRIP.c index 1eb1a44..2d43be9f 100644 --- a/net/netfilter/xt_TCPOPTSTRIP.c +++ b/net/netfilter/xt_TCPOPTSTRIP.c @@ -38,7 +38,6 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb, struct tcphdr *tcph; u_int16_t n, o; u_int8_t *opt; - int len; /* This is a fragment, no TCP header is available */ if (par->fragoff != 0) @@ -47,11 +46,6 @@ tcpoptstrip_mangle_packet(struct sk_buff *skb, if (!skb_make_writable(skb, skb->len)) return NF_DROP; - len = skb->len - tcphoff; - if (len < (int)sizeof(struct tcphdr) || - tcp_hdr(skb)->doff * 4 > len) - return NF_DROP; - tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff); opt = (u_int8_t *)tcph;
In commit bc6bcb59 ("netfilter: xt_TCPOPTSTRIP: fix possible mangling beyond packet boundary"), a check for short TCP header or malformed packet was added. This check is unnecessary, as these packets are already handled in the tcp_error function of nf_conntrack_proto_tcp.c (see /* Not whole TCP header or malformed packet */). In addition, there was an error in the check which was added (len is being calculated incorrectly). In my testing, ALL packets are being dropped by the TCPOPTSTRIP target at present. Revert the unnecessary/incorrect checks. Phil Signed-off-by: Phil Oester <kernel@linuxace.com>