[PATCH] netfilter: synproxy: erroneous TCP mss option fixed.

[PATCH] netfilter: synproxy: erroneous TCP mss option fixed.

[İbrahim Ercan]

Syn proxy isn't setting mss value correctly on client syn-ack packet.

It was sending same mss value with client send instead of the value
user set in iptables rule.
This patch fix that wrong behavior by passing client mss information
to synproxy_send_client_synack correctly.

Signed-off-by: Ibrahim Ercan <ibrahim.metu@gmail.com>

diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c
b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 64d9563..e0bd504 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -69,13 +69,13 @@ synproxy_send_tcp(struct net *net,
 static void
 synproxy_send_client_synack(struct net *net,
                            const struct sk_buff *skb, const struct tcphdr *th,
-                           const struct synproxy_options *opts)
+                           const struct synproxy_options *opts, const
u16 client_mssinfo)
 {
        struct sk_buff *nskb;
        struct iphdr *iph, *niph;
        struct tcphdr *nth;
        unsigned int tcp_hdr_size;
-       u16 mss = opts->mss;
+       u16 mss = client_mssinfo;

        iph = ip_hdr(skb);

@@ -264,6 +264,7 @@ synproxy_tg4(struct sk_buff *skb, const struct
xt_action_param *par)
        struct synproxy_net *snet = synproxy_pernet(net);
        struct synproxy_options opts = {};
        struct tcphdr *th, _th;
+       u16 client_mssinfo;

        if (nf_ip_checksum(skb, xt_hooknum(par), par->thoff, IPPROTO_TCP))
                return NF_DROP;
@@ -283,6 +284,8 @@ synproxy_tg4(struct sk_buff *skb, const struct
xt_action_param *par)
                        opts.options |= XT_SYNPROXY_OPT_ECN;

                opts.options &= info->options;
+               client_mssinfo = opts.mss;
+               opts.mss = info->mss;
                if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
                        synproxy_init_timestamp_cookie(info, &opts);
                else
@@ -290,7 +293,7 @@ synproxy_tg4(struct sk_buff *skb, const struct
xt_action_param *par)
                                          XT_SYNPROXY_OPT_SACK_PERM |
                                          XT_SYNPROXY_OPT_ECN);

-               synproxy_send_client_synack(net, skb, th, &opts);
+               synproxy_send_client_synack(net, skb, th, &opts,
client_mssinfo);
                consume_skb(skb);
                return NF_STOLEN;
        } else if (th->ack && !(th->fin || th->rst || th->syn)) {
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c
b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index 41325d5..676de53 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -83,13 +83,13 @@ synproxy_send_tcp(struct net *net,
 static void
 synproxy_send_client_synack(struct net *net,
                            const struct sk_buff *skb, const struct tcphdr *th,
-                           const struct synproxy_options *opts)
+                           const struct synproxy_options *opts, const
u16 client_mssinfo)
 {
        struct sk_buff *nskb;
        struct ipv6hdr *iph, *niph;
        struct tcphdr *nth;
        unsigned int tcp_hdr_size;
-       u16 mss = opts->mss;
+       u16 mss = client_mssinfo;

        iph = ipv6_hdr(skb);

@@ -278,6 +278,7 @@ synproxy_tg6(struct sk_buff *skb, const struct
xt_action_param *par)
        struct synproxy_net *snet = synproxy_pernet(net);
        struct synproxy_options opts = {};
        struct tcphdr *th, _th;
+       u16 client_mssinfo;

        if (nf_ip6_checksum(skb, xt_hooknum(par), par->thoff, IPPROTO_TCP))
                return NF_DROP;
@@ -297,6 +298,8 @@ synproxy_tg6(struct sk_buff *skb, const struct
xt_action_param *par)
                        opts.options |= XT_SYNPROXY_OPT_ECN;

                opts.options &= info->options;
+               client_mssinfo = opts.mss;
+               opts.mss = info->mss;
                if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
                        synproxy_init_timestamp_cookie(info, &opts);
                else
@@ -304,7 +307,7 @@ synproxy_tg6(struct sk_buff *skb, const struct
xt_action_param *par)
                                          XT_SYNPROXY_OPT_SACK_PERM |
                                          XT_SYNPROXY_OPT_ECN);

-               synproxy_send_client_synack(net, skb, th, &opts);
+               synproxy_send_client_synack(net, skb, th, &opts,
client_mssinfo);
                consume_skb(skb);
                return NF_STOLEN;

Re: [PATCH] netfilter: synproxy: erroneous TCP mss option fixed.

[Florian Westphal]

İbrahim Ercan <ibrahim.metu@gmail.com> wrote:
> Syn proxy isn't setting mss value correctly on client syn-ack packet.
> 
> It was sending same mss value with client send instead of the value
> user set in iptables rule.
> This patch fix that wrong behavior by passing client mss information
> to synproxy_send_client_synack correctly.

Patch is line-wrapped by MUA, but other than that it looks correct.

Thanks for fixing this bug.

Acked-by: Florian Westphal <fw@strlen.de>

Re: [PATCH] netfilter: synproxy: erroneous TCP mss option fixed.

[Pablo Neira Ayuso]

On Mon, Jun 24, 2019 at 03:28:07PM +0300, İbrahim Ercan wrote:
> Syn proxy isn't setting mss value correctly on client syn-ack packet.
> 
> It was sending same mss value with client send instead of the value
> user set in iptables rule.
> This patch fix that wrong behavior by passing client mss information
> to synproxy_send_client_synack correctly.

Cannot apply, wrapped by MUA, please re-submit.

Thanks.

[PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Ibrahim Ercan]

Syn proxy isn't setting mss value correctly on client syn-ack packet.
It was sending same mss value with client send instead of the value user set in iptables rule. This patch fix that wrong behavior by passing client mss information to synproxy_send_client_synack correctly.

Signed-off-by: Ibrahim Ercan <ibrahim.ercan@labristeknoloji.com>
---
 net/ipv4/netfilter/ipt_SYNPROXY.c  | 9 ++++++---
 net/ipv6/netfilter/ip6t_SYNPROXY.c | 9 ++++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 64d9563..e0bd504 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -69,13 +69,13 @@ synproxy_send_tcp(struct net *net,
 static void
 synproxy_send_client_synack(struct net *net,
 			    const struct sk_buff *skb, const struct tcphdr *th,
-			    const struct synproxy_options *opts)
+			    const struct synproxy_options *opts, const u16 client_mssinfo)
 {
 	struct sk_buff *nskb;
 	struct iphdr *iph, *niph;
 	struct tcphdr *nth;
 	unsigned int tcp_hdr_size;
-	u16 mss = opts->mss;
+	u16 mss = client_mssinfo;
 
 	iph = ip_hdr(skb);
 
@@ -264,6 +264,7 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
 	struct synproxy_net *snet = synproxy_pernet(net);
 	struct synproxy_options opts = {};
 	struct tcphdr *th, _th;
+	u16 client_mssinfo;
 
 	if (nf_ip_checksum(skb, xt_hooknum(par), par->thoff, IPPROTO_TCP))
 		return NF_DROP;
@@ -283,6 +284,8 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
 			opts.options |= XT_SYNPROXY_OPT_ECN;
 
 		opts.options &= info->options;
+		client_mssinfo = opts.mss;
+		opts.mss = info->mss;
 		if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
 			synproxy_init_timestamp_cookie(info, &opts);
 		else
@@ -290,7 +293,7 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
 					  XT_SYNPROXY_OPT_SACK_PERM |
 					  XT_SYNPROXY_OPT_ECN);
 
-		synproxy_send_client_synack(net, skb, th, &opts);
+		synproxy_send_client_synack(net, skb, th, &opts, client_mssinfo);
 		consume_skb(skb);
 		return NF_STOLEN;
 	} else if (th->ack && !(th->fin || th->rst || th->syn)) {
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index 41325d5..676de53 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -83,13 +83,13 @@ synproxy_send_tcp(struct net *net,
 static void
 synproxy_send_client_synack(struct net *net,
 			    const struct sk_buff *skb, const struct tcphdr *th,
-			    const struct synproxy_options *opts)
+			    const struct synproxy_options *opts, const u16 client_mssinfo)
 {
 	struct sk_buff *nskb;
 	struct ipv6hdr *iph, *niph;
 	struct tcphdr *nth;
 	unsigned int tcp_hdr_size;
-	u16 mss = opts->mss;
+	u16 mss = client_mssinfo;
 
 	iph = ipv6_hdr(skb);
 
@@ -278,6 +278,7 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 	struct synproxy_net *snet = synproxy_pernet(net);
 	struct synproxy_options opts = {};
 	struct tcphdr *th, _th;
+	u16 client_mssinfo;
 
 	if (nf_ip6_checksum(skb, xt_hooknum(par), par->thoff, IPPROTO_TCP))
 		return NF_DROP;
@@ -297,6 +298,8 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 			opts.options |= XT_SYNPROXY_OPT_ECN;
 
 		opts.options &= info->options;
+		client_mssinfo = opts.mss;
+		opts.mss = info->mss;
 		if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
 			synproxy_init_timestamp_cookie(info, &opts);
 		else
@@ -304,7 +307,7 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 					  XT_SYNPROXY_OPT_SACK_PERM |
 					  XT_SYNPROXY_OPT_ECN);
 
-		synproxy_send_client_synack(net, skb, th, &opts);
+		synproxy_send_client_synack(net, skb, th, &opts, client_mssinfo);
 		consume_skb(skb);
 		return NF_STOLEN;
 
-- 
2.7.4

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Pablo Neira Ayuso]

On Tue, Jun 25, 2019 at 08:42:04AM +0300, Ibrahim Ercan wrote:
> Syn proxy isn't setting mss value correctly on client syn-ack packet.
> It was sending same mss value with client send instead of the value user set in iptables rule. This patch fix that wrong behavior by passing client mss information to synproxy_send_client_synack correctly.
> 
> Signed-off-by: Ibrahim Ercan <ibrahim.ercan@labristeknoloji.com>
> ---
>  net/ipv4/netfilter/ipt_SYNPROXY.c  | 9 ++++++---
>  net/ipv6/netfilter/ip6t_SYNPROXY.c | 9 ++++++---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
> index 64d9563..e0bd504 100644
> --- a/net/ipv4/netfilter/ipt_SYNPROXY.c
> +++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
> @@ -69,13 +69,13 @@ synproxy_send_tcp(struct net *net,
>  static void
>  synproxy_send_client_synack(struct net *net,
>  			    const struct sk_buff *skb, const struct tcphdr *th,
> -			    const struct synproxy_options *opts)
> +			    const struct synproxy_options *opts, const u16 client_mssinfo)
>  {
>  	struct sk_buff *nskb;
>  	struct iphdr *iph, *niph;
>  	struct tcphdr *nth;
>  	unsigned int tcp_hdr_size;
> -	u16 mss = opts->mss;
> +	u16 mss = client_mssinfo;
>  
>  	iph = ip_hdr(skb);
>  
> @@ -264,6 +264,7 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
>  	struct synproxy_net *snet = synproxy_pernet(net);
>  	struct synproxy_options opts = {};
>  	struct tcphdr *th, _th;
> +	u16 client_mssinfo;
>  
>  	if (nf_ip_checksum(skb, xt_hooknum(par), par->thoff, IPPROTO_TCP))
>  		return NF_DROP;
> @@ -283,6 +284,8 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
>  			opts.options |= XT_SYNPROXY_OPT_ECN;
>  
>  		opts.options &= info->options;
> +		client_mssinfo = opts.mss;
> +		opts.mss = info->mss;

No need for this new client_mssinfo variable, right? I mean, you can
just set:

        opts.mss = info->mss;

and use it from synproxy_send_client_synack().

This patch will be smaller.

>  		if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
>  			synproxy_init_timestamp_cookie(info, &opts);
>  		else
> @@ -290,7 +293,7 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
>  					  XT_SYNPROXY_OPT_SACK_PERM |
>  					  XT_SYNPROXY_OPT_ECN);
>  
> -		synproxy_send_client_synack(net, skb, th, &opts);
> +		synproxy_send_client_synack(net, skb, th, &opts, client_mssinfo);
>  		consume_skb(skb);
>  		return NF_STOLEN;
>  	} else if (th->ack && !(th->fin || th->rst || th->syn)) {

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >  		opts.options &= info->options;
> > +		client_mssinfo = opts.mss;
> > +		opts.mss = info->mss;
> 
> No need for this new client_mssinfo variable, right? I mean, you can
> just set:
> 
>         opts.mss = info->mss;
> 
> and use it from synproxy_send_client_synack().

I thought that as well but we need both mss values,
the one configured in the target (info->mss) and the
ine received from the peer.

The former is what we announce to peer in the syn/ack
(as tcp option), the latter is what we need to encode
in the syncookie (to decode it on cookie ack).

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Pablo Neira Ayuso]

On Thu, Jun 27, 2019 at 09:00:19PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >  		opts.options &= info->options;
> > > +		client_mssinfo = opts.mss;
> > > +		opts.mss = info->mss;
> > 
> > No need for this new client_mssinfo variable, right? I mean, you can
> > just set:
> > 
> >         opts.mss = info->mss;
> > 
> > and use it from synproxy_send_client_synack().
> 
> I thought that as well but we need both mss values,
> the one configured in the target (info->mss) and the
> ine received from the peer.
> 
> The former is what we announce to peer in the syn/ack
> (as tcp option), the latter is what we need to encode
> in the syncookie (to decode it on cookie ack).

I see, probably place client_mss field into the synproxy_options
structure?

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Jun 27, 2019 at 09:00:19PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > >  		opts.options &= info->options;
> > > > +		client_mssinfo = opts.mss;
> > > > +		opts.mss = info->mss;
> > > 
> > > No need for this new client_mssinfo variable, right? I mean, you can
> > > just set:
> > > 
> > >         opts.mss = info->mss;
> > > 
> > > and use it from synproxy_send_client_synack().
> > 
> > I thought that as well but we need both mss values,
> > the one configured in the target (info->mss) and the
> > ine received from the peer.
> > 
> > The former is what we announce to peer in the syn/ack
> > (as tcp option), the latter is what we need to encode
> > in the syncookie (to decode it on cookie ack).
> 
> I see, probably place client_mss field into the synproxy_options
> structure?

I worked on a fix for this too (Ibrahim was faster), I
tried to rename opts.mss so we have

u16 mss_peer;
u16 mss_configured;

but I got confused myself as to where which mss is to be used.

perhaps
u16 mss_option;
u16 mss_encode;

... would have been better.

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Pablo Neira Ayuso]

On Thu, Jun 27, 2019 at 09:21:09PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Jun 27, 2019 at 09:00:19PM +0200, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > >  		opts.options &= info->options;
> > > > > +		client_mssinfo = opts.mss;
> > > > > +		opts.mss = info->mss;
> > > > 
> > > > No need for this new client_mssinfo variable, right? I mean, you can
> > > > just set:
> > > > 
> > > >         opts.mss = info->mss;
> > > > 
> > > > and use it from synproxy_send_client_synack().
> > > 
> > > I thought that as well but we need both mss values,
> > > the one configured in the target (info->mss) and the
> > > ine received from the peer.
> > > 
> > > The former is what we announce to peer in the syn/ack
> > > (as tcp option), the latter is what we need to encode
> > > in the syncookie (to decode it on cookie ack).
> > 
> > I see, probably place client_mss field into the synproxy_options
> > structure?
> 
> I worked on a fix for this too (Ibrahim was faster), I
> tried to rename opts.mss so we have
> 
> u16 mss_peer;
> u16 mss_configured;
> 
> but I got confused myself as to where which mss is to be used.
> 
> perhaps
> u16 mss_option;
> u16 mss_encode;
> 
> ... would have been better.

I would leave the opts.mss as is by now. Given there will be a
conflict between nf-next and nf, I was trying to minimize the number
of chunks for this fix, but that's just my preference (I'll have to
sort out this it seems).

Then, adding follow up patches to rename fields would be great indeed
as you suggest.

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Pablo Neira Ayuso]

Hi Ibrahim,

On Thu, Jun 27, 2019 at 09:27:05PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Jun 27, 2019 at 09:21:09PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Thu, Jun 27, 2019 at 09:00:19PM +0200, Florian Westphal wrote:
> > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
[...]
> > > I see, probably place client_mss field into the synproxy_options
> > > structure?
> > 
> > I worked on a fix for this too (Ibrahim was faster), I
> > tried to rename opts.mss so we have
> > 
> > u16 mss_peer;
> > u16 mss_configured;
> > 
> > but I got confused myself as to where which mss is to be used.
> > 
> > perhaps
> > u16 mss_option;
> > u16 mss_encode;
> > 
> > ... would have been better.
> 
> I would leave the opts.mss as is by now. Given there will be a
> conflict between nf-next and nf, I was trying to minimize the number
> of chunks for this fix, but that's just my preference (I'll have to
> sort out this it seems).
> 
> Then, adding follow up patches to rename fields would be great indeed
> as you suggest.

@Ibrahim: Would you follow up with patch v3?

I'd name this 'mss_backend' to opts, instead of adding client_mss as
parameter. Since this is the MSS that the server backend behind the
proxy.

I don't mind names, if you prefer Florian's, that's also fine. I'd
just like not to add a new parameter to synproxy_send_client_synack().

Thanks.

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[İbrahim Ercan]

On Mon, Jul 1, 2019 at 9:58 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi Ibrahim,
>
> On Thu, Jun 27, 2019 at 09:27:05PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Jun 27, 2019 at 09:21:09PM +0200, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > On Thu, Jun 27, 2019 at 09:00:19PM +0200, Florian Westphal wrote:
> > > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> [...]
> > > > I see, probably place client_mss field into the synproxy_options
> > > > structure?
> > >
> > > I worked on a fix for this too (Ibrahim was faster), I
> > > tried to rename opts.mss so we have
> > >
> > > u16 mss_peer;
> > > u16 mss_configured;
> > >
> > > but I got confused myself as to where which mss is to be used.
> > >
> > > perhaps
> > > u16 mss_option;
> > > u16 mss_encode;
> > >
> > > ... would have been better.
> >
> > I would leave the opts.mss as is by now. Given there will be a
> > conflict between nf-next and nf, I was trying to minimize the number
> > of chunks for this fix, but that's just my preference (I'll have to
> > sort out this it seems).
> >
> > Then, adding follow up patches to rename fields would be great indeed
> > as you suggest.
>
> @Ibrahim: Would you follow up with patch v3?
>
> I'd name this 'mss_backend' to opts, instead of adding client_mss as
> parameter. Since this is the MSS that the server backend behind the
> proxy.
>
> I don't mind names, if you prefer Florian's, that's also fine. I'd
> just like not to add a new parameter to synproxy_send_client_synack().
>
> Thanks.

Sorry for late reply. I was offline for 3 weeks. I will send new patch asap.

Re: [PATCH v2] netfilter: synproxy: erroneous TCP mss option fixed.

[Pablo Neira Ayuso]

On Mon, Jul 22, 2019 at 11:31:45AM +0300, İbrahim Ercan wrote:
> On Mon, Jul 1, 2019 at 9:58 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > Hi Ibrahim,
> >
> > On Thu, Jun 27, 2019 at 09:27:05PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Jun 27, 2019 at 09:21:09PM +0200, Florian Westphal wrote:
> > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > > On Thu, Jun 27, 2019 at 09:00:19PM +0200, Florian Westphal wrote:
> > > > > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > [...]
> > > > > I see, probably place client_mss field into the synproxy_options
> > > > > structure?
> > > >
> > > > I worked on a fix for this too (Ibrahim was faster), I
> > > > tried to rename opts.mss so we have
> > > >
> > > > u16 mss_peer;
> > > > u16 mss_configured;
> > > >
> > > > but I got confused myself as to where which mss is to be used.
> > > >
> > > > perhaps
> > > > u16 mss_option;
> > > > u16 mss_encode;
> > > >
> > > > ... would have been better.
> > >
> > > I would leave the opts.mss as is by now. Given there will be a
> > > conflict between nf-next and nf, I was trying to minimize the number
> > > of chunks for this fix, but that's just my preference (I'll have to
> > > sort out this it seems).
> > >
> > > Then, adding follow up patches to rename fields would be great indeed
> > > as you suggest.
> >
> > @Ibrahim: Would you follow up with patch v3?
> >
> > I'd name this 'mss_backend' to opts, instead of adding client_mss as
> > parameter. Since this is the MSS that the server backend behind the
> > proxy.
> >
> > I don't mind names, if you prefer Florian's, that's also fine. I'd
> > just like not to add a new parameter to synproxy_send_client_synack().
> >
> > Thanks.
> 
> Sorry for late reply. I was offline for 3 weeks. I will send new patch asap.

Please, do, based it on kernel 5.2

Fernando already made a patch for 5.3-rc, we'll take your patch for
-stable, as a backport.

[PATCH v3] netfilter: synproxy: erroneous TCP mss option fixed.

[Ibrahim Ercan]

synproxy_options has been modified as recommended by Pablo.

Signed-off-by: Ibrahim Ercan <ibrahim.ercan@labrisnetworks.com>
---
 include/net/netfilter/nf_conntrack_synproxy.h | 3 ++-
 net/ipv4/netfilter/ipt_SYNPROXY.c             | 6 ++++--
 net/ipv6/netfilter/ip6t_SYNPROXY.c            | 6 ++++--
 net/netfilter/nf_synproxy_core.c              | 4 ++--
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_synproxy.h b/include/net/netfilter/nf_conntrack_synproxy.h
index 2c7559a..d4b44b3 100644
--- a/include/net/netfilter/nf_conntrack_synproxy.h
+++ b/include/net/netfilter/nf_conntrack_synproxy.h
@@ -66,7 +66,8 @@ static inline struct synproxy_net *synproxy_pernet(struct net *net)
 struct synproxy_options {
 	u8				options;
 	u8				wscale;
-	u16				mss;
+	u16				mss_option;
+	u16				mss_encode;
 	u32				tsval;
 	u32				tsecr;
 };
diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 64d9563..6e230a6 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -75,7 +75,7 @@ synproxy_send_client_synack(struct net *net,
 	struct iphdr *iph, *niph;
 	struct tcphdr *nth;
 	unsigned int tcp_hdr_size;
-	u16 mss = opts->mss;
+	u16 mss = opts->mss_encode;
 
 	iph = ip_hdr(skb);
 
@@ -246,7 +246,7 @@ synproxy_recv_client_ack(struct net *net,
 	}
 
 	this_cpu_inc(snet->stats->cookie_valid);
-	opts->mss = mss;
+	opts->mss_option = mss;
 	opts->options |= XT_SYNPROXY_OPT_MSS;
 
 	if (opts->options & XT_SYNPROXY_OPT_TIMESTAMP)
@@ -283,6 +283,8 @@ synproxy_tg4(struct sk_buff *skb, const struct xt_action_param *par)
 			opts.options |= XT_SYNPROXY_OPT_ECN;
 
 		opts.options &= info->options;
+		opts.mss_encode = opts.mss_option;
+		opts.mss_option = info->mss;
 		if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
 			synproxy_init_timestamp_cookie(info, &opts);
 		else
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index 41325d5..36313b0 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -89,7 +89,7 @@ synproxy_send_client_synack(struct net *net,
 	struct ipv6hdr *iph, *niph;
 	struct tcphdr *nth;
 	unsigned int tcp_hdr_size;
-	u16 mss = opts->mss;
+	u16 mss = opts->mss_encode;
 
 	iph = ipv6_hdr(skb);
 
@@ -260,7 +260,7 @@ synproxy_recv_client_ack(struct net *net,
 	}
 
 	this_cpu_inc(snet->stats->cookie_valid);
-	opts->mss = mss;
+	opts->mss_option = mss;
 	opts->options |= XT_SYNPROXY_OPT_MSS;
 
 	if (opts->options & XT_SYNPROXY_OPT_TIMESTAMP)
@@ -297,6 +297,8 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 			opts.options |= XT_SYNPROXY_OPT_ECN;
 
 		opts.options &= info->options;
+		opts.mss_encode = opts.mss_option;
+		opts.mss_option = info->mss;
 		if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
 			synproxy_init_timestamp_cookie(info, &opts);
 		else
diff --git a/net/netfilter/nf_synproxy_core.c b/net/netfilter/nf_synproxy_core.c
index 8ce74ed..74ff90a 100644
--- a/net/netfilter/nf_synproxy_core.c
+++ b/net/netfilter/nf_synproxy_core.c
@@ -56,7 +56,7 @@ synproxy_parse_options(const struct sk_buff *skb, unsigned int doff,
 			switch (opcode) {
 			case TCPOPT_MSS:
 				if (opsize == TCPOLEN_MSS) {
-					opts->mss = get_unaligned_be16(ptr);
+					opts->mss_option = get_unaligned_be16(ptr);
 					opts->options |= XT_SYNPROXY_OPT_MSS;
 				}
 				break;
@@ -115,7 +115,7 @@ synproxy_build_options(struct tcphdr *th, const struct synproxy_options *opts)
 	if (options & XT_SYNPROXY_OPT_MSS)
 		*ptr++ = htonl((TCPOPT_MSS << 24) |
 			       (TCPOLEN_MSS << 16) |
-			       opts->mss);
+			       opts->mss_option);
 
 	if (options & XT_SYNPROXY_OPT_TIMESTAMP) {
 		if (options & XT_SYNPROXY_OPT_SACK_PERM)
-- 
2.7.4