2020-07-30 03:48:52

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: [PATCH v5 0/4] LSM: Measure security module data

Critical data structures of security modules are currently not measured.
Therefore an attestation service, for instance, would not be able to
attest whether the security modules are always operating with the policies
and configuration that the system administrator had setup. The policies
and configuration for the security modules could be tampered with by
malware by exploiting kernel vulnerabilities or modified through some
inadvertent actions on the system. Measuring such critical data would
enable an attestation service to better assess the state of the system.

IMA subsystem measures system files, command line arguments passed to
kexec, boot aggregate, keys, etc. It can be used to measure critical
data structures of security modules as well.

This change aims to address measuring critical data structures
of security modules when they are initialized and when they are
updated at runtime.

This series is based on commit 3db0d0c276a7 ("integrity: remove
redundant initialization of variable ret") in next-integrity

Change log:

v5:
=> Append timestamp to "event name" string in the call to
the IMA hooks so that LSM data is always measured by IMA.
=> Removed workqueue patch that was handling periodic checking
of the LSM data. This change will be introduced as a separate
patch set while keeping this patch set focussed on measuring
the LSM data on initialization and on updates at runtime.
=> Handle early boot measurement of LSM data.

v4:
=> Added LSM_POLICY func and IMA hook to measure LSM policy.
=> Pass SELinux policy data, instead of the hash of the policy,
to the IMA hook to measure.
=> Include "initialized" flag in SELinux measurement.
Also, measure SELinux state even when initialization is not yet
completed. But measure SELinux policy only after initialization.

v3:
=> Loop through policy_capabilities to build the state data
to measure instead of hardcoding to current set of
policy capabilities.
=> Added error log messages for failure conditions.

v2:
=> Pass selinux_state struct as parameter to the function
that measures SELinux data.
=> Use strings from selinux_policycap_names array for SELinux
state measurement.
=> Refactored security_read_policy() to alloc kernel or user
virtual memory and then read the SELinux policy.

v1:
=> Per Stephen Smalley's suggestion added selinux_state booleans
and hash of SELinux policy in the measured data for SELinux.
=> Call IMA hook from the security module directly instead of
redirecting through the LSM.

Lakshmi Ramasubramanian (4):
IMA: Add func to measure LSM state and policy
IMA: Define IMA hooks to measure LSM state and policy
LSM: Define SELinux function to measure state and policy
IMA: Handle early boot data measurement

Documentation/ABI/testing/ima_policy | 9 +
include/linux/ima.h | 14 ++
security/integrity/ima/Kconfig | 5 +-
security/integrity/ima/Makefile | 2 +-
security/integrity/ima/ima.h | 45 +++--
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_asymmetric_keys.c | 6 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 64 ++++++-
security/integrity/ima/ima_policy.c | 33 +++-
security/integrity/ima/ima_queue_data.c | 175 +++++++++++++++++++
security/integrity/ima/ima_queue_keys.c | 174 ------------------
security/selinux/Makefile | 2 +
security/selinux/hooks.c | 1 +
security/selinux/include/security.h | 15 ++
security/selinux/measure.c | 150 ++++++++++++++++
security/selinux/selinuxfs.c | 3 +
security/selinux/ss/services.c | 71 +++++++-
18 files changed, 551 insertions(+), 222 deletions(-)
create mode 100644 security/integrity/ima/ima_queue_data.c
delete mode 100644 security/integrity/ima/ima_queue_keys.c
create mode 100644 security/selinux/measure.c

--
2.27.0


2020-07-30 03:49:19

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: [PATCH v5 2/4] IMA: Define IMA hooks to measure LSM state and policy

IMA subsystem needs to define IMA hooks that the security modules can
call to measure state and policy data.

Define two new IMA hooks, namely ima_lsm_state() and ima_lsm_policy(),
that the security modules can call to measure LSM state and LSM policy
respectively. Return the status of the measurement operation from these
two IMA hooks.

Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
---
include/linux/ima.h | 14 +++++++++
security/integrity/ima/ima.h | 6 ++--
security/integrity/ima/ima_main.c | 50 ++++++++++++++++++++++++++-----
3 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index d15100de6cdd..442ca0dce3c8 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,6 +26,10 @@ 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(int kernel_fd, const void *buf, int size);
+extern int ima_measure_lsm_state(const char *lsm_event_name, const void *buf,
+ int size);
+extern int ima_measure_lsm_policy(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 +108,16 @@ static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
}

static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {}
+static inline int ima_measure_lsm_state(const char *lsm_event_name,
+ const void *buf, int size)
+{
+ return -EOPNOTSUPP;
+}
+static inline int ima_measure_lsm_policy(const char *lsm_event_name,
+ const void *buf, int size)
+{
+ return -EOPNOTSUPP;
+}
#endif /* CONFIG_IMA */

#ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 1b5f4b2f17d0..8ed9f5e1dd40 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -267,9 +267,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
struct evm_ima_xattr_data *xattr_value,
int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc);
-void process_buffer_measurement(struct inode *inode, const void *buf, int size,
- const char *eventname, enum ima_hooks func,
- int pcr, const char *keyring);
+int process_buffer_measurement(struct inode *inode, const void *buf, int size,
+ const char *eventname, enum ima_hooks func,
+ int pcr, const char *keyring);
void ima_audit_measurement(struct integrity_iint_cache *iint,
const unsigned char *filename);
int ima_alloc_init_template(struct ima_event_data *event_data,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8a91711ca79b..74d421e40c8f 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -736,9 +736,9 @@ int ima_load_data(enum kernel_load_data_id id)
*
* Based on policy, the buffer is measured into the ima log.
*/
-void process_buffer_measurement(struct inode *inode, const void *buf, int size,
- const char *eventname, enum ima_hooks func,
- int pcr, const char *keyring)
+int process_buffer_measurement(struct inode *inode, const void *buf, int size,
+ const char *eventname, enum ima_hooks func,
+ int pcr, const char *keyring)
{
int ret = 0;
const char *audit_cause = "ENOMEM";
@@ -758,7 +758,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
u32 secid;

if (!ima_policy_flag)
- return;
+ return 0;

/*
* Both LSM hooks and auxilary based buffer measurements are
@@ -772,7 +772,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
action = ima_get_action(inode, current_cred(), secid, 0, func,
&pcr, &template, keyring);
if (!(action & IMA_MEASURE))
- return;
+ return 0;
}

if (!pcr)
@@ -787,7 +787,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
pr_err("template %s init failed, result: %d\n",
(strlen(template->name) ?
template->name : template->fmt), ret);
- return;
+ return ret;
}
}

@@ -819,7 +819,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
func_measure_str(func),
audit_cause, ret, 0, ret);

- return;
+ return ret;
}

/**
@@ -846,6 +846,42 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size)
fdput(f);
}

+/**
+ * ima_measure_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.
+ */
+int ima_measure_lsm_state(const char *lsm_event_name, const void *buf,
+ int size)
+{
+ if (!lsm_event_name || !buf || !size)
+ return -EINVAL;
+
+ return process_buffer_measurement(NULL, buf, size, lsm_event_name,
+ LSM_STATE, 0, NULL);
+}
+
+/**
+ * ima_measure_lsm_policy - measure LSM specific policy
+ * @lsm_event_name: LSM event
+ * @buf: pointer to buffer containing LSM specific policy
+ * @size: Number of bytes in buf
+ *
+ * Buffers can only be measured, not appraised.
+ */
+int ima_measure_lsm_policy(const char *lsm_event_name, const void *buf,
+ int size)
+{
+ if (!lsm_event_name || !buf || !size)
+ return -EINVAL;
+
+ return process_buffer_measurement(NULL, buf, size, lsm_event_name,
+ LSM_POLICY, 0, NULL);
+}
+
static int __init init_ima(void)
{
int error;
--
2.27.0

2020-07-30 15:07:43

by Tyler Hicks

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] IMA: Define IMA hooks to measure LSM state and policy

On 2020-07-29 20:47:22, Lakshmi Ramasubramanian wrote:
> IMA subsystem needs to define IMA hooks that the security modules can
> call to measure state and policy data.
>
> Define two new IMA hooks, namely ima_lsm_state() and ima_lsm_policy(),
> that the security modules can call to measure LSM state and LSM policy
> respectively. Return the status of the measurement operation from these
> two IMA hooks.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>

Reviewed-by: Tyler Hicks <[email protected]>

Tyler

> ---
> include/linux/ima.h | 14 +++++++++
> security/integrity/ima/ima.h | 6 ++--
> security/integrity/ima/ima_main.c | 50 ++++++++++++++++++++++++++-----
> 3 files changed, 60 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index d15100de6cdd..442ca0dce3c8 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -26,6 +26,10 @@ 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(int kernel_fd, const void *buf, int size);
> +extern int ima_measure_lsm_state(const char *lsm_event_name, const void *buf,
> + int size);
> +extern int ima_measure_lsm_policy(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 +108,16 @@ static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> }
>
> static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {}
> +static inline int ima_measure_lsm_state(const char *lsm_event_name,
> + const void *buf, int size)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int ima_measure_lsm_policy(const char *lsm_event_name,
> + const void *buf, int size)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif /* CONFIG_IMA */
>
> #ifndef CONFIG_IMA_KEXEC
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 1b5f4b2f17d0..8ed9f5e1dd40 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -267,9 +267,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
> struct evm_ima_xattr_data *xattr_value,
> int xattr_len, const struct modsig *modsig, int pcr,
> struct ima_template_desc *template_desc);
> -void process_buffer_measurement(struct inode *inode, const void *buf, int size,
> - const char *eventname, enum ima_hooks func,
> - int pcr, const char *keyring);
> +int process_buffer_measurement(struct inode *inode, const void *buf, int size,
> + const char *eventname, enum ima_hooks func,
> + int pcr, const char *keyring);
> void ima_audit_measurement(struct integrity_iint_cache *iint,
> const unsigned char *filename);
> int ima_alloc_init_template(struct ima_event_data *event_data,
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 8a91711ca79b..74d421e40c8f 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -736,9 +736,9 @@ int ima_load_data(enum kernel_load_data_id id)
> *
> * Based on policy, the buffer is measured into the ima log.
> */
> -void process_buffer_measurement(struct inode *inode, const void *buf, int size,
> - const char *eventname, enum ima_hooks func,
> - int pcr, const char *keyring)
> +int process_buffer_measurement(struct inode *inode, const void *buf, int size,
> + const char *eventname, enum ima_hooks func,
> + int pcr, const char *keyring)
> {
> int ret = 0;
> const char *audit_cause = "ENOMEM";
> @@ -758,7 +758,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
> u32 secid;
>
> if (!ima_policy_flag)
> - return;
> + return 0;
>
> /*
> * Both LSM hooks and auxilary based buffer measurements are
> @@ -772,7 +772,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
> action = ima_get_action(inode, current_cred(), secid, 0, func,
> &pcr, &template, keyring);
> if (!(action & IMA_MEASURE))
> - return;
> + return 0;
> }
>
> if (!pcr)
> @@ -787,7 +787,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
> pr_err("template %s init failed, result: %d\n",
> (strlen(template->name) ?
> template->name : template->fmt), ret);
> - return;
> + return ret;
> }
> }
>
> @@ -819,7 +819,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
> func_measure_str(func),
> audit_cause, ret, 0, ret);
>
> - return;
> + return ret;
> }
>
> /**
> @@ -846,6 +846,42 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size)
> fdput(f);
> }
>
> +/**
> + * ima_measure_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.
> + */
> +int ima_measure_lsm_state(const char *lsm_event_name, const void *buf,
> + int size)
> +{
> + if (!lsm_event_name || !buf || !size)
> + return -EINVAL;
> +
> + return process_buffer_measurement(NULL, buf, size, lsm_event_name,
> + LSM_STATE, 0, NULL);
> +}
> +
> +/**
> + * ima_measure_lsm_policy - measure LSM specific policy
> + * @lsm_event_name: LSM event
> + * @buf: pointer to buffer containing LSM specific policy
> + * @size: Number of bytes in buf
> + *
> + * Buffers can only be measured, not appraised.
> + */
> +int ima_measure_lsm_policy(const char *lsm_event_name, const void *buf,
> + int size)
> +{
> + if (!lsm_event_name || !buf || !size)
> + return -EINVAL;
> +
> + return process_buffer_measurement(NULL, buf, size, lsm_event_name,
> + LSM_POLICY, 0, NULL);
> +}
> +
> static int __init init_ima(void)
> {
> int error;
> --
> 2.27.0