On Tue, May 26, 2020 at 08:59:29AM -0500, Eric W. Biederman wrote:

Adrian Reber [off-list ref] writes:

quoted

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

quoted

quoted

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.

As mentioned in the other thread, CRIU can work with read access to
map_files.

quoted

quoted

quoted

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.

I will send out a v2 with a renamed capability soon and also include
map_files to be readable with that capability.

quoted

quoted

quoted

 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?

The two things are setting a PID via ns_last_pid/clone3() and reading
map_files.

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.

Also in this thread Kamil mentioned that they also need calling prctl
with PR_SET_MM during restore in their production setup.

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.

I will prepare a new version of this patch using CAP_CHECKPOINT_RESTORE
for ns_last_pid/clone3(), map_files, and prctl with PR_SET_MM.

		Adrian