| Message ID | 20200805004331.20652-1-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | LSM: Measure security module data | expand |
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.
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. 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. But like I said above, with minimal change in IMA other security modules can be supported to measure STATE and POLICY data. -lakshmi
On Tue, 2020-08-04 at 17:43 -0700, 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. From a high level review, "Critical data structures" should be the focus of this patch set. Measuring "critical data structures" should be independent of measuring the "policy" being loaded. The in memory policy hash could be an example of data included in the "critical data structures". Keep this patch set simple. Mimi
On Tue, 2020-08-04 at 18:04 -0700, 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. Agreed. Without reviewing the patch set in depth, I'm not quite sure why this patch set needs to be limited to measuring just LSM critical data and can't be generalized. The patch set could be titled "ima: measuring critical data" or "ima: measuring critical kernel data". Measuring SELinux critical data would be an example of its usage. For an example, refer to the ima_file_check hook, which is an example of IMA being called directly, not via an LSM hook. Mimi
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? > > 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. 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 > >
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 > > > >
On 8/5/20 8:45 AM, Tyler Hicks wrote: > 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. Tyler - I am not sure why AppArmor state\policy would be ignored when both SELinux and AppArmor are enabled. Could you please clarify? When both the security modules are enabled, IMA policy validator would look like below: if (IS_ENABLED(CONFIG_SECURITY_SELINUX) || IS_ENABLED(CONFIG_SECURITY_APPARMOR)) && strcmp(args[0].from, "LSM_STATE") == 0) entry->func = LSM_STATE; Similar one for LSM_POLICY validation. Both SELinux and AppArmor can call ima_measure_lsm_state() and ima_measure_lsm_policy() to measure state and policy respectively. I don't think we should go the route of defining IMA hooks per security module (i.e., SELINUX_STATE, APPARMOR_STATE, SELINUX_POLICY, etc.) Instead keep the hook generic that any SM could use - which is what I have tried to address in this patch series. >> >>> >>> 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. Good point - if other SMs have data besides state and policy, we can define IMA hooks to measure that as well. But I still think it is not the right approach to call this SELINUX_STATE and SELINUX_POLICY - it will lead to unnecessary code duplication in IMA as more SMs are onboarded, in my opinion. Correct me if I am wrong. -lakshmi > > 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. > > >> >> 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 >>> >>>
On 2020-08-05 09:07:48, Lakshmi Ramasubramanian wrote: > On 8/5/20 8:45 AM, Tyler Hicks wrote: > > 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. > > Tyler - I am not sure why AppArmor state\policy would be ignored when both > SELinux and AppArmor are enabled. Could you please clarify? I think Casey is talking about now (when AppArmor is not supported by this feature) and you're talking about the future (when AppArmor may be supported by this feature). Now, a "measure func=LSM_STATE" rule would be accepted by the IMA policy parser but do nothing for the AppArmor LSM. The rule may do something in the future but there's no difference in feedback to the IMA policy author between now and the future. Tyler > > When both the security modules are enabled, IMA policy validator would look > like below: > > if (IS_ENABLED(CONFIG_SECURITY_SELINUX) || > IS_ENABLED(CONFIG_SECURITY_APPARMOR)) && > strcmp(args[0].from, "LSM_STATE") == 0) > entry->func = LSM_STATE; > > Similar one for LSM_POLICY validation. > > Both SELinux and AppArmor can call ima_measure_lsm_state() and > ima_measure_lsm_policy() to measure state and policy respectively. > > I don't think we should go the route of defining IMA hooks per security > module (i.e., SELINUX_STATE, APPARMOR_STATE, SELINUX_POLICY, etc.) Instead > keep the hook generic that any SM could use - which is what I have tried to > address in this patch series. > > > > > > > > > > > > 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. > > Good point - if other SMs have data besides state and policy, we can define > IMA hooks to measure that as well. > > But I still think it is not the right approach to call this SELINUX_STATE > and SELINUX_POLICY - it will lead to unnecessary code duplication in IMA as > more SMs are onboarded, in my opinion. Correct me if I am wrong. > > -lakshmi > > > > > 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. > > > > > > > > > > > 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 > > > > > > > >
On 8/5/20 9:14 AM, Tyler Hicks wrote: > On 2020-08-05 09:07:48, Lakshmi Ramasubramanian wrote: >> On 8/5/20 8:45 AM, Tyler Hicks wrote: >>> 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. >> >> Tyler - I am not sure why AppArmor state\policy would be ignored when both >> SELinux and AppArmor are enabled. Could you please clarify? > > I think Casey is talking about now (when AppArmor is not supported by > this feature) and you're talking about the future (when AppArmor may be > supported by this feature). Got it - thanks for clarifying. But with the current code if SELinux is enabled on the system, but AppArmor is not and the IMA policy contains "measure func=LSM_STATE" then the policy will be rejected as "-EINVAL". So the policy author would get a feedback even now. Correct me if I am wrong. -lakshmi
On 2020-08-05 09:21:24, Lakshmi Ramasubramanian wrote: > On 8/5/20 9:14 AM, Tyler Hicks wrote: > > On 2020-08-05 09:07:48, Lakshmi Ramasubramanian wrote: > > > On 8/5/20 8:45 AM, Tyler Hicks wrote: > > > > 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. > > > > > > Tyler - I am not sure why AppArmor state\policy would be ignored when both > > > SELinux and AppArmor are enabled. Could you please clarify? > > > > I think Casey is talking about now (when AppArmor is not supported by > > this feature) and you're talking about the future (when AppArmor may be > > supported by this feature). > > Got it - thanks for clarifying. > > But with the current code if SELinux is enabled on the system, but AppArmor > is not and the IMA policy contains "measure func=LSM_STATE" then the policy > will be rejected as "-EINVAL". The AppArmor policy load? Yes, the load will fail. What Casey is talking about is when SELinux and AppArmor are enabled in the kernel config but AppArmor is active. This scenario is true in distro kernels such as Ubuntu's kernel: $ grep -e CONFIG_SECURITY_SELINUX= -e CONFIG_SECURITY_APPARMOR= /boot/config-5.4.0-42-generic CONFIG_SECURITY_SELINUX=y CONFIG_SECURITY_APPARMOR=y $ cat /sys/kernel/security/lsm lockdown,capability,yama,apparmor Casey also likely has LSM stacking in mind here where SELinux and AppArmor could both be active at the same time but the LSM stacking series is not yet applied. Tyler > So the policy author would get a feedback even now. > Correct me if I am wrong. > > -lakshmi
On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote: > 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 Kees is cleaning up the firmware code which differentiated between how firmware was loaded. There will be a single firmware enumeration. Similarly, the new IMA hook to measure critical state may be placed in multiple places. Is there really a need from a policy perspective for differentiating the source of the critical state being measurind? The data being measured should include some identifying information. I think moving away from the idea that measuring "critical" data should be limited to LSMs, will clarify this. Mimi
On 8/5/20 10:03 AM, Mimi Zohar wrote: > On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote: > >> 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 > > Kees is cleaning up the firmware code which differentiated between how > firmware was loaded. There will be a single firmware enumeration. > > Similarly, the new IMA hook to measure critical state may be placed in > multiple places. Is there really a need from a policy perspective for > differentiating the source of the critical state being measurind? The > data being measured should include some identifying information. Yes Mimi - SELinux is including the identifying information in the "event name" field. Please see a sample measurement of STATE and POLICY for SELinux below: 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state-1595389364:287899386 696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303 10 f4a7...9408 ima-ng sha256:8d1d...1834 selinux-policy-hash-1595389353:863934271 > > I think moving away from the idea that measuring "critical" data should > be limited to LSMs, will clarify this. > Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"? I think that is a good idea. -lakshmi
On 8/5/2020 9:32 AM, Tyler Hicks wrote: > On 2020-08-05 09:21:24, Lakshmi Ramasubramanian wrote: >> On 8/5/20 9:14 AM, Tyler Hicks wrote: >>> On 2020-08-05 09:07:48, Lakshmi Ramasubramanian wrote: >>>> On 8/5/20 8:45 AM, Tyler Hicks wrote: >>>>> 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. >>>> Tyler - I am not sure why AppArmor state\policy would be ignored when both >>>> SELinux and AppArmor are enabled. Could you please clarify? >>> I think Casey is talking about now (when AppArmor is not supported by >>> this feature) and you're talking about the future (when AppArmor may be >>> supported by this feature). >> Got it - thanks for clarifying. >> >> But with the current code if SELinux is enabled on the system, but AppArmor >> is not and the IMA policy contains "measure func=LSM_STATE" then the policy >> will be rejected as "-EINVAL". > The AppArmor policy load? Yes, the load will fail. > > What Casey is talking about is when SELinux and AppArmor are enabled in > the kernel config but AppArmor is active. This scenario is true in > distro kernels such as Ubuntu's kernel: > > $ grep -e CONFIG_SECURITY_SELINUX= -e CONFIG_SECURITY_APPARMOR= /boot/config-5.4.0-42-generic > CONFIG_SECURITY_SELINUX=y > CONFIG_SECURITY_APPARMOR=y > $ cat /sys/kernel/security/lsm > lockdown,capability,yama,apparmor Yes. This is one reason that a compile time check is inappropriate. You also have the case with SELinux where you can unload the module. It's deprecated, but still possible. If you boot with SELinux compiled in, but with security=none on the boot line you also have a case where the compile time check is inappropriate. > Casey also likely has LSM stacking in mind here where SELinux and > AppArmor could both be active at the same time but the LSM stacking > series is not yet applied. It isn't in Linus' kernel, but is available in Ubuntu. The audit/IMA rule selection get hairy in the multiple security module case, but you don't need to add that for the proposed scheme to be flawed. > > Tyler > >> So the policy author would get a feedback even now. >> Correct me if I am wrong. >> >> -lakshmi
On 8/5/2020 10:25 AM, Lakshmi Ramasubramanian wrote: > On 8/5/20 10:03 AM, Mimi Zohar wrote: >> On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote: >> >>> 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 >> >> Kees is cleaning up the firmware code which differentiated between how >> firmware was loaded.?? There will be a single firmware enumeration. >> >> Similarly, the new IMA hook to measure critical state may be placed in >> multiple places.? Is there really a need from a policy perspective for >> differentiating the source of the critical state being measurind??? The >> data being measured should include some identifying information. > > Yes Mimi - SELinux is including the identifying information in the "event name" field. Please see a sample measurement of STATE and POLICY for SELinux below: > > 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state-1595389364:287899386 696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303 > > 10 f4a7...9408 ima-ng sha256:8d1d...1834 selinux-policy-hash-1595389353:863934271 > >> >> I think moving away from the idea that measuring "critical" data should >> be limited to LSMs, will clarify this. >> > > Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"? Policy, state, history or whim, it should be up to the security module to determine what data it cares about, and how it should be measured. Smack does not keep its policy in a single blob of data, it uses lists which can be modified at will. Your definition of the behavior for LSM_POLICY wouldn't work for Smack. That doesn't mean that there isn't a viable way to measure the Smack policy, it just means that IMA isn't the place for it. If SELinux wants its data measured, SELinux should be providing the mechanism to do it. I guess that I'm agreeing with you in part. If you want a generic measurement of "critical data", you don't need to assign a type to it, you have the caller (a security module, a device driver or whatever) identify itself and how it is going to deal with the data. That's very different from what you've done to date. > > I think that is a good idea. > > ?-lakshmi
On 8/5/20 10:57 AM, Casey Schaufler wrote: > On 8/5/2020 10:25 AM, Lakshmi Ramasubramanian wrote: >> On 8/5/20 10:03 AM, Mimi Zohar wrote: >>> On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote: >>> >>>> 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 >>> >>> Kees is cleaning up the firmware code which differentiated between how >>> firmware was loaded.?? There will be a single firmware enumeration. >>> >>> Similarly, the new IMA hook to measure critical state may be placed in >>> multiple places.? Is there really a need from a policy perspective for >>> differentiating the source of the critical state being measurind??? The >>> data being measured should include some identifying information. >> >> Yes Mimi - SELinux is including the identifying information in the "event name" field. Please see a sample measurement of STATE and POLICY for SELinux below: >> >> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state-1595389364:287899386 696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303 >> >> 10 f4a7...9408 ima-ng sha256:8d1d...1834 selinux-policy-hash-1595389353:863934271 >> >>> >>> I think moving away from the idea that measuring "critical" data should >>> be limited to LSMs, will clarify this. >>> >> >> Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"? > > Policy, state, history or whim, it should be up to the security module > to determine what data it cares about, and how it should be measured. > Smack does not keep its policy in a single blob of data, it uses lists > which can be modified at will. Your definition of the behavior for > LSM_POLICY wouldn't work for Smack. That doesn't mean that there isn't > a viable way to measure the Smack policy, it just means that IMA isn't > the place for it. If SELinux wants its data measured, SELinux should be > providing the mechanism to do it. > > I guess that I'm agreeing with you in part. If you want a generic measurement > of "critical data", you don't need to assign a type to it, you have the > caller (a security module, a device driver or whatever) identify itself and > how it is going to deal with the data. That's very different from what you've > done to date. Agree. Like Stephen had stated earlier, the reason we kept separate hooks (STATE and POLICY) is because the data for state is usually small and therefore we measure the entire data. Whereas, policy data is usually quite large (a few MB) and hence we measure a hash of the policy. If change to a generic measurement of "critical data", at the least IMA should provide a way to measure "data" and "hash(data)". And, with the caller providing the identifying information, there would be no need to call it "LSM_STATE" or "SELINUX_STATE" or such. -lakshmi
On 8/5/2020 11:08 AM, Lakshmi Ramasubramanian wrote: > On 8/5/20 10:57 AM, Casey Schaufler wrote: >> On 8/5/2020 10:25 AM, Lakshmi Ramasubramanian wrote: >>> On 8/5/20 10:03 AM, Mimi Zohar wrote: >>>> On Wed, 2020-08-05 at 10:45 -0500, Tyler Hicks wrote: >>>> >>>>> 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 >>>> >>>> Kees is cleaning up the firmware code which differentiated between how >>>> firmware was loaded.?? There will be a single firmware enumeration. >>>> >>>> Similarly, the new IMA hook to measure critical state may be placed in >>>> multiple places.? Is there really a need from a policy perspective for >>>> differentiating the source of the critical state being measurind??? The >>>> data being measured should include some identifying information. >>> >>> Yes Mimi - SELinux is including the identifying information in the "event name" field. Please see a sample measurement of STATE and POLICY for SELinux below: >>> >>> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state-1595389364:287899386 696e697469616c697a65643d313b656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574776f726b5f706565725f636f6e74726f6c733d313b6f70656e5f7065726d733d313b657874656e6465645f736f636b65745f636c6173733d313b616c776179735f636865636b5f6e6574776f726b3d303b6367726f75705f7365636c6162656c3d313b6e6e705f6e6f737569645f7472616e736974696f6e3d313b67656e66735f7365636c6162656c5f73796d6c696e6b733d303 >>> >>> 10 f4a7...9408 ima-ng sha256:8d1d...1834 selinux-policy-hash-1595389353:863934271 >>> >>>> >>>> I think moving away from the idea that measuring "critical" data should >>>> be limited to LSMs, will clarify this. >>>> >>> >>> Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"? >> >> Policy, state, history or whim, it should be up to the security module >> to determine what data it cares about, and how it should be measured. >> Smack does not keep its policy in a single blob of data, it uses lists >> which can be modified at will. Your definition of the behavior for >> LSM_POLICY wouldn't work for Smack. That doesn't mean that there isn't >> a viable way to measure the Smack policy, it just means that IMA isn't >> the place for it. If SELinux wants its data measured, SELinux should be >> providing the mechanism to do it. >> >> I guess that I'm agreeing with you in part. If you want a generic measurement >> of "critical data", you don't need to assign a type to it, you have the >> caller (a security module, a device driver or whatever) identify itself and >> how it is going to deal with the data. That's very different from what you've >> done to date. > > Agree. > > Like Stephen had stated earlier, the reason we kept separate hooks (STATE and POLICY) is because the data for state is usually small and therefore we measure the entire data. Whereas, policy data is usually quite large (a few MB) and hence we measure a hash of the policy. SELinux should determine how it wants its data measured. SELinux, not IMA, should determine if some "critical data" be measured in total, by its hash or as a count of policy rules. It would be handy for IMA to supply functions to do data blobs and hashes, but it should be up to the caller to decide if they meet their needs. > > If change to a generic measurement of "critical data", at the least IMA should provide a way to measure "data" and "hash(data)". > > And, with the caller providing the identifying information, there would be no need to call it "LSM_STATE" or "SELINUX_STATE" or such. > > ?-lakshmi > >
On 8/5/20 11:25 AM, Casey Schaufler wrote: >>>>> >>>>> I think moving away from the idea that measuring "critical" data should >>>>> be limited to LSMs, will clarify this. >>>>> >>>> >>>> Are you suggesting that instead of calling the hooks LSM_STATE and LSM_POLICY, we should keep it more generic so that it can be utilized by any subsystem to measure their "critical data"? >>> >>> Policy, state, history or whim, it should be up to the security module >>> to determine what data it cares about, and how it should be measured. >>> Smack does not keep its policy in a single blob of data, it uses lists >>> which can be modified at will. Your definition of the behavior for >>> LSM_POLICY wouldn't work for Smack. That doesn't mean that there isn't >>> a viable way to measure the Smack policy, it just means that IMA isn't >>> the place for it. If SELinux wants its data measured, SELinux should be >>> providing the mechanism to do it. >>> >>> I guess that I'm agreeing with you in part. If you want a generic measurement >>> of "critical data", you don't need to assign a type to it, you have the >>> caller (a security module, a device driver or whatever) identify itself and >>> how it is going to deal with the data. That's very different from what you've >>> done to date. >> >> Agree. >> >> Like Stephen had stated earlier, the reason we kept separate hooks (STATE and POLICY) is because the data for state is usually small and therefore we measure the entire data. Whereas, policy data is usually quite large (a few MB) and hence we measure a hash of the policy. > > SELinux should determine how it wants its data measured. > SELinux, not IMA, should determine if some "critical data" > be measured in total, by its hash or as a count of policy > rules. It would be handy for IMA to supply functions to do > data blobs and hashes, but it should be up to the caller to > decide if they meet their needs. > Per feedback from you all, my colleague Tushar has posted a patch series that defines a generic IMA hook to measure critical data from other subsystems (such as SELinux, AppArmor, Device-Mapper targets, etc.) Link to the patch series is given below: https://patchwork.kernel.org/patch/11711249/ Shortly I will re-post the SELinux state and hash of policy measurement patch that will be based on the above patch series. thanks, -lakshmi
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. IMA subsystem measures system files, command line arguments passed to kexec, boot aggregate, keys, etc. It can be used to measure critical data structures of security modules as well. This change aims to address measuring critical data structures of security modules when they are initialized and when they are updated at runtime. This series is based on commit 3db0d0c276a7 ("integrity: remove redundant initialization of variable ret") in next-integrity Change log: v6: => Use kvmalloc for payload data for early boot data measurement since payload size may exceed the limit supported by kmalloc. => Fixed IMA policy rule match error when checking for IMA hook func LSM_STATE and LSM_POLICY. => Enable early boot data measurement and IMA hook func LSM_STATE and LSM_POLICY when SELinux is enabled. v5: => Append timestamp to "event name" string in the call to the IMA hooks so that LSM data is always measured by IMA. => Removed workqueue patch that was handling periodic checking of the LSM data. This change will be introduced as a separate patch set while keeping this patch set focussed on measuring the LSM data on initialization and on updates at runtime. => Handle early boot measurement of LSM data. v4: => Added LSM_POLICY func and IMA hook to measure LSM policy. => Pass SELinux policy data, instead of the hash of the policy, to the IMA hook to measure. => Include "initialized" flag in SELinux measurement. Also, measure SELinux state even when initialization is not yet completed. But measure SELinux policy only after initialization. v3: => Loop through policy_capabilities to build the state data to measure instead of hardcoding to current set of policy capabilities. => Added error log messages for failure conditions. v2: => Pass selinux_state struct as parameter to the function that measures SELinux data. => Use strings from selinux_policycap_names array for SELinux state measurement. => Refactored security_read_policy() to alloc kernel or user virtual memory and then read the SELinux policy. v1: => Per Stephen Smalley's suggestion added selinux_state booleans and hash of SELinux policy in the measured data for SELinux. => Call IMA hook from the security module directly instead of redirecting through the LSM. Lakshmi Ramasubramanian (4): IMA: Add func to measure LSM state and policy IMA: Define IMA hooks to measure LSM state and policy LSM: Define SELinux function to measure state and policy IMA: Handle early boot data measurement Documentation/ABI/testing/ima_policy | 9 + include/linux/ima.h | 14 ++ security/integrity/ima/Kconfig | 5 +- security/integrity/ima/Makefile | 2 +- security/integrity/ima/ima.h | 45 +++-- security/integrity/ima/ima_api.c | 2 +- security/integrity/ima/ima_asymmetric_keys.c | 6 +- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_main.c | 64 ++++++- security/integrity/ima/ima_policy.c | 36 +++- security/integrity/ima/ima_queue_data.c | 190 +++++++++++++++++++ security/integrity/ima/ima_queue_keys.c | 174 ----------------- security/selinux/Makefile | 2 + security/selinux/hooks.c | 1 + security/selinux/include/security.h | 15 ++ security/selinux/measure.c | 150 +++++++++++++++ security/selinux/selinuxfs.c | 3 + security/selinux/ss/services.c | 71 ++++++- 18 files changed, 569 insertions(+), 222 deletions(-) create mode 100644 security/integrity/ima/ima_queue_data.c delete mode 100644 security/integrity/ima/ima_queue_keys.c create mode 100644 security/selinux/measure.c