olce@ reported an issue where the credentials used for mapped
user exports (for the NFS server) could have cr_ngroups == 0.
At first I thought this was a mountd bug, but he pointed out the
exports(5) manpage, which says:

Note that user: should be used to distinguish a credential containing
no groups from a complete credential for that user.
The group names may be quoted, or use backslash escaping.

As such, this is not just an allowed case, but a documented one.
(This snippet from exports(5) goes all the way back to May 1994
when the man page was imported from 4.4BSD Lite.)

Note that these credentials are not POSIX syscall ones.
They are used specifically by the NFS server for file access.
The good news is that the current main sources appear to
always funnel down into groupmember() to check this.

The not so good news is that commit 7f92e57 (Jun 20, 2009)
broke groupmember() for the case where cr_ngroups == 0,
assuming there would always be at least one group (cr_groups[0]
or cr_gid, if you prefer).

So, what should we do about this?

#1 A simple patch can be applied to groupmember() and a couple
     of places in the NFS server code, so that cr_ngroups == 0 again
     works correctly for this case.
#2 Decide that cr_ngroups == 0 should no longer be supported and
     patch accordingly.
OR ???

Personally, I am thinking that #1 should be done right away and
MFC'd to stable/14 and stable/13 so that the currently documented
behaviour is supported for FreeBSD 13 and FreeBSD14.
(To do otherwise would seem to be a POLA violation to me.)

Then, the FreeBSD community needs to decide if #2 should be done
(or document that the cr_ngroups == 0 case needs to work correctly).

Please respond with your opinion w.r.t. how to handle this.

Note that if a file with these pemissions:
rw-r----- 1 root games 409 Dec 30 2023 foo
were exported to a client with the following exports(5) line:
/home -sec=sys -maproot=1001:

Then, "root" on an NFS mounted client tried to read the file,
the attempt should fail (assuming root in not a member of games).
However, if cr_groups[0] just happened to have 13 in it (it is random
junk when cr_ngroups == 0), the read would succeed.
--> This vulnerability can be avoided by never using the syntax
      "=<one value>:" for -maproot or -mapall in /etc/exports.
Should so@ do some sort of announcement w.r.t. this?

Thanks in advance for your comments, rick
ps: Yes, I cross posted, since I wanted both developers and users
      to see this.

Reply via email to