[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200805154504.GB4365@sequoia>
Date: Wed, 5 Aug 2020 10:45:04 -0500
From: Tyler Hicks <tyhicks@...ux.microsoft.com>
To: Casey Schaufler <casey@...aufler-ca.com>
Cc: Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>,
zohar@...ux.ibm.com, stephen.smalley.work@...il.com,
sashal@...nel.org, jmorris@...ei.org,
linux-integrity@...r.kernel.org, selinux@...r.kernel.org,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 0/4] LSM: Measure security module data
On 2020-08-05 08:36:40, Casey Schaufler wrote:
> On 8/4/2020 6:14 PM, Lakshmi Ramasubramanian wrote:
> > On 8/4/20 6:04 PM, Casey Schaufler wrote:
> >> On 8/4/2020 5:43 PM, Lakshmi Ramasubramanian wrote:
> >>> Critical data structures of security modules are currently not measured.
> >>> Therefore an attestation service, for instance, would not be able to
> >>> attest whether the security modules are always operating with the policies
> >>> and configuration that the system administrator had setup. The policies
> >>> and configuration for the security modules could be tampered with by
> >>> malware by exploiting kernel vulnerabilities or modified through some
> >>> inadvertent actions on the system. Measuring such critical data would
> >>> enable an attestation service to better assess the state of the system.
> >>
> >> I still wonder why you're calling this an LSM change/feature when
> >> all the change is in IMA and SELinux. You're not putting anything
> >> into the LSM infrastructure, not are you using the LSM infrastructure
> >> to achieve your ends. Sure, you *could* support other security modules
> >> using this scheme, but you have a configuration dependency on
> >> SELinux, so that's at best going to be messy. If you want this to
> >> be an LSM "feature" you need to use the LSM hooking mechanism.
> >
> >>
> >> I'm not objecting to the feature. It adds value. But as you've
> >> implemented it it is either an IMA extension to SELinux, or an
> >> SELiux extension to IMA. Could AppArmor add hooks for this without
> >> changing the IMA code? It doesn't look like it to me.
> >
> > The check in IMA to allow the new IMA hook func LSM_STATE and LSM_POLICY when SELinux is enabled is just because SELinux is the only security module using these hooks now.
> >
> > To enable AppArmor, for instance, to use the new IMA hooks to measure state and policy would just require adding the check for CONFIG_SECURITY_APPARMOR. Other than that, there are no IMA changes needed to support AppArmor or other such security modules.
>
> This is exactly what I'm objecting to. What if a system has both SELinux
> and AppArmor compiled in? What if it has both enabled?
The SELinux state and policy would be measured but the AppArmor
state/policy would be silently ignored. This isn't ideal as the IMA
policy author would need to read the kernel code to figure out which
LSMs are going to be measured.
>
> >
> > Please see Patch 1/4
> >
> > + else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
> > + strcmp(args[0].from, "LSM_STATE") == 0)
> > + entry->func = LSM_STATE;
> > + else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
> > + strcmp(args[0].from, "LSM_POLICY") == 0)
> > + entry->func = LSM_POLICY;
> >
> > And, if early boot measurement is needed for AppArmor the following change in IMA's Kconfig
> >
> > Patch 4/4
> >
> > +config IMA_QUEUE_EARLY_BOOT_DATA
> > bool
> > + depends on SECURITY_SELINUX || (IMA_MEASURE_ASYMMETRIC_KEYS && SYSTEM_TRUSTED_KEYRING)
> > default y
> >
> > If you think calling this an "LSM feature" is not appropriate, please suggest a better phrase.
>
> In the code above you are under CONFIG_SECURITY_SELINUX.
> I would suggest that it's an SELinux feature, so you should
> be using SELINUX_STATE and SELINUX_POLICY, as I suggested
> before. Just because SELinux has state and policy to measure
> doesn't mean that a different module might not have other data,
> such as history, that should be covered as well.
In addition to SELINUX_STATE and SELINUX_POLICY, we should also consider
the proposed LSM_STATE and LSM_POLICY func values but require an "lsm"
rule conditional.
So the current proposed rules:
measure func=LSM_STATE
measure func=LSM_POLICY
Would become:
measure func=LSM_STATE lsm=selinux
measure func=LSM_POLICY lsm=selinux
The following rules would be rejected:
measure func=LSM_STATE
measure func=LSM_POLICY
measure func=LSM_STATE lsm=apparmor
measure func=LSM_POLICY lsm=smack
Of course, the apparmor and smack rules could/would be allowed when
proper support is in place.
Tyler
>
> I realize that IMA already has compile time dependencies to
> determine which xattrs to measure. There's no reason that
> the xattr list couldn't be determined at boot time, with
> each security module providing the XATTR_NAME values it
> uses.
>
> >
> > But like I said above, with minimal change in IMA other security modules can be supported to measure STATE and POLICY data.
> >
> > -lakshmi
> >
> >
Powered by blists - more mailing lists