Re: [PATCH v3] selinux: measure state and policy capabilities

From: Lakshmi Ramasubramanian
Date: Thu Mar 04 2021 - 14:22:11 EST


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@xxxxxxxxxxxxxxxxxxx>
Suggested-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
Suggested-by: Paul Moore <paul@xxxxxxxxxxxxxx>
---
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,