Patchwork netfilter: xt_TCPMSS: Avoid violating RFC 879 in absence of MSS option

login
register
mail settings
Submitter Phil Oester
Date June 4, 2013, 3:09 p.m.
Message ID <20130604150927.GA9108@gmail.com>
Download mbox | patch
Permalink /patch/248681/
State Accepted
Headers show

Comments

Phil Oester - June 4, 2013, 3:09 p.m.
As reported in bug #662, the clamp-mss-to-pmtu option of the xt_TCPMSS target
can cause issues connecting to websites if there was no MSS option present in
the original SYN packet from the client.  In these cases, it adds an MSS higher
than the default specified in RFC 879.  Fix this by never setting a value > 536
IFF none was specified by the client.  

This closes bug #662.

Phil

Signed-off-by: Phil Oester <kernel@linuxace.com>
Pablo Neira - June 5, 2013, 12:09 p.m.
On Tue, Jun 04, 2013 at 11:09:27AM -0400, Phil Oester wrote:
> As reported in bug #662, the clamp-mss-to-pmtu option of the xt_TCPMSS target
> can cause issues connecting to websites if there was no MSS option present in
> the original SYN packet from the client.  In these cases, it adds an MSS higher
> than the default specified in RFC 879.  Fix this by never setting a value > 536
> IFF none was specified by the client.  
> 
> This closes bug #662.

Applied to the nf tree, thanks Phil.

BTW, this target does not seem to make safe fragmentation handling. We
need a patch similar to:

commit bc6bcb59dd7c184d229f9e86d08aa56059938a4c
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Tue May 7 03:22:18 2013 +0200

    netfilter: xt_TCPOPTSTRIP: fix possible mangling beyond packet boundary
--
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

Patch

diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index a75240f..53af7db 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -125,6 +125,13 @@  tcpmss_mangle_packet(struct sk_buff *skb,
 
 	skb_put(skb, TCPOLEN_MSS);
 
+	/*
+	 * RFC 879 states that the default MSS is 536 without specific
+	 * knowledge that the destination host is prepared to accept larger.
+	 * Since no MSS was provided, we MUST NOT set a value > 536.
+	 */
+	newmss = min(newmss, (u16)536);
+
 	opt = (u_int8_t *)tcph + sizeof(struct tcphdr);
 	memmove(opt + TCPOLEN_MSS, opt, tcplen - sizeof(struct tcphdr));