Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1642543 > unrolled thread

[PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] IMA: new parser for ima_restore_measurement_list() Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-16 15:00 +0200
    [PATCH 4/7] ima: declare get_binary_runtime_size() as non-static Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-16 15:00 +0200
    [PATCH 1/7] ima: introduce ima_parse_buf() Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-16 15:00 +0200
    [PATCH 7/7] ima: fix get_binary_runtime_size() Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-16 15:00 +0200
    [PATCH 6/7] ima: add securityfs interface to restore a measurements list Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-16 15:00 +0200
    [PATCH 5/7] ima: add securityfs interface to save a measurements list with kexec header Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-16 15:00 +0200
    Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for  ima_restore_measurement_list() Ken Goldman <kgold@linux.vnet.ibm.com> - 2017-05-16 21:10 +0200
      Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for  ima_restore_measurement_list() Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-17 09:30 +0200
        Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for  ima_restore_measurement_list() Ken Goldman <kgold@linux.vnet.ibm.com> - 2017-05-17 18:30 +0200
          Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for  ima_restore_measurement_list() Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-18 11:40 +0200
            Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for  ima_restore_measurement_list() Ken Goldman <kgold@linux.vnet.ibm.com> - 2017-05-23 22:50 +0200
              Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for  ima_restore_measurement_list() Roberto Sassu <roberto.sassu@huawei.com> - 2017-05-24 10:30 +0200
            Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for  ima_restore_measurement_list() Ken Goldman <kgold@linux.vnet.ibm.com> - 2017-05-23 23:20 +0200

#1642543 — [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

A new IMA measurement list format, called Crypto Agile, will be introduced
shortly to take full advantage of the algorithm flexibility of TPM 2.0.
With the new format, it will be possible to provide for each list entry
multiple digests, each calculated with an algorithm supported by the TPM.
Those digests will be used by remote entities to verify the integrity of
the measurements list.

The current (SHA1) and the new (Crypto Agile) format definitions are:

SHA1: pcr[4] digest[20]
      template_name_len[4] template_name[template_name_len]
      template_data_len[4] template_data[template_data_len]

Crypto Agile: pcr[4] total_digest_len[4]
              digest1_len[4] digest1[digest1_len] ...
              digestN_len[4] digestN[digestN_len]
              template_name_len[4] template_name[template_name_len]
              template_data_len[4] template_data[template_data_len]

IMA must be able to parse lists with these formats, in order to restore
a measurement list after kexec(). This functionality is implemented
in ima_restore_measurement_list().

With the SHA1 format, the measurement entry header is parsed by casting
a buffer to two structures (binary_hdr_v1 and binary_data_v1), whose
members have the same length of the fields mentioned above.

Since clang does not support variable-length arrays in structures (VLAIS),
the binary_hdr_v1 should be split in two parts, to parse the Crypto Agile
format: one for pcr and digests with variable length, and the other
for template name. This would make the parsing code more complex,
as casting would have been done with three structures.

Given that in most cases, in the binary format of measurements, a length
field is prepended to data, a better solution would be to use only one
function to parse all length-data combinations.

This patch set introduces a new function, called ima_parse_buf(), which
takes as input a buffer and writes lengths and pointers of parsed data
to an array of ima_field_data structures, currently used for template
data fields. ima_restore_measurement_list() and ima_restore_template_data()
are modified to use the new function.

This patch set also introduces two new securityfs interfaces, for testing
purposes, to save the binary measurements list with the kexec header and
to restore it.

Finally, a list entry size calculation issue is fixed in the last patch.

Roberto Sassu (7):
  ima: introduce ima_parse_buf()
  ima: use ima_parse_buf() to parse measurements headers
  ima: use ima_parse_buf() to parse template data
  ima: declare get_binary_runtime_size() as non-static
  ima: add securityfs interface to save a measurements list with kexec
    header
  ima: add securityfs interface to restore a measurements list
  ima: fix get_binary_runtime_size()

 security/integrity/ima/Kconfig            |   8 ++
 security/integrity/ima/ima.h              |   3 +
 security/integrity/ima/ima_fs.c           |  80 +++++++++++++++++--
 security/integrity/ima/ima_kexec.c        |   2 +-
 security/integrity/ima/ima_queue.c        |   6 +-
 security/integrity/ima/ima_template.c     | 126 ++++++++++--------------------
 security/integrity/ima/ima_template_lib.c |  80 +++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |   8 ++
 8 files changed, 217 insertions(+), 96 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1642544 — [PATCH 4/7] ima: declare get_binary_runtime_size() as non-static

The function get_binary_runtime_size(), renamed to
ima_get_template_entry_size(), is now declared as non-static, so that
it can be used by callers outside ima_queue.c to calculate the size
of a given measurement entry.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h       | 1 +
 security/integrity/ima/ima_queue.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b563fbd..10ef9c8 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -131,6 +131,7 @@ extern bool ima_canonical_fmt;
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
+int ima_get_template_entry_size(struct ima_template_entry *entry);
 int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode,
 			   const unsigned char *filename);
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index f628968..24984a2 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -74,7 +74,7 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
  * binary_runtime_measurement list entry, which contains a
  * couple of variable length fields (e.g template name and data).
  */
-static int get_binary_runtime_size(struct ima_template_entry *entry)
+int ima_get_template_entry_size(struct ima_template_entry *entry)
 {
 	int size = 0;
 
@@ -118,7 +118,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 	if (binary_runtime_size != ULONG_MAX) {
 		int size;
 
-		size = get_binary_runtime_size(entry);
+		size = ima_get_template_entry_size(entry);
 		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
 		     binary_runtime_size + size : ULONG_MAX;
 	}
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1642545 — [PATCH 1/7] ima: introduce ima_parse_buf()

ima_parse_buf() takes as input the buffer start and end pointers, and
stores the result in a static array of ima_field_data structures,
where the len field contains the length parsed from the buffer, and
the data field contains the address of the buffer just after the length.
Optionally, the function returns the current value of the buffer pointer
and the number of array elements written.

A bitmap has been added as parameter of ima_parse_buf() to handle
the cases where the length is not prepended to data. Each bit corresponds
to an element of the ima_field_data array. If a bit is set, the length
is not parsed from the buffer, but is read from the corresponding element
of the array (the length must be set before calling the function).

ima_parse_buf() can perform three checks upon request by callers,
depending on the enforce mask passed to it:

- ENFORCE_FIELDS: matching of number of fields (length-data combination)
  - there must be enough data in the buffer to parse the number of fields
    requested (output: current value of buffer pointer)
- ENFORCE_BUFEND: matching of buffer end
  - the ima_field_data array must be large enough to contain lengths and
    data pointers for the amount of data requested (output: number
    of fields written)
- ENFORCE_FIELDS | ENFORCE_BUFEND: matching of both

Use cases

- measurement entry header: ENFORCE_FIELDS | ENFORCE_BUFEND
  - four fields must be parsed: pcr, digest, template name, template data
  - ENFORCE_BUFEND is enforced only for the last measurement entry
- template digest (Crypto Agile): ENFORCE_BUFEND
  - since only the total template digest length is known, the function
    parses length-data combinations until the buffer end is reached
- template data: ENFORCE_FIELDS | ENFORCE_BUFEND
  - since the number of fields and the total template data length
    are known, the function can perform both checks

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_template_lib.c | 61 +++++++++++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  6 +++
 2 files changed, 67 insertions(+)

diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index f9ba37b..28af43f 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -159,6 +159,67 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
 	ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
 }
 
+/**
+ * ima_parse_buf() - Parses lengths and data from an input buffer
+ * @bufstartp:       Buffer start address.
+ * @bufendp:         Buffer end address.
+ * @bufcurp:         Pointer to remaining (non-parsed) data.
+ * @maxfields:       Length of fields array.
+ * @fields:          Array containing lengths and pointers of parsed data.
+ * @curfields:       Number of array items containing parsed data.
+ * @len_mask:        Bitmap (if bit is set, data length should not be parsed).
+ * @enforce_mask:    Check if curfields == maxfields and/or bufcurp == bufendp.
+ * @bufname:         String identifier of the input buffer.
+ *
+ * Return: 0 on success, -EINVAL on error.
+ */
+int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
+		  int maxfields, struct ima_field_data *fields, int *curfields,
+		  unsigned long *len_mask, int enforce_mask, char *bufname)
+{
+	void *bufp = bufstartp;
+	int i;
+
+	for (i = 0; i < maxfields; i++) {
+		if (len_mask == NULL || !test_bit(i, len_mask)) {
+			if (bufp > (bufendp - sizeof(u32)))
+				break;
+
+			fields[i].len = *(u32 *)bufp;
+			if (ima_canonical_fmt)
+				fields[i].len = le32_to_cpu(fields[i].len);
+
+			bufp += sizeof(u32);
+		}
+
+		if (bufp > (bufendp - fields[i].len))
+			break;
+
+		fields[i].data = bufp;
+		bufp += fields[i].len;
+	}
+
+	if ((enforce_mask & ENFORCE_FIELDS) && i != maxfields) {
+		pr_err("%s: nr of fields mismatch: expected: %d, current: %d\n",
+		       bufname, maxfields, i);
+		return -EINVAL;
+	}
+
+	if ((enforce_mask & ENFORCE_BUFEND) && bufp != bufendp) {
+		pr_err("%s: buf end mismatch: expected: %p, current: %p\n",
+		       bufname, bufendp, bufp);
+		return -EINVAL;
+	}
+
+	if (curfields)
+		*curfields = i;
+
+	if (bufcurp)
+		*bufcurp = bufp;
+
+	return 0;
+}
+
 static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo,
 				       struct ima_field_data *field_data)
 {
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index c344530..6a3d8b8 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -18,6 +18,9 @@
 #include <linux/seq_file.h>
 #include "ima.h"
 
+#define ENFORCE_FIELDS 0x00000001
+#define ENFORCE_BUFEND 0x00000002
+
 void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data);
 void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
@@ -26,6 +29,9 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data);
 void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
 			   struct ima_field_data *field_data);
+int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
+		  int maxfields, struct ima_field_data *fields, int *curfields,
+		  unsigned long *len_mask, int enforce_mask, char *bufname);
 int ima_eventdigest_init(struct ima_event_data *event_data,
 			 struct ima_field_data *field_data);
 int ima_eventname_init(struct ima_event_data *event_data,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1642546 — [PATCH 7/7] ima: fix get_binary_runtime_size()

Remove '+ 1' from 'size += strlen(entry->template_desc->name) + 1;',
as the template name is sent to userspace without the '\0' character.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 24984a2..2f7e03c 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -81,7 +81,7 @@ int ima_get_template_entry_size(struct ima_template_entry *entry)
 	size += sizeof(u32);	/* pcr */
 	size += sizeof(entry->digest);
 	size += sizeof(int);	/* template name size field */
-	size += strlen(entry->template_desc->name) + 1;
+	size += strlen(entry->template_desc->name);
 	size += sizeof(entry->template_data_len);
 	size += entry->template_data_len;
 	return size;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1642548 — [PATCH 6/7] ima: add securityfs interface to restore a measurements list

Through the new interface restore_kexec_list, it will be possible
to restore a measurements list, previously read from
binary_kexec_runtime_measurements.

The patch reuses the policy functions to create a buffer, read
the measurements and call ima_restore_measurement_list().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index a93f941..6e3f93f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -77,6 +77,7 @@ static const struct file_operations ima_measurements_count_ops = {
 };
 
 static struct dentry *binary_kexec_runtime_measurements;
+static struct dentry *restore_kexec_list;
 
 /* returns pointer to hlist_node */
 static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
@@ -297,7 +298,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.release = seq_release,
 };
 
-static ssize_t ima_read_policy(char *path)
+static ssize_t ima_read_policy(char *path, bool khdr)
 {
 	void *data;
 	char *datap;
@@ -317,6 +318,13 @@ static ssize_t ima_read_policy(char *path)
 	}
 
 	datap = data;
+
+	if (khdr) {
+		rc = ima_restore_measurement_list(size, data);
+		size = 0;
+		goto out;
+	}
+
 	while (size > 0 && (p = strsep(&datap, "\n"))) {
 		pr_debug("rule: %s\n", p);
 		rc = ima_parse_add_rule(p);
@@ -325,6 +333,7 @@ static ssize_t ima_read_policy(char *path)
 		size -= rc;
 	}
 
+out:
 	vfree(data);
 	if (rc < 0)
 		return rc;
@@ -337,6 +346,7 @@ static ssize_t ima_read_policy(char *path)
 static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 				size_t datalen, loff_t *ppos)
 {
+	bool khdr = file->f_path.dentry == restore_kexec_list;
 	char *data;
 	ssize_t result;
 
@@ -363,8 +373,13 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 	if (result < 0)
 		goto out_free;
 
+	if (khdr && data[0] != '/') {
+		mutex_unlock(&ima_write_mutex);
+		goto out_free;
+	}
+
 	if (data[0] == '/') {
-		result = ima_read_policy(data);
+		result = ima_read_policy(data, khdr);
 	} else if (ima_appraise & IMA_APPRAISE_POLICY) {
 		pr_err("IMA: signed policy file (specified as an absolute pathname) required\n");
 		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
@@ -393,7 +408,7 @@ static struct dentry *violations;
 static struct dentry *ima_policy;
 
 enum ima_fs_flags {
-	IMA_FS_BUSY,
+	IMA_FS_BUSY, IMA_RESTORE_LIST_BUSY,
 };
 
 static unsigned long ima_fs_flags;
@@ -412,6 +427,9 @@ static const struct seq_operations ima_policy_seqops = {
  */
 static int ima_open_policy(struct inode *inode, struct file *filp)
 {
+	bool khdr = filp->f_path.dentry == restore_kexec_list;
+	unsigned long bit = khdr ? IMA_RESTORE_LIST_BUSY : IMA_FS_BUSY;
+
 	if (!(filp->f_flags & O_WRONLY)) {
 #ifndef	CONFIG_IMA_READ_POLICY
 		return -EACCES;
@@ -423,7 +441,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 		return seq_open(filp, &ima_policy_seqops);
 #endif
 	}
-	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+	if (test_and_set_bit(bit, &ima_fs_flags))
 		return -EBUSY;
 	return 0;
 }
@@ -439,6 +457,11 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 {
 	const char *cause = valid_policy ? "completed" : "failed";
 
+	if (file->f_path.dentry == restore_kexec_list) {
+		clear_bit(IMA_RESTORE_LIST_BUSY, &ima_fs_flags);
+		return 0;
+	}
+
 	if ((file->f_flags & O_ACCMODE) == O_RDONLY)
 		return seq_release(inode, file);
 
@@ -522,9 +545,16 @@ int __init ima_fs_init(void)
 				   &ima_measurements_ops);
 	if (IS_ERR(binary_kexec_runtime_measurements))
 		goto out;
+
+	restore_kexec_list = securityfs_create_file("restore_kexec_list",
+						    S_IWUSR, ima_dir, NULL,
+						    &ima_measure_policy_ops);
+	if (IS_ERR(restore_kexec_list))
+		goto out;
 #endif
 	return 0;
 out:
+	securityfs_remove(restore_kexec_list);
 	securityfs_remove(binary_kexec_runtime_measurements);
 	securityfs_remove(violations);
 	securityfs_remove(runtime_measurements_count);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1642550 — [PATCH 5/7] ima: add securityfs interface to save a measurements list with kexec header

Through the new interface binary_kexec_runtime_measurements, it will be
possible to read the same content returned by binary_runtime_measurements,
with the kexec header prepended.

The new interface has been added for testing ima_restore_measurement_list()
which, at the moment, works only on PPC systems. The interface for reading
the binary list with the kexec header will be provided in a separate patch.

The patch reuses ima_measurements_start() and ima_measurements_next()
to send the measurements list to userspace. Their behavior changes
depending on the current dentry.

To provide the correct information in the kexec header,
ima_measurements_start() has to iterate over the whole list and calculate
the number of entries and the total size. It is not possible to read
the value of the global variable binary_runtime_size and ima_htable.len
without taking ima_extend_list_mutex, because there might have been a list
add between the two read operations.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/Kconfig            |  8 ++++++
 security/integrity/ima/ima.h              |  2 ++
 security/integrity/ima/ima_fs.c           | 42 ++++++++++++++++++++++++++++---
 security/integrity/ima/ima_kexec.c        |  2 +-
 security/integrity/ima/ima_template.c     |  2 +-
 security/integrity/ima/ima_template_lib.c | 19 ++++++++++++++
 security/integrity/ima/ima_template_lib.h |  2 ++
 7 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 370eb2f..0f60c04 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -39,6 +39,14 @@ config IMA_KEXEC
 	   Depending on the IMA policy, the measurement list can grow to
 	   be very large.
 
+config IMA_KEXEC_TESTING
+	bool "Enable securityfs interfaces to save/restore measurement list"
+	depends on IMA
+	default n
+	help
+	   Use binary_kexec_runtime_measurements to save the binary list
+	   with the kexec header; use restore_kexec_list to restore a list.
+
 config IMA_MEASURE_PCR_IDX
 	int
 	depends on IMA
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 10ef9c8..416497b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -49,6 +49,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
 #define IMA_TEMPLATE_IMA_NAME "ima"
 #define IMA_TEMPLATE_IMA_FMT "d|n"
 
+#define IMA_KEXEC_HDR_VERSION 1
+
 /* current content of the policy */
 extern int ima_policy_flag;
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index ca303e5..a93f941 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -25,6 +25,7 @@
 #include <linux/vmalloc.h>
 
 #include "ima.h"
+#include "ima_template_lib.h"
 
 static DEFINE_MUTEX(ima_write_mutex);
 
@@ -75,28 +76,52 @@ static const struct file_operations ima_measurements_count_ops = {
 	.llseek = generic_file_llseek,
 };
 
+static struct dentry *binary_kexec_runtime_measurements;
+
 /* returns pointer to hlist_node */
 static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
 {
 	loff_t l = *pos;
 	struct ima_queue_entry *qe;
+	struct ima_queue_entry *qe_found = NULL;
+	unsigned long size = 0, count = 0;
+	bool khdr = m->file->f_path.dentry == binary_kexec_runtime_measurements;
 
 	/* we need a lock since pos could point beyond last element */
 	rcu_read_lock();
 	list_for_each_entry_rcu(qe, &ima_measurements, later) {
-		if (!l--) {
-			rcu_read_unlock();
-			return qe;
+		if (!l) {
+			qe_found = qe_found ? qe_found : qe;
+
+			if (!khdr)
+				break;
+
+			if (*pos)
+				break;
+
+			size += ima_get_template_entry_size(qe->entry);
+			count++;
+			m->private = qe;
+			continue;
 		}
+		l--;
 	}
 	rcu_read_unlock();
-	return NULL;
+
+	if (khdr && size)
+		ima_show_kexec_hdr(m, count, size);
+
+	return qe_found;
 }
 
 static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
 {
+	bool khdr = m->file->f_path.dentry == binary_kexec_runtime_measurements;
 	struct ima_queue_entry *qe = v;
 
+	if (khdr && qe == m->private)
+		return NULL;
+
 	/* lock protects when reading beyond last element
 	 * against concurrent list-extension
 	 */
@@ -490,8 +515,17 @@ int __init ima_fs_init(void)
 	if (IS_ERR(ima_policy))
 		goto out;
 
+#ifdef CONFIG_IMA_KEXEC_TESTING
+	binary_kexec_runtime_measurements =
+	    securityfs_create_file("binary_kexec_runtime_measurements",
+				   S_IRUSR | S_IRGRP, ima_dir, NULL,
+				   &ima_measurements_ops);
+	if (IS_ERR(binary_kexec_runtime_measurements))
+		goto out;
+#endif
 	return 0;
 out:
+	securityfs_remove(binary_kexec_runtime_measurements);
 	securityfs_remove(violations);
 	securityfs_remove(runtime_measurements_count);
 	securityfs_remove(ascii_runtime_measurements);
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index e473eee..b0b8ed2 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -36,7 +36,7 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 	file.count = sizeof(khdr);	/* reserved space */
 
 	memset(&khdr, 0, sizeof(khdr));
-	khdr.version = 1;
+	khdr.version = IMA_KEXEC_HDR_VERSION;
 	list_for_each_entry_rcu(qe, &ima_measurements, later) {
 		if (file.count < file.size) {
 			khdr.count++;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 7412d02..f86456c 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -347,7 +347,7 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 		khdr->buffer_size = le64_to_cpu(khdr->buffer_size);
 	}
 
-	if (khdr->version != 1) {
+	if (khdr->version != IMA_KEXEC_HDR_VERSION) {
 		pr_err("attempting to restore a incompatible measurement list");
 		return -EINVAL;
 	}
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 28af43f..de2b064 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -159,6 +159,25 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
 	ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
 }
 
+void ima_show_kexec_hdr(struct seq_file *m, unsigned long count,
+			unsigned long size)
+{
+	struct ima_kexec_hdr khdr;
+
+	memset(&khdr, 0, sizeof(khdr));
+	khdr.version = IMA_KEXEC_HDR_VERSION;
+	khdr.count = count;
+	khdr.buffer_size = sizeof(khdr) + size;
+
+	if (ima_canonical_fmt) {
+		khdr.version = cpu_to_le16(khdr.version);
+		khdr.count = cpu_to_le64(khdr.count);
+		khdr.buffer_size = cpu_to_le64(khdr.buffer_size);
+	}
+
+	ima_putc(m, &khdr, sizeof(khdr));
+}
+
 /**
  * ima_parse_buf() - Parses lengths and data from an input buffer
  * @bufstartp:       Buffer start address.
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b8..069e4ba 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -29,6 +29,8 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data);
 void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
 			   struct ima_field_data *field_data);
+void ima_show_kexec_hdr(struct seq_file *m, unsigned long count,
+			unsigned long size);
 int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
 		  int maxfields, struct ima_field_data *fields, int *curfields,
 		  unsigned long *len_mask, int enforce_mask, char *bufname);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1642771 — Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

On 5/16/2017 8:53 AM, Roberto Sassu wrote:
> A new IMA measurement list format, called Crypto Agile, will be introduced
> shortly to take full advantage of the algorithm flexibility of TPM 2.0.
> With the new format, it will be possible to provide for each list entry
> multiple digests, each calculated with an algorithm supported by the TPM.
> Those digests will be used by remote entities to verify the integrity of
> the measurements list.
> 
> The current (SHA1) and the new (Crypto Agile) format definitions are:
> 
> SHA1: pcr[4] digest[20]
>        template_name_len[4] template_name[template_name_len]
>        template_data_len[4] template_data[template_data_len]
> 
> Crypto Agile: pcr[4] total_digest_len[4]
>                digest1_len[4] digest1[digest1_len] ...
>                digestN_len[4] digestN[digestN_len]
>                template_name_len[4] template_name[template_name_len]
>                template_data_len[4] template_data[template_data_len]

1 - In this proposed format, how does the parser or consumer of the log
know what algorithm is used for digestN.
For example, the TCG standard format uses TPML_DIGEST_VALUES
	uint32_t count - the number of digests TPMT_HA
	TPMT_HA digests[]

where a TPMT_HA is
	algorithm identifier
	digest byte array

2 - Not a criticism, just a question for understanding ...  Would it be 
true that the total_digest_length == the sum of all the digestN_len 
values plus 4 bytes for each length.

Does it determine how many digests there are by when the total length is 
consumed?

[toc] | [prev] | [next] | [standalone]

#1643036 — Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

On 5/16/2017 9:00 PM, Ken Goldman wrote:
> On 5/16/2017 8:53 AM, Roberto Sassu wrote:
>> A new IMA measurement list format, called Crypto Agile, will be introduced
>> shortly to take full advantage of the algorithm flexibility of TPM 2.0.
>> With the new format, it will be possible to provide for each list entry
>> multiple digests, each calculated with an algorithm supported by the TPM.
>> Those digests will be used by remote entities to verify the integrity of
>> the measurements list.
>>
>> The current (SHA1) and the new (Crypto Agile) format definitions are:
>>
>> SHA1: pcr[4] digest[20]
>>        template_name_len[4] template_name[template_name_len]
>>        template_data_len[4] template_data[template_data_len]
>>
>> Crypto Agile: pcr[4] total_digest_len[4]
>>                digest1_len[4] digest1[digest1_len] ...
>>                digestN_len[4] digestN[digestN_len]
>>                template_name_len[4] template_name[template_name_len]
>>                template_data_len[4] template_data[template_data_len]
>
> 1 - In this proposed format, how does the parser or consumer of the log
> know what algorithm is used for digestN.

The format of digestN is: <algo name>:\0<digest value>, the same used
for the file digest.


> For example, the TCG standard format uses TPML_DIGEST_VALUES
> 	uint32_t count - the number of digests TPMT_HA
> 	TPMT_HA digests[]
>
> where a TPMT_HA is
> 	algorithm identifier
> 	digest byte array
>
> 2 - Not a criticism, just a question for understanding ...  Would it be
> true that the total_digest_length == the sum of all the digestN_len
> values plus 4 bytes for each length.

Yes.


> Does it determine how many digests there are by when the total length is
> consumed?

Yes. Since the number of digests is unknown until the buffer is parsed,
the parser consumes the data until the buffer end is reached. Then,
it returns the number of digests to the caller.

Roberto

[toc] | [prev] | [next] | [standalone]

#1643499 — Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

On 5/17/2017 3:25 AM, Roberto Sassu wrote:
> 
> The format of digestN is: <algo name>:\0<digest value>, the same used
> for the file digest.

Since the format is changing from the SHA-1 log format anyway ...

How do people feel about the colon and null terminated string format for 
algorithm identifiers?

The TCG standard enumerations are uint16_t, and there is a registry of 
hash algorithms.

As a consuming parser, it feels nice to know it's always 2 bytes and not 
have to worry about a missing colon or a missing nul terminator risking 
a buffer overflow.

[toc] | [prev] | [next] | [standalone]

#1644016 — Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

On 5/17/2017 6:28 PM, Ken Goldman wrote:
> On 5/17/2017 3:25 AM, Roberto Sassu wrote:
>>
>> The format of digestN is: <algo name>:\0<digest value>, the same used
>> for the file digest.
>
> Since the format is changing from the SHA-1 log format anyway ...
>
> How do people feel about the colon and null terminated string format for
> algorithm identifiers?
>
> The TCG standard enumerations are uint16_t, and there is a registry of
> hash algorithms.
>
> As a consuming parser, it feels nice to know it's always 2 bytes and not
> have to worry about a missing colon or a missing nul terminator risking
> a buffer overflow.

There cannot be buffer overflow, because the length of each digest
field is known.

Roberto

[toc] | [prev] | [next] | [standalone]

#1648617 — Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

On 5/18/2017 5:38 AM, Roberto Sassu wrote:
> On 5/17/2017 6:28 PM, Ken Goldman wrote:
>> On 5/17/2017 3:25 AM, Roberto Sassu wrote:
>>>
>>> The format of digestN is: <algo name>:\0<digest value>, the same used
>>> for the file digest.
>>
>> Since the format is changing from the SHA-1 log format anyway ...
>>
>> How do people feel about the colon and null terminated string format for
>> algorithm identifiers?
>>
>> The TCG standard enumerations are uint16_t, and there is a registry of
>> hash algorithms.
>>
>> As a consuming parser, it feels nice to know it's always 2 bytes and not
>> have to worry about a missing colon or a missing nul terminator risking
>> a buffer overflow.
> 
> There cannot be buffer overflow, because the length of each digest
> field is known.
> 
> Roberto
> 

I was not referring to the digest, but the digest algorithm.

I wanted opinions on the colon and null terminated string format for 
algorithm identifiers.

The TCG standard log uses the TCG standard enumerations.  They're always 
exactly 2 bytes.  Parsing is trivial.

If IMA uses strings, the attacker can send, e.g., sha1: and not null 
terminate it.  A careful parser can go a byte at a time until it reaches 
a maximum length - if you specify a maximum length.  But it is an attack 
surface.  Is there a corresponding advantage?

[toc] | [prev] | [next] | [standalone]

#1649243 — Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

On 5/23/2017 10:48 PM, Ken Goldman wrote:
> On 5/18/2017 5:38 AM, Roberto Sassu wrote:
>> On 5/17/2017 6:28 PM, Ken Goldman wrote:
>>> On 5/17/2017 3:25 AM, Roberto Sassu wrote:
>>>>
>>>> The format of digestN is: <algo name>:\0<digest value>, the same used
>>>> for the file digest.
>>>
>>> Since the format is changing from the SHA-1 log format anyway ...
>>>
>>> How do people feel about the colon and null terminated string format for
>>> algorithm identifiers?
>>>
>>> The TCG standard enumerations are uint16_t, and there is a registry of
>>> hash algorithms.
>>>
>>> As a consuming parser, it feels nice to know it's always 2 bytes and not
>>> have to worry about a missing colon or a missing nul terminator risking
>>> a buffer overflow.
>>
>> There cannot be buffer overflow, because the length of each digest
>> field is known.
>>
>> Roberto
>>
>
> I was not referring to the digest, but the digest algorithm.
>
> I wanted opinions on the colon and null terminated string format for
> algorithm identifiers.
>
> The TCG standard log uses the TCG standard enumerations.  They're always
> exactly 2 bytes.  Parsing is trivial.

I have two concerns regarding this:

is there a standard way to convert TPM_ALG_ to strings, like a function
exposed by the TSS? If not, suppose that a parser uses openssl to verify
the integrity of event data, by calculating the digest. Then,
the parser should maintain an association table between TPM_ALG_
and a string (the string will be passed to EVP_get_digestbyname()).
When a new TPM algorithm is added to the TCG registry, all parsers
should be modified to update the association table. If IMA sends
a string, only the crypto subsystem has to be updated.

The format I'm proposing for event data digests would be the same
of that used for file digests. Should IMA provide a list with
two different formats?

Roberto


> If IMA uses strings, the attacker can send, e.g., sha1: and not null
> terminate it.  A careful parser can go a byte at a time until it reaches
> a maximum length - if you specify a maximum length.  But it is an attack
> surface.  Is there a corresponding advantage?
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-devel mailing list
> Linux-ima-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel
>

[toc] | [prev] | [next] | [standalone]

#1648746 — Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

On 5/18/2017 5:38 AM, Roberto Sassu wrote:
> 
> There cannot be buffer overflow, because the length of each digest
> field is known.

Crypto Agile: pcr[4] total_digest_len[4]
                digest1_len[4] digest1[digest1_len] ...

The way I read this, the digest length is supplied by the caller, which 
is slightly different from "known".  For example, if I supply a digest 
length of 0xffffffff, a too trusting (buggy) parser could overflow the 
buffer.

total_digest_len is similarly untrusted.  The attacker can send invalid 
values.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web