Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 16 May 2020 22:55:48 -0500
From:      Kyle Evans <kevans@freebsd.org>
To:        bsd-lists@bsdforge.com
Cc:        "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>,  "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>, "Julian H. Stacey" <jhs@berklix.com>
Subject:   Re: [HEADSUP] Disallowing read() of a directory fd
Message-ID:  <CACNAnaGe4jSKSLMbp317TeyrnQ-HFm4xNMYuegCgtMM4PPVS5g@mail.gmail.com>
In-Reply-To: <b737eb69aad28aff0693fdab8159b86a@udns.ultimatedns.net>
References:  <CACNAnaHW03QT2Fd_Nf8bcb8w5AE6VBkwdCQR7rcoL5T5zYrN2A@mail.gmail.com> <b737eb69aad28aff0693fdab8159b86a@udns.ultimatedns.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, May 16, 2020 at 4:42 PM Chris <bsd-lists@bsdforge.com> wrote:
>
> On Sat, 16 May 2020 13:28:32 -0500 Kyle Evans kevans@freebsd.org said
>
> > On Sat, May 16, 2020 at 12:59 PM Julian H. Stacey <jhs@berklix.com> wrote:
> > >
> > > Kyle Evans wrote:
> > > > [... snip ...]
> > > > That said, I've written a MAC policy that can live atop the current
> > > > patch to lift all of the restrictions except the sysctl needing to be
> > > > set: https://people.freebsd.org/~kevans/mac-read_dir.diff -> I could
> > > > even be convinced fairly easily to commit it, if you'd find that
> > > > acceptable. The policy ends up looking generically useful, as you can
> > > > lift just the jail root restriction or you can allow any user to cat a
> > > > directory.
> > > >
> > > > Thanks,
> > > >
> > > > Kyle Evans
> > >
> > > Thanks,
> > > It's good if its all sysctl without reboot, (taking (phk's I recall) point
> > > about an fs not surviving a reboot)
> > >
> >
> > Yup- assuming you're not in the perhaps rare situation where you both
> > need this *and* you can't get the kmod loaded, it's all sysctl-based.
> >
> > > It sounds useful, if it allows 3 or is that more ? way choice between eg
> > > {old v. new} x { root v. non root } x { inside a jail v. outside } = 8 ?
> > >
> >
> > It's not quite that flexible because this is harder to capture, it
> > allows three different possibilities:
> >
> > - New behavior
> > - Old behavior
> > - Middle ground, where unprivileged users can't `cat .` but jail root can
> >
> > #3 was thrown in because I suspect someone somewhere may not
> > necessarily care for preservation of any old users' capability to do
> > this, but for those systems where the previously cited 'jail root is
> > root' is true (since I think we agree that that can be the case, but
> > often isn't).
> >
> > > If all of that, I guess we'd just be down to a relaxed consideration about
> > > what default mode was for now & later.
> > >
> > > If there was change there, we'd need to check what policy is about giving
> > > advance notice of changes in RELNOTES.
> > >
> > > If RELNOTES required long notice than wanted , that could be worked round
> > > easily by implementing code, & merely issuing notice that defaults would
> > > change to new policy later at releasese x.y.
> > >
> >
> > Here's how this breaks down; these steps haven't been included in the
> > review thus far because I often just assume that it's clear this will
> > happen:
> >
> > - UPDATING: Users of 13.0-CURRENT will receive notice via UPDATING
> > that this is changing, along with mention of the MAC policy to return
> > to the legacy behavior. Some bumps of this nature are to be expected
> > amongst -CURRENT, and this is how we communicate them to those
> > following along at home.
> >
> > - RELNOTES: This will definitely be included in the 13.0 relnotes. I'm
> > tentatively planning to MFC some of the functionality
> > (security.bsd.allow_read_dir but with a default of 1 to preserve
> > branch behavior) to allow users of stable/12 to turn it off and get
> > the 13.0 behavior earlier if they want or see if it breaks any of
> > their stuff, which I will try and use as a vector to get the future
> > default change into the 12.2 release notes as well so that there's
> > plenty of advance notice. I suspect re@ will happily include it, given
> > the history.
> >
> > > I took a quick glance at
> > > https://people.freebsd.org/~kevans/mac-read_dir.diff but I'm sorry
> > > loads of real life distraction h These kinds of bumps are to be expectedere.
> > > I'm sure others will want t
> > > read it. Thanks for working hard to cater for all cases ! :-)
> > >
> >
> > This is fine. =-) Honestly, I have really been trying my best to
> > optimize the outcome for all of our users. I do fully believe at this
> > point in the discussion that the majority of our userbase is best
> > served by changing the default behavior here, and I want to be able to
> > do so without burning community relations.
> >
> > My suspicion is that those of you that do make use of this, do so
> > infrequently and would happily or maybe even usually run a system
> > with:
> >
> > root@viper:~# cat /boot/loader.conf
> > # Of course, this could instead be baked into your kernel config
> > mac_read_dir_load="YES"
> >
> > root@viper:~# cat /etc/sysctl.conf
> > security.mac.read_dir.jail_root=1
> > # The following perhaps even turned off or commented out to serve just
> > as a reminder, until you actually need it
> > security.bsd.allow_read_dir=1
> >
> > My working assumption being that you don't often do this as any old
> > unprivileged user, but might be doing so as jail root. Of course,
> > `security.mac.read_dir.jail_root=1` might instead be
> > `security.mac.read_dir.all_users=1` to fully follow the existing
> > behavior.
> >
> > Thanks,
> No. Thank *you*, Kyle. :-)
> I just want to take this opportunity to thank you for all the work you
> put into this -- both the semi-anticipated code, and perhaps more
> significantly, the _social_ work that went into getting this concept into
> a usable/functional state.
>
> While I have only glossed over your diff, and therefore can't (yet)
> reasonably evaluate it. Do you anticipate any impact/changes to overall
> performance?
>

MAC has its own performance issues, but I think mjg's put some work
into mitigating that since mac_ntpd became commonplace. I wouldn't
otherwise anticipate any real impact for anything but perhaps
micro-benchmarks. Most requests coming into mac_read_dir will
immediately return because it only handles the new privilege for
read(2) of a dirfd. That specific path (read of a dirfd) has become
elongated now that we need to check creds out in the MAC module and it
bypasses the superuser policy if we're in a jail, so high volumes of
directory read(2) might notice some kind of impact, but none of the
scenarios that have been presented to me even qualify for medium
volume and I would find it fairly unlikely if any high-volume scenario
does exist.

> In closing; some of here have been hacking on this code since before Net/1,
> and are fairly sensitive about some new-comer messing with our UNIX
> heritage. I look back fondly remembering waiting for tubes to warm up, and
> threading/spooling up tapes. I don't remember being too fond of having to
> wait for someone elses job to finish, so I could use the damn thing. ;-)
>

Sure, and I completely respect that. I would suspect that most of the
people that I've been discussing this with have been hacking on
systems since before I was even born (Net/1 appears to complete
predate me, for example, by a couple of years), which is another
reason why I'm willing to go the extra mile to make sure that it's a
smooth transition. My goal isn't to sour everyone's cheerios, create
heartburn, or obstruct people from just doing what they need to do
periodically; my goal is to try and improve the user
experience/security for the majority. Does it add a couple more knobs
to do what's been easy for 35+ years? Yes, unfortunately, but the real
need seems to be either infrequent enough that this isn't a major
problem or it will just get thrown in /boot/loader.conf (or even
embedded into kernel configs) and /etc/sysctl.conf to be forgotten
about.

Thanks,

Kyle Evans



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaGe4jSKSLMbp317TeyrnQ-HFm4xNMYuegCgtMM4PPVS5g>