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 > > > >