linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5 v4] Kexec cmdline bufffer measure
@ 2019-05-03 22:25 Prakhar Srivastava
  2019-05-03 22:25 ` [PATCH 1/5 v4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima Prakhar Srivastava
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Prakhar Srivastava @ 2019-05-03 22:25 UTC (permalink / raw)
  To: linux-integrity, linux-secuirty-module, linux-kernel
  Cc: zohar, ebiederm, vgoyal, nayna, nramas, prsriva, Prakhar Srivastava

From: Prakhar Srivastava <prsriva02@gmail.com>

For Kexec scenario(kexec_file_load) cmdline args are passed to the
next kerenel. These cmldine args used to load the next kernel can 
have undesired/unwanted configs. To guard against any unwanted cmdline
args being passed to the next kernel. The current kernel should measure
the cmdline args to the next kernel, the same takes place in the EFI
bootloader. Thus on kexec the boot_aggregate does not change.

Currently the cmdline args are not measured, this changeset adds a new
ima and LSM hook for buffer measure and calls into the same to measure
the cmdline args passed to the next kernel.The cdmline args meassured
can then be used as an attestation criteria.

The ima logs need to injected into the next kernel, which will be followed
up by other patchsets.


Changelog:
v4:
  - per feedback from LSM community, removed the LSM hook and renamed the
    IMA policy to KEXEC_CMDLINE[Suggested by: Mimi Zohar]

v3: (rebase changes to next-general)
  - Add policy checks for buffer[suggested by Mimi Zohar]
  - use the IMA_XATTR to add buffer
  - Add kexec_cmdline used for kexec file load
  - Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]

v2:
  - Add policy checks for buffer[suggested by Mimi Zohar]
  - Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
  - use the IMA_XATTR to add buffer instead of sig template

v1:
  -Add kconfigs to control the ima_buffer_check
  -measure the cmdline args suffixed with the kernel file name
  -add the buffer to the template sig field.

Prakhar Srivastava (5):
  added a new ima policy func buffer_check, and ima hook to measure the
    buffer hash into ima
  add the buffer to the xattr
  add kexec_cmdline used to ima
  added LSM hook to call ima_buffer_check
  removed the LSM hook made available, and renamed the ima_policy to be
    KEXEC_CMDLINE

 Documentation/ABI/testing/ima_policy      |   1 +
 include/linux/ima.h                       |   3 +
 include/linux/security.h                  |   2 +
 kernel/kexec_core.c                       |   2 +-
 kernel/kexec_file.c                       |   4 +
 kernel/kexec_internal.h                   |   4 +-
 security/integrity/ima/ima.h              |   1 +
 security/integrity/ima/ima_api.c          |   1 +
 security/integrity/ima/ima_main.c         | 115 ++++++++++++++++++++++
 security/integrity/ima/ima_policy.c       |   8 ++
 security/integrity/ima/ima_template_lib.c |   3 +-
 security/integrity/integrity.h            |   1 +
 12 files changed, 142 insertions(+), 3 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5 v4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima
  2019-05-03 22:25 [PATCH 0/5 v4] Kexec cmdline bufffer measure Prakhar Srivastava
@ 2019-05-03 22:25 ` Prakhar Srivastava
  2019-05-06 12:13   ` Mimi Zohar
  2019-05-03 22:25 ` [PATCH 2/5 v4] add the buffer to the xattr Prakhar Srivastava
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Prakhar Srivastava @ 2019-05-03 22:25 UTC (permalink / raw)
  To: linux-integrity, linux-secuirty-module, linux-kernel
  Cc: zohar, ebiederm, vgoyal, nayna, nramas, prsriva, Prakhar Srivastava

From: Prakhar Srivastava <prsriva02@gmail.com>

This change adds a new ima policy func buffer_check, and ima hook to
 measure the buffer hash into ima log.

Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
 Documentation/ABI/testing/ima_policy |  1 +
 include/linux/ima.h                  |  5 ++
 security/integrity/ima/ima.h         |  1 +
 security/integrity/ima/ima_api.c     |  1 +
 security/integrity/ima/ima_main.c    | 89 ++++++++++++++++++++++++++++
 security/integrity/ima/ima_policy.c  |  8 +++
 6 files changed, 105 insertions(+)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 74c6702de74e..12cfe3ff2dea 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -29,6 +29,7 @@ Description:
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
+				[BUFFER_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value
diff --git a/include/linux/ima.h b/include/linux/ima.h
index dc12fbcf484c..f0abade74707 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,6 +26,8 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
+extern void ima_buffer_check(const void *buff, int size,
+				const char *eventname);
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
@@ -92,6 +94,9 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 	return;
 }
 
+static inline void ima_buffer_check(const void *buff, int size,
+		const char *eventname)
+{}
 #endif /* CONFIG_IMA */
 
 #ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..de70df132575 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -184,6 +184,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	hook(KEXEC_KERNEL_CHECK)	\
 	hook(KEXEC_INITRAMFS_CHECK)	\
 	hook(POLICY_CHECK)		\
+	hook(BUFFER_CHECK)		\
 	hook(MAX_CHECK)
 #define __ima_hook_enumify(ENUM)	ENUM,
 
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c7505fb122d4..cb3f67b366f1 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -169,6 +169,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  *		subj=, obj=, type=, func=, mask=, fsmagic=
  *	subj,obj, and type: are LSM specific.
  *	func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
+ *	| BUFFER_CHECK
  *	mask: contains the permission mask
  *	fsmagic: hex value
  *
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 357edd140c09..3db3f3966ac7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -576,6 +576,95 @@ int ima_load_data(enum kernel_load_data_id id)
 	return 0;
 }
 
+/*
+ * process_buffer_measurement - Measure the buffer passed to ima log.
+ * (Instead of using the file hash the buffer hash is used).
+ * @buff - The buffer that needs to be added to the log
+ * @size - size of buffer(in bytes)
+ * @eventname - this is eventname used for the various buffers
+ * that can be measured.
+ *
+ * The buffer passed is added to the ima logs.
+ * If the sig template is used, then the sig field contains the buffer.
+ *
+ * On success return 0.
+ * On error cases surface errors from ima calls.
+ */
+static int process_buffer_measurement(const void *buff, int size,
+				const char *eventname, const struct cred *cred,
+				u32 secid)
+{
+	int ret = -EINVAL;
+	struct ima_template_entry *entry = NULL;
+	struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
+	struct ima_event_data event_data = {iint, NULL, NULL,
+					    NULL, 0, NULL};
+	struct {
+		struct ima_digest_data hdr;
+		char digest[IMA_MAX_DIGEST_SIZE];
+	} hash;
+	int violation = 0;
+	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+
+	if (!buff || size ==  0 || !eventname)
+		goto err_out;
+
+	if (ima_get_action(NULL, cred, secid, 0, BUFFER_CHECK, &pcr)
+		!= IMA_MEASURE)
+		goto err_out;
+
+	memset(iint, 0, sizeof(*iint));
+	memset(&hash, 0, sizeof(hash));
+
+	event_data.filename = eventname;
+
+	iint->ima_hash = &hash.hdr;
+	iint->ima_hash->algo = ima_hash_algo;
+	iint->ima_hash->length = hash_digest_size[ima_hash_algo];
+
+	ret = ima_calc_buffer_hash(buff, size, iint->ima_hash);
+	if (ret < 0)
+		goto err_out;
+
+	ret = ima_alloc_init_template(&event_data, &entry);
+	if (ret < 0)
+		goto err_out;
+
+	ret = ima_store_template(entry, violation, NULL,
+					buff, pcr);
+	if (ret < 0) {
+		ima_free_template_entry(entry);
+		goto err_out;
+	}
+
+	return 0;
+
+err_out:
+	pr_err("Error in adding buffer measure: %d\n", ret);
+	return ret;
+}
+
+/**
+ * ima_buffer_check - based on policy, collect & store buffer measurement
+ * @buf: pointer to buffer
+ * @size: size of buffer
+ * @eventname: event name identifier
+ *
+ * Buffers can only be measured, not appraised.  The buffer identifier
+ * is used as the measurement list entry name (eg. boot_cmdline).
+ */
+void ima_buffer_check(const void *buf, int size, const char *eventname)
+{
+	u32 secid;
+
+	if (buf && size != 0 && eventname) {
+		security_task_getsecid(current, &secid);
+		process_buffer_measurement(buf, size, eventname,
+				current_cred(), secid);
+	}
+}
+
+
 static int __init init_ima(void)
 {
 	int error;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e0cc323f948f..b12551ed191c 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -291,6 +291,12 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 {
 	int i;
 
+	// Incase of BUFFER_CHECK, Inode is NULL
+	if (!inode) {
+		if ((rule->flags & IMA_FUNC) && (rule->func == func))
+			return true;
+		return false;
+	}
 	if ((rule->flags & IMA_FUNC) &&
 	    (rule->func != func && func != POST_SETATTR))
 		return false;
@@ -869,6 +875,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = KEXEC_INITRAMFS_CHECK;
 			else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
 				entry->func = POLICY_CHECK;
+			else if (strcmp(args[0].from, "BUFFER_CHECK") == 0)
+				entry->func = BUFFER_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5 v4] add the buffer to the xattr
  2019-05-03 22:25 [PATCH 0/5 v4] Kexec cmdline bufffer measure Prakhar Srivastava
  2019-05-03 22:25 ` [PATCH 1/5 v4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima Prakhar Srivastava
@ 2019-05-03 22:25 ` Prakhar Srivastava
  2019-05-06 12:13   ` Mimi Zohar
  2019-05-03 22:25 ` [PATCH 3/5 v4] add kexec_cmdline used to ima Prakhar Srivastava
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Prakhar Srivastava @ 2019-05-03 22:25 UTC (permalink / raw)
  To: linux-integrity, linux-secuirty-module, linux-kernel
  Cc: zohar, ebiederm, vgoyal, nayna, nramas, prsriva, Prakhar Srivastava

From: Prakhar Srivastava <prsriva02@gmail.com>

This change adds the buffer passed in to the xattr used for
template entries.

Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
 security/integrity/ima/ima_main.c         | 37 ++++++++++++++++++++---
 security/integrity/ima/ima_template_lib.c |  3 +-
 security/integrity/integrity.h            |  1 +
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 3db3f3966ac7..7362952ab273 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -603,16 +603,37 @@ static int process_buffer_measurement(const void *buff, int size,
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];
 	} hash;
+		struct buffer_xattr {
+		enum evm_ima_xattr_type type;
+		u16 buff_length;
+		unsigned char buff[0];
+	};
+
 	int violation = 0;
 	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+	struct buffer_xattr *buffer_event_data = NULL;
+	int alloc_length = 0;
+	int action = 0;
 
 	if (!buff || size ==  0 || !eventname)
 		goto err_out;
 
-	if (ima_get_action(NULL, cred, secid, 0, BUFFER_CHECK, &pcr)
-		!= IMA_MEASURE)
+	action = ima_get_action(NULL, cred, secid, 0, BUFFER_CHECK, &pcr);
+	if (!(action & IMA_AUDIT) && !(action & IMA_MEASURE))
+		goto err_out;
+
+	alloc_length = sizeof(struct buffer_xattr) + size;
+	buffer_event_data = kzalloc(alloc_length, GFP_KERNEL);
+	if (!buffer_event_data)
 		goto err_out;
 
+	buffer_event_data->type = IMA_XATTR_BUFFER;
+	buffer_event_data->buff_length = size;
+	memcpy(buffer_event_data->buff, buff, size);
+
+	event_data.xattr_value = (struct evm_ima_xattr_data *)buffer_event_data;
+	event_data.xattr_len = alloc_length;
+
 	memset(iint, 0, sizeof(*iint));
 	memset(&hash, 0, sizeof(hash));
 
@@ -630,17 +651,23 @@ static int process_buffer_measurement(const void *buff, int size,
 	if (ret < 0)
 		goto err_out;
 
-	ret = ima_store_template(entry, violation, NULL,
+	if (action & IMA_MEASURE)
+		ret = ima_store_template(entry, violation, NULL,
 					buff, pcr);
+
 	if (ret < 0) {
 		ima_free_template_entry(entry);
 		goto err_out;
 	}
 
-	return 0;
+	if (action & IMA_AUDIT)
+		ima_audit_measurement(iint, event_data.filename);
+
+	ret = 0;
 
 err_out:
-	pr_err("Error in adding buffer measure: %d\n", ret);
+	kfree(buffer_event_data);
+	pr_debug("%s return: %d\n", __func__, ret);
 	return ret;
 }
 
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 513b457ae900..d22de3d8fcd9 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -383,7 +383,8 @@ int ima_eventsig_init(struct ima_event_data *event_data,
 {
 	struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
 
-	if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
+	if ((!xattr_value) || !((xattr_value->type == EVM_IMA_XATTR_DIGSIG) ||
+		(xattr_value->type == IMA_XATTR_BUFFER)))
 		return 0;
 
 	return ima_write_template_field_data(xattr_value, event_data->xattr_len,
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 7de59f44cba3..14ef904f091d 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -74,6 +74,7 @@ enum evm_ima_xattr_type {
 	EVM_IMA_XATTR_DIGSIG,
 	IMA_XATTR_DIGEST_NG,
 	EVM_XATTR_PORTABLE_DIGSIG,
+	IMA_XATTR_BUFFER,
 	IMA_XATTR_LAST
 };
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5 v4] add kexec_cmdline used to ima
  2019-05-03 22:25 [PATCH 0/5 v4] Kexec cmdline bufffer measure Prakhar Srivastava
  2019-05-03 22:25 ` [PATCH 1/5 v4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima Prakhar Srivastava
  2019-05-03 22:25 ` [PATCH 2/5 v4] add the buffer to the xattr Prakhar Srivastava
@ 2019-05-03 22:25 ` Prakhar Srivastava
  2019-05-03 22:25 ` [PATCH 4/5 v4] added LSM hook to call ima_buffer_check Prakhar Srivastava
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Prakhar Srivastava @ 2019-05-03 22:25 UTC (permalink / raw)
  To: linux-integrity, linux-secuirty-module, linux-kernel
  Cc: zohar, ebiederm, vgoyal, nayna, nramas, prsriva, Prakhar Srivastava

From: Prakhar Srivastava <prsriva02@gmail.com>

prepend the kernel file name to the cmdline args
to avoid conflicrts in case of multiple kexec.
Pass the new generated buffer to ima for measurmenet.

Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
 kernel/kexec_core.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
 kernel/kexec_file.c | 14 +++++++++++
 2 files changed, 71 insertions(+)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index d7140447be75..4667f03d406e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1213,3 +1213,60 @@ void __weak arch_kexec_protect_crashkres(void)
 
 void __weak arch_kexec_unprotect_crashkres(void)
 {}
+
+/**
+ * kexec_cmdline_prepend_img_name - prepare the buffer with cmdline
+ * that needs to be measured
+ * @outbuf - out buffer that contains the formated string
+ * @kernel_fd - the file identifier for the kerenel image
+ * @cmdline_ptr - ptr to the cmdline buffer
+ * @cmdline_len - len of the buffer.
+ *
+ * This generates a buffer in the format Kerenelfilename::cmdline
+ *
+ * On success return 0.
+ * On failure return -EINVAL.
+ */
+int kexec_cmdline_prepend_img_name(char **outbuf, int kernel_fd,
+				const char *cmdline_ptr,
+				unsigned long cmdline_len)
+{
+	int ret = -EINVAL;
+	struct fd f = {};
+	int size = 0;
+	char *buf = NULL;
+	char delimiter[] = "::";
+
+	if (!outbuf || !cmdline_ptr)
+		goto out;
+
+	f = fdget(kernel_fd);
+	if (!f.file)
+		goto out;
+
+	size = (f.file->f_path.dentry->d_name.len + cmdline_len - 1+
+			ARRAY_SIZE(delimiter)) - 1;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (!buf)
+		goto out;
+
+	memcpy(buf, f.file->f_path.dentry->d_name.name,
+		f.file->f_path.dentry->d_name.len);
+	memcpy(buf + f.file->f_path.dentry->d_name.len,
+		delimiter, ARRAY_SIZE(delimiter) - 1);
+	memcpy(buf + f.file->f_path.dentry->d_name.len +
+		ARRAY_SIZE(delimiter) - 1,
+		cmdline_ptr, cmdline_len - 1);
+
+	*outbuf = buf;
+	ret = size;
+
+	pr_debug("kexec cmdline buff: %s\n", buf);
+
+out:
+	if (f.file)
+		fdput(f);
+
+	return ret;
+}
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f1d0e00a3971..d287e139085c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -191,6 +191,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	int ret = 0;
 	void *ldata;
 	loff_t size;
+	char *buff_to_measure = NULL;
+	int buff_to_measure_size = 0;
 
 	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
 				       &size, INT_MAX, READING_KEXEC_IMAGE);
@@ -241,6 +243,16 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 			ret = -EINVAL;
 			goto out;
 		}
+
+		/* IMA measures the cmdline args passed to the next kernel*/
+		buff_to_measure_size =
+			kexec_cmdline_prepend_img_name(&buff_to_measure,
+			kernel_fd, image->cmdline_buf, image->cmdline_buf_len);
+
+		ima_buffer_check(buff_to_measure, buff_to_measure_size,
+					"kexec_cmdline");
+
+
 	}
 
 	/* Call arch image load handlers */
@@ -253,7 +265,9 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 
 	image->image_loader_data = ldata;
 out:
+
 	/* In case of error, free up all allocated memory in this function */
+	kfree(buff_to_measure);
 	if (ret)
 		kimage_file_post_load_cleanup(image);
 	return ret;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5 v4] added LSM hook to call ima_buffer_check
  2019-05-03 22:25 [PATCH 0/5 v4] Kexec cmdline bufffer measure Prakhar Srivastava
                   ` (2 preceding siblings ...)
  2019-05-03 22:25 ` [PATCH 3/5 v4] add kexec_cmdline used to ima Prakhar Srivastava
@ 2019-05-03 22:25 ` Prakhar Srivastava
  2019-05-03 22:25 ` [PATCH 5/5 v4] removed the LSM hook made available, and renamed the ima_policy to be KEXEC_CMDLINE Prakhar Srivastava
  2019-05-06 12:12 ` [PATCH 0/5 v4] Kexec cmdline bufffer measure Mimi Zohar
  5 siblings, 0 replies; 10+ messages in thread
From: Prakhar Srivastava @ 2019-05-03 22:25 UTC (permalink / raw)
  To: linux-integrity, linux-secuirty-module, linux-kernel
  Cc: zohar, ebiederm, vgoyal, nayna, nramas, prsriva, Prakhar Srivastava

From: Prakhar Srivastava <prsriva02@gmail.com>

add a LAM hook for kexec_cmldine args to be made available to
other LSMs.

Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
 include/linux/lsm_hooks.h | 3 +++
 include/linux/security.h  | 3 +++
 kernel/kexec_internal.h   | 4 +++-
 security/security.c       | 6 ++++++
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a240a3fc5fc4..f18562c1eb24 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1672,6 +1672,8 @@ union security_list_options {
 	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
 	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
 
+	int (*buffer_check)(const void *buff, int size, const char *eventname);
+
 #ifdef CONFIG_SECURITY_NETWORK
 	int (*unix_stream_connect)(struct sock *sock, struct sock *other,
 					struct sock *newsk);
@@ -1945,6 +1947,7 @@ struct security_hook_heads {
 	struct hlist_head inode_notifysecctx;
 	struct hlist_head inode_setsecctx;
 	struct hlist_head inode_getsecctx;
+	struct hlist_head buffer_check;
 #ifdef CONFIG_SECURITY_NETWORK
 	struct hlist_head unix_stream_connect;
 	struct hlist_head unix_may_send;
diff --git a/include/linux/security.h b/include/linux/security.h
index 49f2685324b0..8dece6da0dda 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -388,6 +388,7 @@ void security_inode_invalidate_secctx(struct inode *inode);
 int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
 int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
 int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
+void security_buffer_measure(const void *buff, int size, char *eventname);
 #else /* CONFIG_SECURITY */
 
 static inline int call_lsm_notifier(enum lsm_event event, void *data)
@@ -1188,6 +1189,8 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
 {
 	return -EOPNOTSUPP;
 }
+static inline void security_buffer_measure(const void *buff, int size, char *eventname)
+{ }
 #endif	/* CONFIG_SECURITY */
 
 #ifdef CONFIG_SECURITY_NETWORK
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index 48aaf2ac0d0d..9f967fbb5aa0 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -12,7 +12,9 @@ int kimage_load_segment(struct kimage *image, struct kexec_segment *segment);
 void kimage_terminate(struct kimage *image);
 int kimage_is_destination_range(struct kimage *image,
 				unsigned long start, unsigned long end);
-
+int kexec_cmdline_prepend_img_name(char **outbuf, int kernel_fd,
+				const char *cmdline_ptr,
+				unsigned long cmdline_len);
 extern struct mutex kexec_mutex;
 
 #ifdef CONFIG_KEXEC_FILE
diff --git a/security/security.c b/security/security.c
index 23cbb1a295a3..2b575a40470e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -754,6 +754,12 @@ int security_bprm_check(struct linux_binprm *bprm)
 	return ima_bprm_check(bprm);
 }
 
+void security_buffer_measure(const void *buff, int size, char *eventname)
+{
+	call_void_hook(buffer_check, buff, size, eventname);
+	return ima_buffer_check(buff, size, eventname);
+}
+
 void security_bprm_committing_creds(struct linux_binprm *bprm)
 {
 	call_void_hook(bprm_committing_creds, bprm);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5 v4] removed the LSM hook made available, and renamed the ima_policy to be KEXEC_CMDLINE
  2019-05-03 22:25 [PATCH 0/5 v4] Kexec cmdline bufffer measure Prakhar Srivastava
                   ` (3 preceding siblings ...)
  2019-05-03 22:25 ` [PATCH 4/5 v4] added LSM hook to call ima_buffer_check Prakhar Srivastava
@ 2019-05-03 22:25 ` Prakhar Srivastava
  2019-05-06 12:13   ` Mimi Zohar
  2019-05-06 12:12 ` [PATCH 0/5 v4] Kexec cmdline bufffer measure Mimi Zohar
  5 siblings, 1 reply; 10+ messages in thread
From: Prakhar Srivastava @ 2019-05-03 22:25 UTC (permalink / raw)
  To: linux-integrity, linux-secuirty-module, linux-kernel
  Cc: zohar, ebiederm, vgoyal, nayna, nramas, prsriva, Prakhar Srivastava

From: Prakhar Srivastava <prsriva02@gmail.com>

Per suggestions from the community, removed the LSM hook.
and renamed the buffer_check func and policy to kexec_cmdline
[suggested by: Mimi Zohar]
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
 Documentation/ABI/testing/ima_policy |  2 +-
 include/linux/ima.h                  |  6 +--
 include/linux/lsm_hooks.h            |  3 --
 include/linux/security.h             |  1 -
 kernel/kexec_core.c                  | 59 +---------------------------
 kernel/kexec_file.c                  | 14 +------
 security/integrity/ima/ima.h         |  2 +-
 security/integrity/ima/ima_api.c     |  2 +-
 security/integrity/ima/ima_main.c    | 11 +++---
 security/integrity/ima/ima_policy.c  |  4 +-
 security/security.c                  |  6 ---
 11 files changed, 15 insertions(+), 95 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 12cfe3ff2dea..62e7cd687e9c 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -29,7 +29,7 @@ Description:
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
-				[BUFFER_CHECK]
+				[KEXEC_CMDLINE]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value
diff --git a/include/linux/ima.h b/include/linux/ima.h
index f0abade74707..2c7a22231008 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,8 +26,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
-extern void ima_buffer_check(const void *buff, int size,
-				const char *eventname);
+extern void ima_kexec_cmdline(const void *buff, int size);
 
 #ifdef CONFIG_IMA_KEXEC
 extern void ima_add_kexec_buffer(struct kimage *image);
@@ -94,8 +93,7 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 	return;
 }
 
-static inline void ima_buffer_check(const void *buff, int size,
-		const char *eventname)
+static inline void ima_kexec_cmdline(const void *buff, int size)
 {}
 #endif /* CONFIG_IMA */
 
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index f18562c1eb24..a240a3fc5fc4 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1672,8 +1672,6 @@ union security_list_options {
 	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
 	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
 
-	int (*buffer_check)(const void *buff, int size, const char *eventname);
-
 #ifdef CONFIG_SECURITY_NETWORK
 	int (*unix_stream_connect)(struct sock *sock, struct sock *other,
 					struct sock *newsk);
@@ -1947,7 +1945,6 @@ struct security_hook_heads {
 	struct hlist_head inode_notifysecctx;
 	struct hlist_head inode_setsecctx;
 	struct hlist_head inode_getsecctx;
-	struct hlist_head buffer_check;
 #ifdef CONFIG_SECURITY_NETWORK
 	struct hlist_head unix_stream_connect;
 	struct hlist_head unix_may_send;
diff --git a/include/linux/security.h b/include/linux/security.h
index 8dece6da0dda..8a129664ba4e 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -388,7 +388,6 @@ void security_inode_invalidate_secctx(struct inode *inode);
 int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
 int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
 int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
-void security_buffer_measure(const void *buff, int size, char *eventname);
 #else /* CONFIG_SECURITY */
 
 static inline int call_lsm_notifier(enum lsm_event event, void *data)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 4667f03d406e..8c0a83980d72 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1212,61 +1212,4 @@ void __weak arch_kexec_protect_crashkres(void)
 {}
 
 void __weak arch_kexec_unprotect_crashkres(void)
-{}
-
-/**
- * kexec_cmdline_prepend_img_name - prepare the buffer with cmdline
- * that needs to be measured
- * @outbuf - out buffer that contains the formated string
- * @kernel_fd - the file identifier for the kerenel image
- * @cmdline_ptr - ptr to the cmdline buffer
- * @cmdline_len - len of the buffer.
- *
- * This generates a buffer in the format Kerenelfilename::cmdline
- *
- * On success return 0.
- * On failure return -EINVAL.
- */
-int kexec_cmdline_prepend_img_name(char **outbuf, int kernel_fd,
-				const char *cmdline_ptr,
-				unsigned long cmdline_len)
-{
-	int ret = -EINVAL;
-	struct fd f = {};
-	int size = 0;
-	char *buf = NULL;
-	char delimiter[] = "::";
-
-	if (!outbuf || !cmdline_ptr)
-		goto out;
-
-	f = fdget(kernel_fd);
-	if (!f.file)
-		goto out;
-
-	size = (f.file->f_path.dentry->d_name.len + cmdline_len - 1+
-			ARRAY_SIZE(delimiter)) - 1;
-
-	buf = kzalloc(size, GFP_KERNEL);
-	if (!buf)
-		goto out;
-
-	memcpy(buf, f.file->f_path.dentry->d_name.name,
-		f.file->f_path.dentry->d_name.len);
-	memcpy(buf + f.file->f_path.dentry->d_name.len,
-		delimiter, ARRAY_SIZE(delimiter) - 1);
-	memcpy(buf + f.file->f_path.dentry->d_name.len +
-		ARRAY_SIZE(delimiter) - 1,
-		cmdline_ptr, cmdline_len - 1);
-
-	*outbuf = buf;
-	ret = size;
-
-	pr_debug("kexec cmdline buff: %s\n", buf);
-
-out:
-	if (f.file)
-		fdput(f);
-
-	return ret;
-}
+{}
\ No newline at end of file
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index d287e139085c..2eb977984537 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -191,8 +191,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	int ret = 0;
 	void *ldata;
 	loff_t size;
-	char *buff_to_measure = NULL;
-	int buff_to_measure_size = 0;
 
 	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
 				       &size, INT_MAX, READING_KEXEC_IMAGE);
@@ -244,15 +242,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 			goto out;
 		}
 
-		/* IMA measures the cmdline args passed to the next kernel*/
-		buff_to_measure_size =
-			kexec_cmdline_prepend_img_name(&buff_to_measure,
-			kernel_fd, image->cmdline_buf, image->cmdline_buf_len);
-
-		ima_buffer_check(buff_to_measure, buff_to_measure_size,
-					"kexec_cmdline");
-
-
+		/* IMA measures the cmdline args passed to the next kernel */
+		ima_kexec_cmdline(image->cmdline_buf, image->cmdline_buf_len - 1);
 	}
 
 	/* Call arch image load handlers */
@@ -267,7 +258,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 out:
 
 	/* In case of error, free up all allocated memory in this function */
-	kfree(buff_to_measure);
 	if (ret)
 		kimage_file_post_load_cleanup(image);
 	return ret;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index de70df132575..226a26d8de09 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -184,7 +184,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	hook(KEXEC_KERNEL_CHECK)	\
 	hook(KEXEC_INITRAMFS_CHECK)	\
 	hook(POLICY_CHECK)		\
-	hook(BUFFER_CHECK)		\
+	hook(KEXEC_CMDLINE)		\
 	hook(MAX_CHECK)
 #define __ima_hook_enumify(ENUM)	ENUM,
 
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index cb3f67b366f1..800d965232e5 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -169,7 +169,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  *		subj=, obj=, type=, func=, mask=, fsmagic=
  *	subj,obj, and type: are LSM specific.
  *	func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
- *	| BUFFER_CHECK
+ *	| KEXEC_CMDLINE
  *	mask: contains the permission mask
  *	fsmagic: hex value
  *
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 7362952ab273..fc9cef54e37c 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -618,7 +618,7 @@ static int process_buffer_measurement(const void *buff, int size,
 	if (!buff || size ==  0 || !eventname)
 		goto err_out;
 
-	action = ima_get_action(NULL, cred, secid, 0, BUFFER_CHECK, &pcr);
+	action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr);
 	if (!(action & IMA_AUDIT) && !(action & IMA_MEASURE))
 		goto err_out;
 
@@ -672,21 +672,20 @@ static int process_buffer_measurement(const void *buff, int size,
 }
 
 /**
- * ima_buffer_check - based on policy, collect & store buffer measurement
+ * ima_kexec_cmdline - based on policy, collect & store buffer measurement
  * @buf: pointer to buffer
  * @size: size of buffer
- * @eventname: event name identifier
  *
  * Buffers can only be measured, not appraised.  The buffer identifier
  * is used as the measurement list entry name (eg. boot_cmdline).
  */
-void ima_buffer_check(const void *buf, int size, const char *eventname)
+void ima_kexec_cmdline(const void *buf, int size)
 {
 	u32 secid;
 
-	if (buf && size != 0 && eventname) {
+	if (buf && size != 0) {
 		security_task_getsecid(current, &secid);
-		process_buffer_measurement(buf, size, eventname,
+		process_buffer_measurement(buf, size, "Kexec-cmdline",
 				current_cred(), secid);
 	}
 }
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index b12551ed191c..7ae59afbf28f 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -875,8 +875,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = KEXEC_INITRAMFS_CHECK;
 			else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
 				entry->func = POLICY_CHECK;
-			else if (strcmp(args[0].from, "BUFFER_CHECK") == 0)
-				entry->func = BUFFER_CHECK;
+			else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0)
+				entry->func = KEXEC_CMDLINE;
 			else
 				result = -EINVAL;
 			if (!result)
diff --git a/security/security.c b/security/security.c
index 2b575a40470e..23cbb1a295a3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -754,12 +754,6 @@ int security_bprm_check(struct linux_binprm *bprm)
 	return ima_bprm_check(bprm);
 }
 
-void security_buffer_measure(const void *buff, int size, char *eventname)
-{
-	call_void_hook(buffer_check, buff, size, eventname);
-	return ima_buffer_check(buff, size, eventname);
-}
-
 void security_bprm_committing_creds(struct linux_binprm *bprm)
 {
 	call_void_hook(bprm_committing_creds, bprm);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/5 v4] Kexec cmdline bufffer measure
  2019-05-03 22:25 [PATCH 0/5 v4] Kexec cmdline bufffer measure Prakhar Srivastava
                   ` (4 preceding siblings ...)
  2019-05-03 22:25 ` [PATCH 5/5 v4] removed the LSM hook made available, and renamed the ima_policy to be KEXEC_CMDLINE Prakhar Srivastava
@ 2019-05-06 12:12 ` Mimi Zohar
  5 siblings, 0 replies; 10+ messages in thread
From: Mimi Zohar @ 2019-05-06 12:12 UTC (permalink / raw)
  To: Prakhar Srivastava, linux-integrity, linux-secuirty-module, linux-kernel
  Cc: ebiederm, vgoyal, nayna, nramas, prsriva

On Fri, 2019-05-03 at 15:25 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> For Kexec scenario(kexec_file_load) cmdline args are passed to the
> next kerenel. These cmldine args used to load the next kernel can 
> have undesired/unwanted configs. To guard against any unwanted cmdline
> args being passed to the next kernel. The current kernel should measure
> the cmdline args to the next kernel, the same takes place in the EFI
> bootloader. Thus on kexec the boot_aggregate does not change.

The boot command line is calculated and added to the current running
kernel's measurement list.  On a soft reboot like kexec, the PCRs are
not reset to zero.  Refer to commit 94c3aac567a9 ("ima: on soft
reboot, restore the measurement list") patch description.

> Currently the cmdline args are not measured, this changeset adds a new
> ima and LSM hook for buffer measure and calls into the same to measure
> the cmdline args passed to the next kernel.The cdmline args meassured
> can then be used as an attestation criteria.

Please update this paragraph to reflect the current patch set. 

> 
> The ima logs need to injected into the next kernel, which will be followed
> up by other patchsets.

The log isn't "injected" into the next kernel, but needs to be carried
over, as described in the above referenced commit.

Mimi


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/5 v4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima
  2019-05-03 22:25 ` [PATCH 1/5 v4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima Prakhar Srivastava
@ 2019-05-06 12:13   ` Mimi Zohar
  0 siblings, 0 replies; 10+ messages in thread
From: Mimi Zohar @ 2019-05-06 12:13 UTC (permalink / raw)
  To: Prakhar Srivastava, linux-integrity, linux-secuirty-module, linux-kernel
  Cc: ebiederm, vgoyal, nayna, nramas, prsriva

On Fri, 2019-05-03 at 15:25 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> This change adds a new ima policy func buffer_check, and ima hook to
>  measure the buffer hash into ima log.

This patch description says "what" you're during without the
motivation.  Please write an appropriate patch description, starting
with some background information.  Refer to section "2) Describe your
changes" of Documentation/process/submitting-patches.rst.

<snip>

> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 357edd140c09..3db3f3966ac7 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -576,6 +576,95 @@ int ima_load_data(enum kernel_load_data_id id)
>  	return 0;
>  }
>  
> +/*
> + * process_buffer_measurement - Measure the buffer passed to ima log.
> + * (Instead of using the file hash the buffer hash is used).
> + * @buff - The buffer that needs to be added to the log

"buf" with a single 'f' is the commonly used variable name.

> + * @size - size of buffer(in bytes)
> + * @eventname - this is eventname used for the various buffers
> + * that can be measured.

This patch set introduces measuring the boot command line.  There
aren't any other buffers being measured.  Please remove the reference
to other buffers.

> + *
> + * The buffer passed is added to the ima logs.
> + * If the sig template is used, then the sig field contains the buffer.

It doesn't look like this particular patch adds the boot command line
string to the measurement list sig field.  Please remove this comment.

I've previously said you can't overload the sig field for storing the
boot command line string.  Please define a new template field.

> + *
> + * On success return 0.
> + * On error cases surface errors from ima calls.
> + */
> +static int process_buffer_measurement(const void *buff, int size,
> +				const char *eventname, const struct cred *cred,
> +				u32 secid)
> +{
> +	int ret = -EINVAL;
> +	struct ima_template_entry *entry = NULL;
> +	struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
> +	struct ima_event_data event_data = {iint, NULL, NULL,
> +					    NULL, 0, NULL};
> +	struct {
> +		struct ima_digest_data hdr;
> +		char digest[IMA_MAX_DIGEST_SIZE];
> +	} hash;
> +	int violation = 0;
> +	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
> +
> +	if (!buff || size ==  0 || !eventname)
> +		goto err_out;

There is only one caller to this function.  This can never happen.
 Please remove this test.

> +
> +	if (ima_get_action(NULL, cred, secid, 0, BUFFER_CHECK, &pcr)
> +		!= IMA_MEASURE)
> +		goto err_out;

The IMA policy defines what should and shouldn't be measured.  Not
having a rule to measure the boot command line can not be considered
an error.

> +
> +	memset(iint, 0, sizeof(*iint));
> +	memset(&hash, 0, sizeof(hash));
> +
> +	event_data.filename = eventname;
> +
> +	iint->ima_hash = &hash.hdr;
> +	iint->ima_hash->algo = ima_hash_algo;
> +	iint->ima_hash->length = hash_digest_size[ima_hash_algo];
> +
> +	ret = ima_calc_buffer_hash(buff, size, iint->ima_hash);
> +	if (ret < 0)
> +		goto err_out;
> +
> +	ret = ima_alloc_init_template(&event_data, &entry);
> +	if (ret < 0)
> +		goto err_out;
> +
> +	ret = ima_store_template(entry, violation, NULL,
> +					buff, pcr);
> +	if (ret < 0) {
> +		ima_free_template_entry(entry);
> +		goto err_out;
> +	}
> +
> +	return 0;
> +
> +err_out:
> +	pr_err("Error in adding buffer measure: %d\n", ret);
> +	return ret;

Please remove.


> +}
> +
> +/**
> + * ima_buffer_check - based on policy, collect & store buffer measurement
> + * @buf: pointer to buffer
> + * @size: size of buffer
> + * @eventname: event name identifier
> + *
> + * Buffers can only be measured, not appraised.  The buffer identifier
> + * is used as the measurement list entry name (eg. boot_cmdline).
> + */
> +void ima_buffer_check(const void *buf, int size, const char *eventname)
> +{
> +	u32 secid;
> +
> +	if (buf && size != 0 && eventname) {
> +		security_task_getsecid(current, &secid);
> +		process_buffer_measurement(buf, size, eventname,
> +				current_cred(), secid);
> +	}
> +}
> +
> +
>  static int __init init_ima(void)
>  {
>  	int error;
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index e0cc323f948f..b12551ed191c 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -291,6 +291,12 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  {
>  	int i;
>  
> +	// Incase of BUFFER_CHECK, Inode is NULL

Comments use the /* ... */ syntax.  Please refer to section "8)
Commenting" of Documentation/process/coding-style.rst.

Mimi

> +	if (!inode) {
> +		if ((rule->flags & IMA_FUNC) && (rule->func == func))
> +			return true;
> +		return false;
> +	}
>  	if ((rule->flags & IMA_FUNC) &&
>  	    (rule->func != func && func != POST_SETATTR))
>  		return false;


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/5 v4] add the buffer to the xattr
  2019-05-03 22:25 ` [PATCH 2/5 v4] add the buffer to the xattr Prakhar Srivastava
@ 2019-05-06 12:13   ` Mimi Zohar
  0 siblings, 0 replies; 10+ messages in thread
From: Mimi Zohar @ 2019-05-06 12:13 UTC (permalink / raw)
  To: Prakhar Srivastava, linux-integrity, linux-secuirty-module, linux-kernel
  Cc: ebiederm, vgoyal, nayna, nramas, prsriva

On Fri, 2019-05-03 at 15:25 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> This change adds the buffer passed in to the xattr used for
> template entries.

Please update this patch description with a clear explanation of the
problem and what you're trying to accomplish with this patch.

> 
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> ---
>  security/integrity/ima/ima_main.c         | 37 ++++++++++++++++++++---
>  security/integrity/ima/ima_template_lib.c |  3 +-
>  security/integrity/integrity.h            |  1 +
>  3 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 3db3f3966ac7..7362952ab273 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -603,16 +603,37 @@ static int process_buffer_measurement(const void *buff, int size,
>  		struct ima_digest_data hdr;
>  		char digest[IMA_MAX_DIGEST_SIZE];
>  	} hash;
> +		struct buffer_xattr {

Improperly indented.

Mimi


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/5 v4] removed the LSM hook made available, and renamed the ima_policy to be KEXEC_CMDLINE
  2019-05-03 22:25 ` [PATCH 5/5 v4] removed the LSM hook made available, and renamed the ima_policy to be KEXEC_CMDLINE Prakhar Srivastava
@ 2019-05-06 12:13   ` Mimi Zohar
  0 siblings, 0 replies; 10+ messages in thread
From: Mimi Zohar @ 2019-05-06 12:13 UTC (permalink / raw)
  To: Prakhar Srivastava, linux-integrity, linux-secuirty-module, linux-kernel
  Cc: ebiederm, vgoyal, nayna, nramas, prsriva

On Fri, 2019-05-03 at 15:25 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> Per suggestions from the community, removed the LSM hook.
> and renamed the buffer_check func and policy to kexec_cmdline
> [suggested by: Mimi Zohar]

To improve readability of the patches, please fold this patch into the
other patches appropriately.

Mimi


> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> ---
>  Documentation/ABI/testing/ima_policy |  2 +-
>  include/linux/ima.h                  |  6 +--
>  include/linux/lsm_hooks.h            |  3 --
>  include/linux/security.h             |  1 -
>  kernel/kexec_core.c                  | 59 +---------------------------
>  kernel/kexec_file.c                  | 14 +------
>  security/integrity/ima/ima.h         |  2 +-
>  security/integrity/ima/ima_api.c     |  2 +-
>  security/integrity/ima/ima_main.c    | 11 +++---
>  security/integrity/ima/ima_policy.c  |  4 +-
>  security/security.c                  |  6 ---
>  11 files changed, 15 insertions(+), 95 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 12cfe3ff2dea..62e7cd687e9c 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -29,7 +29,7 @@ Description:
>  		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
>  				[FIRMWARE_CHECK]
>  				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> -				[BUFFER_CHECK]
> +				[KEXEC_CMDLINE]
>  			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
>  			       [[^]MAY_EXEC]
>  			fsmagic:= hex value
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index f0abade74707..2c7a22231008 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -26,8 +26,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
>  extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  			      enum kernel_read_file_id id);
>  extern void ima_post_path_mknod(struct dentry *dentry);
> -extern void ima_buffer_check(const void *buff, int size,
> -				const char *eventname);
> +extern void ima_kexec_cmdline(const void *buff, int size);
>  
>  #ifdef CONFIG_IMA_KEXEC
>  extern void ima_add_kexec_buffer(struct kimage *image);
> @@ -94,8 +93,7 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
>  	return;
>  }
>  
> -static inline void ima_buffer_check(const void *buff, int size,
> -		const char *eventname)
> +static inline void ima_kexec_cmdline(const void *buff, int size)
>  {}
>  #endif /* CONFIG_IMA */
>  
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index f18562c1eb24..a240a3fc5fc4 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1672,8 +1672,6 @@ union security_list_options {
>  	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
>  	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
>  
> -	int (*buffer_check)(const void *buff, int size, const char *eventname);
> -
>  #ifdef CONFIG_SECURITY_NETWORK
>  	int (*unix_stream_connect)(struct sock *sock, struct sock *other,
>  					struct sock *newsk);
> @@ -1947,7 +1945,6 @@ struct security_hook_heads {
>  	struct hlist_head inode_notifysecctx;
>  	struct hlist_head inode_setsecctx;
>  	struct hlist_head inode_getsecctx;
> -	struct hlist_head buffer_check;
>  #ifdef CONFIG_SECURITY_NETWORK
>  	struct hlist_head unix_stream_connect;
>  	struct hlist_head unix_may_send;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 8dece6da0dda..8a129664ba4e 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -388,7 +388,6 @@ void security_inode_invalidate_secctx(struct inode *inode);
>  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
>  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
>  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> -void security_buffer_measure(const void *buff, int size, char *eventname);
>  #else /* CONFIG_SECURITY */
>  
>  static inline int call_lsm_notifier(enum lsm_event event, void *data)
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 4667f03d406e..8c0a83980d72 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1212,61 +1212,4 @@ void __weak arch_kexec_protect_crashkres(void)
>  {}
>  
>  void __weak arch_kexec_unprotect_crashkres(void)
> -{}
> -
> -/**
> - * kexec_cmdline_prepend_img_name - prepare the buffer with cmdline
> - * that needs to be measured
> - * @outbuf - out buffer that contains the formated string
> - * @kernel_fd - the file identifier for the kerenel image
> - * @cmdline_ptr - ptr to the cmdline buffer
> - * @cmdline_len - len of the buffer.
> - *
> - * This generates a buffer in the format Kerenelfilename::cmdline
> - *
> - * On success return 0.
> - * On failure return -EINVAL.
> - */
> -int kexec_cmdline_prepend_img_name(char **outbuf, int kernel_fd,
> -				const char *cmdline_ptr,
> -				unsigned long cmdline_len)
> -{
> -	int ret = -EINVAL;
> -	struct fd f = {};
> -	int size = 0;
> -	char *buf = NULL;
> -	char delimiter[] = "::";
> -
> -	if (!outbuf || !cmdline_ptr)
> -		goto out;
> -
> -	f = fdget(kernel_fd);
> -	if (!f.file)
> -		goto out;
> -
> -	size = (f.file->f_path.dentry->d_name.len + cmdline_len - 1+
> -			ARRAY_SIZE(delimiter)) - 1;
> -
> -	buf = kzalloc(size, GFP_KERNEL);
> -	if (!buf)
> -		goto out;
> -
> -	memcpy(buf, f.file->f_path.dentry->d_name.name,
> -		f.file->f_path.dentry->d_name.len);
> -	memcpy(buf + f.file->f_path.dentry->d_name.len,
> -		delimiter, ARRAY_SIZE(delimiter) - 1);
> -	memcpy(buf + f.file->f_path.dentry->d_name.len +
> -		ARRAY_SIZE(delimiter) - 1,
> -		cmdline_ptr, cmdline_len - 1);
> -
> -	*outbuf = buf;
> -	ret = size;
> -
> -	pr_debug("kexec cmdline buff: %s\n", buf);
> -
> -out:
> -	if (f.file)
> -		fdput(f);
> -
> -	return ret;
> -}
> +{}
> \ No newline at end of file
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index d287e139085c..2eb977984537 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -191,8 +191,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
>  	int ret = 0;
>  	void *ldata;
>  	loff_t size;
> -	char *buff_to_measure = NULL;
> -	int buff_to_measure_size = 0;
>  
>  	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
>  				       &size, INT_MAX, READING_KEXEC_IMAGE);
> @@ -244,15 +242,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
>  			goto out;
>  		}
>  
> -		/* IMA measures the cmdline args passed to the next kernel*/
> -		buff_to_measure_size =
> -			kexec_cmdline_prepend_img_name(&buff_to_measure,
> -			kernel_fd, image->cmdline_buf, image->cmdline_buf_len);
> -
> -		ima_buffer_check(buff_to_measure, buff_to_measure_size,
> -					"kexec_cmdline");
> -
> -
> +		/* IMA measures the cmdline args passed to the next kernel */
> +		ima_kexec_cmdline(image->cmdline_buf, image->cmdline_buf_len - 1);
>  	}
>  
>  	/* Call arch image load handlers */
> @@ -267,7 +258,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
>  out:
>  
>  	/* In case of error, free up all allocated memory in this function */
> -	kfree(buff_to_measure);
>  	if (ret)
>  		kimage_file_post_load_cleanup(image);
>  	return ret;
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index de70df132575..226a26d8de09 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -184,7 +184,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
>  	hook(KEXEC_KERNEL_CHECK)	\
>  	hook(KEXEC_INITRAMFS_CHECK)	\
>  	hook(POLICY_CHECK)		\
> -	hook(BUFFER_CHECK)		\
> +	hook(KEXEC_CMDLINE)		\
>  	hook(MAX_CHECK)
>  #define __ima_hook_enumify(ENUM)	ENUM,
>  
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index cb3f67b366f1..800d965232e5 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -169,7 +169,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
>   *		subj=, obj=, type=, func=, mask=, fsmagic=
>   *	subj,obj, and type: are LSM specific.
>   *	func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
> - *	| BUFFER_CHECK
> + *	| KEXEC_CMDLINE
>   *	mask: contains the permission mask
>   *	fsmagic: hex value
>   *
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 7362952ab273..fc9cef54e37c 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -618,7 +618,7 @@ static int process_buffer_measurement(const void *buff, int size,
>  	if (!buff || size ==  0 || !eventname)
>  		goto err_out;
>  
> -	action = ima_get_action(NULL, cred, secid, 0, BUFFER_CHECK, &pcr);
> +	action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr);
>  	if (!(action & IMA_AUDIT) && !(action & IMA_MEASURE))
>  		goto err_out;
>  
> @@ -672,21 +672,20 @@ static int process_buffer_measurement(const void *buff, int size,
>  }
>  
>  /**
> - * ima_buffer_check - based on policy, collect & store buffer measurement
> + * ima_kexec_cmdline - based on policy, collect & store buffer measurement
>   * @buf: pointer to buffer
>   * @size: size of buffer
> - * @eventname: event name identifier
>   *
>   * Buffers can only be measured, not appraised.  The buffer identifier
>   * is used as the measurement list entry name (eg. boot_cmdline).
>   */
> -void ima_buffer_check(const void *buf, int size, const char *eventname)
> +void ima_kexec_cmdline(const void *buf, int size)
>  {
>  	u32 secid;
>  
> -	if (buf && size != 0 && eventname) {
> +	if (buf && size != 0) {
>  		security_task_getsecid(current, &secid);
> -		process_buffer_measurement(buf, size, eventname,
> +		process_buffer_measurement(buf, size, "Kexec-cmdline",
>  				current_cred(), secid);
>  	}
>  }
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index b12551ed191c..7ae59afbf28f 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -875,8 +875,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  				entry->func = KEXEC_INITRAMFS_CHECK;
>  			else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
>  				entry->func = POLICY_CHECK;
> -			else if (strcmp(args[0].from, "BUFFER_CHECK") == 0)
> -				entry->func = BUFFER_CHECK;
> +			else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0)
> +				entry->func = KEXEC_CMDLINE;
>  			else
>  				result = -EINVAL;
>  			if (!result)
> diff --git a/security/security.c b/security/security.c
> index 2b575a40470e..23cbb1a295a3 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -754,12 +754,6 @@ int security_bprm_check(struct linux_binprm *bprm)
>  	return ima_bprm_check(bprm);
>  }
>  
> -void security_buffer_measure(const void *buff, int size, char *eventname)
> -{
> -	call_void_hook(buffer_check, buff, size, eventname);
> -	return ima_buffer_check(buff, size, eventname);
> -}
> -
>  void security_bprm_committing_creds(struct linux_binprm *bprm)
>  {
>  	call_void_hook(bprm_committing_creds, bprm);


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-05-06 12:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-03 22:25 [PATCH 0/5 v4] Kexec cmdline bufffer measure Prakhar Srivastava
2019-05-03 22:25 ` [PATCH 1/5 v4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima Prakhar Srivastava
2019-05-06 12:13   ` Mimi Zohar
2019-05-03 22:25 ` [PATCH 2/5 v4] add the buffer to the xattr Prakhar Srivastava
2019-05-06 12:13   ` Mimi Zohar
2019-05-03 22:25 ` [PATCH 3/5 v4] add kexec_cmdline used to ima Prakhar Srivastava
2019-05-03 22:25 ` [PATCH 4/5 v4] added LSM hook to call ima_buffer_check Prakhar Srivastava
2019-05-03 22:25 ` [PATCH 5/5 v4] removed the LSM hook made available, and renamed the ima_policy to be KEXEC_CMDLINE Prakhar Srivastava
2019-05-06 12:13   ` Mimi Zohar
2019-05-06 12:12 ` [PATCH 0/5 v4] Kexec cmdline bufffer measure Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).