[PATCH v3 0/4] Add a new ima_hook buffer_check to measure buffers critical for attestation

[PATCH v3 0/4] Add a new ima_hook buffer_check to measure buffers critical for attestation

[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:
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 (4):
  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

 Documentation/ABI/testing/ima_policy      |   1 +
 include/linux/ima.h                       |   5 +
 include/linux/lsm_hooks.h                 |   3 +
 include/linux/security.h                  |   3 +
 kernel/kexec_core.c                       |  57 +++++++++++
 kernel/kexec_file.c                       |  14 +++
 kernel/kexec_internal.h                   |   4 +-
 security/integrity/ima/ima.h              |   1 +
 security/integrity/ima/ima_api.c          |   1 +
 security/integrity/ima/ima_main.c         | 116 ++++++++++++++++++++++
 security/integrity/ima/ima_policy.c       |   8 ++
 security/integrity/ima/ima_template_lib.c |   3 +-
 security/integrity/integrity.h            |   1 +
 security/security.c                       |   6 ++
 14 files changed, 221 insertions(+), 2 deletions(-)

-- 
2.19.1

[PATCH v3 1/4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima

[Prakhar Srivastava]

From: Prakhar Srivastava <prsriva02@gmail.com>

added a new ima policy func buffer_check, and ima hook to
 measure the buffer hash into ima logs.

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.19.1

[PATCH v3 2/4] add the buffer to the xattr

[Prakhar Srivastava]

From: Prakhar Srivastava <prsriva02@gmail.com>

add the buffer to the xattr for a buffer case

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.19.1

[PATCH v3 3/4] add kexec_cmdline used to ima

[Prakhar Srivastava]

From: Prakhar Srivastava <prsriva02@gmail.com>

prepend the kernel file name to kexec_cmdline 
before measuring the buffer.

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.19.1

[PATCH v3 4/4] added LSM hook to call ima_buffer_check

[Prakhar Srivastava]

From: Prakhar Srivastava <prsriva02@gmail.com>

added LSM hook to call ima_buffer_check

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.19.1

Re: [PATCH v3 1/4] added a new ima policy func buffer_check, and ima hook to measure the buffer hash into ima

[Mimi Zohar]

On Mon, 2019-04-29 at 14:47 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> added a new ima policy func buffer_check, and ima hook to
>  measure the buffer hash into ima logs.

When defining a new LSN/IMA hook please conform to the existing naming
conventions.  Generally LSM hooks are specific to a particular
function.  In this instance, the name of the hook would be something
like security_kexec_cmdline() or ima_kexec_cmdline(), which would call
the generic process_buffer_measurement() you've defined.

Mimi

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

Re: [PATCH v3 2/4] add the buffer to the xattr

[Mimi Zohar]

On Mon, 2019-04-29 at 14:47 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> add the buffer to the xattr for a buffer case

Please write full patch descriptions, here and in the other patches,
explaining the current status with the motivation for the change.

> 
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> ---

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

Although scripts/Lindent has its problems, it does a good job with
code formatting.  There's no reason here for the line breakage.

Mimi

Re: [PATCH v3 3/4] add kexec_cmdline used to ima

[Mimi Zohar]

On Mon, 2019-04-29 at 14:47 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> prepend the kernel file name to kexec_cmdline 
> before measuring the buffer.
> 

kexec doesn't really know or care about IMA.  Other than the IMA call,
itself, nothing should be added to kexec files.  As mentioned in 1/4,
the IMA hook would be named something like ima_kexec_cmdline().

Mimi

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

Re: [PATCH v3 4/4] added LSM hook to call ima_buffer_check

[Mimi Zohar]

On Mon, 2019-04-29 at 14:47 -0700, Prakhar Srivastava wrote:
> From: Prakhar Srivastava <prsriva02@gmail.com>
> 
> added LSM hook to call ima_buffer_check

Casey just responded, "I can imagine an LSM that cares about the
command line, but I don't have interest in it for any work I have in
progress."  Unless one of the other LSM maintainers is interested,
let's leave it as an IMA only hook.

Mimi

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

Re: [PATCH v3 3/4] add kexec_cmdline used to ima

[Al Viro]

On Thu, May 02, 2019 at 12:52:35PM -0400, Mimi Zohar wrote:
> On Mon, 2019-04-29 at 14:47 -0700, Prakhar Srivastava wrote:
> > From: Prakhar Srivastava <prsriva02@gmail.com>

> kexec doesn't really know or care about IMA.  Other than the IMA call,
> itself, nothing should be added to kexec files.  As mentioned in 1/4,
> the IMA hook would be named something like ima_kexec_cmdline().

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

Another thing is that it's so obviously racy, it's not even funny.
Consider what rename(2) in parallel will do to that.