IMA does not measure duplicate buffer data since TPM extend is a very
expensive operation. However, in some cases for integrity critical
data, the measurement of duplicate data is necessary to accurately
determine the current state of the system. Eg, SELinux state changing
from 'audit', to 'enforcing', and back to 'audit' again. In this
example, currently, IMA will not measure the last state change to
'audit'. This limits the ability of attestation services to accurately
determine the current state of the integrity critical data on the
system.
This series addresses this gap by providing the ability to measure
duplicate entries for integrity critical data, driven by policy.
This series is based on the following repo/branch/commit:
repo: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
branch: next-integrity-testing
commit b3f82afc1041 ("IMA: Measure kernel version in early boot")
Tushar Sugandhi (3):
IMA: add policy condition to measure duplicate critical data
IMA: update functions to read allow_dup policy condition
IMA: add support to measure duplicate buffer for critical data hook
Documentation/ABI/testing/ima_policy | 4 +++-
security/integrity/ima/ima.h | 8 +++----
security/integrity/ima/ima_api.c | 15 +++++++------
security/integrity/ima/ima_appraise.c | 2 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 9 ++++----
security/integrity/ima/ima_policy.c | 31 ++++++++++++++++++++++++---
security/integrity/ima/ima_queue.c | 5 +++--
8 files changed, 54 insertions(+), 22 deletions(-)
--
2.17.1
IMA needs to support duplicate measurements of integrity
critical data to accurately determine the current state of that data
on the system. Further, since measurement of duplicate data is not
required for all the use cases, it needs to be policy driven.
Define "allow_dup", a new IMA policy condition, for the IMA func
CRITICAL_DATA to allow duplicate buffer measurement of integrity
critical data.
Limit the ability to measure duplicate buffer data when action is
"measure" and func is CRITICAL_DATA.
Signed-off-by: Tushar Sugandhi <[email protected]>
---
Documentation/ABI/testing/ima_policy | 4 +++-
security/integrity/ima/ima_policy.c | 24 ++++++++++++++++++++++--
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index bc8e1cbe5e61..9598287e3bbf 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -27,7 +27,7 @@ Description:
lsm: [[subj_user=] [subj_role=] [subj_type=]
[obj_user=] [obj_role=] [obj_type=]]
option: [[appraise_type=]] [template=] [permit_directio]
- [appraise_flag=] [keyrings=]
+ [appraise_flag=] [keyrings=] [allow_dup]
base:
func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK]MODULE_CHECK]
[FIRMWARE_CHECK]
@@ -55,6 +55,8 @@ Description:
label:= [selinux]|[kernel_info]|[data_label]
data_label:= a unique string used for grouping and limiting critical data.
For example, "selinux" to measure critical data for SELinux.
+ allow_dup allows measurement of duplicate data. Only valid
+ when action is "measure" and func is CRITICAL_DATA.
default policy:
# PROC_SUPER_MAGIC
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 9b45d064a87d..b89eb768dd05 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -35,6 +35,7 @@
#define IMA_FSNAME 0x0200
#define IMA_KEYRINGS 0x0400
#define IMA_LABEL 0x0800
+#define IMA_ALLOW_DUP 0x1000
#define UNKNOWN 0
#define MEASURE 0x0001 /* same as IMA_MEASURE */
@@ -87,6 +88,7 @@ struct ima_rule_entry {
char *fsname;
struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */
struct ima_rule_opt_list *label; /* Measure data grouped under this label */
+ bool allow_dup; /* Allow duplicate buffer entry measurement */
struct ima_template_desc *template;
};
@@ -942,7 +944,7 @@ enum {
Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
Opt_appraise_type, Opt_appraise_flag,
Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
- Opt_label, Opt_err
+ Opt_label, Opt_allow_dup, Opt_err
};
static const match_table_t policy_tokens = {
@@ -980,6 +982,7 @@ static const match_table_t policy_tokens = {
{Opt_template, "template=%s"},
{Opt_keyrings, "keyrings=%s"},
{Opt_label, "label=%s"},
+ {Opt_allow_dup, "allow_dup"},
{Opt_err, NULL}
};
@@ -1148,7 +1151,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
return false;
if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
- IMA_LABEL))
+ IMA_LABEL | IMA_ALLOW_DUP))
return false;
if (ima_rule_contains_lsm_cond(entry))
@@ -1184,6 +1187,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->uid_op = &uid_eq;
entry->fowner_op = &uid_eq;
entry->action = UNKNOWN;
+ entry->allow_dup = false;
while ((p = strsep(&rule, " \t")) != NULL) {
substring_t args[MAX_OPT_ARGS];
int token;
@@ -1375,6 +1379,19 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->flags |= IMA_LABEL;
break;
+ case Opt_allow_dup:
+ ima_log_string(ab, "allow_dup", NULL);
+
+ if ((entry->func != CRITICAL_DATA) ||
+ (entry->action != MEASURE)) {
+ result = -EINVAL;
+ break;
+ }
+
+ entry->allow_dup = true;
+
+ entry->flags |= IMA_ALLOW_DUP;
+ break;
case Opt_fsuuid:
ima_log_string(ab, "fsuuid", args[0].from);
@@ -1761,6 +1778,9 @@ int ima_policy_show(struct seq_file *m, void *v)
seq_puts(m, " ");
}
+ if (entry->flags & IMA_ALLOW_DUP)
+ seq_puts(m, "allow_dup");
+
if (entry->flags & IMA_PCR) {
snprintf(tbuf, sizeof(tbuf), "%d", entry->pcr);
seq_printf(m, pt(Opt_pcr), tbuf);
--
2.17.1
Hi Tushar,
On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
> IMA does not measure duplicate buffer data since TPM extend is a very
> expensive operation. However, in some cases for integrity critical
> data, the measurement of duplicate data is necessary to accurately
> determine the current state of the system. Eg, SELinux state changing
> from 'audit', to 'enforcing', and back to 'audit' again. In this
> example, currently, IMA will not measure the last state change to
> 'audit'. This limits the ability of attestation services to accurately
> determine the current state of the integrity critical data on the
> system.
>
> This series addresses this gap by providing the ability to measure
> duplicate entries for integrity critical data, driven by policy.
The same reason for re-measuring buffer data is equally applicable to
files. In both cases, the file or the buffer isn't re-measured if it
already exists in the htable. Please don't limit this patch set to
just buffer data.
thanks,
Mimi
Hi Tushar,
On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
> IMA needs to support duplicate measurements of integrity
> critical data to accurately determine the current state of that data
> on the system. Further, since measurement of duplicate data is not
> required for all the use cases, it needs to be policy driven.
>
> Define "allow_dup", a new IMA policy condition, for the IMA func
> CRITICAL_DATA to allow duplicate buffer measurement of integrity
> critical data.
>
> Limit the ability to measure duplicate buffer data when action is
> "measure" and func is CRITICAL_DATA.
Why?!
>
> Signed-off-by: Tushar Sugandhi <[email protected]>
> ---
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 9b45d064a87d..b89eb768dd05 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -35,6 +35,7 @@
> #define IMA_FSNAME 0x0200
> #define IMA_KEYRINGS 0x0400
> #define IMA_LABEL 0x0800
> +#define IMA_ALLOW_DUP 0x1000
>
> #define UNKNOWN 0
> #define MEASURE 0x0001 /* same as IMA_MEASURE */
> @@ -87,6 +88,7 @@ struct ima_rule_entry {
> char *fsname;
> struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */
> struct ima_rule_opt_list *label; /* Measure data grouped under this label */
Defining a new boolean entry shouldn't be necessary. The other
boolean values are just stored in "flags".
> struct ima_template_desc *template;
> };
thanks,
Mimi
Hi Tushar,
On Mon, 2021-02-08 at 15:22 -0500, Mimi Zohar wrote:
> On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
> > IMA does not measure duplicate buffer data since TPM extend is a very
> > expensive operation. However, in some cases for integrity critical
> > data, the measurement of duplicate data is necessary to accurately
> > determine the current state of the system. Eg, SELinux state changing
> > from 'audit', to 'enforcing', and back to 'audit' again. In this
> > example, currently, IMA will not measure the last state change to
> > 'audit'. This limits the ability of attestation services to accurately
> > determine the current state of the integrity critical data on the
> > system.
> >
> > This series addresses this gap by providing the ability to measure
> > duplicate entries for integrity critical data, driven by policy.
>
> The same reason for re-measuring buffer data is equally applicable to
> files. In both cases, the file or the buffer isn't re-measured if it
> already exists in the htable. Please don't limit this patch set to
> just buffer data.
Instead of making the change on a per measurement rule basis, disabling
"htable" would be the simplest way of forcing re-measurements. All
that would be needed is a new Kconfig (e.g. CONFIG_IMA_DISABLE_HTABLE)
and the associated test in ima_add_template_entry().
thanks,
Mimi
On 2021-02-08 12:45 p.m., Mimi Zohar wrote:
> Hi Tushar,
>
> On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
>> IMA needs to support duplicate measurements of integrity
>> critical data to accurately determine the current state of that data
>> on the system. Further, since measurement of duplicate data is not
>> required for all the use cases, it needs to be policy driven.
>>
>> Define "allow_dup", a new IMA policy condition, for the IMA func
>> CRITICAL_DATA to allow duplicate buffer measurement of integrity
>> critical data.
>>
>> Limit the ability to measure duplicate buffer data when action is
>> "measure" and func is CRITICAL_DATA.
>
> Why?!
>
I wasn't sure if it would break any use-case by supporting this for all
the files / buffers. That's why I only wanted to address the scenario
that we discussed in the last series (critical data measurement).
But as you suggested in this series' cover letter response, I am happy
to extend it to other scenarios (by disabling "htable" using new Kconfig
(e.g. CONFIG_IMA_DISABLE_HTABLE)
>>
>> Signed-off-by: Tushar Sugandhi <[email protected]>
>> ---
>>
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index 9b45d064a87d..b89eb768dd05 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -35,6 +35,7 @@
>> #define IMA_FSNAME 0x0200
>> #define IMA_KEYRINGS 0x0400
>> #define IMA_LABEL 0x0800
>> +#define IMA_ALLOW_DUP 0x1000
>>
>> #define UNKNOWN 0
>> #define MEASURE 0x0001 /* same as IMA_MEASURE */
>> @@ -87,6 +88,7 @@ struct ima_rule_entry {
>> char *fsname;
>> struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */
>> struct ima_rule_opt_list *label; /* Measure data grouped under this label */
>
> Defining a new boolean entry shouldn't be necessary. The other
> boolean values are just stored in "flags".
>
Thanks. Will do the same here.
Thanks,
Tushar
>> struct ima_template_desc *template;
>> };
>
> thanks,
>
> Mimi
>
On Tue, 2021-02-09 at 10:23 -0800, Tushar Sugandhi wrote:
> > On Mon, 2021-02-08 at 15:22 -0500, Mimi Zohar wrote:
> >> On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
> >>> IMA does not measure duplicate buffer data since TPM extend is a very
> >>> expensive operation. However, in some cases for integrity critical
> >>> data, the measurement of duplicate data is necessary to accurately
> >>> determine the current state of the system. Eg, SELinux state changing
> >>> from 'audit', to 'enforcing', and back to 'audit' again. In this
> >>> example, currently, IMA will not measure the last state change to
> >>> 'audit'. This limits the ability of attestation services to accurately
> >>> determine the current state of the integrity critical data on the
> >>> system.
> >>>
> >>> This series addresses this gap by providing the ability to measure
> >>> duplicate entries for integrity critical data, driven by policy.
> >>
> >> The same reason for re-measuring buffer data is equally applicable to
> >> files. In both cases, the file or the buffer isn't re-measured if it
> >> already exists in the htable. Please don't limit this patch set to
> >> just buffer data.
> >
> Agreed. I wasn't sure if you wanted the support for files, or other
> buffer measurement scenarios, except critical data. So I started the
> implementation with supporting just critical data. Happy to extend it
> to files and other buffer measurement scenarios as you suggested.
>
> > Instead of making the change on a per measurement rule basis, disabling
> > "htable" would be the simplest way of forcing re-measurements. All
> > that would be needed is a new Kconfig (e.g. CONFIG_IMA_DISABLE_HTABLE)
> > and the associated test in ima_add_template_entry().
> >
> Agreed. Earlier I wasn't sure if you wanted allow_dup support for all
> the scenarios. Now that it is clear, I will implement it as you
> suggested. Thank you so much for the pointers. Appreciate it.
There are two different solutions - per measurement rule, disabling
htable - being discussed. Disabling htable requires miminumal
changes. Which version are you thinking of implementing?
thanks,
Mimi
Thank you Mimi for reviewing this series.
On 2021-02-08 1:10 p.m., Mimi Zohar wrote:
> Hi Tushar,
>
>
> On Mon, 2021-02-08 at 15:22 -0500, Mimi Zohar wrote:
>> On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
>>> IMA does not measure duplicate buffer data since TPM extend is a very
>>> expensive operation. However, in some cases for integrity critical
>>> data, the measurement of duplicate data is necessary to accurately
>>> determine the current state of the system. Eg, SELinux state changing
>>> from 'audit', to 'enforcing', and back to 'audit' again. In this
>>> example, currently, IMA will not measure the last state change to
>>> 'audit'. This limits the ability of attestation services to accurately
>>> determine the current state of the integrity critical data on the
>>> system.
>>>
>>> This series addresses this gap by providing the ability to measure
>>> duplicate entries for integrity critical data, driven by policy.
>>
>> The same reason for re-measuring buffer data is equally applicable to
>> files. In both cases, the file or the buffer isn't re-measured if it
>> already exists in the htable. Please don't limit this patch set to
>> just buffer data.
>
Agreed. I wasn't sure if you wanted the support for files, or other
buffer measurement scenarios, except critical data. So I started the
implementation with supporting just critical data. Happy to extend it
to files and other buffer measurement scenarios as you suggested.
> Instead of making the change on a per measurement rule basis, disabling
> "htable" would be the simplest way of forcing re-measurements. All
> that would be needed is a new Kconfig (e.g. CONFIG_IMA_DISABLE_HTABLE)
> and the associated test in ima_add_template_entry().
>
Agreed. Earlier I wasn't sure if you wanted allow_dup support for all
the scenarios. Now that it is clear, I will implement it as you
suggested. Thank you so much for the pointers. Appreciate it.
Thanks,
Tushar
> thanks,
>
> Mimi
>
On 2021-02-09 10:53 a.m., Mimi Zohar wrote:
> On Tue, 2021-02-09 at 10:23 -0800, Tushar Sugandhi wrote:
>>> On Mon, 2021-02-08 at 15:22 -0500, Mimi Zohar wrote:
>>>> On Fri, 2021-01-29 at 16:45 -0800, Tushar Sugandhi wrote:
>>>>> IMA does not measure duplicate buffer data since TPM extend is a very
>>>>> expensive operation. However, in some cases for integrity critical
>>>>> data, the measurement of duplicate data is necessary to accurately
>>>>> determine the current state of the system. Eg, SELinux state changing
>>>>> from 'audit', to 'enforcing', and back to 'audit' again. In this
>>>>> example, currently, IMA will not measure the last state change to
>>>>> 'audit'. This limits the ability of attestation services to accurately
>>>>> determine the current state of the integrity critical data on the
>>>>> system.
>>>>>
>>>>> This series addresses this gap by providing the ability to measure
>>>>> duplicate entries for integrity critical data, driven by policy.
>>>>
>>>> The same reason for re-measuring buffer data is equally applicable to
>>>> files. In both cases, the file or the buffer isn't re-measured if it
>>>> already exists in the htable. Please don't limit this patch set to
>>>> just buffer data.
>>>
>> Agreed. I wasn't sure if you wanted the support for files, or other
>> buffer measurement scenarios, except critical data. So I started the
>> implementation with supporting just critical data. Happy to extend it
>> to files and other buffer measurement scenarios as you suggested.
>>
>>> Instead of making the change on a per measurement rule basis, disabling
>>> "htable" would be the simplest way of forcing re-measurements. All
>>> that would be needed is a new Kconfig (e.g. CONFIG_IMA_DISABLE_HTABLE)
>>> and the associated test in ima_add_template_entry().
>>>
>> Agreed. Earlier I wasn't sure if you wanted allow_dup support for all
>> the scenarios. Now that it is clear, I will implement it as you
>> suggested. Thank you so much for the pointers. Appreciate it.
>
> There are two different solutions - per measurement rule, disabling
> htable - being discussed. Disabling htable requires miminumal
> changes. Which version are you thinking of implementing?
>
I am thinking of implementing "disabling 'htable' using a new Kconfig
(e.g. CONFIG_IMA_DISABLE_HTABLE)". That is, not using the var
ima_htable or ima_lookup_digest_entry() if that CONFIG is set.
So the duplicate measurements are allowed when the CONFIG is set.
This would cover all the measurement scenarios through a single CONFIG
setting.
I am not planning to implement it as a "per measurement rule".
Sorry it wasn't clear in my earlier response.
Thanks,
Tushar
> thanks,
>
> Mimi
>