Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
From: Mimi Zohar <zohar@linux.ibm.com>
To: Matthew Garrett <matthewgarrett@google.com>,
	linux-integrity@vger.kernel.org,
	prakhar srivastava <prsriva02@gmail.com>,
	Thiago Jung Bauermann <bauerman@linux.ibm.com>,
	Roberto Sassu <roberto.sassu@huawei.com>
Cc: zohar@linux.vnet.ibm.com, dmitry.kasatkin@gmail.com,
	miklos@szeredi.hu, linux-fsdevel@vger.kernel.org,
	viro@zeniv.linux.org.uk, Matthew Garrett <mjg59@google.com>
Subject: Re: [PATCH V3 6/6] IMA: Allow profiles to define the desired IMA template
Date: Fri, 17 May 2019 19:47:20 -0400
Message-ID: <1558136840.4507.91.camel@linux.ibm.com> (raw)
In-Reply-To: <20190517212448.14256-7-matthewgarrett@google.com>

[Cc'ing Prakhar, Thiago, Roberto]

Hi Matthew,

On Fri, 2019-05-17 at 14:24 -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>

All of sudden there are three different patch sets needing to include
different types of measurements in the measurement list.  Matthew's
VFS sourced file hashes, Thiago's kexec kernel image appended
signature, and Prakhar's kexec boot command line.  I'd like you to
work together to get this upstreamed.

Matthew, I'm going to ask you to separate out this patch from this
patch set.  Roberto, Thiago, Prakhar, I'm going to ask you to review
Matthew's patch.  I'm expecting all of the patchsets will be re-posted 
based on it.

thanks!

Mimi


> ---
>  Documentation/ABI/testing/ima_policy  |  3 ++-
>  security/integrity/ima/ima.h          |  7 +++++--
>  security/integrity/ima/ima_api.c      |  7 +++++--
>  security/integrity/ima/ima_appraise.c |  2 +-
>  security/integrity/ima/ima_main.c     |  9 ++++++---
>  security/integrity/ima/ima_policy.c   | 24 ++++++++++++++++++++++--
>  security/integrity/ima/ima_template.c | 10 ++++++++--
>  security/integrity/integrity.h        |  1 +
>  8 files changed, 50 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 6a517282068d..f707ef7eda88 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -24,7 +24,7 @@ Description:
>  				[euid=] [fowner=] [fsname=] [subtype=]]
>  			lsm:	[[subj_user=] [subj_role=] [subj_type=]
>  				 [obj_user=] [obj_role=] [obj_type=]]
> -			option:	[[appraise_type=] [permit_directio]
> +			option:	[[appraise_type=] [template=] [permit_directio]
>  			         [trust_vfs]]
> 
>  		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
> @@ -41,6 +41,7 @@ Description:
>  			fowner:= decimal value
>  		lsm:  	are LSM specific
>  		option:	appraise_type:= [imasig]
> +			template:= name of an IMA template type (eg, d-ng)
>  			pcr:= decimal value
>  			permit_directio:= allow directio accesses
>  			trust_vfs:= trust VFS-provided hash values
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d99b867bdc53..29a71c2e6cfa 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -148,6 +148,7 @@ int ima_init_crypto(void);
>  void ima_putc(struct seq_file *m, void *data, int datalen);
>  void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
>  struct ima_template_desc *ima_template_desc_current(void);
> +struct ima_template_desc *lookup_template_desc(const char *name);
>  int ima_restore_measurement_entry(struct ima_template_entry *entry);
>  int ima_restore_measurement_list(loff_t bufsize, void *buf);
>  int ima_measurements_show(struct seq_file *m, void *v);
> @@ -194,7 +195,8 @@ enum ima_hooks {
> 
>  /* LIM API function definitions */
>  int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
> -		   int mask, enum ima_hooks func, int *pcr);
> +		   int mask, enum ima_hooks func, int *pcr,
> +		   struct ima_template_desc **template_desc);
>  int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
>  int ima_collect_measurement(struct integrity_iint_cache *iint,
>  			    struct file *file, void *buf, loff_t size,
> @@ -215,7 +217,8 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
> 
>  /* IMA policy related functions */
>  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);
>  void ima_init_policy(void);
>  void ima_update_policy(void);
>  void ima_update_policy_flag(void);
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 55bafce3d9c0..457b071669ff 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -164,6 +164,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
>   *        MAY_APPEND)
>   * @func: caller identifier
>   * @pcr: pointer filled in if matched measure policy sets pcr=
> + * @template_desc: pointer filled in if matched measure policy sets template=
>   *
>   * The policy is defined in terms of keypairs:
>   *		subj=, obj=, type=, func=, mask=, fsmagic=
> @@ -176,13 +177,15 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
>   *
>   */
>  int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
> -		   int mask, enum ima_hooks func, int *pcr)
> +		   int mask, enum ima_hooks func, int *pcr,
> +		   struct ima_template_desc **template_desc)
>  {
>  	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
> 
>  	flags &= ima_policy_flag;
> 
> -	return ima_match_policy(inode, cred, secid, func, mask, flags, pcr);
> +	return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,
> +				template_desc);
>  }
> 
>  /*
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 5fb7127bbe68..2f6536ab69e8 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -57,7 +57,7 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
> 
>  	security_task_getsecid(current, &secid);
>  	return ima_match_policy(inode, current_cred(), secid, func, mask,
> -				IMA_APPRAISE | IMA_HASH, NULL);
> +				IMA_APPRAISE | IMA_HASH, NULL, NULL);
>  }
> 
>  static int ima_fix_xattr(struct dentry *dentry,
> 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' */
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index c293cbc6c578..33c52466bc8a 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -82,6 +82,7 @@ struct ima_rule_entry {
>  	} lsm[MAX_LSM_RULES];
>  	char *fsname;
>  	char *subtype;
> +	struct ima_template_desc *template;
>  };
> 
>  /*
> @@ -403,6 +404,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.
> @@ -412,7 +414,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);
> @@ -444,6 +447,9 @@ 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->flags & IMA_TEMPLATE)
> +			*template_desc = entry->template;
> +
>  		if (!actmask)
>  			break;
>  	}
> @@ -681,7 +687,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_trust_vfs, Opt_err
> +	Opt_pcr, Opt_trust_vfs, Opt_template, Opt_err
>  };
> 
>  static const match_table_t policy_tokens = {
> @@ -717,6 +723,7 @@ static const match_table_t policy_tokens = {
>  	{Opt_permit_directio, "permit_directio"},
>  	{Opt_pcr, "pcr=%s"},
>  	{Opt_trust_vfs, "trust_vfs"},
> +	{Opt_template, "template=%s"},
>  	{Opt_err, NULL}
>  };
> 
> @@ -770,6 +777,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  	char *from;
>  	char *p;
>  	bool uid_token;
> +	struct ima_template_desc *template_desc;
>  	int result = 0;
> 
>  	ab = integrity_audit_log_start(audit_context(), GFP_KERNEL,
> @@ -1079,6 +1087,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;
>  		case Opt_err:
>  			ima_log_string(ab, "UNKNOWN", p);
> @@ -1358,6 +1376,8 @@ int ima_policy_show(struct seq_file *m, void *v)
>  			}
>  		}
>  	}
> +	if (entry->flags & IMA_TEMPLATE)
> +		seq_printf(m, "template=%s ", entry->template->name);
>  	if (entry->flags & IMA_DIGSIG_REQUIRED)
>  		seq_puts(m, "appraise_type=imasig ");
>  	if (entry->flags & IMA_PERMIT_DIRECTIO)
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 78bd8fea8b35..aea95754d523 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -50,7 +50,6 @@ static const struct ima_template_field supported_fields[] = {
>  #define MAX_TEMPLATE_NAME_LEN 15
> 
>  static struct ima_template_desc *ima_template;
> -static struct ima_template_desc *lookup_template_desc(const char *name);
>  static int template_desc_init_fields(const char *template_fmt,
>  				     const struct ima_template_field ***fields,
>  				     int *num_fields);
> @@ -111,7 +110,7 @@ static int __init ima_template_fmt_setup(char *str)
>  }
>  __setup("ima_template_fmt=", ima_template_fmt_setup);
> 
> -static struct ima_template_desc *lookup_template_desc(const char *name)
> +struct ima_template_desc *lookup_template_desc(const char *name)
>  {
>  	struct ima_template_desc *template_desc;
>  	int found = 0;
> @@ -120,6 +119,13 @@ static struct ima_template_desc *lookup_template_desc(const char *name)
>  	list_for_each_entry_rcu(template_desc, &defined_templates, list) {
>  		if ((strcmp(template_desc->name, name) == 0) ||
>  		    (strcmp(template_desc->fmt, name) == 0)) {
> +			/*
> +			 * template_desc_init_fields() will return immediately
> +			 * if the template is already initialised
> +			 */
> +			template_desc_init_fields(template_desc->fmt,
> +						  &(template_desc->fields),
> +						  &(template_desc->num_fields));
>  			found = 1;
>  			break;
>  		}
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 9d74119bcdfd..9f647b04fc23 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -37,6 +37,7 @@
>  #define EVM_IMMUTABLE_DIGSIG	0x08000000
>  #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
>  #define IMA_TRUST_VFS		0x20000000
> +#define IMA_TEMPLATE		0x40000000
> 
>  #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>  				 IMA_HASH | IMA_APPRAISE_SUBMASK)


  reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-17 21:24 [PATCH V3 0/6] IMA: Support asking the VFS for a file hash Matthew Garrett
2019-05-17 21:24 ` [PATCH V3 1/6] VFS: Add a call to obtain a file's hash Matthew Garrett
2019-05-18 11:25   ` kbuild test robot
2019-05-20  7:10   ` Johannes Thumshirn
2019-05-17 21:24 ` [PATCH V3 2/6] FUSE: Allow filesystems to provide gethash methods Matthew Garrett
2019-05-17 21:24 ` [PATCH V3 3/6] IMA: Allow rule matching on filesystem subtype Matthew Garrett
2019-05-17 21:24 ` [PATCH V3 4/6] IMA: Optionally make use of filesystem-provided hashes Matthew Garrett
2019-05-17 21:24 ` [PATCH V3 5/6] IMA: Add a ima-vfs-sig measurement template Matthew Garrett
2019-05-18 10:04   ` kbuild test robot
2019-05-17 21:24 ` [PATCH V3 6/6] IMA: Allow profiles to define the desired IMA template Matthew Garrett
2019-05-17 23:47   ` Mimi Zohar [this message]
2019-05-20 20:59     ` Matthew Garrett
2019-05-20 21:26       ` Mimi Zohar
2019-05-21  0:10         ` prakhar srivastava

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1558136840.4507.91.camel@linux.ibm.com \
    --to=zohar@linux.ibm.com \
    --cc=bauerman@linux.ibm.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=matthewgarrett@google.com \
    --cc=miklos@szeredi.hu \
    --cc=mjg59@google.com \
    --cc=prsriva02@gmail.com \
    --cc=roberto.sassu@huawei.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org linux-integrity@archiver.kernel.org
	public-inbox-index linux-integrity


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/ public-inbox