linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Safonov <0x7f454c46@gmail.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: dancol@google.com, rdunlap@infradead.org, christian@brauner.io,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	open list <linux-kernel@vger.kernel.org>,
	Serge Hallyn <serge@hallyn.com>,
	jannh@google.com, Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	cyphar@cyphar.com, Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	Linux API <linux-api@vger.kernel.org>,
	timmurray@google.com, Kees Cook <keescook@chromium.org>,
	jengelh@inai.de, Andrei Vagin <avagin@gmail.com>
Subject: Re: [PATCH] proc: allow killing processes via file descriptors
Date: Mon, 19 Nov 2018 16:13:30 +0000	[thread overview]
Message-ID: <CAJwJo6aXQUtciO7kPSENoajV6JUqUGJxCh_0yPf5D8GfbLegzA@mail.gmail.com> (raw)
In-Reply-To: <CALCETrVscRwQG55-j1SKc2TmSb1-=5861804ojUuviNzdyDOrA@mail.gmail.com>

On Sun, 18 Nov 2018 at 18:30, Andy Lutomirski <luto@kernel.org> wrote:
> Here's my point: if we're really going to make a new API to manipulate
> processes by their fd, I think we should have at least a decent idea
> of how that API will get extended in the future.  Right now, we have
> an extremely awkward situation where opening an fd in /proc requires
> certain capabilities or uids, and using those fds often also checks
> current's capabilities, and the target process may have changed its
> own security context, including gaining privilege via SUID, SGID, or
> LSM transition rules in the mean time.  This has been a huge source of
> security bugs.  It would be nice to have a model for future APIs that
> avoids these problems.
>
> And I didn't say in my proposal that a process's identity should
> fundamentally change when it calls execve().  I'm suggesting that
> certain operations that could cause a process to gain privilege or
> otherwise require greater permission to introspect (mainly execve)
> could be handled by invalidating the new process management fds.
> Sure, if init re-execs itself, it's still PID 1, but that doesn't
> necessarily mean that:
>
> fd = process_open_management_fd(1);
> [init reexecs]
> process_do_something(fd);
>
> needs to work.
>
> >
> > > setresuid() has no effect
> > > here -- if you have W access to the process and the process calls
> > > setresuid(), you still have W access.
> >
> > Now you've created a situation in which an operation that security
> > policy previously blocked now becomes possible, invaliding previous
> > designs based on the old security invariant. That's the definition of
> > introducing a security hole.
>
> I think you're overstating your case.  To a pretty good approximation,
> setresuid() allows the caller to remove elements from the set {ruid,
> suid, euid}, unless the caller has CAP_SETUID.  If you could ptrace a
> process before it calls setresuid(), you might as well be able to
> ptrace() it after, since you could have just ptraced it and made it
> call setresuid() while still ptracing it.  Similarly, it seems like
> it's probably safe to be able to open an fd that lets you watch the
> exit status of a process, have the process call setresuid(), and still
> see the exit status.
>
> Regardless of how you feel about these issues, if you're going to add
> an API by which you open an fd, wait for a process to exit, and read
> the exit status, you need to define the conditions under which you may
> open the fd and under which you may read the exit status once you have
> the fd.  There are probably multiple valid answers, but the question
> still needs to be answered.  My POLLERR hack, aside from being ugly,
> avoids this particular issue because it merely lets you wait for
> something you already could have observed using readdir().

Beg your pardon for hijacking the thread..

I wonder how fast it would be holding a pid with another open()ed fd.
And then you need to read comm (or how you filter whom to kill).
It seems to me that procfs will be even slower with this safe-way.
But I might misunderstand the idea, excuses.

So, I just wanted to gently remind about procfs with netlink socket[1].
It seems to me that whenever you receive() pid information, you
can request some uniq 64(?) bit number and kill the process using it.
Whenever uniqueness of 64-bit number to handle pids will be a question
the netlink message might be painlessly extended to 128 or whatever.

Also, it may provide the facilities to atomically kill process say by name
by adding another field to netlink message.

Probably, if it's time to add a new API for procfs, netlink may be more
desirable.

[1]: https://lwn.net/Articles/650243/

Thanks,
             Dmitry

  parent reply	other threads:[~2018-11-19 16:13 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-18 11:17 Christian Brauner
2018-11-18 13:59 ` Daniel Colascione
2018-11-18 15:38   ` Andy Lutomirski
2018-11-18 15:53     ` Daniel Colascione
2018-11-18 16:17       ` Andy Lutomirski
2018-11-18 16:29         ` Daniel Colascione
2018-11-18 17:13           ` Andy Lutomirski
2018-11-18 17:17             ` Daniel Colascione
2018-11-18 17:43               ` Eric W. Biederman
2018-11-18 17:45                 ` Andy Lutomirski
2018-11-18 17:56                 ` Daniel Colascione
2018-11-18 16:33         ` Randy Dunlap
2018-11-18 16:48           ` Daniel Colascione
2018-11-18 17:09             ` Andy Lutomirski
2018-11-18 17:24               ` Daniel Colascione
2018-11-18 17:42                 ` Andy Lutomirski
2018-11-18 17:51                   ` Daniel Colascione
2018-11-18 18:28                     ` Andy Lutomirski
2018-11-18 18:43                       ` Daniel Colascione
2018-11-18 19:05                         ` Aleksa Sarai
2018-11-18 19:44                           ` Daniel Colascione
2018-11-18 20:15                             ` Christian Brauner
2018-11-18 20:21                               ` Daniel Colascione
2018-11-18 20:28                             ` Andy Lutomirski
2018-11-18 20:32                               ` Daniel Colascione
2018-11-19  1:43                                 ` Andy Lutomirski
2018-11-18 20:43                               ` Christian Brauner
2018-11-18 20:54                                 ` Daniel Colascione
2018-11-18 21:23                                   ` Christian Brauner
2018-11-18 21:30                                     ` Christian Brauner
2018-11-19  0:31                                       ` Daniel Colascione
2018-11-19  0:40                                         ` Christian Brauner
2018-11-19  0:09                             ` Aleksa Sarai
2018-11-19  0:53                               ` Daniel Colascione
2018-11-19  1:16                                 ` Daniel Colascione
2018-11-19 16:13                       ` Dmitry Safonov [this message]
2018-11-19 16:26                         ` [PATCH] proc: allow killing processes via file descriptors (Larger pids) Eric W. Biederman
2018-11-19 16:27                         ` [PATCH] proc: allow killing processes via file descriptors Daniel Colascione
2018-11-19 20:21                           ` Aleksa Sarai
2018-11-19  2:47                   ` Al Viro
2018-11-19  3:01                     ` Andy Lutomirski
2018-11-18 17:41     ` Christian Brauner
2018-11-18 17:44       ` Andy Lutomirski
2018-11-18 18:07       ` Daniel Colascione
2018-11-18 18:15         ` Andy Lutomirski
2018-11-18 18:31           ` Daniel Colascione
2018-11-18 19:24         ` Christian Brauner
2018-11-19  0:08         ` Aleksa Sarai
2018-11-19  1:14           ` Daniel Colascione
2018-11-18 16:03 ` Daniel Colascione
2018-11-19 10:56 ` kbuild test robot
2018-11-19 14:15 ` David Laight
2018-11-19 15:49 ` Dave Martin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJwJo6aXQUtciO7kPSENoajV6JUqUGJxCh_0yPf5D8GfbLegzA@mail.gmail.com \
    --to=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=avagin@gmail.com \
    --cc=christian@brauner.io \
    --cc=cyphar@cyphar.com \
    --cc=dancol@google.com \
    --cc=ebiederm@xmission.com \
    --cc=jannh@google.com \
    --cc=jengelh@inai.de \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=oleg@redhat.com \
    --cc=rdunlap@infradead.org \
    --cc=serge@hallyn.com \
    --cc=timmurray@google.com \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH] proc: allow killing processes via file descriptors' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox