[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87273030-2303-e791-4e5d-25373faf0880@linux.microsoft.com>
Date: Thu, 4 Mar 2021 11:20:27 -0800
From: Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
To: zohar@...ux.ibm.com, paul@...l-moore.com,
stephen.smalley.work@...il.com
Cc: tusharsu@...ux.microsoft.com, tyhicks@...ux.microsoft.com,
casey@...aufler-ca.com, agk@...hat.com, snitzer@...hat.com,
gmazyland@...il.com, sashal@...nel.org, jmorris@...ei.org,
linux-integrity@...r.kernel.org, selinux@...r.kernel.org,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] selinux: measure state and policy capabilities
On 2/12/21 8:37 AM, Lakshmi Ramasubramanian wrote:
Hi Paul,
> SELinux stores the configuration state and the policy capabilities
> in kernel memory. Changes to this data at runtime would have an impact
> on the security guarantees provided by SELinux. Measuring this data
> through IMA subsystem provides a tamper-resistant way for
> an attestation service to remotely validate it at runtime.
>
> Measure the configuration state and policy capabilities by calling
> the IMA hook ima_measure_critical_data().
>
I have addressed your comments on the v2 patch for selinux measurement
using IMA. Could you please let me know if there are any other comments
that I need to address in this patch?
Thanks for your review and help so far.
-lakshmi
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
> Suggested-by: Stephen Smalley <stephen.smalley.work@...il.com>
> Suggested-by: Paul Moore <paul@...l-moore.com>
> ---
> security/selinux/ima.c | 87 ++++++++++++++++++++++++++++++++--
> security/selinux/include/ima.h | 6 +++
> security/selinux/selinuxfs.c | 6 +++
> security/selinux/ss/services.c | 2 +-
> 4 files changed, 96 insertions(+), 5 deletions(-)
>
> diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> index 03715893ff97..34d421861bfc 100644
> --- a/security/selinux/ima.c
> +++ b/security/selinux/ima.c
> @@ -13,18 +13,83 @@
> #include "ima.h"
>
> /*
> - * selinux_ima_measure_state - Measure hash of the SELinux policy
> + * selinux_ima_collect_state - Read selinux configuration settings
> *
> - * @state: selinux state struct
> + * @state: selinux_state
> *
> - * NOTE: This function must be called with policy_mutex held.
> + * On success returns the configuration settings string.
> + * On error, returns NULL.
> */
> -void selinux_ima_measure_state(struct selinux_state *state)
> +static char *selinux_ima_collect_state(struct selinux_state *state)
> {
> + const char *on = "=1;", *off = "=0;";
> + char *buf;
> + int buf_len, len, i, rc;
> +
> + buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1;
> +
> + len = strlen(on);
> + for (i = 0; i < __POLICYDB_CAPABILITY_MAX; i++)
> + buf_len += strlen(selinux_policycap_names[i]) + len;
> +
> + buf = kzalloc(buf_len, GFP_KERNEL);
> + if (!buf)
> + return NULL;
> +
> + rc = strscpy(buf, "initialized", buf_len);
> + WARN_ON(rc < 0);
> +
> + rc = strlcat(buf, selinux_initialized(state) ? on : off, buf_len);
> + WARN_ON(rc >= buf_len);
> +
> + rc = strlcat(buf, "enforcing", buf_len);
> + WARN_ON(rc >= buf_len);
> +
> + rc = strlcat(buf, enforcing_enabled(state) ? on : off, buf_len);
> + WARN_ON(rc >= buf_len);
> +
> + rc = strlcat(buf, "checkreqprot", buf_len);
> + WARN_ON(rc >= buf_len);
> +
> + rc = strlcat(buf, checkreqprot_get(state) ? on : off, buf_len);
> + WARN_ON(rc >= buf_len);
> +
> + for (i = 0; i < __POLICYDB_CAPABILITY_MAX; i++) {
> + rc = strlcat(buf, selinux_policycap_names[i], buf_len);
> + WARN_ON(rc >= buf_len);
> +
> + rc = strlcat(buf, state->policycap[i] ? on : off, buf_len);
> + WARN_ON(rc >= buf_len);
> + }
> +
> + return buf;
> +}
> +
> +/*
> + * selinux_ima_measure_state_locked - Measure SELinux state and hash of policy
> + *
> + * @state: selinux state struct
> + */
> +void selinux_ima_measure_state_locked(struct selinux_state *state)
> +{
> + char *state_str = NULL;
> void *policy = NULL;
> size_t policy_len;
> int rc = 0;
>
> + WARN_ON(!mutex_is_locked(&state->policy_mutex));
> +
> + state_str = selinux_ima_collect_state(state);
> + if (!state_str) {
> + pr_err("SELinux: %s: failed to read state.\n", __func__);
> + return;
> + }
> +
> + ima_measure_critical_data("selinux", "selinux-state",
> + state_str, strlen(state_str), false);
> +
> + kfree(state_str);
> +
> /*
> * Measure SELinux policy only after initialization is completed.
> */
> @@ -42,3 +107,17 @@ void selinux_ima_measure_state(struct selinux_state *state)
>
> vfree(policy);
> }
> +
> +/*
> + * selinux_ima_measure_state - Measure SELinux state and hash of policy
> + *
> + * @state: selinux state struct
> + */
> +void selinux_ima_measure_state(struct selinux_state *state)
> +{
> + WARN_ON(mutex_is_locked(&state->policy_mutex));
> +
> + mutex_lock(&state->policy_mutex);
> + selinux_ima_measure_state_locked(state);
> + mutex_unlock(&state->policy_mutex);
> +}
> diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h
> index d69c36611423..75ca92b4a462 100644
> --- a/security/selinux/include/ima.h
> +++ b/security/selinux/include/ima.h
> @@ -15,10 +15,16 @@
>
> #ifdef CONFIG_IMA
> extern void selinux_ima_measure_state(struct selinux_state *selinux_state);
> +extern void selinux_ima_measure_state_locked(
> + struct selinux_state *selinux_state);
> #else
> static inline void selinux_ima_measure_state(struct selinux_state *selinux_state)
> {
> }
> +static inline void selinux_ima_measure_state_locked(
> + struct selinux_state *selinux_state)
> +{
> +}
> #endif
>
> #endif /* _SELINUX_IMA_H_ */
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 4bde570d56a2..26ec58593ba1 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -41,6 +41,7 @@
> #include "security.h"
> #include "objsec.h"
> #include "conditional.h"
> +#include "ima.h"
>
> enum sel_inos {
> SEL_ROOT_INO = 2,
> @@ -182,6 +183,8 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
> selinux_status_update_setenforce(state, new_value);
> if (!new_value)
> call_blocking_lsm_notifier(LSM_POLICY_CHANGE, NULL);
> +
> + selinux_ima_measure_state(state);
> }
> length = count;
> out:
> @@ -762,6 +765,9 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf,
>
> checkreqprot_set(fsi->state, (new_value ? 1 : 0));
> length = count;
> +
> + selinux_ima_measure_state(fsi->state);
> +
> out:
> kfree(page);
> return length;
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 2106b5d383e7..cb2866489363 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -2179,7 +2179,7 @@ static void selinux_notify_policy_change(struct selinux_state *state,
> selinux_status_update_policyload(state, seqno);
> selinux_netlbl_cache_invalidate();
> selinux_xfrm_notify_policyload();
> - selinux_ima_measure_state(state);
> + selinux_ima_measure_state_locked(state);
> }
>
> void selinux_policy_commit(struct selinux_state *state,
>
Powered by blists - more mailing lists