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

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

Hi Matthew,

On Thu, 2019-05-23 at 11:18 -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>
> ---
> 
> Rebased on next-integrity without relying on any other patches.

Thank you.

<snip>

> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 357edd140c09..f23069d9e43d 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -174,7 +174,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  {
>  	struct inode *inode = file_inode(file);
>  	struct integrity_iint_cache *iint = NULL;
> -	struct ima_template_desc *template_desc;
> +	struct ima_template_desc *template_desc = NULL;
>  	char *pathbuf = NULL;
>  	char filename[NAME_MAX];
>  	const char *pathname = NULL;
> @@ -192,7 +192,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  	 * bitmask based on the appraise/audit/measurement policy.
>  	 * Included is the appraise submask.
>  	 */
> -	action = ima_get_action(inode, cred, secid, mask, func, &pcr);
> +	action = ima_get_action(inode, cred, secid, mask, func, &pcr,
> +				&template_desc);
>  	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
>  			   (ima_policy_flag & IMA_MEASURE));
>  	if (!action && !violation_check)
> @@ -275,7 +276,9 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  		goto out_locked;
>  	}
> 
> -	template_desc = ima_template_desc_current();
> +	if (!template_desc)
> +		template_desc = ima_template_desc_current();
> +
>  	if ((action & IMA_APPRAISE_SUBMASK) ||
>  		    strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
>  		/* read 'security.ima' */


Once you have "template_desc", it somehow needs to be passed to
ima_store_measurement() and on to ima_alloc_init_template().


> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 0f6fe53cef09..643490f9f0ad 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c

< snip >

> @@ -1058,6 +1066,16 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			else
>  				entry->flags |= IMA_PCR;
> 
> +			break;
> +		case Opt_template:
> +			ima_log_string(ab, "template", args[0].from);
> +			template_desc = lookup_template_desc(args[0].from);
> +			if (!template_desc) {
> +				result = -EINVAL;
> +			} else {
> +				entry->template = template_desc;
> +				entry->flags |= IMA_TEMPLATE;
> +			}
>  			break;

The "template" option is only relevant to measurement.  Please make
sure that the policy rule action is for "MEASURE".  The documentation
should reflect that as well.

Mimi


>  		case Opt_err:
>  			ima_log_string(ab, "UNKNOWN", p);
>