From patchwork Tue Sep 20 15:01:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 672301 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3sdmDN1C0zz9ryv for ; Wed, 21 Sep 2016 01:01:24 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=Xe/qShc3; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932708AbcITPBR (ORCPT ); Tue, 20 Sep 2016 11:01:17 -0400 Received: from mail-pa0-f43.google.com ([209.85.220.43]:35918 "EHLO mail-pa0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932174AbcITPBP (ORCPT ); Tue, 20 Sep 2016 11:01:15 -0400 Received: by mail-pa0-f43.google.com with SMTP id id6so7918476pad.3; Tue, 20 Sep 2016 08:01:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=niRlKuzJ4Hgk7M3HMXsfHT5N+OIrEZ8Mj1G/bGKcNAI=; b=Xe/qShc31/u8WVhd8ALhTzr/FI6nFlSr276QzTg3YHYy17r3nZZ/nM6zNUqsRwjSeZ G69+/5pYKNzXr9VXkwQj3onnFlOiGG9ff4a4WE5oAE6cbFS5HX+jQ8bAHWtWMx8iX8Re AMckBW/ZBhniOCOdZmWzptzAf/2KkRY4XfDC0Yt7XFK8T5tcLu+VcbTjtXZHxDcfn151 wdG/5AtLGjM6bnuTLNbNvY9SB6llpa/q83+M52c1/4hLEdiawJRIH4viDi8vJkSnvlI/ vC9pkevQk9/9axJvThXl1XPOJ+JZTCz3KGMbe0qtlJmVE69U0tlzveMJKA2RrZKgQs+i OhZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=niRlKuzJ4Hgk7M3HMXsfHT5N+OIrEZ8Mj1G/bGKcNAI=; b=ZS2Wv3haVLBafQdsNgoUIsVYruaVgmNL3XHYKO0fQqiYPgZfzRGYFufZMijRhrpck3 8Nf/spk56wfZmrZEeI3AS7A8QEejMoRPqcizTqJZL91ig43x2Xl9lJTvomTOIT7xeBcW LyK4B9BeBkZnwbwzO6pEvM3bPwTE9YfZrCAxNDCm9gBmQqT9TZMjvYCaKzy9yjrHSJXO 7UdrtzYEUQ8BvsXdgC8ojeFar0Q4sfPzWU0GjFA0NxqDlCTNwvv+yp5CIEaaSMg+p8G/ kzoVp/30xkPHbPD+C+8pwWVsK2rnFigw1aPFOh74qmEF6AxNigrLwvJLjo5AQL/8O51L wlXw== X-Gm-Message-State: AE9vXwPZ9ih8jbbCPp5EddvqzuCx0q+wCUDHanQ/bwSt+HWek9Hqpa3NQaeEcJ6LjXV9/w== X-Received: by 10.66.249.103 with SMTP id yt7mr28179187pac.46.1474383674108; Tue, 20 Sep 2016 08:01:14 -0700 (PDT) Received: from ?IPv6:2620:0:1000:1704:e452:8718:4c81:d449? ([2620:0:1000:1704:e452:8718:4c81:d449]) by smtp.googlemail.com with ESMTPSA id i4sm27084244pav.27.2016.09.20.08.01.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Sep 2016 08:01:12 -0700 (PDT) Message-ID: <1474383672.23058.19.camel@edumazet-glaptop3.roam.corp.google.com> Subject: Re: [PATCH] netfilter: xt_socket: fix transparent match for IPv6 request sockets From: Eric Dumazet To: KOVACS Krisztian Cc: Pablo Neira Ayuso , netfilter-devel@vger.kernel.org, Alex Badics , netdev Date: Tue, 20 Sep 2016 08:01:12 -0700 In-Reply-To: <20160920132637.221892-1-hidden@balabit.com> References: <20160920132637.221892-1-hidden@balabit.com> X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 2016-09-20 at 15:26 +0200, KOVACS Krisztian wrote: > The introduction of TCP_NEW_SYN_RECV state, and the addition of request > sockets to the ehash table seems to have broken the --transparent option > of the socket match for IPv6 (around commit a9407000). > > Now that the socket lookup finds the TCP_NEW_SYN_RECV socket instead of the > listener, the --transparent option tries to match on the no_srccheck flag > of the request socket. > > Unfortunately, that flag was only set for IPv4 sockets in tcp_v4_init_req() > by copying the transparent flag of the listener socket. This effectively > causes '-m socket --transparent' not match on the ACK packet sent by the > client in a TCP handshake. > > This change adds the same code initializing no_srccheck to > tcp_v6_init_req(), rendering the above scenario working again. > > Signed-off-by: Alex Badics > Signed-off-by: KOVACS Krisztian > --- > net/ipv6/tcp_ipv6.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c > index 94f4f89..21f2e5c 100644 > --- a/net/ipv6/tcp_ipv6.c > +++ b/net/ipv6/tcp_ipv6.c > @@ -690,6 +690,7 @@ static void tcp_v6_init_req(struct request_sock *req, > > ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr; > ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr; > + ireq->no_srccheck = inet_sk(sk_listener)->transparent; > > /* So that link locals have meaning */ > if (!sk_listener->sk_bound_dev_if && Thanks a lot for the bug hunting guys. Could you add the precise tag to help stable backports ? Fixes: 12-digit-sha1 ("patch title") Since it is a netdev patch, I would also copy netdev@ (done here) Also what about moving this (IPv4/IPv6 common code) before the af_ops->init_req(req, sk, skb); call, since it is no longer family specific ? Something like : diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 3ebf45b38bc3..0fccfd6cc258 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -6264,6 +6264,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops, /* Note: tcp_v6_init_req() might override ir_iif for link locals */ inet_rsk(req)->ir_iif = inet_request_bound_dev_if(sk, skb); + ireq->no_srccheck = inet_sk(sk)->transparent; af_ops->init_req(req, sk, skb); if (security_inet_conn_request(sk, skb, req)) diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 7158d4f8dae4..b448eb9fe1b9 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1195,7 +1195,6 @@ static void tcp_v4_init_req(struct request_sock *req, sk_rcv_saddr_set(req_to_sk(req), ip_hdr(skb)->daddr); sk_daddr_set(req_to_sk(req), ip_hdr(skb)->saddr); - ireq->no_srccheck = inet_sk(sk_listener)->transparent; ireq->opt = tcp_v4_save_options(skb); }