Re: [PATCH v7 5/8] IMA: limit critical data measurement based on a label

[Tushar Sugandhi <tusharsu@xxxxxxxxxxxxxxxxxxx>]

On 2020-12-10 3:15 p.m., Tyler Hicks wrote:
> On 2020-12-09 11:42:09, Tushar Sugandhi wrote:
> > System administrators should be able to limit which kernel subsystems
> > they want to measure the critical data for. To enable that, an IMA policy
> > condition to choose specific kernel subsystems is needed. This policy
> > condition would constrain the measurement of the critical data based on
> > a label for the given subsystems.
> >
> > Add a new IMA policy condition - "data_source:=" to the IMA func
> > CRITICAL_DATA to allow measurement of various kernel subsystems. This
> > policy condition would enable the system administrators to restrict the
> > measurement to the labels listed in "data_source:=".
> >
> > Limit the measurement to the labels that are specified in the IMA
> > policy - CRITICAL_DATA+"data_source:=". If "data_sources:=" is not
> > provided with the func CRITICAL_DATA, the data from all the
> > supported kernel subsystems is measured.
> >
> > Signed-off-by: Tushar Sugandhi <tusharsu@xxxxxxxxxxxxxxxxxxx>
>
> This patch will look good once all the IMA_DATA_SOURCE stuff is moved
> over from patch #4.
>
> Tyler
Sounds good. Will do.
~Tushar

> > ---
> >   Documentation/ABI/testing/ima_policy |  2 ++
> >   security/integrity/ima/ima_policy.c  | 26 +++++++++++++++++++++++++-
> >   2 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> > index 6ec7daa87cba..0f4ee9e0a455 100644
> > --- a/Documentation/ABI/testing/ima_policy
> > +++ b/Documentation/ABI/testing/ima_policy
> > @@ -52,6 +52,8 @@ Description:
> >   			template:= name of a defined IMA template type
> >   			(eg, ima-ng). Only valid when action is "measure".
> >   			pcr:= decimal value
> > +			data_source:= [label]
> > +			label:= a unique string used for grouping and limiting critical data.
> >   
> >   		  default policy:
> >   			# PROC_SUPER_MAGIC
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index 9a8ee80a3128..7486d09a3f60 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -934,7 +934,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_source, Opt_err
> >   };
> >   
> >   static const match_table_t policy_tokens = {
> > @@ -971,6 +971,7 @@ static const match_table_t policy_tokens = {
> >   	{Opt_pcr, "pcr=%s"},
> >   	{Opt_template, "template=%s"},
> >   	{Opt_keyrings, "keyrings=%s"},
> > +	{Opt_data_source, "data_source=%s"},
> >   	{Opt_err, NULL}
> >   };
> >   
> > @@ -1350,6 +1351,23 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> >   
> >   			entry->flags |= IMA_KEYRINGS;
> >   			break;
> > +		case Opt_data_source:
> > +			ima_log_string(ab, "data_source", args[0].from);
> > +
> > +			if (entry->data_source) {
> > +				result = -EINVAL;
> > +				break;
> > +			}
> > +
> > +			entry->data_source = ima_alloc_rule_opt_list(args);
> > +			if (IS_ERR(entry->data_source)) {
> > +				result = PTR_ERR(entry->data_source);
> > +				entry->data_source = NULL;
> > +				break;
> > +			}
> > +
> > +			entry->flags |= IMA_DATA_SOURCE;
> > +			break;
> >   		case Opt_fsuuid:
> >   			ima_log_string(ab, "fsuuid", args[0].from);
> >   
> > @@ -1730,6 +1748,12 @@ int ima_policy_show(struct seq_file *m, void *v)
> >   		seq_puts(m, " ");
> >   	}
> >   
> > +	if (entry->flags & IMA_DATA_SOURCE) {
> > +		seq_puts(m, "data_source=");
> > +		ima_show_rule_opt_list(m, entry->data_source);
> > +		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