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
