Re: [PATCH 4/5] LSM: Define SELinux function to measure security state

Lakshmi Ramasubramanian
2020-06-15T16:45:51Z

On 6/15/20 4:57 AM, Stephen Smalley wrote:

Hi Stephen,

Thanks for reviewing the patches.

>> +void security_state_change(char *lsm_name, void *state, int state_len)
>> +{
>> + ima_lsm_state(lsm_name, state, state_len);
>> +}
>> +
>
> What's the benefit of this trivial function instead of just calling
> ima_lsm_state() directly?

One of the feedback Casey Schaufler had given earlier was that calling
an IMA function directly from SELinux (or, any of the Security Modules)
would be a layering violation.

LSM framework (security/security.c) already calls IMA functions now (for
example, ima_bprm_check() is called from security_bprm_check()). I
followed the same pattern for measuring LSM data as well.

Please let me know if I misunderstood Casey's comment.

>> +static int selinux_security_state(char **lsm_name, void **state,
>> + int *state_len)
>> +{
>> + int rc = 0;
>> + char *new_state;
>> + static char *security_state_string = "enabled=%d;enforcing=%d";
>> +
>> + *lsm_name = kstrdup("selinux", GFP_KERNEL);
>> + if (!*lsm_name)
>> + return -ENOMEM;
>> +
>> + new_state = kzalloc(strlen(security_state_string) + 1, GFP_KERNEL);
>> + if (!new_state) {
>> + kfree(*lsm_name);
>> + *lsm_name = NULL;
>> + rc = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + *state_len = sprintf(new_state, security_state_string,
>> + !selinux_disabled(&selinux_state),
>> + enforcing_enabled(&selinux_state));
>
> I think I mentioned this on a previous version of these patches, but I
> would recommend including more than just the enabled and enforcing
> states in your measurement. Other low-hanging fruit would be the
> other selinux_state booleans (checkreqprot, initialized,
> policycap[0..__POLICYDB_CAPABILITY_MAX]). Going a bit further one
> could take a hash of the loaded policy by using security_read_policy()
> and then computing a hash using whatever hash ima prefers over the
> returned data,len pair. You likely also need to think about how to
> allow future extensibility of the state in a backward-compatible
> manner, so that future additions do not immediately break systems
> relying on older measurements.
>

Sure - I will address this one in the next update.

thanks,
-lakshmi