Message-ID: <CAHC9VhQY6H4JxOvSYWk2cpH8E3LYeOkMP_ay+ih+ULKKdeob=Q@mail.gmail.com>
Date:   Tue, 9 Aug 2022 12:47:03 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Frederick Lawler <fred@...udflare.com>, kpsingh@...nel.org,
        revest@...omium.org, jackmanb@...omium.org, ast@...nel.org,
        daniel@...earbox.net, andrii@...nel.org, kafai@...com,
        songliubraving@...com, yhs@...com, john.fastabend@...il.com,
        jmorris@...ei.org, serge@...lyn.com,
        stephen.smalley.work@...il.com, eparis@...isplace.org,
        shuah@...nel.org, brauner@...nel.org, casey@...aufler-ca.com,
        bpf@...r.kernel.org, linux-security-module@...r.kernel.org,
        selinux@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        kernel-team@...udflare.com, cgzones@...glemail.com,
        karl@...badwolfsecurity.com
Subject: Re: [PATCH v4 0/4] Introduce security_create_user_ns()

On Tue, Aug 9, 2022 at 12:08 PM Eric W. Biederman <ebiederm@...ssion.com> wrote:
> Paul Moore <paul@...l-moore.com> writes:
> > On Mon, Aug 8, 2022 at 3:43 PM Eric W. Biederman <ebiederm@...ssion.com> wrote:
> >> "Eric W. Biederman" <ebiederm@...ssion.com> writes:
> >> > Paul Moore <paul@...l-moore.com> writes:
> >> >
> >> >>> I did provide constructive feedback.  My feedback to his problem
> >> >>> was to address the real problem of bugs in the kernel.
> >> >>
> >> >> We've heard from several people who have use cases which require
> >> >> adding LSM-level access controls and observability to user namespace
> >> >> creation.  This is the problem we are trying to solve here; if you do
> >> >> not like the approach proposed in this patchset please suggest another
> >> >> implementation that allows LSMs visibility into user namespace
> >> >> creation.
> >> >
> >> > Please stop, ignoring my feedback, not detailing what problem or
> >> > problems you are actually trying to be solved, and threatening to merge
> >> > code into files that I maintain that has the express purpose of breaking
> >> > my users.
> >> >
> >> > You just artificially constrained the problems, so that no other
> >> > solution is acceptable.  On that basis alone I am object to this whole
> >> > approach to steam roll over me and my code.
> >>
> >> If you want an example of what kind of harm it can cause to introduce a
> >> failure where no failure was before I invite you to look at what
> >> happened with sendmail when setuid was modified to fail, when changing
> >> the user of a process would cause RLIMIT_NPROC to be exceeded.
> >
> > I think we are all familiar with the sendmail capabilities bug and the
> > others like it, but using that as an excuse to block additional access
> > controls seems very weak.  The Linux Kernel is very different from
> > when the sendmail bug hit (what was that, ~20 years ago?), with
> > advancements in capabilities and other discretionary controls, as well
> > as mandatory access controls which have enabled Linux to be certified
> > through a number of third party security evaluations.
>
> If you are familiar with scenarios like that then why is there not
> being due diligence performed to ensure that userspace won't break?

What level of due diligence would satisfy you Eric?  This feels very
much like an unbounded ask that can be used to permanently block any
effort to add any form of additional access control around user
namespace creation.  If that isn't the case, and this request is being
made in good faith, please elaborate on what you believe would be
sufficient analysis of this patch?

Personally, the fact that the fork()/clone() variants and the
unshare() syscall all can fail and return error codes to userspace
seems like a reasonable level of safety.  If userspace is currently
ignoring the return values of fork/clone/unshare that is a problem
independent of this patchset.  Even looking at the existing
create_user_ns() function shows several cases where the user namespace
code can forcibly error out due to access controls, memory pressure,
etc.  I also see that additional restrictions were put on user
namespace creation after it was introduced, e.g. the chroot
restriction; I'm assuming that didn't break "your" users?  If you can
describe the analysis you did on that change, perhaps we can do the
same for this patchset and call it sufficient, yes?

> >> I am not arguing that what you are proposing is that bad but unexpected
> >> failures cause real problems, and at a minimum that needs a better
> >> response than: "There is at least one user that wants a failure here".
> >
> > Let me fix that for you: "There are multiple users who want to have
> > better visibility and access control for user namespace creation."
>
> Visibility sure.  Design a proper hook for that.  All the proposed hook
> can do is print an audit message.  It can't allocate or manage any state
> as there is not the corresponding hook when a user namespace is freed.
> So the proposed hook is not appropriate for increasing visibility.

Auditing very much increases visibility.  Look at what the BPF LSM can
do, observability is one of its primary goals.

> Access control.  Not a chance unless it is carefully designed and
> reviewed.

See the above.  The relevant syscalls already have the risk of
failure, if userspace is assuming fork/clone/unshare/etc. will never
fail then that application is broken independent of this discussion.

-- 
paul-moore.com

Powered by blists - more mailing lists