[PATCH 07/10] LSM: SafeSetID: rewrite userspace API to atomic updates

[Jann Horn]

On Wed, Apr 10, 2019 at 7:24 PM Kees Cook <keescook at chromium.org> wrote:
> On Wed, Apr 10, 2019 at 9:56 AM Micah Morton <mortonm at chromium.org> wrote:
> > From: Jann Horn <jannh at google.com>
> >
> > The current API of the SafeSetID LSM uses one write() per rule, and applies
> > each written rule instantly. This has several downsides:
> >
> >  - While a policy is being loaded, once a single parent-child pair has been
> >    loaded, the parent is restricted to that specific child, even if
> >    subsequent rules would allow transitions to other child UIDs. This means
> >    that during policy loading, set*uid() can randomly fail.
> >  - To replace the policy without rebooting, it is necessary to first flush
> >    all old rules. This creates a time window in which no constraints are
> >    placed on the use of CAP_SETUID.
> >  - If we want to perform sanity checks on the final policy, this requires
> >    that the policy isn't constructed in a piecemeal fashion without telling
> >    the kernel when it's done.
> >
> > Other kernel APIs - including things like the userns code and netfilter -
> > avoid this problem by performing updates atomically. Luckily, SafeSetID
> > hasn't landed in a stable (upstream) release yet, so maybe it's not too
> > late to completely change the API.
> >
> > The new API for SafeSetID is: If you want to change the policy, open
> > "safesetid/whitelist_policy" and write the entire policy,
> > newline-delimited, in there.
>
> So the entire policy is expected to be sent in a single write() call?
>
> open()
> write(policy1)
> write(policy2)
> close()
>
> means only policy2 is active?

No; if you do that, the first write() sets policy1, and the second
write() fails with -EINVAL because of the "if (*ppos != 0) return
-EINVAL;" in safesetid_file_write() (which already exists in the
current version of the LSM).

> I thought policy was meant to be built
> over time? i.e. new policy could get appended to existing?

That's what the current API does; as I've explained in the commit
message, I think that that's a bad idea.

Are you asking because you have a usecase where you actually want to
"append" rules after loading an initial policy?
If so, I think that the simplest way to do that would be to have
userspace concatenate the policies and then shove the result of that
into the kernel. Otherwise:
 - you'd need a way to distinguish between policy replacement and
appending to policy
 - to securely replace an existing policy, userspace would always have
to concatenate all the new policy fragments anyway