[Devel] Re: [PATCH 2/5] cr: checkpoint the active LSM and add RESTART_KEEP_LSM flag

[Serge E. Hallyn]

Quoting Casey Schaufler (casey at schaufler-ca.com):
> Serge E. Hallyn wrote:
> > [ patch 1 was a trivial non-security patch so if you didn't see
> > it, you didn't miss anything ]
> >
> > The RESTART_KEEP_LSM flag indicates that the LSM should
> > attempt to reuse checkpointed security labels.  It is always
> > invalid when the LSM at restart differs from that at checkpoint.
> > It is currently only usable for capabilities.
> >   
> 
> Can you imagine a scenario in which restoring a process on a
> system with a different LSM configuration makes any sense at all?

Without RESTART_KEEP_LSM absolutely.

With RESTART_KEEP_LSM, on a system with a different LSM loaded,
certainly not.

With RESTART_KEEP_LSM, on a system with the same LSM but a different
policy, yes I do.  If any checkpointed contexts have been invalidated
in the new policy, then restart with RESTART_KEEP_LSM should fail (*1).
If the contexts are still valid, then it seems reasonable to
assume that bin_t, user_t, etc, still basically mean what they
meant before.  No reason to refuse restart just because I loaded
a policy module for postfix, imo.

I could add both an lsm-module and lsm-policy version to the
checkpoint header, where the lsm-policy might be a sha1sum of
the whole policy, but that seems like overkill, a lot of
overhead, and probably a maintenance headache for the lsm-module
version.

> Goodness gracious, even if the "old" environment and the "new"
> are both SELinux and the policies are different I can't see how
> you could make any sort of claim that restoring the process is
> safe.

In what sense do you mean 'unsafe'?  The initial creation or
access to any checkpointed resource always happens with the
sys_restart() caller's and existing object's contexts, so there
should be no opportunity for accessing data which the old policy
allowed but the new does not.  It's possible that the task will
fail because of a more restrictive new policy, but so be it.

>  The same goes for having file based capabilities on one
> and not on the other.

A task running as uid 500 with cap_dac_read_search=ep could have
been started at least 3 ways:
	1. a uid 500 task executing a file with cap_dac_read_search=ep
	file caps
	2. a root task executing a file which does prctl(PR_SET_KEEPCAPS),
	setuid(500), and drops the other caps,
	3. a uid 500 task executing a setuid root version of the
	file in (2).

When we are restarting a task, we don't really care which of
the above ways it might have gotten its privileges.

I certainly could be overlooking something - what do you see as the
safety problem?

> It seems that the check you've chosen is necessary, but far from
> sufficient.

Thanks much for giving this some thought.  Without doubt this
is tricky business, and I was definately hoping for some serious
discussion.

thanks,
-serge

*1 - Hmm, in Smack, if the caller has CAP_MAC_ADMIN, then he
can load new, currently undefined types, right?  I guess that
could be a problem, though again I assume the new type should
simply have no accesses and so shouldn't be unsafe.
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers