Page MenuHomeFreeBSD

NFS users: Do not pass effective GID in group list, fill all fields
Needs ReviewPublic

Authored by olce on Aug 29 2025, 11:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 9, 10:38 PM
Unknown Object (File)
Wed, Oct 8, 8:47 PM
Unknown Object (File)
Fri, Oct 3, 9:42 AM
Unknown Object (File)
Sat, Sep 27, 3:19 AM
Unknown Object (File)
Fri, Sep 26, 6:28 AM
Unknown Object (File)
Wed, Sep 24, 5:36 PM
Unknown Object (File)
Tue, Sep 23, 2:42 PM
Unknown Object (File)
Thu, Sep 18, 6:40 PM

Details

Reviewers
kevans
rmacklem
Summary

No functional change (intended).

This commit is a cleanup and defense-in-depth change that:

  1. Removes the effective GID from the group list passed by nfsuserd(8) to the NFS server via nfssvc(NFSSVC_IDNAME), as this information is in theory redundant since there is also a 'nid_gid' field in 'struct nfsd_idargs'.
  2. Ensure nfsuserd(8) effectively fills up the 'nid_gid' correctly in this case.
  3. Ensure nfsuserd(8) always initializes all fields in 'nid_gid' to sensible default values, to protect for kernel-side changes that would start using some of them.
  4. Ensure that the default UID and GID obtained by the resolution of "nouser" and "nogroup" are always used as placeholders, instead of the default values (which, while here, have been replaced by the usual macros).

For point 1., now that the passed group list can be empty (if there are
no supplementary groups), change detection of whether group information
was filled in nfssvc_idname() by checking if 'nid_gid' is the default
group (which implied doing point 4.).

Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 66676
Build 63559: arc lint + arc unit

Event Timeline

olce requested review of this revision.Aug 29 2025, 11:04 PM

I'll admit I do not understand what your recent gid
changes are, so I will resign as a reviewer.

However, I will note that "effective gid" has nothing
to do with the nfsuserd daemon.
Here's what the nfsuserd daemon does:

  • It maps between a group name and gid. (Such as "operator" vs 5.) This is a single gid# from the group database. These mappings occur on both NFSv4 client and NFSv4 server, since it is the name "operator" that goes on-the-wire for Getattr and Setattr.

Now, the other thing the nfsuserd daemon can
do (if the -manage-gids command line is specified)
is map a client uid (in the RPC header unless it is
a Kerberos mount) to a set of gids for that uid.
(It first gets the gid from the passwd entry and
then gets the list of additional gids from the
group database. Since it knows nothing about
whether or not a process on the client is running
as a different "effective gid", the server is not
affected by this. This option is a work-around
for the 16 additional groups (for a total of 17)
that can be put in the RPC header for AUTH_SYS
(any non-Kerberos mount).

So, in summary, it uses the password and group
databases only and has no knowledge of anything
like a setegid() having been done on the client process.

Btw, the same is true for Kerberos mounts. The uid and
gid list is generated from the password and group
databases by the gssd daemon using the principal
name (normally a user name like "rmacklem" that is
looked up in the password database).
(Again, the gssd daemon knows nothing about a
setegid() that might have been done on the process
on the client.)

NFS is not (and will never be) a POSIX complaint
file system. The client simply does its best to
"pretend" it is POSIX.

For this example, I think the only need to to ensure
that all the gids (the one in the password database+
all of the additional groups in the group database)
are provided for the "-manage-gids" option.

I'll admit I do not understand what your recent gid
changes are, so I will resign as a reviewer.

We (@kevans and I) have changed:

  1. Where the effective GID is stored in struct ucred: Now in a separate field named cr_gid, and no more as cr_groups[0]; cr_groups[] now only contains supplementary groups. This split was done very long ago in NetBSD (even before the OpenBSD fork), and is also how Linux works. It is the logical thing to do, as the effective GID may be changed by setuid programs, including those not owned by root, whereas supplementary groups are immutable (except to root of course). That simplifies and clarifies kernel code (with the aim of making it obviously correct).
  2. The contract of setgroups(2)/getgroups(2): They now only set or retrieve the supplementary groups, and no more the effective GID. This is to be consistent with other systems (*BSD and Linux do that, except DragonflyBSD that inherited from us). Without this change, "portable" programs not written with enough care (as POSIX leaves that open) generally expect the other systems' behavior, potentially causing security problems (e.g., setting the effective GID by mistake; @kevans recently uncovered another such bug in sshd).

However, I will note that "effective gid" has nothing
to do with the nfsuserd daemon.

(snip)

Thanks, I'm indeed aware of all that.

So, in summary, it uses the password and group
databases only and has no knowledge of anything
like a setegid() having been done on the client process.

By the very fact nfsuserd uses the password and group databases, it *does* a distinction between the "primary" (/"effective") group and the supplementary ones: The first comes from the password database, the second come from the group database.

It's true that the server doesn't have knowledge of the client using setegid() (and doesn't care), but that is in reality independent and doesn't change the fact in the previous paragraph.

That said, we (@kevans and I) still have to have a related discussion concerning the behavior of initgroups() (should the primary group specified in the password database be registered also as a supplementary one?).

For this example, I think the only need to to ensure
that all the gids (the one in the password database+
all of the additional groups in the group database)
are provided for the "-manage-gids" option.

That's the basic functional requirement, and it is still true. This change's concen is to bring consistency in which fields of the message structure groups are passed, allowing to ensure that at least one group always is (as struct nfsid_args already has a nid_gid field; struct ucred in the kernel needs at least the effective GID). @rmacklem: Please speak up if you object to that part. I've also wanted to ensure that all fields of the structure are filled, even those irrelevant to the current request (they are initialized to default values).