ima: select ima-buf template for buffer measurement

The default IMA template for measuring buffer should be 'ima-buf' - so that the measured buffer is correctly included in the IMA measurement log entry. But the default IMA template used for all policy rules is the value set for CONFIG_IMA_DEFAULT_TEMPLATE if the policy rule does not specify a template.

IMA does not take into account the template
requirements of different rules when choosing a default template for a given policy rule. This breaks the buffer measurement if the template is not provided as part of the rule because the default template could be different than 'ima-buf'.

For example, the following IMA policy rule enables measuring the command line arguments passed to the new kernel on kexec system call.

measure func=KEXEC_CMDLINE

The IMA template selected should be 'ima-buf' to have the measured command line arguments included in the IMA measurement log entry. Instead the default IMA template is selected, which could be different than 'ima-buf'.

Initialize a global 'ima-buf' template and select that template, by default, for measuring buffer.

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
 security/integrity/ima/ima.h          |  1 +
 security/integrity/ima/ima_main.c     | 17 +++++------------
 security/integrity/ima/ima_policy.c   |  2 +-
 security/integrity/ima/ima_template.c | 25 +++++++++++++++++++++++++
 4 files changed, 32 insertions(+), 13 deletions(-)

security/integrity/ima/ima.h +1 -0

index 6ebefec616e4..8e8b1e3cb847 100644
@@ -156,6 +156,7 @@ int template_desc_init_fields(const char *template_fmt,
 			      const struct ima_template_field ***fields,
 			      int *num_fields);
 struct ima_template_desc *ima_template_desc_current(void);
+struct ima_template_desc *ima_template_desc_buf(void);
 struct ima_template_desc *lookup_template_desc(const char *name);
 bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);

security/integrity/ima/ima_main.c +2 -5

index a962b23e0429..3646ae763ba9 100644
@@ -413,7 +413,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
  */
 int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
 {
-	struct ima_template_desc *template;
+	struct ima_template_desc *template = NULL;
 	struct file *file = vma->vm_file;
 	char filename[NAME_MAX];
 	char *pathbuf = NULL;

@@ -802,7 +802,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
 					    .filename = eventname,
 					    .buf = buf,
 					    .buf_len = size};
-	struct ima_template_desc *template = NULL;
+	struct ima_template_desc *template = ima_template_desc_buf();
 	struct {
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];

@@ -833,16 +833,9 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
 		pcr = CONFIG_IMA_MEASURE_PCR_IDX;
 
 	if (!template) {
-		template = lookup_template_desc("ima-buf");
-		ret = template_desc_init_fields(template->fmt,
-						&(template->fields),

security/integrity/ima/ima_main.c +0 -0

 	}
 
 	iint.ima_hash = &hash.hdr;

security/integrity/ima/ima_policy.c +1 -1

index 9b5adeaa47fc..823a0c1379cb 100644
@@ -628,7 +628,7 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
 
-	if (template_desc)
+	if (template_desc && !*template_desc)
 		*template_desc = ima_template_desc_current();
 
 	rcu_read_lock();

security/integrity/ima/ima_template.c +9 -0

index 1e89e2d3851f..e53fce2c1610 100644
@@ -55,6 +55,7 @@ static const struct ima_template_field supported_fields[] = {
 #define MAX_TEMPLATE_NAME_LEN sizeof("d-ng|n-ng|sig|buf|d-modisg|modsig")
 
 static struct ima_template_desc *ima_template;
+static struct ima_template_desc *ima_buf_template;
 
 /**
  * ima_template_has_modsig - Check whether template has modsig-related fields.

@@ -252,6 +253,30 @@ struct ima_template_desc *ima_template_desc_current(void)
 	return ima_template;
 }
 
+struct ima_template_desc *ima_template_desc_buf(void)
+{
+	struct ima_template_desc *template = NULL;
+	int ret = 0;
+
+	if (ima_buf_template)
+		return ima_buf_template;
+

security/integrity/ima/ima_template.c +6 -0

+
+	ima_buf_template = template;
+
+	return ima_buf_template;
+}
+
 int __init ima_init_template(void)
 {
 	struct ima_template_desc *template = ima_template_desc_current();

Hi Lakshmi,

Quoted 5 lines · Lakshmi Ramasubramanian

The default IMA template for measuring buffer should be 'ima-buf' - so that the measured buffer is correctly included in the IMA measurement log entry. But the default IMA template used for all policy rules is the value set for CONFIG_IMA_DEFAULT_TEMPLATE if the policy rule does not specify a template.

The second sentence defines the current status. The first sentence describes the problem. I would reverse the sentence order.

^measuring buffer -> buffer measurements

Quoted 6 lines

IMA does not take into account the template
requirements of different rules when choosing a default template for a given policy rule. This breaks the buffer measurement if the template is not provided as part of the rule because the default template could be different than 'ima-buf'.

Does the above paragraph add anything new? Instead describe the problem. Perhaps something like:

With the default template format, buffer measurements are added to the measurement list, but do not include the buffer data, making it difficult, if not impossible, to validate. Including "ima-buf" template records in the measurement list by default, should not impact existing attestation servers without "ima-buf" template support.

Quoted 7 lines

For example, the following IMA policy rule enables measuring the command line arguments passed to the new kernel on kexec system call.

measure func=KEXEC_CMDLINE

Quoted 4 lines

The IMA template selected should be 'ima-buf' to have the measured command line arguments included in the IMA measurement log entry. Instead the default IMA template is selected, which could be different than 'ima-buf'.

When upstreaming a new type of measurement, including an example provides how to validate the new template data. Not every patch description requires an example.

Quoted 4 lines

Initialize a global 'ima-buf' template and select that template, by default, for measuring buffer.

Good.

Quoted 55 lines

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

---
  security/integrity/ima/ima.h          |  1 +
  security/integrity/ima/ima_main.c     | 17 +++++------------
  security/integrity/ima/ima_policy.c   |  2 +-
  security/integrity/ima/ima_template.c | 25 +++++++++++++++++++++++++
  4 files changed, 32 insertions(+), 13 deletions(-)

security/integrity/ima/ima.h +1 -0

index 6ebefec616e4..8e8b1e3cb847 100644
@@ -156,6 +156,7 @@ int template_desc_init_fields(const char *template_fmt,
  			      const struct ima_template_field ***fields,
  			      int *num_fields);
  struct ima_template_desc *ima_template_desc_current(void);
+struct ima_template_desc *ima_template_desc_buf(void);
  struct ima_template_desc *lookup_template_desc(const char *name);
  bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
  int ima_restore_measurement_entry(struct ima_template_entry *entry);

security/integrity/ima/ima_main.c +2 -10

index a962b23e0429..3646ae763ba9 100644
@@ -413,7 +413,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
   */
  int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
  {
-	struct ima_template_desc *template;
+	struct ima_template_desc *template = NULL;
  	struct file *file = vma->vm_file;
  	char filename[NAME_MAX];
  	char *pathbuf = NULL;

@@ -802,7 +802,7 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
  					    .filename = eventname,
  					    .buf = buf,
  					    .buf_len = size};
-	struct ima_template_desc *template = NULL;
+	struct ima_template_desc *template = ima_template_desc_buf();
  	struct {
  		struct ima_digest_data hdr;
  		char digest[IMA_MAX_DIGEST_SIZE];

@@ -833,16 +833,9 @@ void process_buffer_measurement(struct inode *inode, const void *buf, int size,
  		pcr = CONFIG_IMA_MEASURE_PCR_IDX;
 
  	if (!template) {
-		template = lookup_template_desc("ima-buf");
-		ret = template_desc_init_fields(template->fmt,
-						&(template->fields),
-						&(template->num_fields));
-		if (ret < 0) {
-			pr_err("template %s init failed, result: %d\n",
-			       (strlen(template->name) ?
-				template->name : template->fmt), ret);

security/integrity/ima/ima_main.c void process_buffer_measurement(struct inode *inode, const void *buf, int size, · line 838

Quoted 5 lines

security/integrity/ima/ima_main.c +3 -2

-			return;
-		}
+		ret = -EINVAL;
+		audit_cause = "ima_template_desc_buf";
+		goto out;

Normally a test follows the variable assignment, but in this case, the check is being deferred in case there isn't a policy rule.

Quoted 47 lines

security/integrity/ima/ima_main.c +0 -0

  	}
 
  	iint.ima_hash = &hash.hdr;

security/integrity/ima/ima_policy.c +1 -1

index 9b5adeaa47fc..823a0c1379cb 100644
@@ -628,7 +628,7 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
  	struct ima_rule_entry *entry;
  	int action = 0, actmask = flags | (flags << 1);
 
-	if (template_desc)
+	if (template_desc && !*template_desc)
  		*template_desc = ima_template_desc_current();
 
  	rcu_read_lock();

security/integrity/ima/ima_template.c +15 -0

index 1e89e2d3851f..e53fce2c1610 100644
@@ -55,6 +55,7 @@ static const struct ima_template_field supported_fields[] = {
  #define MAX_TEMPLATE_NAME_LEN sizeof("d-ng|n-ng|sig|buf|d-modisg|modsig")
 
  static struct ima_template_desc *ima_template;
+static struct ima_template_desc *ima_buf_template;
 
  /**
   * ima_template_has_modsig - Check whether template has modsig-related fields.

@@ -252,6 +253,30 @@ struct ima_template_desc *ima_template_desc_current(void)
  	return ima_template;
  }
 
+struct ima_template_desc *ima_template_desc_buf(void)
+{
+	struct ima_template_desc *template = NULL;
+	int ret = 0;
+
+	if (ima_buf_template)
+		return ima_buf_template;
+
+	ima_init_template_list();
+	template = lookup_template_desc("ima-buf");
+	if (!template)
+		return NULL;
+
+	ret = template_desc_init_fields(template->fmt,

security/integrity/ima/ima_template.c struct ima_template_desc *ima_template_desc_current(void) · line 273

Quoted 4 lines

security/integrity/ima/ima_template.c +4 -0

+					&(template->fields),
+					&(template->num_fields));
+	if (ret)
+		return NULL;

Instead of initializing the fields here, maybe it should be done in ima_init_template()? That would remove the deferred !template test in process_buffer_measurement() and would also simplify this function.

Quoted 10 lines

security/integrity/ima/ima_template.c +6 -0

+
+	ima_buf_template = template;
+
+	return ima_buf_template;
+}
+
  int __init ima_init_template(void)
  {
  	struct ima_template_desc *template = ima_template_desc_current();