Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Leonard Crestez <cdleonard@gmail.com>
To:     David Ahern <dsahern@kernel.org>,
        Eric Dumazet <edumazet@google.com>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        Jakub Kicinski <kuba@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Kuniyuki Iwashima <kuniyu@amazon.co.jp>,
        Yonghong Song <yhs@fb.com>,
        Alexander Duyck <alexanderduyck@fb.com>,
        Florian Westphal <fw@strlen.de>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: [PATCH v2 0/4] tcp: md5: Fix overlap between vrf and non-vrf keys
Date:   Wed, 13 Oct 2021 09:50:34 +0300
Message-Id: <cover.1634107317.git.cdleonard@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

With net.ipv4.tcp_l3mdev_accept=1 it is possible for a listen socket to
accept connection from the same client address in different VRFs. It is
also possible to set different MD5 keys for these clients which differ
only in the tcpm_l3index field.

This appears to work when distinguishing between different VRFs but not
between non-VRF and VRF connections. In particular:

 * tcp_md5_do_lookup_exact will match a non-vrf key against a vrf key.
This means that adding a key with l3index != 0 after a key with l3index
== 0 will cause the earlier key to be deleted. Both keys can be present
if the non-vrf key is added later.
 * _tcp_md5_do_lookup can match a non-vrf key before a vrf key. This
casues failures if the passwords differ.

This can be fixed by making tcp_md5_do_lookup_exact perform an actual
exact comparison on l3index and by making  __tcp_md5_do_lookup perfer
vrf-bound keys above other considerations like prefixlen.

The fact that keys with l3index==0 affect VRF connections is usually
not desirable, VRFs are meant to be completely independent. This
behavior needs to preserved for backwards compatiblity. Also,
applications can just bind listen sockets to VRF and never specify
TCP_MD5SIG_IFINDEX at all.

So far the combination of TCP_MD5SIG_IFINDEX with tcpm_ifindex == 0
was an error, accept this to mean "key only applies to default VRF".
This is what applications using VRFs for traffic separation want.

This also contains tests for the second part. It does not contain
tests for overlapping keys, that would require more changes in
nettest to add multiple keys. These scenarios are also covered by
my tests for TCP-AO, especially around this area:
https://github.com/cdleonard/tcp-authopt-test/blob/main/tcp_authopt_test/test_vrf_bind.py

Changes since V1:
* Accept (TCP_MD5SIG_IFINDEX with tcpm_ifindex == 0)
* Add flags for explicitly including or excluding TCP_MD5SIG_IFINDEX
to nettest
* Add few more tests in fcnal-test.sh.
Link to v1: https://lore.kernel.org/netdev/3d8387d499f053dba5cd9184c0f7b8445c4470c6.1633542093.git.cdleonard@gmail.com/

Leonard Crestez (4):
  tcp: md5: Fix overlap between vrf and non-vrf keys
  tcp: md5: Allow MD5SIG_FLAG_IFINDEX with ifindex=0
  selftests: nettest: Add --{do,no}-bind-key-ifindex
  selftests: net/fcnal: Test --{do,no}-bind-key-ifindex

 include/net/tcp.h                         |  5 +-
 net/ipv4/tcp_ipv4.c                       | 45 ++++++++++++-----
 net/ipv6/tcp_ipv6.c                       | 15 +++---
 tools/testing/selftests/net/fcnal-test.sh | 60 +++++++++++++++++++++++
 tools/testing/selftests/net/nettest.c     | 28 ++++++++++-
 5 files changed, 130 insertions(+), 23 deletions(-)


base-commit: d1f24712a86abd04d82cf4b00fb4ab8ff2d23c8a
-- 
2.25.1