Keys created or updated in the system are currently not measured.
Therefore an attestation service, for instance, would not be able to
attest whether or not the trusted keys keyring(s), for instance, contain
only known good (trusted) keys.
IMA measures system files, command line arguments passed to kexec,
boot aggregate, etc. It can be used to measure keys as well.
But there is no mechanism available in the kernel for IMA to
know when a key is created or updated.
This change aims to address measuring keys created or updated
in the system.
To achieve the above the following changes have been made:
- Added a new IMA hook namely, ima_post_key_create_or_update, which
measures the key. This IMA hook is called from key_create_or_update
function. The key measurement can be controlled through IMA policy.
A new IMA policy function KEY_CHECK has been added to measure keys.
"keyrings=" option can be specified for KEY_CHECK to limit
measuring the keys loaded onto the specified keyrings only.
# measure keys loaded onto any keyring
measure func=KEY_CHECK
# measure keys loaded onto the IMA keyring only
measure func=KEY_CHECK keyring=".ima"
# measure keys on the BUILTIN and IMA keyrings into a different PCR
measure func=KEY_CHECK keyring=".builtin_trusted_keys|.ima" pcr=11
Testing performed:
* Booted the kernel with this change.
* When KEY_CHECK policy is set IMA measures keys loaded
onto any keyring (keyrings= option not specified).
* Keys are not measured when KEY_CHECK is not set.
* When keyrings= option is specified for KEY_CHECK then only the keys
loaded onto a keyring specified in the option is measured.
* Added a new key to a keyring.
=> Added keys to .ima and .evm keyrings.
* Added the same key again.
=> Add the same key to .ima and .evm keyrings.
Change Log:
v8:
=> Updated ima_match_keyring() function to check for
whole keyring name match.
=> Used CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE instead of
CONFIG_KEYS to build ima_asymmetric_keys.c and enable
the IMA hook to measure keys since this config handles
the required build time dependencies better.
=> Updated patch description to illustrate verification
of key measurement.
v7:
=> Removed CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS option and used
CONFIG_KEYS instead for ima_asymmetric_keys.c
=> Added the patches related to "keyrings=" option support to
this patch set.
v6:
=> Rebased the changes to v5.4-rc7
=> Renamed KEYRING_CHECK to KEY_CHECK per Mimi's suggestion.
=> Excluded the patches that add support for limiting key
measurement to specific keyrings ("keyrings=" option
for "measure func=KEY_CHECK" in the IMA policy).
Also, excluded the patches that add support for deferred
processing of keys (queue support).
These patches will be added in separate patch sets later.
v5:
=> Reorganized the patches to add measurement of keys through
the IMA hook without any queuing and then added queuing support.
=> Updated the queuing functions to minimize code executed inside mutex.
=> Process queued keys after custom IMA policies have been applied.
v4:
=> Rebased the changes to v5.4-rc3
=> Applied the following dependent patch set first
and then added new changes.
https://lore.kernel.org/linux-integrity/[email protected]
=> Refactored the patch set to separate out changes related to
func KEYRING_CHECK and options keyrings into different patches.
=> Moved the functions to queue and dequeue keys for measurement
from ima_queue.c to a new file ima_asymmetric_keys.c.
=> Added a new config namely CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
to compile ima_asymmetric_keys.c
v3:
=> Added KEYRING_CHECK for measuring keys. This can optionally specify
keyrings to measure.
=> Updated ima_get_action() and related functions to return
the keyrings if specified in the policy.
=> process_buffer_measurement() function is updated to take keyring
as a parameter. The key will be measured if the policy includes
the keyring in the list of measured keyrings. If the policy does not
specify any keyrings then all keys are measured.
v2:
=> Per suggestion from Mimi reordered the patch set to first
enable measuring keys added or updated in the system.
And, then scope the measurement to keys added to
builtin_trusted_keys keyring through ima policy.
=> Removed security_key_create_or_update function and instead
call ima hook, to measure the key, directly from
key_create_or_update function.
v1:
=> LSM function for key_create_or_update. It calls ima.
=> Added ima hook for measuring keys
=> ima measures keys based on ima policy.
v0:
=> Added LSM hook for key_create_or_update.
=> Measure keys added to builtin or secondary trusted keys keyring.
Lakshmi Ramasubramanian (5):
IMA: Add KEY_CHECK func to measure keys
IMA: Define an IMA hook to measure keys
KEYS: Call the IMA hook to measure keys
IMA: Add support to limit measuring keys
IMA: Read keyrings= option from the IMA policy
Documentation/ABI/testing/ima_policy | 17 +++-
include/linux/ima.h | 13 +++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 9 +-
security/integrity/ima/ima_api.c | 8 +-
security/integrity/ima/ima_appraise.c | 4 +-
security/integrity/ima/ima_asymmetric_keys.c | 57 +++++++++++
security/integrity/ima/ima_main.c | 9 +-
security/integrity/ima/ima_policy.c | 100 +++++++++++++++++--
security/keys/key.c | 7 ++
10 files changed, 205 insertions(+), 20 deletions(-)
create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
--
2.17.1
Measure asymmetric keys used for verifying file signatures,
certificates, etc.
This patch defines a new IMA hook namely ima_post_key_create_or_update()
to measure asymmetric keys.
The IMA hook is defined in a new file namely ima_asymmetric_keys.c
which is built only if CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE is enabled.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
Cc: Sasha Levin <[email protected]>
Cc: James Morris <[email protected]>
---
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
2 files changed, 52 insertions(+)
create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 31d57cdf2421..207a0a9eb72c 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -12,3 +12,4 @@ ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
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_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += ima_asymmetric_keys.o
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
new file mode 100644
index 000000000000..f6884641a622
--- /dev/null
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Microsoft Corporation
+ *
+ * Author: Lakshmi Ramasubramanian ([email protected])
+ *
+ * File: ima_asymmetric_keys.c
+ * Defines an IMA hook to measure asymmetric keys on key
+ * create or update.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.h>
+#include "ima.h"
+
+/**
+ * ima_post_key_create_or_update - measure asymmetric keys
+ * @keyring: keyring to which the key is linked to
+ * @key: created or updated key
+ * @flags: key flags
+ * @create: flag indicating whether the key was created or updated
+ *
+ * Keys can only be measured, not appraised.
+ */
+void ima_post_key_create_or_update(struct key *keyring, struct key *key,
+ unsigned long flags, bool create)
+{
+ const struct public_key *pk;
+
+ /* Only asymmetric keys are handled by this hook. */
+ if (key->type != &key_type_asymmetric)
+ return;
+
+ /* Get the public_key of the given asymmetric key to measure. */
+ pk = key->payload.data[asym_crypto];
+
+ /*
+ * keyring->description points to the name of the keyring
+ * (such as ".builtin_trusted_keys", ".ima", etc.) to
+ * which the given key is linked to.
+ *
+ * The name of the keyring is passed in the "eventname"
+ * parameter to process_buffer_measurement() and is set
+ * in the "eventname" field in ima_event_data for
+ * the key measurement IMA event.
+ */
+ process_buffer_measurement(pk->key, pk->keylen,
+ keyring->description, KEY_CHECK, 0);
+}
--
2.17.1
Measure keys loaded onto any keyring.
This patch defines a new IMA policy func namely KEY_CHECK to
measure keys. Updated ima_match_rules() to check for KEY_CHECK
and ima_parse_rule() to handle KEY_CHECK.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
Cc: Sasha Levin <[email protected]>
Cc: James Morris <[email protected]>
---
Documentation/ABI/testing/ima_policy | 7 ++++++-
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_policy.c | 4 +++-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 29aaedf33246..3823c27894c5 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]
+ [KEXEC_CMDLINE] [KEY_CHECK]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
@@ -113,3 +113,8 @@ Description:
Example of appraise rule allowing modsig appended signatures:
appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
+
+ Example of measure rule using KEY_CHECK to measure all keys:
+
+ measure func=KEY_CHECK
+
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index df4ca482fb53..fe6c698617bd 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -193,6 +193,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
hook(KEXEC_INITRAMFS_CHECK) \
hook(POLICY_CHECK) \
hook(KEXEC_CMDLINE) \
+ hook(KEY_CHECK) \
hook(MAX_CHECK)
#define __ima_hook_enumify(ENUM) ENUM,
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f19a895ad7cd..1525a28fd705 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -373,7 +373,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
{
int i;
- if (func == KEXEC_CMDLINE) {
+ if ((func == KEXEC_CMDLINE) || (func == KEY_CHECK)) {
if ((rule->flags & IMA_FUNC) && (rule->func == func))
return true;
return false;
@@ -997,6 +997,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->func = POLICY_CHECK;
else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0)
entry->func = KEXEC_CMDLINE;
+ else if (strcmp(args[0].from, "KEY_CHECK") == 0)
+ entry->func = KEY_CHECK;
else
result = -EINVAL;
if (!result)
--
2.17.1
Call the IMA hook from key_create_or_update function to measure
the key when a new key is created or an existing key is updated.
This patch adds the call to the IMA hook from key_create_or_update
function to measure the key on key create or update.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
Cc: David Howells <[email protected]>
Cc: Sasha Levin <[email protected]>
Cc: James Morris <[email protected]>
---
include/linux/ima.h | 13 +++++++++++++
security/keys/key.c | 7 +++++++
2 files changed, 20 insertions(+)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 6d904754d858..6b0824b7a32f 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -25,6 +25,12 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
extern void ima_post_path_mknod(struct dentry *dentry);
extern void ima_kexec_cmdline(const void *buf, int size);
+#ifdef CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+extern void ima_post_key_create_or_update(struct key *keyring,
+ struct key *key,
+ unsigned long flags, bool create);
+#endif
+
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
@@ -101,6 +107,13 @@ static inline void ima_add_kexec_buffer(struct kimage *image)
{}
#endif
+#ifndef CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+static inline void ima_post_key_create_or_update(struct key *keyring,
+ struct key *key,
+ unsigned long flags,
+ bool create) {}
+#endif
+
#ifdef CONFIG_IMA_APPRAISE
extern bool is_ima_appraise_enabled(void);
extern void ima_inode_post_setattr(struct dentry *dentry);
diff --git a/security/keys/key.c b/security/keys/key.c
index 764f4c57913e..a0f1e7b3b8b9 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -13,6 +13,7 @@
#include <linux/security.h>
#include <linux/workqueue.h>
#include <linux/random.h>
+#include <linux/ima.h>
#include <linux/err.h>
#include "internal.h"
@@ -936,6 +937,8 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_link_end;
}
+ ima_post_key_create_or_update(keyring, key, flags, true);
+
key_ref = make_key_ref(key, is_key_possessed(keyring_ref));
error_link_end:
@@ -965,6 +968,10 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
}
key_ref = __key_update(key_ref, &prep);
+
+ if (!IS_ERR(key_ref))
+ ima_post_key_create_or_update(keyring, key, flags, false);
+
goto error_free_prep;
}
EXPORT_SYMBOL(key_create_or_update);
--
2.17.1
Read "keyrings=" option, if specified in the IMA policy, and store in
the list of IMA rules when the configured IMA policy is read.
This patch defines a new policy token enum namely Opt_keyrings
and an option flag IMA_KEYRINGS for reading "keyrings=" option
from the IMA policy.
Updated ima_parse_rule() to parse "keyrings=" option in the policy.
Updated ima_policy_show() to display "keyrings=" option.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
Cc: Sasha Levin <[email protected]>
Cc: James Morris <[email protected]>
---
security/integrity/ima/ima_policy.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d9400585fcda..78b25f083fe1 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -34,6 +34,7 @@
#define IMA_EUID 0x0080
#define IMA_PCR 0x0100
#define IMA_FSNAME 0x0200
+#define IMA_KEYRINGS 0x0400
#define UNKNOWN 0
#define MEASURE 0x0001 /* same as IMA_MEASURE */
@@ -825,7 +826,8 @@ enum {
Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
Opt_appraise_type, Opt_appraise_flag,
- Opt_permit_directio, Opt_pcr, Opt_template, Opt_err
+ Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
+ Opt_err
};
static const match_table_t policy_tokens = {
@@ -861,6 +863,7 @@ static const match_table_t policy_tokens = {
{Opt_permit_directio, "permit_directio"},
{Opt_pcr, "pcr=%s"},
{Opt_template, "template=%s"},
+ {Opt_keyrings, "keyrings=%s"},
{Opt_err, NULL}
};
@@ -1110,6 +1113,23 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
result = 0;
entry->flags |= IMA_FSNAME;
break;
+ case Opt_keyrings:
+ ima_log_string(ab, "keyrings", args[0].from);
+
+ if ((entry->keyrings) ||
+ (entry->action != MEASURE) ||
+ (entry->func != KEY_CHECK)) {
+ result = -EINVAL;
+ break;
+ }
+ entry->keyrings = kstrdup(args[0].from, GFP_KERNEL);
+ if (!entry->keyrings) {
+ result = -ENOMEM;
+ break;
+ }
+ result = 0;
+ entry->flags |= IMA_KEYRINGS;
+ break;
case Opt_fsuuid:
ima_log_string(ab, "fsuuid", args[0].from);
@@ -1485,6 +1505,13 @@ int ima_policy_show(struct seq_file *m, void *v)
seq_puts(m, " ");
}
+ if (entry->flags & IMA_KEYRINGS) {
+ if (entry->keyrings != NULL)
+ snprintf(tbuf, sizeof(tbuf), "%s", entry->keyrings);
+ seq_printf(m, pt(Opt_keyrings), tbuf);
+ seq_puts(m, " ");
+ }
+
if (entry->flags & IMA_PCR) {
snprintf(tbuf, sizeof(tbuf), "%d", entry->pcr);
seq_printf(m, pt(Opt_pcr), tbuf);
--
2.17.1
> On Nov 18, 2019, at 3:38 PM, Lakshmi Ramasubramanian <[email protected]> wrote:
>
> Call the IMA hook from key_create_or_update function to measure
> the key when a new key is created or an existing key is updated.
>
> This patch adds the call to the IMA hook from key_create_or_update
> function to measure the key on key create or update.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> Cc: David Howells <[email protected]>
> Cc: Sasha Levin <[email protected]>
> Cc: James Morris <[email protected]>
> ---
> include/linux/ima.h | 13 +++++++++++++
> security/keys/key.c | 7 +++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6d904754d858..6b0824b7a32f 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -25,6 +25,12 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
> extern void ima_post_path_mknod(struct dentry *dentry);
> extern void ima_kexec_cmdline(const void *buf, int size);
>
> +#ifdef CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> +extern void ima_post_key_create_or_update(struct key *keyring,
> + struct key *key,
> + unsigned long flags, bool create);
> +#endif
The extern void ima_post_key_create_or_update will only be defined if CONFIG_IMA=y.
> +
> #ifdef CONFIG_IMA_KEXEC
> extern void ima_add_kexec_buffer(struct kimage *image);
> #endif
> @@ -101,6 +107,13 @@ static inline void ima_add_kexec_buffer(struct kimage *image)
> {}
> #endif
>
> +#ifndef CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> +static inline void ima_post_key_create_or_update(struct key *keyring,
> + struct key *key,
> + unsigned long flags,
> + bool create) {}
> +#endif
> +
> #ifdef CONFIG_IMA_APPRAISE
> extern bool is_ima_appraise_enabled(void);
> extern void ima_inode_post_setattr(struct dentry *dentry);
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 764f4c57913e..a0f1e7b3b8b9 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -13,6 +13,7 @@
> #include <linux/security.h>
> #include <linux/workqueue.h>
> #include <linux/random.h>
> +#include <linux/ima.h>
> #include <linux/err.h>
> #include "internal.h"
>
> @@ -936,6 +937,8 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
> goto error_link_end;
> }
>
> + ima_post_key_create_or_update(keyring, key, flags, true);
This will cause a compile error if CONFIG_IMA is not defined and CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE=y.
security/keys/key.c: In function 'key_create_or_update':
security/keys/key.c:940:2: error: implicit declaration of function 'ima_post_key_create_or_update'; did you mean 'key_create_or_update'? [-Werror=implicit-function-declaration]
ima_post_key_create_or_update(keyring, key, flags, true);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
key_create_or_update
cc1: some warnings being treated as errors
> +
> key_ref = make_key_ref(key, is_key_possessed(keyring_ref));
>
> error_link_end:
> @@ -965,6 +968,10 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
> }
>
> key_ref = __key_update(key_ref, &prep);
> +
> + if (!IS_ERR(key_ref))
> + ima_post_key_create_or_update(keyring, key, flags, false);
> +
> goto error_free_prep;
> }
> EXPORT_SYMBOL(key_create_or_update);
> --
> 2.17.1
>
On 11/18/19 5:18 PM, Eric Snowberg wrote:
>> +#ifdef CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
>> +extern void ima_post_key_create_or_update(struct key *keyring,
>> + struct key *key,
>> + unsigned long flags, bool create);
>> +#endif
>
> The extern void ima_post_key_create_or_update will only be defined if CONFIG_IMA=y.
>
>
> This will cause a compile error if CONFIG_IMA is not defined and CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE=y.
>
> security/keys/key.c: In function 'key_create_or_update':
> security/keys/key.c:940:2: error: implicit declaration of function 'ima_post_key_create_or_update'; did you mean 'key_create_or_update'? [-Werror=implicit-function-declaration]
> ima_post_key_create_or_update(keyring, key, flags, true);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> key_create_or_update
> cc1: some warnings being treated as errors
You are right - Thanks for catching this error.
I'll fix this and send an update.
thanks,
-lakshmi
> On Nov 18, 2019, at 3:38 PM, Lakshmi Ramasubramanian <[email protected]> wrote:
>
> Measure asymmetric keys used for verifying file signatures,
> certificates, etc.
>
> This patch defines a new IMA hook namely ima_post_key_create_or_update()
> to measure asymmetric keys.
>
> The IMA hook is defined in a new file namely ima_asymmetric_keys.c
> which is built only if CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE is enabled.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> Cc: Sasha Levin <[email protected]>
> Cc: James Morris <[email protected]>
> ---
> security/integrity/ima/Makefile | 1 +
> security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
> 2 files changed, 52 insertions(+)
> create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
>
> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> index 31d57cdf2421..207a0a9eb72c 100644
> --- a/security/integrity/ima/Makefile
> +++ b/security/integrity/ima/Makefile
> @@ -12,3 +12,4 @@ ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
> 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_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += ima_asymmetric_keys.o
> diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
> new file mode 100644
> index 000000000000..f6884641a622
> --- /dev/null
> +++ b/security/integrity/ima/ima_asymmetric_keys.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Microsoft Corporation
> + *
> + * Author: Lakshmi Ramasubramanian ([email protected])
> + *
> + * File: ima_asymmetric_keys.c
> + * Defines an IMA hook to measure asymmetric keys on key
> + * create or update.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <crypto/public_key.h>
> +#include <keys/asymmetric-type.h>
> +#include "ima.h"
> +
> +/**
> + * ima_post_key_create_or_update - measure asymmetric keys
> + * @keyring: keyring to which the key is linked to
> + * @key: created or updated key
> + * @flags: key flags
> + * @create: flag indicating whether the key was created or updated
> + *
> + * Keys can only be measured, not appraised.
> + */
> +void ima_post_key_create_or_update(struct key *keyring, struct key *key,
> + unsigned long flags, bool create)
> +{
> + const struct public_key *pk;
> +
> + /* Only asymmetric keys are handled by this hook. */
> + if (key->type != &key_type_asymmetric)
> + return;
> +
> + /* Get the public_key of the given asymmetric key to measure. */
> + pk = key->payload.data[asym_crypto];
> +
> + /*
> + * keyring->description points to the name of the keyring
> + * (such as ".builtin_trusted_keys", ".ima", etc.) to
> + * which the given key is linked to.
> + *
> + * The name of the keyring is passed in the "eventname"
> + * parameter to process_buffer_measurement() and is set
> + * in the "eventname" field in ima_event_data for
> + * the key measurement IMA event.
> + */
> + process_buffer_measurement(pk->key, pk->keylen,
> + keyring->description, KEY_CHECK, 0);
I’m interested in using this patch series, however I get the following on every boot:
[ 1.185105] Loading compiled-in X.509 certificates
[ 1.190240] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 1.191835] #PF: supervisor read access in kernel mode
[ 1.193041] #PF: error_code(0x0000) - not-present page
[ 1.194224] PGD 0 P4D 0
[ 1.194832] Oops: 0000 [#1] SMP PTI
[ 1.195654] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.4.0-rc7.imakeys.rc1.x86_64 #1
[ 1.197667] Hardware name:
[ 1.198987] RIP: 0010:ima_match_policy+0x69/0x4e0
[ 1.200072] Code: 48 89 45 90 65 48 8b 04 25 28 00 00 00 48 89 45 d0 31 c0 4d 85 ff 74 08 e8 94 14 00 00 49 89 07 48 8b 05 8a 43 7f 01 45 31 e4 <48> 8b 18 48 39 d8 0f 84 25 02 00 00 41 8d 46 f5 45 89 e0 4c 8b 65
[ 1.204401] RSP: 0018:ffffb9f30001bac8 EFLAGS: 00010246
[ 1.205622] RAX: 0000000000000000 RBX: ffff9e659de81800 RCX: 000000000000000c
[ 1.207275] RDX: 0000000000000000 RSI: ffffffff9b13cdf8 RDI: ffffffff9b13cdf8
[ 1.208938] RBP: ffffb9f30001bb48 R08: ffffffff9b529200 R09: 0000000000000000
[ 1.210560] R10: ffff9e6447d06c00 R11: 0000000082c49530 R12: 0000000000000000
[ 1.212279] R13: 0000000000000000 R14: 000000000000000c R15: ffffb9f30001bbb0
[ 1.213944] FS: 0000000000000000(0000) GS:ffff9e65b7a80000(0000) knlGS:0000000000000000
[ 1.215768] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1.217114] CR2: 0000000000000000 CR3: 000000014240a001 CR4: 0000000000760ee0
[ 1.218734] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1.220481] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1.222139] PKRU: 55555554
[ 1.222749] Call Trace:
[ 1.223344] ? crypto_destroy_tfm+0x5f/0xb0
[ 1.224315] ima_get_action+0x2c/0x30
[ 1.225148] process_buffer_measurement+0x1da/0x230
[ 1.226306] ima_post_key_create_or_update+0x3b/0x40
[ 1.227459] key_create_or_update+0x371/0x5c0
[ 1.228494] load_system_certificate_list+0x99/0xfa
[ 1.229588] ? system_trusted_keyring_init+0xfb/0xfb
[ 1.230705] ? do_early_param+0x95/0x95
[ 1.231574] do_one_initcall+0x4a/0x1fa
[ 1.232444] ? do_early_param+0x95/0x95
[ 1.233313] kernel_init_freeable+0x1c2/0x267
[ 1.234300] ? rest_init+0xb0/0xb0
[ 1.235075] kernel_init+0xe/0x110
[ 1.235842] ret_from_fork+0x24/0x50
[ 1.236659] Modules linked in:
[ 1.237358] CR2: 0000000000000000
[ 1.238112] ---[ end trace 163f3f61dfaef23f ]—
I believe this is because ima_rules used within ima_match_policy has not been initialized yet, when process_buffer_measurement is called above.
On 11/20/2019 3:28 PM, Eric Snowberg wrote:
Hi Eric,
>
> I’m interested in using this patch series, however I get the following on every boot:
> [ 1.222749] Call Trace:
> [ 1.223344] ? crypto_destroy_tfm+0x5f/0xb0
> [ 1.224315] ima_get_action+0x2c/0x30
> [ 1.225148] process_buffer_measurement+0x1da/0x230
> [ 1.226306] ima_post_key_create_or_update+0x3b/0x40
This is happening because IMA is not yet initialized when the IMA hook
is called.
I had the following check in process_buffer_measurement() as part of my
patch, but removed it since it is being upstreamed separately (by Mimi)
if (!ima_policy_flag)
return;
Until this change is in, please add the above line locally on entry to
process_buffer_measurement() to get around the issue.
thanks,
-lakshmi
On Wed, 2019-11-20 at 15:40 -0800, Lakshmi Ramasubramanian wrote:
> On 11/20/2019 3:28 PM, Eric Snowberg wrote:
> Hi Eric,
>
> >
> > I’m interested in using this patch series, however I get the following on every boot:
>
> > [ 1.222749] Call Trace:
> > [ 1.223344] ? crypto_destroy_tfm+0x5f/0xb0
> > [ 1.224315] ima_get_action+0x2c/0x30
> > [ 1.225148] process_buffer_measurement+0x1da/0x230
> > [ 1.226306] ima_post_key_create_or_update+0x3b/0x40
>
> This is happening because IMA is not yet initialized when the IMA hook
> is called.
>
> I had the following check in process_buffer_measurement() as part of my
> patch, but removed it since it is being upstreamed separately (by Mimi)
>
> if (!ima_policy_flag)
> return;
Did you post it as a separate patch? I can't seem to find it.
Mimi
>
> Until this change is in, please add the above line locally on entry to
> process_buffer_measurement() to get around the issue.
>
> thanks,
> -lakshmi
On 11/20/19 5:22 PM, Mimi Zohar wrote:
>> I had the following check in process_buffer_measurement() as part of my
>> patch, but removed it since it is being upstreamed separately (by Mimi)
>>
>> if (!ima_policy_flag)
>> return;
>
> Did you post it as a separate patch? I can't seem to find it.
>
> Mimi
No - I removed the above change from my patch since you mentioned it's
being upstreamed separately.
I didn't realize you wanted me to include the above change alone in a
separate patch (in my patch set). Sorry - I guess I misunderstood.
I can do that when I send an update - I expect to by the end of this week.
thanks,
-lakshmi
On 11/20/19 5:22 PM, Mimi Zohar wrote:
>> I had the following check in process_buffer_measurement() as part of my
>> patch, but removed it since it is being upstreamed separately (by Mimi)
>>
>> if (!ima_policy_flag)
>> return;
>
> Did you post it as a separate patch? I can't seem to find it.
>
> Mimi
>
I have sent a separate patch with just this change (to check
ima_policy_flag in process_buffer_measurement()).
thanks,
-lakshmi