On Fri, Jun 12, 2020 at 10:42 PM Lakshmi Ramasubramanian
<nramas@xxxxxxxxxxxxxxxxxxx> wrote:
>
> SELinux needs to implement the interface function, security_state(), for
> the LSM to gather SELinux data for measuring. Define the security_state()
> function in SELinux.
>
> The security modules should be able to notify the LSM when there is
> a change in the module's data. Define a function namely
> security_state_change() in the LSM that the security modules
> can call to provide the updated data for measurement.
>
> Call security_state_change() function from SELinux to report data
> when SELinux's state is updated.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
> ---
> diff --git a/security/security.c b/security/security.c
> index a6e2d1cd95af..e7175db5a093 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -238,6 +238,11 @@ static void __init initialize_lsm(struct lsm_info *lsm)
> }
> }
>
> +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?
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7e954b555be6..bbc908a1fcd1 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7225,6 +7225,47 @@ static __init int selinux_init(void)
> return 0;
> }
>
> +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.
