nfs: Default to nfs_reserved_port_only="YES"

[markj]

Summary

This setting causes the NFS server to check that all RPCs are sent from
a privileged (<= 1023) port, rejecting those that are not. This
slightly raises the bar for a user with network access to an
unauthenticated NFS server to access exported NFS filesystems.

Users that use traditional NFS clients (e.g., those provided by FreeBSD
or Linux) should not see any difference, assuming that unprivileged
filesystem mounting is disallowed.

Note that the setting is per-VNET, so may be overridden in VNET jails
without affecting the rest of the system.

Discussed with: freebsd-arch@
Sponsored by: The FreeBSD Foundation

[rmacklem]

lgtm. I cannot recall if there is any man page change
needed for this?

[bz]

In D44906#1023836, @rmacklem wrote:

lgtm. I cannot recall if there is any man page change
needed for this?

man 5 rc.conf doesn't seem to specify a default; so at least that is fine.

[emaste]

Should we change the in-kernel default as well? It will normally be overridden by the rc.conf setting so it doesn't have a practical impact but probably good for consistency.

[bz]

In D44906#1023886, @emaste wrote:

Should we change the in-kernel default as well? It will normally be overridden by the rc.conf setting so it doesn't have a practical impact but probably good for consistency.

Would NFS Root be affected by that? Hmm it's a tunable so less of a problem in case people do have trouble. That should probably be documented somewhere then though..

[markj]

In D44906#1023888, @bz wrote:

In D44906#1023886, @emaste wrote:

Should we change the in-kernel default as well? It will normally be overridden by the rc.conf setting so it doesn't have a practical impact but probably good for consistency.

Would NFS Root be affected by that? Hmm it's a tunable so less of a problem in case people do have trouble.

This is a server setting, so I wouldn't expect so.

[olce]

There's also NFS_REQRSVPORT, but it is unrelated I guess?

In D44906#1023886, @emaste wrote:

Should we change the in-kernel default as well? It will normally be overridden by the rc.conf setting so it doesn't have a practical impact but probably good for consistency.

+1

[bz]

In D44906#1023889, @markj wrote:

In D44906#1023888, @bz wrote:

In D44906#1023886, @emaste wrote:

Should we change the in-kernel default as well? It will normally be overridden by the rc.conf setting so it doesn't have a practical impact but probably good for consistency.

Would NFS Root be affected by that? Hmm it's a tunable so less of a problem in case people do have trouble.

This is a server setting, so I wouldn't expect so.

Sorry for the noise. 3:20AM and just had a NFS Root client "hang" for the 5min timeout. Time to stop for yestoday.

[markj]

In D44906#1023891, @olce wrote:

There's also NFS_REQRSVPORT, but it is unrelated I guess?

Oh hmm, not unrelated, but at a glance I think it is superseded by the privport check. That is, all code paths which lead there first have to go through the privport check. But I could be wrong about this.

[olce]

In D44906#1023893, @markj wrote:

Oh hmm, not unrelated, but at a glance I think it is superseded by the privport check. That is, all code paths which lead there first have to go through the privport check. But I could be wrong about this.

It feels like NFS_REQRSVPORT is older but apparently it was introduced by the same (initial) commit as the if (nfs_privport ... check (9ec7b004d0edb). Maybe replacing NFS_REQRSVPORT by a runtime check for nfs_privport would make sense, but I don't really know this code. It may be instead that the code under NFS_REQRSVPORT is redundant, I'm not sure either. In any case, this would be a separate change.

[rmacklem]

In D44906#1023894, @olce wrote:

In D44906#1023893, @markj wrote:

Oh hmm, not unrelated, but at a glance I think it is superseded by the privport check. That is, all code paths which lead there first have to go through the privport check. But I could be wrong about this.

It feels like NFS_REQRSVPORT is older but apparently it was introduced by the same (initial) commit as the if (nfs_privport ... check (9ec7b004d0edb). Maybe replacing NFS_REQRSVPORT by a runtime check for nfs_privport would make sense, but I don't really know this code. It may be instead that the code under NFS_REQRSVPORT is redundant, I'm not sure either. In any case, this would be a separate change.

Wow, this one is embarrassing... 25+ years ago I was using OpenBSD for various things and
put this code in (OpenBSD didn't have a port# check). Then, 25 years ago, I did the first
rendition of NFSv4 starting from that OpenBSD code. The comment comes from the fact
that the NFSv4 working group was anti-reserved port# in those days.

It's cruft that has never been built for FreeBSD. I can get rid of it (or Mark can).

[markj]

In D44906#1023898, @rmacklem wrote:

In D44906#1023894, @olce wrote:

In D44906#1023893, @markj wrote:

Oh hmm, not unrelated, but at a glance I think it is superseded by the privport check. That is, all code paths which lead there first have to go through the privport check. But I could be wrong about this.

It feels like NFS_REQRSVPORT is older but apparently it was introduced by the same (initial) commit as the if (nfs_privport ... check (9ec7b004d0edb). Maybe replacing NFS_REQRSVPORT by a runtime check for nfs_privport would make sense, but I don't really know this code. It may be instead that the code under NFS_REQRSVPORT is redundant, I'm not sure either. In any case, this would be a separate change.

Wow, this one is embarrassing... 25+ years ago I was using OpenBSD for various things and
put this code in (OpenBSD didn't have a port# check). Then, 25 years ago, I did the first
rendition of NFSv4 starting from that OpenBSD code. The comment comes from the fact
that the NFSv4 working group was anti-reserved port# in those days.

It's cruft that has never been built for FreeBSD. I can get rid of it (or Mark can).

I'll leave it to you. :)

[markj]

Initialize the sysctl to 1 as well.

[rmacklem]