Critical data structures of security modules need to be measured to
enable an attestation service to verify if the configuration and
policies for the security modules have been setup correctly and
that they haven't been tampered with at runtime. A new IMA policy is
required for handling this measurement.
Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
measure the state and the policy provided by the security modules.
Update ima_match_rules() and ima_validate_rule() to check for
the new func and ima_parse_rule() to handle the new func.
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
---
Documentation/ABI/testing/ima_policy | 9 ++++++++
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_policy.c | 31 ++++++++++++++++++++++++----
4 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index cd572912c593..b7c7fb548c0c 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -30,6 +30,7 @@ Description:
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
[KEXEC_CMDLINE] [KEY_CHECK]
+ [LSM_STATE] [LSM_POLICY]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
@@ -125,3 +126,11 @@ Description:
keys added to .builtin_trusted_keys or .ima keyring:
measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
+
+ Example of measure rule using LSM_STATE to measure LSM state:
+
+ measure func=LSM_STATE template=ima-buf
+
+ Example of measure rule using LSM_POLICY to measure LSM policy:
+
+ measure func=LSM_POLICY template=ima-ng
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 38043074ce5e..1b5f4b2f17d0 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -200,6 +200,8 @@ static inline unsigned int ima_hash_key(u8 *digest)
hook(POLICY_CHECK, policy) \
hook(KEXEC_CMDLINE, kexec_cmdline) \
hook(KEY_CHECK, key) \
+ hook(LSM_STATE, lsm_state) \
+ hook(LSM_POLICY, lsm_policy) \
hook(MAX_CHECK, none)
#define __ima_hook_enumify(ENUM, str) ENUM,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 4f39fb93f278..8c8b4e4a6493 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -176,7 +176,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* 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 | KEY_CHECK
+ * | KEXEC_CMDLINE | KEY_CHECK | LSM_STATE | LSM_POLICY
* mask: contains the permission mask
* fsmagic: hex value
*
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 07f033634b27..a0f5c39d9084 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -442,13 +442,20 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
{
int i;
- if (func == KEY_CHECK) {
- return (rule->flags & IMA_FUNC) && (rule->func == func) &&
- ima_match_keyring(rule, keyring, cred);
- }
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
return false;
+
+ switch (func) {
+ case KEY_CHECK:
+ return ima_match_keyring(rule, keyring, cred);
+ case LSM_STATE:
+ case LSM_POLICY:
+ return true;
+ default:
+ break;
+ }
+
if ((rule->flags & IMA_MASK) &&
(rule->mask != mask && func != POST_SETATTR))
return false;
@@ -1044,6 +1051,18 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
if (ima_rule_contains_lsm_cond(entry))
return false;
+ break;
+ case LSM_STATE:
+ case LSM_POLICY:
+ if (entry->action & ~(MEASURE | DONT_MEASURE))
+ return false;
+
+ if (entry->flags & ~(IMA_FUNC | IMA_PCR))
+ return false;
+
+ if (ima_rule_contains_lsm_cond(entry))
+ return false;
+
break;
default:
return false;
@@ -1176,6 +1195,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->func = KEXEC_CMDLINE;
else if (strcmp(args[0].from, "KEY_CHECK") == 0)
entry->func = KEY_CHECK;
+ else if (strcmp(args[0].from, "LSM_STATE") == 0)
+ entry->func = LSM_STATE;
+ else if (strcmp(args[0].from, "LSM_POLICY") == 0)
+ entry->func = LSM_POLICY;
else
result = -EINVAL;
if (!result)
--
2.27.0
On 2020-07-29 20:47:21, Lakshmi Ramasubramanian wrote:
> Critical data structures of security modules need to be measured to
> enable an attestation service to verify if the configuration and
> policies for the security modules have been setup correctly and
> that they haven't been tampered with at runtime. A new IMA policy is
> required for handling this measurement.
>
> Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
> measure the state and the policy provided by the security modules.
> Update ima_match_rules() and ima_validate_rule() to check for
> the new func and ima_parse_rule() to handle the new func.
>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> ---
> Documentation/ABI/testing/ima_policy | 9 ++++++++
> security/integrity/ima/ima.h | 2 ++
> security/integrity/ima/ima_api.c | 2 +-
> security/integrity/ima/ima_policy.c | 31 ++++++++++++++++++++++++----
> 4 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index cd572912c593..b7c7fb548c0c 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -30,6 +30,7 @@ Description:
> [FIRMWARE_CHECK]
> [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> [KEXEC_CMDLINE] [KEY_CHECK]
> + [LSM_STATE] [LSM_POLICY]
> mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
> [[^]MAY_EXEC]
> fsmagic:= hex value
> @@ -125,3 +126,11 @@ Description:
> keys added to .builtin_trusted_keys or .ima keyring:
>
> measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
> +
> + Example of measure rule using LSM_STATE to measure LSM state:
> +
> + measure func=LSM_STATE template=ima-buf
> +
> + Example of measure rule using LSM_POLICY to measure LSM policy:
> +
> + measure func=LSM_POLICY template=ima-ng
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 38043074ce5e..1b5f4b2f17d0 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -200,6 +200,8 @@ static inline unsigned int ima_hash_key(u8 *digest)
> hook(POLICY_CHECK, policy) \
> hook(KEXEC_CMDLINE, kexec_cmdline) \
> hook(KEY_CHECK, key) \
> + hook(LSM_STATE, lsm_state) \
> + hook(LSM_POLICY, lsm_policy) \
> hook(MAX_CHECK, none)
>
> #define __ima_hook_enumify(ENUM, str) ENUM,
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 4f39fb93f278..8c8b4e4a6493 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -176,7 +176,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
> * 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 | KEY_CHECK
> + * | KEXEC_CMDLINE | KEY_CHECK | LSM_STATE | LSM_POLICY
> * mask: contains the permission mask
> * fsmagic: hex value
> *
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 07f033634b27..a0f5c39d9084 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -442,13 +442,20 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> {
> int i;
>
> - if (func == KEY_CHECK) {
> - return (rule->flags & IMA_FUNC) && (rule->func == func) &&
> - ima_match_keyring(rule, keyring, cred);
> - }
> if ((rule->flags & IMA_FUNC) &&
> (rule->func != func && func != POST_SETATTR))
> return false;
> +
> + switch (func) {
> + case KEY_CHECK:
> + return ima_match_keyring(rule, keyring, cred);
> + case LSM_STATE:
> + case LSM_POLICY:
> + return true;
> + default:
> + break;
> + }
> +
> if ((rule->flags & IMA_MASK) &&
> (rule->mask != mask && func != POST_SETATTR))
> return false;
> @@ -1044,6 +1051,18 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
> if (ima_rule_contains_lsm_cond(entry))
> return false;
>
> + break;
> + case LSM_STATE:
> + case LSM_POLICY:
> + if (entry->action & ~(MEASURE | DONT_MEASURE))
> + return false;
> +
> + if (entry->flags & ~(IMA_FUNC | IMA_PCR))
> + return false;
> +
> + if (ima_rule_contains_lsm_cond(entry))
> + return false;
> +
> break;
> default:
> return false;
> @@ -1176,6 +1195,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> entry->func = KEXEC_CMDLINE;
> else if (strcmp(args[0].from, "KEY_CHECK") == 0)
> entry->func = KEY_CHECK;
> + else if (strcmp(args[0].from, "LSM_STATE") == 0)
> + entry->func = LSM_STATE;
> + else if (strcmp(args[0].from, "LSM_POLICY") == 0)
> + entry->func = LSM_POLICY;
This patch generally looks really good to me with the exception of one
thing...
We should only accept rules with these specified hook functions when an
LSM that has measurement support is enabled. This messes up the ordering
of your patch series but it could be as simple as doing this:
else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
strcmp(args[0].from, "LSM_STATE") == 0)
entry->func = LSM_STATE;
Or you could do something a little more complex, like what's done with
CONFIG_IMA_LSM_RULES. You could create a CONFIG_IMA_MEASURE_LSM option
that's default enabled but depends on CONFIG_SECURITY_SELINUX and then
check for IS_ENABLED(CONFIG_IMA_MEASURE_LSM) in ima_parse_rule().
I'd personally opt for just placing the
IS_ENABLED(CONFIG_SECURITY_SELINUX) check directly into
ima_parse_rule().
Tyler
> else
> result = -EINVAL;
> if (!result)
> --
> 2.27.0
On 7/30/20 8:02 AM, Tyler Hicks wrote:
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index 07f033634b27..a0f5c39d9084 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -442,13 +442,20 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>> {
>> int i;
>>
>> - if (func == KEY_CHECK) {
>> - return (rule->flags & IMA_FUNC) && (rule->func == func) &&
>> - ima_match_keyring(rule, keyring, cred);
>> - }
>> if ((rule->flags & IMA_FUNC) &&
>> (rule->func != func && func != POST_SETATTR))
>> return false;
>> +
>> + switch (func) {
>> + case KEY_CHECK:
>> + return ima_match_keyring(rule, keyring, cred);
>> + case LSM_STATE:
>> + case LSM_POLICY:
>> + return true;
>> + default:
>> + break;
>> + }
>> +
>> if ((rule->flags & IMA_MASK) &&
>> (rule->mask != mask && func != POST_SETATTR))
>> return false;
>> @@ -1044,6 +1051,18 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>> if (ima_rule_contains_lsm_cond(entry))
>> return false;
>>
>> + break;
>> + case LSM_STATE:
>> + case LSM_POLICY:
>> + if (entry->action & ~(MEASURE | DONT_MEASURE))
>> + return false;
>> +
>> + if (entry->flags & ~(IMA_FUNC | IMA_PCR))
>> + return false;
>> +
>> + if (ima_rule_contains_lsm_cond(entry))
>> + return false;
>> +
>> break;
>> default:
>> return false;
>> @@ -1176,6 +1195,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>> entry->func = KEXEC_CMDLINE;
>> else if (strcmp(args[0].from, "KEY_CHECK") == 0)
>> entry->func = KEY_CHECK;
>> + else if (strcmp(args[0].from, "LSM_STATE") == 0)
>> + entry->func = LSM_STATE;
>> + else if (strcmp(args[0].from, "LSM_POLICY") == 0)
>> + entry->func = LSM_POLICY;
>
> This patch generally looks really good to me with the exception of one
> thing...
>
> We should only accept rules with these specified hook functions when an
> LSM that has measurement support is enabled. This messes up the ordering
> of your patch series but it could be as simple as doing this:
>
> else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
> strcmp(args[0].from, "LSM_STATE") == 0)
> entry->func = LSM_STATE;
>
> Or you could do something a little more complex, like what's done with
> CONFIG_IMA_LSM_RULES. You could create a CONFIG_IMA_MEASURE_LSM option
> that's default enabled but depends on CONFIG_SECURITY_SELINUX and then
> check for IS_ENABLED(CONFIG_IMA_MEASURE_LSM) in ima_parse_rule().
>
> I'd personally opt for just placing the
> IS_ENABLED(CONFIG_SECURITY_SELINUX) check directly into
> ima_parse_rule().
>
The LSM hook can be used by any security module (not just SELinux) to
measure their data.
I have implemented measurement in SELinux to illustrate the usage.
Maybe, I can add the check you have suggested for now and when more
security modules start using this IMA policy additional checks can be
added as appropriate.
thanks,
-lakshmi
On 2020-07-30 08:15:34, Lakshmi Ramasubramanian wrote:
> On 7/30/20 8:02 AM, Tyler Hicks wrote:
>
> > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > > index 07f033634b27..a0f5c39d9084 100644
> > > --- a/security/integrity/ima/ima_policy.c
> > > +++ b/security/integrity/ima/ima_policy.c
> > > @@ -442,13 +442,20 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> > > {
> > > int i;
> > > - if (func == KEY_CHECK) {
> > > - return (rule->flags & IMA_FUNC) && (rule->func == func) &&
> > > - ima_match_keyring(rule, keyring, cred);
> > > - }
> > > if ((rule->flags & IMA_FUNC) &&
> > > (rule->func != func && func != POST_SETATTR))
> > > return false;
> > > +
> > > + switch (func) {
> > > + case KEY_CHECK:
> > > + return ima_match_keyring(rule, keyring, cred);
> > > + case LSM_STATE:
> > > + case LSM_POLICY:
> > > + return true;
> > > + default:
> > > + break;
> > > + }
> > > +
> > > if ((rule->flags & IMA_MASK) &&
> > > (rule->mask != mask && func != POST_SETATTR))
> > > return false;
> > > @@ -1044,6 +1051,18 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
> > > if (ima_rule_contains_lsm_cond(entry))
> > > return false;
> > > + break;
> > > + case LSM_STATE:
> > > + case LSM_POLICY:
> > > + if (entry->action & ~(MEASURE | DONT_MEASURE))
> > > + return false;
> > > +
> > > + if (entry->flags & ~(IMA_FUNC | IMA_PCR))
> > > + return false;
> > > +
> > > + if (ima_rule_contains_lsm_cond(entry))
> > > + return false;
> > > +
> > > break;
> > > default:
> > > return false;
> > > @@ -1176,6 +1195,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> > > entry->func = KEXEC_CMDLINE;
> > > else if (strcmp(args[0].from, "KEY_CHECK") == 0)
> > > entry->func = KEY_CHECK;
> > > + else if (strcmp(args[0].from, "LSM_STATE") == 0)
> > > + entry->func = LSM_STATE;
> > > + else if (strcmp(args[0].from, "LSM_POLICY") == 0)
> > > + entry->func = LSM_POLICY;
> >
> > This patch generally looks really good to me with the exception of one
> > thing...
> >
> > We should only accept rules with these specified hook functions when an
> > LSM that has measurement support is enabled. This messes up the ordering
> > of your patch series but it could be as simple as doing this:
> >
> > else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
> > strcmp(args[0].from, "LSM_STATE") == 0)
> > entry->func = LSM_STATE;
> >
> > Or you could do something a little more complex, like what's done with
> > CONFIG_IMA_LSM_RULES. You could create a CONFIG_IMA_MEASURE_LSM option
> > that's default enabled but depends on CONFIG_SECURITY_SELINUX and then
> > check for IS_ENABLED(CONFIG_IMA_MEASURE_LSM) in ima_parse_rule().
> >
> > I'd personally opt for just placing the
> > IS_ENABLED(CONFIG_SECURITY_SELINUX) check directly into
> > ima_parse_rule().
> >
>
> The LSM hook can be used by any security module (not just SELinux) to
> measure their data.
>
> I have implemented measurement in SELinux to illustrate the usage. Maybe, I
> can add the check you have suggested for now and when more security modules
> start using this IMA policy additional checks can be added as appropriate.
Yes, that's what I envision.
The main idea is that there's negative feedback to userspace when IMA
can't possibly do anything with an LSM_STATE/LSM_POLICY rule.
Tyler
>
> thanks,
> -lakshmi
On 7/30/2020 8:02 AM, Tyler Hicks wrote:
> On 2020-07-29 20:47:21, Lakshmi Ramasubramanian wrote:
>> Critical data structures of security modules need to be measured to
>> enable an attestation service to verify if the configuration and
>> policies for the security modules have been setup correctly and
>> that they haven't been tampered with at runtime. A new IMA policy is
>> required for handling this measurement.
>>
>> Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
>> measure the state and the policy provided by the security modules.
If, as you suggest below, this is SELinux specific,
these should be SELINUX_STATE and SELINUX_POLICY.
It makes me very uncomfortable when I see LSM used
in cases where SELinux is required. The LSM is supposed
to be an agnostic interface, so if you need to throw
if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
into the IMA code you're clearly not thinking in terms
of the LSM layer. I have no problem with seeing SELinux
oriented and/or specific code in IMA if that's what you want.
Just don't call it LSM.
>> Update ima_match_rules() and ima_validate_rule() to check for
>> the new func and ima_parse_rule() to handle the new func.
>>
>> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
>> ---
>> Documentation/ABI/testing/ima_policy | 9 ++++++++
>> security/integrity/ima/ima.h | 2 ++
>> security/integrity/ima/ima_api.c | 2 +-
>> security/integrity/ima/ima_policy.c | 31 ++++++++++++++++++++++++----
>> 4 files changed, 39 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
>> index cd572912c593..b7c7fb548c0c 100644
>> --- a/Documentation/ABI/testing/ima_policy
>> +++ b/Documentation/ABI/testing/ima_policy
>> @@ -30,6 +30,7 @@ Description:
>> [FIRMWARE_CHECK]
>> [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
>> [KEXEC_CMDLINE] [KEY_CHECK]
>> + [LSM_STATE] [LSM_POLICY]
>> mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
>> [[^]MAY_EXEC]
>> fsmagic:= hex value
>> @@ -125,3 +126,11 @@ Description:
>> keys added to .builtin_trusted_keys or .ima keyring:
>>
>> measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
>> +
>> + Example of measure rule using LSM_STATE to measure LSM state:
>> +
>> + measure func=LSM_STATE template=ima-buf
>> +
>> + Example of measure rule using LSM_POLICY to measure LSM policy:
>> +
>> + measure func=LSM_POLICY template=ima-ng
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index 38043074ce5e..1b5f4b2f17d0 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -200,6 +200,8 @@ static inline unsigned int ima_hash_key(u8 *digest)
>> hook(POLICY_CHECK, policy) \
>> hook(KEXEC_CMDLINE, kexec_cmdline) \
>> hook(KEY_CHECK, key) \
>> + hook(LSM_STATE, lsm_state) \
>> + hook(LSM_POLICY, lsm_policy) \
>> hook(MAX_CHECK, none)
>>
>> #define __ima_hook_enumify(ENUM, str) ENUM,
>> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
>> index 4f39fb93f278..8c8b4e4a6493 100644
>> --- a/security/integrity/ima/ima_api.c
>> +++ b/security/integrity/ima/ima_api.c
>> @@ -176,7 +176,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
>> * 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 | KEY_CHECK
>> + * | KEXEC_CMDLINE | KEY_CHECK | LSM_STATE | LSM_POLICY
>> * mask: contains the permission mask
>> * fsmagic: hex value
>> *
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index 07f033634b27..a0f5c39d9084 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -442,13 +442,20 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>> {
>> int i;
>>
>> - if (func == KEY_CHECK) {
>> - return (rule->flags & IMA_FUNC) && (rule->func == func) &&
>> - ima_match_keyring(rule, keyring, cred);
>> - }
>> if ((rule->flags & IMA_FUNC) &&
>> (rule->func != func && func != POST_SETATTR))
>> return false;
>> +
>> + switch (func) {
>> + case KEY_CHECK:
>> + return ima_match_keyring(rule, keyring, cred);
>> + case LSM_STATE:
>> + case LSM_POLICY:
>> + return true;
>> + default:
>> + break;
>> + }
>> +
>> if ((rule->flags & IMA_MASK) &&
>> (rule->mask != mask && func != POST_SETATTR))
>> return false;
>> @@ -1044,6 +1051,18 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>> if (ima_rule_contains_lsm_cond(entry))
>> return false;
>>
>> + break;
>> + case LSM_STATE:
>> + case LSM_POLICY:
>> + if (entry->action & ~(MEASURE | DONT_MEASURE))
>> + return false;
>> +
>> + if (entry->flags & ~(IMA_FUNC | IMA_PCR))
>> + return false;
>> +
>> + if (ima_rule_contains_lsm_cond(entry))
>> + return false;
>> +
>> break;
>> default:
>> return false;
>> @@ -1176,6 +1195,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>> entry->func = KEXEC_CMDLINE;
>> else if (strcmp(args[0].from, "KEY_CHECK") == 0)
>> entry->func = KEY_CHECK;
>> + else if (strcmp(args[0].from, "LSM_STATE") == 0)
>> + entry->func = LSM_STATE;
>> + else if (strcmp(args[0].from, "LSM_POLICY") == 0)
>> + entry->func = LSM_POLICY;
> This patch generally looks really good to me with the exception of one
> thing...
>
> We should only accept rules with these specified hook functions when an
> LSM that has measurement support is enabled. This messes up the ordering
> of your patch series but it could be as simple as doing this:
>
> else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
> strcmp(args[0].from, "LSM_STATE") == 0)
> entry->func = LSM_STATE;
>
> Or you could do something a little more complex, like what's done with
> CONFIG_IMA_LSM_RULES. You could create a CONFIG_IMA_MEASURE_LSM option
> that's default enabled but depends on CONFIG_SECURITY_SELINUX and then
> check for IS_ENABLED(CONFIG_IMA_MEASURE_LSM) in ima_parse_rule().
>
> I'd personally opt for just placing the
> IS_ENABLED(CONFIG_SECURITY_SELINUX) check directly into
> ima_parse_rule().
>
> Tyler
>
>> else
>> result = -EINVAL;
>> if (!result)
>> --
>> 2.27.0
On 7/30/20 9:19 AM, Casey Schaufler wrote:
>>> Critical data structures of security modules need to be measured to
>>> enable an attestation service to verify if the configuration and
>>> policies for the security modules have been setup correctly and
>>> that they haven't been tampered with at runtime. A new IMA policy is
>>> required for handling this measurement.
>>>
>>> Define two new IMA policy func namely LSM_STATE and LSM_POLICY to
>>> measure the state and the policy provided by the security modules.
>
> If, as you suggest below, this is SELinux specific,
> these should be SELINUX_STATE and SELINUX_POLICY.
> It makes me very uncomfortable when I see LSM used
> in cases where SELinux is required. The LSM is supposed
> to be an agnostic interface, so if you need to throw
>
> if (IS_ENABLED(CONFIG_SECURITY_SELINUX) &&
>
> into the IMA code you're clearly not thinking in terms
> of the LSM layer. I have no problem with seeing SELinux
> oriented and/or specific code in IMA if that's what you want.
> Just don't call it LSM.
The hook defined in IMA is not SELinux specific - it is generic enough
to be used by any security module to measure their STATE and POLICY.
I have implemented the measurement for SELinux to illustrate the usage.
Tyler's suggestion was to allow this IMA policy only when component(s)
that are using it are also enabled.
-lakshmi