Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Adrian Reber <areber@redhat.com>
Cc: "Casey Schaufler" <casey@schaufler-ca.com>,
	"Christian Brauner" <christian.brauner@ubuntu.com>,
	"Pavel Emelyanov" <ovzxemul@gmail.com>,
	"Oleg Nesterov" <oleg@redhat.com>,
	"Dmitry Safonov" <0x7f454c46@gmail.com>,
	"Andrei Vagin" <avagin@gmail.com>,
	"Nicolas Viennot" <Nicolas.Viennot@twosigma.com>,
	"Michał Cłapiński" <mclapinski@google.com>,
	"Kamil Yurtsever" <kyurtsever@google.com>,
	"Dirk Petersen" <dipeit@gmail.com>,
	"Christine Flood" <chf@redhat.com>,
	"Mike Rapoport" <rppt@linux.ibm.com>,
	"Radostin Stoyanov" <rstoyanov1@gmail.com>,
	"Cyrill Gorcunov" <gorcunov@openvz.org>,
	"Serge Hallyn" <serge@hallyn.com>,
	"Stephen Smalley" <stephen.smalley.work@gmail.com>,
	"Sargun Dhillon" <sargun@sargun.me>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Aaron Goidel" <acgoide@tycho.nsa.gov>,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, selinux@vger.kernel.org,
	"Eric Paris" <eparis@parisplace.org>,
	"Jann Horn" <jannh@google.com>
Subject: Re: [PATCH] capabilities: Introduce CAP_RESTORE
Date: Tue, 26 May 2020 08:59:29 -0500
Message-ID: <877dwybxvi.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20200525080541.GF104922@dcbz.redhat.com> (Adrian Reber's message of "Mon, 25 May 2020 10:05:41 +0200")

Adrian Reber <areber@redhat.com> writes:

> On Fri, May 22, 2020 at 09:40:37AM -0700, Casey Schaufler wrote:

>> What are the other blockers? Are you going to suggest additional new
>> capabilities to clear them?
>
> As mentioned somewhere else access to /proc/<pid>/map_files/ would be
> helpful. Right now I am testing with a JVM and it works without root
> just with the attached patch. Without access to /proc/<pid>/map_files/
> not everything CRIU can do will actually work, but we are a lot closer
> to what our users have been asking for.

The current permission checks on /proc/<pid>/map_files/ are simply
someone being over-cautious.

Someone needs to think through the threat landscape and figure out what
permission checks are actually needed.

Making the permission check ns_capable instead of capable is a
no-brainer.  Figuring out which user_ns to test against might be a
we bit harder.

We could probably even allow the owner of the process to open the files
but that requires someone doing the work of thinking through how
being able to opening files that you have mmaped might be a problem.

>> > There are probably a few more things guarded by CAP_SYS_ADMIN required
>> > to run checkpoint/restore as non-root,
>> 
>> If you need CAP_SYS_ADMIN anyway you're not gaining anything by
>> separating out CAP_RESTORE.
>
> No, as described we can checkpoint and restore a JVM with this patch and
> it also solves the problem the set_ns_last_pid fork() loop daemon tries
> to solve. It is not enough to support the full functionality of CRIU as
> map_files is also important, but we do not need CAP_SYS_ADMIN and
> CAP_RESTORE. Only CAP_RESTORE would be necessary.
>
> With a new capability users can enable checkpoint/restore as non-root
> without giving CRIU access to any of the other possibilities offered by
> CAP_SYS_ADMIN. Setting a PID and map_files have been introduced for CRIU
> and used to live behind CONFIG_CHECKPOINT_RESTORE. Having a capability
> for checkpoint/restore would make it easier for CRIU users to run it as
> non-root and make it very clear what is possible when giving CRIU the
> new capability. No other things would be allowed than necessary for
> checkpoint/restore. Setting a PID is most important for the restore part
> and reading map_files would be helpful during checkpoint. So it actually
> should be called CAP_CHECKPOINT_RESTORE as Christian mentioned in
> another email.

Please if one is for checkpoint and one is for restore asking for a pair
of capabilities is probably more appropriate.

>> >  but by applying this patch I can
>> > already checkpoint and restore processes as non-root. As there are
>> > already multiple workarounds I would prefer to do it correctly in the
>> > kernel to avoid that CRIU users are starting to invent more workarounds.
>> 
>> You've presented a couple of really inappropriate implementations
>> that would qualify as workarounds. But the other two are completely
>> appropriate within the system security policy. They don't "get around"
>> the problem, they use existing mechanisms as they are intended.
>
> I agree with the user namespace approach to be appropriate, but not the
> CAP_SYS_ADMIN approach as CRIU only needs a tiny subset (2 things) of
> what CAP_SYS_ADMIN allows.


If we are only talking 2 things can you please include in your patchset
a patch enabling those 2 things?

But even more than this we need a request that asks not for the least
you can possibly ask for but asks for what you need to do a good job.

I am having visions of a recurring discussion that says can we add one
more permission check to CAP_RESTORE or CAP_CHECKPOINT when they are
things we could know today.

Eric

  parent reply index

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-22  5:53 Adrian Reber
2020-05-22  7:53 ` Christian Brauner
2020-05-22 18:02   ` Andrei Vagin
2020-05-22 13:41 ` Christian Brauner
2020-05-22 16:40 ` Casey Schaufler
2020-05-23  4:27   ` Andrei Vagin
2020-05-25  2:01     ` Casey Schaufler
2020-05-25  8:05   ` Adrian Reber
2020-05-25 18:55     ` Casey Schaufler
2020-05-27 13:48       ` Adrian Reber
2020-05-27 15:57         ` Casey Schaufler
2020-05-27 16:37           ` Nicolas Viennot
2020-05-27 16:46             ` Casey Schaufler
2020-05-26 13:59     ` Eric W. Biederman [this message]
     [not found]       ` <CALKUemw0UZ67yaDwAomHh0n8QZfjd52QvgEXTJ4R3JSrQjZX9g@mail.gmail.com>
2020-05-26 19:19         ` Casey Schaufler
2020-05-26 19:51         ` Jann Horn
2020-05-27 14:14       ` Adrian Reber
2020-05-27 15:29         ` Christian Brauner
2020-05-27 18:05           ` Nicolas Viennot
2020-05-28  9:48             ` Christian Brauner
2020-06-08  2:09               ` Andrei Vagin
2020-05-25 21:53 ` Jann Horn
2020-05-26  9:09   ` Radostin Stoyanov
2020-06-12  0:17 ` Matt Helsley
2020-06-12 14:39   ` Christian Brauner

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=877dwybxvi.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=0x7f454c46@gmail.com \
    --cc=Nicolas.Viennot@twosigma.com \
    --cc=acgoide@tycho.nsa.gov \
    --cc=areber@redhat.com \
    --cc=arnd@arndb.de \
    --cc=avagin@gmail.com \
    --cc=casey@schaufler-ca.com \
    --cc=chf@redhat.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=dipeit@gmail.com \
    --cc=eparis@parisplace.org \
    --cc=gorcunov@openvz.org \
    --cc=jannh@google.com \
    --cc=kyurtsever@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mclapinski@google.com \
    --cc=oleg@redhat.com \
    --cc=ovzxemul@gmail.com \
    --cc=rppt@linux.ibm.com \
    --cc=rstoyanov1@gmail.com \
    --cc=sargun@sargun.me \
    --cc=selinux@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=stephen.smalley.work@gmail.com \
    /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

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git