Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/9] KEYS: Measure keys when they are created or updated
@ 2019-10-31  1:19 Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update Lakshmi Ramasubramanian
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

Problem Statement:

Keys created or updated in the system are currently not being 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.

   In this change set a new IMA policy function KEYRING_CHECK has been
   added to measure keys. The policy can optionally specify a set of
   keyrings to measure. By default all keyrings are included in
   the measurement when KEYRING_CHECK policy is specified.

   # measure all keys
   measure func=KEYRING_CHECK

   # measure keys on the IMA keyring
   measure func=KEYRING_CHECK keyring=".ima"

   # measure keys on the BUILTIN and IMA keyrings into a different PCR
   measure func=KEYRING_CHECK keyring=".builtin_trusted_keys|.ima" pcr=11

Testing performed:

  * Booted the kernel with this change.
  * Executed keyctl tests from the Linux Test Project (LTP)
  * All keys are measured when only KEYRING_CHECK is set.
  * Only keys added to the given keyrings are measured
    when keyrings option is set.
  * Keys are not measured when KEYRING_CHECK is not set.
  * Key is queued for measurement if IMA is not yet initialized
    and processed when IMA is initialized.
  * Key is measured rightaway when IMA is initialized.
  * Added a new key to a keyring and verified "key create" code path.
    => In this case added a key to .ima keyring.
  * Added the same key again and verified "key update" code path.
    => Add the same key to .ima keyring.

Change Log:

  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.

Background:

Currently ima measures file hashes and .ima signatures. ima signatures
are validated against keys in the ".ima" keyring. If the kernel is built
with CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY enabled,
then all keys in ".ima" keyring must be signed by a key in
".builtin_trusted_keys" or ".secondary_trusted_keys" keyrings.

Although ima supports the above configuration, not having an insight
into what keys are present in these trusted keys keyrings would prevent
an attestation service from validating a client machine.
 
On systems with CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
enabled, measuring keys in the  ".builtin_trusted_keys" keyring provides
a mechanism to attest that the client's system binaries are indeed signed
by signers that chain to known trusted keys.

Without this change, to attest the clients one needs to maintain
an "allowed list" of file hashes of all versions of all client binaries
that are deployed on the clients in the enterprise. That is a huge
operational challenge in a large scale environment of clients with
heterogenous builds. This also limits scalability and agility of
rolling out frequent client binary updates.

Questions and concerns raised by reviewers on this patch set:

Question 1:
Is "Signed with a trusted key" equal to "Trusted file"?
Doesn't the service need the hashes of the system files to determine
whether a file is trusted or not?
"Signed with a trusted key" does not equal "Trusted"

Answer:
Agree "Signed with a trusted key" may not equal "Trusted".
To address this, the attesting service can maintain a small
manageable set of bad hashes (a "Blocked list") and a list of
trusted keys expected in client's .builtin_trusted_keys" keyring.
Using this data, the service can detect the presence of
"Disallowed (untrusted) version of client binaries".

Question 2:
Providing more data to the service (such as ".builtin_trusted_keys"),
empowers the service  to deny access to clients (block clients).
IMA walks a fine line in enforcing and measuring file integrity.
This patchset breaches that fine line and in doing so brings back
the fears of trusted computing.

Answer:
Any new measurement we add in IMA will provide more data to service
and can enable it to deny access to clients. It is not clear why this patch
set would breach the fine line between measuring and enforcing.
Since this patch set is disabled by default and enabled through
CONFIG_IMA_MEASURE_TRUSTED_KEYS, only those enterprises that
require this new measurement can opt-in for it. Since it is disabled
by default, it does not restrict the autonomy of independent users
who are unaffected by attestation.

Question 3:
IMA log already contains a pointer to the IMA keys used for signature
verification. Why does the service need to care what keys were used
to sign (install) the IMA keys? What is gained by measuring the keys
in the ".builtin_trusted_keys"


Answer:
To attest the clients using the current IMA log, service needs to maintain
hashes of all the deployed versions of all the system binaries for their
enterprise. This will introduce a very high operational overhead in
a large scale environment of clients with heterogenous builds.
This limits scalability and agility of rolling out frequent client
binary updates.


On the other hand, with the current patch set, we will have IMA
validate the file signature on the clients and the service validate
that the IMA keys were installed using trusted keys.


This provides a chain of trust:
    => IMA Key validates file signature on the client
    => Built-In trusted key attests IMA key on the client
    => Attestation service attests the Built-In trusted keys
         reported by the client in the IMA log


This approach, therefore, would require the service to maintain
a manageble set of trusted keys that it receives from a trusted source.
And, verify if the clients only have keys from that set of trusted keys.

Question 4:
Where will the attestation service receive the keys to validate against?

Answer:
Attestation service will receive the keys from a trusted source such as
the enterprise build services that provides the client builds.
The service will use this set of keys to verify that the keys reported by
the clients in the IMA log contains only keys from this trusted list.


Question 5:
What is changing in the IMA log through this patch set?


Answer:
This patch set does not remove any data that is currently included
in the IMA log. It only adds more data to the IMA log - the data on
".builtin_trusted_keys"

Lakshmi Ramasubramanian (9):
  KEYS: Defined an IMA hook to measure keys on key create or update
  KEYS: Defined functions to queue and dequeue keys for measurement
  KEYS: Added KEYRING_CHECK policy for key measurement
  KEYS: Updated IMA policy functions for handling key measurement
  KEYS: Updated ima_get_action() to return keyrings if specified in the
    policy
  KEYS: Measure key if the IMA policy allows measurement for the given
    keyring
  KEYS: Queue key for measurement if IMA is not yet initialized. Measure
    queued keys when IMA initialization is completed
  KEYS: Added a boolean flag for IMA initialization status.
    ima_policy_flag cannot be relied upon for knowing IMA initialization
    status because ima_policy_flag can be set to 0 when IMA is
    initialized, but there is no IMA policy configured.
  KEYS: Call the IMA hook to measure key when a new key is created or an
    existing key is updated

 Documentation/ABI/testing/ima_policy  | 15 +++++
 include/linux/ima.h                   |  7 ++
 security/integrity/ima/ima.h          | 27 ++++++--
 security/integrity/ima/ima_api.c      |  7 +-
 security/integrity/ima/ima_appraise.c |  2 +-
 security/integrity/ima/ima_init.c     | 10 ++-
 security/integrity/ima/ima_main.c     | 57 ++++++++++++++--
 security/integrity/ima/ima_policy.c   | 42 +++++++++++-
 security/integrity/ima/ima_queue.c    | 93 +++++++++++++++++++++++++++
 security/keys/key.c                   |  9 +++
 10 files changed, 254 insertions(+), 15 deletions(-)

-- 
2.17.1


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

* [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  2019-10-31  9:10   ` Sasha Levin
  2019-10-31 12:10   ` Mimi Zohar
  2019-10-31  1:19 ` [PATCH v3 2/9] KEYS: Defined functions to queue and dequeue keys for measurement Lakshmi Ramasubramanian
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

Asymmetric keys used for verifying file signatures or certificates
are currently not included in the IMA measurement list.

This patch defines a new IMA hook namely ima_post_key_create_or_update()
to measure asymmetric keys.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima.h      |  2 ++
 security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 997a57137351..22d0628faf56 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -21,6 +21,8 @@
 #include <linux/tpm.h>
 #include <linux/audit.h>
 #include <crypto/hash_info.h>
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.h>
 
 #include "../integrity.h"
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 492b8f241d39..18e1bc105be7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -635,6 +635,9 @@ void process_buffer_measurement(const void *buf, int size,
 	int action = 0;
 	u32 secid;
 
+	if (!ima_policy_flag)
+		return;
+
 	if (func) {
 		security_task_getsecid(current, &secid);
 		action = ima_get_action(NULL, current_cred(), secid, 0, func,
@@ -695,6 +698,29 @@ void ima_kexec_cmdline(const void *buf, int size)
 	}
 }
 
+/**
+ * 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;
+
+	if (key->type != &key_type_asymmetric)
+		return;
+
+	pk = key->payload.data[asym_crypto];
+	process_buffer_measurement(pk->key, pk->keylen,
+				   keyring->description,
+				   NONE, 0);
+}
+
 static int __init init_ima(void)
 {
 	int error;
-- 
2.17.1


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

* [PATCH v3 2/9] KEYS: Defined functions to queue and dequeue keys for measurement
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  2019-10-31 12:10   ` Mimi Zohar
  2019-10-31  1:19 ` [PATCH v3 3/9] KEYS: Added KEYRING_CHECK policy for key measurement Lakshmi Ramasubramanian
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

Key measurements cannot be done if the IMA hook to measure keys is
called before IMA is initialized. Key measurement needs to be deferred
if IMA is not yet initialized. Queued keys need to be processed when
IMA initialization is completed.

This patch defines functions to queue and de-queue keys for measurement.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima.h       | 12 ++++
 security/integrity/ima/ima_queue.c | 92 ++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 22d0628faf56..b9600070e415 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -198,6 +198,16 @@ enum ima_hooks {
 	__ima_hooks(__ima_hook_enumify)
 };
 
+/*
+ * To track keys that need to be measured.
+ */
+struct ima_measure_key_entry {
+	struct list_head list;
+	void *public_key;
+	u32  public_key_len;
+	char *keyring_name;
+};
+
 /* 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,
@@ -224,6 +234,8 @@ int ima_store_template(struct ima_template_entry *entry, int violation,
 		       const unsigned char *filename, int pcr);
 void ima_free_template_entry(struct ima_template_entry *entry);
 const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
+int ima_queue_key_for_measurement(struct key *keyring, struct key *key);
+void ima_measure_queued_keys(void);
 
 /* IMA policy related functions */
 int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 1ce8b1701566..f2503f10abf4 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -46,6 +46,12 @@ struct ima_h_table ima_htable = {
  */
 static DEFINE_MUTEX(ima_extend_list_mutex);
 
+/*
+ * To synchronize access to the list of keys that need to be measured
+ */
+static DEFINE_MUTEX(ima_measure_keys_mutex);
+static LIST_HEAD(ima_measure_keys);
+
 /* lookup up the digest value in the hash table, and return the entry */
 static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
 						       int pcr)
@@ -232,3 +238,89 @@ int __init ima_init_digests(void)
 
 	return 0;
 }
+
+static void ima_free_measure_key_entry(struct ima_measure_key_entry *entry)
+{
+	if (entry != NULL) {
+		if (entry->public_key != NULL)
+			kzfree(entry->public_key);
+		if (entry->keyring_name != NULL)
+			kzfree(entry->keyring_name);
+		kzfree(entry);
+	}
+}
+
+static struct ima_measure_key_entry *ima_alloc_measure_key_entry(
+	struct key *keyring,
+	struct key *key)
+{
+	int rc = 0;
+	const struct public_key *pk;
+	size_t keyring_name_len;
+	struct ima_measure_key_entry *entry = NULL;
+
+	pk = key->payload.data[asym_crypto];
+	keyring_name_len = strlen(keyring->description) + 1;
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (entry != NULL) {
+		entry->public_key = kzalloc(pk->keylen, GFP_KERNEL);
+		entry->keyring_name =
+			kzalloc(keyring_name_len, GFP_KERNEL);
+	}
+
+	if ((entry == NULL) || (entry->public_key == NULL) ||
+	    (entry->keyring_name == NULL)) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	strcpy(entry->keyring_name, keyring->description);
+	memcpy(entry->public_key, pk->key, pk->keylen);
+	entry->public_key_len = pk->keylen;
+	rc = 0;
+
+out:
+	if (rc) {
+		ima_free_measure_key_entry(entry);
+		entry = NULL;
+	}
+
+	return entry;
+}
+
+int ima_queue_key_for_measurement(struct key *keyring, struct key *key)
+{
+	int rc = 0;
+	struct ima_measure_key_entry *entry = NULL;
+
+	mutex_lock(&ima_measure_keys_mutex);
+
+	entry = ima_alloc_measure_key_entry(keyring, key);
+	if (entry != NULL) {
+		INIT_LIST_HEAD(&entry->list);
+		list_add_tail(&entry->list, &ima_measure_keys);
+	} else
+		rc = -ENOMEM;
+
+	mutex_unlock(&ima_measure_keys_mutex);
+
+	return rc;
+}
+
+void ima_measure_queued_keys(void)
+{
+	struct ima_measure_key_entry *entry, *tmp;
+
+	mutex_lock(&ima_measure_keys_mutex);
+
+	list_for_each_entry_safe(entry, tmp, &ima_measure_keys, list) {
+		process_buffer_measurement(entry->public_key,
+					   entry->public_key_len,
+					   entry->keyring_name,
+					   NONE, 0);
+		list_del(&entry->list);
+		ima_free_measure_key_entry(entry);
+	}
+
+	mutex_unlock(&ima_measure_keys_mutex);
+}
-- 
2.17.1


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

* [PATCH v3 3/9] KEYS: Added KEYRING_CHECK policy for key measurement
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 2/9] KEYS: Defined functions to queue and dequeue keys for measurement Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  2019-10-31 12:10   ` Mimi Zohar
  2019-10-31  1:19 ` [PATCH v3 4/9] KEYS: Updated IMA policy functions for handling " Lakshmi Ramasubramanian
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

An IMA policy to manage measurement of keys is not supported.
A new IMA policy is needed to manage the measurement of keys.
A policy option is also needed to allow measurement of keys
linked to a given set of keyrings only.

This patch defines KEYRING_CHECK and keyrings in IMA policy
for this purpose. 

KEYRING_CHECK can be added in the IMA policy to measure keys.
keyrings can be, optionally, set to only measure keys
added or updated to a given set of keyrings. If keyrings is not
specified for KEYRING_CHECK, keys added or updated in
all keyrings are measured.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 Documentation/ABI/testing/ima_policy | 15 +++++++++++++++
 security/integrity/ima/ima.h         |  1 +
 security/integrity/ima/ima_main.c    |  2 +-
 security/integrity/ima/ima_policy.c  |  2 +-
 security/integrity/ima/ima_queue.c   |  2 +-
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index fc376a323908..757faf1a1a27 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,10 +25,12 @@ Description:
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [template=] [permit_directio]
+				[keyrings=]
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
 				[KEXEC_CMDLINE]
+				[KEYRING_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value
@@ -38,6 +40,9 @@ Description:
 			fowner:= decimal value
 		lsm:  	are LSM specific
 		option:	appraise_type:= [imasig]
+			keyrings: = list of keyrings to measure
+			(eg, .builtin_trusted_keys|.ima). Only valid
+			when action is "measure" and func is KEYRING_CHECK.
 			template:= name of a defined IMA template type
 			(eg, ima-ng). Only valid when action is "measure".
 			pcr:= decimal value
@@ -105,3 +110,13 @@ Description:
 
 			measure func=KEXEC_KERNEL_CHECK pcr=4
 			measure func=KEXEC_INITRAMFS_CHECK pcr=5
+
+		Example of measure rules using KEYRING_CHECK
+			To measure keys added to
+			.builtin_trusted_keys or .ima keyring:
+
+			measure func=KEYRING_CHECK keyrings=.builtin_trusted_keys|.ima
+
+			To measure keys added to all keyrings:
+
+			measure func=KEYRING_CHECK
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b9600070e415..12e9ec6847b5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -191,6 +191,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	hook(KEXEC_INITRAMFS_CHECK)	\
 	hook(POLICY_CHECK)		\
 	hook(KEXEC_CMDLINE)		\
+	hook(KEYRING_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 18e1bc105be7..72ae0878ec5d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -718,7 +718,7 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
 	pk = key->payload.data[asym_crypto];
 	process_buffer_measurement(pk->key, pk->keylen,
 				   keyring->description,
-				   NONE, 0);
+				   KEYRING_CHECK, 0);
 }
 
 static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 6df7f641ff66..0cc49f2d5233 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -370,7 +370,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 == KEYRING_CHECK)) {
 		if ((rule->flags & IMA_FUNC) && (rule->func == func))
 			return true;
 		return false;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index f2503f10abf4..5625381c5a97 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -317,7 +317,7 @@ void ima_measure_queued_keys(void)
 		process_buffer_measurement(entry->public_key,
 					   entry->public_key_len,
 					   entry->keyring_name,
-					   NONE, 0);
+					   KEYRING_CHECK, 0);
 		list_del(&entry->list);
 		ima_free_measure_key_entry(entry);
 	}
-- 
2.17.1


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

* [PATCH v3 4/9] KEYS: Updated IMA policy functions for handling key measurement
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (2 preceding siblings ...)
  2019-10-31  1:19 ` [PATCH v3 3/9] KEYS: Added KEYRING_CHECK policy for key measurement Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` " Lakshmi Ramasubramanian
  2019-10-31 12:10   ` Mimi Zohar
  2019-10-31  1:19 ` [PATCH v3 5/9] KEYS: Updated ima_get_action() to return keyrings if specified in the policy Lakshmi Ramasubramanian
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

Information regarding what keyrings need to be measured is missing.

A new field in the IMA policy, namely, keyrings is added to
convey what keyrings need to be measured.

This patch updates the IMA function to retrieve keyrings from the policy.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima.h          |  6 ++--
 security/integrity/ima/ima_api.c      |  3 +-
 security/integrity/ima/ima_appraise.c |  2 +-
 security/integrity/ima/ima_policy.c   | 40 +++++++++++++++++++++++++--
 4 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 12e9ec6847b5..3539a159a7ac 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -240,8 +240,10 @@ void ima_measure_queued_keys(void);
 
 /* 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);
+		     enum ima_hooks func, int mask,
+		     int flags, int *pcr,
+		     struct ima_template_desc **template_desc,
+		     char **keyrings);
 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 f614e22bf39f..f488d1cead79 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -175,6 +175,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  *	subj,obj, and type: are LSM specific.
  *	func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
  *	| KEXEC_CMDLINE
+ *      | KEYRING_CHECK
  *	mask: contains the permission mask
  *	fsmagic: hex value
  *
@@ -190,7 +191,7 @@ int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
 	flags &= ima_policy_flag;
 
 	return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,
-				template_desc);
+				template_desc, NULL);
 }
 
 /*
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 89b83194d1dc..5bed19be0f6a 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -54,7 +54,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,
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 0cc49f2d5233..b972a32ccb1b 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -31,6 +31,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 */
@@ -76,6 +77,7 @@ struct ima_rule_entry {
 		int type;	/* audit type */
 	} lsm[MAX_LSM_RULES];
 	char *fsname;
+	char *keyrings; /* Keyrings to measure */
 	struct ima_template_desc *template;
 };
 
@@ -476,6 +478,7 @@ 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
+ * @keyrings: keyrings for this rule, if specified
  *
  * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
  * conditions.
@@ -486,7 +489,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,
+		     char **keyrings)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
@@ -518,6 +522,9 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 		if ((pcr) && (entry->flags & IMA_PCR))
 			*pcr = entry->pcr;
 
+		if ((keyrings) && (entry->flags & IMA_KEYRINGS))
+			*keyrings = entry->keyrings;
+
 		if (template_desc && entry->template)
 			*template_desc = entry->template;
 		else if (template_desc)
@@ -761,7 +768,7 @@ enum {
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
 	Opt_appraise_type, Opt_permit_directio,
-	Opt_pcr, Opt_template, Opt_err
+	Opt_pcr, Opt_template, Opt_keyrings, Opt_err
 };
 
 static const match_table_t policy_tokens = {
@@ -796,6 +803,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}
 };
 
@@ -959,6 +967,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, "KEYRING_CHECK") == 0)
+				entry->func = KEYRING_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)
@@ -1011,6 +1021,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 != KEYRING_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);
 
@@ -1371,6 +1398,15 @@ 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);
+		else
+			snprintf(tbuf, sizeof(tbuf), "%s", "All 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	[flat|nested] 22+ messages in thread

* [PATCH v3 5/9] KEYS: Updated ima_get_action() to return keyrings if specified in the policy
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (3 preceding siblings ...)
  2019-10-31  1:19 ` [PATCH v3 4/9] KEYS: Updated IMA policy functions for handling " Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 6/9] KEYS: Measure key if the IMA policy allows measurement for the given keyring Lakshmi Ramasubramanian
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

Information regarding what keyrings need to be measured is missing.
ima_get_action() needs to retrieve the keyrings, if specified for
KEYRING_CHECK. 

This patch adds a new out parameter to ima_get_action() to return
keyrings read from the policy.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima.h      | 3 ++-
 security/integrity/ima/ima_api.c  | 6 ++++--
 security/integrity/ima/ima_main.c | 8 ++++++--
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3539a159a7ac..ded78af94e69 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -212,7 +212,8 @@ struct ima_measure_key_entry {
 /* 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,
+		   char **keyrings);
 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,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index f488d1cead79..77ac076672e1 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -169,6 +169,7 @@ 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=
+ * @keyrings: pointer filled in if matched measure policy sets keyrings=
  *
  * The policy is defined in terms of keypairs:
  *		subj=, obj=, type=, func=, mask=, fsmagic=
@@ -184,14 +185,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,
+		   char **keyrings)
 {
 	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, NULL);
+				template_desc, keyrings);
 }
 
 /*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 72ae0878ec5d..cbc7de87106f 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -214,7 +214,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)
@@ -627,6 +627,7 @@ void process_buffer_measurement(const void *buf, int size,
 					    .buf = buf,
 					    .buf_len = size};
 	struct ima_template_desc *template = NULL;
+	char *keyrings = NULL;
 	struct {
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];
@@ -641,11 +642,14 @@ 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, &keyrings);
 		if (!(action & IMA_MEASURE))
 			return;
 	}
 
+	if (keyrings != NULL)
+		keyrings = NULL;
+
 	if (!pcr)
 		pcr = CONFIG_IMA_MEASURE_PCR_IDX;
 
-- 
2.17.1


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

* [PATCH v3 6/9] KEYS: Measure key if the IMA policy allows measurement for the given keyring
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (4 preceding siblings ...)
  2019-10-31  1:19 ` [PATCH v3 5/9] KEYS: Updated ima_get_action() to return keyrings if specified in the policy Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 7/9] KEYS: Queue key for measurement if IMA is not yet initialized. Measure queued keys when IMA initialization is completed Lakshmi Ramasubramanian
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

process_buffer_measurement() does not know which keyring the given key
is being linked to. It needs the keyring name to determine whether 
or not the given key needs to be measured.

This patch adds a new parameter "keyring" to process_buffer_measurement()
to convey which keyring the given key is linked to.

If KEYRING_CHECK alone is set in the policy, all keys are measured.
If a list of keyrings is also specified in the policy then only keys
linked to those keyrings will be measured.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima.h       |  2 +-
 security/integrity/ima/ima_main.c  | 24 +++++++++++++++++++-----
 security/integrity/ima/ima_queue.c |  3 ++-
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ded78af94e69..f8bf5c24e0d0 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -225,7 +225,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,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index cbc7de87106f..bd835ec89ead 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -612,12 +612,22 @@ 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 for the measurement
+ *
+ *	The following scenarios are possible with respect to
+ *	the parameter "keyring":
+ *	1, keyring is NULL. In this case buffer is measured.
+ *	2, keyring is not NULL, but ima_get_action returned
+ *	   a NULL keyrings. In this case also the buffer is measured.
+ *	3, keyring is not NULL and ima_get_action returned
+ *	   a non-NULL keyrings. In this case measure the buffer
+ *	   only if the given keyring is present in the keyrings.
  *
  * 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;
@@ -647,8 +657,10 @@ void process_buffer_measurement(const void *buf, int size,
 			return;
 	}
 
-	if (keyrings != NULL)
-		keyrings = NULL;
+	if ((keyring != NULL) && (keyrings != NULL)
+	    && (strstr(keyrings, keyring) == NULL)) {
+		return;
+	}
 
 	if (!pcr)
 		pcr = CONFIG_IMA_MEASURE_PCR_IDX;
@@ -698,7 +710,8 @@ 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);
 	}
 }
 
@@ -722,7 +735,8 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
 	pk = key->payload.data[asym_crypto];
 	process_buffer_measurement(pk->key, pk->keylen,
 				   keyring->description,
-				   KEYRING_CHECK, 0);
+				   KEYRING_CHECK, 0,
+				   keyring->description);
 }
 
 static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 5625381c5a97..805dcacb48e6 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -317,7 +317,8 @@ void ima_measure_queued_keys(void)
 		process_buffer_measurement(entry->public_key,
 					   entry->public_key_len,
 					   entry->keyring_name,
-					   KEYRING_CHECK, 0);
+					   KEYRING_CHECK, 0,
+					   entry->keyring_name);
 		list_del(&entry->list);
 		ima_free_measure_key_entry(entry);
 	}
-- 
2.17.1


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

* [PATCH v3 7/9] KEYS: Queue key for measurement if IMA is not yet initialized. Measure queued keys when IMA initialization is completed
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (5 preceding siblings ...)
  2019-10-31  1:19 ` [PATCH v3 6/9] KEYS: Measure key if the IMA policy allows measurement for the given keyring Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 8/9] KEYS: Added a boolean flag for IMA initialization status Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 9/9] KEYS: Call the IMA hook to measure key when a new key is created or an existing key is updated Lakshmi Ramasubramanian
  8 siblings, 0 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

Keys need to be queued when the IMA hook to measure keys is called before
IMA is initialized. Keys queued for measurement need to be processed when
IMA initialization is completed.

This patch adds the call to queue and de-queue keys for measurement.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima_init.c | 7 ++++++-
 security/integrity/ima/ima_main.c | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 5d55ade5f3b9..91eaa5f2d008 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -131,5 +131,10 @@ int __init ima_init(void)
 
 	ima_init_policy();
 
-	return ima_fs_init();
+	rc = ima_fs_init();
+	if (rc != 0)
+		return rc;
+
+	ima_measure_queued_keys();
+	return 0;
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index bd835ec89ead..2ad05563542c 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -732,6 +732,11 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
 	if (key->type != &key_type_asymmetric)
 		return;
 
+	if (!ima_policy_flag) {
+		ima_queue_key_for_measurement(keyring, key);
+		return;
+	}
+
 	pk = key->payload.data[asym_crypto];
 	process_buffer_measurement(pk->key, pk->keylen,
 				   keyring->description,
-- 
2.17.1


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

* [PATCH v3 8/9] KEYS: Added a boolean flag for IMA initialization status.
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (6 preceding siblings ...)
  2019-10-31  1:19 ` [PATCH v3 7/9] KEYS: Queue key for measurement if IMA is not yet initialized. Measure queued keys when IMA initialization is completed Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  2019-10-31  1:19 ` [PATCH v3 9/9] KEYS: Call the IMA hook to measure key when a new key is created or an existing key is updated Lakshmi Ramasubramanian
  8 siblings, 0 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

IMA hook does not know whether a key can be measured right away or
the key needs to be queued to be measured at a later time.

This patch defines a flag to indicate the IMA initialization status.
IMA hook will use this flag to determine if a key can be measured
right away or the key needs to be queued to be measured at a later time.

ima_policy_flag cannot be relied upon for knowing IMA initialization
status because ima_policy_flag will be set to 0 when either IMA
is not initialized or the IMA policy itself is empty.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 security/integrity/ima/ima.h      | 1 +
 security/integrity/ima/ima_init.c | 3 +++
 security/integrity/ima/ima_main.c | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index f8bf5c24e0d0..5abc5a0b4591 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -54,6 +54,7 @@ extern int ima_policy_flag;
 extern int ima_hash_algo;
 extern int ima_appraise;
 extern struct tpm_chip *ima_tpm_chip;
+extern bool ima_initialized;
 
 /* IMA event related data */
 struct ima_event_data {
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 91eaa5f2d008..8734ed5322c7 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -23,6 +23,7 @@
 /* name for boot aggregate entry */
 static const char boot_aggregate_name[] = "boot_aggregate";
 struct tpm_chip *ima_tpm_chip;
+bool ima_initialized;
 
 /* Add the boot aggregate to the IMA measurement list and extend
  * the PCR register.
@@ -135,6 +136,8 @@ int __init ima_init(void)
 	if (rc != 0)
 		return rc;
 
+	ima_initialized = true;
+
 	ima_measure_queued_keys();
 	return 0;
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2ad05563542c..e4c5e7150611 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -732,7 +732,7 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
 	if (key->type != &key_type_asymmetric)
 		return;
 
-	if (!ima_policy_flag) {
+	if (!ima_initialized) {
 		ima_queue_key_for_measurement(keyring, key);
 		return;
 	}
-- 
2.17.1


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

* [PATCH v3 9/9] KEYS: Call the IMA hook to measure key when a new key is created or an existing key is updated
  2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
                   ` (7 preceding siblings ...)
  2019-10-31  1:19 ` [PATCH v3 8/9] KEYS: Added a boolean flag for IMA initialization status Lakshmi Ramasubramanian
@ 2019-10-31  1:19 ` Lakshmi Ramasubramanian
  8 siblings, 0 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31  1:19 UTC (permalink / raw)
  To: zohar, dhowells, matthewgarrett, sashal, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings
  Cc: prsriva

key_create_or_update function needs to call the IMA hook 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.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 include/linux/ima.h | 7 +++++++
 security/keys/key.c | 9 +++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index a20ad398d260..f085f1c6ef34 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -24,6 +24,9 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
 extern void ima_kexec_cmdline(const void *buf, int size);
+extern void ima_post_key_create_or_update(struct key *keyring,
+					  struct key *key,
+					  unsigned long flags, bool create);
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
@@ -91,6 +94,10 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 }
 
 static inline void ima_kexec_cmdline(const void *buf, int size) {}
+static inline void ima_post_key_create_or_update(struct key *keyring,
+						 struct key *key,
+						 unsigned long flags,
+						 bool create) {}
 #endif /* CONFIG_IMA */
 
 #ifndef CONFIG_IMA_KEXEC
diff --git a/security/keys/key.c b/security/keys/key.c
index 764f4c57913e..7c39054d8da6 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);
+	if (!IS_ERR(key_ref)) {
+		/* let the ima module know about the updated key. */
+		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] 22+ messages in thread

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31  1:19 ` [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update Lakshmi Ramasubramanian
@ 2019-10-31  9:10   ` Sasha Levin
  2019-10-31 15:27     ` Lakshmi Ramasubramanian
  2019-10-31 12:10   ` Mimi Zohar
  1 sibling, 1 reply; 22+ messages in thread
From: Sasha Levin @ 2019-10-31  9:10 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian
  Cc: zohar, dhowells, matthewgarrett, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings, prsriva

On Wed, Oct 30, 2019 at 06:19:02PM -0700, Lakshmi Ramasubramanian wrote:
>Asymmetric keys used for verifying file signatures or certificates
>are currently not included in the IMA measurement list.
>
>This patch defines a new IMA hook namely ima_post_key_create_or_update()
>to measure asymmetric keys.
>
>Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

What are the prerequisites for this patch?

-- 
Thanks,
Sasha

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

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31  1:19 ` [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update Lakshmi Ramasubramanian
  2019-10-31  9:10   ` Sasha Levin
@ 2019-10-31 12:10   ` Mimi Zohar
  2019-10-31 15:08     ` Lakshmi Ramasubramanian
  1 sibling, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2019-10-31 12:10 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-kernel, linux-integrity, linux-security-module,
	keyrings
  Cc: prsriva

On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
> Asymmetric keys used for verifying file signatures or certificates
> are currently not included in the IMA measurement list.
> 
> This patch defines a new IMA hook namely ima_post_key_create_or_update()
> to measure asymmetric keys.

It's not enough for the kernel to be able to compile the kernel after
applying all the patches in a patch set.  After applying each patch,
the kernel should build properly, otherwise it is not bi-sect safe.
 Refer to "3) Separate your changes" of
"Documentation/process/submitting-patches.rst.
 
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  security/integrity/ima/ima.h      |  2 ++
>  security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 997a57137351..22d0628faf56 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -21,6 +21,8 @@
>  #include <linux/tpm.h>
>  #include <linux/audit.h>
>  #include <crypto/hash_info.h>
> +#include <crypto/public_key.h>
> +#include <keys/asymmetric-type.h>
>  
>  #include "../integrity.h"
>  
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 492b8f241d39..18e1bc105be7 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -635,6 +635,9 @@ void process_buffer_measurement(const void *buf, int size,
>  	int action = 0;
>  	u32 secid;
>  
> +	if (!ima_policy_flag)
> +		return;
> +
>  	if (func) {
>  		security_task_getsecid(current, &secid);
>  		action = ima_get_action(NULL, current_cred(), secid, 0, func,
> @@ -695,6 +698,29 @@ void ima_kexec_cmdline(const void *buf, int size)
>  	}
>  }
>  
> +/**
> + * 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;
> +
> +	if (key->type != &key_type_asymmetric)
> +		return;
> +
> +	pk = key->payload.data[asym_crypto];
> +	process_buffer_measurement(pk->key, pk->keylen,
> +				   keyring->description,
> +				   NONE, 0);

This patch should also define the new "func".

Mimi

> +}
> +
>  static int __init init_ima(void)
>  {
>  	int error;


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

* Re: [PATCH v3 2/9] KEYS: Defined functions to queue and dequeue keys for measurement
  2019-10-31  1:19 ` [PATCH v3 2/9] KEYS: Defined functions to queue and dequeue keys for measurement Lakshmi Ramasubramanian
@ 2019-10-31 12:10   ` Mimi Zohar
  2019-10-31 15:09     ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2019-10-31 12:10 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-kernel, linux-integrity, linux-security-module,
	keyrings
  Cc: prsriva

On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
> Key measurements cannot be done if the IMA hook to measure keys is
> called before IMA is initialized. Key measurement needs to be deferred
> if IMA is not yet initialized. Queued keys need to be processed when
> IMA initialization is completed.
> 
> This patch defines functions to queue and de-queue keys for measurement.

I would defer this patch until later in the patch set, after having
the basic measuring of keys in place.

Mimi


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

* Re: [PATCH v3 3/9] KEYS: Added KEYRING_CHECK policy for key measurement
  2019-10-31  1:19 ` [PATCH v3 3/9] KEYS: Added KEYRING_CHECK policy for key measurement Lakshmi Ramasubramanian
@ 2019-10-31 12:10   ` Mimi Zohar
  0 siblings, 0 replies; 22+ messages in thread
From: Mimi Zohar @ 2019-10-31 12:10 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-kernel, linux-integrity, linux-security-module,
	keyrings
  Cc: prsriva

On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
> An IMA policy to manage measurement of keys is not supported.
> A new IMA policy is needed to manage the measurement of keys.
> A policy option is also needed to allow measurement of keys
> linked to a given set of keyrings only.
> 
> This patch defines KEYRING_CHECK and keyrings in IMA policy
> for this purpose. 

"KEYRING_CHECK" and "keyrings" are not related.   One is a "func"
name, while the other is an IMA policy option.  This should be broken
up into two different patches.  When defining a new policy option, the
only code in that patch should be the new policy option.

> 
> KEYRING_CHECK can be added in the IMA policy to measure keys.
> keyrings can be, optionally, set to only measure keys
> added or updated to a given set of keyrings. If keyrings is not
> specified for KEYRING_CHECK, keys added or updated in
> all keyrings are measured.
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  Documentation/ABI/testing/ima_policy | 15 +++++++++++++++
>  security/integrity/ima/ima.h         |  1 +
>  security/integrity/ima/ima_main.c    |  2 +-
>  security/integrity/ima/ima_policy.c  |  2 +-
>  security/integrity/ima/ima_queue.c   |  2 +-
>  5 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index fc376a323908..757faf1a1a27 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -25,10 +25,12 @@ Description:
>  			lsm:	[[subj_user=] [subj_role=] [subj_type=]
>  				 [obj_user=] [obj_role=] [obj_type=]]
>  			option:	[[appraise_type=]] [template=] [permit_directio]
> +				[keyrings=]
>  		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
>  				[FIRMWARE_CHECK]
>  				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
>  				[KEXEC_CMDLINE]
> +				[KEYRING_CHECK]

This patch is measuring keys, not keyrings.


>  			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
>  			       [[^]MAY_EXEC]
>  			fsmagic:= hex value
> @@ -38,6 +40,9 @@ Description:
>  			fowner:= decimal value
>  		lsm:  	are LSM specific
>  		option:	appraise_type:= [imasig]
> +			keyrings: = list of keyrings to measure
> +			(eg, .builtin_trusted_keys|.ima). Only valid
> +			when action is "measure" and func is KEYRING_CHECK.
>  			template:= name of a defined IMA template type
>  			(eg, ima-ng). Only valid when action is "measure".
>  			pcr:= decimal value
> @@ -105,3 +110,13 @@ Description:
>  
>  			measure func=KEXEC_KERNEL_CHECK pcr=4
>  			measure func=KEXEC_INITRAMFS_CHECK pcr=5
> +
> +		Example of measure rules using KEYRING_CHECK
> +			To measure keys added to
> +			.builtin_trusted_keys or .ima keyring:
> +
> +			measure func=KEYRING_CHECK keyrings=.builtin_trusted_keys|.ima
> +
> +			To measure keys added to all keyrings:
> +
> +			measure func=KEYRING_CHECK

The patch that introduces the new IMA "func" should document the new
IMA "func".  The patch that introduces the new "keyring=" policy
option should document the new IMA policy option.  Examples could be
included in each of the patches descriptions.


> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index b9600070e415..12e9ec6847b5 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -191,6 +191,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
>  	hook(KEXEC_INITRAMFS_CHECK)	\
>  	hook(POLICY_CHECK)		\
>  	hook(KEXEC_CMDLINE)		\
> +	hook(KEYRING_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 18e1bc105be7..72ae0878ec5d 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -718,7 +718,7 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
>  	pk = key->payload.data[asym_crypto];
>  	process_buffer_measurement(pk->key, pk->keylen,
>  				   keyring->description,
> -				   NONE, 0);
> +				   KEYRING_CHECK, 0);
>  }
>  
>  static int __init init_ima(void)
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 6df7f641ff66..0cc49f2d5233 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -370,7 +370,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 == KEYRING_CHECK)) {
>  		if ((rule->flags & IMA_FUNC) && (rule->func == func))
>  			return true;
>  		return false;
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index f2503f10abf4..5625381c5a97 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -317,7 +317,7 @@ void ima_measure_queued_keys(void)
>  		process_buffer_measurement(entry->public_key,
>  					   entry->public_key_len,
>  					   entry->keyring_name,
> -					   NONE, 0);
> +					   KEYRING_CHECK, 0);

Changing a newly defined call should be an indication that the patch
ordering is wrong.  If the new "func" was defined prior or with the
new IMA hook, then this change wouldn't be needed.

Mimi


>  		list_del(&entry->list);
>  		ima_free_measure_key_entry(entry);
>  	}


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

* Re: [PATCH v3 4/9] KEYS: Updated IMA policy functions for handling key measurement
  2019-10-31  1:19 ` [PATCH v3 4/9] KEYS: Updated IMA policy functions for handling " Lakshmi Ramasubramanian
@ 2019-10-31 12:10   ` Mimi Zohar
  0 siblings, 0 replies; 22+ messages in thread
From: Mimi Zohar @ 2019-10-31 12:10 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
	jamorris, linux-kernel, linux-integrity, linux-security-module,
	keyrings
  Cc: prsriva

This patch adds support for "keyring=".  The patch title should
reflect it.

On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
> Information regarding what keyrings need to be measured is missing.
> 
> A new field in the IMA policy, namely, keyrings is added to
> convey what keyrings need to be measured.
> 
> This patch updates the IMA function to retrieve keyrings from the policy.

Defining a new policy option should be separate from modifying
functions.

Mimi


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

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31 12:10   ` Mimi Zohar
@ 2019-10-31 15:08     ` Lakshmi Ramasubramanian
  2019-10-31 15:27       ` Sasha Levin
  0 siblings, 1 reply; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31 15:08 UTC (permalink / raw)
  To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-kernel, linux-integrity, linux-security-module, keyrings
  Cc: prsriva

On 10/31/19 5:10 AM, Mimi Zohar wrote:

> On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
>> Asymmetric keys used for verifying file signatures or certificates
>> are currently not included in the IMA measurement list.
>>
>> This patch defines a new IMA hook namely ima_post_key_create_or_update()
>> to measure asymmetric keys.
> 
> It's not enough for the kernel to be able to compile the kernel after
> applying all the patches in a patch set.  After applying each patch,
> the kernel should build properly, otherwise it is not bi-sect safe.
>   Refer to "3) Separate your changes" of
> "Documentation/process/submitting-patches.rst.

I started with kernel version 5.3 for this patch set.
I applied Nayna's process_buffer_measurement() patch and then built my 
changes on top of that.
This patch has no other dependency as far as I know.

Are you seeing a build break after applying this patch alone?

(PATCH v3 1/9) KEYS: Defined an IMA hook to measure keys on key create 
or update
> 
> This patch should also define the new "func".
> 

Ok - I'll make that change.

thanks,
  -lakshmi

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

* Re: [PATCH v3 2/9] KEYS: Defined functions to queue and dequeue keys for measurement
  2019-10-31 12:10   ` Mimi Zohar
@ 2019-10-31 15:09     ` Lakshmi Ramasubramanian
  0 siblings, 0 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31 15:09 UTC (permalink / raw)
  To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
	linux-kernel, linux-integrity, linux-security-module, keyrings
  Cc: prsriva

On 10/31/19 5:10 AM, Mimi Zohar wrote:

> On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
>> Key measurements cannot be done if the IMA hook to measure keys is
>> called before IMA is initialized. Key measurement needs to be deferred
>> if IMA is not yet initialized. Queued keys need to be processed when
>> IMA initialization is completed.
>>
>> This patch defines functions to queue and de-queue keys for measurement.
> 
> I would defer this patch until later in the patch set, after having
> the basic measuring of keys in place.

Ok - I'll move this this patch.

thanks,
  -lakshmi




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

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31 15:08     ` Lakshmi Ramasubramanian
@ 2019-10-31 15:27       ` Sasha Levin
  2019-10-31 15:37         ` Mimi Zohar
  0 siblings, 1 reply; 22+ messages in thread
From: Sasha Levin @ 2019-10-31 15:27 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian
  Cc: Mimi Zohar, dhowells, matthewgarrett, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings, prsriva

On Thu, Oct 31, 2019 at 08:08:48AM -0700, Lakshmi Ramasubramanian wrote:
>On 10/31/19 5:10 AM, Mimi Zohar wrote:
>
>>On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
>>>Asymmetric keys used for verifying file signatures or certificates
>>>are currently not included in the IMA measurement list.
>>>
>>>This patch defines a new IMA hook namely ima_post_key_create_or_update()
>>>to measure asymmetric keys.
>>
>>It's not enough for the kernel to be able to compile the kernel after
>>applying all the patches in a patch set.  After applying each patch,
>>the kernel should build properly, otherwise it is not bi-sect safe.
>>  Refer to "3) Separate your changes" of
>>"Documentation/process/submitting-patches.rst.
>
>I started with kernel version 5.3 for this patch set.
>I applied Nayna's process_buffer_measurement() patch and then built my 
>changes on top of that.
>This patch has no other dependency as far as I know.
>
>Are you seeing a build break after applying this patch alone?
>
>(PATCH v3 1/9) KEYS: Defined an IMA hook to measure keys on key create 
>or update

I couldn't even apply this patch: Nayna's series (v10) doesn't apply on
top of 5.3 to begin with, and while it does apply on mainline, this
first patch wouldn't apply on top.

-- 
Thanks,
Sasha

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

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31  9:10   ` Sasha Levin
@ 2019-10-31 15:27     ` Lakshmi Ramasubramanian
  2019-10-31 15:36       ` Sasha Levin
  0 siblings, 1 reply; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31 15:27 UTC (permalink / raw)
  To: Sasha Levin
  Cc: zohar, dhowells, matthewgarrett, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings, prsriva

On 10/31/19 2:10 AM, Sasha Levin wrote:

Hi Sasha,

> On Wed, Oct 30, 2019 at 06:19:02PM -0700, Lakshmi Ramasubramanian wrote:
>> Asymmetric keys used for verifying file signatures or certificates
>> are currently not included in the IMA measurement list.
>>
>> This patch defines a new IMA hook namely ima_post_key_create_or_update()
>> to measure asymmetric keys.
>>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> 
> What are the prerequisites for this patch?

I built this patch set on kernel v5.3

I applied the following patch provided by Nayna Jain@IBM and then added 
my changes:

	[PATCH v9 5/8] ima: make process_buffer_measurement() generic

thanks,
  -lakshmi


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

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31 15:27     ` Lakshmi Ramasubramanian
@ 2019-10-31 15:36       ` Sasha Levin
  0 siblings, 0 replies; 22+ messages in thread
From: Sasha Levin @ 2019-10-31 15:36 UTC (permalink / raw)
  To: Lakshmi Ramasubramanian
  Cc: zohar, dhowells, matthewgarrett, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings, prsriva

On Thu, Oct 31, 2019 at 08:27:47AM -0700, Lakshmi Ramasubramanian wrote:
>On 10/31/19 2:10 AM, Sasha Levin wrote:
>
>Hi Sasha,
>
>>On Wed, Oct 30, 2019 at 06:19:02PM -0700, Lakshmi Ramasubramanian wrote:
>>>Asymmetric keys used for verifying file signatures or certificates
>>>are currently not included in the IMA measurement list.
>>>
>>>This patch defines a new IMA hook namely ima_post_key_create_or_update()
>>>to measure asymmetric keys.
>>>
>>>Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>
>>What are the prerequisites for this patch?
>
>I built this patch set on kernel v5.3
>
>I applied the following patch provided by Nayna Jain@IBM and then 
>added my changes:
>
>	[PATCH v9 5/8] ima: make process_buffer_measurement() generic

$ git checkout v5.3
HEAD is now at 4d856f72c10ec Linux 5.3
$ git am ~/incoming/_PATCH_v9_5-8_ima_make_process_buffer_measurement__generic.patch
Applying: ima: make process_buffer_measurement() generic
error: patch failed: security/integrity/ima/ima.h:217
error: security/integrity/ima/ima.h: patch does not apply

What am I missing?

-- 
Thanks,
Sasha

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

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31 15:27       ` Sasha Levin
@ 2019-10-31 15:37         ` Mimi Zohar
  2019-10-31 15:42           ` Lakshmi Ramasubramanian
  0 siblings, 1 reply; 22+ messages in thread
From: Mimi Zohar @ 2019-10-31 15:37 UTC (permalink / raw)
  To: Sasha Levin, Lakshmi Ramasubramanian
  Cc: dhowells, matthewgarrett, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings, prsriva

On Thu, 2019-10-31 at 11:27 -0400, Sasha Levin wrote:
> On Thu, Oct 31, 2019 at 08:08:48AM -0700, Lakshmi Ramasubramanian wrote:
> >On 10/31/19 5:10 AM, Mimi Zohar wrote:
> >
> >>On Wed, 2019-10-30 at 18:19 -0700, Lakshmi Ramasubramanian wrote:
> >>>Asymmetric keys used for verifying file signatures or certificates
> >>>are currently not included in the IMA measurement list.
> >>>
> >>>This patch defines a new IMA hook namely ima_post_key_create_or_update()
> >>>to measure asymmetric keys.
> >>
> >>It's not enough for the kernel to be able to compile the kernel after
> >>applying all the patches in a patch set.  After applying each patch,
> >>the kernel should build properly, otherwise it is not bi-sect safe.
> >>  Refer to "3) Separate your changes" of
> >>"Documentation/process/submitting-patches.rst.
> >
> >I started with kernel version 5.3 for this patch set.
> >I applied Nayna's process_buffer_measurement() patch and then built my 
> >changes on top of that.
> >This patch has no other dependency as far as I know.
> >
> >Are you seeing a build break after applying this patch alone?
> >
> >(PATCH v3 1/9) KEYS: Defined an IMA hook to measure keys on key create 
> >or update
> 
> I couldn't even apply this patch: Nayna's series (v10) doesn't apply on
> top of 5.3 to begin with, and while it does apply on mainline, this
> first patch wouldn't apply on top.

Lakshmi, development is always on top of mainline.  In this case,
 please use 5.4.0-rc3 and apply Nayna's v10 patch set.

Mimi


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

* Re: [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update
  2019-10-31 15:37         ` Mimi Zohar
@ 2019-10-31 15:42           ` Lakshmi Ramasubramanian
  0 siblings, 0 replies; 22+ messages in thread
From: Lakshmi Ramasubramanian @ 2019-10-31 15:42 UTC (permalink / raw)
  To: Mimi Zohar, Sasha Levin
  Cc: dhowells, matthewgarrett, jamorris, linux-kernel,
	linux-integrity, linux-security-module, keyrings, prsriva

On 10/31/19 8:37 AM, Mimi Zohar wrote:

>> I couldn't even apply this patch: Nayna's series (v10) doesn't apply  >> top of 5.3 to begin with, and while it does apply on mainline, 
this>> first patch wouldn't apply on top.
> Lakshmi, development is always on top of mainline.  In this case,
>   please use 5.4.0-rc3 and apply Nayna's v10 patch set.
> 
> Mimi


Thanks for the info Mimi.

I initially started with v5.4, but the kernel I built wouldn't boot on 
my machine :(

I'll update to the latest v5.4 changes and try again.

thanks,
  -lakshmi

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

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31  1:19 [PATCH v3 0/9] KEYS: Measure keys when they are created or updated Lakshmi Ramasubramanian
2019-10-31  1:19 ` [PATCH v3 1/9] KEYS: Defined an IMA hook to measure keys on key create or update Lakshmi Ramasubramanian
2019-10-31  9:10   ` Sasha Levin
2019-10-31 15:27     ` Lakshmi Ramasubramanian
2019-10-31 15:36       ` Sasha Levin
2019-10-31 12:10   ` Mimi Zohar
2019-10-31 15:08     ` Lakshmi Ramasubramanian
2019-10-31 15:27       ` Sasha Levin
2019-10-31 15:37         ` Mimi Zohar
2019-10-31 15:42           ` Lakshmi Ramasubramanian
2019-10-31  1:19 ` [PATCH v3 2/9] KEYS: Defined functions to queue and dequeue keys for measurement Lakshmi Ramasubramanian
2019-10-31 12:10   ` Mimi Zohar
2019-10-31 15:09     ` Lakshmi Ramasubramanian
2019-10-31  1:19 ` [PATCH v3 3/9] KEYS: Added KEYRING_CHECK policy for key measurement Lakshmi Ramasubramanian
2019-10-31 12:10   ` Mimi Zohar
2019-10-31  1:19 ` [PATCH v3 4/9] KEYS: Updated IMA policy functions for handling " Lakshmi Ramasubramanian
2019-10-31 12:10   ` Mimi Zohar
2019-10-31  1:19 ` [PATCH v3 5/9] KEYS: Updated ima_get_action() to return keyrings if specified in the policy Lakshmi Ramasubramanian
2019-10-31  1:19 ` [PATCH v3 6/9] KEYS: Measure key if the IMA policy allows measurement for the given keyring Lakshmi Ramasubramanian
2019-10-31  1:19 ` [PATCH v3 7/9] KEYS: Queue key for measurement if IMA is not yet initialized. Measure queued keys when IMA initialization is completed Lakshmi Ramasubramanian
2019-10-31  1:19 ` [PATCH v3 8/9] KEYS: Added a boolean flag for IMA initialization status Lakshmi Ramasubramanian
2019-10-31  1:19 ` [PATCH v3 9/9] KEYS: Call the IMA hook to measure key when a new key is created or an existing key is updated 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