IMA: Deferred measurement of keys
Subsystems (5)
v9 cover IMA: Deferred measurement of keys
The IMA subsystem supports measuring asymmetric keys when the key is created or updated[1]. But keys created or updated before a custom IMA policy is loaded are currently not measured. This includes keys added, for instance, to either the .ima or .builtin_trusted_keys keyrings, which happens early in the boot process.
Measuring the early boot keys, by design, requires loading a custom IMA policy. This change adds support for queuing keys created or updated before a custom IMA policy is loaded. The queued keys are processed when a custom policy is loaded. Keys created or updated after a custom policy is loaded are measured immediately (not queued). In the case when a custom policy is not loaded within 5 minutes of IMA initialization, the queued keys are freed.
[1] https://lore.kernel.org/linux-integrity/[email protected]/
Changelog:
v9
=> Rebased the changes to v5.5-rc7
=> Defined an intermediate Kconfig boolean option namely
IMA_QUEUE_EARLY_BOOT_KEYS to declare the deferred key measurement functions.
=> Use delayed workqueue to free the queued keys when a custom IMA
policy is not loaded.
v8
=> Rebased the changes to linux-next
=> Need to apply the following patch first
https://lore.kernel.org/linux-integrity/[email protected]/
v7
=> Updated cover letter per Mimi's suggestions. => Updated "Reported-by" tag to be specific about
the issues fixed in the patch.
v6
=> Replaced mutex with a spinlock to sychronize access to
queued keys. This fixes the problem reported by "kernel test robot <[email protected]>"
https://lore.kernel.org/linux-integrity/[email protected]/T/#t
=> Changed ima_queue_key() to a static function. This fixes
the issue reported by "kbuild test robot <[email protected]>"
https://lore.kernel.org/linux-integrity/[email protected]/
=> Added the patch to free the queued keys if a custom IMA policy
was not loaded to this patch set.
v5
=> Removed temp keys list in ima_process_queued_keys()
v4
=> Check and set ima_process_keys flag with mutex held.
v3
=> Defined ima_process_keys flag to be static. => Set ima_process_keys with ima_keys_mutex held. => Added a comment in ima_process_queued_keys() function
to state the use of temporary list for keys.
v2
=> Rebased the changes to v5.5-rc1
=> Updated function names, variable names, and code comments
to be less verbose.
v1
=> Code cleanup
v0
=> Based changes on v5.4-rc8
=> The following patchsets should be applied in that order
https://lore.kernel.org/linux-integrity/[email protected] https://lore.kernel.org/linux-integrity/[email protected]/
=> Added functions to queue and dequeue keys, and process
the queued keys when custom IMA policies are applied.
Lakshmi Ramasubramanian (3):
IMA: Define workqueue for early boot key measurements
IMA: Call workqueue functions to measure queued keys
IMA: Defined delayed workqueue to free the queued keys
security/integrity/ima/Kconfig | 5 + security/integrity/ima/Makefile | 1 + security/integrity/ima/ima.h | 24 +++ security/integrity/ima/ima_asymmetric_keys.c | 8 + security/integrity/ima/ima_init.c | 8 +- security/integrity/ima/ima_policy.c | 3 + security/integrity/ima/ima_queue_keys.c | 171 +++++++++++++++++++ 7 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 security/integrity/ima/ima_queue_keys.c
Signature · 1 line
2.17.1
PATCH v9 1/3 IMA: Define workqueue for early boot key measurements
Measuring keys requires a custom IMA policy to be loaded. Keys created or updated before a custom IMA policy is loaded should be queued and the keys should be processed after a custom policy is loaded.
This patch defines workqueue for queuing keys when a custom IMA policy has not yet been loaded. An intermediate Kconfig boolean option namely IMA_QUEUE_EARLY_BOOT_KEYS is used to declare the workqueue functions.
A flag namely ima_process_keys is used to check if the key should be queued or should be processed immediately.
--- security/integrity/ima/Kconfig | 5 + security/integrity/ima/Makefile | 1 + security/integrity/ima/ima.h | 22 ++++ security/integrity/ima/ima_queue_keys.c | 137 ++++++++++++++++++++++++ 4 files changed, 165 insertions(+) create mode 100644 security/integrity/ima/ima_queue_keys.c
security/integrity/ima/Kconfig +5 -0
@@ -317,3 +317,8 @@ config IMA_MEASURE_ASYMMETRIC_KEYS depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE=y default y +config IMA_QUEUE_EARLY_BOOT_KEYS + bool + depends on IMA_MEASURE_ASYMMETRIC_KEYS + depends on SYSTEM_TRUSTED_KEYRING + default y
security/integrity/ima/Makefile +1 -0
@@ -13,3 +13,4 @@ ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o obj-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o +obj-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o
security/integrity/ima/ima.h +22 -0
@@ -205,6 +205,28 @@ extern const char *const func_tokens[];struct modsig ; +#ifdef CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS +/* +* To track keys that need to be measured. +*/ +struct ima_key_entry { +struct list_head list; +void *payload; +size_t payload_len; +char *keyring_name; +}; +bool ima_should_queue_key (void ); +bool ima_queue_key (struct key *keyring,const void *payload, +size_t payload_len); +void ima_process_queued_keys (void ); +#else +static inline bool ima_should_queue_key (void ) {return false; } +static inline bool ima_queue_key (struct key *keyring, +const void *payload, +size_t payload_len) {return false; } +static inline void ima_process_queued_keys (void ) {} +#endif /* CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS */ +/* LIM API function definitions */ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, int mask, enum ima_hooks func, int *pcr,
security/integrity/ima/ima_queue_keys.c new file +137 -0
137 lines not on this page · raw message
Signature · 1 line
2.17.1
Mimi Zohar
maintainerRe: [PATCH v9 1/3] IMA: Define workqueue for early boot key measurements
Hi Lakshmi,
Quoted 4 lines · Lakshmi Ramasubramanian
Measuring keys requires a custom IMA policy to be loaded. Keys created or updated before a custom IMA policy is loaded should be queued and the keys should be processed after a custom policy is loaded.
The addition of "and the keys" makes the sentence a run-on. I've reverted this change.
Quoted 9 lines
This patch defines workqueue for queuing keys when a custom IMA policy has not yet been loaded. An intermediate
Kconfigboolean option namelyIMA_QUEUE_EARLY_BOOT_KEYSis used to declare the workqueue functions.A flag namely
ima_process_keysis used to check if the key should be queued or should be processed immediately.Signed-off-by Lakshmi Ramasubramanian <[email protected]>
Thanks! With this patch, if queueing keys ever becomes an issue, resolving it will be straight forward.
This patch set is now in the next-integrity-testing branch. Assuming there are no problems, it will be merged into the next-integrity branch, requiring a rebase. I'm sorry for any inconvenience this may cause.
Mimi
Quoted 206 lines
--- security/integrity/ima/Kconfig | 5 + security/integrity/ima/Makefile | 1 + security/integrity/ima/ima.h | 22 ++++ security/integrity/ima/ima_queue_keys.c | 137 ++++++++++++++++++++++++ 4 files changed, 165 insertions(+) create mode 100644 security/integrity/ima/ima_queue_keys.csecurity/integrity/ima/Kconfig +5
-0@@ -317,3 +317,8 @@ config IMA_MEASURE_ASYMMETRIC_KEYS depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE=y default y +config IMA_QUEUE_EARLY_BOOT_KEYS + bool + depends on IMA_MEASURE_ASYMMETRIC_KEYS + depends on SYSTEM_TRUSTED_KEYRING + default ysecurity/integrity/ima/Makefile +1
-0@@ -13,3 +13,4 @@ ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o obj-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o +obj-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.osecurity/integrity/ima/ima.h +22
-0@@ -205,6 +205,28 @@ extern const char *const func_tokens[];struct modsig ; +#ifdef CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS +/* +* To track keys that need to be measured. +*/ +struct ima_key_entry { +struct list_head list; +void *payload; +size_t payload_len; +char *keyring_name; +}; +bool ima_should_queue_key (void ); +bool ima_queue_key (struct key *keyring,const void *payload, +size_t payload_len); +void ima_process_queued_keys (void ); +#else +static inline bool ima_should_queue_key (void ) {return false; } +static inline bool ima_queue_key (struct key *keyring, +const void *payload, +size_t payload_len) {return false; } +static inline void ima_process_queued_keys (void ) {} +#endif /* CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS */ +/* LIM API function definitions */ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, int mask, enum ima_hooks func, int *pcr,security/integrity/ima/ima_queue_keys.c new file +137
-0@@ -0,0 +1,137 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* +* Copyright (C) 2019 Microsoft Corporation +* +* Author: Lakshmi Ramasubramanian ([email protected]) +* +* File: ima_queue_keys.c +* Enables deferred processing of keys +*/ + +#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt + +#include <keys/asymmetric-type.h> +#include "ima.h" + +/* +* Flag to indicate whether a key can be processed +* right away or should be queued for processing later. +*/ +static bool ima_process_keys; + +/* +* To synchronize access to the list of keys that need to be measured +*/ +static DEFINE_MUTEX (ima_keys_lock); +static LIST_HEAD (ima_keys); + +static void ima_free_key_entry (struct ima_key_entry *entry) +{ +if (entry) { +kfree (entry->payload); +kfree (entry->keyring_name); +kfree (entry); + } +} + +static struct ima_key_entry *ima_alloc_key_entry (struct key *keyring, +const void *payload, +size_t payload_len) +{ +int rc =0 ; +struct ima_key_entry *entry; + + entry =kzalloc (sizeof (*entry),GFP_KERNEL ); +if (entry) { + entry->payload =kmemdup (payload, payload_len,GFP_KERNEL ); + entry->keyring_name =kstrdup (keyring->description, +GFP_KERNEL ); + entry->payload_len = payload_len; + } + +if ((entry =NULL ) || (entry->payload =NULL ) || + (entry->keyring_name =NULL )) { + rc = -ENOMEM ; + goto out; + } + +INIT_LIST_HEAD (&entry->list); + +out: +if (rc) { +ima_free_key_entry (entry); + entry =NULL ; + } + +return entry; +} + +bool ima_queue_key (struct key *keyring,const void *payload, +size_t payload_len) +{ +bool queued = false; +struct ima_key_entry *entry; + + entry =ima_alloc_key_entry (keyring, payload, payload_len); +if (!entry) +return false; + +mutex_lock (&ima_keys_lock); +if (!ima_process_keys) { +list_add_tail (&entry->list, &ima_keys); + queued = true; + } +mutex_unlock (&ima_keys_lock); + +if (!queued) +ima_free_key_entry (entry); + +return queued; +} + +/* +* ima_process_queued_keys() - process keys queued for measurement +* +* This function sets ima_process_keys to true and processes queued keys. +* From here on keys will be processed right away (not queued). +*/ +void ima_process_queued_keys (void ) +{ +struct ima_key_entry *entry, *tmp; +bool process = false; + +if (ima_process_keys) +return ; + +/* +* Since ima_process_keys is set to true, any new key will be +* processed immediately and not be queued to ima_keys list. +* First one setting the ima_process_keys flag to true will +* process the queued keys. +*/ +mutex_lock (&ima_keys_lock); +if (!ima_process_keys) { + ima_process_keys = true; + process = true; + } +mutex_unlock (&ima_keys_lock); + +if (!process) +return ; + + +list_for_each_entry_safe (entry, tmp, &ima_keys, list) { +process_buffer_measurement (entry->payload, + entry->payload_len, + entry->keyring_name, +KEY_CHECK ,0 , + entry->keyring_name); +list_del (&entry->list); +ima_free_key_entry (entry); + } +} + +inline bool ima_should_queue_key (void ) +{ +return !ima_process_keys; +}
PATCH v9 2/3 IMA: Call workqueue functions to measure queued keys
In reply to Lakshmi RamasubramanianMeasuring keys requires a custom IMA policy to be loaded. Keys should be queued for measurement if a custom IMA policy is not yet loaded. Keys queued for measurement, if any, should be processed when a custom IMA policy is loaded.
This patch updates the IMA hook function ima_post_key_create_or_update() to queue the key if a custom IMA policy has not yet been loaded. And, ima_update_policy() function, which is called when
a custom IMA policy is loaded, is updated to process queued keys.
--- security/integrity/ima/ima_asymmetric_keys.c | 8 ++++++++ security/integrity/ima/ima_policy.c | 3 +++ 2 files changed, 11 insertions(+)
security/integrity/ima/ima_asymmetric_keys.c +8 -0
@@ -30,6 +30,8 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key, const void *payload, size_t payload_len, unsigned long flags, bool create) { + bool queued = false; +/* Only asymmetric keys are handled by this hook. */ if (key->type != &key_type_asymmetric)return ;
@@ -37,6 +39,12 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,if (!payload || (payload_len =0 ))return ; +if (ima_should_queue_key ()) + queued =ima_queue_key (keyring, payload, payload_len); + +if (queued) +return ; +/* * keyring->description points to the name of the keyring * (such as ".builtin_trusted_keys", ".ima", etc.) to
security/integrity/ima/ima_policy.c +3 -0
@@ -807,6 +807,9 @@ void ima_update_policy(void)kfree (arch_policy_entry); }ima_update_policy_flag (); + +/* Custom IMA policy has been loaded */ +ima_process_queued_keys (); }/* Keep the enumeration in sync with the policy_tokens! */
Signature · 1 line
2.17.1
PATCH v9 3/3 IMA: Defined delayed workqueue to free the queued keys
In reply to Lakshmi RamasubramanianKeys queued for measurement should be freed if a custom IMA policy was not loaded. Otherwise, the keys will remain queued forever consuming kernel memory.
This patch defines a delayed workqueue to handle the above scenario. The workqueue handler is setup to execute 5 minutes after IMA initialization is completed.
If a custom IMA policy is loaded before the workqueue handler is scheduled to execute, the workqueue task is cancelled and any queued keys are processed for measurement. But if a custom policy was not loaded then the queued keys are just freed when the delayed workqueue handler is run.
function called from invalid context
ima_init_key_queue() function.
--- security/integrity/ima/ima.h | 2 ++ security/integrity/ima/ima_init.c | 8 ++++- security/integrity/ima/ima_queue_keys.c | 44 ++++++++++++++++++++++--- 3 files changed, 48 insertions(+), 6 deletions(-)
security/integrity/ima/ima.h +2 -0
@@ -215,11 +215,13 @@ struct ima_key_entry {size_t payload_len;char *keyring_name; }; +void ima_init_key_queue (void );bool ima_should_queue_key (void );bool ima_queue_key (struct key *keyring,const void *payload,size_t payload_len);void ima_process_queued_keys (void );#else +static inline void ima_init_key_queue (void ) {}static inline bool ima_should_queue_key (void ) {return false; } static inline bool ima_queue_key(struct key *keyring, const void *payload,
security/integrity/ima/ima_init.c +7 -1
@@ -131,5 +131,11 @@ int __init ima_init(void)ima_init_policy (); -return ima_fs_init (); + rc =ima_fs_init (); +if (rc !=0 ) +return rc; + +ima_init_key_queue (); + +return rc; }
security/integrity/ima/ima_queue_keys.c +39 -5
64 lines not on this page · raw message
Signature · 1 line
2.17.1