git: d18c9a919795 - main - RPCBIND: skip ipv6 link local when request is not from link local address

From: David Bright <dab_at_FreeBSD.org>
Date: Mon, 04 Oct 2021 17:46:59 UTC
The branch main has been updated by dab:

URL: https://cgit.FreeBSD.org/src/commit/?id=d18c9a91979543adc182c7b28819691b64fda388

commit d18c9a91979543adc182c7b28819691b64fda388
Author:     David Bright <dab@FreeBSD.org>
AuthorDate: 2021-10-04 15:43:41 +0000
Commit:     David Bright <dab@FreeBSD.org>
CommitDate: 2021-10-04 17:45:26 +0000

    RPCBIND: skip ipv6 link local when request is not from link local address
    
    RPCINFO on macOS behaves different compared to other linux clients and
    doesn't provide request address in rpcb structure of the
    RPCBPROC_GETADDRLIST call which doesn't seem to be forbidden.
    
    In this case RPCBIND uses RPC call's source address and picks a
    closest corresponding local address. If there are no addresses in the
    same subnet as the source address, return of RPCBIND may vary
    depending on the order of addresses returned in getifaddrs. If a link
    local precedes global address it may be returned even if the request
    comes from neither a link local nor from link local in a different
    scope, which will prevent services like nfs from working in tpc6
    scenario on macOS clients. Issue can be seen only on FreeBSD rpcbind
    port due to changes in workflow of addrmerge call.
    
    Submitted by:   Dmitry Ovsyannikov (Dmitry.Ovsyannikov@dell.com)
    Reviewers:      dab
    Differential Revision:  https://reviews.freebsd.org/D31491
    Sponsored by:   Dell EMC
    MFC to: stable/12, stable/13
    MFC after:      1 week
---
 usr.sbin/rpcbind/tests/addrmerge_test.c | 34 +++++++++++++++++++++++++++++++++
 usr.sbin/rpcbind/util.c                 | 12 +++++++-----
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/usr.sbin/rpcbind/tests/addrmerge_test.c b/usr.sbin/rpcbind/tests/addrmerge_test.c
index e1600f1a4837..a29dc30fdde8 100644
--- a/usr.sbin/rpcbind/tests/addrmerge_test.c
+++ b/usr.sbin/rpcbind/tests/addrmerge_test.c
@@ -257,6 +257,22 @@ mock_tun0(void)
 	    IFF_UP | IFF_RUNNING | IFF_POINTOPOINT | IFF_MULTICAST, 0, false);
 }
 
+static void
+mock_mlxen0(void)
+{
+	mock_ifaddr4("mlxen0", "192.0.3.1", "255.255.255.128", "192.0.3.127",
+	    IFF_UP | IFF_BROADCAST | IFF_RUNNING | IFF_SIMPLEX | IFF_MULTICAST,
+	    false);
+	/* Setting link local address before ipv6 address*/
+	mock_ifaddr6("mlxen0", "fe80::4", "ffff:ffff:ffff:ffff::",
+	    "fe80::ffff:ffff:ffff:ffff",
+	    IFF_UP | IFF_BROADCAST | IFF_RUNNING | IFF_SIMPLEX | IFF_MULTICAST,
+	    3, false);
+	mock_ifaddr6("mlxen0", "2001:db8::7", "ffff:ffff:ffff:ffff::",
+	    "2001:db8::ffff:ffff:ffff:ffff",
+	    IFF_UP | IFF_BROADCAST | IFF_RUNNING | IFF_SIMPLEX | IFF_MULTICAST,
+	    0, false);
+}
 
 /* Stub rpcbind functions */
 int
@@ -835,6 +851,23 @@ ATF_TC_BODY(addrmerge_recvdstaddr6_rev, tc)
 	ATF_CHECK_STREQ("2001:db8::2.3.46", maddr);
 	free(maddr);
 }
+
+ATF_TC_WITHOUT_HEAD(addrmerge_ipv6_other_subnet);
+ATF_TC_BODY(addrmerge_ipv6_other_subnet, tc)
+{
+	char *maddr;
+
+	/* getifaddrs will return link local before normal ipv6 */
+	mock_lo0();
+	mock_mlxen0();
+	
+	maddr = do_addrmerge6("2001:db8:1::1.3.46");
+
+	/* We must return the closest ipv6 address*/
+	ATF_REQUIRE(maddr != NULL);
+	ATF_CHECK_STREQ("2001:db8::7.3.46", maddr);
+	free(maddr);
+}
 #endif /* INET6 */
 
 
@@ -864,6 +897,7 @@ ATF_TP_ADD_TCS(tp)
 	ATF_TP_ADD_TC(tp, addrmerge_ipv6_linklocal_rev);
 	ATF_TP_ADD_TC(tp, addrmerge_recvdstaddr6);
 	ATF_TP_ADD_TC(tp, addrmerge_recvdstaddr6_rev);
+	ATF_TP_ADD_TC(tp, addrmerge_ipv6_other_subnet);
 #endif
 
 	return (atf_no_error());
diff --git a/usr.sbin/rpcbind/util.c b/usr.sbin/rpcbind/util.c
index 455578b657fb..e497e9227690 100644
--- a/usr.sbin/rpcbind/util.c
+++ b/usr.sbin/rpcbind/util.c
@@ -229,17 +229,19 @@ addrmerge(struct netbuf *caller, const char *serv_uaddr, const char *clnt_uaddr,
 			 * a link-local address then use the scope id to see
 			 * which one.
 			 */
-			if (IN6_IS_ADDR_LINKLOCAL(&SA2SIN6ADDR(ifsa)) &&
-			    IN6_IS_ADDR_LINKLOCAL(&SA2SIN6ADDR(caller_sa)) &&
-			    IN6_IS_ADDR_LINKLOCAL(&SA2SIN6ADDR(hint_sa))) {
-				if (SA2SIN6(ifsa)->sin6_scope_id ==
-				    SA2SIN6(caller_sa)->sin6_scope_id) {
+			if (IN6_IS_ADDR_LINKLOCAL(&SA2SIN6ADDR(ifsa))) {
+				if (IN6_IS_ADDR_LINKLOCAL(&SA2SIN6ADDR(caller_sa)) &&
+				    IN6_IS_ADDR_LINKLOCAL(&SA2SIN6ADDR(hint_sa)) &&
+				    (SA2SIN6(ifsa)->sin6_scope_id ==
+				     SA2SIN6(caller_sa)->sin6_scope_id)) {
 					const int goodness = 3;
 
 					if (bestif_goodness < goodness) {
 						bestif = ifap;
 						bestif_goodness = goodness;
 					}
+				} else {
+					continue;
 				}
 			}
 		}