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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 15 Jun 2020 07:57:40 -0400
From:   Stephen Smalley <stephen.smalley.work@...il.com>
To:     Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
Cc:     Mimi Zohar <zohar@...ux.ibm.com>,
        Stephen Smalley <stephen.smalley@...il.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        James Morris <jmorris@...ei.org>,
        linux-integrity@...r.kernel.org,
        LSM List <linux-security-module@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/5] LSM: Define SELinux function to measure security state

On Fri, Jun 12, 2020 at 10:42 PM Lakshmi Ramasubramanian
<nramas@...ux.microsoft.com> 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@...ux.microsoft.com>
> ---
> 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.