Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v6 0/3] KEYS: Measure keys when they are created or updated
@ 2019-11-13 18:46 Lakshmi Ramasubramanian
  2019-11-13 18:46 ` [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

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.

   # measure keys loaded onto any keyring
   measure func=KEY_CHECK

Testing performed:

  * Booted the kernel with this change.
  * When KEY_CHECK policy is set IMA measures keys loaded
    onto any keyring.
  * Keys are not measured when KEY_CHECK is not set.
  * 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:

  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/1572492694-6520-1-git-send-email-zohar@linux.ibm.com
  => 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 (3):
  IMA: Add KEY_CHECK func to measure keys
  IMA: Define an IMA hook to measure keys
  KEYS: Call the IMA hook to measure keys

 Documentation/ABI/testing/ima_policy         |  6 ++-
 include/linux/ima.h                          | 13 +++++
 security/integrity/ima/Kconfig               | 14 ++++++
 security/integrity/ima/Makefile              |  1 +
 security/integrity/ima/ima.h                 |  1 +
 security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
 security/integrity/ima/ima_main.c            |  7 +++
 security/integrity/ima/ima_policy.c          |  4 +-
 security/keys/key.c                          |  9 ++++
 9 files changed, 104 insertions(+), 2 deletions(-)
 create mode 100644 security/integrity/ima/ima_asymmetric_keys.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
  2019-11-13 18:46 [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
@ 2019-11-13 18:46 ` Lakshmi Ramasubramanian
  2019-11-13 20:14   ` Mimi Zohar
  2019-11-13 18:46 ` [PATCH v6 2/3] IMA: Define an IMA hook " Lakshmi Ramasubramanian
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

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 <nramas@linux.microsoft.com>
---
 Documentation/ABI/testing/ima_policy | 6 +++++-
 security/integrity/ima/ima.h         | 1 +
 security/integrity/ima/ima_main.c    | 7 +++++++
 security/integrity/ima/ima_policy.c  | 4 +++-
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 29aaedf33246..066d32797500 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,7 @@ 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_main.c b/security/integrity/ima/ima_main.c
index d7e987baf127..12684e8d7124 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
 	int action = 0;
 	u32 secid;
 
+	/*
+	 * If IMA is not yet initialized or IMA policy is empty
+	 * then there is no need to measure.
+	 */
+	if (!ima_policy_flag)
+		return;
+
 	/*
 	 * Both LSM hooks and auxilary based buffer measurements are
 	 * based on policy.  To avoid code duplication, differentiate
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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
  2019-11-13 18:46 [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
  2019-11-13 18:46 ` [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
@ 2019-11-13 18:46 ` " Lakshmi Ramasubramanian
  2019-11-13 20:09   ` Mimi Zohar
  2019-11-13 18:46 ` [PATCH v6 3/3] KEYS: Call the " Lakshmi Ramasubramanian
  2019-11-13 22:02 ` [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Mimi Zohar
  3 siblings, 1 reply; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

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.

Note that currently IMA subsystem can be enabled without
enabling KEYS subsystem.

Adding support for measuring asymmetric keys in IMA requires KEYS
subsystem to be enabled. To handle this dependency a new config
namely CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS has been added. Enabling
this config requires the following configs to be enabled:
    CONFIG_IMA, CONFIG_KEYS, CONFIG_ASYMMETRIC_KEY_TYPE, and
    CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE.

CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is off by default.

The IMA hook is defined in a new file namely ima_asymmetric_keys.c
which is built only if CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is enabled.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/Kconfig               | 14 ++++++
 security/integrity/ima/Makefile              |  1 +
 security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 security/integrity/ima/ima_asymmetric_keys.c

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 838476d780e5..c6d14884bc19 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -310,3 +310,17 @@ config IMA_APPRAISE_SIGNED_INIT
 	default n
 	help
 	   This option requires user-space init to be signed.
+
+config IMA_MEASURE_ASYMMETRIC_KEYS
+	bool "Enable measuring asymmetric keys on key create or update"
+	depends on IMA
+	depends on KEYS
+	depends on ASYMMETRIC_KEY_TYPE
+	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+	default n
+	help
+	   This option enables measuring asymmetric keys when
+	   the key is created or updated. Additionally, IMA policy
+	   needs to be configured to either measure keys linked to
+	   any keyring or only measure keys linked to the keyrings
+	   specified in the IMA policy through the keyrings= option.
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 31d57cdf2421..3e9d0ad68c7b 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_IMA_MEASURE_ASYMMETRIC_KEYS) += 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 (nramas@linux.microsoft.com)
+ *
+ * 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6 3/3] KEYS: Call the IMA hook to measure keys
  2019-11-13 18:46 [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
  2019-11-13 18:46 ` [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
  2019-11-13 18:46 ` [PATCH v6 2/3] IMA: Define an IMA hook " Lakshmi Ramasubramanian
@ 2019-11-13 18:46 ` " Lakshmi Ramasubramanian
  2019-11-13 20:09   ` Mimi Zohar
  2019-11-13 22:02 ` [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Mimi Zohar
  3 siblings, 1 reply; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

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 <nramas@linux.microsoft.com>
---
 include/linux/ima.h | 13 +++++++++++++
 security/keys/key.c |  9 +++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 6d904754d858..ec5afe319ab7 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_IMA_MEASURE_ASYMMETRIC_KEYS
+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_IMA_MEASURE_ASYMMETRIC_KEYS
+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..9782d4d046fd 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,9 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 		goto error_link_end;
 	}
 
+	/* let the ima module know about the created key. */
+	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 +969,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 	}
 
 	key_ref = __key_update(key_ref, &prep);
+
+	/* let the ima module know about the updated key. */
+	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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
  2019-11-13 18:46 ` [PATCH v6 2/3] IMA: Define an IMA hook " Lakshmi Ramasubramanian
@ 2019-11-13 20:09   ` Mimi Zohar
  2019-11-13 20:52     ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 14+ messages in thread
From: Mimi Zohar @ 2019-11-13 20:09 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-integrity, linux-security-module, keyrings,
	linux-kernel

On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian 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.
> 
> Note that currently IMA subsystem can be enabled without
> enabling KEYS subsystem.
> 
> Adding support for measuring asymmetric keys in IMA requires KEYS
> subsystem to be enabled. To handle this dependency a new config
> namely CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS has been added. Enabling
> this config requires the following configs to be enabled:
>     CONFIG_IMA, CONFIG_KEYS, CONFIG_ASYMMETRIC_KEY_TYPE, and
>     CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE.
> 
> CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is off by default.
> 
> The IMA hook is defined in a new file namely ima_asymmetric_keys.c
> which is built only if CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is enabled.
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

All that is is needed is the key and public_key structures, which are
defined in include/linux/keys.h and include/crypto/public_key.h.  If
the keys subsystem is disabled, then the new IMA hook won't be called.
 There's no need for a new Kconfig option or a new file.

Please move the hook to just after ima_kexec_cmdline().

Mimi

> ---
>  security/integrity/ima/Kconfig               | 14 ++++++
>  security/integrity/ima/Makefile              |  1 +
>  security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
>  3 files changed, 66 insertions(+)
>  create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 838476d780e5..c6d14884bc19 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -310,3 +310,17 @@ config IMA_APPRAISE_SIGNED_INIT
>  	default n
>  	help
>  	   This option requires user-space init to be signed.
> +
> +config IMA_MEASURE_ASYMMETRIC_KEYS
> +	bool "Enable measuring asymmetric keys on key create or update"
> +	depends on IMA
> +	depends on KEYS
> +	depends on ASYMMETRIC_KEY_TYPE
> +	depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> +	default n
> +	help
> +	   This option enables measuring asymmetric keys when
> +	   the key is created or updated. Additionally, IMA policy
> +	   needs to be configured to either measure keys linked to
> +	   any keyring or only measure keys linked to the keyrings
> +	   specified in the IMA policy through the keyrings= option.
> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> index 31d57cdf2421..3e9d0ad68c7b 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_IMA_MEASURE_ASYMMETRIC_KEYS) += 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 (nramas@linux.microsoft.com)
> + *
> + * 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);
> +}


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 3/3] KEYS: Call the IMA hook to measure keys
  2019-11-13 18:46 ` [PATCH v6 3/3] KEYS: Call the " Lakshmi Ramasubramanian
@ 2019-11-13 20:09   ` Mimi Zohar
  0 siblings, 0 replies; 14+ messages in thread
From: Mimi Zohar @ 2019-11-13 20:09 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-integrity, linux-security-module, keyrings,
	linux-kernel

On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian 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 <nramas@linux.microsoft.com>
> ---
>  include/linux/ima.h | 13 +++++++++++++
>  security/keys/key.c |  9 +++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6d904754d858..ec5afe319ab7 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_IMA_MEASURE_ASYMMETRIC_KEYS
> +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_IMA_MEASURE_ASYMMETRIC_KEYS
> +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..9782d4d046fd 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,9 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
>  		goto error_link_end;
>  	}
>  
> +	/* let the ima module know about the created key. */

This comment and the one below doesn't provide any additional
information.  Please remove them.

> +	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 +969,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
>  	}
>  
>  	key_ref = __key_update(key_ref, &prep);
> +
> +	/* let the ima module know about the updated key. */
> +	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);


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
  2019-11-13 18:46 ` [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
@ 2019-11-13 20:14   ` Mimi Zohar
  2019-11-13 20:21     ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 14+ messages in thread
From: Mimi Zohar @ 2019-11-13 20:14 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-integrity, linux-security-module, keyrings,
	linux-kernel

On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian wrote:
> 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 <nramas@linux.microsoft.com>
> ---
>  Documentation/ABI/testing/ima_policy | 6 +++++-
>  security/integrity/ima/ima.h         | 1 +
>  security/integrity/ima/ima_main.c    | 7 +++++++
>  security/integrity/ima/ima_policy.c  | 4 +++-
>  4 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 29aaedf33246..066d32797500 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,7 @@ 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_main.c b/security/integrity/ima/ima_main.c
> index d7e987baf127..12684e8d7124 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
>  	int action = 0;
>  	u32 secid;
>  
> +	/*
> +	 * If IMA is not yet initialized or IMA policy is empty
> +	 * then there is no need to measure.
> +	 */
> +	if (!ima_policy_flag)
> +		return;
> +

This addition has nothing to do with defining a new IMA hook and
should be a separate patch.  This can be posted independently of this
patch set.

Mimi

>  	/*
>  	 * Both LSM hooks and auxilary based buffer measurements are
>  	 * based on policy.  To avoid code duplication, differentiate
> 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)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
  2019-11-13 20:14   ` Mimi Zohar
@ 2019-11-13 20:21     ` Lakshmi Ramasubramanian
  2019-11-13 20:24       ` Mimi Zohar
  0 siblings, 1 reply; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 20:21 UTC (permalink / raw)
  To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

On 11/13/2019 12:14 PM, Mimi Zohar wrote:

>> @@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
>>   	int action = 0;
>>   	u32 secid;
>>   
>> +	/*
>> +	 * If IMA is not yet initialized or IMA policy is empty
>> +	 * then there is no need to measure.
>> +	 */
>> +	if (!ima_policy_flag)
>> +		return;
>> +
> 
> This addition has nothing to do with defining a new IMA hook and
> should be a separate patch.  This can be posted independently of this
> patch set.
> 
> Mimi

I'll move this change to a different patch,
but it has to be either part of this patch set or the above change alone 
needs to be taken before this patch set for the following reason:

The IMA hook gets called early in the boot process (for example, when 
builtin_trusted_keys are added). If the above check is not there, 
ima_get_action() is called and causes kernel panic (since IMA is not yet 
initialized).

thanks,
  -lakshmi



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
  2019-11-13 20:21     ` Lakshmi Ramasubramanian
@ 2019-11-13 20:24       ` Mimi Zohar
  0 siblings, 0 replies; 14+ messages in thread
From: Mimi Zohar @ 2019-11-13 20:24 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-integrity, linux-security-module, keyrings,
	linux-kernel

On Wed, 2019-11-13 at 12:21 -0800, Lakshmi Ramasubramanian wrote:
> On 11/13/2019 12:14 PM, Mimi Zohar wrote:
> 
> >> @@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
> >>   	int action = 0;
> >>   	u32 secid;
> >>   
> >> +	/*
> >> +	 * If IMA is not yet initialized or IMA policy is empty
> >> +	 * then there is no need to measure.
> >> +	 */
> >> +	if (!ima_policy_flag)
> >> +		return;
> >> +
> > 
> > This addition has nothing to do with defining a new IMA hook and
> > should be a separate patch.  This can be posted independently of this
> > patch set.
> > 
> > Mimi
> 
> I'll move this change to a different patch,
> but it has to be either part of this patch set or the above change alone 
> needs to be taken before this patch set for the following reason:
> 
> The IMA hook gets called early in the boot process (for example, when 
> builtin_trusted_keys are added). If the above check is not there, 
> ima_get_action() is called and causes kernel panic (since IMA is not yet 
> initialized).

It will be upstreamed prior to this patch set.
 
Mimi


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
  2019-11-13 20:09   ` Mimi Zohar
@ 2019-11-13 20:52     ` Lakshmi Ramasubramanian
  2019-11-13 21:18       ` Mimi Zohar
  0 siblings, 1 reply; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 20:52 UTC (permalink / raw)
  To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

On 11/13/19 12:09 PM, Mimi Zohar wrote:
>
> All that is is needed is the key and public_key structures, which are
> defined in include/linux/keys.h and include/crypto/public_key.h.  If
> the keys subsystem is disabled, then the new IMA hook won't be called.
>   There's no need for a new Kconfig option or a new file.
> 
> Please move the hook to just after ima_kexec_cmdline().
> 
> Mimi

Yes - IMA hook won't be called when KEYS subsystem is disabled.

But, build dependency is breaking since "struct key" is not defined 
without CONFIG_KEYS.

Sasha was able to craft a .config that enabled IMA without enabling KEYS 
and found the build break.

Please see the build output he'd shared.

***********************************************************************

In file included from security/integrity/ima/ima.h:25,
                 from security/integrity/ima/ima_fs.c:26:
./include/keys/asymmetric-type.h: In function ‘asymmetric_key_ids’:
./include/keys/asymmetric-type.h:72:12: error: dereferencing pointer to 
incomplete type ‘const struct key’
  return key->payload.data[asym_key_ids];
            ^~
make[3]: *** [scripts/Makefile.build:266: 
security/integrity/ima/ima_fs.o] Error 1
make[3]: *** Waiting for unfinished jobs....
In file included from security/integrity/ima/ima.h:25,
                 from security/integrity/ima/ima_queue.c:22:
./include/keys/asymmetric-type.h: In function ‘asymmetric_key_ids’:
./include/keys/asymmetric-type.h:72:12: error: dereferencing pointer to 
incomplete type ‘const struct key’
  return key->payload.data[asym_key_ids];

***********************************************************************

thanks,
  -lakshmi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
  2019-11-13 20:52     ` Lakshmi Ramasubramanian
@ 2019-11-13 21:18       ` Mimi Zohar
  2019-11-13 22:01         ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 14+ messages in thread
From: Mimi Zohar @ 2019-11-13 21:18 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-integrity, linux-security-module, keyrings,
	linux-kernel

On Wed, 2019-11-13 at 12:52 -0800, Lakshmi Ramasubramanian wrote:
> On 11/13/19 12:09 PM, Mimi Zohar wrote:
> >
> > All that is is needed is the key and public_key structures, which are
> > defined in include/linux/keys.h and include/crypto/public_key.h.  If
> > the keys subsystem is disabled, then the new IMA hook won't be called.
> >   There's no need for a new Kconfig option or a new file.
> > 
> > Please move the hook to just after ima_kexec_cmdline().
> > 
> > Mimi
> 
> Yes - IMA hook won't be called when KEYS subsystem is disabled.
> 
> But, build dependency is breaking since "struct key" is not defined 
> without CONFIG_KEYS.
> 
> Sasha was able to craft a .config that enabled IMA without enabling KEYS 
> and found the build break.

Yes, thanks for pointing out the "#ifdef CONFIG_KEYS" in keys.h.  A
separate file is needed, as you pointed out, but still no need for a
new Kconfig.  The ima/Makefile can be based on CONFIG_KEYS.

Mimi


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
  2019-11-13 21:18       ` Mimi Zohar
@ 2019-11-13 22:01         ` Lakshmi Ramasubramanian
  0 siblings, 0 replies; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 22:01 UTC (permalink / raw)
  To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

On 11/13/19 1:18 PM, Mimi Zohar wrote:

> Yes, thanks for pointing out the "#ifdef CONFIG_KEYS" in keys.h.  A
> separate file is needed, as you pointed out, but still no need for a
> new Kconfig.  The ima/Makefile can be based on CONFIG_KEYS.
> 
> Mimi

Sure - i'll try it now and send an updated patch set.

thanks,
  -lakshmi


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/3] KEYS: Measure keys when they are created or updated
  2019-11-13 18:46 [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (2 preceding siblings ...)
  2019-11-13 18:46 ` [PATCH v6 3/3] KEYS: Call the " Lakshmi Ramasubramanian
@ 2019-11-13 22:02 ` Mimi Zohar
  2019-11-13 22:04   ` Lakshmi Ramasubramanian
  3 siblings, 1 reply; 14+ messages in thread
From: Mimi Zohar @ 2019-11-13 22:02 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-integrity, linux-security-module, keyrings,
	linux-kernel

On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian wrote:
> 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.
> 
>    # measure keys loaded onto any keyring
>    measure func=KEY_CHECK

When re-posting this patch set, please include the support for
specifying the "keyrings=" policy option, as an additional patch.

Mimi


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v6 0/3] KEYS: Measure keys when they are created or updated
  2019-11-13 22:02 ` [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Mimi Zohar
@ 2019-11-13 22:04   ` Lakshmi Ramasubramanian
  0 siblings, 0 replies; 14+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-13 22:04 UTC (permalink / raw)
  To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-integrity, linux-security-module, keyrings, linux-kernel

On 11/13/2019 2:02 PM, Mimi Zohar wrote:

> 
> When re-posting this patch set, please include the support for
> specifying the "keyrings=" policy option, as an additional patch.
> 
> Mimi

Sure - will do.

thanks,
  -lakshmi


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-13 18:46 [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
2019-11-13 18:46 ` [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
2019-11-13 20:14   ` Mimi Zohar
2019-11-13 20:21     ` Lakshmi Ramasubramanian
2019-11-13 20:24       ` Mimi Zohar
2019-11-13 18:46 ` [PATCH v6 2/3] IMA: Define an IMA hook " Lakshmi Ramasubramanian
2019-11-13 20:09   ` Mimi Zohar
2019-11-13 20:52     ` Lakshmi Ramasubramanian
2019-11-13 21:18       ` Mimi Zohar
2019-11-13 22:01         ` Lakshmi Ramasubramanian
2019-11-13 18:46 ` [PATCH v6 3/3] KEYS: Call the " Lakshmi Ramasubramanian
2019-11-13 20:09   ` Mimi Zohar
2019-11-13 22:02 ` [PATCH v6 0/3] KEYS: Measure keys when they are created or updated Mimi Zohar
2019-11-13 22:04   ` Lakshmi Ramasubramanian

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git