linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/5] KEYS: Measure keys when they are created or updated
@ 2019-11-18 22:38 Lakshmi Ramasubramanian
  2019-11-18 22:38 ` [PATCH v8 1/5] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-18 22:38 UTC (permalink / raw)
  To: zohar, linux-integrity; +Cc: linux-kernel, keyrings

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/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 (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


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

* [PATCH v8 1/5] IMA: Add KEY_CHECK func to measure keys
  2019-11-18 22:38 [PATCH v8 0/5] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
@ 2019-11-18 22:38 ` Lakshmi Ramasubramanian
  2019-11-18 22:38 ` [PATCH v8 2/5] IMA: Define an IMA hook " Lakshmi Ramasubramanian
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-18 22:38 UTC (permalink / raw)
  To: zohar, linux-integrity; +Cc: linux-kernel, keyrings

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>
Cc: Sasha Levin <sashal@kernel.org>
Cc: James Morris <jamorris@linux.microsoft.com>
---
 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


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

* [PATCH v8 2/5] IMA: Define an IMA hook to measure keys
  2019-11-18 22:38 [PATCH v8 0/5] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
  2019-11-18 22:38 ` [PATCH v8 1/5] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
@ 2019-11-18 22:38 ` Lakshmi Ramasubramanian
  2019-11-20 23:28   ` Eric Snowberg
  2019-11-18 22:38 ` [PATCH v8 3/5] KEYS: Call the " Lakshmi Ramasubramanian
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-18 22:38 UTC (permalink / raw)
  To: zohar, linux-integrity; +Cc: linux-kernel, keyrings

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 <nramas@linux.microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: James Morris <jamorris@linux.microsoft.com>
---
 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 (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 related	[flat|nested] 16+ messages in thread

* [PATCH v8 3/5] KEYS: Call the IMA hook to measure keys
  2019-11-18 22:38 [PATCH v8 0/5] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
  2019-11-18 22:38 ` [PATCH v8 1/5] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
  2019-11-18 22:38 ` [PATCH v8 2/5] IMA: Define an IMA hook " Lakshmi Ramasubramanian
@ 2019-11-18 22:38 ` Lakshmi Ramasubramanian
  2019-11-19  1:18   ` Eric Snowberg
  2019-11-18 22:38 ` [PATCH v8 4/5] IMA: Add support to limit measuring keys Lakshmi Ramasubramanian
  2019-11-18 22:38 ` [PATCH v8 5/5] IMA: Read keyrings= option from the IMA policy Lakshmi Ramasubramanian
  4 siblings, 1 reply; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-18 22:38 UTC (permalink / raw)
  To: zohar, linux-integrity; +Cc: linux-kernel, keyrings

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>
Cc: David Howells <dhowells@redhat.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: James Morris <jamorris@linux.microsoft.com>
---
 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


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

* [PATCH v8 4/5] IMA: Add support to limit measuring keys
  2019-11-18 22:38 [PATCH v8 0/5] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (2 preceding siblings ...)
  2019-11-18 22:38 ` [PATCH v8 3/5] KEYS: Call the " Lakshmi Ramasubramanian
@ 2019-11-18 22:38 ` Lakshmi Ramasubramanian
  2019-11-20 23:19   ` Mimi Zohar
  2019-11-18 22:38 ` [PATCH v8 5/5] IMA: Read keyrings= option from the IMA policy Lakshmi Ramasubramanian
  4 siblings, 1 reply; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-18 22:38 UTC (permalink / raw)
  To: zohar, linux-integrity; +Cc: linux-kernel, keyrings

Limit measuring keys to those keys being loaded onto a given set of
keyrings only.

This patch defines a new IMA policy option namely "keyrings=" that
can be used to specify a set of keyrings. If this option is specified
in the policy for "measure func=KEY_CHECK" then only the keys
loaded onto a keyring given in the "keyrings=" option are measured.

Added a new parameter namely "keyring" (name of the keyring) to
process_buffer_measurement(). The keyring name is passed to
ima_get_action() to determine the required action.
ima_match_rules() is updated to check keyring in the policy, if
specified, for KEY_CHECK function.

The following example illustrates how key measurement can be verified.

Sample IMA Policy entry to measure keys
(Added in the file /etc/ima/ima-policy):
measure func=KEY_CHECK keyrings=.ima|.evm|.blacklist template=ima-buf

Build the kernel with this patch set applied and reboot to that kernel.

Ensure the IMA policy is applied:

root@nramas:/home/nramas# cat /sys/kernel/security/ima/policy
measure func=KEY_CHECK keyrings=.ima|.evm|.blacklist template=ima-buf

View the initial IMA measurement log:

root@nramas:/home/nramas# cat /sys/kernel/security/ima/ascii_runtime_measurements
10 67ec... ima-ng sha1:b5466c508583f0e633df83aa58fc7c5b67ccf667 boot_aggregate

Now, add a certificate in DER format (for example, x509_ima.der) to
the .ima keyring:

root@nramas:/home/nramas# keyctl show %:.ima
Keyring
 547515640 ---lswrv      0     0  keyring: .ima

root@nramas:/home/nramas# evmctl import x509_ima.der 547515640

root@nramas:/home/nramas# keyctl show %:.ima
Keyring
 547515640 ---lswrv      0     0  keyring: .ima
 809678766 --als--v      0     0   \_ asymmetric: hostname: whoami signing key: 052dd247dc3c36...

View the updated IMA measurement log:

root@nramas:/home/nramas# cat /sys/kernel/security/ima/ascii_runtime_measurements
10 67ec... ima-ng sha1:b5466c508583f0e633df83aa58fc7c5b67ccf667 boot_aggregate
10 3adf... ima-buf sha256:27c915b8ddb9fae7214cf0a8a7043cc3eeeaa7539bcb136f8427067b5f6c3b7b .ima 30818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001
root@nramas:/home/nramas#

The public key of x509_ima.der certificate and the key's SHA-256 hash
are included in the IMA log.

For example, in the above IMA log entry the public key is the following:

30818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001

sha256:27c915b8ddb9fae7214cf0a8a7043cc3eeeaa7539bcb136f8427067b5f6c3b7b

root@nramas:/home/nramas# cat /sys/kernel/security/ima/ascii_runtime_measurements |
                          grep " .ima" | cut -d' ' -f 6 | xxd -r -p | sha256sum
27c915b8ddb9fae7214cf0a8a7043cc3eeeaa7539bcb136f8427067b5f6c3b7b
root@nramas:/home/nramas#

SHA-256 hash in the IMA log and the above output should match.

Now run the following "openssl" command to display
various fields of x509_ima.der certificate:

Verify the "Modulus" and the "Exponent" with that
in the public key data in the IMA log entry.
Note that the "Modulus" in the IMA log entry follows
the RSA Header (For example, 308189028181)
The "Exponent" is the last 3 hex numbers in the IMA log
(For example, 0x01 0x00 0x01)

root@nramas:/home/nramas# openssl x509 -in x509_ima.der -inform der -noout -text
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            5b:e0:23:4f:f3:ad:f0:50:34:9b:33:b8:94:65:a6:aa:b6:e3:39:f7
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: O = hostname, CN = whoami signing key, emailAddress = whoami@hostname
        Validity
            Not Before: Aug 22 02:29:02 2019 GMT
            Not After : Aug 21 02:29:02 2020 GMT
        Subject: O = hostname, CN = whoami signing key, emailAddress = whoami@hostname
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (1024 bit)
                Modulus:
                    00:ee:96:b2:64:07:2a:42:88:8f:78:a2:f9:b8:19:
                    84:67:a3:ad:97:d1:26:f3:d1:cc:1c:24:d2:3e:71:
                    85:cc:74:3b:04:d4:a5:42:54:ca:16:e1:e1:1e:d4:
                    45:0d:eb:98:b1:f7:bb:42:88:42:45:70:fa:bc:fc:
                    6d:5a:a9:3a:2a:14:fa:2b:78:35:ac:87:7c:fe:a7:
                    61:e5:ff:41:4c:6e:e2:74:ef:f2:6f:8b:d6:c4:84:
                    31:2e:56:61:92:99:ac:f0:db:d2:24:b8:7c:38:83:
                    b6:6a:93:93:d2:1a:f8:96:24:58:66:3b:0a:c1:70:
                    6c:63:77:3c:d5:0e:82:36:27
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Basic Constraints: critical
                CA:FALSE
            X509v3 Key Usage: 
                Digital Signature
            X509v3 Subject Key Identifier: 
                05:2D:D2:47:DC:3C:36:D6:D6:06:75:FE:7A:E8:69:79:0B:E5:61:71
            X509v3 Authority Key Identifier: 
                keyid:E3:67:10:F0:83:4C:97:3E:D9:4A:18:6F:BC:D2:23:75:B4:5E:24:54

    Signature Algorithm: sha256WithRSAEncryption
         b1:2f:ae:ff:1e:0e:39:0c:fd:5e:b7:14:0a:f3:b7:a6:53:cb:
         49:c6:ab:0a:23:be:24:c0:35:33:1d:76:00:c8:f7:58:f9:df:
         7f:df:c5:ee:b6:fe:c3:58:59:20:3e:ca:0e:4f:01:f9:a7:9a:
         58:be:63:09:47:cb:95:9a:52:d3:f2:de:96:f2:10:d4:92:47:
         c3:3a:62:26:dc:2a:52:ee:54:10:69:ed:3c:62:1f:87:67:fd:
         36:a0:61:e9:a6:1a:db:5d:1d:d3:44:99:d9:9a:1c:e6:ba:a4:
         96:b4:f5:e2:26:8b:fc:52:c3:ee:a4:a6:b7:b5:18:1f:08:52:
         4a:ee
root@nramas:/home/nramas#

An ima-sig entry for a kernel module, say, kheaders.ko
from the IMA log entry is given below:

10 0c98... ima-sig
sha256:3bc6ed4f0b4d6e31bc1dbc9ef844605abc7afdc6d81a57d77a1ec9407997c40
2 /usr/lib/modules/5.4.0-rc3+/kernel/kernel/kheaders.ko
03020BE561710100abcde...

In the above 0BE56171 is the Key ID of the key used to verify
the IMA signature. This Key ID is the last 4 hex digits of
the subject key identifier displayed in openssl output
for the certificate x509_ima.der (Which is the IMA certificate
used to sign the kernel module).

X509v3 Subject Key Identifier:
   05:2D:D2:47:DC:3C:36:D6:D6:06:75:FE:7A:E8:69:79:0B:E5:61:71

The ima-modsig entry for the same kernel module is:

10 82aa... ima-modsig
sha256:3bc6ed4f0b4d6e31bc1dbc9ef844605abc7afdc6d81a57d77a1ec9407997c40
2 /usr/lib/modules/5.4.0rc3+/kernel/kernel/kheaders.ko  
sha256:77fa889b35a05338ec52e51591c1b89d4c8d1c99a21251d7c22b1a8642a6bad3
30818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001

If the kernel module was signed by x509_ima.der certificate then
the public key entry in the ima-modsig should match the public key
for the key measurement for x509_ima.der.

The above can be used to correlate the key measurement IMA entry,
ima-sig and ima-modsig entries using the same key.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: James Morris <jamorris@linux.microsoft.com>
---
 Documentation/ABI/testing/ima_policy         | 10 ++-
 security/integrity/ima/ima.h                 |  8 ++-
 security/integrity/ima/ima_api.c             |  8 ++-
 security/integrity/ima/ima_appraise.c        |  4 +-
 security/integrity/ima/ima_asymmetric_keys.c |  8 ++-
 security/integrity/ima/ima_main.c            |  9 +--
 security/integrity/ima/ima_policy.c          | 67 ++++++++++++++++++--
 7 files changed, 96 insertions(+), 18 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 3823c27894c5..5a941ed20fa3 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,7 +25,7 @@ Description:
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [template=] [permit_directio]
-				[appraise_flag=]
+				[appraise_flag=] [keyrings=]
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
@@ -42,6 +42,9 @@ Description:
 			appraise_flag:= [check_blacklist]
 			Currently, blacklist check is only for files signed with appended
 			signature.
+			keyrings:= list of keyrings
+			(eg, .builtin_trusted_keys|.ima). Only valid
+			when action is "measure" and func is KEY_CHECK.
 			template:= name of a defined IMA template type
 			(eg, ima-ng). Only valid when action is "measure".
 			pcr:= decimal value
@@ -118,3 +121,8 @@ Description:
 
 			measure func=KEY_CHECK
 
+		Example of measure rule using KEY_CHECK to only measure
+		keys added to .builtin_trusted_keys or .ima keyring:
+
+			measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
+
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fe6c698617bd..f06238e41a7c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -208,7 +208,8 @@ struct modsig;
 /* 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,
-		   struct ima_template_desc **template_desc);
+		   struct ima_template_desc **template_desc,
+		   const char *keyring);
 int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
 			    struct file *file, void *buf, loff_t size,
@@ -220,7 +221,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   struct ima_template_desc *template_desc);
 void process_buffer_measurement(const void *buf, int size,
 				const char *eventname, enum ima_hooks func,
-				int pcr);
+				int pcr, const char *keyring);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename);
 int ima_alloc_init_template(struct ima_event_data *event_data,
@@ -235,7 +236,8 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
 /* IMA policy related functions */
 int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 		     enum ima_hooks func, int mask, int flags, int *pcr,
-		     struct ima_template_desc **template_desc);
+		     struct ima_template_desc **template_desc,
+		     const char *keyring);
 void ima_init_policy(void);
 void ima_update_policy(void);
 void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 610759fe63b8..f6bc00914aa5 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -169,12 +169,13 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  * @func: caller identifier
  * @pcr: pointer filled in if matched measure policy sets pcr=
  * @template_desc: pointer filled in if matched measure policy sets template=
+ * @keyring: keyring name used to determine the action
  *
  * The policy is defined in terms of keypairs:
  *		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
+ *	| KEXEC_CMDLINE | KEY_CHECK
  *	mask: contains the permission mask
  *	fsmagic: hex value
  *
@@ -183,14 +184,15 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  */
 int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
 		   int mask, enum ima_hooks func, int *pcr,
-		   struct ima_template_desc **template_desc)
+		   struct ima_template_desc **template_desc,
+		   const char *keyring)
 {
 	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
 
 	flags &= ima_policy_flag;
 
 	return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,
-				template_desc);
+				template_desc, keyring);
 }
 
 /*
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 300c8d2943c5..a9649b04b9f1 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -55,7 +55,7 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
 
 	security_task_getsecid(current, &secid);
 	return ima_match_policy(inode, current_cred(), secid, func, mask,
-				IMA_APPRAISE | IMA_HASH, NULL, NULL);
+				IMA_APPRAISE | IMA_HASH, NULL, NULL, NULL);
 }
 
 static int ima_fix_xattr(struct dentry *dentry,
@@ -330,7 +330,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint,
 		if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
 			process_buffer_measurement(digest, digestsize,
 						   "blacklisted-hash", NONE,
-						   pcr);
+						   pcr, NULL);
 	}
 
 	return rc;
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
index f6884641a622..8c692eb08a0a 100644
--- a/security/integrity/ima/ima_asymmetric_keys.c
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -45,7 +45,13 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
 	 * parameter to process_buffer_measurement() and is set
 	 * in the "eventname" field in ima_event_data for
 	 * the key measurement IMA event.
+	 *
+	 * The name of the keyring is also passed in the "keyring"
+	 * parameter to process_buffer_measurement() to check
+	 * if the IMA policy is configured to measure a key linked
+	 * to the given keyring.
 	 */
 	process_buffer_measurement(pk->key, pk->keylen,
-				   keyring->description, KEY_CHECK, 0);
+				   keyring->description, KEY_CHECK, 0,
+				   keyring->description);
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d7e987baf127..6d0bf241ebf8 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -215,7 +215,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	 * Included is the appraise submask.
 	 */
 	action = ima_get_action(inode, cred, secid, mask, func, &pcr,
-				&template_desc);
+				&template_desc, NULL);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
 			   (ima_policy_flag & IMA_MEASURE));
 	if (!action && !violation_check)
@@ -632,12 +632,13 @@ int ima_load_data(enum kernel_load_data_id id)
  * @eventname: event name to be used for the buffer entry.
  * @func: IMA hook
  * @pcr: pcr to extend the measurement
+ * @keyring: keyring name to determine the action to be performed
  *
  * Based on policy, the buffer is measured into the ima log.
  */
 void process_buffer_measurement(const void *buf, int size,
 				const char *eventname, enum ima_hooks func,
-				int pcr)
+				int pcr, const char *keyring)
 {
 	int ret = 0;
 	struct ima_template_entry *entry = NULL;
@@ -665,7 +666,7 @@ void process_buffer_measurement(const void *buf, int size,
 	if (func) {
 		security_task_getsecid(current, &secid);
 		action = ima_get_action(NULL, current_cred(), secid, 0, func,
-					&pcr, &template);
+					&pcr, &template, keyring);
 		if (!(action & IMA_MEASURE))
 			return;
 	}
@@ -718,7 +719,7 @@ void ima_kexec_cmdline(const void *buf, int size)
 {
 	if (buf && size != 0)
 		process_buffer_measurement(buf, size, "kexec-cmdline",
-					   KEXEC_CMDLINE, 0);
+					   KEXEC_CMDLINE, 0, NULL);
 }
 
 static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 1525a28fd705..d9400585fcda 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -79,6 +79,7 @@ struct ima_rule_entry {
 		int type;	/* audit type */
 	} lsm[MAX_LSM_RULES];
 	char *fsname;
+	char *keyrings; /* Measure keys added to these keyrings */
 	struct ima_template_desc *template;
 };
 
@@ -356,6 +357,55 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
 	return NOTIFY_OK;
 }
 
+/**
+ * ima_match_keyring - determine whether the keyring matches the measure rule
+ * @rule: a pointer to a rule
+ * @keyring: name of the keyring to match against the measure rule
+ *
+ * If the measure action for KEY_CHECK does not specify keyrings=
+ * option then return true (Measure all keys).
+ * Else, return true if the given keyring name is present in
+ * the keyrings= option. False, otherwise.
+ */
+static bool ima_match_keyring(struct ima_rule_entry *rule,
+			      const char *keyring)
+{
+	const char *p;
+
+	/* If "keyrings=" is not specified all keys are measured. */
+	if (!rule->keyrings)
+		return true;
+
+	if (!keyring)
+		return false;
+
+	/*
+	 * "keyrings=" is specified in the policy in the format below:
+	 *   keyrings=.builtin_trusted_keys|.ima|.evm
+	 *
+	 * Each keyring name in the option is separated by a '|' and
+	 * the last keyring name is null terminated.
+	 *
+	 * The given keyring is considered matched only if
+	 * the whole keyring name matched a keyring name specified
+	 * in the "keyrings=" option.
+	 */
+	p = strstr(rule->keyrings, keyring);
+	if (p) {
+		/*
+		 * Found a substring match. Check if the character
+		 * at the end of the keyring name is | (keyring name
+		 * separator) or is the terminating null character.
+		 * If yes, we have a whole string match.
+		 */
+		p += strlen(keyring);
+		if (*p == '|' || *p == '\0')
+			return true;
+	}
+
+	return false;
+}
+
 /**
  * ima_match_rules - determine whether an inode matches the measure rule.
  * @rule: a pointer to a rule
@@ -364,18 +414,23 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
  * @secid: the secid of the task to be validated
  * @func: LIM hook identifier
  * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
+ * @keyring: keyring name to check in policy for KEY_CHECK func
  *
  * Returns true on rule match, false on failure.
  */
 static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 			    const struct cred *cred, u32 secid,
-			    enum ima_hooks func, int mask)
+			    enum ima_hooks func, int mask,
+			    const char *keyring)
 {
 	int i;
 
 	if ((func == KEXEC_CMDLINE) || (func == KEY_CHECK)) {
-		if ((rule->flags & IMA_FUNC) && (rule->func == func))
+		if ((rule->flags & IMA_FUNC) && (rule->func == func)) {
+			if (func == KEY_CHECK)
+				return ima_match_keyring(rule, keyring);
 			return true;
+		}
 		return false;
 	}
 	if ((rule->flags & IMA_FUNC) &&
@@ -479,6 +534,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
  * @pcr: set the pcr to extend
  * @template_desc: the template that should be used for this rule
+ * @keyring: the keyring name, if given, to be used to check in the policy.
+ *           keyring can be NULL if func is anything other than KEY_CHECK.
  *
  * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
  * conditions.
@@ -489,7 +546,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  */
 int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 		     enum ima_hooks func, int mask, int flags, int *pcr,
-		     struct ima_template_desc **template_desc)
+		     struct ima_template_desc **template_desc,
+		     const char *keyring)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
@@ -503,7 +561,8 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 		if (!(entry->action & actmask))
 			continue;
 
-		if (!ima_match_rules(entry, inode, cred, secid, func, mask))
+		if (!ima_match_rules(entry, inode, cred, secid, func, mask,
+				     keyring))
 			continue;
 
 		action |= entry->flags & IMA_ACTION_FLAGS;
-- 
2.17.1


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

* [PATCH v8 5/5] IMA: Read keyrings= option from the IMA policy
  2019-11-18 22:38 [PATCH v8 0/5] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (3 preceding siblings ...)
  2019-11-18 22:38 ` [PATCH v8 4/5] IMA: Add support to limit measuring keys Lakshmi Ramasubramanian
@ 2019-11-18 22:38 ` Lakshmi Ramasubramanian
  4 siblings, 0 replies; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-18 22:38 UTC (permalink / raw)
  To: zohar, linux-integrity; +Cc: linux-kernel, keyrings

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 <nramas@linux.microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: James Morris <jamorris@linux.microsoft.com>
---
 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


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

* Re: [PATCH v8 3/5] KEYS: Call the IMA hook to measure keys
  2019-11-18 22:38 ` [PATCH v8 3/5] KEYS: Call the " Lakshmi Ramasubramanian
@ 2019-11-19  1:18   ` Eric Snowberg
  2019-11-19  1:58     ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Snowberg @ 2019-11-19  1:18 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian; +Cc: zohar, linux-integrity, linux-kernel, keyrings


> On Nov 18, 2019, at 3:38 PM, Lakshmi Ramasubramanian <nramas@linux.microsoft.com> 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>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Sasha Levin <sashal@kernel.org>
> Cc: James Morris <jamorris@linux.microsoft.com>
> ---
> 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
> 


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

* Re: [PATCH v8 3/5] KEYS: Call the IMA hook to measure keys
  2019-11-19  1:18   ` Eric Snowberg
@ 2019-11-19  1:58     ` Lakshmi Ramasubramanian
  0 siblings, 0 replies; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-19  1:58 UTC (permalink / raw)
  To: Eric Snowberg; +Cc: zohar, linux-integrity, linux-kernel, keyrings

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


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

* Re: [PATCH v8 4/5] IMA: Add support to limit measuring keys
  2019-11-18 22:38 ` [PATCH v8 4/5] IMA: Add support to limit measuring keys Lakshmi Ramasubramanian
@ 2019-11-20 23:19   ` Mimi Zohar
  2019-11-21  0:03     ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 16+ messages in thread
From: Mimi Zohar @ 2019-11-20 23:19 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, linux-integrity; +Cc: linux-kernel, keyrings

On Mon, 2019-11-18 at 14:38 -0800, Lakshmi Ramasubramanian wrote:
> Limit measuring keys to those keys being loaded onto a given set of
> keyrings only.
> 
> This patch defines a new IMA policy option namely "keyrings=" that
> can be used to specify a set of keyrings. If this option is specified
> in the policy for "measure func=KEY_CHECK" then only the keys
> loaded onto a keyring given in the "keyrings=" option are measured.
> 
> Added a new parameter namely "keyring" (name of the keyring) to
> process_buffer_measurement(). The keyring name is passed to
> ima_get_action() to determine the required action.
> ima_match_rules() is updated to check keyring in the policy, if
> specified, for KEY_CHECK function.
> 
> The following example illustrates how key measurement can be verified.
> 
> Sample IMA Policy entry to measure keys
> (Added in the file /etc/ima/ima-policy):
> measure func=KEY_CHECK keyrings=.ima|.evm|.blacklist template=ima-buf
> 
> Build the kernel with this patch set applied and reboot to that kernel.
> 
> Ensure the IMA policy is applied:
> 
> root@nramas:/home/nramas# cat /sys/kernel/security/ima/policy
> measure func=KEY_CHECK keyrings=.ima|.evm|.blacklist template=ima-buf
> 
> View the initial IMA measurement log:
> 
> root@nramas:/home/nramas# cat /sys/kernel/security/ima/ascii_runtime_measurements
> 10 67ec... ima-ng sha1:b5466c508583f0e633df83aa58fc7c5b67ccf667 boot_aggregate
> 
> Now, add a certificate in DER format (for example, x509_ima.der) to
> the .ima keyring:
> 
> root@nramas:/home/nramas# keyctl show %:.ima
> Keyring
>  547515640 ---lswrv      0     0  keyring: .ima
> 
> root@nramas:/home/nramas# evmctl import x509_ima.der 547515640
> 
> root@nramas:/home/nramas# keyctl show %:.ima
> Keyring
>  547515640 ---lswrv      0     0  keyring: .ima
>  809678766 --als--v      0     0   \_ asymmetric: hostname: whoami signing key: 052dd247dc3c36...
> 
> View the updated IMA measurement log:
> 
> root@nramas:/home/nramas# cat /sys/kernel/security/ima/ascii_runtime_measurements
> 10 67ec... ima-ng sha1:b5466c508583f0e633df83aa58fc7c5b67ccf667 boot_aggregate
> 10 3adf... ima-buf sha256:27c915b8ddb9fae7214cf0a8a7043cc3eeeaa7539bcb136f8427067b5f6c3b7b .ima 30818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001
> root@nramas:/home/nramas#
> 
> The public key of x509_ima.der certificate and the key's SHA-256 hash
> are included in the IMA log.
> 
> For example, in the above IMA log entry the public key is the following:
> 
> 30818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001
> 
> sha256:27c915b8ddb9fae7214cf0a8a7043cc3eeeaa7539bcb136f8427067b5f6c3b7b
> 
> root@nramas:/home/nramas# cat /sys/kernel/security/ima/ascii_runtime_measurements |
>                           grep " .ima" | cut -d' ' -f 6 | xxd -r -p | sha256sum
> 27c915b8ddb9fae7214cf0a8a7043cc3eeeaa7539bcb136f8427067b5f6c3b7b
> root@nramas:/home/nramas#
> 
> SHA-256 hash in the IMA log and the above output should match.
> 
> Now run the following "openssl" command to display
> various fields of x509_ima.der certificate:
> 
> Verify the "Modulus" and the "Exponent" with that
> in the public key data in the IMA log entry.
> Note that the "Modulus" in the IMA log entry follows
> the RSA Header (For example, 308189028181)
> The "Exponent" is the last 3 hex numbers in the IMA log
> (For example, 0x01 0x00 0x01)
> 
> root@nramas:/home/nramas# openssl x509 -in x509_ima.der -inform der -noout -text
> Certificate:
>     Data:
>         Version: 3 (0x2)
>         Serial Number:
>             5b:e0:23:4f:f3:ad:f0:50:34:9b:33:b8:94:65:a6:aa:b6:e3:39:f7
>         Signature Algorithm: sha256WithRSAEncryption
>         Issuer: O = hostname, CN = whoami signing key, emailAddress = whoami@hostname
>         Validity
>             Not Before: Aug 22 02:29:02 2019 GMT
>             Not After : Aug 21 02:29:02 2020 GMT
>         Subject: O = hostname, CN = whoami signing key, emailAddress = whoami@hostname
>         Subject Public Key Info:
>             Public Key Algorithm: rsaEncryption
>                 RSA Public-Key: (1024 bit)
>                 Modulus:
>                     00:ee:96:b2:64:07:2a:42:88:8f:78:a2:f9:b8:19:
>                     84:67:a3:ad:97:d1:26:f3:d1:cc:1c:24:d2:3e:71:
>                     85:cc:74:3b:04:d4:a5:42:54:ca:16:e1:e1:1e:d4:
>                     45:0d:eb:98:b1:f7:bb:42:88:42:45:70:fa:bc:fc:
>                     6d:5a:a9:3a:2a:14:fa:2b:78:35:ac:87:7c:fe:a7:
>                     61:e5:ff:41:4c:6e:e2:74:ef:f2:6f:8b:d6:c4:84:
>                     31:2e:56:61:92:99:ac:f0:db:d2:24:b8:7c:38:83:
>                     b6:6a:93:93:d2:1a:f8:96:24:58:66:3b:0a:c1:70:
>                     6c:63:77:3c:d5:0e:82:36:27
>                 Exponent: 65537 (0x10001)
>         X509v3 extensions:
>             X509v3 Basic Constraints: critical
>                 CA:FALSE
>             X509v3 Key Usage: 
>                 Digital Signature
>             X509v3 Subject Key Identifier: 
>                 05:2D:D2:47:DC:3C:36:D6:D6:06:75:FE:7A:E8:69:79:0B:E5:61:71
>             X509v3 Authority Key Identifier: 
>                 keyid:E3:67:10:F0:83:4C:97:3E:D9:4A:18:6F:BC:D2:23:75:B4:5E:24:54
> 
>     Signature Algorithm: sha256WithRSAEncryption
>          b1:2f:ae:ff:1e:0e:39:0c:fd:5e:b7:14:0a:f3:b7:a6:53:cb:
>          49:c6:ab:0a:23:be:24:c0:35:33:1d:76:00:c8:f7:58:f9:df:
>          7f:df:c5:ee:b6:fe:c3:58:59:20:3e:ca:0e:4f:01:f9:a7:9a:
>          58:be:63:09:47:cb:95:9a:52:d3:f2:de:96:f2:10:d4:92:47:
>          c3:3a:62:26:dc:2a:52:ee:54:10:69:ed:3c:62:1f:87:67:fd:
>          36:a0:61:e9:a6:1a:db:5d:1d:d3:44:99:d9:9a:1c:e6:ba:a4:
>          96:b4:f5:e2:26:8b:fc:52:c3:ee:a4:a6:b7:b5:18:1f:08:52:
>          4a:ee
> root@nramas:/home/nramas#
> 
> An ima-sig entry for a kernel module, say, kheaders.ko
> from the IMA log entry is given below:
> 
> 10 0c98... ima-sig
> sha256:3bc6ed4f0b4d6e31bc1dbc9ef844605abc7afdc6d81a57d77a1ec9407997c40
> 2 /usr/lib/modules/5.4.0-rc3+/kernel/kernel/kheaders.ko
> 03020BE561710100abcde...
> 
> In the above 0BE56171 is the Key ID of the key used to verify
> the IMA signature. This Key ID is the last 4 hex digits of
> the subject key identifier displayed in openssl output
> for the certificate x509_ima.der (Which is the IMA certificate
> used to sign the kernel module).
> 
> X509v3 Subject Key Identifier:
>    05:2D:D2:47:DC:3C:36:D6:D6:06:75:FE:7A:E8:69:79:0B:E5:61:71
> 
> The ima-modsig entry for the same kernel module is:
> 
> 10 82aa... ima-modsig
> sha256:3bc6ed4f0b4d6e31bc1dbc9ef844605abc7afdc6d81a57d77a1ec9407997c40
> 2 /usr/lib/modules/5.4.0rc3+/kernel/kernel/kheaders.ko  
> sha256:77fa889b35a05338ec52e51591c1b89d4c8d1c99a21251d7c22b1a8642a6bad3
> 30818902818100ee96b264072a42888f78a2f9b8198467a3ad97d126f3d1cc1c24d23e7185cc743b04d4a54254ca16e1e11ed4450deb98b1f7bb4288424570fabcfc6d5aa93a2a14fa2b7835ac877cfea761e5ff414c6ee274eff26f8bd6c484312e56619299acf0dbd224b87c3883b66a9393d21af8962458663b0ac1706c63773cd50e8236270203010001
> 
> If the kernel module was signed by x509_ima.der certificate then
> the public key entry in the ima-modsig should match the public key
> for the key measurement for x509_ima.der.
> 
> The above can be used to correlate the key measurement IMA entry,
> ima-sig and ima-modsig entries using the same key.

True, but associating the public key measurement with the file
signature requires information from the certificate (e.g. issuer,
serial number, and/or subject, subject keyid).

For a regression test, it would be nice if the key measurement,
itself, contained everything needed in order to validate the file
signatures in the measurement list.

Mimi


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

* Re: [PATCH v8 2/5] IMA: Define an IMA hook to measure keys
  2019-11-18 22:38 ` [PATCH v8 2/5] IMA: Define an IMA hook " Lakshmi Ramasubramanian
@ 2019-11-20 23:28   ` Eric Snowberg
  2019-11-20 23:40     ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Snowberg @ 2019-11-20 23:28 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian; +Cc: zohar, linux-integrity, linux-kernel, keyrings


> On Nov 18, 2019, at 3:38 PM, Lakshmi Ramasubramanian <nramas@linux.microsoft.com> 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 <nramas@linux.microsoft.com>
> Cc: Sasha Levin <sashal@kernel.org>
> Cc: James Morris <jamorris@linux.microsoft.com>
> ---
> 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 (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);

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.




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

* Re: [PATCH v8 2/5] IMA: Define an IMA hook to measure keys
  2019-11-20 23:28   ` Eric Snowberg
@ 2019-11-20 23:40     ` Lakshmi Ramasubramanian
  2019-11-21  1:22       ` Mimi Zohar
  0 siblings, 1 reply; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-20 23:40 UTC (permalink / raw)
  To: Eric Snowberg; +Cc: zohar, linux-integrity, linux-kernel, keyrings

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

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

* Re: [PATCH v8 4/5] IMA: Add support to limit measuring keys
  2019-11-20 23:19   ` Mimi Zohar
@ 2019-11-21  0:03     ` Lakshmi Ramasubramanian
  2019-11-21  0:53       ` Mimi Zohar
  0 siblings, 1 reply; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-21  0:03 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity; +Cc: linux-kernel, keyrings

On 11/20/2019 3:19 PM, Mimi Zohar wrote:

Hi Mimi,

>> The above can be used to correlate the key measurement IMA entry,
>> ima-sig and ima-modsig entries using the same key.
> 
> True, but associating the public key measurement with the file
> signature requires information from the certificate (e.g. issuer,
> serial number, and/or subject, subject keyid).
> 
> For a regression test, it would be nice if the key measurement,
> itself, contained everything needed in order to validate the file
> signatures in the measurement list.

I am just trying to understand your asks - Please clarify:

1, My change includes only the public key and not the entire certificate 
information in the measured buffer.

Should I update this current patch set to measure the entire cert. Or, 
can that be done as a separate patch set?

2, Should a regression test be part of this patch set for the key 
measurement changes to be accepted?

thanks,
  -lakshmi

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

* Re: [PATCH v8 4/5] IMA: Add support to limit measuring keys
  2019-11-21  0:03     ` Lakshmi Ramasubramanian
@ 2019-11-21  0:53       ` Mimi Zohar
  0 siblings, 0 replies; 16+ messages in thread
From: Mimi Zohar @ 2019-11-21  0:53 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, linux-integrity; +Cc: linux-kernel, keyrings

On Wed, 2019-11-20 at 16:03 -0800, Lakshmi Ramasubramanian wrote:
> On 11/20/2019 3:19 PM, Mimi Zohar wrote:
> 
> Hi Mimi,
> 
> >> The above can be used to correlate the key measurement IMA entry,
> >> ima-sig and ima-modsig entries using the same key.
> > 
> > True, but associating the public key measurement with the file
> > signature requires information from the certificate (e.g. issuer,
> > serial number, and/or subject, subject keyid).
> > 
> > For a regression test, it would be nice if the key measurement,
> > itself, contained everything needed in order to validate the file
> > signatures in the measurement list.
> 
> I am just trying to understand your asks - Please clarify:
> 
> 1, My change includes only the public key and not the entire certificate 
> information in the measured buffer.
> 
> Should I update this current patch set to measure the entire cert. Or, 
> can that be done as a separate patch set?
> 
> 2, Should a regression test be part of this patch set for the key 
> measurement changes to be accepted?

Once the key measurement is defined and upstreamed, changing it would
result in a regression.  If we think that it would change multiple
times, then perhaps the buffer measurement needs to contain some sort
of versioning.

I would very much like for a regression test to be included in this
patch set, but it isn't a requirement, as long as everything needed
for verifying a signature is included in the key measurement.

Mimi


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

* Re: [PATCH v8 2/5] IMA: Define an IMA hook to measure keys
  2019-11-20 23:40     ` Lakshmi Ramasubramanian
@ 2019-11-21  1:22       ` Mimi Zohar
  2019-11-21  1:32         ` Lakshmi Ramasubramanian
  2019-11-21 17:16         ` Lakshmi Ramasubramanian
  0 siblings, 2 replies; 16+ messages in thread
From: Mimi Zohar @ 2019-11-21  1:22 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, Eric Snowberg
  Cc: linux-integrity, linux-kernel, keyrings

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


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

* Re: [PATCH v8 2/5] IMA: Define an IMA hook to measure keys
  2019-11-21  1:22       ` Mimi Zohar
@ 2019-11-21  1:32         ` Lakshmi Ramasubramanian
  2019-11-21 17:16         ` Lakshmi Ramasubramanian
  1 sibling, 0 replies; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-21  1:32 UTC (permalink / raw)
  To: Mimi Zohar, Eric Snowberg; +Cc: linux-integrity, linux-kernel, keyrings

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

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

* Re: [PATCH v8 2/5] IMA: Define an IMA hook to measure keys
  2019-11-21  1:22       ` Mimi Zohar
  2019-11-21  1:32         ` Lakshmi Ramasubramanian
@ 2019-11-21 17:16         ` Lakshmi Ramasubramanian
  1 sibling, 0 replies; 16+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-11-21 17:16 UTC (permalink / raw)
  To: Mimi Zohar, Eric Snowberg; +Cc: linux-integrity, linux-kernel, keyrings

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

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

end of thread, other threads:[~2019-11-21 17:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-18 22:38 [PATCH v8 0/5] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
2019-11-18 22:38 ` [PATCH v8 1/5] IMA: Add KEY_CHECK func to measure keys Lakshmi Ramasubramanian
2019-11-18 22:38 ` [PATCH v8 2/5] IMA: Define an IMA hook " Lakshmi Ramasubramanian
2019-11-20 23:28   ` Eric Snowberg
2019-11-20 23:40     ` Lakshmi Ramasubramanian
2019-11-21  1:22       ` Mimi Zohar
2019-11-21  1:32         ` Lakshmi Ramasubramanian
2019-11-21 17:16         ` Lakshmi Ramasubramanian
2019-11-18 22:38 ` [PATCH v8 3/5] KEYS: Call the " Lakshmi Ramasubramanian
2019-11-19  1:18   ` Eric Snowberg
2019-11-19  1:58     ` Lakshmi Ramasubramanian
2019-11-18 22:38 ` [PATCH v8 4/5] IMA: Add support to limit measuring keys Lakshmi Ramasubramanian
2019-11-20 23:19   ` Mimi Zohar
2019-11-21  0:03     ` Lakshmi Ramasubramanian
2019-11-21  0:53       ` Mimi Zohar
2019-11-18 22:38 ` [PATCH v8 5/5] IMA: Read keyrings= option from the IMA policy Lakshmi Ramasubramanian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).