netdev - [REGRESSION] connection timeout with routes to VRF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 11 Jun 2022 13:14:43 +0200
From:   Jan Luebbe <jluebbe@...net.de>
To:     David Ahern <dsahern@...nel.org>,
        Robert Shearman <robertshearman@...il.com>
Cc:     Mike Manning <mvrmanning@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        regressions@...ts.linux.dev,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: [REGRESSION] connection timeout with routes to VRF

Hi,

TL;DR: We think we have found a regression in the handling of VRF route leaking
caused by "net: allow binding socket in a VRF when there's an unbound socket"
(3c82a21f4320).


We've been using VRF on 4.19 (from Debian buster) for serveral years and want to
update. After updating to 5.10 (from bullseye), we can no longer use routes
pointed at the VRF to reach services in the VRF

We use only one VRF, which contains some wireguard VPN interfaces.
Then we have routes outside the VRF to make the network connected to it
reachable from normal processes.

Our minimized test case looks like this:
 ip rule add pref 32765 from all lookup local
 ip rule del pref 0 from all lookup local
 ip link add red type vrf table 1000
 ip link set red up
 ip route add vrf red unreachable default metric 8192
 ip addr add dev red 172.16.0.1/24
 ip route add 172.16.0.0/24 dev red
 ip vrf exec red socat -dd TCP-LISTEN:1234,reuseaddr,fork SYSTEM:"echo connected" &
 sleep 1
 nc 172.16.0.1 1234 < /dev/null

While in our real setup, we connect to hosts reachable via th VRF's VPN
interfaces, the issue also shows up with a simple listening socket in the VRF.
In that case, the SYN-ACK from the remote host leads to a RST from the VRF, so
it seems that the outbound socket is not found.

A bisection with this leads to "net: allow binding socket in a VRF when there's
an unbound socket" (3c82a21f4320).

Before 3c82a21f4320, this connects fine:
 2022/06/11 11:11:03 socat[326] N listening on AF=2 0.0.0.0:1234
 nc 172.16.0.1 1234
 2022/06/11 11:11:04 socat[326] N accepting connection from AF=2 172.16.0.1:44164 on AF=2 172.16.0.1:1234
 ...
 connected

Since 3c82a21f4320, the connection does not complete:
 2022/06/11 11:16:31 socat[324] N listening on AF=2 0.0.0.0:1234
 + nc 172.16.0.1 1234
 (... times out)

We've retested with v5.19-rc1-262-g0885eacdc81f, and continue to get the
timeout.

The partial revert
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index 98e1ec1a14f0..41e7f20d7e51 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -310,8 +310,9 @@ static inline struct sock *inet_lookup_listener(struct net *net,
 #define INET_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif, __sdif) \
        (((__sk)->sk_portpair == (__ports))                     &&      \
         ((__sk)->sk_addrpair == (__cookie))                    &&      \
-        (((__sk)->sk_bound_dev_if == (__dif))                  ||      \
-         ((__sk)->sk_bound_dev_if == (__sdif)))                &&      \
+        (!(__sk)->sk_bound_dev_if      ||                              \
+          ((__sk)->sk_bound_dev_if == (__dif))                 ||      \
+          ((__sk)->sk_bound_dev_if == (__sdif)))               &&      \
         net_eq(sock_net(__sk), (__net)))
 #else /* 32-bit arch */
 #define INET_ADDR_COOKIE(__name, __saddr, __daddr) \
@@ -321,8 +322,9 @@ static inline struct sock *inet_lookup_listener(struct net *net,
        (((__sk)->sk_portpair == (__ports))             &&              \
         ((__sk)->sk_daddr      == (__saddr))           &&              \
         ((__sk)->sk_rcv_saddr  == (__daddr))           &&              \
-        (((__sk)->sk_bound_dev_if == (__dif))          ||              \
-         ((__sk)->sk_bound_dev_if == (__sdif)))        &&              \
+        (!(__sk)->sk_bound_dev_if      ||                              \
+          ((__sk)->sk_bound_dev_if == (__dif))         ||              \
+          ((__sk)->sk_bound_dev_if == (__sdif)))       &&              \
         net_eq(sock_net(__sk), (__net)))
 #endif /* 64-bit arch */

restores the original behavior when applied on v5.18. This doesn't apply
directly on master, as the macro was replaced by an inline function in "inet:
add READ_ONCE(sk->sk_bound_dev_if) in INET_MATCH()" (4915d50e300e).

I have to admit I don't quite understand 3c82a21f4320, so I'm not sure how to
proceed. What would be broken by the partial revert above? Are there better ways
to configure routing into the VRF than simply "ip route add 172.16.0.0/24 dev
red" that still work?

Thanks,
Jan

#regzbot introduced: 3c82a21f4320