Re: [PATCH v24 00/25] LSM: Module stacking for AppArmor

From: Casey Schaufler <casey@schaufler-ca.com>
To: Topi Miettinen <toiwoton@gmail.com>,
	casey.schaufler@intel.com, jmorris@namei.org,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Cc: linux-audit@redhat.com, keescook@chromium.org,
	john.johansen@canonical.com, penguin-kernel@i-love.sakura.ne.jp,
	paul@paul-moore.com, sds@tycho.nsa.gov,
	linux-kernel@vger.kernel.org,
	Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH v24 00/25] LSM: Module stacking for AppArmor
Date: Tue, 2 Feb 2021 10:06:29 -0800	[thread overview]
Message-ID: <3ac446c9-1af7-04dd-561d-6ec1dbb146b9@schaufler-ca.com> (raw)
In-Reply-To: <c5c40a66-b36d-73ab-6c92-f4d1f5f4ad35@gmail.com>

On 2/2/2021 9:12 AM, Topi Miettinen wrote:
> On 2.2.2021 17.30, Casey Schaufler wrote:
>> On 2/2/2021 4:05 AM, Topi Miettinen wrote:
>>> On 26.1.2021 18.40, Casey Schaufler wrote:
>>>> This patchset provides the changes required for
>>>> the AppArmor security module to stack safely with any other.
>>>
>>> In my test, when kernel command line has apparmor before selinux in lsm= entry, the boot is not successful with enforcing=1:
>>> systemd[1]: Failed to compute init label, ignoring.
>>> systemd[1]: Failed to set SELinux security context system_u:object_r:cgroup_t:s0 for /sys/fs/cgroup: Invalid argument
>>> systemd[1]: Failed to set SELinux security context system_u:object_r:pstore_t:s0 for /sys/fs/pstore: Invalid argument
>>> systemd[1]: Failed to set SELinux security context system_u:object_r:sysfs_t:s0 for /sys/firmware/efi/efivars: Invalid argument
>>> ...
>>> Failed to drop capability bounding set of usermode helpers: Operation not permitted
>>> Failed to drop capability bounding set of usermode helpers.
>>> systemd[1]: Freezing execution.
>>
>> Systemd has extensive support for SELinux. That's good.
>> It doesn't have an understanding of what needs to be done
>> if SELinux is active but not the default security module
>> for interfaces including SO_PEERSEC and /proc/*/attr/*.
>> That's going to take some work.
>
> Ok. What will be the replacement for SO_PEERSEC? Systemd calls getsockopt(fd, SOL_SOCKET, SO_PEERSEC, s, &n).

Dealing with SO_PEERSEC has been discussed at length, and I
wouldn't say that anyone is really happy with the conclusions.
The patch set presented uses the interface_lsm to determine
which module's data is presented in SO_PEERSEC. The interface_lsm
is controlled by writing the desired security module name to
/proc/self/attr/interface_lsm. The addition of SO_PEERCONTEXT,
which would contain all active security module data, has been
proposed as a follow-on but is not included in this patch set.

>
> Is the /proc part something that should be fixed on systemd side, or can perhaps the SELinux libraries hide this from applications?

It's unfortunate that the early days of the LSM where dominated
by the mindset that security had to be a complete solution. I
was in that camp myself for a good long time, but came to
recognize that attempting to solve all security problems for
everyone using one mechanism was destined to excess. Because
user-space development has assumed a single LSM for so long it's
hard to imagine that there won't need to be changes in both the
libraries and the programs. Because SELinux has the longest
history and most complete distribution integration it will also
have the most trouble with sharing the LSM stack. 

>
>>
>>>
>>> Probably SELinux libraries can't find or set the labels for the PID1 or any file systems. Before the init label message, systemd calls getcon_raw(), getfilecon_raw(), string_to_security_class() and security_compute_create_raw(), so one of these don't understand the LSM stacking.
>>
>> That is correct.
>>
>>>
>>> Also the policy needs updating to handle process2:setdisplay:
>>> SELinux:  Permission setdisplay in class process2 not defined in policy.
>>> SELinux: the above unknown classes and permissions will be denied
>>>
>>> With enforcing=0, many services start, but for example systemd-journald doesn't. This is probably related to the earlier problem with labels (maybe libraries try to use SELinux labels where kernel wants AppArmor profiles):
>>> systemd[1]: Failed to set SELinux security context system_u:object_r:init_runtime_t:s0 for /run/systemd/units/invocation:systemd-user-sessions.service: Invalid argument
>>
>> This is also an artifact of systemd seeing AppArmor information
>> instead of SELinux contexts.
>
> Will SELinux libraries choose automatically the correct way to set labels in the future?

I expect so eventually. The SELinux developers have not been
especially enthusiastic about the prospect of module stacking.
Once it is available I expect to see some accommodation, but
not necessarily to the level you might like. The patch set here
is strongly influenced by the assumption that putting the most
highly integrated module first (SELinux on Fedora, AppArmor on
Ubuntu, Smack on Tizen, ...) is going to get you most of what
you need. Whoever wants to add Smack to Ubuntu is going to have
some work to do.

Stacking AppArmor with SELinux is a real use case in the container
world, but that's not the real focus of this effort. I have seen
several cases where security features have not been implemented
because they couldn't be added to a system that also required
SELinux, AppArmor or Smack. I have seen many proposals for changes
to existing security modules that where outside their scope just
because there was no other way.

>
>>>
>>> Switching the order so that apparmor is after selinux, boot is successful. Loading AppArmor profiles needs a permission from SELinux:
>>>
>>> Feb 02 08:53:15 audit[963]: AVC avc:  denied  { mac_admin } for  pid=963 comm="apparmor_parser" capability=33 scontext=system_u:system_r:initrc_t:s0 tcontext=system_u:system_r:initrc_t:s0 tclass=capability2 permissive=0
>>> Feb 02 08:53:15 audit[963]: AVC apparmor="STATUS" operation="profile_replace" info="not policy admin" error=-13 profile="unconfined" pid=963 comm="apparmor_parser"
>>> Feb 02 08:53:15 audit: AUDIT1420 subj_selinux=system_u:system_r:initrc_t:s0 subj_apparmor==unconfined
>>> Feb 02 08:53:15 audit[963]: SYSCALL arch=c000003e syscall=1 success=no exit=-13 a0=7 a1=7a8f2ff04f80 a2=1e09 a3=0 items=0 ppid=961 pid=963 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="apparmor_parser" exe="/usr/sbin/apparmor_parser" subj=? key=(null)
>>> Feb 02 08:53:15 audit: PROCTITLE proctitle=2F7362696E2F61707061726D6F725F706172736572002D2D77726974652D6361636865002D2D7265706C616365002D2D002F6574632F61707061726D6F722E64
>>> Feb 02 08:53:15 apparmor.systemd[963]: /sbin/apparmor_parser: Unable to replace "/lib/systemd/systemd-resolved".  Permission denied; attempted to load a profile while confined?
>>>
>>> This just seems to need TE rules for the apparmor_parser.
>>>
>>> Double equal sign in subj_apparmor==unconfined looks odd, should that be just one like subj_selinux?
>>
>> The audit code is reporting what AppArmor provides.
>> I agree that this looks odd.
>>
>>>
>>>
>>> Tools like ps, and KDE and Gnome System Monitors only show SELinux context, but it would be nice if MAC contexts for all enabled LSMs were shown.
>>
>> I agree. How this should be done has been a topic of
>> lively debate for some time.
>>
>>>
>>> -Topi
>>
>> Thank you for this report. Which distribution are you using?
>> I have been testing with Fedora (SELinux + AppArmor) and Ubuntu
>> (AppArmor + Smack). I would be very interested to see how a
>> distribution that doesn't use systemd behaves.
>
> This is Debian with systemd, I'm using SELinux + TOMOYO + AppArmor.

Great to hear. Thanks again.