Date: Thu, 28 Nov 2024 11:55:07 -0800 From: Rick Macklem <rick.macklem@gmail.com> To: rb@gid.co.uk Cc: FreeBSD CURRENT <freebsd-current@freebsd.org>, Michael Proto <mike@jellydonut.org> Subject: Re: RFC: fixing PR#282995 Message-ID: <CAM5tNy5YzxPZSV1JKuro4awBwaB9hrb5UCADAGKC4Q4sukgHdQ@mail.gmail.com> In-Reply-To: <4FEA762C-5E0F-4D3F-82D9-DF93CF1BC1F5@gid.co.uk> References: <CAM5tNy4YHAPUgZddok1U3Oz3vFB26-FC5M6Ocwx7bZhWm%2BUX4Q@mail.gmail.com> <F949CCDA-D424-4F83-9A0A-EE8ED7C54A10@gid.co.uk> <CAM5tNy6t_nb4pPTr1N=0%2B7jh6PHDuRz2V%2BJm575TCFpZhjje0A@mail.gmail.com> <4FEA762C-5E0F-4D3F-82D9-DF93CF1BC1F5@gid.co.uk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Nov 28, 2024 at 7:49=E2=80=AFAM <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=E2=80=AFAM 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 poi=
nt.
> >>>
> >>> 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_mntonn=
ame.
> >>> (The check for "it is a mount point" depended on mount(2) failing bec=
ause
> >>> 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=E2=80=99s 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 -alldir=
s
> >>> is specified, plus update exports(5) to state this semantic cle=
arly.
> >>> or
> >>> (B) - Patch mountd so that it enforces "must be a mount point when -a=
lldirs
> >>> is specified, but only enabled via a new mountd command line op=
tion.
> >>> --> ie. Leave the default as not enforced, but allow enforcemen=
t 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 -al=
ldirs
> >> is specified, but provide a new mountd command line option to re=
store the old behaviour.
> >> --> ie. Default as enforced, but allow an override based on a n=
ew 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
>
>
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAM5tNy5YzxPZSV1JKuro4awBwaB9hrb5UCADAGKC4Q4sukgHdQ>