Skip to content

Commit b119ef0

Browse files
committed
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: dfdcada ("Add the new kernel-mode NFS Lock Manager.") MFC after: 2 days Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D52962
1 parent f7c4f80 commit b119ef0

File tree

1 file changed

+23
-10
lines changed

1 file changed

+23
-10
lines changed

sys/rpc/authunix_prot.c

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@
5050

5151
#include <rpc/rpc_com.h>
5252

53-
/* gids compose part of a credential; there may not be more than 16 of them */
54-
#define NGRPS 16
55-
5653
/*
5754
* XDR for unix authentication parameters.
5855
*/
@@ -65,13 +62,10 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
6562
char hostbuf[MAXHOSTNAMELEN];
6663

6764
if (xdrs->x_op == XDR_ENCODE) {
68-
/*
69-
* Restrict name length to 255 according to RFC 1057.
70-
*/
7165
getcredhostname(NULL, hostbuf, sizeof(hostbuf));
7266
namelen = strlen(hostbuf);
73-
if (namelen > 255)
74-
namelen = 255;
67+
if (namelen > AUTH_SYS_MAX_HOSTNAME)
68+
namelen = AUTH_SYS_MAX_HOSTNAME;
7569
} else {
7670
namelen = 0;
7771
}
@@ -87,6 +81,8 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
8781
if (!xdr_opaque(xdrs, hostbuf, namelen))
8882
return (FALSE);
8983
} else {
84+
if (namelen > AUTH_SYS_MAX_HOSTNAME)
85+
return (FALSE);
9086
xdr_setpos(xdrs, xdr_getpos(xdrs) + RNDUP(namelen));
9187
}
9288

@@ -112,13 +108,30 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
112108
*/
113109
MPASS(cred->cr_ngroups <= XU_NGROUPS);
114110
supp_ngroups = cred->cr_ngroups - 1;
115-
if (supp_ngroups > NGRPS)
116-
supp_ngroups = NGRPS;
111+
if (supp_ngroups > AUTH_SYS_MAX_GROUPS)
112+
/* With current values, this should never execute. */
113+
supp_ngroups = AUTH_SYS_MAX_GROUPS;
117114
}
118115

119116
if (!xdr_uint32_t(xdrs, &supp_ngroups))
120117
return (FALSE);
121118

119+
/*
120+
* Because we cannot store more than XU_NGROUPS in total (16 at time of
121+
* this writing), for now we choose to be strict with respect to RFC
122+
* 5531's maximum number of supplementary groups (AUTH_SYS_MAX_GROUPS).
123+
* That would also be an accidental DoS prevention measure if the
124+
* request handling code didn't try to reassemble it in full without any
125+
* size limits. Although AUTH_SYS_MAX_GROUPS and XU_NGROUPS are equal,
126+
* since the latter includes the "effective" GID, we cannot store the
127+
* last group of a message with exactly AUTH_SYS_MAX_GROUPS
128+
* supplementary groups. We accept such messages so as not to violate
129+
* the protocol, silently dropping the last group on the floor.
130+
*/
131+
132+
if (xdrs->x_op != XDR_ENCODE && supp_ngroups > AUTH_SYS_MAX_GROUPS)
133+
return (FALSE);
134+
122135
junk = 0;
123136
for (i = 0; i < supp_ngroups; ++i)
124137
if (!xdr_uint32_t(xdrs, i < XU_NGROUPS - 1 ?

0 commit comments

Comments
 (0)