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:
v7:
=> Removed CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS option and used
CONFIG_KEYS instead for ima_asymmetric_keys.c
=> Added the patches related to "keyrings=" option support to
this patch set.
v6:
=> Rebased the changes to v5.4-rc7
=> Renamed KEYRING_CHECK to KEY_CHECK per Mimi's suggestion.
=> Excluded the patches that add support for limiting key
measurement to specific keyrings ("keyrings=" option
for "measure func=KEY_CHECK" in the IMA policy).
Also, excluded the patches that add support for deferred
processing of keys (queue support).
These patches will be added in separate patch sets later.
v5:
=> Reorganized the patches to add measurement of keys through
the IMA hook without any queuing and then added queuing support.
=> Updated the queuing functions to minimize code executed inside mutex.
=> Process queued keys after custom IMA policies have been applied.
v4:
=> Rebased the changes to v5.4-rc3
=> Applied the following dependent patch set first
and then added new changes.
https://lore.kernel.org/linux-integrity/[email protected]
=> Refactored the patch set to separate out changes related to
func KEYRING_CHECK and options keyrings into different patches.
=> Moved the functions to queue and dequeue keys for measurement
from ima_queue.c to a new file ima_asymmetric_keys.c.
=> Added a new config namely CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
to compile ima_asymmetric_keys.c
v3:
=> Added KEYRING_CHECK for measuring keys. This can optionally specify
keyrings to measure.
=> Updated ima_get_action() and related functions to return
the keyrings if specified in the policy.
=> process_buffer_measurement() function is updated to take keyring
as a parameter. The key will be measured if the policy includes
the keyring in the list of measured keyrings. If the policy does not
specify any keyrings then all keys are measured.
v2:
=> Per suggestion from Mimi reordered the patch set to first
enable measuring keys added or updated in the system.
And, then scope the measurement to keys added to
builtin_trusted_keys keyring through ima policy.
=> Removed security_key_create_or_update function and instead
call ima hook, to measure the key, directly from
key_create_or_update function.
v1:
=> LSM function for key_create_or_update. It calls ima.
=> Added ima hook for measuring keys
=> ima measures keys based on ima policy.
v0:
=> Added LSM hook for key_create_or_update.
=> Measure keys added to builtin or secondary trusted keys keyring.
Lakshmi Ramasubramanian (5):
IMA: Add KEY_CHECK func to measure keys
IMA: Define an IMA hook to measure keys
KEYS: Call the IMA hook to measure keys
IMA: Add support to limit measuring keys
IMA: Read keyrings= option from the IMA policy
Documentation/ABI/testing/ima_policy | 17 ++++-
include/linux/ima.h | 13 ++++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 9 ++-
security/integrity/ima/ima_api.c | 8 ++-
security/integrity/ima/ima_appraise.c | 4 +-
security/integrity/ima/ima_asymmetric_keys.c | 57 ++++++++++++++++
security/integrity/ima/ima_main.c | 9 +--
security/integrity/ima/ima_policy.c | 70 ++++++++++++++++++--
security/keys/key.c | 7 ++
10 files changed, 175 insertions(+), 20 deletions(-)
create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
--
2.17.1
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.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
---
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 | 37 +++++++++++++++++---
7 files changed, 66 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..76da4f17bc79 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,25 @@ 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)
+{
+ if ((keyring == NULL) || (rule->keyrings == NULL))
+ return true;
+ else
+ return (strstr(rule->keyrings, keyring) != NULL);
+}
+
/**
* ima_match_rules - determine whether an inode matches the measure rule.
* @rule: a pointer to a rule
@@ -364,18 +384,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 +504,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 +516,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 +531,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
Read "keyrings=" option, if specified in the IMA policy, and store in
the list of IMA rules when the configured IMA policy is read.
This patch defines a new policy token enum namely Opt_keyrings
and an option flag IMA_KEYRINGS for reading "keyrings=" option
from the IMA policy.
Updated ima_parse_rule() to parse "keyrings=" option in the policy.
Updated ima_policy_show() to display "keyrings=" option.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
---
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 76da4f17bc79..577a51a548fb 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 */
@@ -795,7 +796,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 = {
@@ -831,6 +833,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}
};
@@ -1080,6 +1083,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);
@@ -1455,6 +1475,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
Call the IMA hook from key_create_or_update function to measure
the key when a new key is created or an existing key is updated.
This patch adds the call to the IMA hook from key_create_or_update
function to measure the key on key create or update.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
---
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..0af88b781ab6 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_KEYS
+extern void ima_post_key_create_or_update(struct key *keyring,
+ struct key *key,
+ unsigned long flags, bool create);
+#endif
+
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
@@ -101,6 +107,13 @@ static inline void ima_add_kexec_buffer(struct kimage *image)
{}
#endif
+#ifndef CONFIG_KEYS
+static inline void ima_post_key_create_or_update(struct key *keyring,
+ struct key *key,
+ unsigned long flags,
+ bool create) {}
+#endif
+
#ifdef CONFIG_IMA_APPRAISE
extern bool is_ima_appraise_enabled(void);
extern void ima_inode_post_setattr(struct dentry *dentry);
diff --git a/security/keys/key.c b/security/keys/key.c
index 764f4c57913e..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
On Wed, 2019-11-13 at 19:12 -0800, Lakshmi Ramasubramanian wrote:
> +/**
> + * 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)
> +{
> + if ((keyring == NULL) || (rule->keyrings == NULL)
> + return true;
If the policy requires matching a specific keyring, then the "keyring"
needs to match. The logic, here, isn't quite right.
> + else
> + return (strstr(rule->keyrings, keyring) != NULL);
if (rule->keyrings) {
if (!keyring)
return false;
return (strstr(rule->keyrings, keyring) != NULL);
}
return true;
Keyrings may be created by userspace with any name (e.g. foo, foobar,
...). A keyring name might be a subset of another keyring name. For
example, with the policy "keyrings=foobar", keys being loaded on "foo"
would also be measured. Using strstr() will not achieve what is
needed.
Mimi
> +}
> +
> /**
> * ima_match_rules - determine whether an inode matches the measure rule.
> * @rule: a pointer to a rule
> @@ -364,18 +384,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) &&
On Wed, 2019-11-13 at 19:12 -0800, Lakshmi Ramasubramanian wrote:
> Call the IMA hook from key_create_or_update function to measure
> the key when a new key is created or an existing key is updated.
>
> This patch adds the call to the IMA hook from key_create_or_update
> function to measure the key on key create or update.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
No need to Cc David Howells on the entire patch set. Just Cc him,
here, after your tag.
With this patch, keys are now being measured. With the boot command
line, we can verify the measurement entry against /proc/cmdline. How
can the key measurement entry be verified? Please include that
information, here, in this patch description.
Also, can the key data, now included in the measurement list, be used
to verify signatures in the ima-sig or ima-modsig templates? Is there
a way of correlating a signature with a key? Perhaps include a
kselftest as an example.
Mimi
> ---
> 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..0af88b781ab6 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_KEYS
> +extern void ima_post_key_create_or_update(struct key *keyring,
> + struct key *key,
> + unsigned long flags, bool create);
> +#endif
> +
> #ifdef CONFIG_IMA_KEXEC
> extern void ima_add_kexec_buffer(struct kimage *image);
> #endif
> @@ -101,6 +107,13 @@ static inline void ima_add_kexec_buffer(struct kimage *image)
> {}
> #endif
>
> +#ifndef CONFIG_KEYS
> +static inline void ima_post_key_create_or_update(struct key *keyring,
> + struct key *key,
> + unsigned long flags,
> + bool create) {}
> +#endif
> +
> #ifdef CONFIG_IMA_APPRAISE
> extern bool is_ima_appraise_enabled(void);
> extern void ima_inode_post_setattr(struct dentry *dentry);
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 764f4c57913e..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);
On 11/14/2019 6:37 AM, Mimi Zohar wrote:
> Keyrings may be created by userspace with any name (e.g. foo, foobar,
> ...). A keyring name might be a subset of another keyring name. For
> example, with the policy "keyrings=foobar", keys being loaded on "foo"
> would also be measured. Using strstr() will not achieve what is
> needed.
>
> Mimi
Very good catch - I missed that :(
Will fix and send an update.
thanks,
-lakshmi
On 11/14/2019 6:54 AM, Mimi Zohar wrote:
> No need to Cc David Howells on the entire patch set. Just Cc him,
> here, after your tag.
ok
> With this patch, keys are now being measured. With the boot command
> line, we can verify the measurement entry against /proc/cmdline. How
> can the key measurement entry be verified? Please include that
> information, here, in this patch description.
Glad you could verify measurement of keys. Thanks a lot for trying it.
Will add information on testing\validating the feature.
> Also, can the key data, now included in the measurement list, be used
> to verify signatures in the ima-sig or ima-modsig templates? Is there
> a way of correlating a signature with a key? Perhaps include a
> kselftest as an example.
>
> Mimi
I am not familiar with kselftest. Will take a look and see if it'd be
possible to correlate a signature with a key.
thanks,
-lakshmi
On Thu, 2019-11-14 at 10:24 -0800, Lakshmi Ramasubramanian wrote:
> On 11/14/2019 6:54 AM, Mimi Zohar wrote:
> > With this patch, keys are now being measured. With the boot command
> > line, we can verify the measurement entry against /proc/cmdline. How
> > can the key measurement entry be verified? Please include that
> > information, here, in this patch description.
>
> Glad you could verify measurement of keys. Thanks a lot for trying it.
>
> Will add information on testing\validating the feature.
Thank you.
>
> > Also, can the key data, now included in the measurement list, be used
> > to verify signatures in the ima-sig or ima-modsig templates? Is there
> > a way of correlating a signature with a key? Perhaps include a
> > kselftest as an example.
>
> I am not familiar with kselftest. Will take a look and see if it'd be
> possible to correlate a signature with a key.
I'd like the measurement list to be self contained, allowing a
regression test, for example, to walk the measurement list, verifying
the file signatures stored in the measurement list based on the key
measurement(s).
It isn't so much about Kselftest, but implementing a regression test
(eg. Kselftest, LTP, ima-evm-utils, ...) as a PoC, in order to know
that the key measurement contains everything needed to identify the
key (eg. keyid, certificate serial number, ...) and verify file
signatures.
Mimi