[CRIU] [RFC] lsm: add support for c/ring LSM profiles

[Pavel Emelyanov]

On 04/30/2015 01:30 PM, Tycho Andersen wrote:

>> A zdtm test would be a good example of how this thing works too.
> 
> Yes, I've attached one so you can see, and I'll re send it when I get
> time to respin the series from our discussion here.

OK, thanks.

>>> 2. lsm_profile maybe should live on pstree_item instead of ->core, but ->core
>>>    is the only thing passed to sigreturn_restore().
>>
>> That's because variable named "current" is accessible all over the restorer
>> code and points to the pstree_item being restored :)
>>
>>> 3. I don't know how to restore selinux policies since we don't actually exec()
>>
>> Can you shed more light on this too? I see the lsm_set_lable() performed even
>> for selinux, so what's the policies are and what the problem with their restore is?
> 
> Earlier in the patch it dies if you have an selinux label:
> 
> +                       pr_err("I don't know how to set an selinux context without exec.");
> 
> although we could/should probably error out here too just to be
> defensive.
> 
> As far as I can see (mostly just based on the initial mail [1], maybe
> this has changed more recently), SELinux contexts can only change on
> exec. Since we don't actually exec at the end of a process, I'm not
> sure how to restore them (maybe it's not possible without a kernel
> patch?).
> 
> [1]: https://lwn.net/Articles/28222/

Hm... After grepping the kernel it looks like /proc/pid/attr/current even for
selinux can change the task's label w/o exec, but performs more checks.

Can you try playing with it?

>>> +
>>> +	get_label = NULL;
>>> +	lsmtype = LSMTYPE__NO_LSM;
>>> +	name = "none";
>>> +}
>>
>>> +int validate_lsm()
>>> +{
>>> +	struct pstree_item *it;
>>> +
>>> +	if (name == NULL)
>>> +		get_host_lsm();
>>> +
>>> +	if (image_lsm == LSMTYPE__NO_LSM || image_lsm == lsmtype)
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * This is really only a problem if the processes have actually
>>> +	 * specified an LSM profile. If not, we won't restore anything anyway,
>>
>> But what does the lsm_set_label do?
> 
> We catch the null (i.e. no security) profiles and set lsm_profile to
> NULL in that case, and lsm_set_label restores things only when there
> is a value of lsm_profile, so it should be a no-op in this case.
> 
>>> +	 * so it's fine.
>>> +	 */
>>> +	pr_warn("lsm types do not match: host %d migratee %d\n", lsmtype, image_lsm);
>>> +
>>> +	for_each_pstree_item(it) {
>>> +		if (it->core[0]->lsm_profile) {
>>> +			pr_err("mismatched lsm types and lsm profile specified\n");
>>> +			return -1;
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>
>>> @@ -80,4 +80,6 @@ message core_entry {
>>>  	optional task_core_entry	tc		= 3;
>>>  	optional task_kobj_ids_entry	ids		= 4;
>>>  	optional thread_core_entry	thread_core	= 5;
>>> +
>>> +	optional string			lsm_profile	= 9;
>>
>> This is per-task thing, so the task_core_entry seem to be better place.
> 
> Ok, or on pstree directly, since it is actually visible thanks to your
> comment above? (That makes the most sense to me, since that's where
> the pid info lives too, and the profiles are accessed via pid in most
> cases.)

The pstree is supposed to keep tree linkage only, all things that sit
on in-kernel task_struct are about to be stored in core image and other
things (from other in-kernel objects) in other images (well, we have a 
mess here probably).

After your explanations and kernel grep I see that from the kernel 
perspective the label sits on the creds structure together with task's
[se]?[gu]id-s, caps and bset masks. Thus the creds.img is the place
for it.

-- Pavel