Re: RFC: fixing PR#282995

On Thu, Nov 28, 2024 at 7:49 AM <rb@gid.co.uk> wrote:
>
>
>
> > On 28 Nov 2024, at 15:04, Rick Macklem <rick.macklem@gmail.com> wrote:
> >
> > On Thu, Nov 28, 2024 at 4:36 AM Bob Bishop <rb@gid.co.uk> wrote:
> >>
> >> Hi,
> >>
> >>> On 27 Nov 2024, at 21:56, Rick Macklem <rick.macklem@gmail.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>> PR#282995 reports that the "-alldirs" export option is broken,
> >>> since it allows an export where the directory path is not a mount point.
> >>>
> >>> I'll admit I did not recall this semantic for -alldirs and I now see it is only
> >>> documented in the "Examples" section of exports(5).
> >>>
> >>> Looking at the code, it appears this was broken between releng1 and
> >>> releng2.0 (about 30years ago) when the call to mount(2) in mountd.c
> >>> was changed from using the path in the exports line to using f_mntonname.
> >>> (The check for "it is a mount point" depended on mount(2) failing because
> >>> the path was not a mount point.)
> >>>
> >>> I do believe the semantic is a useful one,
> >>
> >> Why?
> > Suppose /cdrom is where a CD is mounted sometimes.
> > If this is exported when the CD is not mounted, it will export
> > the "/" file system.
> > --> This export is probably not what the sysadmin wanted.
> >      mountd does now generate a warning about this, which
> >      was how the exporter spotted the bug.
> > For example (the line in /etc/exports):
> > /cdrom -alldirs
> > will export "/" to "the world" if /cdrom is not mounted.
>
> I will agree that is undesirable.
>
> > The example in the exports(5) man page claims the export
> > line will fail, so "/" would not be exported. This seems like
> > a better semantic to me.
>
> It’s certainly safer but will cause client mounts to fail as well. It would be nicer to export an empty directory.
A couple of comments here (mostly for everyone else reading this).
1 - From a security point of view, exports apply to server file
systems, not directories.
     (They are stored in the kernel on file system mount points.)
2 - The "administrative controls" which allow mounts for only certain
directories within a server file
      system is not a significant security tool. They can be easily
circumvented and only work for
      NFSv3 and not NFSv4, since they only affect the NFSv3 sideband
Mount protocol.
3 - The whole purpose of exports(5) is to make undesirable NFS client
mounts fail.

Personally, I wish these "administrative controls" did not exist. They
were created way back in
the mid-1980s so that BSD (CSRG) could be "Sun compatible". When I got
involved in NFS
on FreeBSD I tried to convince the "collective" to get rid of them,
but there was pushback,
due to that being a POLA violation.
--> As such, they still exist. They still cause confusion w.r.t. what
is exported and I, personally,
     recommend they be avoided when a exports(5) file is created in
order to minimize the
     risk of exporting some file system that is undesirable from a
security perspective.

rick
ps: Thanks for the comments. I am proceeding with (C).

>
> > rick
> >
> >>
> >>> although making it that way
> >>> after 30years might be construed as a POLA violation?
> >>>
> >>> So, what do others think I should do with this?
> >>> (A) - Patch mountd to enforce the "must be a mount point when -alldirs
> >>>       is specified, plus update exports(5) to state this semantic clearly.
> >>> or
> >>> (B) - Patch mountd so that it enforces "must be a mount point when -alldirs
> >>>       is specified, but only enabled via a new mountd command line option.
> >>>       --> ie. Leave the default as not enforced, but allow enforcement based
> >>>                 on a new mountd option.
> >>>            - Document this in both exports(5) and mountd(8).
> >>> or
> >>> ???
> >>
> >> (C) - Patch mountd so that it enforces "must be a mount point when -alldirs
> >>       is specified, but provide a new mountd command line option to restore the old behaviour.
> >>       --> ie. Default as enforced, but allow an override based  on a new mountd option.
> >>            - Document this in both exports(5) and mountd(8).
> >>
> >> I think that (A) is too POLA-unfriendly.
> >>
> >>> Thanks in advance for your comments, rick
> >>>
> >>
> >> --
> >> Bob Bishop
> >> rb@gid.co.uk
> >>
> >>
> >>
> >>
> >
>
> --
> Bob Bishop
> rb@gid.co.uk
>
>
>
>