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 | 67 ++++++++++++++++++--
7 files changed, 96 insertions(+), 18 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 066d32797500..cd572912c593 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
@@ -117,3 +120,8 @@ Description:
Example of measure rule using KEY_CHECK to measure all keys:
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 2308adcdeff3..ca895f9a6504 100644
--- a/security/integrity/ima/ima_asymmetric_keys.c
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -46,7 +46,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(payload, plen,
- 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 9b35db2fc777..2272c3255c7d 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;
@@ -668,7 +669,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;
}
@@ -721,7 +722,7 @@ void ima_kexec_cmdline(const void *buf, int size)
{
if (buf && size != 0)
process_buffer_measurement(buf, size, "kexec-cmdline",
- KEXEC_CMDLINE, 0);
+ KEXEC_CMDLINE, 0, NULL);
}
static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 1525a28fd705..d9400585fcda 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -79,6 +79,7 @@ struct ima_rule_entry {
int type; /* audit type */
} lsm[MAX_LSM_RULES];
char *fsname;
+ char *keyrings; /* Measure keys added to these keyrings */
struct ima_template_desc *template;
};
@@ -356,6 +357,55 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
return NOTIFY_OK;
}
+/**
+ * ima_match_keyring - determine whether the keyring matches the measure rule
+ * @rule: a pointer to a rule
+ * @keyring: name of the keyring to match against the measure rule
+ *
+ * If the measure action for KEY_CHECK does not specify keyrings=
+ * option then return true (Measure all keys).
+ * Else, return true if the given keyring name is present in
+ * the keyrings= option. False, otherwise.
+ */
+static bool ima_match_keyring(struct ima_rule_entry *rule,
+ const char *keyring)
+{
+ const char *p;
+
+ /* If "keyrings=" is not specified all keys are measured. */
+ if (!rule->keyrings)
+ return true;
+
+ if (!keyring)
+ return false;
+
+ /*
+ * "keyrings=" is specified in the policy in the format below:
+ * keyrings=.builtin_trusted_keys|.ima|.evm
+ *
+ * Each keyring name in the option is separated by a '|' and
+ * the last keyring name is null terminated.
+ *
+ * The given keyring is considered matched only if
+ * the whole keyring name matched a keyring name specified
+ * in the "keyrings=" option.
+ */
+ p = strstr(rule->keyrings, keyring);
+ if (p) {
+ /*
+ * Found a substring match. Check if the character
+ * at the end of the keyring name is | (keyring name
+ * separator) or is the terminating null character.
+ * If yes, we have a whole string match.
+ */
+ p += strlen(keyring);
+ if (*p == '|' || *p == '\0')
+ return true;
+ }
+
+ return false;
+}
+
/**
* ima_match_rules - determine whether an inode matches the measure rule.
* @rule: a pointer to a rule
@@ -364,18 +414,23 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
* @secid: the secid of the task to be validated
* @func: LIM hook identifier
* @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
+ * @keyring: keyring name to check in policy for KEY_CHECK func
*
* Returns true on rule match, false on failure.
*/
static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
const struct cred *cred, u32 secid,
- enum ima_hooks func, int mask)
+ enum ima_hooks func, int mask,
+ const char *keyring)
{
int i;
if ((func == KEXEC_CMDLINE) || (func == KEY_CHECK)) {
- if ((rule->flags & IMA_FUNC) && (rule->func == func))
+ if ((rule->flags & IMA_FUNC) && (rule->func == func)) {
+ if (func == KEY_CHECK)
+ return ima_match_keyring(rule, keyring);
return true;
+ }
return false;
}
if ((rule->flags & IMA_FUNC) &&
@@ -479,6 +534,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
* @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
* @pcr: set the pcr to extend
* @template_desc: the template that should be used for this rule
+ * @keyring: the keyring name, if given, to be used to check in the policy.
+ * keyring can be NULL if func is anything other than KEY_CHECK.
*
* Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
* conditions.
@@ -489,7 +546,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
*/
int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
enum ima_hooks func, int mask, int flags, int *pcr,
- struct ima_template_desc **template_desc)
+ struct ima_template_desc **template_desc,
+ const char *keyring)
{
struct ima_rule_entry *entry;
int action = 0, actmask = flags | (flags << 1);
@@ -503,7 +561,8 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
if (!(entry->action & actmask))
continue;
- if (!ima_match_rules(entry, inode, cred, secid, func, mask))
+ if (!ima_match_rules(entry, inode, cred, secid, func, mask,
+ keyring))
continue;
action |= entry->flags & IMA_ACTION_FLAGS;
--
2.17.1
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -79,6 +79,7 @@ struct ima_rule_entry {
> int type; /* audit type */
> } lsm[MAX_LSM_RULES];
> char *fsname;
> + char *keyrings; /* Measure keys added to these keyrings */
> struct ima_template_desc *template;
> };
>
> @@ -356,6 +357,55 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> return NOTIFY_OK;
> }
>
> +/**
> + * ima_match_keyring - determine whether the keyring matches the measure rule
> + * @rule: a pointer to a rule
> + * @keyring: name of the keyring to match against the measure rule
> + *
> + * If the measure action for KEY_CHECK does not specify keyrings=
> + * option then return true (Measure all keys).
> + * Else, return true if the given keyring name is present in
> + * the keyrings= option. False, otherwise.
This is suppose to be a comment, not code or pseudo code. Please
refer to the section "Comments" in Documentation/process/coding-
style.rst.
> + */
> +static bool ima_match_keyring(struct ima_rule_entry *rule,
> + const char *keyring)
> +{
> + const char *p;
> +
> + /* If "keyrings=" is not specified all keys are measured. */
> + if (!rule->keyrings)
> + return true;
> +
> + if (!keyring)
> + return false;
> +
> + /*
> + * "keyrings=" is specified in the policy in the format below:
> + * keyrings=.builtin_trusted_keys|.ima|.evm
> + *
> + * Each keyring name in the option is separated by a '|' and
> + * the last keyring name is null terminated.
> + *
> + * The given keyring is considered matched only if
> + * the whole keyring name matched a keyring name specified
> + * in the "keyrings=" option.
> + */
> + p = strstr(rule->keyrings, keyring);
> + if (p) {
> + /*
> + * Found a substring match. Check if the character
> + * at the end of the keyring name is | (keyring name
> + * separator) or is the terminating null character.
> + * If yes, we have a whole string match.
> + */
> + p += strlen(keyring);
> + if (*p == '|' || *p == '\0')
> + return true;
> + }
> +
Using "while strsep()" would simplify this code, removing the need for
such a long comment.
Mimi
> + return false;
> +}
> +
On 11/27/19 10:52 AM, Mimi Zohar wrote:
Hi Mimi,
>> +static bool ima_match_keyring(struct ima_rule_entry *rule,
>> + const char *keyring)
>> +{
>> + /*
>> + * "keyrings=" is specified in the policy in the format below:
>> + * keyrings=.builtin_trusted_keys|.ima|.evm
>> + *
>> + * Each keyring name in the option is separated by a '|' and
>> + * the last keyring name is null terminated.
>> + *
>> + * The given keyring is considered matched only if
>> + * the whole keyring name matched a keyring name specified
>> + * in the "keyrings=" option.
>> + */
>> + p = strstr(rule->keyrings, keyring);
>> + if (p) {
>> + /*
>> + * Found a substring match. Check if the character
>> + * at the end of the keyring name is | (keyring name
>> + * separator) or is the terminating null character.
>> + * If yes, we have a whole string match.
>> + */
>> + p += strlen(keyring);
>> + if (*p == '|' || *p == '\0')
>> + return true;
>> + }
>> +
>
> Using "while strsep()" would simplify this code, removing the need for
> such a long comment.
>
> Mimi
strsep() modifies the source string (replaces the delimiter with '\0'
and also updates the source string pointer). I am not sure it can be
used for our scenario. Please correct me if I am wrong.
Initial IMA policy:
-------------------
measure func=KEY_CHECK
keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist template=ima-buf
Policy after adding a key to .ima keyring:
------------------------------------------
measure func=KEY_CHECK keyrings=.evm|.builtin_trusted_keys|.blacklist
template=ima-buf
Policy after adding a key to a keyring that is not listed in the policy:
------------------------------------------------------------------------
measure func=KEY_CHECK keyrings= template=ima-buf
********************************************************************************
Please see the description from the man page for strsep():
http://man7.org/linux/man-pages/man3/strsep.3.html
char *strsep(char **stringp, const char *delim);
This function finds the first token in the string *stringp, that is
delimited by one of the bytes in the string delim. This token is
terminated by overwriting the delimiter with a null byte ('\0'), and
*stringp is updated to point past the token.
thanks,
-lakshmi
On Wed, 2019-11-27 at 16:44 -0800, Lakshmi Ramasubramanian wrote:
> On 11/27/19 10:52 AM, Mimi Zohar wrote:
>
> Hi Mimi,
>
> >> +static bool ima_match_keyring(struct ima_rule_entry *rule,
> >> + const char *keyring)
> >> +{
> >> + /*
> >> + * "keyrings=" is specified in the policy in the format below:
> >> + * keyrings=.builtin_trusted_keys|.ima|.evm
> >> + *
> >> + * Each keyring name in the option is separated by a '|' and
> >> + * the last keyring name is null terminated.
> >> + *
> >> + * The given keyring is considered matched only if
> >> + * the whole keyring name matched a keyring name specified
> >> + * in the "keyrings=" option.
> >> + */
> >> + p = strstr(rule->keyrings, keyring);
> >> + if (p) {
> >> + /*
> >> + * Found a substring match. Check if the character
> >> + * at the end of the keyring name is | (keyring name
> >> + * separator) or is the terminating null character.
> >> + * If yes, we have a whole string match.
> >> + */
> >> + p += strlen(keyring);
> >> + if (*p == '|' || *p == '\0')
> >> + return true;
This code checks that the keyring name isn't suffixed, but not
prefixed.
> >> + }
> >> +
> >
> > Using "while strsep()" would simplify this code, removing the need for
> > such a long comment.
> >
> > Mimi
>
> strsep() modifies the source string (replaces the delimiter with '\0'
> and also updates the source string pointer). I am not sure it can be
> used for our scenario. Please correct me if I am wrong.
>
> Initial IMA policy:
> -------------------
> measure func=KEY_CHECK
> keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist template=ima-buf
>
> Policy after adding a key to .ima keyring:
> ------------------------------------------
> measure func=KEY_CHECK keyrings=.evm|.builtin_trusted_keys|.blacklist
> template=ima-buf
>
> Policy after adding a key to a keyring that is not listed in the policy:
> ------------------------------------------------------------------------
> measure func=KEY_CHECK keyrings= template=ima-buf
>
> ********************************************************************************
>
> Please see the description from the man page for strsep():
>
> http://man7.org/linux/man-pages/man3/strsep.3.html
>
> char *strsep(char **stringp, const char *delim);
>
> This function finds the first token in the string *stringp, that is
> delimited by one of the bytes in the string delim. This token is
> terminated by overwriting the delimiter with a null byte ('\0'), and
> *stringp is updated to point past the token.
Yes, you would have to make a copy of the string before using
strsep(). You could always use kstrdup(), remembering to free it, or
allocate the memory just once, and then just use memcpy.
Mimi
Hi Lakshmi,
On Tue, 2019-11-26 at 17:56 -0800, Lakshmi Ramasubramanian wrote:
> Limit measuring keys to those keys being loaded onto a given set of
> keyrings only.
>
> This patch defines a new IMA policy option namely "keyrings=" that
> can be used to specify a set of keyrings. If this option is specified
> in the policy for "measure func=KEY_CHECK" then only the keys
> loaded onto a keyring given in the "keyrings=" option are measured.
>
> Added a new parameter namely "keyring" (name of the keyring) to
> process_buffer_measurement(). The keyring name is passed to
> ima_get_action() to determine the required action.
> ima_match_rules() is updated to check keyring in the policy, if
> specified, for KEY_CHECK function.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
A keyring can be created by any user with any keyring name, other than
ones dot prefixed, which are limited to the trusted builtin keyrings.
With a policy of "func=KEY_CHECK template=ima-buf keyrings=foo", for
example, keys loaded onto any keyring named "foo" will be measured.
For files, the IMA policy may be constrained to a particular uid/gid.
An additional method of identifying or constraining keyring names
needs to be defined.
Mimi
On 12/3/2019 4:25 AM, Mimi Zohar wrote:
Hi Mimi,
> Hi Lakshmi,
>
> A keyring can be created by any user with any keyring name, other than
> ones dot prefixed, which are limited to the trusted builtin keyrings.
> With a policy of "func=KEY_CHECK template=ima-buf keyrings=foo", for
> example, keys loaded onto any keyring named "foo" will be measured.
> For files, the IMA policy may be constrained to a particular uid/gid.
> An additional method of identifying or constraining keyring names
> needs to be defined.
>
I agree - this is a good idea.
Do you think this can be added as a separate patch set - enhancement to
the current one, or should this be addressed in the current set?
I was just about to send an update today that addresses your latest
comments. Will hold it if you think the above change should be included
now. Please let me know.
thanks,
-lakshmi
On Tue, 2019-12-03 at 08:13 -0800, Lakshmi Ramasubramanian wrote:
> On 12/3/2019 4:25 AM, Mimi Zohar wrote:
>
> Hi Mimi,
>
> > Hi Lakshmi,
> >
> > A keyring can be created by any user with any keyring name, other than
> > ones dot prefixed, which are limited to the trusted builtin keyrings.
> > With a policy of "func=KEY_CHECK template=ima-buf keyrings=foo", for
> > example, keys loaded onto any keyring named "foo" will be measured.
> > For files, the IMA policy may be constrained to a particular uid/gid.
> > An additional method of identifying or constraining keyring names
> > needs to be defined.
> >
>
> I agree - this is a good idea.
>
> Do you think this can be added as a separate patch set - enhancement to
> the current one, or should this be addressed in the current set?
>
> I was just about to send an update today that addresses your latest
> comments. Will hold it if you think the above change should be included
> now. Please let me know.
I'm hoping that it won't be all that difficult to implement and could
be included in this patch set as an additional patch.
Mimi
Hi Mimi,
On 12/3/2019 4:25 AM, Mimi Zohar wrote:
>
> A keyring can be created by any user with any keyring name, other than
> ones dot prefixed, which are limited to the trusted builtin keyrings.
> With a policy of "func=KEY_CHECK template=ima-buf keyrings=foo", for
> example, keys loaded onto any keyring named "foo" will be measured.
> For files, the IMA policy may be constrained to a particular uid/gid.
> An additional method of identifying or constraining keyring names
> needs to be defined.
>
> Mimi
>
Are you expecting a functionality like the following?
=> Measure keys added to keyring "foo", but exclude certain keys
(based on some key identifier)
Sample policy might look like below:
action=MEASURE func=KEY_CHECK keyrings=foo|bar
action=DONT_MEASURE key_magic=blah
So a key with key_magic "blah" will not be measured even though it is
added to the keyring "foo". But any other key added to "foo" will be
measured.
What would the constraining field in the key may be - Can it be SHA256
hash of the public key? Or, some other unique identifier?
Could you please clarify?
thanks,
-lakshmi
On Tue, 2019-12-03 at 11:45 -0800, Lakshmi Ramasubramanian wrote:
> Hi Mimi,
>
> On 12/3/2019 4:25 AM, Mimi Zohar wrote:
>
> >
> > A keyring can be created by any user with any keyring name, other than
> > ones dot prefixed, which are limited to the trusted builtin keyrings.
> > With a policy of "func=KEY_CHECK template=ima-buf keyrings=foo", for
> > example, keys loaded onto any keyring named "foo" will be measured.
> > For files, the IMA policy may be constrained to a particular uid/gid.
> > An additional method of identifying or constraining keyring names
> > needs to be defined.
> >
> > Mimi
> >
>
> Are you expecting a functionality like the following?
>
> => Measure keys added to keyring "foo", but exclude certain keys
> (based on some key identifier)
>
> Sample policy might look like below:
>
> action=MEASURE func=KEY_CHECK keyrings=foo|bar
> action=DONT_MEASURE key_magic=blah
>
> So a key with key_magic "blah" will not be measured even though it is
> added to the keyring "foo". But any other key added to "foo" will be
> measured.
>
> What would the constraining field in the key may be - Can it be SHA256
> hash of the public key? Or, some other unique identifier?
>
> Could you please clarify?
Suppose both root and uid 1000 define a keyring named "foo". The
current "keyrings=foo" will measure all keys added to either keyring
named "foo". There needs to be a way to limit measuring keys to a
particular keyring named "foo".
Mimi
On 12/3/2019 12:06 PM, Mimi Zohar wrote:
> Suppose both root and uid 1000 define a keyring named "foo". The
> current "keyrings=foo" will measure all keys added to either keyring
> named "foo". There needs to be a way to limit measuring keys to a
> particular keyring named "foo".
>
> Mimi
Thanks for clarifying.
Suppose two different non-root users create keyring with the same name
"foo" and, say, both are measured, how would we know which keyring
measurement belongs to which user?
Wouldn't it be sufficient to include only keyrings created by "root"
(UID value 0) in the key measurement? This will include all the builtin
trusted keyrings (such as .builtin_trusted_keys,
.secondary_trusted_keys, .ima, .evm, etc.).
What would be the use case for including keyrings created by non-root
users in key measurement?
Also, since the UID for non-root users can be any integer value (greater
than 0), can an an administrator craft a generic IMA policy that would
be applicable to all clients in an enterprise?
thanks,
-lakshmi
[Cc'ing Mat Martineau]
On Tue, 2019-12-03 at 15:37 -0800, Lakshmi Ramasubramanian wrote:
> On 12/3/2019 12:06 PM, Mimi Zohar wrote:
>
> > Suppose both root and uid 1000 define a keyring named "foo". The
> > current "keyrings=foo" will measure all keys added to either keyring
> > named "foo". There needs to be a way to limit measuring keys to a
> > particular keyring named "foo".
> >
> > Mimi
>
> Thanks for clarifying.
>
> Suppose two different non-root users create keyring with the same name
> "foo" and, say, both are measured, how would we know which keyring
> measurement belongs to which user?
>
> Wouldn't it be sufficient to include only keyrings created by "root"
> (UID value 0) in the key measurement? This will include all the builtin
> trusted keyrings (such as .builtin_trusted_keys,
> .secondary_trusted_keys, .ima, .evm, etc.).
>
> What would be the use case for including keyrings created by non-root
> users in key measurement?
>
> Also, since the UID for non-root users can be any integer value (greater
> than 0), can an an administrator craft a generic IMA policy that would
> be applicable to all clients in an enterprise?
The integrity subsystem, and other concepts upstreamed to support it,
are being used by different people/companies in different ways. I
know some of the ways, but not all, as how it is being used. For
example, Mat Martineau gave an LSS2019-NA talk titled "Using and
Implementing Keyring Restrictions for Userspace". I don't know if he
would be interested in measuring keys on these restricted userspace
keyrings, but before we limit how a new feature works, we should at
least look to see if that limitation is really necessary.
Mimi
On 12/4/19 3:16 AM, Mimi Zohar wrote:
Hi Mimi,
>
> The integrity subsystem, and other concepts upstreamed to support it,
> are being used by different people/companies in different ways. I
> know some of the ways, but not all, as how it is being used. For
> example, Mat Martineau gave an LSS2019-NA talk titled "Using and
> Implementing Keyring Restrictions for Userspace". I don't know if he
> would be interested in measuring keys on these restricted userspace
> keyrings, but before we limit how a new feature works, we should at
> least look to see if that limitation is really necessary.
>
> Mimi
I have updated the patch per your suggestion - if uid is specified in
the policy for key measurement, then it is checked against the current
user's credential.
Please review the updated patch set (v10).
thanks,
-lakshmi
On Wed, 4 Dec 2019, Mimi Zohar wrote:
> [Cc'ing Mat Martineau]
>
> On Tue, 2019-12-03 at 15:37 -0800, Lakshmi Ramasubramanian wrote:
>> On 12/3/2019 12:06 PM, Mimi Zohar wrote:
>>
>>> Suppose both root and uid 1000 define a keyring named "foo". The
>>> current "keyrings=foo" will measure all keys added to either keyring
>>> named "foo". There needs to be a way to limit measuring keys to a
>>> particular keyring named "foo".
>>>
>>> Mimi
>>
>> Thanks for clarifying.
>>
>> Suppose two different non-root users create keyring with the same name
>> "foo" and, say, both are measured, how would we know which keyring
>> measurement belongs to which user?
>>
>> Wouldn't it be sufficient to include only keyrings created by "root"
>> (UID value 0) in the key measurement? This will include all the builtin
>> trusted keyrings (such as .builtin_trusted_keys,
>> .secondary_trusted_keys, .ima, .evm, etc.).
>>
>> What would be the use case for including keyrings created by non-root
>> users in key measurement?
>>
>> Also, since the UID for non-root users can be any integer value (greater
>> than 0), can an an administrator craft a generic IMA policy that would
>> be applicable to all clients in an enterprise?
>
> The integrity subsystem, and other concepts upstreamed to support it,
> are being used by different people/companies in different ways. I
> know some of the ways, but not all, as how it is being used. For
> example, Mat Martineau gave an LSS2019-NA talk titled "Using and
> Implementing Keyring Restrictions for Userspace". I don't know if he
> would be interested in measuring keys on these restricted userspace
> keyrings, but before we limit how a new feature works, we should at
> least look to see if that limitation is really necessary.
The use cases I'm most familiar with could have a use for key measurement
for something like enterprise Wi-Fi root certificates. I'm not sure of the
best way to uniquely identify a key to measure in that scenario, it could
be anchored in various ways (process, session, thread, or user keyrings,
for example) and may be owned by a non-root user. As Lakshmi noted above,
key names are not unique, and I'll add that namespace considerations may
come in to play too.
Keys (including keyrings like .builtin_trusted_keys, .ima, etc) can be
linked to multiple keyrings, maybe you could create a system-level
.ima_measured keyring. You could measure keys that are accessible from
that keyring, and opt in more keys for measurement by linking them to
.ima_measured or a keyring nested within .ima_measured.
--
Mat Martineau
Intel