SELinux stores the active policy in memory, so the changes to this data
at runtime would have an impact on the security guarantees provided
by SELinux. Measuring in-memory SELinux policy through IMA subsystem
provides a secure way for the attestation service to remotely validate
the policy contents at runtime.
Measure the hash of the loaded policy by calling the IMA hook
ima_measure_critical_data(). Since the size of the loaded policy
can be large (several MB), measure the hash of the policy instead of
the entire policy to avoid bloating the IMA log entry.
To enable SELinux data measurement, the following steps are required:
1, Add "ima_policy=critical_data" to the kernel command line arguments
to enable measuring SELinux data at boot time.
For example,
BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data
2, Add the following rule to /etc/ima/ima-policy
measure func=CRITICAL_DATA label=selinux
Sample measurement of the hash of SELinux policy:
To verify the measured data with the current SELinux policy run
the following commands and verify the output hash values match.
sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6
Note that the actual verification of SELinux policy would require loading
the expected policy into an identical kernel on a pristine/known-safe
system and run the sha256sum /sys/kernel/selinux/policy there to get
the expected hash.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
Suggested-by: Stephen Smalley <[email protected]>
Acked-by: Paul Moore <[email protected]>
Reviewed-by: Tyler Hicks <[email protected]>
---
Documentation/ABI/testing/ima_policy | 3 +-
security/selinux/Makefile | 2 +
security/selinux/ima.c | 44 +++++++++++++++++++
security/selinux/include/ima.h | 24 +++++++++++
security/selinux/include/security.h | 3 +-
security/selinux/ss/services.c | 64 ++++++++++++++++++++++++----
6 files changed, 129 insertions(+), 11 deletions(-)
create mode 100644 security/selinux/ima.c
create mode 100644 security/selinux/include/ima.h
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 54fe1c15ed50..8365596cb42b 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -52,8 +52,9 @@ Description:
template:= name of a defined IMA template type
(eg, ima-ng). Only valid when action is "measure".
pcr:= decimal value
- label:= [data_label]
+ label:= [selinux]|[data_label]
data_label:= a unique string used for grouping and limiting critical data.
+ For example, "selinux" to measure critical data for SELinux.
default policy:
# PROC_SUPER_MAGIC
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 4d8e0e8adf0b..776162444882 100644
--- a/security/selinux/Makefile
+++ b/security/selinux/Makefile
@@ -16,6 +16,8 @@ selinux-$(CONFIG_NETLABEL) += netlabel.o
selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o
+selinux-$(CONFIG_IMA) += ima.o
+
ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include
$(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h
diff --git a/security/selinux/ima.c b/security/selinux/ima.c
new file mode 100644
index 000000000000..03715893ff97
--- /dev/null
+++ b/security/selinux/ima.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021 Microsoft Corporation
+ *
+ * Author: Lakshmi Ramasubramanian ([email protected])
+ *
+ * Measure critical data structures maintainted by SELinux
+ * using IMA subsystem.
+ */
+#include <linux/vmalloc.h>
+#include <linux/ima.h>
+#include "security.h"
+#include "ima.h"
+
+/*
+ * selinux_ima_measure_state - Measure hash of the SELinux policy
+ *
+ * @state: selinux state struct
+ *
+ * NOTE: This function must be called with policy_mutex held.
+ */
+void selinux_ima_measure_state(struct selinux_state *state)
+{
+ void *policy = NULL;
+ size_t policy_len;
+ int rc = 0;
+
+ /*
+ * Measure SELinux policy only after initialization is completed.
+ */
+ if (!selinux_initialized(state))
+ return;
+
+ rc = security_read_state_kernel(state, &policy, &policy_len);
+ if (rc) {
+ pr_err("SELinux: %s: failed to read policy %d.\n", __func__, rc);
+ return;
+ }
+
+ ima_measure_critical_data("selinux", "selinux-policy-hash",
+ policy, policy_len, true);
+
+ vfree(policy);
+}
diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h
new file mode 100644
index 000000000000..d69c36611423
--- /dev/null
+++ b/security/selinux/include/ima.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2021 Microsoft Corporation
+ *
+ * Author: Lakshmi Ramasubramanian ([email protected])
+ *
+ * Measure critical data structures maintainted by SELinux
+ * using IMA subsystem.
+ */
+
+#ifndef _SELINUX_IMA_H_
+#define _SELINUX_IMA_H_
+
+#include "security.h"
+
+#ifdef CONFIG_IMA
+extern void selinux_ima_measure_state(struct selinux_state *selinux_state);
+#else
+static inline void selinux_ima_measure_state(struct selinux_state *selinux_state)
+{
+}
+#endif
+
+#endif /* _SELINUX_IMA_H_ */
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 3cc8bab31ea8..29cae32d3fc5 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -229,7 +229,8 @@ void selinux_policy_cancel(struct selinux_state *state,
struct selinux_policy *policy);
int security_read_policy(struct selinux_state *state,
void **data, size_t *len);
-
+int security_read_state_kernel(struct selinux_state *state,
+ void **data, size_t *len);
int security_policycap_supported(struct selinux_state *state,
unsigned int req_cap);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 9704c8a32303..cc8dbc4ed8db 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -65,6 +65,7 @@
#include "ebitmap.h"
#include "audit.h"
#include "policycap_names.h"
+#include "ima.h"
/* Forward declaration. */
static int context_struct_to_string(struct policydb *policydb,
@@ -2180,6 +2181,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);
}
void selinux_policy_commit(struct selinux_state *state,
@@ -3875,8 +3877,33 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
}
#endif /* CONFIG_NETLABEL */
+/**
+ * __security_read_policy - read the policy.
+ * @policy: SELinux policy
+ * @data: binary policy data
+ * @len: length of data in bytes
+ *
+ */
+static int __security_read_policy(struct selinux_policy *policy,
+ void *data, size_t *len)
+{
+ int rc;
+ struct policy_file fp;
+
+ fp.data = data;
+ fp.len = *len;
+
+ rc = policydb_write(&policy->policydb, &fp);
+ if (rc)
+ return rc;
+
+ *len = (unsigned long)fp.data - (unsigned long)data;
+ return 0;
+}
+
/**
* security_read_policy - read the policy.
+ * @state: selinux_state
* @data: binary policy data
* @len: length of data in bytes
*
@@ -3885,8 +3912,6 @@ int security_read_policy(struct selinux_state *state,
void **data, size_t *len)
{
struct selinux_policy *policy;
- int rc;
- struct policy_file fp;
policy = rcu_dereference_protected(
state->policy, lockdep_is_held(&state->policy_mutex));
@@ -3898,14 +3923,35 @@ int security_read_policy(struct selinux_state *state,
if (!*data)
return -ENOMEM;
- fp.data = *data;
- fp.len = *len;
+ return __security_read_policy(policy, *data, len);
+}
- rc = policydb_write(&policy->policydb, &fp);
- if (rc)
- return rc;
+/**
+ * security_read_state_kernel - read the policy.
+ * @state: selinux_state
+ * @data: binary policy data
+ * @len: length of data in bytes
+ *
+ * Allocates kernel memory for reading SELinux policy.
+ * This function is for internal use only and should not
+ * be used for returning data to user space.
+ *
+ * This function must be called with policy_mutex held.
+ */
+int security_read_state_kernel(struct selinux_state *state,
+ void **data, size_t *len)
+{
+ struct selinux_policy *policy;
- *len = (unsigned long)fp.data - (unsigned long)*data;
- return 0;
+ policy = rcu_dereference_protected(
+ state->policy, lockdep_is_held(&state->policy_mutex));
+ if (!policy)
+ return -EINVAL;
+
+ *len = policy->policydb.len;
+ *data = vmalloc(*len);
+ if (!*data)
+ return -ENOMEM;
+ return __security_read_policy(policy, *data, len);
}
--
2.30.0
On Thu, Jan 14, 2021 at 2:15 PM Lakshmi Ramasubramanian
<[email protected]> wrote:
>
> SELinux stores the active policy in memory, so the changes to this data
> at runtime would have an impact on the security guarantees provided
> by SELinux. Measuring in-memory SELinux policy through IMA subsystem
> provides a secure way for the attestation service to remotely validate
> the policy contents at runtime.
>
> Measure the hash of the loaded policy by calling the IMA hook
> ima_measure_critical_data(). Since the size of the loaded policy
> can be large (several MB), measure the hash of the policy instead of
> the entire policy to avoid bloating the IMA log entry.
>
> To enable SELinux data measurement, the following steps are required:
>
> 1, Add "ima_policy=critical_data" to the kernel command line arguments
> to enable measuring SELinux data at boot time.
> For example,
> BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data
>
> 2, Add the following rule to /etc/ima/ima-policy
> measure func=CRITICAL_DATA label=selinux
>
> Sample measurement of the hash of SELinux policy:
>
> To verify the measured data with the current SELinux policy run
> the following commands and verify the output hash values match.
>
> sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
>
> grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6
>
> Note that the actual verification of SELinux policy would require loading
> the expected policy into an identical kernel on a pristine/known-safe
> system and run the sha256sum /sys/kernel/selinux/policy there to get
> the expected hash.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> Suggested-by: Stephen Smalley <[email protected]>
> Acked-by: Paul Moore <[email protected]>
> Reviewed-by: Tyler Hicks <[email protected]>
> ---
> Documentation/ABI/testing/ima_policy | 3 +-
> security/selinux/Makefile | 2 +
> security/selinux/ima.c | 44 +++++++++++++++++++
> security/selinux/include/ima.h | 24 +++++++++++
> security/selinux/include/security.h | 3 +-
> security/selinux/ss/services.c | 64 ++++++++++++++++++++++++----
> 6 files changed, 129 insertions(+), 11 deletions(-)
> create mode 100644 security/selinux/ima.c
> create mode 100644 security/selinux/include/ima.h
I think this has changed enough that keeping the "Acked-by" and
"Reviewed-by" tags is probably not a good choice. I took a quick look
and this still looks okay from a SELinux perspective, I'll leave Mimi
to comment on it from a IMA perspective.
Unless Tyler has reviewed this version prior to your posting, it might
be a good idea to remove his "Reviewed-by" unless he has a chance to
look this over again before it is merged.
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 54fe1c15ed50..8365596cb42b 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -52,8 +52,9 @@ Description:
> template:= name of a defined IMA template type
> (eg, ima-ng). Only valid when action is "measure".
> pcr:= decimal value
> - label:= [data_label]
> + label:= [selinux]|[data_label]
> data_label:= a unique string used for grouping and limiting critical data.
> + For example, "selinux" to measure critical data for SELinux.
>
> default policy:
> # PROC_SUPER_MAGIC
> diff --git a/security/selinux/Makefile b/security/selinux/Makefile
> index 4d8e0e8adf0b..776162444882 100644
> --- a/security/selinux/Makefile
> +++ b/security/selinux/Makefile
> @@ -16,6 +16,8 @@ selinux-$(CONFIG_NETLABEL) += netlabel.o
>
> selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o
>
> +selinux-$(CONFIG_IMA) += ima.o
> +
> ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include
>
> $(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h
> diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> new file mode 100644
> index 000000000000..03715893ff97
> --- /dev/null
> +++ b/security/selinux/ima.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Microsoft Corporation
> + *
> + * Author: Lakshmi Ramasubramanian ([email protected])
> + *
> + * Measure critical data structures maintainted by SELinux
> + * using IMA subsystem.
> + */
> +#include <linux/vmalloc.h>
> +#include <linux/ima.h>
> +#include "security.h"
> +#include "ima.h"
> +
> +/*
> + * selinux_ima_measure_state - Measure hash of the SELinux policy
> + *
> + * @state: selinux state struct
> + *
> + * NOTE: This function must be called with policy_mutex held.
> + */
> +void selinux_ima_measure_state(struct selinux_state *state)
> +{
> + void *policy = NULL;
> + size_t policy_len;
> + int rc = 0;
> +
> + /*
> + * Measure SELinux policy only after initialization is completed.
> + */
> + if (!selinux_initialized(state))
> + return;
> +
> + rc = security_read_state_kernel(state, &policy, &policy_len);
> + if (rc) {
> + pr_err("SELinux: %s: failed to read policy %d.\n", __func__, rc);
> + return;
> + }
> +
> + ima_measure_critical_data("selinux", "selinux-policy-hash",
> + policy, policy_len, true);
> +
> + vfree(policy);
> +}
> diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h
> new file mode 100644
> index 000000000000..d69c36611423
> --- /dev/null
> +++ b/security/selinux/include/ima.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2021 Microsoft Corporation
> + *
> + * Author: Lakshmi Ramasubramanian ([email protected])
> + *
> + * Measure critical data structures maintainted by SELinux
> + * using IMA subsystem.
> + */
> +
> +#ifndef _SELINUX_IMA_H_
> +#define _SELINUX_IMA_H_
> +
> +#include "security.h"
> +
> +#ifdef CONFIG_IMA
> +extern void selinux_ima_measure_state(struct selinux_state *selinux_state);
> +#else
> +static inline void selinux_ima_measure_state(struct selinux_state *selinux_state)
> +{
> +}
> +#endif
> +
> +#endif /* _SELINUX_IMA_H_ */
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 3cc8bab31ea8..29cae32d3fc5 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -229,7 +229,8 @@ void selinux_policy_cancel(struct selinux_state *state,
> struct selinux_policy *policy);
> int security_read_policy(struct selinux_state *state,
> void **data, size_t *len);
> -
> +int security_read_state_kernel(struct selinux_state *state,
> + void **data, size_t *len);
> int security_policycap_supported(struct selinux_state *state,
> unsigned int req_cap);
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 9704c8a32303..cc8dbc4ed8db 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -65,6 +65,7 @@
> #include "ebitmap.h"
> #include "audit.h"
> #include "policycap_names.h"
> +#include "ima.h"
>
> /* Forward declaration. */
> static int context_struct_to_string(struct policydb *policydb,
> @@ -2180,6 +2181,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);
> }
>
> void selinux_policy_commit(struct selinux_state *state,
> @@ -3875,8 +3877,33 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
> }
> #endif /* CONFIG_NETLABEL */
>
> +/**
> + * __security_read_policy - read the policy.
> + * @policy: SELinux policy
> + * @data: binary policy data
> + * @len: length of data in bytes
> + *
> + */
> +static int __security_read_policy(struct selinux_policy *policy,
> + void *data, size_t *len)
> +{
> + int rc;
> + struct policy_file fp;
> +
> + fp.data = data;
> + fp.len = *len;
> +
> + rc = policydb_write(&policy->policydb, &fp);
> + if (rc)
> + return rc;
> +
> + *len = (unsigned long)fp.data - (unsigned long)data;
> + return 0;
> +}
> +
> /**
> * security_read_policy - read the policy.
> + * @state: selinux_state
> * @data: binary policy data
> * @len: length of data in bytes
> *
> @@ -3885,8 +3912,6 @@ int security_read_policy(struct selinux_state *state,
> void **data, size_t *len)
> {
> struct selinux_policy *policy;
> - int rc;
> - struct policy_file fp;
>
> policy = rcu_dereference_protected(
> state->policy, lockdep_is_held(&state->policy_mutex));
> @@ -3898,14 +3923,35 @@ int security_read_policy(struct selinux_state *state,
> if (!*data)
> return -ENOMEM;
>
> - fp.data = *data;
> - fp.len = *len;
> + return __security_read_policy(policy, *data, len);
> +}
>
> - rc = policydb_write(&policy->policydb, &fp);
> - if (rc)
> - return rc;
> +/**
> + * security_read_state_kernel - read the policy.
> + * @state: selinux_state
> + * @data: binary policy data
> + * @len: length of data in bytes
> + *
> + * Allocates kernel memory for reading SELinux policy.
> + * This function is for internal use only and should not
> + * be used for returning data to user space.
> + *
> + * This function must be called with policy_mutex held.
> + */
> +int security_read_state_kernel(struct selinux_state *state,
> + void **data, size_t *len)
> +{
> + struct selinux_policy *policy;
>
> - *len = (unsigned long)fp.data - (unsigned long)*data;
> - return 0;
> + policy = rcu_dereference_protected(
> + state->policy, lockdep_is_held(&state->policy_mutex));
> + if (!policy)
> + return -EINVAL;
> +
> + *len = policy->policydb.len;
> + *data = vmalloc(*len);
> + if (!*data)
> + return -ENOMEM;
>
> + return __security_read_policy(policy, *data, len);
> }
> --
> 2.30.0
>
--
paul moore
http://www.paul-moore.com
On 2021-01-14 14:29:09, Paul Moore wrote:
> On Thu, Jan 14, 2021 at 2:15 PM Lakshmi Ramasubramanian
> <[email protected]> wrote:
> >
> > SELinux stores the active policy in memory, so the changes to this data
> > at runtime would have an impact on the security guarantees provided
> > by SELinux. Measuring in-memory SELinux policy through IMA subsystem
> > provides a secure way for the attestation service to remotely validate
> > the policy contents at runtime.
> >
> > Measure the hash of the loaded policy by calling the IMA hook
> > ima_measure_critical_data(). Since the size of the loaded policy
> > can be large (several MB), measure the hash of the policy instead of
> > the entire policy to avoid bloating the IMA log entry.
> >
> > To enable SELinux data measurement, the following steps are required:
> >
> > 1, Add "ima_policy=critical_data" to the kernel command line arguments
> > to enable measuring SELinux data at boot time.
> > For example,
> > BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data
> >
> > 2, Add the following rule to /etc/ima/ima-policy
> > measure func=CRITICAL_DATA label=selinux
> >
> > Sample measurement of the hash of SELinux policy:
> >
> > To verify the measured data with the current SELinux policy run
> > the following commands and verify the output hash values match.
> >
> > sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
> >
> > grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6
> >
> > Note that the actual verification of SELinux policy would require loading
> > the expected policy into an identical kernel on a pristine/known-safe
> > system and run the sha256sum /sys/kernel/selinux/policy there to get
> > the expected hash.
> >
> > Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> > Suggested-by: Stephen Smalley <[email protected]>
> > Acked-by: Paul Moore <[email protected]>
> > Reviewed-by: Tyler Hicks <[email protected]>
> > ---
> > Documentation/ABI/testing/ima_policy | 3 +-
> > security/selinux/Makefile | 2 +
> > security/selinux/ima.c | 44 +++++++++++++++++++
> > security/selinux/include/ima.h | 24 +++++++++++
> > security/selinux/include/security.h | 3 +-
> > security/selinux/ss/services.c | 64 ++++++++++++++++++++++++----
> > 6 files changed, 129 insertions(+), 11 deletions(-)
> > create mode 100644 security/selinux/ima.c
> > create mode 100644 security/selinux/include/ima.h
>
> I think this has changed enough that keeping the "Acked-by" and
> "Reviewed-by" tags is probably not a good choice. I took a quick look
> and this still looks okay from a SELinux perspective, I'll leave Mimi
> to comment on it from a IMA perspective.
>
> Unless Tyler has reviewed this version prior to your posting, it might
> be a good idea to remove his "Reviewed-by" unless he has a chance to
> look this over again before it is merged.
Thanks for calling this out. I hadn't reviewed it prior to the posting
but I was keeping an eye on the thread.
This new revision still looks good to me and I like the idea of
controlling re-measurements via policy. So,
Reviewed-by: Tyler Hicks <[email protected]>
Tyler
>
> > diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> > index 54fe1c15ed50..8365596cb42b 100644
> > --- a/Documentation/ABI/testing/ima_policy
> > +++ b/Documentation/ABI/testing/ima_policy
> > @@ -52,8 +52,9 @@ Description:
> > template:= name of a defined IMA template type
> > (eg, ima-ng). Only valid when action is "measure".
> > pcr:= decimal value
> > - label:= [data_label]
> > + label:= [selinux]|[data_label]
> > data_label:= a unique string used for grouping and limiting critical data.
> > + For example, "selinux" to measure critical data for SELinux.
> >
> > default policy:
> > # PROC_SUPER_MAGIC
> > diff --git a/security/selinux/Makefile b/security/selinux/Makefile
> > index 4d8e0e8adf0b..776162444882 100644
> > --- a/security/selinux/Makefile
> > +++ b/security/selinux/Makefile
> > @@ -16,6 +16,8 @@ selinux-$(CONFIG_NETLABEL) += netlabel.o
> >
> > selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o
> >
> > +selinux-$(CONFIG_IMA) += ima.o
> > +
> > ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include
> >
> > $(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h
> > diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> > new file mode 100644
> > index 000000000000..03715893ff97
> > --- /dev/null
> > +++ b/security/selinux/ima.c
> > @@ -0,0 +1,44 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2021 Microsoft Corporation
> > + *
> > + * Author: Lakshmi Ramasubramanian ([email protected])
> > + *
> > + * Measure critical data structures maintainted by SELinux
> > + * using IMA subsystem.
> > + */
> > +#include <linux/vmalloc.h>
> > +#include <linux/ima.h>
> > +#include "security.h"
> > +#include "ima.h"
> > +
> > +/*
> > + * selinux_ima_measure_state - Measure hash of the SELinux policy
> > + *
> > + * @state: selinux state struct
> > + *
> > + * NOTE: This function must be called with policy_mutex held.
> > + */
> > +void selinux_ima_measure_state(struct selinux_state *state)
> > +{
> > + void *policy = NULL;
> > + size_t policy_len;
> > + int rc = 0;
> > +
> > + /*
> > + * Measure SELinux policy only after initialization is completed.
> > + */
> > + if (!selinux_initialized(state))
> > + return;
> > +
> > + rc = security_read_state_kernel(state, &policy, &policy_len);
> > + if (rc) {
> > + pr_err("SELinux: %s: failed to read policy %d.\n", __func__, rc);
> > + return;
> > + }
> > +
> > + ima_measure_critical_data("selinux", "selinux-policy-hash",
> > + policy, policy_len, true);
> > +
> > + vfree(policy);
> > +}
> > diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h
> > new file mode 100644
> > index 000000000000..d69c36611423
> > --- /dev/null
> > +++ b/security/selinux/include/ima.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2021 Microsoft Corporation
> > + *
> > + * Author: Lakshmi Ramasubramanian ([email protected])
> > + *
> > + * Measure critical data structures maintainted by SELinux
> > + * using IMA subsystem.
> > + */
> > +
> > +#ifndef _SELINUX_IMA_H_
> > +#define _SELINUX_IMA_H_
> > +
> > +#include "security.h"
> > +
> > +#ifdef CONFIG_IMA
> > +extern void selinux_ima_measure_state(struct selinux_state *selinux_state);
> > +#else
> > +static inline void selinux_ima_measure_state(struct selinux_state *selinux_state)
> > +{
> > +}
> > +#endif
> > +
> > +#endif /* _SELINUX_IMA_H_ */
> > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> > index 3cc8bab31ea8..29cae32d3fc5 100644
> > --- a/security/selinux/include/security.h
> > +++ b/security/selinux/include/security.h
> > @@ -229,7 +229,8 @@ void selinux_policy_cancel(struct selinux_state *state,
> > struct selinux_policy *policy);
> > int security_read_policy(struct selinux_state *state,
> > void **data, size_t *len);
> > -
> > +int security_read_state_kernel(struct selinux_state *state,
> > + void **data, size_t *len);
> > int security_policycap_supported(struct selinux_state *state,
> > unsigned int req_cap);
> >
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index 9704c8a32303..cc8dbc4ed8db 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -65,6 +65,7 @@
> > #include "ebitmap.h"
> > #include "audit.h"
> > #include "policycap_names.h"
> > +#include "ima.h"
> >
> > /* Forward declaration. */
> > static int context_struct_to_string(struct policydb *policydb,
> > @@ -2180,6 +2181,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);
> > }
> >
> > void selinux_policy_commit(struct selinux_state *state,
> > @@ -3875,8 +3877,33 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
> > }
> > #endif /* CONFIG_NETLABEL */
> >
> > +/**
> > + * __security_read_policy - read the policy.
> > + * @policy: SELinux policy
> > + * @data: binary policy data
> > + * @len: length of data in bytes
> > + *
> > + */
> > +static int __security_read_policy(struct selinux_policy *policy,
> > + void *data, size_t *len)
> > +{
> > + int rc;
> > + struct policy_file fp;
> > +
> > + fp.data = data;
> > + fp.len = *len;
> > +
> > + rc = policydb_write(&policy->policydb, &fp);
> > + if (rc)
> > + return rc;
> > +
> > + *len = (unsigned long)fp.data - (unsigned long)data;
> > + return 0;
> > +}
> > +
> > /**
> > * security_read_policy - read the policy.
> > + * @state: selinux_state
> > * @data: binary policy data
> > * @len: length of data in bytes
> > *
> > @@ -3885,8 +3912,6 @@ int security_read_policy(struct selinux_state *state,
> > void **data, size_t *len)
> > {
> > struct selinux_policy *policy;
> > - int rc;
> > - struct policy_file fp;
> >
> > policy = rcu_dereference_protected(
> > state->policy, lockdep_is_held(&state->policy_mutex));
> > @@ -3898,14 +3923,35 @@ int security_read_policy(struct selinux_state *state,
> > if (!*data)
> > return -ENOMEM;
> >
> > - fp.data = *data;
> > - fp.len = *len;
> > + return __security_read_policy(policy, *data, len);
> > +}
> >
> > - rc = policydb_write(&policy->policydb, &fp);
> > - if (rc)
> > - return rc;
> > +/**
> > + * security_read_state_kernel - read the policy.
> > + * @state: selinux_state
> > + * @data: binary policy data
> > + * @len: length of data in bytes
> > + *
> > + * Allocates kernel memory for reading SELinux policy.
> > + * This function is for internal use only and should not
> > + * be used for returning data to user space.
> > + *
> > + * This function must be called with policy_mutex held.
> > + */
> > +int security_read_state_kernel(struct selinux_state *state,
> > + void **data, size_t *len)
> > +{
> > + struct selinux_policy *policy;
> >
> > - *len = (unsigned long)fp.data - (unsigned long)*data;
> > - return 0;
> > + policy = rcu_dereference_protected(
> > + state->policy, lockdep_is_held(&state->policy_mutex));
> > + if (!policy)
> > + return -EINVAL;
> > +
> > + *len = policy->policydb.len;
> > + *data = vmalloc(*len);
> > + if (!*data)
> > + return -ENOMEM;
> >
> > + return __security_read_policy(policy, *data, len);
> > }
> > --
> > 2.30.0
> >
>
>
> --
> paul moore
> http://www.paul-moore.com
>
On 1/14/21 11:58 AM, Tyler Hicks wrote:
> On 2021-01-14 14:29:09, Paul Moore wrote:
>> On Thu, Jan 14, 2021 at 2:15 PM Lakshmi Ramasubramanian
>> <[email protected]> wrote:
>>>
>>> SELinux stores the active policy in memory, so the changes to this data
>>> at runtime would have an impact on the security guarantees provided
>>> by SELinux. Measuring in-memory SELinux policy through IMA subsystem
>>> provides a secure way for the attestation service to remotely validate
>>> the policy contents at runtime.
>>>
>>> Measure the hash of the loaded policy by calling the IMA hook
>>> ima_measure_critical_data(). Since the size of the loaded policy
>>> can be large (several MB), measure the hash of the policy instead of
>>> the entire policy to avoid bloating the IMA log entry.
>>>
>>> To enable SELinux data measurement, the following steps are required:
>>>
>>> 1, Add "ima_policy=critical_data" to the kernel command line arguments
>>> to enable measuring SELinux data at boot time.
>>> For example,
>>> BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data
>>>
>>> 2, Add the following rule to /etc/ima/ima-policy
>>> measure func=CRITICAL_DATA label=selinux
>>>
>>> Sample measurement of the hash of SELinux policy:
>>>
>>> To verify the measured data with the current SELinux policy run
>>> the following commands and verify the output hash values match.
>>>
>>> sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
>>>
>>> grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6
>>>
>>> Note that the actual verification of SELinux policy would require loading
>>> the expected policy into an identical kernel on a pristine/known-safe
>>> system and run the sha256sum /sys/kernel/selinux/policy there to get
>>> the expected hash.
>>>
>>> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
>>> Suggested-by: Stephen Smalley <[email protected]>
>>> Acked-by: Paul Moore <[email protected]>
>>> Reviewed-by: Tyler Hicks <[email protected]>
>>> ---
>>> Documentation/ABI/testing/ima_policy | 3 +-
>>> security/selinux/Makefile | 2 +
>>> security/selinux/ima.c | 44 +++++++++++++++++++
>>> security/selinux/include/ima.h | 24 +++++++++++
>>> security/selinux/include/security.h | 3 +-
>>> security/selinux/ss/services.c | 64 ++++++++++++++++++++++++----
>>> 6 files changed, 129 insertions(+), 11 deletions(-)
>>> create mode 100644 security/selinux/ima.c
>>> create mode 100644 security/selinux/include/ima.h
>>
>> I think this has changed enough that keeping the "Acked-by" and
>> "Reviewed-by" tags is probably not a good choice. I took a quick look
>> and this still looks okay from a SELinux perspective, I'll leave Mimi
>> to comment on it from a IMA perspective.
Thanks for reviewing the change Paul.
>>
>> Unless Tyler has reviewed this version prior to your posting, it might
>> be a good idea to remove his "Reviewed-by" unless he has a chance to
>> look this over again before it is merged.
>
> Thanks for calling this out. I hadn't reviewed it prior to the posting
> but I was keeping an eye on the thread.
>
> This new revision still looks good to me and I like the idea of
> controlling re-measurements via policy. So,
>
> Reviewed-by: Tyler Hicks <[email protected]>
Thanks for the quick response Tyler.
-lakshmi
On Thu, Jan 14, 2021 at 2:15 PM Lakshmi Ramasubramanian
<[email protected]> wrote:
>
> SELinux stores the active policy in memory, so the changes to this data
> at runtime would have an impact on the security guarantees provided
> by SELinux. Measuring in-memory SELinux policy through IMA subsystem
> provides a secure way for the attestation service to remotely validate
> the policy contents at runtime.
>
> Measure the hash of the loaded policy by calling the IMA hook
> ima_measure_critical_data(). Since the size of the loaded policy
> can be large (several MB), measure the hash of the policy instead of
> the entire policy to avoid bloating the IMA log entry.
>
> To enable SELinux data measurement, the following steps are required:
>
> 1, Add "ima_policy=critical_data" to the kernel command line arguments
> to enable measuring SELinux data at boot time.
> For example,
> BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data
>
> 2, Add the following rule to /etc/ima/ima-policy
> measure func=CRITICAL_DATA label=selinux
>
> Sample measurement of the hash of SELinux policy:
>
> To verify the measured data with the current SELinux policy run
> the following commands and verify the output hash values match.
>
> sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
>
> grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6
>
> Note that the actual verification of SELinux policy would require loading
> the expected policy into an identical kernel on a pristine/known-safe
> system and run the sha256sum /sys/kernel/selinux/policy there to get
> the expected hash.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> Suggested-by: Stephen Smalley <[email protected]>
> Acked-by: Paul Moore <[email protected]>
> Reviewed-by: Tyler Hicks <[email protected]>
> ---
> Documentation/ABI/testing/ima_policy | 3 +-
> security/selinux/Makefile | 2 +
> security/selinux/ima.c | 44 +++++++++++++++++++
> security/selinux/include/ima.h | 24 +++++++++++
> security/selinux/include/security.h | 3 +-
> security/selinux/ss/services.c | 64 ++++++++++++++++++++++++----
> 6 files changed, 129 insertions(+), 11 deletions(-)
> create mode 100644 security/selinux/ima.c
> create mode 100644 security/selinux/include/ima.h
Hi Mimi,
Just checking as I didn't see a reply to this from you in my inbox,
you merged this into the IMA linux-next branch, yes?
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 54fe1c15ed50..8365596cb42b 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -52,8 +52,9 @@ Description:
> template:= name of a defined IMA template type
> (eg, ima-ng). Only valid when action is "measure".
> pcr:= decimal value
> - label:= [data_label]
> + label:= [selinux]|[data_label]
> data_label:= a unique string used for grouping and limiting critical data.
> + For example, "selinux" to measure critical data for SELinux.
>
> default policy:
> # PROC_SUPER_MAGIC
> diff --git a/security/selinux/Makefile b/security/selinux/Makefile
> index 4d8e0e8adf0b..776162444882 100644
> --- a/security/selinux/Makefile
> +++ b/security/selinux/Makefile
> @@ -16,6 +16,8 @@ selinux-$(CONFIG_NETLABEL) += netlabel.o
>
> selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o
>
> +selinux-$(CONFIG_IMA) += ima.o
> +
> ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include
>
> $(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h
> diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> new file mode 100644
> index 000000000000..03715893ff97
> --- /dev/null
> +++ b/security/selinux/ima.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Microsoft Corporation
> + *
> + * Author: Lakshmi Ramasubramanian ([email protected])
> + *
> + * Measure critical data structures maintainted by SELinux
> + * using IMA subsystem.
> + */
> +#include <linux/vmalloc.h>
> +#include <linux/ima.h>
> +#include "security.h"
> +#include "ima.h"
> +
> +/*
> + * selinux_ima_measure_state - Measure hash of the SELinux policy
> + *
> + * @state: selinux state struct
> + *
> + * NOTE: This function must be called with policy_mutex held.
> + */
> +void selinux_ima_measure_state(struct selinux_state *state)
> +{
> + void *policy = NULL;
> + size_t policy_len;
> + int rc = 0;
> +
> + /*
> + * Measure SELinux policy only after initialization is completed.
> + */
> + if (!selinux_initialized(state))
> + return;
> +
> + rc = security_read_state_kernel(state, &policy, &policy_len);
> + if (rc) {
> + pr_err("SELinux: %s: failed to read policy %d.\n", __func__, rc);
> + return;
> + }
> +
> + ima_measure_critical_data("selinux", "selinux-policy-hash",
> + policy, policy_len, true);
> +
> + vfree(policy);
> +}
> diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h
> new file mode 100644
> index 000000000000..d69c36611423
> --- /dev/null
> +++ b/security/selinux/include/ima.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2021 Microsoft Corporation
> + *
> + * Author: Lakshmi Ramasubramanian ([email protected])
> + *
> + * Measure critical data structures maintainted by SELinux
> + * using IMA subsystem.
> + */
> +
> +#ifndef _SELINUX_IMA_H_
> +#define _SELINUX_IMA_H_
> +
> +#include "security.h"
> +
> +#ifdef CONFIG_IMA
> +extern void selinux_ima_measure_state(struct selinux_state *selinux_state);
> +#else
> +static inline void selinux_ima_measure_state(struct selinux_state *selinux_state)
> +{
> +}
> +#endif
> +
> +#endif /* _SELINUX_IMA_H_ */
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 3cc8bab31ea8..29cae32d3fc5 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -229,7 +229,8 @@ void selinux_policy_cancel(struct selinux_state *state,
> struct selinux_policy *policy);
> int security_read_policy(struct selinux_state *state,
> void **data, size_t *len);
> -
> +int security_read_state_kernel(struct selinux_state *state,
> + void **data, size_t *len);
> int security_policycap_supported(struct selinux_state *state,
> unsigned int req_cap);
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 9704c8a32303..cc8dbc4ed8db 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -65,6 +65,7 @@
> #include "ebitmap.h"
> #include "audit.h"
> #include "policycap_names.h"
> +#include "ima.h"
>
> /* Forward declaration. */
> static int context_struct_to_string(struct policydb *policydb,
> @@ -2180,6 +2181,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);
> }
>
> void selinux_policy_commit(struct selinux_state *state,
> @@ -3875,8 +3877,33 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
> }
> #endif /* CONFIG_NETLABEL */
>
> +/**
> + * __security_read_policy - read the policy.
> + * @policy: SELinux policy
> + * @data: binary policy data
> + * @len: length of data in bytes
> + *
> + */
> +static int __security_read_policy(struct selinux_policy *policy,
> + void *data, size_t *len)
> +{
> + int rc;
> + struct policy_file fp;
> +
> + fp.data = data;
> + fp.len = *len;
> +
> + rc = policydb_write(&policy->policydb, &fp);
> + if (rc)
> + return rc;
> +
> + *len = (unsigned long)fp.data - (unsigned long)data;
> + return 0;
> +}
> +
> /**
> * security_read_policy - read the policy.
> + * @state: selinux_state
> * @data: binary policy data
> * @len: length of data in bytes
> *
> @@ -3885,8 +3912,6 @@ int security_read_policy(struct selinux_state *state,
> void **data, size_t *len)
> {
> struct selinux_policy *policy;
> - int rc;
> - struct policy_file fp;
>
> policy = rcu_dereference_protected(
> state->policy, lockdep_is_held(&state->policy_mutex));
> @@ -3898,14 +3923,35 @@ int security_read_policy(struct selinux_state *state,
> if (!*data)
> return -ENOMEM;
>
> - fp.data = *data;
> - fp.len = *len;
> + return __security_read_policy(policy, *data, len);
> +}
>
> - rc = policydb_write(&policy->policydb, &fp);
> - if (rc)
> - return rc;
> +/**
> + * security_read_state_kernel - read the policy.
> + * @state: selinux_state
> + * @data: binary policy data
> + * @len: length of data in bytes
> + *
> + * Allocates kernel memory for reading SELinux policy.
> + * This function is for internal use only and should not
> + * be used for returning data to user space.
> + *
> + * This function must be called with policy_mutex held.
> + */
> +int security_read_state_kernel(struct selinux_state *state,
> + void **data, size_t *len)
> +{
> + struct selinux_policy *policy;
>
> - *len = (unsigned long)fp.data - (unsigned long)*data;
> - return 0;
> + policy = rcu_dereference_protected(
> + state->policy, lockdep_is_held(&state->policy_mutex));
> + if (!policy)
> + return -EINVAL;
> +
> + *len = policy->policydb.len;
> + *data = vmalloc(*len);
> + if (!*data)
> + return -ENOMEM;
>
> + return __security_read_policy(policy, *data, len);
> }
> --
> 2.30.0
--
paul moore
http://www.paul-moore.com
On Fri, 2021-01-22 at 15:24 -0500, Paul Moore wrote:
> On Thu, Jan 14, 2021 at 2:15 PM Lakshmi Ramasubramanian
> <[email protected]> wrote:
> >
> > SELinux stores the active policy in memory, so the changes to this data
> > at runtime would have an impact on the security guarantees provided
> > by SELinux. Measuring in-memory SELinux policy through IMA subsystem
> > provides a secure way for the attestation service to remotely validate
> > the policy contents at runtime.
> >
> > Measure the hash of the loaded policy by calling the IMA hook
> > ima_measure_critical_data(). Since the size of the loaded policy
> > can be large (several MB), measure the hash of the policy instead of
> > the entire policy to avoid bloating the IMA log entry.
> >
> > To enable SELinux data measurement, the following steps are required:
> >
> > 1, Add "ima_policy=critical_data" to the kernel command line arguments
> > to enable measuring SELinux data at boot time.
> > For example,
> > BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data
> >
> > 2, Add the following rule to /etc/ima/ima-policy
> > measure func=CRITICAL_DATA label=selinux
> >
> > Sample measurement of the hash of SELinux policy:
> >
> > To verify the measured data with the current SELinux policy run
> > the following commands and verify the output hash values match.
> >
> > sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1
> >
> > grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6
> >
> > Note that the actual verification of SELinux policy would require loading
> > the expected policy into an identical kernel on a pristine/known-safe
> > system and run the sha256sum /sys/kernel/selinux/policy there to get
> > the expected hash.
> >
> > Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> > Suggested-by: Stephen Smalley <[email protected]>
> > Acked-by: Paul Moore <[email protected]>
> > Reviewed-by: Tyler Hicks <[email protected]>
> > ---
> > Documentation/ABI/testing/ima_policy | 3 +-
> > security/selinux/Makefile | 2 +
> > security/selinux/ima.c | 44 +++++++++++++++++++
> > security/selinux/include/ima.h | 24 +++++++++++
> > security/selinux/include/security.h | 3 +-
> > security/selinux/ss/services.c | 64 ++++++++++++++++++++++++----
> > 6 files changed, 129 insertions(+), 11 deletions(-)
> > create mode 100644 security/selinux/ima.c
> > create mode 100644 security/selinux/include/ima.h
>
> Hi Mimi,
>
> Just checking as I didn't see a reply to this from you in my inbox,
> you merged this into the IMA linux-next branch, yes?
The patches are first staged in the linux-integrity #next-integrity-
testing branch, before being staged in the #next-integrity branch,
which is picked up by linux-next. Sorry, they've been staged in the
next-integrity-testing branch, but are now next-integrity.
Mim