* [PATCH v3 1/5] IMA: Add LSM_STATE func to measure LSM data
2020-07-17 22:28 [PATCH v3 0/5] LSM: Measure security module state Lakshmi Ramasubramanian
@ 2020-07-17 22:28 ` Lakshmi Ramasubramanian
2020-07-17 22:28 ` [PATCH v3 2/5] IMA: Define an IMA hook " Lakshmi Ramasubramanian
` (3 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-17 22:28 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey
Cc: jmorris, linux-integrity, selinux, linux-security-module, linux-kernel
Critical data structures of security modules need to be measured to
enable an attestation service to verify if the policies and
configuration have been setup correctly and that they haven't been
tampered with at runtime. A new IMA policy is required for handling
this measurement.
Define a new IMA policy func namely LSM_STATE to measure data provided
by security modules. Update ima_match_rules() to check for LSM_STATE
and ima_parse_rule() to handle LSM_STATE.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
Documentation/ABI/testing/ima_policy | 6 +++++-
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_policy.c | 29 +++++++++++++++++++++++-----
4 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index cd572912c593..355bc3eade33 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -29,7 +29,7 @@ Description:
base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
- [KEXEC_CMDLINE] [KEY_CHECK]
+ [KEXEC_CMDLINE] [KEY_CHECK] [LSM_STATE]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
@@ -125,3 +125,7 @@ Description:
keys added to .builtin_trusted_keys or .ima keyring:
measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
+
+ Example of measure rule using LSM_STATE to measure LSM data:
+
+ measure func=LSM_STATE
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 4515975cc540..880fda11a61b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -200,6 +200,7 @@ static inline unsigned int ima_hash_key(u8 *digest)
hook(POLICY_CHECK, policy) \
hook(KEXEC_CMDLINE, kexec_cmdline) \
hook(KEY_CHECK, key) \
+ hook(LSM_STATE, lsm_state) \
hook(MAX_CHECK, none)
#define __ima_hook_enumify(ENUM, str) ENUM,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index bf22de8b7ce0..0cebd2404dcf 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -176,7 +176,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* subj=, obj=, type=, func=, mask=, fsmagic=
* subj,obj, and type: are LSM specific.
* func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
- * | KEXEC_CMDLINE | KEY_CHECK
+ * | KEXEC_CMDLINE | KEY_CHECK | LSM_STATE
* mask: contains the permission mask
* fsmagic: hex value
*
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 66aa3e17a888..fc8457d9242b 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -417,15 +417,31 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
const char *keyring)
{
int i;
+ int funcmatch = 0;
- if ((func == KEXEC_CMDLINE) || (func == KEY_CHECK)) {
+ switch (func) {
+ case KEXEC_CMDLINE:
+ case KEY_CHECK:
+ case LSM_STATE:
if ((rule->flags & IMA_FUNC) && (rule->func == func)) {
if (func == KEY_CHECK)
- return ima_match_keyring(rule, keyring, cred);
- return true;
- }
- return false;
+ funcmatch = ima_match_keyring(rule, keyring,
+ cred) ? 1 : -1;
+ else
+ funcmatch = 1;
+ } else
+ funcmatch = -1;
+
+ break;
+
+ default:
+ funcmatch = 0;
+ break;
}
+
+ if (funcmatch)
+ return (funcmatch == 1) ? true : false;
+
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
return false;
@@ -1068,6 +1084,9 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->func = KEXEC_CMDLINE;
else if (strcmp(args[0].from, "KEY_CHECK") == 0)
entry->func = KEY_CHECK;
+ else if (strcmp(args[0].from, "LSM_STATE") == 0)
+ entry->func = LSM_STATE;
+
else
result = -EINVAL;
if (!result)
--
2.27.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/5] IMA: Define an IMA hook to measure LSM data
2020-07-17 22:28 [PATCH v3 0/5] LSM: Measure security module state Lakshmi Ramasubramanian
2020-07-17 22:28 ` [PATCH v3 1/5] IMA: Add LSM_STATE func to measure LSM data Lakshmi Ramasubramanian
@ 2020-07-17 22:28 ` Lakshmi Ramasubramanian
2020-07-17 22:28 ` [PATCH v3 3/5] LSM: Add security_measure_data in lsm_info struct Lakshmi Ramasubramanian
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-17 22:28 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey
Cc: jmorris, linux-integrity, selinux, linux-security-module, linux-kernel
IMA subsystem needs to define an IMA hook that the security modules can
call to measure critical data of the security modules.
Define a new IMA hook, namely ima_lsm_state(), that the security modules
can call to measure data.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
include/linux/ima.h | 4 ++++
security/integrity/ima/ima_main.c | 17 +++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 9164e1534ec9..7e2686f4953a 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,6 +26,7 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
extern void ima_post_path_mknod(struct dentry *dentry);
extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
extern void ima_kexec_cmdline(const void *buf, int size);
+extern void ima_lsm_state(const char *lsm_event_name, const void *buf, int size);
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
@@ -104,6 +105,9 @@ static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
}
static inline void ima_kexec_cmdline(const void *buf, int size) {}
+
+static inline void ima_lsm_state(const char *lsm_event_name,
+ const void *buf, int size) {}
#endif /* CONFIG_IMA */
#ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8351b2fd48e0..04d9a1d35300 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -835,6 +835,23 @@ void ima_kexec_cmdline(const void *buf, int size)
KEXEC_CMDLINE, 0, NULL);
}
+/**
+ * ima_lsm_state - measure LSM specific state
+ * @lsm_event_name: LSM event
+ * @buf: pointer to buffer containing LSM specific state
+ * @size: Number of bytes in buf
+ *
+ * Buffers can only be measured, not appraised.
+ */
+void ima_lsm_state(const char *lsm_event_name, const void *buf, int size)
+{
+ if (!lsm_event_name || !buf || !size)
+ return;
+
+ process_buffer_measurement(buf, size, lsm_event_name,
+ LSM_STATE, 0, NULL);
+}
+
static int __init init_ima(void)
{
int error;
--
2.27.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 3/5] LSM: Add security_measure_data in lsm_info struct
2020-07-17 22:28 [PATCH v3 0/5] LSM: Measure security module state Lakshmi Ramasubramanian
2020-07-17 22:28 ` [PATCH v3 1/5] IMA: Add LSM_STATE func to measure LSM data Lakshmi Ramasubramanian
2020-07-17 22:28 ` [PATCH v3 2/5] IMA: Define an IMA hook " Lakshmi Ramasubramanian
@ 2020-07-17 22:28 ` Lakshmi Ramasubramanian
2020-07-17 22:28 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Lakshmi Ramasubramanian
2020-07-17 22:28 ` [PATCH v3 5/5] LSM: Define workqueue for measuring security module state Lakshmi Ramasubramanian
4 siblings, 0 replies; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-17 22:28 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey
Cc: jmorris, linux-integrity, selinux, linux-security-module, linux-kernel
The security modules that require their data to be measured using
the IMA subsystem need to define a function that the LSM can call
to trigger the measurement.
Add a function pointer field namely security_measure_data in lsm_info
structure. Update LSM to call this security module function, if defined,
to measure the security module's data using the IMA subsystem.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
include/linux/lsm_hooks.h | 3 +++
security/security.c | 48 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 95b7c1d32062..17afdf319c55 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1591,6 +1591,9 @@ struct lsm_info {
int *enabled; /* Optional: controlled by CONFIG_LSM */
int (*init)(void); /* Required. */
struct lsm_blob_sizes *blobs; /* Optional: for blob sharing. */
+ void (*security_measure_data)(void); /* Optional: for measuring
+ * security module data.
+ */
};
extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
diff --git a/security/security.c b/security/security.c
index 70a7ad357bc6..88ce1b780ffd 100644
--- a/security/security.c
+++ b/security/security.c
@@ -86,6 +86,9 @@ static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
static __initdata struct lsm_info **ordered_lsms;
static __initdata struct lsm_info *exclusive;
+static struct lsm_info *security_state_lsms;
+static int security_state_lsms_count;
+
static __initdata bool debug;
#define init_debug(...) \
do { \
@@ -235,6 +238,45 @@ static void __init initialize_lsm(struct lsm_info *lsm)
}
}
+static void measure_security_state(struct lsm_info *lsm)
+{
+ if (!lsm->security_measure_data)
+ return;
+
+ lsm->security_measure_data();
+}
+
+static void __init initialize_security_state_lsms(void)
+{
+ struct lsm_info **lsm;
+ int count = 0;
+ int inx;
+
+ for (lsm = ordered_lsms; *lsm; lsm++) {
+ if ((*lsm)->security_measure_data)
+ count++;
+ }
+
+ if (count == 0)
+ return;
+
+ security_state_lsms = kcalloc(count, sizeof(struct lsm_info),
+ GFP_KERNEL);
+ if (!security_state_lsms)
+ return;
+
+ inx = 0;
+ for (lsm = ordered_lsms; *lsm; lsm++) {
+ if ((*lsm)->security_measure_data) {
+ security_state_lsms[inx].security_measure_data =
+ (*lsm)->security_measure_data;
+ inx++;
+ }
+ }
+
+ security_state_lsms_count = count;
+}
+
/* Populate ordered LSMs list from comma-separated LSM name list. */
static void __init ordered_lsm_parse(const char *order, const char *origin)
{
@@ -352,8 +394,12 @@ static void __init ordered_lsm_init(void)
lsm_early_cred((struct cred *) current->cred);
lsm_early_task(current);
- for (lsm = ordered_lsms; *lsm; lsm++)
+ for (lsm = ordered_lsms; *lsm; lsm++) {
initialize_lsm(*lsm);
+ measure_security_state(*lsm);
+ }
+
+ initialize_security_state_lsms();
kfree(ordered_lsms);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-17 22:28 [PATCH v3 0/5] LSM: Measure security module state Lakshmi Ramasubramanian
` (2 preceding siblings ...)
2020-07-17 22:28 ` [PATCH v3 3/5] LSM: Add security_measure_data in lsm_info struct Lakshmi Ramasubramanian
@ 2020-07-17 22:28 ` Lakshmi Ramasubramanian
2020-07-18 3:14 ` kernel test robot
` (4 more replies)
2020-07-17 22:28 ` [PATCH v3 5/5] LSM: Define workqueue for measuring security module state Lakshmi Ramasubramanian
4 siblings, 5 replies; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-17 22:28 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey
Cc: jmorris, linux-integrity, selinux, linux-security-module, linux-kernel
SELinux configuration and policy are some of the critical data for this
security module that needs to be measured. To enable this measurement
SELinux needs to implement the interface function,
security_measure_data(), that the LSM can call.
Define the security_measure_data() function in SELinux to measure SELinux
configuration and policy. Call this function to measure SELinux data
when there is a change in the security module's state.
Sample measurement of SELinux state and hash of the policy:
10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state 656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b
10 f4a7...9408 ima-buf sha256:4941...68fc selinux-policy-hash 8d1d...1834
To verify the measurement check the following:
Execute the following command to extract the measured data
from the IMA log for SELinux configuration (selinux-state).
cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep -m 1 "selinux-state" | cut -d' ' -f 6 | xxd -r -p
The output should be the list of key-value pairs. For example,
enabled=1;enforcing=0;checkreqprot=1;network_peer_controls=1;open_perms=1;extended_socket_class=1;always_check_network=0;cgroup_seclabel=1;nnp_nosuid_transition=1;genfs_seclabel_symlinks=0;
To verify the measured data with the current SELinux state:
=> enabled should be set to 1 if /sys/fs/selinux folder exists,
0 otherwise
For other entries, compare the integer value in the files
=> /sys/fs/selinux/enforce
=> /sys/fs/selinux/checkreqprot
And, each of the policy capabilities files under
=> /sys/fs/selinux/policy_capabilities
The data for selinux-policy-hash is the SHA256 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
cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep -m 1 "selinux-policy-hash" | cut -d' ' -f 6
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
security/selinux/Makefile | 2 +
security/selinux/hooks.c | 4 +
security/selinux/include/security.h | 18 ++++
security/selinux/measure.c | 155 ++++++++++++++++++++++++++++
security/selinux/selinuxfs.c | 1 +
security/selinux/ss/services.c | 66 ++++++++++--
6 files changed, 237 insertions(+), 9 deletions(-)
create mode 100644 security/selinux/measure.c
diff --git a/security/selinux/Makefile b/security/selinux/Makefile
index 4d8e0e8adf0b..83d512116341 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) += measure.o
+
ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include
$(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index efa6108b1ce9..cda1d328339f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7259,6 +7259,8 @@ static __init int selinux_init(void)
fs_validate_description("selinux", selinux_fs_parameters);
+ selinux_init_measurement();
+
return 0;
}
@@ -7284,6 +7286,7 @@ DEFINE_LSM(selinux) = {
.enabled = &selinux_enabled_boot,
.blobs = &selinux_blob_sizes,
.init = selinux_init,
+ .security_measure_data = selinux_measure_data,
};
#if defined(CONFIG_NETFILTER)
@@ -7394,6 +7397,7 @@ int selinux_disable(struct selinux_state *state)
}
selinux_mark_disabled(state);
+ selinux_measure_state(state);
pr_info("SELinux: Disabled at runtime.\n");
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index b0e02cfe3ce1..1f41c16a4845 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -222,16 +222,34 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
}
+static inline bool selinux_checkreqprot(const struct selinux_state *state)
+{
+ return READ_ONCE(state->checkreqprot);
+}
+
int security_mls_enabled(struct selinux_state *state);
int security_load_policy(struct selinux_state *state,
void *data, size_t len);
int security_read_policy(struct selinux_state *state,
void **data, size_t *len);
+int security_read_policy_kernel(struct selinux_state *state,
+ void **data, size_t *len);
size_t security_policydb_len(struct selinux_state *state);
int security_policycap_supported(struct selinux_state *state,
unsigned int req_cap);
+#ifdef CONFIG_IMA
+extern void __init selinux_init_measurement(void);
+extern void selinux_measure_data(void);
+extern void selinux_measure_state(struct selinux_state *selinux_state);
+#else
+static inline void __init selinux_init_measurement(void) {}
+static inline void selinux_measure_data(void) {}
+static inline void selinux_measure_state(
+ struct selinux_state *selinux_state) {}
+#endif
+
#define SEL_VEC_MAX 32
struct av_decision {
u32 allowed;
diff --git a/security/selinux/measure.c b/security/selinux/measure.c
new file mode 100644
index 000000000000..659011637ae7
--- /dev/null
+++ b/security/selinux/measure.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Measure SELinux state using IMA subsystem.
+ */
+#include <linux/ima.h>
+#include "security.h"
+
+/* Pre-allocated buffer used for measuring state */
+static char *selinux_state_string;
+static size_t selinux_state_string_len;
+static char *str_format = "%s=%d;";
+static int selinux_state_count;
+
+void __init selinux_init_measurement(void)
+{
+ int i;
+
+ /*
+ * enabled
+ * enforcing
+ * checkreqport
+ * All policy capability flags
+ */
+ selinux_state_count = 3 + __POLICYDB_CAPABILITY_MAX;
+
+ selinux_state_string_len = snprintf(NULL, 0, str_format,
+ "enabled", 0);
+ selinux_state_string_len += snprintf(NULL, 0, str_format,
+ "enforcing", 0);
+ selinux_state_string_len += snprintf(NULL, 0, str_format,
+ "checkreqprot", 0);
+ for (i = 3; i < selinux_state_count; i++) {
+ selinux_state_string_len +=
+ snprintf(NULL, 0, str_format,
+ selinux_policycap_names[i-3], 0);
+ }
+ ++selinux_state_string_len;
+
+ selinux_state_string = kzalloc(selinux_state_string_len, GFP_KERNEL);
+ if (!selinux_state_string) {
+ pr_warn("Failed to alloc memory for SELinux measurement\n");
+ selinux_state_string_len = 0;
+ }
+}
+
+static int selinux_hash_policy(const char *hash_alg_name,
+ void *policy, size_t policy_len,
+ void **policy_hash, int *policy_hash_len)
+{
+ struct crypto_shash *tfm;
+ struct shash_desc *desc = NULL;
+ void *digest = NULL;
+ int desc_size;
+ int digest_size;
+ int ret = 0;
+
+ tfm = crypto_alloc_shash(hash_alg_name, 0, 0);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+ digest_size = crypto_shash_digestsize(tfm);
+
+ digest = kmalloc(digest_size, GFP_KERNEL);
+ if (!digest) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ desc = kzalloc(desc_size, GFP_KERNEL);
+ if (!desc) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ desc->tfm = tfm;
+
+ ret = crypto_shash_digest(desc, policy, policy_len, digest);
+ if (ret < 0)
+ goto error;
+
+ *policy_hash_len = digest_size;
+ *policy_hash = digest;
+ digest = NULL;
+
+error:
+ kfree(desc);
+ kfree(digest);
+
+ crypto_free_shash(tfm);
+
+ return ret;
+}
+
+void selinux_measure_state(struct selinux_state *selinux_state)
+{
+ void *policy = NULL;
+ void *policy_hash = NULL;
+ size_t curr, buflen;
+ int i, policy_hash_len, rc = 0;
+
+ if (!selinux_initialized(selinux_state)) {
+ pr_warn("%s: SELinux not yet initialized.\n", __func__);
+ return;
+ }
+
+ if (!selinux_state_string) {
+ pr_warn("%s: Buffer for state not allocated.\n", __func__);
+ return;
+ }
+
+ curr = snprintf(selinux_state_string, selinux_state_string_len,
+ str_format, "enabled",
+ !selinux_disabled(selinux_state));
+ curr += snprintf((selinux_state_string + curr),
+ (selinux_state_string_len - curr),
+ str_format, "enforcing",
+ enforcing_enabled(selinux_state));
+ curr += snprintf((selinux_state_string + curr),
+ (selinux_state_string_len - curr),
+ str_format, "checkreqprot",
+ selinux_checkreqprot(selinux_state));
+
+ for (i = 3; i < selinux_state_count; i++) {
+ curr += snprintf((selinux_state_string + curr),
+ (selinux_state_string_len - curr),
+ str_format,
+ selinux_policycap_names[i - 3],
+ selinux_state->policycap[i - 3]);
+ }
+
+ if (curr >= 0 && curr < selinux_state_string_len)
+ ima_lsm_state("selinux-state", selinux_state_string, curr);
+ else {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ rc = security_read_policy_kernel(selinux_state, &policy, &buflen);
+ if (!rc)
+ rc = selinux_hash_policy("sha256", policy, buflen,
+ &policy_hash, &policy_hash_len);
+ if (!rc)
+ ima_lsm_state("selinux-policy-hash", policy_hash,
+ policy_hash_len);
+
+out:
+ vfree(policy);
+ kfree(policy_hash);
+}
+
+void selinux_measure_data(void)
+{
+ selinux_measure_state(&selinux_state);
+}
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 4781314c2510..b1f70739d709 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -173,6 +173,7 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
from_kuid(&init_user_ns, audit_get_loginuid(current)),
audit_get_sessionid(current));
enforcing_set(state, new_value);
+ selinux_measure_state(&selinux_state);
if (new_value)
avc_ss_reset(state->avc, 0);
selnl_notify_setenforce(new_value);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index ef0afd878bfc..79a6b462f1fe 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -3720,14 +3720,22 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
}
#endif /* CONFIG_NETLABEL */
+static int security_read_policy_len(struct selinux_state *state, size_t *len)
+{
+ if (!selinux_initialized(state))
+ return -EINVAL;
+
+ *len = security_policydb_len(state);
+ return 0;
+}
+
/**
* security_read_policy - read the policy.
* @data: binary policy data
* @len: length of data in bytes
- *
*/
-int security_read_policy(struct selinux_state *state,
- void **data, size_t *len)
+int security_read_selinux_policy(struct selinux_state *state,
+ void **data, size_t *len)
{
struct policydb *policydb = &state->ss->policydb;
int rc;
@@ -3736,12 +3744,6 @@ int security_read_policy(struct selinux_state *state,
if (!selinux_initialized(state))
return -EINVAL;
- *len = security_policydb_len(state);
-
- *data = vmalloc_user(*len);
- if (!*data)
- return -ENOMEM;
-
fp.data = *data;
fp.len = *len;
@@ -3754,5 +3756,51 @@ int security_read_policy(struct selinux_state *state,
*len = (unsigned long)fp.data - (unsigned long)*data;
return 0;
+}
+
+/**
+ * security_read_policy - read the policy.
+ * @data: binary policy data
+ * @len: length of data in bytes
+ *
+ */
+int security_read_policy(struct selinux_state *state,
+ void **data, size_t *len)
+{
+ int rc;
+
+ rc = security_read_policy_len(state, len);
+ if (rc)
+ return rc;
+
+ *data = vmalloc_user(*len);
+ if (!*data)
+ return -ENOMEM;
+
+ return security_read_selinux_policy(state, data, len);
+}
+
+/**
+ * security_read_policy_kernel - read the policy.
+ * @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
+ */
+int security_read_policy_kernel(struct selinux_state *state,
+ void **data, size_t *len)
+{
+ int rc;
+
+ rc = security_read_policy_len(state, len);
+ if (rc)
+ return rc;
+
+ *data = vmalloc(*len);
+ if (!*data)
+ return -ENOMEM;
+ return security_read_selinux_policy(state, data, len);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-17 22:28 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Lakshmi Ramasubramanian
@ 2020-07-18 3:14 ` kernel test robot
2020-07-20 2:04 ` Lakshmi Ramasubramanian
2020-07-18 3:38 ` kernel test robot
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: kernel test robot @ 2020-07-18 3:14 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, zohar, stephen.smalley.work, casey
Cc: kbuild-all, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3792 bytes --]
Hi Lakshmi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on integrity/next-integrity]
[cannot apply to pcmoore-selinux/next security/next-testing linus/master v5.8-rc5 next-20200717]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lakshmi-Ramasubramanian/LSM-Measure-security-module-state/20200718-063111
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
config: parisc-allyesconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
security/selinux/measure.c: In function 'selinux_measure_state':
security/selinux/measure.c:132:11: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
132 | if (curr >= 0 && curr < selinux_state_string_len)
| ^~
>> security/selinux/measure.c:148:2: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
148 | vfree(policy);
| ^~~~~
| kvfree
cc1: some warnings being treated as errors
vim +148 security/selinux/measure.c
94
95 void selinux_measure_state(struct selinux_state *selinux_state)
96 {
97 void *policy = NULL;
98 void *policy_hash = NULL;
99 size_t curr, buflen;
100 int i, policy_hash_len, rc = 0;
101
102 if (!selinux_initialized(selinux_state)) {
103 pr_warn("%s: SELinux not yet initialized.\n", __func__);
104 return;
105 }
106
107 if (!selinux_state_string) {
108 pr_warn("%s: Buffer for state not allocated.\n", __func__);
109 return;
110 }
111
112 curr = snprintf(selinux_state_string, selinux_state_string_len,
113 str_format, "enabled",
114 !selinux_disabled(selinux_state));
115 curr += snprintf((selinux_state_string + curr),
116 (selinux_state_string_len - curr),
117 str_format, "enforcing",
118 enforcing_enabled(selinux_state));
119 curr += snprintf((selinux_state_string + curr),
120 (selinux_state_string_len - curr),
121 str_format, "checkreqprot",
122 selinux_checkreqprot(selinux_state));
123
124 for (i = 3; i < selinux_state_count; i++) {
125 curr += snprintf((selinux_state_string + curr),
126 (selinux_state_string_len - curr),
127 str_format,
128 selinux_policycap_names[i - 3],
129 selinux_state->policycap[i - 3]);
130 }
131
> 132 if (curr >= 0 && curr < selinux_state_string_len)
133 ima_lsm_state("selinux-state", selinux_state_string, curr);
134 else {
135 rc = -EINVAL;
136 goto out;
137 }
138
139 rc = security_read_policy_kernel(selinux_state, &policy, &buflen);
140 if (!rc)
141 rc = selinux_hash_policy("sha256", policy, buflen,
142 &policy_hash, &policy_hash_len);
143 if (!rc)
144 ima_lsm_state("selinux-policy-hash", policy_hash,
145 policy_hash_len);
146
147 out:
> 148 vfree(policy);
149 kfree(policy_hash);
150 }
151
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65334 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-18 3:14 ` kernel test robot
@ 2020-07-20 2:04 ` Lakshmi Ramasubramanian
0 siblings, 0 replies; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-20 2:04 UTC (permalink / raw)
To: kernel test robot, zohar, stephen.smalley.work, casey
Cc: kbuild-all, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
On 7/17/20 8:14 PM, kernel test robot wrote:
> Hi Lakshmi,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on integrity/next-integrity]
> [cannot apply to pcmoore-selinux/next security/next-testing linus/master v5.8-rc5 next-20200717]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
Thank you for catching this.
I did not see these failures with the compiler and make options I'd used.
Am able to reproduce the errors with the instructions you'd provided.
Will post the updated patches shortly.
thanks,
-lakshmi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-17 22:28 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Lakshmi Ramasubramanian
2020-07-18 3:14 ` kernel test robot
@ 2020-07-18 3:38 ` kernel test robot
2020-07-18 15:31 ` kernel test robot
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2020-07-18 3:38 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, zohar, stephen.smalley.work, casey
Cc: kbuild-all, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4885 bytes --]
Hi Lakshmi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on integrity/next-integrity]
[cannot apply to pcmoore-selinux/next security/next-testing linus/master v5.8-rc5 next-20200717]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lakshmi-Ramasubramanian/LSM-Measure-security-module-state/20200718-063111
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
security/selinux/measure.c: In function 'selinux_hash_policy':
>> security/selinux/measure.c:57:8: error: implicit declaration of function 'crypto_alloc_shash' [-Werror=implicit-function-declaration]
57 | tfm = crypto_alloc_shash(hash_alg_name, 0, 0);
| ^~~~~~~~~~~~~~~~~~
>> security/selinux/measure.c:57:6: warning: assignment to 'struct crypto_shash *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
57 | tfm = crypto_alloc_shash(hash_alg_name, 0, 0);
| ^
>> security/selinux/measure.c:61:14: error: implicit declaration of function 'crypto_shash_descsize' [-Werror=implicit-function-declaration]
61 | desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
| ^~~~~~~~~~~~~~~~~~~~~
>> security/selinux/measure.c:61:50: error: dereferencing pointer to incomplete type 'struct shash_desc'
61 | desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
| ^~~~~
>> security/selinux/measure.c:62:16: error: implicit declaration of function 'crypto_shash_digestsize' [-Werror=implicit-function-declaration]
62 | digest_size = crypto_shash_digestsize(tfm);
| ^~~~~~~~~~~~~~~~~~~~~~~
>> security/selinux/measure.c:78:8: error: implicit declaration of function 'crypto_shash_digest' [-Werror=implicit-function-declaration]
78 | ret = crypto_shash_digest(desc, policy, policy_len, digest);
| ^~~~~~~~~~~~~~~~~~~
>> security/selinux/measure.c:90:2: error: implicit declaration of function 'crypto_free_shash' [-Werror=implicit-function-declaration]
90 | crypto_free_shash(tfm);
| ^~~~~~~~~~~~~~~~~
security/selinux/measure.c: In function 'selinux_measure_state':
security/selinux/measure.c:132:11: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
132 | if (curr >= 0 && curr < selinux_state_string_len)
| ^~
security/selinux/measure.c:148:2: error: implicit declaration of function 'vfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
148 | vfree(policy);
| ^~~~~
| kvfree
cc1: some warnings being treated as errors
vim +/crypto_alloc_shash +57 security/selinux/measure.c
45
46 static int selinux_hash_policy(const char *hash_alg_name,
47 void *policy, size_t policy_len,
48 void **policy_hash, int *policy_hash_len)
49 {
50 struct crypto_shash *tfm;
51 struct shash_desc *desc = NULL;
52 void *digest = NULL;
53 int desc_size;
54 int digest_size;
55 int ret = 0;
56
> 57 tfm = crypto_alloc_shash(hash_alg_name, 0, 0);
58 if (IS_ERR(tfm))
59 return PTR_ERR(tfm);
60
> 61 desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> 62 digest_size = crypto_shash_digestsize(tfm);
63
64 digest = kmalloc(digest_size, GFP_KERNEL);
65 if (!digest) {
66 ret = -ENOMEM;
67 goto error;
68 }
69
70 desc = kzalloc(desc_size, GFP_KERNEL);
71 if (!desc) {
72 ret = -ENOMEM;
73 goto error;
74 }
75
76 desc->tfm = tfm;
77
> 78 ret = crypto_shash_digest(desc, policy, policy_len, digest);
79 if (ret < 0)
80 goto error;
81
82 *policy_hash_len = digest_size;
83 *policy_hash = digest;
84 digest = NULL;
85
86 error:
87 kfree(desc);
88 kfree(digest);
89
> 90 crypto_free_shash(tfm);
91
92 return ret;
93 }
94
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66232 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-17 22:28 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Lakshmi Ramasubramanian
2020-07-18 3:14 ` kernel test robot
2020-07-18 3:38 ` kernel test robot
@ 2020-07-18 15:31 ` kernel test robot
2020-07-18 15:31 ` [RFC PATCH] LSM: security_read_selinux_policy() can be static kernel test robot
2020-07-20 14:31 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Stephen Smalley
4 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2020-07-18 15:31 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, zohar, stephen.smalley.work, casey
Cc: kbuild-all, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]
Hi Lakshmi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on integrity/next-integrity]
[cannot apply to pcmoore-selinux/next security/next-testing linus/master v5.8-rc5 next-20200717]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lakshmi-Ramasubramanian/LSM-Measure-security-module-state/20200718-063111
base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
config: x86_64-randconfig-s022-20200717 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-49-g707c5017-dirty
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> security/selinux/ss/services.c:3737:5: sparse: sparse: symbol 'security_read_selinux_policy' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36178 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC PATCH] LSM: security_read_selinux_policy() can be static
2020-07-17 22:28 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Lakshmi Ramasubramanian
` (2 preceding siblings ...)
2020-07-18 15:31 ` kernel test robot
@ 2020-07-18 15:31 ` kernel test robot
2020-07-20 14:31 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Stephen Smalley
4 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2020-07-18 15:31 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, zohar, stephen.smalley.work, casey
Cc: kbuild-all, jmorris, linux-integrity, selinux,
linux-security-module, linux-kernel
Signed-off-by: kernel test robot <lkp@intel.com>
---
services.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 79a6b462f1fe9..4374c75f91a21 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -3734,8 +3734,8 @@ static int security_read_policy_len(struct selinux_state *state, size_t *len)
* @data: binary policy data
* @len: length of data in bytes
*/
-int security_read_selinux_policy(struct selinux_state *state,
- void **data, size_t *len)
+static int security_read_selinux_policy(struct selinux_state *state,
+ void **data, size_t *len)
{
struct policydb *policydb = &state->ss->policydb;
int rc;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-17 22:28 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Lakshmi Ramasubramanian
` (3 preceding siblings ...)
2020-07-18 15:31 ` [RFC PATCH] LSM: security_read_selinux_policy() can be static kernel test robot
@ 2020-07-20 14:31 ` Stephen Smalley
2020-07-20 15:17 ` Lakshmi Ramasubramanian
4 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2020-07-20 14:31 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On Fri, Jul 17, 2020 at 6:28 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> SELinux configuration and policy are some of the critical data for this
> security module that needs to be measured. To enable this measurement
> SELinux needs to implement the interface function,
> security_measure_data(), that the LSM can call.
>
> Define the security_measure_data() function in SELinux to measure SELinux
> configuration and policy. Call this function to measure SELinux data
> when there is a change in the security module's state.
>
> Sample measurement of SELinux state and hash of the policy:
>
> 10 e32e...5ac3 ima-buf sha256:86e8...4594 selinux-state 656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b
> 10 f4a7...9408 ima-buf sha256:4941...68fc selinux-policy-hash 8d1d...1834
>
> To verify the measurement check the following:
>
> Execute the following command to extract the measured data
> from the IMA log for SELinux configuration (selinux-state).
>
> cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep -m 1 "selinux-state" | cut -d' ' -f 6 | xxd -r -p
>
> The output should be the list of key-value pairs. For example,
> enabled=1;enforcing=0;checkreqprot=1;network_peer_controls=1;open_perms=1;extended_socket_class=1;always_check_network=0;cgroup_seclabel=1;nnp_nosuid_transition=1;genfs_seclabel_symlinks=0;
>
> To verify the measured data with the current SELinux state:
>
> => enabled should be set to 1 if /sys/fs/selinux folder exists,
> 0 otherwise
>
> For other entries, compare the integer value in the files
> => /sys/fs/selinux/enforce
> => /sys/fs/selinux/checkreqprot
> And, each of the policy capabilities files under
> => /sys/fs/selinux/policy_capabilities
>
> The data for selinux-policy-hash is the SHA256 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
>
> cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements | grep -m 1 "selinux-policy-hash" | cut -d' ' -f 6
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> diff --git a/security/selinux/measure.c b/security/selinux/measure.c
> new file mode 100644
> index 000000000000..659011637ae7
> --- /dev/null
> +++ b/security/selinux/measure.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Measure SELinux state using IMA subsystem.
> + */
> +#include <linux/ima.h>
> +#include "security.h"
> +
> +/* Pre-allocated buffer used for measuring state */
> +static char *selinux_state_string;
> +static size_t selinux_state_string_len;
> +static char *str_format = "%s=%d;";
> +static int selinux_state_count;
> +
> +void __init selinux_init_measurement(void)
> +{
> + int i;
> +
> + /*
> + * enabled
> + * enforcing
> + * checkreqport
checkreqprot (spelling)
What about initialized? Or do you consider that to be implicitly
true/1 else we wouldn't be taking a measurement? Only caveat there is
that it provides one more means of disabling measurements (at the same
time as disabling enforcement) by setting it to false/0 via kernel
write flaw.
> + * All policy capability flags
> + */
> + selinux_state_count = 3 + __POLICYDB_CAPABILITY_MAX;
> +
> + selinux_state_string_len = snprintf(NULL, 0, str_format,
> + "enabled", 0);
> + selinux_state_string_len += snprintf(NULL, 0, str_format,
> + "enforcing", 0);
> + selinux_state_string_len += snprintf(NULL, 0, str_format,
> + "checkreqprot", 0);
> + for (i = 3; i < selinux_state_count; i++) {
> + selinux_state_string_len +=
> + snprintf(NULL, 0, str_format,
> + selinux_policycap_names[i-3], 0);
> + }
What's the benefit of this pattern versus just making the loop go from
0 to __POLICYDB_CAPABILITY_MAX and using selinux_policycap_names[i]?
> +void selinux_measure_state(struct selinux_state *selinux_state)
> +{
> + void *policy = NULL;
> + void *policy_hash = NULL;
> + size_t curr, buflen;
> + int i, policy_hash_len, rc = 0;
> +
> + if (!selinux_initialized(selinux_state)) {
> + pr_warn("%s: SELinux not yet initialized.\n", __func__);
> + return;
> + }
We could measure the global state variables before full SELinux
initialization (i.e. policy load).
Only the policy hash depends on having loaded the policy.
> +
> + if (!selinux_state_string) {
> + pr_warn("%s: Buffer for state not allocated.\n", __func__);
> + return;
> + }
> +
> + curr = snprintf(selinux_state_string, selinux_state_string_len,
> + str_format, "enabled",
> + !selinux_disabled(selinux_state));
> + curr += snprintf((selinux_state_string + curr),
> + (selinux_state_string_len - curr),
> + str_format, "enforcing",
> + enforcing_enabled(selinux_state));
> + curr += snprintf((selinux_state_string + curr),
> + (selinux_state_string_len - curr),
> + str_format, "checkreqprot",
> + selinux_checkreqprot(selinux_state));
> +
> + for (i = 3; i < selinux_state_count; i++) {
> + curr += snprintf((selinux_state_string + curr),
> + (selinux_state_string_len - curr),
> + str_format,
> + selinux_policycap_names[i - 3],
> + selinux_state->policycap[i - 3]);
> + }
Same question here as for the previous loop; seems cleaner to go from
0 to __POLICYDB_CAPABILITY_MAX and use [i].
What public git tree / branch would you recommend trying to use your
patches against? Didn't seem to apply to any of the obvious ones.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 14:31 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Stephen Smalley
@ 2020-07-20 15:17 ` Lakshmi Ramasubramanian
2020-07-20 17:06 ` Stephen Smalley
0 siblings, 1 reply; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-20 15:17 UTC (permalink / raw)
To: Stephen Smalley
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On 7/20/20 7:31 AM, Stephen Smalley wrote:
>> +void __init selinux_init_measurement(void)
>> +{
>> + int i;
>> +
>> + /*
>> + * enabled
>> + * enforcing
>> + * checkreqport
>
> checkreqprot (spelling)
:( - will fix that.
>
> What about initialized? Or do you consider that to be implicitly
> true/1 else we wouldn't be taking a measurement? Only caveat there is
> that it provides one more means of disabling measurements (at the same
> time as disabling enforcement) by setting it to false/0 via kernel
> write flaw.
Yes - I was thinking measuring SELinux state would be meaningful only
when initialized is set to true/1.
I can include "initialized" as well in the measurement.
>
>> + * All policy capability flags
>> + */
>> + selinux_state_count = 3 + __POLICYDB_CAPABILITY_MAX;
>> +
>> + selinux_state_string_len = snprintf(NULL, 0, str_format,
>> + "enabled", 0);
>> + selinux_state_string_len += snprintf(NULL, 0, str_format,
>> + "enforcing", 0);
>> + selinux_state_string_len += snprintf(NULL, 0, str_format,
>> + "checkreqprot", 0);
>> + for (i = 3; i < selinux_state_count; i++) {
>> + selinux_state_string_len +=
>> + snprintf(NULL, 0, str_format,
>> + selinux_policycap_names[i-3], 0);
>> + }
>
> What's the benefit of this pattern versus just making the loop go from
> 0 to __POLICYDB_CAPABILITY_MAX and using selinux_policycap_names[i]?
No real benefit - I was just trying to use selinux_state_count.
I'll change the loop to go from 0 to POLICY_CAP_MAX
>
>> +void selinux_measure_state(struct selinux_state *selinux_state)
>> +{
>> + void *policy = NULL;
>> + void *policy_hash = NULL;
>> + size_t curr, buflen;
>> + int i, policy_hash_len, rc = 0;
>> +
>> + if (!selinux_initialized(selinux_state)) {
>> + pr_warn("%s: SELinux not yet initialized.\n", __func__);
>> + return;
>> + }
>
> We could measure the global state variables before full SELinux
> initialization (i.e. policy load).
> Only the policy hash depends on having loaded the policy.
Thanks for the information. I'll measure the state variables always and
measure policy only if "initialized" is true/1.
>
>> +
>> + if (!selinux_state_string) {
>> + pr_warn("%s: Buffer for state not allocated.\n", __func__);
>> + return;
>> + }
>> +
>> + curr = snprintf(selinux_state_string, selinux_state_string_len,
>> + str_format, "enabled",
>> + !selinux_disabled(selinux_state));
>> + curr += snprintf((selinux_state_string + curr),
>> + (selinux_state_string_len - curr),
>> + str_format, "enforcing",
>> + enforcing_enabled(selinux_state));
>> + curr += snprintf((selinux_state_string + curr),
>> + (selinux_state_string_len - curr),
>> + str_format, "checkreqprot",
>> + selinux_checkreqprot(selinux_state));
>> +
>> + for (i = 3; i < selinux_state_count; i++) {
>> + curr += snprintf((selinux_state_string + curr),
>> + (selinux_state_string_len - curr),
>> + str_format,
>> + selinux_policycap_names[i - 3],
>> + selinux_state->policycap[i - 3]);
>> + }
>
> Same question here as for the previous loop; seems cleaner to go from
> 0 to __POLICYDB_CAPABILITY_MAX and use [i].
Will change it.
>
> What public git tree / branch would you recommend trying to use your
> patches against? Didn't seem to apply to any of the obvious ones.
>
Please try it on Mimi's next-integrity branch
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-integrity
You can try it on Linus's mainline as well if you apply the following
patch first (have mentioned that in the Cover letter as well)
https://patchwork.kernel.org/patch/11612989/
Thanks for trying out the changes. Please let me know the defects you find.
Just to let you know - I am making the following change (will update in
the next patch):
=> Save the last policy hash and state string in selinux_state struct.
=> Measure policy and hash only if it has changed since the last
measurement.
=> Also, suffix the IMA event name used with time stamp. For example,
10 e32e...5ac3 ima-buf sha256:86e8...4594
selinux-state-1595257807:874963248
656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b
10 f4a7...9408 ima-buf sha256:4941...68fc
selinux-policy-hash-1595257807:874963248
8d1d...1834
The above will ensure the following sequence will be measured:
#1 State A - Measured
#2 Change from State A to State B - Measured
#3 Change from State B back to State A - Since the measured data is
same as in #1, the change will be measured only if the event name is
different between #1 and #3
thanks,
-lakshmi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 15:17 ` Lakshmi Ramasubramanian
@ 2020-07-20 17:06 ` Stephen Smalley
2020-07-20 17:26 ` Mimi Zohar
2020-07-20 17:34 ` Lakshmi Ramasubramanian
0 siblings, 2 replies; 21+ messages in thread
From: Stephen Smalley @ 2020-07-20 17:06 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On Mon, Jul 20, 2020 at 11:17 AM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
> Thanks for trying out the changes. Please let me know the defects you find.
>
> Just to let you know - I am making the following change (will update in
> the next patch):
>
> => Save the last policy hash and state string in selinux_state struct.
> => Measure policy and hash only if it has changed since the last
> measurement.
> => Also, suffix the IMA event name used with time stamp. For example,
>
> 10 e32e...5ac3 ima-buf sha256:86e8...4594
> selinux-state-1595257807:874963248
> 656e61626c65643d313b656e666f7263696e673d303b636865636b72657170726f743d313b6e6574706565723d313b6f70656e7065726d3d313b657874736f636b636c6173733d313b616c776179736e6574776f726b3d303b6367726f75707365636c6162656c3d313b6e6e706e6f737569647472616e736974696f6e3d313b67656e66737365636c6162656c73796d6c696e6b3d303b
>
> 10 f4a7...9408 ima-buf sha256:4941...68fc
> selinux-policy-hash-1595257807:874963248
> 8d1d...1834
>
> The above will ensure the following sequence will be measured:
> #1 State A - Measured
> #2 Change from State A to State B - Measured
> #3 Change from State B back to State A - Since the measured data is
> same as in #1, the change will be measured only if the event name is
> different between #1 and #3
Perhaps the timestamp / sequence number should be part of the hashed
data instead of the event name?
I can see the appraiser wanting to know two things:
1) The current state of the system (e.g. is it enforcing, is the
currently loaded policy the expected one?).
2) Has the system ever been in an unexpected state (e.g. was it
temporarily switched to permissive or had an unexpected policy
loaded?)
I applied the patch series on top of the next-integrity branch, added
measure func=LSM_STATE to ima-policy, and booted that kernel. I get
the following entries in ascii_runtime_measurements, but seemingly
missing the final field:
10 8a09c48af4f8a817f59b495bd82971e096e2e367 ima-ng
sha256:21c3d7b09b62b4d0b3ed15ba990f816b94808f90b76787bfae755c4b3a44cd24
selinux-state
10 e610908931d70990a2855ddb33c16af2d82ce56a ima-ng
sha256:c8898652afd5527ef4eaf8d85f5fee1d91fcccee34bc97f6e55b96746bedb318
selinux-policy-hash
Thus, I cannot verify. What am I missing?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 17:06 ` Stephen Smalley
@ 2020-07-20 17:26 ` Mimi Zohar
2020-07-20 17:34 ` Lakshmi Ramasubramanian
1 sibling, 0 replies; 21+ messages in thread
From: Mimi Zohar @ 2020-07-20 17:26 UTC (permalink / raw)
To: Stephen Smalley, Lakshmi Ramasubramanian, Tyler Hicks,
Prakhar Srivastava
Cc: Casey Schaufler, James Morris, linux-integrity, SElinux list,
LSM List, linux-kernel
On Mon, 2020-07-20 at 13:06 -0400, Stephen Smalley wrote:
>
>
> I applied the patch series on top of the next-integrity branch, added
> measure func=LSM_STATE to ima-policy, and booted that kernel. I get
> the following entries in ascii_runtime_measurements, but seemingly
> missing the final field:
>
> 10 8a09c48af4f8a817f59b495bd82971e096e2e367 ima-ng
> sha256:21c3d7b09b62b4d0b3ed15ba990f816b94808f90b76787bfae755c4b3a44cd24
> selinux-state
> 10 e610908931d70990a2855ddb33c16af2d82ce56a ima-ng
> sha256:c8898652afd5527ef4eaf8d85f5fee1d91fcccee34bc97f6e55b96746bedb318
> selinux-policy-hash
>
> Thus, I cannot verify. What am I missing?
Missing is "template=ima-buf" on the policy rule.
Tyler's patch set just added some support for verifying the policy.
Refer to ima_validate_rule(). There are still some things missing.
For example, nayna noticed that making sure that asymmetric key
support is enabled. Another example is requiring "template=" for any
of the buffer measurements. Template names can be defined
dynamically, so it will need to support either format:
measure func=KEXEC_CMDLINE template=ima-buf
measure func=KEXEC_CMDLINE template=d-ng|n-ng|buf
Mimi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 17:06 ` Stephen Smalley
2020-07-20 17:26 ` Mimi Zohar
@ 2020-07-20 17:34 ` Lakshmi Ramasubramanian
2020-07-20 17:40 ` Stephen Smalley
1 sibling, 1 reply; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-20 17:34 UTC (permalink / raw)
To: Stephen Smalley
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On 7/20/20 10:06 AM, Stephen Smalley wrote:
>> The above will ensure the following sequence will be measured:
>> #1 State A - Measured
>> #2 Change from State A to State B - Measured
>> #3 Change from State B back to State A - Since the measured data is
>> same as in #1, the change will be measured only if the event name is
>> different between #1 and #3
>
> Perhaps the timestamp / sequence number should be part of the hashed
> data instead of the event name?
If the timestamp/seqno is part of the hashed data, on every call to
measure IMA will add a new entry in the IMA log. This would fill up the
IMA log - even when there is no change in the measured data.
To avoid that I keep the last measurement in SELinux and measure only
when there is a change with the timestamp in the event name.
> I can see the appraiser wanting to know two things:
> 1) The current state of the system (e.g. is it enforcing, is the
> currently loaded policy the expected one?).
> 2) Has the system ever been in an unexpected state (e.g. was it
> temporarily switched to permissive or had an unexpected policy
> loaded?)
Yes - you are right.
The appraiser will have to look at the entire IMA log (and the
corresponding TPM PCR data) to know the above.
Time t0 => State of the system measured
Time tn => State changed and the new state measured
Time tm => State changed again and the new state measured.
Say, the measurement at "Time tn" was an illegal change, the appraiser
would know.
>
> I applied the patch series on top of the next-integrity branch, added
> measure func=LSM_STATE to ima-policy, and booted that kernel. I get
> the following entries in ascii_runtime_measurements, but seemingly
> missing the final field:
>
> 10 8a09c48af4f8a817f59b495bd82971e096e2e367 ima-ng
> sha256:21c3d7b09b62b4d0b3ed15ba990f816b94808f90b76787bfae755c4b3a44cd24
> selinux-state
> 10 e610908931d70990a2855ddb33c16af2d82ce56a ima-ng
> sha256:c8898652afd5527ef4eaf8d85f5fee1d91fcccee34bc97f6e55b96746bedb318
> selinux-policy-hash
>
> Thus, I cannot verify. What am I missing?
>
Looks like the template used is ima-ng which doesn't include the
measured buffer. Please set template to "ima-buf" in the policy.
For example,
measure func=LSM_STATE template=ima-buf
thanks,
-lakshmi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 17:34 ` Lakshmi Ramasubramanian
@ 2020-07-20 17:40 ` Stephen Smalley
2020-07-20 17:49 ` Stephen Smalley
0 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2020-07-20 17:40 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On Mon, Jul 20, 2020 at 1:34 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 7/20/20 10:06 AM, Stephen Smalley wrote:
>
> >> The above will ensure the following sequence will be measured:
> >> #1 State A - Measured
> >> #2 Change from State A to State B - Measured
> >> #3 Change from State B back to State A - Since the measured data is
> >> same as in #1, the change will be measured only if the event name is
> >> different between #1 and #3
> >
> > Perhaps the timestamp / sequence number should be part of the hashed
> > data instead of the event name?
>
> If the timestamp/seqno is part of the hashed data, on every call to
> measure IMA will add a new entry in the IMA log. This would fill up the
> IMA log - even when there is no change in the measured data.
>
> To avoid that I keep the last measurement in SELinux and measure only
> when there is a change with the timestamp in the event name.
>
> > I can see the appraiser wanting to know two things:
> > 1) The current state of the system (e.g. is it enforcing, is the
> > currently loaded policy the expected one?).
> > 2) Has the system ever been in an unexpected state (e.g. was it
> > temporarily switched to permissive or had an unexpected policy
> > loaded?)
>
> Yes - you are right.
> The appraiser will have to look at the entire IMA log (and the
> corresponding TPM PCR data) to know the above.
>
> Time t0 => State of the system measured
> Time tn => State changed and the new state measured
> Time tm => State changed again and the new state measured.
>
> Say, the measurement at "Time tn" was an illegal change, the appraiser
> would know.
>
> >
> > I applied the patch series on top of the next-integrity branch, added
> > measure func=LSM_STATE to ima-policy, and booted that kernel. I get
> > the following entries in ascii_runtime_measurements, but seemingly
> > missing the final field:
> >
> > 10 8a09c48af4f8a817f59b495bd82971e096e2e367 ima-ng
> > sha256:21c3d7b09b62b4d0b3ed15ba990f816b94808f90b76787bfae755c4b3a44cd24
> > selinux-state
> > 10 e610908931d70990a2855ddb33c16af2d82ce56a ima-ng
> > sha256:c8898652afd5527ef4eaf8d85f5fee1d91fcccee34bc97f6e55b96746bedb318
> > selinux-policy-hash
> >
> > Thus, I cannot verify. What am I missing?
> >
>
> Looks like the template used is ima-ng which doesn't include the
> measured buffer. Please set template to "ima-buf" in the policy.
>
> For example,
> measure func=LSM_STATE template=ima-buf
It seems like one shouldn't need to manually specify it if it is the
only template that yields a useful result for the LSM_STATE function?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 17:40 ` Stephen Smalley
@ 2020-07-20 17:49 ` Stephen Smalley
2020-07-20 18:27 ` Lakshmi Ramasubramanian
0 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2020-07-20 17:49 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On Mon, Jul 20, 2020 at 1:40 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Jul 20, 2020 at 1:34 PM Lakshmi Ramasubramanian
> <nramas@linux.microsoft.com> wrote:
> >
> > On 7/20/20 10:06 AM, Stephen Smalley wrote:
> >
> > >> The above will ensure the following sequence will be measured:
> > >> #1 State A - Measured
> > >> #2 Change from State A to State B - Measured
> > >> #3 Change from State B back to State A - Since the measured data is
> > >> same as in #1, the change will be measured only if the event name is
> > >> different between #1 and #3
> > >
> > > Perhaps the timestamp / sequence number should be part of the hashed
> > > data instead of the event name?
> >
> > If the timestamp/seqno is part of the hashed data, on every call to
> > measure IMA will add a new entry in the IMA log. This would fill up the
> > IMA log - even when there is no change in the measured data.
> >
> > To avoid that I keep the last measurement in SELinux and measure only
> > when there is a change with the timestamp in the event name.
> >
> > > I can see the appraiser wanting to know two things:
> > > 1) The current state of the system (e.g. is it enforcing, is the
> > > currently loaded policy the expected one?).
> > > 2) Has the system ever been in an unexpected state (e.g. was it
> > > temporarily switched to permissive or had an unexpected policy
> > > loaded?)
> >
> > Yes - you are right.
> > The appraiser will have to look at the entire IMA log (and the
> > corresponding TPM PCR data) to know the above.
> >
> > Time t0 => State of the system measured
> > Time tn => State changed and the new state measured
> > Time tm => State changed again and the new state measured.
> >
> > Say, the measurement at "Time tn" was an illegal change, the appraiser
> > would know.
> >
> > >
> > > I applied the patch series on top of the next-integrity branch, added
> > > measure func=LSM_STATE to ima-policy, and booted that kernel. I get
> > > the following entries in ascii_runtime_measurements, but seemingly
> > > missing the final field:
> > >
> > > 10 8a09c48af4f8a817f59b495bd82971e096e2e367 ima-ng
> > > sha256:21c3d7b09b62b4d0b3ed15ba990f816b94808f90b76787bfae755c4b3a44cd24
> > > selinux-state
> > > 10 e610908931d70990a2855ddb33c16af2d82ce56a ima-ng
> > > sha256:c8898652afd5527ef4eaf8d85f5fee1d91fcccee34bc97f6e55b96746bedb318
> > > selinux-policy-hash
> > >
> > > Thus, I cannot verify. What am I missing?
> > >
> >
> > Looks like the template used is ima-ng which doesn't include the
> > measured buffer. Please set template to "ima-buf" in the policy.
> >
> > For example,
> > measure func=LSM_STATE template=ima-buf
>
> It seems like one shouldn't need to manually specify it if it is the
> only template that yields a useful result for the LSM_STATE function?
Actually, if we used ima-ng template for selinux-policy-hash, then
instead of needing to hash the policy
first and passing the hash to IMA, we could just pass the policy as
the buffer and IMA would take care of the hashing, right?
And we only need to use ima-buf for the selinux-state if we want the
measurement list to include the string value that
was hashed; if we just want to compare against a known-good, it would
suffice to use ima-ng for it as well, right?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 17:49 ` Stephen Smalley
@ 2020-07-20 18:27 ` Lakshmi Ramasubramanian
2020-07-20 18:44 ` Stephen Smalley
0 siblings, 1 reply; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-20 18:27 UTC (permalink / raw)
To: Stephen Smalley
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On 7/20/20 10:49 AM, Stephen Smalley wrote:
>>>
>>> Looks like the template used is ima-ng which doesn't include the
>>> measured buffer. Please set template to "ima-buf" in the policy.
>>>
>>> For example,
>>> measure func=LSM_STATE template=ima-buf
>>
>> It seems like one shouldn't need to manually specify it if it is the
>> only template that yields a useful result for the LSM_STATE function?
>
> Actually, if we used ima-ng template for selinux-policy-hash, then
> instead of needing to hash the policy
> first and passing the hash to IMA, we could just pass the policy as
> the buffer and IMA would take care of the hashing, right?
That is correct.
The IMA hook I've added to measure LSM structures is a generic one that
can be used by any security module (SM). I feel it would be better to
not have policy or state or any such SM specific logic in IMA, but leave
that to the individual SM to handle.
What do you think?
> And we only need to use ima-buf for the selinux-state if we want the
> measurement list to include the string value that
> was hashed; if we just want to compare against a known-good, it would
> suffice to use ima-ng for it as well, right?
>
-lakshmi
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 18:27 ` Lakshmi Ramasubramanian
@ 2020-07-20 18:44 ` Stephen Smalley
2020-07-20 18:59 ` Lakshmi Ramasubramanian
0 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2020-07-20 18:44 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On Mon, Jul 20, 2020 at 2:27 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 7/20/20 10:49 AM, Stephen Smalley wrote:
>
> >>>
> >>> Looks like the template used is ima-ng which doesn't include the
> >>> measured buffer. Please set template to "ima-buf" in the policy.
> >>>
> >>> For example,
> >>> measure func=LSM_STATE template=ima-buf
> >>
> >> It seems like one shouldn't need to manually specify it if it is the
> >> only template that yields a useful result for the LSM_STATE function?
> >
> > Actually, if we used ima-ng template for selinux-policy-hash, then
> > instead of needing to hash the policy
> > first and passing the hash to IMA, we could just pass the policy as
> > the buffer and IMA would take care of the hashing, right?
>
> That is correct.
>
> The IMA hook I've added to measure LSM structures is a generic one that
> can be used by any security module (SM). I feel it would be better to
> not have policy or state or any such SM specific logic in IMA, but leave
> that to the individual SM to handle.
>
> What do you think?
It is correct to remain security module agnostic. However, I think
you can remain LSM-neutral while still avoiding the double hashing of
the policy here. Can't you just pass in the policy itself as the
buffer and let IMA hash it? Then you can let the policy author decide
on the template to be used (ima-buf versus ima-ng). If you want to
support the use of different templates for different "kinds" of LSM
state (e.g. state versus policy) you could either provide two funcs
(LSM_STATE, LSM_POLICY) or otherwise support selection based on some
other attribute.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/5] LSM: Define SELinux function to measure security state
2020-07-20 18:44 ` Stephen Smalley
@ 2020-07-20 18:59 ` Lakshmi Ramasubramanian
0 siblings, 0 replies; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-20 18:59 UTC (permalink / raw)
To: Stephen Smalley
Cc: Mimi Zohar, Casey Schaufler, James Morris, linux-integrity,
SElinux list, LSM List, linux-kernel
On 7/20/20 11:44 AM, Stephen Smalley wrote:
>>>
>>> Actually, if we used ima-ng template for selinux-policy-hash, then
>>> instead of needing to hash the policy
>>> first and passing the hash to IMA, we could just pass the policy as
>>> the buffer and IMA would take care of the hashing, right?
>>
>> That is correct.
>>
>> The IMA hook I've added to measure LSM structures is a generic one that
>> can be used by any security module (SM). I feel it would be better to
>> not have policy or state or any such SM specific logic in IMA, but leave
>> that to the individual SM to handle.
>>
>> What do you think?
>
> It is correct to remain security module agnostic. However, I think
> you can remain LSM-neutral while still avoiding the double hashing of
> the policy here. Can't you just pass in the policy itself as the
> buffer and let IMA hash it?
Yes - that is an option. If I do that then, as you have stated below,
we'll need to two funcs -
one that will only add the hash but not the entire data payload in the
IMA log (i.e., "ima-ng")
and, the other that handles hashing and including date payload (i.e.,
"ima-buf").
Then you can let the policy author decide
> on the template to be used (ima-buf versus ima-ng). If you want to
> support the use of different templates for different "kinds" of LSM
> state (e.g. state versus policy) you could either provide two funcs
> (LSM_STATE, LSM_POLICY) or otherwise support selection based on some
> other attribute.
>
I can do the above.
-lakshmi
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 5/5] LSM: Define workqueue for measuring security module state
2020-07-17 22:28 [PATCH v3 0/5] LSM: Measure security module state Lakshmi Ramasubramanian
` (3 preceding siblings ...)
2020-07-17 22:28 ` [PATCH v3 4/5] LSM: Define SELinux function to measure security state Lakshmi Ramasubramanian
@ 2020-07-17 22:28 ` Lakshmi Ramasubramanian
4 siblings, 0 replies; 21+ messages in thread
From: Lakshmi Ramasubramanian @ 2020-07-17 22:28 UTC (permalink / raw)
To: zohar, stephen.smalley.work, casey
Cc: jmorris, linux-integrity, selinux, linux-security-module, linux-kernel
Data structures critical to the functioning of a security module could
be tampered with by malware or changed inadvertently at runtime
thereby disabling or reducing the security guarantees provided by
the security module. Such critical data need to be periodically checked
and measured, if there is any change. This would enable an attestation
service, for instance, to verify that the security modules are operating
with the configuration and policy setup by the system administrator.
Define a workqueue in the LSM and invoke the security modules in
the workqueue handler to check their data and measure.
Note that the data given by the security module would be measured by
the IMA subsystem only if it has changed since the last time it was
measured.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
security/security.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/security/security.c b/security/security.c
index 88ce1b780ffd..6e746a024ed5 100644
--- a/security/security.c
+++ b/security/security.c
@@ -89,6 +89,11 @@ static __initdata struct lsm_info *exclusive;
static struct lsm_info *security_state_lsms;
static int security_state_lsms_count;
+static long security_state_timeout = 300000; /* 5 Minutes */
+static void security_state_handler(struct work_struct *work);
+static DECLARE_DELAYED_WORK(security_state_delayed_work,
+ security_state_handler);
+
static __initdata bool debug;
#define init_debug(...) \
do { \
@@ -277,6 +282,26 @@ static void __init initialize_security_state_lsms(void)
security_state_lsms_count = count;
}
+static void initialize_security_state_monitor(void)
+{
+ if (security_state_lsms_count == 0)
+ return;
+
+ schedule_delayed_work(&security_state_delayed_work,
+ msecs_to_jiffies(security_state_timeout));
+}
+
+static void security_state_handler(struct work_struct *work)
+{
+ int inx;
+
+ for (inx = 0; inx < security_state_lsms_count; inx++)
+ measure_security_state(&(security_state_lsms[inx]));
+
+ schedule_delayed_work(&security_state_delayed_work,
+ msecs_to_jiffies(security_state_timeout));
+}
+
/* Populate ordered LSMs list from comma-separated LSM name list. */
static void __init ordered_lsm_parse(const char *order, const char *origin)
{
@@ -400,6 +425,7 @@ static void __init ordered_lsm_init(void)
}
initialize_security_state_lsms();
+ initialize_security_state_monitor();
kfree(ordered_lsms);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 21+ messages in thread