[PATCH v2 0/7] ima: Add template fields to verify EVM portable signatures

[PATCH v2 0/7] ima: Add template fields to verify EVM portable signatures

[Roberto Sassu]

The recent patch set 'evm: Improve usability of portable signatures' added
the possibility to include EVM portable signatures in the IMA measurement
list.

However, the information necessary to verify the signature were not
included in the IMA measurement list. This patch set introduces new
template fields to accomplish this goal:

- 'iuid': the inode UID;
- 'igid': the inode GID;
- 'imode': the inode mode;
- 'xattrnames': a list of xattr names (separated by |), only if the xattr is
  present;
- 'xattrlengths': a list of xattr lengths (u32), only if the xattr is present;
- 'xattrvalues': a list of xattr values;

Patch 1 adds an helper function to show integers in the measurement list.
Patches 2, 3 and 5 introduce new template fields. Patch 4 make it possible
to verify EVM portable signatures which protect xattrs belonging to LSMs
not enabled in the target platform. Patch 6 introduces the new IMA template
evm-sig. Patch 7 fixes a small issue in evm_write_xattrs() when audit is
not enabled.

This patch set has been tested with:

https://github.com/robertosassu/ima-evm-utils/blob/ima-template-fields-v2-devel-v5/tests/verify_evmsig.test
https://github.com/robertosassu/ima-evm-utils/blob/ima-template-fields-v2-devel-v5/tests/evm_hmac_non_enabled_xattrs.test

The first test aims at checking whether the EVM portable signature included
in the measurement list can be verified with the information also in the
measurement list.

It uses two methods for the verification: the first creates a copy of a
measured file, sets metadata parsed from the measurement list to that copy
and calls evmctl to verify the signature; the second lets evmctl verify the
measurement list directly.

The test is performed without and with an idmapped mount. Given that IMA
always provides the original UID and GID, no more actions are needed in the
second case.

The second test verifies that setting a non-enabled xattr does not change
the HMAC.

The test results are available at:

https://travis-ci.com/github/robertosassu/ima-evm-utils/jobs/508604164
https://travis-ci.com/github/robertosassu/ima-evm-utils/jobs/508604168

This patch set has been also tested on s390x, with and without the
canonical format enabled (the test results are not shown, as the UML kernel
used in Travis is not available for this architecture).

Changelog

v1:
- remove the mntuidmap and mntgidmap template fields and always display the
  original inode UID and GID (suggested by Christian Brauner)
- replace the evmxattrs template field with xattrnames, xattrlengths and
  xattrvalues (suggested by Mimi)
- introduce the new IMA template evm-sig (suggested by Mimi)
- use only one variable in ima_eventinodedac_init_common() (suggested by
  Mimi)

Roberto Sassu (7):
  ima: Add ima_show_template_uint() template library function
  ima: Define new template fields iuid and igid
  ima: Define new template field imode
  evm: Verify portable signatures against all protected xattrs
  ima: Define new template fields xattrnames, xattrlengths and
    xattrvalues
  ima: Define new template evm-sig
  evm: Don't return an error in evm_write_xattrs() if audit is not
    enabled

 Documentation/security/IMA-templates.rst  |   8 +
 include/linux/evm.h                       |  16 ++
 security/integrity/evm/evm.h              |   1 +
 security/integrity/evm/evm_crypto.c       |   7 +
 security/integrity/evm/evm_main.c         | 125 ++++++++++++++--
 security/integrity/evm/evm_secfs.c        |  18 ++-
 security/integrity/ima/ima_template.c     |  18 +++
 security/integrity/ima/ima_template_lib.c | 169 +++++++++++++++++++++-
 security/integrity/ima/ima_template_lib.h |  14 ++
 9 files changed, 362 insertions(+), 14 deletions(-)

-- 
2.25.1

[PATCH v2 1/7] ima: Add ima_show_template_uint() template library function

[Roberto Sassu]

This patch introduces the new function ima_show_template_uint(). This can
be used for showing integers of different sizes in ASCII format. The
function ima_show_template_data_ascii() automatically determines how to
print a stored integer by checking the integer size.

If integers have been written in canonical format,
ima_show_template_data_ascii() calls the appropriate leXX_to_cpu() function
to correctly display the value.

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

diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 4314d9a3514c..f23296c33da1 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -24,7 +24,8 @@ enum data_formats {
 	DATA_FMT_DIGEST = 0,
 	DATA_FMT_DIGEST_WITH_ALGO,
 	DATA_FMT_STRING,
-	DATA_FMT_HEX
+	DATA_FMT_HEX,
+	DATA_FMT_UINT
 };
 
 static int ima_write_template_field_data(const void *data, const u32 datalen,
@@ -88,6 +89,35 @@ static void ima_show_template_data_ascii(struct seq_file *m,
 	case DATA_FMT_STRING:
 		seq_printf(m, "%s", buf_ptr);
 		break;
+	case DATA_FMT_UINT:
+		switch (field_data->len) {
+		case sizeof(u8):
+			seq_printf(m, "%u", *(u8 *)buf_ptr);
+			break;
+		case sizeof(u16):
+			if (ima_canonical_fmt)
+				seq_printf(m, "%u",
+					   le16_to_cpu(*(u16 *)buf_ptr));
+			else
+				seq_printf(m, "%u", *(u16 *)buf_ptr);
+			break;
+		case sizeof(u32):
+			if (ima_canonical_fmt)
+				seq_printf(m, "%u",
+					   le32_to_cpu(*(u32 *)buf_ptr));
+			else
+				seq_printf(m, "%u", *(u32 *)buf_ptr);
+			break;
+		case sizeof(u64):
+			if (ima_canonical_fmt)
+				seq_printf(m, "%llu",
+					   le64_to_cpu(*(u64 *)buf_ptr));
+			else
+				seq_printf(m, "%llu", *(u64 *)buf_ptr);
+			break;
+		default:
+			break;
+		}
 	default:
 		break;
 	}
@@ -163,6 +193,12 @@ void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
 	ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
 }
 
+void ima_show_template_uint(struct seq_file *m, enum ima_show_type show,
+			    struct ima_field_data *field_data)
+{
+	ima_show_template_field_data(m, show, DATA_FMT_UINT, field_data);
+}
+
 /**
  * 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 f4b2a2056d1d..54b67c80b315 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -27,6 +27,8 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
 			   struct ima_field_data *field_data);
 void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
 			   struct ima_field_data *field_data);
+void ima_show_template_uint(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);
-- 
2.25.1

[PATCH v2 2/7] ima: Define new template fields iuid and igid

[Roberto Sassu]

This patch defines the new template fields iuid and igid, which include
respectively the inode UID and GID. For idmapped mounts, still the original
UID and GID are provided.

These fields can be used to verify the EVM portable signature, if it was
included with the template fields sig or evmsig.

Cc: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/security/IMA-templates.rst  |  2 +
 security/integrity/ima/ima_template.c     |  4 ++
 security/integrity/ima/ima_template_lib.c | 45 +++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  4 ++
 4 files changed, 55 insertions(+)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 9f3e86ab028a..bf8ce4cf5878 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -75,6 +75,8 @@ descriptors by adding their identifier to the format string
  - 'modsig' the appended file signature;
  - 'buf': the buffer data that was used to generate the hash without size limitations;
  - 'evmsig': the EVM portable signature;
+ - 'iuid': the inode UID;
+ - 'igid': the inode GID;
 
 
 Below, there is the list of defined template descriptors:
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 7a60848c04a5..a5ecd9e2581b 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -47,6 +47,10 @@ static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_sig},
 	{.field_id = "evmsig", .field_init = ima_eventevmsig_init,
 	 .field_show = ima_show_template_sig},
+	{.field_id = "iuid", .field_init = ima_eventinodeuid_init,
+	 .field_show = ima_show_template_uint},
+	{.field_id = "igid", .field_init = ima_eventinodegid_init,
+	 .field_show = ima_show_template_uint},
 };
 
 /*
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index f23296c33da1..87b40f391739 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -551,3 +551,48 @@ int ima_eventevmsig_init(struct ima_event_data *event_data,
 	kfree(xattr_data);
 	return rc;
 }
+
+static int ima_eventinodedac_init_common(struct ima_event_data *event_data,
+					 struct ima_field_data *field_data,
+					 bool get_uid)
+{
+	unsigned int id;
+
+	if (!event_data->file)
+		return 0;
+
+	if (get_uid)
+		id = i_uid_read(file_inode(event_data->file));
+	else
+		id = i_gid_read(file_inode(event_data->file));
+
+	if (ima_canonical_fmt) {
+		if (sizeof(id) == sizeof(u16))
+			id = cpu_to_le16(id);
+		else
+			id = cpu_to_le32(id);
+	}
+
+	return ima_write_template_field_data((void *)&id, sizeof(id),
+					     DATA_FMT_UINT, field_data);
+}
+
+/*
+ *  ima_eventinodeuid_init - include the inode UID as part of the template
+ *  data
+ */
+int ima_eventinodeuid_init(struct ima_event_data *event_data,
+			   struct ima_field_data *field_data)
+{
+	return ima_eventinodedac_init_common(event_data, field_data, true);
+}
+
+/*
+ *  ima_eventinodegid_init - include the inode GID as part of the template
+ *  data
+ */
+int ima_eventinodegid_init(struct ima_event_data *event_data,
+			   struct ima_field_data *field_data)
+{
+	return ima_eventinodedac_init_common(event_data, field_data, false);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 54b67c80b315..b0aaf109f386 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -50,4 +50,8 @@ int ima_eventmodsig_init(struct ima_event_data *event_data,
 			 struct ima_field_data *field_data);
 int ima_eventevmsig_init(struct ima_event_data *event_data,
 			 struct ima_field_data *field_data);
+int ima_eventinodeuid_init(struct ima_event_data *event_data,
+			   struct ima_field_data *field_data);
+int ima_eventinodegid_init(struct ima_event_data *event_data,
+			   struct ima_field_data *field_data);
 #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
2.25.1

[PATCH v2 3/7] ima: Define new template field imode

[Roberto Sassu]

This patch defines the new template field imode, which includes the
inode mode. It can be used by a remote verifier to verify the EVM portable
signature, if it was included with the template fields sig or evmsig.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/security/IMA-templates.rst  |  1 +
 security/integrity/ima/ima_template.c     |  2 ++
 security/integrity/ima/ima_template_lib.c | 22 ++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  2 ++
 4 files changed, 27 insertions(+)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index bf8ce4cf5878..65c1ce451d08 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -77,6 +77,7 @@ descriptors by adding their identifier to the format string
  - 'evmsig': the EVM portable signature;
  - 'iuid': the inode UID;
  - 'igid': the inode GID;
+ - 'imode': the inode mode;
 
 
 Below, there is the list of defined template descriptors:
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index a5ecd9e2581b..43784f2bf8bd 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -51,6 +51,8 @@ static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_uint},
 	{.field_id = "igid", .field_init = ima_eventinodegid_init,
 	 .field_show = ima_show_template_uint},
+	{.field_id = "imode", .field_init = ima_eventinodemode_init,
+	 .field_show = ima_show_template_uint},
 };
 
 /*
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 87b40f391739..3156fb34b1af 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -596,3 +596,25 @@ int ima_eventinodegid_init(struct ima_event_data *event_data,
 {
 	return ima_eventinodedac_init_common(event_data, field_data, false);
 }
+
+/*
+ *  ima_eventinodemode_init - include the inode mode as part of the template
+ *  data
+ */
+int ima_eventinodemode_init(struct ima_event_data *event_data,
+			    struct ima_field_data *field_data)
+{
+	struct inode *inode;
+	umode_t mode;
+
+	if (!event_data->file)
+		return 0;
+
+	inode = file_inode(event_data->file);
+	mode = inode->i_mode;
+	if (ima_canonical_fmt)
+		mode = cpu_to_le16(mode);
+
+	return ima_write_template_field_data((char *)&mode, sizeof(mode),
+					     DATA_FMT_UINT, field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index b0aaf109f386..6509af4a97ee 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -54,4 +54,6 @@ int ima_eventinodeuid_init(struct ima_event_data *event_data,
 			   struct ima_field_data *field_data);
 int ima_eventinodegid_init(struct ima_event_data *event_data,
 			   struct ima_field_data *field_data);
+int ima_eventinodemode_init(struct ima_event_data *event_data,
+			    struct ima_field_data *field_data);
 #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
2.25.1

[PATCH v2 4/7] evm: Verify portable signatures against all protected xattrs

[Roberto Sassu]

Currently, the evm_config_default_xattrnames array contains xattr names
only related to LSMs which are enabled in the kernel configuration.
However, EVM portable signatures do not depend on local information and a
vendor might include in the signature calculation xattrs that are not
enabled in the target platform.

Just including all xattrs names in evm_config_default_xattrnames is not a
safe approach, because a target system might have already calculated
signatures or HMACs based only on the enabled xattrs. After applying this
patch, EVM would verify those signatures and HMACs with all xattrs instead.
The non-enabled ones, which could possibly exist, would cause a
verification error.

Thus, this patch adds a new field named enabled to the xattr_list
structure, which is set to true if the LSM associated to a given xattr name
is enabled in the kernel configuration. The non-enabled xattrs are taken
into account only in evm_calc_hmac_or_hash(), if the passed security.evm
type is EVM_XATTR_PORTABLE_DIGSIG.

The new function evm_protected_xattr_if_enabled() has been defined so that
IMA can include all protected xattrs and not only the enabled ones in the
measurement list, if the new template fields xattrnames, xattrlengths or
xattrvalues have been included in the template format.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/evm.h                 |  6 ++++
 security/integrity/evm/evm.h        |  1 +
 security/integrity/evm/evm_crypto.c |  7 ++++
 security/integrity/evm/evm_main.c   | 56 +++++++++++++++++++++++------
 security/integrity/evm/evm_secfs.c  | 16 +++++++--
 5 files changed, 74 insertions(+), 12 deletions(-)

diff --git a/include/linux/evm.h b/include/linux/evm.h
index 31ef1dbbb3ac..5011a299c251 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -38,6 +38,7 @@ extern int evm_inode_init_security(struct inode *inode,
 				   const struct xattr *xattr_array,
 				   struct xattr *evm);
 extern bool evm_revalidate_status(const char *xattr_name);
+extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
 #ifdef CONFIG_FS_POSIX_ACL
 extern int posix_xattr_acl(const char *xattrname);
 #else
@@ -114,5 +115,10 @@ static inline bool evm_revalidate_status(const char *xattr_name)
 	return false;
 }
 
+static inline int evm_protected_xattr_if_enabled(const char *req_xattr_name)
+{
+	return false;
+}
+
 #endif /* CONFIG_EVM */
 #endif /* LINUX_EVM_H */
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index f2fef2b5ed51..0d44f41d16f8 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -29,6 +29,7 @@
 struct xattr_list {
 	struct list_head list;
 	char *name;
+	bool enabled;
 };
 
 extern int evm_initialized;
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index d76b006cbcc4..1628e2ca9862 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -216,6 +216,13 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		if (strcmp(xattr->name, XATTR_NAME_IMA) == 0)
 			is_ima = true;
 
+		/*
+		 * Skip non-enabled xattrs for locally calculated
+		 * signatures/HMACs.
+		 */
+		if (type != EVM_XATTR_PORTABLE_DIGSIG && !xattr->enabled)
+			continue;
+
 		if ((req_xattr_name && req_xattr_value)
 		    && !strcmp(xattr->name, req_xattr_name)) {
 			error = 0;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 0196168aeb7d..ee4e17a790fb 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -34,24 +34,44 @@ static const char * const integrity_status_msg[] = {
 int evm_hmac_attrs;
 
 static struct xattr_list evm_config_default_xattrnames[] = {
+	{.name = XATTR_NAME_SELINUX,
 #ifdef CONFIG_SECURITY_SELINUX
-	{.name = XATTR_NAME_SELINUX},
+	 .enabled = true
 #endif
+	},
+	{.name = XATTR_NAME_SMACK,
 #ifdef CONFIG_SECURITY_SMACK
-	{.name = XATTR_NAME_SMACK},
+	 .enabled = true
+#endif
+	},
+	{.name = XATTR_NAME_SMACKEXEC,
+#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
+	 .enabled = true
+#endif
+	},
+	{.name = XATTR_NAME_SMACKTRANSMUTE,
 #ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	{.name = XATTR_NAME_SMACKEXEC},
-	{.name = XATTR_NAME_SMACKTRANSMUTE},
-	{.name = XATTR_NAME_SMACKMMAP},
+	 .enabled = true
 #endif
+	},
+	{.name = XATTR_NAME_SMACKMMAP,
+#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
+	 .enabled = true
 #endif
+	},
+	{.name = XATTR_NAME_APPARMOR,
 #ifdef CONFIG_SECURITY_APPARMOR
-	{.name = XATTR_NAME_APPARMOR},
+	 .enabled = true
 #endif
+	},
+	{.name = XATTR_NAME_IMA,
 #ifdef CONFIG_IMA_APPRAISE
-	{.name = XATTR_NAME_IMA},
+	 .enabled = true
 #endif
-	{.name = XATTR_NAME_CAPS},
+	},
+	{.name = XATTR_NAME_CAPS,
+	 .enabled = true
+	},
 };
 
 LIST_HEAD(evm_config_xattrnames);
@@ -76,7 +96,9 @@ static void __init evm_init_config(void)
 
 	pr_info("Initialising EVM extended attributes:\n");
 	for (i = 0; i < xattrs; i++) {
-		pr_info("%s\n", evm_config_default_xattrnames[i].name);
+		pr_info("%s%s\n", evm_config_default_xattrnames[i].name,
+			!evm_config_default_xattrnames[i].enabled ?
+			" (disabled)" : "");
 		list_add_tail(&evm_config_default_xattrnames[i].list,
 			      &evm_config_xattrnames);
 	}
@@ -257,7 +279,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	return evm_status;
 }
 
-static int evm_protected_xattr(const char *req_xattr_name)
+static int evm_protected_xattr_common(const char *req_xattr_name,
+				      bool all_xattrs)
 {
 	int namelen;
 	int found = 0;
@@ -265,6 +288,9 @@ static int evm_protected_xattr(const char *req_xattr_name)
 
 	namelen = strlen(req_xattr_name);
 	list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
+		if (!all_xattrs && !xattr->enabled)
+			continue;
+
 		if ((strlen(xattr->name) == namelen)
 		    && (strncmp(req_xattr_name, xattr->name, namelen) == 0)) {
 			found = 1;
@@ -281,6 +307,16 @@ static int evm_protected_xattr(const char *req_xattr_name)
 	return found;
 }
 
+static int evm_protected_xattr(const char *req_xattr_name)
+{
+	return evm_protected_xattr_common(req_xattr_name, false);
+}
+
+int evm_protected_xattr_if_enabled(const char *req_xattr_name)
+{
+	return evm_protected_xattr_common(req_xattr_name, true);
+}
+
 /**
  * evm_verifyxattr - verify the integrity of the requested xattr
  * @dentry: object of the verify xattr
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index c175e2b659e4..ec3ed75a347d 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -138,8 +138,12 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
 	if (rc)
 		return -ERESTARTSYS;
 
-	list_for_each_entry(xattr, &evm_config_xattrnames, list)
+	list_for_each_entry(xattr, &evm_config_xattrnames, list) {
+		if (!xattr->enabled)
+			continue;
+
 		size += strlen(xattr->name) + 1;
+	}
 
 	temp = kmalloc(size + 1, GFP_KERNEL);
 	if (!temp) {
@@ -148,6 +152,9 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
 	}
 
 	list_for_each_entry(xattr, &evm_config_xattrnames, list) {
+		if (!xattr->enabled)
+			continue;
+
 		sprintf(temp + offset, "%s\n", xattr->name);
 		offset += strlen(xattr->name) + 1;
 	}
@@ -198,6 +205,7 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
 		goto out;
 	}
 
+	xattr->enabled = true;
 	xattr->name = memdup_user_nul(buf, count);
 	if (IS_ERR(xattr->name)) {
 		err = PTR_ERR(xattr->name);
@@ -244,6 +252,10 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
 	list_for_each_entry(tmp, &evm_config_xattrnames, list) {
 		if (strcmp(xattr->name, tmp->name) == 0) {
 			err = -EEXIST;
+			if (!tmp->enabled) {
+				tmp->enabled = true;
+				err = count;
+			}
 			mutex_unlock(&xattr_list_mutex);
 			goto out;
 		}
@@ -255,7 +267,7 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
 	audit_log_end(ab);
 	return count;
 out:
-	audit_log_format(ab, " res=%d", err);
+	audit_log_format(ab, " res=%d", (err < 0) ? err : 0);
 	audit_log_end(ab);
 	if (xattr) {
 		kfree(xattr->name);
-- 
2.25.1

[PATCH v2 5/7] ima: Define new template fields xattrnames, xattrlengths and xattrvalues

[Roberto Sassu]

This patch defines the new template fields xattrnames, xattrlengths and
xattrvalues, which contain respectively a list of xattr names (strings,
separated by |), lengths (u32, hex) and values (hex). If an xattr is not
present, the name and length are not displayed in the measurement list.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/security/IMA-templates.rst  |  4 ++
 include/linux/evm.h                       | 10 ++++
 security/integrity/evm/evm_main.c         | 69 +++++++++++++++++++++++
 security/integrity/ima/ima_template.c     |  9 +++
 security/integrity/ima/ima_template_lib.c | 64 +++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  6 ++
 6 files changed, 162 insertions(+)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 65c1ce451d08..6a58760a0a35 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -78,6 +78,10 @@ descriptors by adding their identifier to the format string
  - 'iuid': the inode UID;
  - 'igid': the inode GID;
  - 'imode': the inode mode;
+ - 'xattrnames': a list of xattr names (separated by |), only if the xattr is
+    present;
+ - 'xattrlengths': a list of xattr lengths (u32), only if the xattr is present;
+ - 'xattrvalues': a list of xattr values;
 
 
 Below, there is the list of defined template descriptors:
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 5011a299c251..4c374be70247 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -39,6 +39,9 @@ extern int evm_inode_init_security(struct inode *inode,
 				   struct xattr *evm);
 extern bool evm_revalidate_status(const char *xattr_name);
 extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
+extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
+				     int buffer_size, char type,
+				     bool canonical_fmt);
 #ifdef CONFIG_FS_POSIX_ACL
 extern int posix_xattr_acl(const char *xattrname);
 #else
@@ -120,5 +123,12 @@ static inline int evm_protected_xattr_if_enabled(const char *req_xattr_name)
 	return false;
 }
 
+static inline int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
+					    int buffer_size, char type,
+					    bool canonical_fmt)
+{
+	return -EOPNOTSUPP;
+}
+
 #endif /* CONFIG_EVM */
 #endif /* LINUX_EVM_H */
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index ee4e17a790fb..2c226e634ae9 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -317,6 +317,75 @@ int evm_protected_xattr_if_enabled(const char *req_xattr_name)
 	return evm_protected_xattr_common(req_xattr_name, true);
 }
 
+/**
+ * evm_read_protected_xattrs - read EVM protected xattr names, lengths, values
+ * @dentry: dentry of the read xattrs
+ * @inode: inode of the read xattrs
+ * @buffer: buffer xattr names, lengths or values are copied to
+ * @buffer_size: size of buffer
+ * @type: n: names, l: lengths, v: values
+ * @canonical_fmt: data format (true: little endian, false: native format)
+ *
+ * Read protected xattr names (separated by |), lengths (u32) or values for a
+ * given dentry and return the total size of copied data. If buffer is NULL,
+ * just return the total size.
+ *
+ * Returns the total size on success, a negative value on error.
+ */
+int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
+			      int buffer_size, char type, bool canonical_fmt)
+{
+	struct xattr_list *xattr;
+	int rc, size, total_size = 0;
+
+	list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
+		rc = __vfs_getxattr(dentry, d_backing_inode(dentry),
+				    xattr->name, NULL, 0);
+		if (rc < 0 && rc == -ENODATA)
+			continue;
+		else if (rc < 0)
+			return rc;
+
+		switch (type) {
+		case 'n':
+			size = strlen(xattr->name) + 1;
+			if (buffer) {
+				if (total_size)
+					*(buffer + total_size - 1) = '|';
+
+				memcpy(buffer + total_size, xattr->name, size);
+			}
+			break;
+		case 'l':
+			size = sizeof(u32);
+			if (buffer) {
+				if (canonical_fmt)
+					rc = cpu_to_le32(rc);
+
+				*(u32 *)(buffer + total_size) = rc;
+			}
+			break;
+		case 'v':
+			size = rc;
+			if (buffer) {
+				rc = __vfs_getxattr(dentry,
+					d_backing_inode(dentry), xattr->name,
+					buffer + total_size,
+					buffer_size - total_size);
+				if (rc < 0)
+					return rc;
+			}
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		total_size += size;
+	}
+
+	return total_size;
+}
+
 /**
  * evm_verifyxattr - verify the integrity of the requested xattr
  * @dentry: object of the verify xattr
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 43784f2bf8bd..159a31d2fcdf 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -53,6 +53,15 @@ static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_uint},
 	{.field_id = "imode", .field_init = ima_eventinodemode_init,
 	 .field_show = ima_show_template_uint},
+	{.field_id = "xattrnames",
+	 .field_init = ima_eventinodexattrnames_init,
+	 .field_show = ima_show_template_string},
+	{.field_id = "xattrlengths",
+	 .field_init = ima_eventinodexattrlengths_init,
+	 .field_show = ima_show_template_sig},
+	{.field_id = "xattrvalues",
+	 .field_init = ima_eventinodexattrvalues_init,
+	 .field_show = ima_show_template_sig},
 };
 
 /*
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 3156fb34b1af..10a1349542e6 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -11,6 +11,7 @@
 
 #include "ima_template_lib.h"
 #include <linux/xattr.h>
+#include <linux/evm.h>
 
 static bool ima_template_hash_algo_allowed(u8 algo)
 {
@@ -618,3 +619,66 @@ int ima_eventinodemode_init(struct ima_event_data *event_data,
 	return ima_write_template_field_data((char *)&mode, sizeof(mode),
 					     DATA_FMT_UINT, field_data);
 }
+
+int ima_eventinodexattrs_init_common(struct ima_event_data *event_data,
+				     struct ima_field_data *field_data,
+				     char type)
+{
+	u8 *buffer = NULL;
+	int rc;
+
+	if (!event_data->file)
+		return 0;
+
+	rc = evm_read_protected_xattrs(file_dentry(event_data->file), NULL, 0,
+				       type, ima_canonical_fmt);
+	if (rc < 0)
+		return 0;
+
+	buffer = kmalloc(rc, GFP_KERNEL);
+	if (!buffer)
+		return 0;
+
+	rc = evm_read_protected_xattrs(file_dentry(event_data->file), buffer,
+				       rc, type, ima_canonical_fmt);
+	if (rc < 0) {
+		rc = 0;
+		goto out;
+	}
+
+	rc = ima_write_template_field_data((char *)buffer, rc, DATA_FMT_HEX,
+					   field_data);
+out:
+	kfree(buffer);
+	return rc;
+}
+
+/*
+ *  ima_eventinodexattrnames_init - include a list of xattr names as part of the
+ *  template data
+ */
+int ima_eventinodexattrnames_init(struct ima_event_data *event_data,
+				  struct ima_field_data *field_data)
+{
+	return ima_eventinodexattrs_init_common(event_data, field_data, 'n');
+}
+
+/*
+ *  ima_eventinodexattrlengths_init - include a list of xattr lengths as part of
+ *  the template data
+ */
+int ima_eventinodexattrlengths_init(struct ima_event_data *event_data,
+				    struct ima_field_data *field_data)
+{
+	return ima_eventinodexattrs_init_common(event_data, field_data, 'l');
+}
+
+/*
+ *  ima_eventinodexattrvalues_init - include a list of xattr values as part of
+ *  the template data
+ */
+int ima_eventinodexattrvalues_init(struct ima_event_data *event_data,
+				   struct ima_field_data *field_data)
+{
+	return ima_eventinodexattrs_init_common(event_data, field_data, 'v');
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6509af4a97ee..c71f1de95753 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -56,4 +56,10 @@ int ima_eventinodegid_init(struct ima_event_data *event_data,
 			   struct ima_field_data *field_data);
 int ima_eventinodemode_init(struct ima_event_data *event_data,
 			    struct ima_field_data *field_data);
+int ima_eventinodexattrnames_init(struct ima_event_data *event_data,
+				  struct ima_field_data *field_data);
+int ima_eventinodexattrlengths_init(struct ima_event_data *event_data,
+				    struct ima_field_data *field_data);
+int ima_eventinodexattrvalues_init(struct ima_event_data *event_data,
+				   struct ima_field_data *field_data);
 #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
2.25.1

[PATCH v2 6/7] ima: Define new template evm-sig

[Roberto Sassu]

With the recent introduction of the evmsig template field, remote verifiers
can obtain the EVM portable signature instead of the IMA signature, to
verify file metadata.

After introducing the new fields to include file metadata in the
measurement list, this patch finally defines the evm-sig template, whose
format is:

d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode

xattrnames, xattrlengths and xattrvalues are populated only from defined
EVM protected xattrs, i.e. the ones that EVM considers to verify the
portable signature. xattrnames and xattrlengths are populated only if the
xattr is present.

xattrnames and xattrlengths are not necessary for verifying the EVM
portable signature, but they are included for completeness of information,
if a remote verifier wants to infer more from file metadata.

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

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 6a58760a0a35..5adc22f99496 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -91,6 +91,7 @@ Below, there is the list of defined template descriptors:
  - "ima-sig": its format is ``d-ng|n-ng|sig``;
  - "ima-buf": its format is ``d-ng|n-ng|buf``;
  - "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``;
+ - "evm-sig": its format is ``d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode``;
 
 
 Use
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 159a31d2fcdf..be435efe6122 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -22,6 +22,9 @@ static struct ima_template_desc builtin_templates[] = {
 	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
 	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
 	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
+	{.name = "evm-sig",
+	 .fmt = "d-ng|n-ng|evmsig|"
+		"xattrnames|xattrlengths|xattrvalues|iuid|igid|imode"},
 	{.name = "", .fmt = ""},	/* placeholder for a custom format */
 };
 
-- 
2.25.1

[PATCH v2 7/7] evm: Don't return an error in evm_write_xattrs() if audit is not enabled

[Roberto Sassu]

This patch avoids that evm_write_xattrs() returns an error when audit is
not enabled. The ab variable can be NULL and still be passed to the other
audit_log_() functions, as those functions do not include any instruction.

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

diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index ec3ed75a347d..07e263ae13e0 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -196,7 +196,7 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
 
 	ab = audit_log_start(audit_context(), GFP_KERNEL,
 			     AUDIT_INTEGRITY_EVM_XATTR);
-	if (!ab)
+	if (!ab && IS_ENABLED(CONFIG_AUDIT))
 		return -ENOMEM;
 
 	xattr = kmalloc(sizeof(struct xattr_list), GFP_KERNEL);
-- 
2.25.1

Re: [PATCH v2 2/7] ima: Define new template fields iuid and igid

[Christian Brauner]

On Fri, May 28, 2021 at 09:38:07AM +0200, Roberto Sassu wrote:
> This patch defines the new template fields iuid and igid, which include
> respectively the inode UID and GID. For idmapped mounts, still the original
> UID and GID are provided.
> 
> These fields can be used to verify the EVM portable signature, if it was
> included with the template fields sig or evmsig.
> 
> Cc: Christian Brauner <christian.brauner@ubuntu.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---

That's fine with me. Thanks, Robert!
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

>  Documentation/security/IMA-templates.rst  |  2 +
>  security/integrity/ima/ima_template.c     |  4 ++
>  security/integrity/ima/ima_template_lib.c | 45 +++++++++++++++++++++++
>  security/integrity/ima/ima_template_lib.h |  4 ++
>  4 files changed, 55 insertions(+)
> 
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index 9f3e86ab028a..bf8ce4cf5878 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -75,6 +75,8 @@ descriptors by adding their identifier to the format string
>   - 'modsig' the appended file signature;
>   - 'buf': the buffer data that was used to generate the hash without size limitations;
>   - 'evmsig': the EVM portable signature;
> + - 'iuid': the inode UID;
> + - 'igid': the inode GID;
>  
>  
>  Below, there is the list of defined template descriptors:
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 7a60848c04a5..a5ecd9e2581b 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -47,6 +47,10 @@ static const struct ima_template_field supported_fields[] = {
>  	 .field_show = ima_show_template_sig},
>  	{.field_id = "evmsig", .field_init = ima_eventevmsig_init,
>  	 .field_show = ima_show_template_sig},
> +	{.field_id = "iuid", .field_init = ima_eventinodeuid_init,
> +	 .field_show = ima_show_template_uint},
> +	{.field_id = "igid", .field_init = ima_eventinodegid_init,
> +	 .field_show = ima_show_template_uint},
>  };
>  
>  /*
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index f23296c33da1..87b40f391739 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -551,3 +551,48 @@ int ima_eventevmsig_init(struct ima_event_data *event_data,
>  	kfree(xattr_data);
>  	return rc;
>  }
> +
> +static int ima_eventinodedac_init_common(struct ima_event_data *event_data,
> +					 struct ima_field_data *field_data,
> +					 bool get_uid)
> +{
> +	unsigned int id;
> +
> +	if (!event_data->file)
> +		return 0;
> +
> +	if (get_uid)
> +		id = i_uid_read(file_inode(event_data->file));
> +	else
> +		id = i_gid_read(file_inode(event_data->file));
> +
> +	if (ima_canonical_fmt) {
> +		if (sizeof(id) == sizeof(u16))
> +			id = cpu_to_le16(id);
> +		else
> +			id = cpu_to_le32(id);
> +	}
> +
> +	return ima_write_template_field_data((void *)&id, sizeof(id),
> +					     DATA_FMT_UINT, field_data);
> +}
> +
> +/*
> + *  ima_eventinodeuid_init - include the inode UID as part of the template
> + *  data
> + */
> +int ima_eventinodeuid_init(struct ima_event_data *event_data,
> +			   struct ima_field_data *field_data)
> +{
> +	return ima_eventinodedac_init_common(event_data, field_data, true);
> +}
> +
> +/*
> + *  ima_eventinodegid_init - include the inode GID as part of the template
> + *  data
> + */
> +int ima_eventinodegid_init(struct ima_event_data *event_data,
> +			   struct ima_field_data *field_data)
> +{
> +	return ima_eventinodedac_init_common(event_data, field_data, false);
> +}
> diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
> index 54b67c80b315..b0aaf109f386 100644
> --- a/security/integrity/ima/ima_template_lib.h
> +++ b/security/integrity/ima/ima_template_lib.h
> @@ -50,4 +50,8 @@ int ima_eventmodsig_init(struct ima_event_data *event_data,
>  			 struct ima_field_data *field_data);
>  int ima_eventevmsig_init(struct ima_event_data *event_data,
>  			 struct ima_field_data *field_data);
> +int ima_eventinodeuid_init(struct ima_event_data *event_data,
> +			   struct ima_field_data *field_data);
> +int ima_eventinodegid_init(struct ima_event_data *event_data,
> +			   struct ima_field_data *field_data);
>  #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
> -- 
> 2.25.1
> 

Re: [PATCH v2 0/7] ima: Add template fields to verify EVM portable signatures

[Mimi Zohar]

On Fri, 2021-05-28 at 09:38 +0200, Roberto Sassu wrote:
> The recent patch set 'evm: Improve usability of portable signatures' added
> the possibility to include EVM portable signatures in the IMA measurement
> list.
> 
> However, the information necessary to verify the signature were not
> included in the IMA measurement list. This patch set introduces new
> template fields to accomplish this goal:
> 
> - 'iuid': the inode UID;
> - 'igid': the inode GID;
> - 'imode': the inode mode;
> - 'xattrnames': a list of xattr names (separated by |), only if the xattr is
>   present;
> - 'xattrlengths': a list of xattr lengths (u32), only if the xattr is present;
> - 'xattrvalues': a list of xattr values;
> 
> Patch 1 adds an helper function to show integers in the measurement list.
> Patches 2, 3 and 5 introduce new template fields. Patch 4 make it possible
> to verify EVM portable signatures which protect xattrs belonging to LSMs
> not enabled in the target platform. Patch 6 introduces the new IMA template
> evm-sig. Patch 7 fixes a small issue in evm_write_xattrs() when audit is
> not enabled.

Thanks, Roberto. 

Applied to: git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
next-integrity-testing branch.

Mimi

[RESEND][PATCH v2 5/7] ima: Define new template fields xattrnames, xattrlengths and xattrvalues

[Roberto Sassu]

This patch defines the new template fields xattrnames, xattrlengths and
xattrvalues, which contain respectively a list of xattr names (strings,
separated by |), lengths (u32, hex) and values (hex). If an xattr is not
present, the name and length are not displayed in the measurement list.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/security/IMA-templates.rst  |  4 ++
 include/linux/evm.h                       | 10 ++++
 security/integrity/evm/evm_main.c         | 69 +++++++++++++++++++++++
 security/integrity/ima/ima_template.c     |  9 +++
 security/integrity/ima/ima_template_lib.c | 64 +++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  6 ++
 6 files changed, 162 insertions(+)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 65c1ce451d08..6a58760a0a35 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -78,6 +78,10 @@ descriptors by adding their identifier to the format string
  - 'iuid': the inode UID;
  - 'igid': the inode GID;
  - 'imode': the inode mode;
+ - 'xattrnames': a list of xattr names (separated by |), only if the xattr is
+    present;
+ - 'xattrlengths': a list of xattr lengths (u32), only if the xattr is present;
+ - 'xattrvalues': a list of xattr values;
 
 
 Below, there is the list of defined template descriptors:
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 5011a299c251..4c374be70247 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -39,6 +39,9 @@ extern int evm_inode_init_security(struct inode *inode,
 				   struct xattr *evm);
 extern bool evm_revalidate_status(const char *xattr_name);
 extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
+extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
+				     int buffer_size, char type,
+				     bool canonical_fmt);
 #ifdef CONFIG_FS_POSIX_ACL
 extern int posix_xattr_acl(const char *xattrname);
 #else
@@ -120,5 +123,12 @@ static inline int evm_protected_xattr_if_enabled(const char *req_xattr_name)
 	return false;
 }
 
+static inline int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
+					    int buffer_size, char type,
+					    bool canonical_fmt)
+{
+	return -EOPNOTSUPP;
+}
+
 #endif /* CONFIG_EVM */
 #endif /* LINUX_EVM_H */
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index ee4e17a790fb..2c226e634ae9 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -317,6 +317,75 @@ int evm_protected_xattr_if_enabled(const char *req_xattr_name)
 	return evm_protected_xattr_common(req_xattr_name, true);
 }
 
+/**
+ * evm_read_protected_xattrs - read EVM protected xattr names, lengths, values
+ * @dentry: dentry of the read xattrs
+ * @inode: inode of the read xattrs
+ * @buffer: buffer xattr names, lengths or values are copied to
+ * @buffer_size: size of buffer
+ * @type: n: names, l: lengths, v: values
+ * @canonical_fmt: data format (true: little endian, false: native format)
+ *
+ * Read protected xattr names (separated by |), lengths (u32) or values for a
+ * given dentry and return the total size of copied data. If buffer is NULL,
+ * just return the total size.
+ *
+ * Returns the total size on success, a negative value on error.
+ */
+int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
+			      int buffer_size, char type, bool canonical_fmt)
+{
+	struct xattr_list *xattr;
+	int rc, size, total_size = 0;
+
+	list_for_each_entry_lockless(xattr, &evm_config_xattrnames, list) {
+		rc = __vfs_getxattr(dentry, d_backing_inode(dentry),
+				    xattr->name, NULL, 0);
+		if (rc < 0 && rc == -ENODATA)
+			continue;
+		else if (rc < 0)
+			return rc;
+
+		switch (type) {
+		case 'n':
+			size = strlen(xattr->name) + 1;
+			if (buffer) {
+				if (total_size)
+					*(buffer + total_size - 1) = '|';
+
+				memcpy(buffer + total_size, xattr->name, size);
+			}
+			break;
+		case 'l':
+			size = sizeof(u32);
+			if (buffer) {
+				if (canonical_fmt)
+					rc = cpu_to_le32(rc);
+
+				*(u32 *)(buffer + total_size) = rc;
+			}
+			break;
+		case 'v':
+			size = rc;
+			if (buffer) {
+				rc = __vfs_getxattr(dentry,
+					d_backing_inode(dentry), xattr->name,
+					buffer + total_size,
+					buffer_size - total_size);
+				if (rc < 0)
+					return rc;
+			}
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		total_size += size;
+	}
+
+	return total_size;
+}
+
 /**
  * evm_verifyxattr - verify the integrity of the requested xattr
  * @dentry: object of the verify xattr
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 43784f2bf8bd..159a31d2fcdf 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -53,6 +53,15 @@ static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_uint},
 	{.field_id = "imode", .field_init = ima_eventinodemode_init,
 	 .field_show = ima_show_template_uint},
+	{.field_id = "xattrnames",
+	 .field_init = ima_eventinodexattrnames_init,
+	 .field_show = ima_show_template_string},
+	{.field_id = "xattrlengths",
+	 .field_init = ima_eventinodexattrlengths_init,
+	 .field_show = ima_show_template_sig},
+	{.field_id = "xattrvalues",
+	 .field_init = ima_eventinodexattrvalues_init,
+	 .field_show = ima_show_template_sig},
 };
 
 /*
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 3156fb34b1af..518fd50ea48a 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -11,6 +11,7 @@
 
 #include "ima_template_lib.h"
 #include <linux/xattr.h>
+#include <linux/evm.h>
 
 static bool ima_template_hash_algo_allowed(u8 algo)
 {
@@ -618,3 +619,66 @@ int ima_eventinodemode_init(struct ima_event_data *event_data,
 	return ima_write_template_field_data((char *)&mode, sizeof(mode),
 					     DATA_FMT_UINT, field_data);
 }
+
+static int ima_eventinodexattrs_init_common(struct ima_event_data *event_data,
+					    struct ima_field_data *field_data,
+					    char type)
+{
+	u8 *buffer = NULL;
+	int rc;
+
+	if (!event_data->file)
+		return 0;
+
+	rc = evm_read_protected_xattrs(file_dentry(event_data->file), NULL, 0,
+				       type, ima_canonical_fmt);
+	if (rc < 0)
+		return 0;
+
+	buffer = kmalloc(rc, GFP_KERNEL);
+	if (!buffer)
+		return 0;
+
+	rc = evm_read_protected_xattrs(file_dentry(event_data->file), buffer,
+				       rc, type, ima_canonical_fmt);
+	if (rc < 0) {
+		rc = 0;
+		goto out;
+	}
+
+	rc = ima_write_template_field_data((char *)buffer, rc, DATA_FMT_HEX,
+					   field_data);
+out:
+	kfree(buffer);
+	return rc;
+}
+
+/*
+ *  ima_eventinodexattrnames_init - include a list of xattr names as part of the
+ *  template data
+ */
+int ima_eventinodexattrnames_init(struct ima_event_data *event_data,
+				  struct ima_field_data *field_data)
+{
+	return ima_eventinodexattrs_init_common(event_data, field_data, 'n');
+}
+
+/*
+ *  ima_eventinodexattrlengths_init - include a list of xattr lengths as part of
+ *  the template data
+ */
+int ima_eventinodexattrlengths_init(struct ima_event_data *event_data,
+				    struct ima_field_data *field_data)
+{
+	return ima_eventinodexattrs_init_common(event_data, field_data, 'l');
+}
+
+/*
+ *  ima_eventinodexattrvalues_init - include a list of xattr values as part of
+ *  the template data
+ */
+int ima_eventinodexattrvalues_init(struct ima_event_data *event_data,
+				   struct ima_field_data *field_data)
+{
+	return ima_eventinodexattrs_init_common(event_data, field_data, 'v');
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6509af4a97ee..c71f1de95753 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -56,4 +56,10 @@ int ima_eventinodegid_init(struct ima_event_data *event_data,
 			   struct ima_field_data *field_data);
 int ima_eventinodemode_init(struct ima_event_data *event_data,
 			    struct ima_field_data *field_data);
+int ima_eventinodexattrnames_init(struct ima_event_data *event_data,
+				  struct ima_field_data *field_data);
+int ima_eventinodexattrlengths_init(struct ima_event_data *event_data,
+				    struct ima_field_data *field_data);
+int ima_eventinodexattrvalues_init(struct ima_event_data *event_data,
+				   struct ima_field_data *field_data);
 #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
2.25.1

Re: [RESEND][PATCH v2 5/7] ima: Define new template fields xattrnames, xattrlengths and xattrvalues

[Mimi Zohar]

Hi Roberto,

On Tue, 2021-06-01 at 10:23 +0200, Roberto Sassu wrote:
> This patch defines the new template fields xattrnames, xattrlengths and
> xattrvalues, which contain respectively a list of xattr names (strings,
> separated by |), lengths (u32, hex) and values (hex). If an xattr is not
> present, the name and length are not displayed in the measurement list.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

Thank you for giving credit to the "kernel test robot".   For a bug fix
patch the above "Reported-by" tag would be appropriate, but for an
existing patch it needs to be qualified.  I've modified it to:

Reported-by: kernel test robot <lkp@intel.com> [Missing prototype def]

thanks,

Mimi

Re: [PATCH v2 6/7] ima: Define new template evm-sig

[Mimi Zohar]

On Fri, 2021-05-28 at 09:38 +0200, Roberto Sassu wrote:
> With the recent introduction of the evmsig template field, remote verifiers
> can obtain the EVM portable signature instead of the IMA signature, to
> verify file metadata.
> 
> After introducing the new fields to include file metadata in the
> measurement list, this patch finally defines the evm-sig template, whose
> format is:
> 
> d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode
> 
> xattrnames, xattrlengths and xattrvalues are populated only from defined
> EVM protected xattrs, i.e. the ones that EVM considers to verify the
> portable signature. xattrnames and xattrlengths are populated only if the
> xattr is present.
> 
> xattrnames and xattrlengths are not necessary for verifying the EVM
> portable signature, but they are included for completeness of information,
> if a remote verifier wants to infer more from file metadata.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  Documentation/security/IMA-templates.rst | 1 +
>  security/integrity/ima/ima_template.c    | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index 6a58760a0a35..5adc22f99496 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -91,6 +91,7 @@ Below, there is the list of defined template descriptors:
>   - "ima-sig": its format is ``d-ng|n-ng|sig``;
>   - "ima-buf": its format is ``d-ng|n-ng|buf``;
>   - "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``;
> + - "evm-sig": its format is ``d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode``;
>  
>  
>  Use
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 159a31d2fcdf..be435efe6122 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -22,6 +22,9 @@ static struct ima_template_desc builtin_templates[] = {
>  	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>  	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
>  	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
> +	{.name = "evm-sig",
> +	 .fmt = "d-ng|n-ng|evmsig|"
> +		"xattrnames|xattrlengths|xattrvalues|iuid|igid|imode"},

checkpatch is complaining "WARNING: quoted string split across lines".

>  	{.name = "", .fmt = ""},	/* placeholder for a custom format */
>  };
>  

The MAX_TEMPLATE_NAME_LEN needs to be updated.

thanks,

Mimi

[RESEND][PATCH v2 6/7] ima: Define new template evm-sig

[Roberto Sassu]

With the recent introduction of the evmsig template field, remote verifiers
can obtain the EVM portable signature instead of the IMA signature, to
verify file metadata.

After introducing the new fields to include file metadata in the
measurement list, this patch finally defines the evm-sig template, whose
format is:

d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode

xattrnames, xattrlengths and xattrvalues are populated only from defined
EVM protected xattrs, i.e. the ones that EVM considers to verify the
portable signature. xattrnames and xattrlengths are populated only if the
xattr is present.

xattrnames and xattrlengths are not necessary for verifying the EVM
portable signature, but they are included for completeness of information,
if a remote verifier wants to infer more from file metadata.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/security/IMA-templates.rst | 1 +
 security/integrity/ima/ima_template.c    | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 6a58760a0a35..5adc22f99496 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -91,6 +91,7 @@ Below, there is the list of defined template descriptors:
  - "ima-sig": its format is ``d-ng|n-ng|sig``;
  - "ima-buf": its format is ``d-ng|n-ng|buf``;
  - "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``;
+ - "evm-sig": its format is ``d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode``;
 
 
 Use
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 159a31d2fcdf..a85963853a91 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -22,6 +22,8 @@ static struct ima_template_desc builtin_templates[] = {
 	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
 	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
 	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
+	{.name = "evm-sig",
+	 .fmt = "d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode"},
 	{.name = "", .fmt = ""},	/* placeholder for a custom format */
 };
 
@@ -69,7 +71,8 @@ static const struct ima_template_field supported_fields[] = {
  * need to be accounted for since they shouldn't be defined in the same template
  * description as 'd-ng' and 'n-ng' respectively.
  */
-#define MAX_TEMPLATE_NAME_LEN sizeof("d-ng|n-ng|sig|buf|d-modisg|modsig")
+#define MAX_TEMPLATE_NAME_LEN \
+	sizeof("d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode")
 
 static struct ima_template_desc *ima_template;
 static struct ima_template_desc *ima_buf_template;
-- 
2.25.1

Re: [RESEND][PATCH v2 6/7] ima: Define new template evm-sig

[Mimi Zohar]

On Thu, 2021-06-03 at 15:17 +0200, Roberto Sassu wrote:
> With the recent introduction of the evmsig template field, remote verifiers
> can obtain the EVM portable signature instead of the IMA signature, to
> verify file metadata.
> 
> After introducing the new fields to include file metadata in the
> measurement list, this patch finally defines the evm-sig template, whose
> format is:
> 
> d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode
> 
> xattrnames, xattrlengths and xattrvalues are populated only from defined
> EVM protected xattrs, i.e. the ones that EVM considers to verify the
> portable signature. xattrnames and xattrlengths are populated only if the
> xattr is present.
> 
> xattrnames and xattrlengths are not necessary for verifying the EVM
> portable signature, but they are included for completeness of information,
> if a remote verifier wants to infer more from file metadata.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  Documentation/security/IMA-templates.rst | 1 +
>  security/integrity/ima/ima_template.c    | 5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index 6a58760a0a35..5adc22f99496 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -91,6 +91,7 @@ Below, there is the list of defined template descriptors:
>   - "ima-sig": its format is ``d-ng|n-ng|sig``;
>   - "ima-buf": its format is ``d-ng|n-ng|buf``;
>   - "ima-modsig": its format is ``d-ng|n-ng|sig|d-modsig|modsig``;
> + - "evm-sig": its format is ``d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode``;
>  
>  
>  Use
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 159a31d2fcdf..a85963853a91 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -22,6 +22,8 @@ static struct ima_template_desc builtin_templates[] = {
>  	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>  	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
>  	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
> +	{.name = "evm-sig",
> +	 .fmt = "d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode"},
>  	{.name = "", .fmt = ""},	/* placeholder for a custom format */
>  };
>  
> @@ -69,7 +71,8 @@ static const struct ima_template_field supported_fields[] = {
>   * need to be accounted for since they shouldn't be defined in the same template
>   * description as 'd-ng' and 'n-ng' respectively.
>   */
> -#define MAX_TEMPLATE_NAME_LEN sizeof("d-ng|n-ng|sig|buf|d-modisg|modsig")
> +#define MAX_TEMPLATE_NAME_LEN \
> +	sizeof("d-ng|n-ng|evmsig|xattrnames|xattrlengths|xattrvalues|iuid|igid|imode")
>  
>  static struct ima_template_desc *ima_template;
>  static struct ima_template_desc *ima_buf_template;

Thanks, Roberto.

Updated and merged next-integrity-testing, next-integrity branches.

thanks,

Mimi