[lxc-devel] [PATCH RFC 0/1] refactor AppArmor into LSM backend, add SELinux support

[Dwight Engen]

On Sat, 21 Sep 2013 00:32:42 -0500
Serge Hallyn <serge.hallyn at ubuntu.com> wrote:

> Quoting Dwight Engen (dwight.engen at oracle.com):
> > This change proposes to add support to LXC for additional LSMs
> > (Linux Security Module), namely SELinux. It does so by turning the
> > existing
> 
> Thanks, Dwight!
> 
> I do some bikeshed arguing below, but I will do a closer review next
> week, hopefully monday.

Okay, thanks!
 
> > AppArmor calls into generic lsm_* calls, which are then handled by
> > one of three LSM drivers: AppArmor, SELinux, or a nop driver.
> > Adding a SMACK driver should be fairly simple. The nop driver is
> > used when LXC has compiled in support for AppArmor or SELinux but
> > neither is enabled in the run time environment.
> > 
> > One minor point of discussion should be whether to keep the
> > aa_profile configuration item and have a separate selinux_context
> > item, or to use the approach taken in this patch which is to
> > genericize the name to lsm_label. Using a single lsm_label implies
> > that the policies will never be used together, which I believe is
> > likely a safe assumption.
> 
> You might be right on this, but there are two counter-arguments, and
> I'm not sure where I stand.  The first counter argument is that I
> might want to share a config or config excerpt (i.e. lxc.include)
> between several hosts, some of which are apparmor-enabled and some
> selinux-enabled. Or even migrate or stop-and-move a container between
> such hosts. Ignoring the labeling issue :) that would not be possible
> with this setup.  Secondly, there *is* on-going work, with a non-zero
> chance of inclusion, to enable stacking LSMs in the kernel.  I doubt
> that even i that case you'd want to use both to confine the
> container, but one never knows.  You might want to use apparmor to
> confine the container as seen from the container, while specifying
> that the contaienr should start as unconfined_t so that the selinux
> policy in the container can work.  (Far-fetched, I agree.)

I wasn't sure which way to go on this either. I think you have good
points there so I'm fine with having the config items be separate :)
Another thing to point out is that I think its nice if lxc always knows
how to parse those config items, even if it wasn't compiled with
AppArmor or SELinux, that way if you are given a config from a machine
that did have it, it won't barf parsing it and cloning would pass it
through. If you disagree, I can put them back behind #ifdefs.

I had seen that there was some thought to stacking LSMs, so I guess we
shouldn't rule that out in case it does happen.
 
> Thirdly, we would at least want lxc.aa_profile to be handled as a
> valid legacy case so that existing containers don't break
> 
> > A larger issue is the semantics around when lxc changes
> > profile/context. Currently, the AppArmor backend uses
> > aa_change_profile() which changes the profile immediately. No
> > analog exists in SELinux, so the SELinux backend uses
> > setexeccon_raw() which only takes effect upon exec(2). We could
> > change the AppArmor backend to use aa_change_onexec() to give them
> > similar semantics, but this would possibly break callers relying on
> > the "immediate
> 
> Note that I wanted to use the aa_change_onexec() originally, but did
> not only because it was broken at the time.

Ahh, I saw that but I didn't know why you opted to use the non-exec
version.
 
> > change" behavior (in particular users of the new attach API calling
> > a function). I don't know how widespread this reliance might be,
> > but I don't think that model is supportable in SELinux. The current
> > patch does not try to resolve the difference, I guess one option is
> > to just leave it that way. Definitely looking for some guidance
> > here.
> 
> In general "now" vs "on-exec" seem to be the two main ways to change
> contexts so I think it's fine to support both.

You mean that its fine that the two backends behave a bit differently?
We can't support "now" in SELinux as far as I can see.