| Message ID | 20200923192011.5293-5-tusharsu@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | IMA: Infrastructure for measurement of critical kernel data | expand |
Hi Tushar, The above Subject line should be truncated to "IMA: add policy to measure critical data". On Wed, 2020-09-23 at 12:20 -0700, Tushar Sugandhi wrote: > There would be several candidate kernel components suitable for IMA > measurement. Not all of them would have support for IMA measurement. This intro is besides the point. The patch description should describe what is meant by "critical data". > Also, system administrators may not want to measure data for all of > them, even when they support IMA measurement. > An IMA policy option > specific to various kernel components is needed to measure their > respective critical data. > > This policy option needs to be constrained to measure data for > specific kernel components that are specified as input values to the > policy option. > > Add a new IMA policy option - "data_sources:=" to allow measuring > various critical kernel components. This policy option would enable the > system administrators to limit the measurement to the components > listed in "data_sources:=", if the components support IMA measurement. > > The new policy option "data_sources:=" is different from the existing > policy option "keyrings:=". > > In case of "keyrings:=", a policy may measure all keyrings (when > "keyrings:=" option is not provided for func KEY_CHECK), or may > constrain which keyrings need to be measured (when "keyrings:=" option > is provided for func KEY_CHECK). > > But unlike "keyrings:=", the entries in "data_sources:=" would have > different data format. Further, the components listed in > "data_sources:=" need to be modified to call IMA to measure their > data. Therefore, unlike "keyrings:=", IMA shouldn't measure all of the > components by default, when "data_sources:=" is not specified. Because > measuring non-vetted components just by specifying them as a policy > option value may impact the overall reliability of the system. > > To address this, "data_sources:=" should be a mandatory policy option > for func=CRITICAL_DATA. This func is introduced in the 5th patch in this > series). And the compile-time vetting functionality described above is > introduced in the 6th patch in this series. > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> I don't understand what you mean by "non-vetted" components, nor how measuring critical data may impact the overall reliability of the system. The system owner or adminstrator defines what they want to measure, including "critical data", based on the policy rules. They might decide that they want to constrain which "critical data" is measured by specifying "data_sources:=", but that decision is their perogative. The default should allow measuring all critical data. A simple example of "critical data" could be some in memory structure, along the lines of __ro_after_init, or hash of the memory structure. Once the data structure is initialized, the "critical data" measurement shouldn't change. From the attestation server perspective, the IMA measurement list would contain a single record unless the critical data changes. The attestation server doesn't need to know anything about the initial value, just that it has changed in order to trigger some sort alert. thanks, Mimi
On 2020-10-22 2:15 p.m., Mimi Zohar wrote: > Hi Tushar, > > The above Subject line should be truncated to "IMA: add policy to > measure critical data". > > On Wed, 2020-09-23 at 12:20 -0700, Tushar Sugandhi wrote: >> There would be several candidate kernel components suitable for IMA >> measurement. Not all of them would have support for IMA measurement. > > This intro is besides the point. The patch description should describe > what is meant by "critical data". > Thanks. I will fix the description to address this. >> Also, system administrators may not want to measure data for all of >> them, even when they support IMA measurement. >> An IMA policy option >> specific to various kernel components is needed to measure their >> respective critical data. >> >> This policy option needs to be constrained to measure data for >> specific kernel components that are specified as input values to the >> policy option. >> >> Add a new IMA policy option - "data_sources:=" to allow measuring >> various critical kernel components. This policy option would enable the >> system administrators to limit the measurement to the components >> listed in "data_sources:=", if the components support IMA measurement. >> >> The new policy option "data_sources:=" is different from the existing >> policy option "keyrings:=". >> >> In case of "keyrings:=", a policy may measure all keyrings (when >> "keyrings:=" option is not provided for func KEY_CHECK), or may >> constrain which keyrings need to be measured (when "keyrings:=" option >> is provided for func KEY_CHECK). >> >> But unlike "keyrings:=", the entries in "data_sources:=" would have >> different data format. Further, the components listed in >> "data_sources:=" need to be modified to call IMA to measure their >> data. Therefore, unlike "keyrings:=", IMA shouldn't measure all of the >> components by default, when "data_sources:=" is not specified. Because >> measuring non-vetted components just by specifying them as a policy >> option value may impact the overall reliability of the system. >> >> To address this, "data_sources:=" should be a mandatory policy option >> for func=CRITICAL_DATA. This func is introduced in the 5th patch in this >> series). And the compile-time vetting functionality described above is >> introduced in the 6th patch in this series. >> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > > I don't understand what you mean by "non-vetted" components, nor how > measuring critical data may impact the overall reliability of the > system. > Tushar: Before we introduced the mechanism to check supported data-sources at compile time (patch 6/6 of this series), there was a back-and-forth on whether “data_sources:=” should be a mandatory policy option, or optional like “keyrings:=”. And we decided to make the “data_sources:=” mandatory. But now that we have the compile time check (patch 6/6 of this series), we can switch to make “data_sources:=” optional (with the default to allow measuring all critical data – just like what you suggested below). I will make the code and description changes accordingly. > The system owner or adminstrator defines what they want to measure, > including "critical data", based on the policy rules. They might > decide that they want to constrain which "critical data" is measured by > specifying "data_sources:=", but that decision is their perogative. > The default should allow measuring all critical data. > Makes sense. To summarize, we will make the decision which sources to measure- based on the sources defined in the allow list (in patch 6) and the sources defined in “data_sources:=”. If “data_sources:=” is not present, we will measure all sources defined in the allow list. Hope my this understanding is correct based on your feedback. > A simple example of "critical data" could be some in memory structure, > along the lines of __ro_after_init, or hash of the memory structure. > Once the data structure is initialized, the "critical data" measurement > shouldn't change. From the attestation server perspective, the IMA > measurement list would contain a single record unless the critical data > changes. The attestation server doesn't need to know anything about > the initial value, just that it has changed in order to trigger some > sort alert. Yes agreed. After the updates (based on your feedback) I stated above, the behavior should remain consistent with what you described here. > > thanks, > > Mimi >
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index cd572912c593..a81cf79fb255 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -48,6 +48,9 @@ Description: template:= name of a defined IMA template type (eg, ima-ng). Only valid when action is "measure". pcr:= decimal value + data_sources:= list of kernel components + (eg, selinux|apparmor|dm-crypt) that contain data critical + to the security of the kernel. default policy: # PROC_SUPER_MAGIC diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8866e84d0062..89452245f54a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -33,6 +33,7 @@ #define IMA_PCR 0x0100 #define IMA_FSNAME 0x0200 #define IMA_KEYRINGS 0x0400 +#define IMA_DATA_SOURCES 0x0800 #define UNKNOWN 0 #define MEASURE 0x0001 /* same as IMA_MEASURE */ @@ -84,6 +85,7 @@ struct ima_rule_entry { } lsm[MAX_LSM_RULES]; char *fsname; struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */ + struct ima_rule_opt_list *data_sources; /* Measure data from these sources */ struct ima_template_desc *template; }; @@ -911,7 +913,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_err + Opt_data_sources, Opt_err }; static const match_table_t policy_tokens = { @@ -948,6 +950,7 @@ static const match_table_t policy_tokens = { {Opt_pcr, "pcr=%s"}, {Opt_template, "template=%s"}, {Opt_keyrings, "keyrings=%s"}, + {Opt_data_sources, "data_sources=%s"}, {Opt_err, NULL} }; @@ -1312,6 +1315,24 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->flags |= IMA_KEYRINGS; break; + case Opt_data_sources: + ima_log_string(ab, "data_sources", + args[0].from); + + if (entry->data_sources) { + result = -EINVAL; + break; + } + + entry->data_sources = ima_alloc_rule_opt_list(args); + if (IS_ERR(entry->data_sources)) { + result = PTR_ERR(entry->data_sources); + entry->data_sources = NULL; + break; + } + + entry->flags |= IMA_DATA_SOURCES; + break; case Opt_fsuuid: ima_log_string(ab, "fsuuid", args[0].from); @@ -1692,6 +1713,12 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, " "); } + if (entry->flags & IMA_DATA_SOURCES) { + seq_puts(m, "data_sources="); + ima_show_rule_opt_list(m, entry->data_sources); + seq_puts(m, " "); + } + if (entry->flags & IMA_PCR) { snprintf(tbuf, sizeof(tbuf), "%d", entry->pcr); seq_printf(m, pt(Opt_pcr), tbuf);
There would be several candidate kernel components suitable for IMA measurement. Not all of them would have support for IMA measurement. Also, system administrators may not want to measure data for all of them, even when they support IMA measurement. An IMA policy option specific to various kernel components is needed to measure their respective critical data. This policy option needs to be constrained to measure data for specific kernel components that are specified as input values to the policy option. Add a new IMA policy option - "data_sources:=" to allow measuring various critical kernel components. This policy option would enable the system administrators to limit the measurement to the components listed in "data_sources:=", if the components support IMA measurement. The new policy option "data_sources:=" is different from the existing policy option "keyrings:=". In case of "keyrings:=", a policy may measure all keyrings (when "keyrings:=" option is not provided for func KEY_CHECK), or may constrain which keyrings need to be measured (when "keyrings:=" option is provided for func KEY_CHECK). But unlike "keyrings:=", the entries in "data_sources:=" would have different data format. Further, the components listed in "data_sources:=" need to be modified to call IMA to measure their data. Therefore, unlike "keyrings:=", IMA shouldn't measure all of the components by default, when "data_sources:=" is not specified. Because measuring non-vetted components just by specifying them as a policy option value may impact the overall reliability of the system. To address this, "data_sources:=" should be a mandatory policy option for func=CRITICAL_DATA. This func is introduced in the 5th patch in this series). And the compile-time vetting functionality described above is introduced in the 6th patch in this series. Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> --- Documentation/ABI/testing/ima_policy | 3 +++ security/integrity/ima/ima_policy.c | 29 +++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-)