[v10,5/9] ima: make process_buffer_measurement() generic

Commit Message

Mimi Zohar Oct. 31, 2019, 3:31 a.m. UTC

From: Nayna Jain <[e-mail]>

process_buffer_measurement() is limited to measuring the kexec boot
command line. This patch makes process_buffer_measurement() more
generic, allowing it to measure other types of buffer data (e.g.
blacklisted binary hashes or key hashes).

process_buffer_measurement() may be called directly from an IMA
hook or as an auxiliary measurement record. In both cases the buffer
measurement is based on policy. This patch modifies the function to
conditionally retrieve the policy defined PCR and template for the IMA
hook case.

Signed-off-by: Nayna Jain <[e-mail]>
[[e-mail]: added comment in process_buffer_measurement()]
Signed-off-by: Mimi Zohar <[e-mail]>
---
 security/integrity/ima/ima.h      |  3 ++
 security/integrity/ima/ima_main.c | 58 +++++++++++++++++++++++++++------------
 2 files changed, 43 insertions(+), 18 deletions(-)

Comments

Lakshmi Ramasubramanian Oct. 31, 2019, 5:02 p.m. UTC | #1

On 10/30/19 8:31 PM, Mimi Zohar wrote:

>   void ima_kexec_cmdline(const void *buf, int size)
>   {
> -	u32 secid;
> -
> -	if (buf && size != 0) {
> -		security_task_getsecid(current, &secid);
> +	if (buf && size != 0)

Open brace { is missing in the above if statement.

>   		process_buffer_measurement(buf, size, "kexec-cmdline",
> -					   current_cred(), secid);
> -	}
> +					   KEXEC_CMDLINE, 0);
>   }

  -lakshmi

Lakshmi Ramasubramanian Oct. 31, 2019, 5:22 p.m. UTC | #2

On 10/31/19 10:02 AM, Lakshmi Ramasubramanian wrote:

> On 10/30/19 8:31 PM, Mimi Zohar wrote:
> 
>>   void ima_kexec_cmdline(const void *buf, int size)
>>   {
>> -    u32 secid;
>> -
>> -    if (buf && size != 0) {
>> -        security_task_getsecid(current, &secid);
>> +    if (buf && size != 0)
> 
> Open brace { is missing in the above if statement.

My mistake -
I now see that the braces {} have been removed in the if statement since 
there is only line body  the call to process_buffer_measurement()

  -lakshmi

> 
>>           process_buffer_measurement(buf, size, "kexec-cmdline",
>> -                       current_cred(), secid);
>> -    }
>> +                       KEXEC_CMDLINE, 0);
>>   }
> 
>   -lakshmi

Michael Ellerman Nov. 14, 2019, 9:08 a.m. UTC | #3

On Thu, 2019-10-31 at 03:31:30 UTC, Mimi Zohar wrote:
> From: Nayna Jain <[e-mail]>
> 
> process_buffer_measurement() is limited to measuring the kexec boot
> command line. This patch makes process_buffer_measurement() more
> generic, allowing it to measure other types of buffer data (e.g.
> blacklisted binary hashes or key hashes).
> 
> process_buffer_measurement() may be called directly from an IMA
> hook or as an auxiliary measurement record. In both cases the buffer
> measurement is based on policy. This patch modifies the function to
> conditionally retrieve the policy defined PCR and template for the IMA
> hook case.
> 
> Signed-off-by: Nayna Jain <[e-mail]>
> [[e-mail]: added comment in process_buffer_measurement()]
> Signed-off-by: Mimi Zohar <[e-mail]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e14555e3d0e9edfad0a6840c0152f71aba97e793

cheers

Patch

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3689081aaf38..a65772ffa427 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -217,6 +217,9 @@  void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, const struct modsig *modsig, int pcr,
 			   struct ima_template_desc *template_desc);
+void process_buffer_measurement(const void *buf, int size,
+				const char *eventname, enum ima_hooks func,
+				int pcr);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename);
 int ima_alloc_init_template(struct ima_event_data *event_data,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 60027c643ecd..a26e3ad4e886 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -626,14 +626,14 @@  int ima_load_data(enum kernel_load_data_id id)
  * @buf: pointer to the buffer that needs to be added to the log.
  * @size: size of buffer(in bytes).
  * @eventname: event name to be used for the buffer entry.
- * @cred: a pointer to a credentials structure for user validation.
- * @secid: the secid of the task to be validated.
+ * @func: IMA hook
+ * @pcr: pcr to extend the measurement
  *
  * Based on policy, the buffer is measured into the ima log.
  */
-static void process_buffer_measurement(const void *buf, int size,
-				       const char *eventname,
-				       const struct cred *cred, u32 secid)
+void process_buffer_measurement(const void *buf, int size,
+				const char *eventname, enum ima_hooks func,
+				int pcr)
 {
 	int ret = 0;
 	struct ima_template_entry *entry = NULL;
@@ -642,19 +642,45 @@  static void process_buffer_measurement(const void *buf, int size,
 					    .filename = eventname,
 					    .buf = buf,
 					    .buf_len = size};
-	struct ima_template_desc *template_desc = NULL;
+	struct ima_template_desc *template = NULL;
 	struct {
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];
 	} hash = {};
 	int violation = 0;
-	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
 	int action = 0;
+	u32 secid;
 
-	action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr,
-				&template_desc);
-	if (!(action & IMA_MEASURE))
-		return;
+	/*
+	 * Both LSM hooks and auxilary based buffer measurements are
+	 * based on policy.  To avoid code duplication, differentiate
+	 * between the LSM hooks and auxilary buffer measurements,
+	 * retrieving the policy rule information only for the LSM hook
+	 * buffer measurements.
+	 */
+	if (func) {
+		security_task_getsecid(current, &secid);
+		action = ima_get_action(NULL, current_cred(), secid, 0, func,
+					&pcr, &template);
+		if (!(action & IMA_MEASURE))
+			return;
+	}
+
+	if (!pcr)
+		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);
+			return;
+		}
+	}
 
 	iint.ima_hash = &hash.hdr;
 	iint.ima_hash->algo = ima_hash_algo;
@@ -664,7 +690,7 @@  static void process_buffer_measurement(const void *buf, int size,
 	if (ret < 0)
 		goto out;
 
-	ret = ima_alloc_init_template(&event_data, &entry, template_desc);
+	ret = ima_alloc_init_template(&event_data, &entry, template);
 	if (ret < 0)
 		goto out;
 
@@ -686,13 +712,9 @@  static void process_buffer_measurement(const void *buf, int size,
  */
 void ima_kexec_cmdline(const void *buf, int size)
 {
-	u32 secid;
-
-	if (buf && size != 0) {
-		security_task_getsecid(current, &secid);
+	if (buf && size != 0)
 		process_buffer_measurement(buf, size, "kexec-cmdline",
-					   current_cred(), secid);
-	}
+					   KEXEC_CMDLINE, 0);
 }
 
 static int __init init_ima(void)