Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 16 Oct 2025 16:58:22 GMT
From:      Olivier Certner <olce@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 34fc20503f04 - stable/15 - sys/rpc: UNIX auth: Use AUTH_SYS_MAX_{GROUPS,HOSTNAME} as limits (1/2)
Message-ID:  <202510161658.59GGwMPh055803@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
The branch stable/15 has been updated by olce:

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

commit 34fc20503f04e3c035844f4bfa8eb72964ccbf68
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2025-10-07 08:46:56 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2025-10-16 16:57:45 +0000

    sys/rpc: UNIX auth: Use AUTH_SYS_MAX_{GROUPS,HOSTNAME} as limits (1/2)
    
    Consistently with the XDR_INLINE() variant of xdr_authunix_parms()
    (_svcauth_unix() in 'svc_auth_unix.c'), reject messages with credentials
    having a machine name length in excess of AUTH_SYS_MAX_HOSTNAME or more
    than AUTH_SYS_MAX_GROUPS supplementary groups, which do not conform to
    RFC 5531.  This is done mainly because we cannot store excess groups
    anyway, even if at odds with the robustness principle ("be liberal in
    what you accept").
    
    While here, make sure the current code is immune to AUTH_SYS_MAX_GROUPS
    changing value (in future RFCs?) even if that seems improbable.
    
    Reviewed by:    rmacklem
    Fixes:          dfdcada31e79 ("Add the new kernel-mode NFS Lock Manager.")
    MFC after:      2 days
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D52962
    
    (cherry picked from commit b119ef0f6a81eb32b0e1cd0075cec499543e7ddd)
---
 sys/rpc/authunix_prot.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/sys/rpc/authunix_prot.c b/sys/rpc/authunix_prot.c
index 89f0ab3ed44e..c1a9f90bbe28 100644
--- a/sys/rpc/authunix_prot.c
+++ b/sys/rpc/authunix_prot.c
@@ -50,9 +50,6 @@
 
 #include <rpc/rpc_com.h>
 
-/* gids compose part of a credential; there may not be more than 16 of them */
-#define NGRPS 16
-
 /*
  * XDR for unix authentication parameters.
  */
@@ -65,13 +62,10 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
 	char hostbuf[MAXHOSTNAMELEN];
 
 	if (xdrs->x_op == XDR_ENCODE) {
-		/*
-		 * Restrict name length to 255 according to RFC 1057.
-		 */
 		getcredhostname(NULL, hostbuf, sizeof(hostbuf));
 		namelen = strlen(hostbuf);
-		if (namelen > 255)
-			namelen = 255;
+		if (namelen > AUTH_SYS_MAX_HOSTNAME)
+			namelen = AUTH_SYS_MAX_HOSTNAME;
 	} else {
 		namelen = 0;
 	}
@@ -87,6 +81,8 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
 		if (!xdr_opaque(xdrs, hostbuf, namelen))
 			return (FALSE);
 	} else {
+		if (namelen > AUTH_SYS_MAX_HOSTNAME)
+			return (FALSE);
 		xdr_setpos(xdrs, xdr_getpos(xdrs) + RNDUP(namelen));
 	}
 
@@ -112,13 +108,30 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
 		 */
 		MPASS(cred->cr_ngroups <= XU_NGROUPS);
 		supp_ngroups = cred->cr_ngroups - 1;
-		if (supp_ngroups > NGRPS)
-			supp_ngroups = NGRPS;
+		if (supp_ngroups > AUTH_SYS_MAX_GROUPS)
+			/* With current values, this should never execute. */
+			supp_ngroups = AUTH_SYS_MAX_GROUPS;
 	}
 
 	if (!xdr_uint32_t(xdrs, &supp_ngroups))
 		return (FALSE);
 
+	/*
+	 * Because we cannot store more than XU_NGROUPS in total (16 at time of
+	 * this writing), for now we choose to be strict with respect to RFC
+	 * 5531's maximum number of supplementary groups (AUTH_SYS_MAX_GROUPS).
+	 * That would also be an accidental DoS prevention measure if the
+	 * request handling code didn't try to reassemble it in full without any
+	 * size limits.  Although AUTH_SYS_MAX_GROUPS and XU_NGROUPS are equal,
+	 * since the latter includes the "effective" GID, we cannot store the
+	 * last group of a message with exactly AUTH_SYS_MAX_GROUPS
+	 * supplementary groups.  We accept such messages so as not to violate
+	 * the protocol, silently dropping the last group on the floor.
+	 */
+
+	if (xdrs->x_op != XDR_ENCODE && supp_ngroups > AUTH_SYS_MAX_GROUPS)
+		return (FALSE);
+
 	junk = 0;
 	for (i = 0; i < supp_ngroups; ++i)
 		if (!xdr_uint32_t(xdrs, i < XU_NGROUPS - 1 ?

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202510161658.59GGwMPh055803>