From nobody Thu Jan 23 13:58:52 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Yf2bw58hbz5l041; Thu, 23 Jan 2025 13:58:52 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Yf2bw4TXYz3bqs; Thu, 23 Jan 2025 13:58:52 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737640732; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=GOgB/7cyjpL2wubQyJZHxXEXU6q2GzrVW6xXVQ3saAM=; b=Lqbo8DLQzVgSmi7DeECJ2e6JXESicm6RST7DqA+xPxTSKPuaoiCPeZh/5Mpu4QQhJCUVqR mLmq+6S3ujE/AZ2fx5tAlmPQD8W9UEK1imSSl9rDA3BwYA+kwa4Ebwnhodxqg74rBdBh2V GeTsOI2Vzz/7zADwjABGW6BLXSAuiKWq1vX7Cpue0tocKqxdzMvECOKNKTbrM0q00wwtqV k+osiONNuJt8//kgZNsM2v5fwZVbln3EKvn2IiopgdYjWiN5FwX82B8yf2rCxR0HKXqzbq AtYYcFcQyxyhghAIWVnw68pQfFFJP0/tsgbi7sigBqVCK5OTCdKSbkR//8DDaA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737640732; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=GOgB/7cyjpL2wubQyJZHxXEXU6q2GzrVW6xXVQ3saAM=; b=ukmu2otBVUUrlp0QsnS4cxN3ybW3P+Tojri99p0MussZqSSDRZZ2hhVx2bUvePmvZstt6q q+nXrv6EP8pY4H+KlD2cj60jNIhhotSADo1qxeSpUUOBlpTHQdHtf7Carvi/2Q67wfZyna h8h/kzVA0ggbs3mFEQaK/5zlMfLtcn2T0BKjo2Wi1cZe8sduJmOrVzWYP34ZdHjX1onS7l ToDkfmug9llzR+wVbIVQPUEpDCzCiAC9pxcEWKo7o9+gpmX4iyOBr4pUx3BIy5H3eX3jx3 Xulr5dC8dBt+vt9ssSst7HRYLHbl9CBaE8u4n9cRt7F8XuD/UlB7AHGuevud+w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737640732; a=rsa-sha256; cv=none; b=gZqfAGguLJLEqx3sAz8ydNIR9gcw+v3I/zgMR/QbOK4vaRRl6vES4TQPGVJFaWtrnYPitR IjhRbkaxfD+tiSghliPPLJG2yEhgLk1im7PJvB39t6Up4iGlNvxfbPDIm0evfRMZe83iG5 ZBzj22OhczhOlmnMujUcu4TqZNsV7dXSpVr8Sa0rA+lpan22eWHN5hhv1GMbovcJCXy1WB +c+vBq7DgWY5s8kwmGMfZF34tmPpebh5n54zGrNBG8T2ZXpln+t7Gc7On1rFveDlLQjtnx CgvWfCEzo/jxmnifJnUGt+EIl02xaCiPF+jUhIz02oC5iD0VBqKIoVqOtb92CA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Yf2bw3xGFzj8t; Thu, 23 Jan 2025 13:58:52 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 50NDwqs8026233; Thu, 23 Jan 2025 13:58:52 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 50NDwqSh026230; Thu, 23 Jan 2025 13:58:52 GMT (envelope-from git) Date: Thu, 23 Jan 2025 13:58:52 GMT Message-Id: <202501231358.50NDwqSh026230@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 49f741240242 - stable/14 - inpcb: Further restrict binding to a port owned by a different UID List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 49f7412402428e827f4f8b3583703a6784baa764 Auto-Submitted: auto-generated The branch stable/14 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=49f7412402428e827f4f8b3583703a6784baa764 commit 49f7412402428e827f4f8b3583703a6784baa764 Author: Mark Johnston AuthorDate: 2024-12-23 15:31:11 +0000 Commit: Mark Johnston CommitDate: 2025-01-23 13:58:07 +0000 inpcb: Further restrict binding to a port owned by a different UID See commit 4f02a7d739b3 for more background. I cannot see a good reason to continue ignoring mismatching UIDs when binding to INADDR_ANY. Looking at the sdr.V2.4a7n sources (mentioned in bugzilla PR 7713), there is a CANT_MCAST_BIND hack wherein the application binds to INADDR_ANY instead of a multicast address, but CANT_MCAST_BIND isn't defined for FreeBSD builds. It seems unlikely that we still have a use-case for allowing sockets from different UIDs to bind to the same port when binding to the unspecified address. And, as noted in D47832, applications like sdr would have been broken by the inverted SO_REUSEPORT check removed in that revision, apparently without any bug reports. Let's break compatibility and simply disallow this case outright. Also, add some comments, remove a hack in a regression test which tests this funtionality, and add a new regression test to exercise the remaining checks that were added in commit 4658dc8325e03. MFC after: 1 month Sponsored by: Klara, Inc. Sponsored by: Stormshield Differential Revision: https://reviews.freebsd.org/D47870 (cherry picked from commit c9756953bded0d8428027fa3e812c9bdac069252) --- sys/netinet/in_pcb.c | 11 ++++- sys/netinet6/in6_pcb.c | 11 ++++- tests/sys/netinet/socket_afinet.c | 86 ++++++++++++++++++++++++++++++++++----- 3 files changed, 94 insertions(+), 14 deletions(-) diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index 15729bcd1ce3..937fa6f826c0 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -921,13 +921,20 @@ in_pcbbind_avail(struct inpcb *inp, const struct in_addr laddr, if (!IN_MULTICAST(ntohl(laddr.s_addr)) && priv_check_cred(inp->inp_cred, PRIV_NETINET_REUSEPORT) != 0) { + /* + * If a socket owned by a different user is already + * bound to this port, fail. In particular, SO_REUSE* + * can only be used to share a port among sockets owned + * by the same user. + * + * However, we can share a port with a connected socket + * which has a unique 4-tuple. + */ t = in_pcblookup_local(inp->inp_pcbinfo, laddr, lport, INPLOOKUP_WILDCARD, cred); if (t != NULL && (inp->inp_socket->so_type != SOCK_STREAM || in_nullhost(t->inp_faddr)) && - (!in_nullhost(laddr) || - !in_nullhost(t->inp_laddr)) && (inp->inp_cred->cr_uid != t->inp_cred->cr_uid)) return (EADDRINUSE); } diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c index b3b2f03451aa..a88e47abef64 100644 --- a/sys/netinet6/in6_pcb.c +++ b/sys/netinet6/in6_pcb.c @@ -241,13 +241,20 @@ in6_pcbbind_avail(struct inpcb *inp, const struct sockaddr_in6 *sin6, if (!IN6_IS_ADDR_MULTICAST(laddr) && priv_check_cred(inp->inp_cred, PRIV_NETINET_REUSEPORT) != 0) { + /* + * If a socket owned by a different user is already + * bound to this port, fail. In particular, SO_REUSE* + * can only be used to share a port among sockets owned + * by the same user. + * + * However, we can share a port with a connected socket + * which has a unique 4-tuple. + */ t = in6_pcblookup_local(inp->inp_pcbinfo, laddr, lport, INPLOOKUP_WILDCARD, cred); if (t != NULL && (inp->inp_socket->so_type != SOCK_STREAM || IN6_IS_ADDR_UNSPECIFIED(&t->in6p_faddr)) && - (!IN6_IS_ADDR_UNSPECIFIED(laddr) || - !IN6_IS_ADDR_UNSPECIFIED(&t->in6p_laddr)) && (inp->inp_cred->cr_uid != t->inp_cred->cr_uid)) return (EADDRINUSE); diff --git a/tests/sys/netinet/socket_afinet.c b/tests/sys/netinet/socket_afinet.c index ba8c03af46a6..6fc98d982602 100644 --- a/tests/sys/netinet/socket_afinet.c +++ b/tests/sys/netinet/socket_afinet.c @@ -337,7 +337,8 @@ ATF_TC_BODY(socket_afinet_bindany, tc) * Returns true if the bind succeeded, and false if it failed with EADDRINUSE. */ static bool -child_bind(const atf_tc_t *tc, int type, struct sockaddr *sa, int opt, bool unpriv) +child_bind(const atf_tc_t *tc, int type, struct sockaddr *sa, int opt, + bool unpriv) { const char *user; pid_t child; @@ -483,16 +484,8 @@ multibind_test(const atf_tc_t *tc, int domain, int type) /* * Multi-binding is only allowed when both * sockets have the same owner. - * - * XXX-MJ we for some reason permit this when - * binding to the unspecified address, but I - * don't think that's right */ - if (flags[flagi] && opts[opti] != 0 && - opts[opti] != SO_REUSEADDR && opti == optj) - ATF_REQUIRE(res); - else - ATF_REQUIRE(!res); + ATF_REQUIRE(!res); } ATF_REQUIRE(close(s) == 0); } @@ -517,6 +510,78 @@ ATF_TC_BODY(socket_afinet_multibind, tc) multibind_test(tc, AF_INET6, SOCK_DGRAM); } +static void +bind_connected_port_test(const atf_tc_t *tc, int domain) +{ + struct sockaddr_in sin; + struct sockaddr_in6 sin6; + struct sockaddr *sinp; + int error, sd[3], tmp; + bool res; + + /* + * Create a connected socket pair. + */ + sd[0] = socket(domain, SOCK_STREAM, 0); + ATF_REQUIRE_MSG(sd[0] >= 0, "socket failed: %s", strerror(errno)); + sd[1] = socket(domain, SOCK_STREAM, 0); + ATF_REQUIRE_MSG(sd[1] >= 0, "socket failed: %s", strerror(errno)); + if (domain == PF_INET) { + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_len = sizeof(sin); + sin.sin_addr.s_addr = htonl(INADDR_ANY); + sin.sin_port = htons(0); + sinp = (struct sockaddr *)&sin; + } else { + ATF_REQUIRE(domain == PF_INET6); + memset(&sin6, 0, sizeof(sin6)); + sin6.sin6_family = AF_INET6; + sin6.sin6_len = sizeof(sin6); + sin6.sin6_addr = in6addr_any; + sin6.sin6_port = htons(0); + sinp = (struct sockaddr *)&sin6; + } + + error = bind(sd[0], sinp, sinp->sa_len); + ATF_REQUIRE_MSG(error == 0, "bind failed: %s", strerror(errno)); + error = listen(sd[0], 1); + ATF_REQUIRE_MSG(error == 0, "listen failed: %s", strerror(errno)); + + error = getsockname(sd[0], sinp, &(socklen_t){ sinp->sa_len }); + ATF_REQUIRE_MSG(error == 0, "getsockname failed: %s", strerror(errno)); + + error = connect(sd[1], sinp, sinp->sa_len); + ATF_REQUIRE_MSG(error == 0, "connect failed: %s", strerror(errno)); + tmp = accept(sd[0], NULL, NULL); + ATF_REQUIRE_MSG(tmp >= 0, "accept failed: %s", strerror(errno)); + ATF_REQUIRE(close(sd[0]) == 0); + sd[0] = tmp; + + /* bind() should succeed even from an unprivileged user. */ + res = child_bind(tc, SOCK_STREAM, sinp, 0, false); + ATF_REQUIRE(!res); + res = child_bind(tc, SOCK_STREAM, sinp, 0, true); + ATF_REQUIRE(!res); +} + +/* + * Normally bind() prevents port stealing by a different user, even when + * SO_REUSE* are specified. However, if the port is bound by a connected + * socket, then it's fair game. + */ +ATF_TC(socket_afinet_bind_connected_port); +ATF_TC_HEAD(socket_afinet_bind_connected_port, tc) +{ + atf_tc_set_md_var(tc, "require.user", "root"); + atf_tc_set_md_var(tc, "require.config", "unprivileged_user"); +} +ATF_TC_BODY(socket_afinet_bind_connected_port, tc) +{ + bind_connected_port_test(tc, AF_INET); + bind_connected_port_test(tc, AF_INET6); +} + ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, socket_afinet); @@ -527,6 +592,7 @@ ATF_TP_ADD_TCS(tp) ATF_TP_ADD_TC(tp, socket_afinet_stream_reconnect); ATF_TP_ADD_TC(tp, socket_afinet_bindany); ATF_TP_ADD_TC(tp, socket_afinet_multibind); + ATF_TP_ADD_TC(tp, socket_afinet_bind_connected_port); return atf_no_error(); }