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

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

[Matthew Garrett]

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.

 Documentation/ABI/testing/ima_policy  |  4 ++--
 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 | 15 +++++++++++----
 security/integrity/integrity.h        |  1 +
 8 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 74c6702de74e..e1a6996e4516 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,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
 
 		default policy:
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..fa4a807bae93 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -147,6 +147,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);
@@ -193,7 +194,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,
@@ -214,7 +216,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 c7505fb122d4..fce8f83c436c 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 0f6fe53cef09..643490f9f0ad 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,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;
 	}
@@ -676,7 +682,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
 };
 
 static const match_table_t policy_tokens = {
@@ -710,6 +716,7 @@ static const match_table_t policy_tokens = {
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_permit_directio, "permit_directio"},
 	{Opt_pcr, "pcr=%s"},
+	{Opt_template, "template=%s"},
 	{Opt_err, NULL}
 };
 
@@ -763,6 +770,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,
@@ -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;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
@@ -1331,6 +1349,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 b631b8bc7624..34f23db2f985 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -47,7 +47,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);
@@ -108,16 +107,24 @@ 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;
+	int result, found = 0;
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(template_desc, &defined_templates, list) {
 		if ((strcmp(template_desc->name, name) == 0) ||
 		    (strcmp(template_desc->fmt, name) == 0)) {
-			found = 1;
+			/*
+			 * template_desc_init_fields() will return immediately
+			 * if the template is already initialised
+			 */
+			result = template_desc_init_fields(template_desc->fmt,
+						 &(template_desc->fields),
+						 &(template_desc->num_fields));
+			if (!result)
+				found = 1;
 			break;
 		}
 	}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 7de59f44cba3..b3e3c58691ea 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -36,6 +36,7 @@
 #define IMA_NEW_FILE		0x04000000
 #define EVM_IMMUTABLE_DIGSIG	0x08000000
 #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
+#define IMA_TEMPLATE		0x20000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
-- 
2.22.0.rc1.257.g3120a18244-goog

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

[Roberto Sassu]

On 5/23/2019 8:18 PM, 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.
> 
>   Documentation/ABI/testing/ima_policy  |  4 ++--
>   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 | 15 +++++++++++----
>   security/integrity/integrity.h        |  1 +
>   8 files changed, 53 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 74c6702de74e..e1a6996e4516 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,7 @@ Description:
>   			fowner:= decimal value
>   		lsm:  	are LSM specific
>   		option:	appraise_type:= [imasig]
> +			template:= name of an IMA template type (eg, d-ng)

IMA template name or custom format (if specified in the kernel command
line, see below).


>   			pcr:= decimal value
>   
>   		default policy:
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d213e835c498..fa4a807bae93 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -147,6 +147,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);
> @@ -193,7 +194,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,
> @@ -214,7 +216,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 c7505fb122d4..fce8f83c436c 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 0f6fe53cef09..643490f9f0ad 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,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;
> +

I would simply return the template, without checking the flags.


>   		if (!actmask)
>   			break;
>   	}
> @@ -676,7 +682,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
>   };
>   
>   static const match_table_t policy_tokens = {
> @@ -710,6 +716,7 @@ static const match_table_t policy_tokens = {
>   	{Opt_appraise_type, "appraise_type=%s"},
>   	{Opt_permit_directio, "permit_directio"},
>   	{Opt_pcr, "pcr=%s"},
> +	{Opt_template, "template=%s"},
>   	{Opt_err, NULL}
>   };
>   
> @@ -763,6 +770,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,
> @@ -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);

Please add:

if (entry->template)
	return -EINVAL;


> +			template_desc = lookup_template_desc(args[0].from);

You assume that the template is already known, while users can specify
in the policy a combination of template fields that is not in the list.


> +			if (!template_desc) {
> +				result = -EINVAL;
> +			} else {
> +				entry->template = template_desc;
> +				entry->flags |= IMA_TEMPLATE;
> +			}
>   			break;
>   		case Opt_err:
>   			ima_log_string(ab, "UNKNOWN", p);
> @@ -1331,6 +1349,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 b631b8bc7624..34f23db2f985 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -47,7 +47,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);
> @@ -108,16 +107,24 @@ 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;
> +	int result, found = 0;
>   
>   	rcu_read_lock();
>   	list_for_each_entry_rcu(template_desc, &defined_templates, list) {
>   		if ((strcmp(template_desc->name, name) == 0) ||
>   		    (strcmp(template_desc->fmt, name) == 0)) {
> -			found = 1;
> +			/*
> +			 * template_desc_init_fields() will return immediately
> +			 * if the template is already initialised
> +			 */
> +			result = template_desc_init_fields(template_desc->fmt,
> +						 &(template_desc->fields),
> +						 &(template_desc->num_fields));
> +			if (!result)
> +				found = 1;
>   			break;
>   		}
>   	}
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 7de59f44cba3..b3e3c58691ea 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -36,6 +36,7 @@
>   #define IMA_NEW_FILE		0x04000000
>   #define EVM_IMMUTABLE_DIGSIG	0x08000000
>   #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
> +#define IMA_TEMPLATE		0x20000000

I think it is not necessary to define a new flag here. It should be
sufficient to check entry->template.

Roberto


>   #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>   				 IMA_HASH | IMA_APPRAISE_SUBMASK)
> 

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI

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

[Matthew Garrett]

On Fri, May 24, 2019 at 2:29 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> On 5/23/2019 8:18 PM, Matthew Garrett wrote:
> >               option: appraise_type:= [imasig]
> > +                     template:= name of an IMA template type (eg, d-ng)
>
> IMA template name or custom format (if specified in the kernel command
> line, see below).

ACK.

> > +             if (template_desc && entry->flags & IMA_TEMPLATE)
> > +                     *template_desc = entry->template;
> > +
>
> I would simply return the template, without checking the flags.

ACK.

> > +             case Opt_template:
> > +                     ima_log_string(ab, "template", args[0].from);
>
> Please add:
>
> if (entry->template)
>         return -EINVAL;

ACK.

>
> > +                     template_desc = lookup_template_desc(args[0].from);
>
> You assume that the template is already known, while users can specify
> in the policy a combination of template fields that is not in the list.

Yes, I'm not attempting to add support for dynamic template definition
in this patch.


> > @@ -36,6 +36,7 @@
> >   #define IMA_NEW_FILE                0x04000000
> >   #define EVM_IMMUTABLE_DIGSIG        0x08000000
> >   #define IMA_FAIL_UNVERIFIABLE_SIGS  0x10000000
> > +#define IMA_TEMPLATE         0x20000000
>
> I think it is not necessary to define a new flag here. It should be
> sufficient to check entry->template.

Ok.

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

[Mimi Zohar]

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);
> 

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

[Mimi Zohar]

Hi Matthew,

Below is one additional comment.

On Fri, 2019-05-24 at 09:31 -0400, Mimi Zohar wrote:
> > 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
> 
> > @@ -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();

This should be moved into ima_match_policy(), so that a valid template
is always returned from ima_get_action().

thanks,

Mimi

>