Re: [PATCH V3] IMA: Allow profiles to define the desired IMA template

[Mimi Zohar <zohar@linux.ibm.com>]

On Mon, 2019-06-03 at 13:13 -0700, Matthew Garrett wrote:
> Admins may wish to log different measurements using different IMA
> templates. Add support for overriding the default template on a per-rule
> basis.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> 
> Updated based on review feedback, verified that I can generate an event
> log that contains multiple different templates.
> 
>  Documentation/ABI/testing/ima_policy  |  6 ++++--
>  security/integrity/ima/ima.h          | 13 +++++++++----
>  security/integrity/ima/ima_api.c      | 24 ++++++++++++++++-------
>  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   | 28 +++++++++++++++++++++++++--
>  security/integrity/ima/ima_template.c | 10 ++++++++--
>  8 files changed, 71 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 74c6702de74e..4ded0668a22d 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -24,8 +24,7 @@ Description:
>  				[euid=] [fowner=] [fsname=]]
>  			lsm:	[[subj_user=] [subj_role=] [subj_type=]
>  				 [obj_user=] [obj_role=] [obj_type=]]
> -			option:	[[appraise_type=]] [permit_directio]
> -
> +			option:	[[appraise_type=]] [template=] [permit_directio]
>  		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
>  				[FIRMWARE_CHECK]
>  				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> @@ -38,6 +37,9 @@ Description:
>  			fowner:= decimal value
>  		lsm:  	are LSM specific
>  		option:	appraise_type:= [imasig]
> +			template:= name or format of a defined IMA template
> +			type (eg,ima-ng or d-ng|n-ng). Only valid when action
> +			is "measure".

This patch only supports specifying the template name, not the
template format description.  Please remove "d-ng|n-ng".

>  			pcr:= decimal value
> 
>  		default policy:

<snip>

> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 0f6fe53cef09..cbae2a3a9c5b 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -80,6 +80,7 @@ struct ima_rule_entry {
>  		int type;	/* audit type */
>  	} lsm[MAX_LSM_RULES];
>  	char *fsname;
> +	struct ima_template_desc *template;
>  };
> 
>  /*
> @@ -397,6 +398,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
>   * @func: IMA hook identifier
>   * @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
>   *
>   * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
>   * conditions.
> @@ -406,7 +408,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
>   * than writes so ima_match_policy() is classical RCU candidate.
>   */
>  int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> -		     enum ima_hooks func, int mask, int flags, int *pcr)
> +		     enum ima_hooks func, int mask, int flags, int *pcr,
> +		     struct ima_template_desc **template_desc)
>  {
>  	struct ima_rule_entry *entry;
>  	int action = 0, actmask = flags | (flags << 1);
> @@ -438,6 +441,11 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
>  		if ((pcr) && (entry->flags & IMA_PCR))
>  			*pcr = entry->pcr;
> 
> +		if (template_desc && entry->template)
> +			*template_desc = entry->template;
> +		else
> +			*template_desc = ima_template_desc_current();
> +

This code is finding the template format, but is subsequently being
replaced with the current description.  One way of fixing this, is by
initializing the template_desc before walking the list.

Mimi

>  		if (!actmask)
>  			break;
>  	}
> @@ -676,7 +684,7 @@ enum {
>  	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
>  	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
>  	Opt_appraise_type, Opt_permit_directio,
> -	Opt_pcr, Opt_err
> +	Opt_pcr, Opt_template, Opt_err
>  };
>