LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH net] ipvs: SNAT packet replies only for NATed connections

To: Julian Anastasov <ja@xxxxxx>
Subject: Re: [PATCH net] ipvs: SNAT packet replies only for NATed connections
Cc: lvs-devel@xxxxxxxxxxxxxxx, Nick Moriarty <nick.moriarty@xxxxxxxxxx>
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Mon, 1 May 2017 09:55:42 +0200
On Sat, Apr 29, 2017 at 08:33:09PM +0300, Julian Anastasov wrote:
> We do not check if packet from real server is for NAT
> connection before performing SNAT. This causes problems
> for setups that use DR/TUN and allow local clients to
> access the real server directly, for example:
> 
> - local client in director creates IPVS-DR/TUN connection
> CIP->VIP and the request packets are routed to RIP.
> Talks are finished but IPVS connection is not expired yet.
> 
> - second local client creates non-IPVS connection CIP->RIP
> with same reply tuple RIP->CIP and when replies are received
> on LOCAL_IN we wrongly assign them for the first client
> connection because RIP->CIP matches the reply direction.
> As result, IPVS SNATs replies for non-IPVS connections.
> 
> The problem is more visible to local UDP clients but in rare
> cases it can happen also for TCP or remote clients when the
> real server sends the reply traffic via the director.
> 
> So, better to be more precise for the reply traffic.
> As replies are not expected for DR/TUN connections, better
> to not touch them.
> 
> Reported-by: Nick Moriarty <nick.moriarty@xxxxxxxxxx>
> Tested-by: Nick Moriarty <nick.moriarty@xxxxxxxxxx>
> Signed-off-by: Julian Anastasov <ja@xxxxxx>
> ---
> 
> I know that 4.11 is to be released soon, I prefer this patch
> to linger in the net tree during the 4.12-rc cycle.

I have no problem with queueing this up as a fix for v4.12 as you describe
but do you also want it to be considered for -stable?

> 
>  net/netfilter/ipvs/ip_vs_core.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index db40050..ee44ed5 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -849,10 +849,8 @@ static int handle_response_icmp(int af, struct sk_buff 
> *skb,
>  {
>       unsigned int verdict = NF_DROP;
>  
> -     if (IP_VS_FWD_METHOD(cp) != 0) {
> -             pr_err("shouldn't reach here, because the box is on the "
> -                    "half connection in the tun/dr module.\n");
> -     }
> +     if (IP_VS_FWD_METHOD(cp) != IP_VS_CONN_F_MASQ)
> +             goto ignore_cp;
>  
>       /* Ensure the checksum is correct */
>       if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
> @@ -886,6 +884,8 @@ static int handle_response_icmp(int af, struct sk_buff 
> *skb,
>               ip_vs_notrack(skb);
>       else
>               ip_vs_update_conntrack(skb, cp, 0);
> +
> +ignore_cp:
>       verdict = NF_ACCEPT;
>  
>  out:
> @@ -1385,8 +1385,11 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int 
> hooknum, struct sk_buff *skb, in
>        */
>       cp = pp->conn_out_get(ipvs, af, skb, &iph);
>  
> -     if (likely(cp))
> +     if (likely(cp)) {
> +             if (IP_VS_FWD_METHOD(cp) != IP_VS_CONN_F_MASQ)
> +                     goto ignore_cp;
>               return handle_response(af, skb, pd, cp, &iph, hooknum);
> +     }
>  
>       /* Check for real-server-started requests */
>       if (atomic_read(&ipvs->conn_out_counter)) {
> @@ -1444,9 +1447,15 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int 
> hooknum, struct sk_buff *skb, in
>                       }
>               }
>       }
> +
> +out:
>       IP_VS_DBG_PKT(12, af, pp, skb, iph.off,
>                     "ip_vs_out: packet continues traversal as normal");
>       return NF_ACCEPT;
> +
> +ignore_cp:
> +     __ip_vs_conn_put(cp);
> +     goto out;
>  }
>  
>  /*
> -- 
> 2.9.3
> 
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>