netdev - Re: [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230815100816.GA24858@bogon>
Date: Tue, 15 Aug 2023 18:08:16 +0800
From: Geliang Tang <geliang.tang@...e.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Andrii Nakryiko <andrii@...nel.org>,
	Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>,
	Yonghong Song <yhs@...com>,
	John Fastabend <john.fastabend@...il.com>,
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>,
	Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
	Florent Revest <revest@...omium.org>,
	Brendan Jackman <jackmanb@...omium.org>,
	Matthieu Baerts <matthieu.baerts@...sares.net>,
	Mat Martineau <martineau@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	John Johansen <john.johansen@...onical.com>,
	Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Stephen Smalley <stephen.smalley.work@...il.com>,
	Eric Paris <eparis@...isplace.org>, Mykola Lysenko <mykolal@...com>,
	Shuah Khan <shuah@...nel.org>, Simon Horman <horms@...nel.org>,
	bpf@...r.kernel.org, netdev@...r.kernel.org, mptcp@...ts.linux.dev,
	linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH mptcp-next v13 4/4] selftests/bpf: Add mptcpify test

On Mon, Aug 14, 2023 at 11:23:49PM -0700, Martin KaFai Lau wrote:
> On 8/11/23 7:54 PM, Geliang Tang wrote:
> > +static int verify_mptcpify(int server_fd)
> > +{
> > +	socklen_t optlen;
> > +	char cmd[256];
> > +	int protocol;
> > +	int err = 0;
> > +
> > +	optlen = sizeof(protocol);
> > +	if (!ASSERT_OK(getsockopt(server_fd, SOL_SOCKET, SO_PROTOCOL, &protocol, &optlen),
> > +		       "getsockopt(SOL_PROTOCOL)"))
> > +		return -1;
> > +
> > +	if (!ASSERT_EQ(protocol, IPPROTO_MPTCP, "protocol isn't MPTCP"))
> > +		err++;
> > +
> > +	/* Output of nstat:
> > +	 *
> > +	 * #kernel
> > +	 * MPTcpExtMPCapableSYNACKRX       1                  0.0
> > +	 */
> > +	snprintf(cmd, sizeof(cmd),
> > +		 "ip netns exec %s nstat -asz %s | awk '%s' | grep -q '%s'",
> > +		 NS_TEST, "MPTcpExtMPCapableSYNACKRX",
> > +		 "NR==1 {next} {print $2}", "1");
> 
> Is the mp-capable something that the regular mptcp user want to learn from a
> fd also? Does it have a simpler way like to learn this, eg. getsockopt(fd,
> SOL_MPTCP, MPTCP_xxx), instead of parsing text output?

Thanks Martin. Yes, you're right. A better one is using getsockopt
(MPTCP_INFO) to get the mptcpi_flags, then test the FALLBACK bit to make
sure this MPTCP connection didn't fallback. This is, in other word, this
MPTCP connection has been established correctly. Something like this:

+       optlen = sizeof(info);
+       if (!ASSERT_OK(getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &info, &optlen),
+                      "getsockopt(MPTCP_INFO)"))
+               return -1;
+
+       if (!ASSERT_FALSE(info.mptcpi_flags & MPTCP_INFO_FLAG_FALLBACK,
+                         "MPTCP fallback"))
+               err++;

It's necessary to add this further check after the MPTCP protocol check
using getsockopt(SOL_PROTOCOL). Since in some cases, the MPTCP protocol
check is not enough. Say, if we change TCP protocol into MPTCP using
"cgroup/sock_create", the hook of BPF_CGROUP_RUN_PROG_INET_SOCK in
inet_create(), this place is too late to change the protocol. Although
sk->sk_protocol is set to MPTCP correctly, and the MPTCP protocol check
using getsockopt(SOL_PROTOCOL) will pass. This MPTCP connection will
fallback to TCP connection. So this further check is needed.

-Geliang

> 
> > +	if (!ASSERT_OK(system(cmd), "No MPTcpExtMPCapableSYNACKRX found!"))
> 
>