| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <492dd4a8-1edd-211f-42d5-59c24731ddbb@linux.microsoft.com>
Date: Thu, 10 Dec 2020 19:26:56 -0800
From: Tushar Sugandhi <tusharsu@...ux.microsoft.com>
To: Tyler Hicks <tyhicks@...ux.microsoft.com>
Cc: zohar@...ux.ibm.com, stephen.smalley.work@...il.com,
casey@...aufler-ca.com, agk@...hat.com, snitzer@...hat.com,
gmazyland@...il.com, paul@...l-moore.com, sashal@...nel.org,
jmorris@...ei.org, nramas@...ux.microsoft.com,
linux-integrity@...r.kernel.org, selinux@...r.kernel.org,
linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org, dm-devel@...hat.com
Subject: Re: [PATCH v7 3/8] IMA: define a hook to measure kernel integrity
critical data
On 2020-12-10 3:02 p.m., Tyler Hicks wrote:
> On 2020-12-09 11:42:07, Tushar Sugandhi wrote:
>> IMA provides capabilities to measure file data, and in-memory buffer
>> data. However, various data structures, policies, and states
>> stored in kernel memory also impact the integrity of the system.
>> Several kernel subsystems contain such integrity critical data. These
>> kernel subsystems help protect the integrity of a device. Currently,
>> IMA does not provide a generic function for kernel subsystems to measure
>> their integrity critical data.
>>
>> Define a new IMA hook - ima_measure_critical_data to measure kernel
>> integrity critical data.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@...ux.microsoft.com>
>> ---
>> Documentation/ABI/testing/ima_policy | 2 +-
>> include/linux/ima.h | 6 +++++
>> security/integrity/ima/ima.h | 1 +
>> security/integrity/ima/ima_api.c | 2 +-
>> security/integrity/ima/ima_main.c | 36 ++++++++++++++++++++++++++++
>> security/integrity/ima/ima_policy.c | 2 ++
>> 6 files changed, 47 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
>> index e35263f97fc1..6ec7daa87cba 100644
>> --- a/Documentation/ABI/testing/ima_policy
>> +++ b/Documentation/ABI/testing/ima_policy
>> @@ -32,7 +32,7 @@ Description:
>> 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] [CRITICAL_DATA]
>> mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
>> [[^]MAY_EXEC]
>> fsmagic:= hex value
>> diff --git a/include/linux/ima.h b/include/linux/ima.h
>> index ac3d82f962f2..675f54db6264 100644
>> --- a/include/linux/ima.h
>> +++ b/include/linux/ima.h
>> @@ -30,6 +30,9 @@ 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 void ima_measure_critical_data(const char *event_name,
>> + const void *buf, int buf_len,
>> + bool measure_buf_hash);
>>
>> #ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
>> extern void ima_appraise_parse_cmdline(void);
>> @@ -122,6 +125,9 @@ 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 void ima_measure_critical_data(const char *event_name,
>> + const void *buf, int buf_len,
>> + bool measure_buf_hash) {}
>> #endif /* CONFIG_IMA */
>>
>> #ifndef CONFIG_IMA_KEXEC
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index fa3044a7539f..7d9deda6a8b3 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -201,6 +201,7 @@ static inline unsigned int ima_hash_key(u8 *digest)
>> hook(POLICY_CHECK, policy) \
>> hook(KEXEC_CMDLINE, kexec_cmdline) \
>> hook(KEY_CHECK, key) \
>> + hook(CRITICAL_DATA, critical_data) \
>> 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 af218babd198..9917e1730cb6 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 | CRITICAL_DATA
>> * mask: contains the permission mask
>> * fsmagic: hex value
>> *
>> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
>> index 03aad13e9e70..ae59f4a4dd70 100644
>> --- a/security/integrity/ima/ima_main.c
>> +++ b/security/integrity/ima/ima_main.c
>> @@ -922,6 +922,42 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size)
>> fdput(f);
>> }
>>
>> +/**
>> + * ima_measure_critical_data - measure kernel integrity critical data
>> + * @event_name: event name to be used for the buffer entry
>> + * @buf: pointer to buffer containing data to measure
>> + * @buf_len: length of buffer(in bytes)
>> + * @measure_buf_hash: measure buffer hash
>> + *
>> + * Measure the kernel subsystem data, critical to the integrity of the kernel,
>> + * into the IMA log and extend the @pcr.
>> + *
>> + * Use @event_name to describe the state/buffer data change.
>> + * Examples of critical data (buf) could be kernel in-memory r/o structures,
> ^
> @buf
>
Will do.
>> + * hash of the memory structures, or data that represents subsystem state
>> + * change.
>> + *
>> + * If @measure_buf_hash is set to true - measure hash of the buffer data,
>> + * else measure the buffer data itself.
>> + * measure_buf_hash can be used to save space, if the data being measured
> ^
> @measure_buf_hash
>
>> + * is too large.
>> + *
>> + * The data (buf) can only be measured, not appraised.
> ^
> @buf
>
Will do.
>> + */
>> +void ima_measure_critical_data(const char *event_name,
>> + const void *buf, int buf_len,
>> + bool measure_buf_hash)
>> +{
>> + if (!event_name || !buf || !buf_len) {
>> + pr_err("Invalid arguments passed to %s().\n", __func__);
>
> This is a problem for the developer making use of the
> ima_measure_critical_data() API and shouldn't be logged, IMO, because a
> user/admin can do nothing about it. I think the error message should be
> dropped.
>
>> + return;
>> + }
>> +
>> + process_buffer_measurement(NULL, buf, buf_len, event_name,
>> + CRITICAL_DATA, 0, NULL,
>> + measure_buf_hash);
>> +}
>> +
>> static int __init init_ima(void)
>> {
>> int error;
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index 25419c7ff50b..2a0c0603626e 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -1251,6 +1251,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>> else if (IS_ENABLED(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) &&
>> strcmp(args[0].from, "KEY_CHECK") == 0)
>> entry->func = KEY_CHECK;
>> + else if (strcmp(args[0].from, "CRITICAL_DATA") == 0)
>> + entry->func = CRITICAL_DATA;
>> else
>> result = -EINVAL;
>> if (!result)
>
> This hunk and the above change to Documentation/ABI/testing/ima_policy
> need to be moved to the next patch when you introduce the policy
> changes.
>
Will do.
~Tushar
> Tyler
>
>> --
>> 2.17.1
>>
Powered by blists - more mailing lists