linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] ima: Add template fields to verify EVM portable signatures
@ 2021-05-20  8:56 Roberto Sassu
  2021-05-20  8:56 ` [PATCH 1/7] ima: Add ima_show_template_uint() template library function Roberto Sassu
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:56 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	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;
- 'mntuidmap': the UID mappings of the idmapped mount (nr extents,
  [ uid_gid_extent1 ] ... [ uid_gid_extentN ], all u32 in canonical
  format);
- 'mntgidmap': the GID mappings of the idmapped mount (same format as
  'mntuidmap');
- 'imode': the inode mode;
- 'evmxattrs': the EVM protected xattrs (num xattrs (u32 in canonical
   format), xattr names separated by \0, xattr lengths (u32 in canonical
   format) and xattr values).

mntuidmap and mntgidmap are not empty only if the measurement is performed
on an idmapped mount. In that case, the inode UID and GID need to be
converted with the provided mappings.

Patches 1-4, 6 introduce new template fields. Patch 5 make it possible to
verify EVM portable signatures which protect xattrs belonging to LSMs not
enabled in the target platform. 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-v1-devel-v1/tests/verify_evmsig.test
https://github.com/robertosassu/ima-evm-utils/blob/ima-template-fields-v1-devel-v1/tests/evm_hmac_non_enabled_xattrs.test

The first test sets the IMA template format to:

d-ng|n-ng|sig|evmxattrs|iuid|igid|imode|mntuidmap|mntgidmap

Then, it creates a test file, sets some metadata and reads the file to
generate a measurement entry. To verify that the information provided by
IMA are correct, the test creates another file and sets the metadata
obtained from the measurement list. Finally, it executes evmctl to verify
the signature on the second file.

The test is performed without and with an idmapped mount. evmctl has been
extended to parse mntuidmap and mntgidmap (only one mapping), so that it
can convert the mapped UID and GID from the measurement list to the
original ones. In this way, the signature can be verified.

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/506431933
https://travis-ci.com/github/robertosassu/ima-evm-utils/jobs/506431937

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

Roberto Sassu (7):
  ima: Add ima_show_template_uint() template library function
  ima: Introduce template fields iuid and igid
  ima: Introduce template fields mntuidmap and mntgidmap
  ima: Introduce template field imode
  evm: Verify portable signatures against all protected xattrs
  ima: Introduce template field evmxattrs
  evm: Don't return an error in evm_write_xattrs() if audit is not
    enabled

 Documentation/security/IMA-templates.rst  |  10 +
 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        |  18 +-
 security/integrity/ima/ima_template.c     |  14 +
 security/integrity/ima/ima_template_lib.c | 322 +++++++++++++++++++++-
 security/integrity/ima/ima_template_lib.h |  14 +
 9 files changed, 434 insertions(+), 14 deletions(-)

-- 
2.25.1


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

* [PATCH 1/7] ima: Add ima_show_template_uint() template library function
  2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
@ 2021-05-20  8:56 ` Roberto Sassu
  2021-05-20  8:56 ` [PATCH 2/7] ima: Introduce template fields iuid and igid Roberto Sassu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:56 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	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


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

* [PATCH 2/7] ima: Introduce template fields iuid and igid
  2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
  2021-05-20  8:56 ` [PATCH 1/7] ima: Add ima_show_template_uint() template library function Roberto Sassu
@ 2021-05-20  8:56 ` Roberto Sassu
  2021-05-20  8:56 ` [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap Roberto Sassu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:56 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	Roberto Sassu, Christian Brauner

This patch introduces the new template fields iuid and igid, which include
respectively the inode UID and GID. If the inode is part of an idmapped
mount, the UID and GID are the mapped ones and need to be translated with
the new fields mntuidmap and mntgidmap included in a subsequent patch, in
order to get the original UID and GID.

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 | 58 +++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  4 ++
 4 files changed, 68 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..a191b861548b 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -551,3 +551,61 @@ 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)
+{
+	struct user_namespace *mnt_userns;
+	uid_t uid;
+	gid_t gid;
+	char *data_ptr = (char *)&uid;
+
+	if (!event_data->file)
+		return 0;
+
+	mnt_userns = file_mnt_user_ns(event_data->file);
+	if (get_uid) {
+		uid = from_kuid(&init_user_ns,
+				i_uid_into_mnt(mnt_userns,
+					       file_inode(event_data->file)));
+	} else {
+		gid = from_kgid(&init_user_ns,
+				i_gid_into_mnt(mnt_userns,
+					       file_inode(event_data->file)));
+		data_ptr = (char *)&gid;
+	}
+
+	if (ima_canonical_fmt) {
+		if (sizeof(uid) == sizeof(u16)) {
+			uid = cpu_to_le16(uid);
+			gid = cpu_to_le16(gid);
+		} else {
+			uid = cpu_to_le32(uid);
+			gid = cpu_to_le32(gid);
+		}
+	}
+
+	return ima_write_template_field_data(data_ptr, sizeof(uid),
+					     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


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

* [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap
  2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
  2021-05-20  8:56 ` [PATCH 1/7] ima: Add ima_show_template_uint() template library function Roberto Sassu
  2021-05-20  8:56 ` [PATCH 2/7] ima: Introduce template fields iuid and igid Roberto Sassu
@ 2021-05-20  8:56 ` Roberto Sassu
  2021-05-20  9:36   ` Christian Brauner
  2021-05-20  8:56 ` [PATCH 4/7] ima: Introduce template field imode Roberto Sassu
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:56 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	Roberto Sassu, Christian Brauner

This patch introduces the new template fields mntuidmap and mntgidmap,
which include respectively the UID and GID mappings of the idmapped mount,
if the user namespace is not the initial one.

These template fields, which should be included whenever the iuid and the
igid fields are included, allow remote verifiers to find the original UID
and GID of the inode during signature verification. The iuid and igid
fields include the mapped UID and GID when the inode is in an idmapped
mount.

This solution has been preferred to providing always the original UID and
GID, regardless of whether the inode is in an idmapped mount or not, as
the mapped UID and GID are those seen by processes and matched with the IMA
policy.

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

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index bf8ce4cf5878..48a2df22a1a1 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -77,6 +77,10 @@ descriptors by adding their identifier to the format string
  - 'evmsig': the EVM portable signature;
  - 'iuid': the inode UID;
  - 'igid': the inode GID;
+ - 'mntuidmap': the UID mappings of the idmapped mount (nr extents,
+    [ uid_gid_extent1 ] ... [ uid_gid_extentN ], all u32 in canonical format);
+ - 'mntgidmap': the GID mappings of the idmapped mount (same format as
+   'mntuidmap');
 
 
 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..19de115b985b 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -51,6 +51,12 @@ 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 = "mntuidmap",
+	 .field_init = ima_eventmnt_userns_uid_map_init,
+	 .field_show = ima_show_template_sig},
+	{.field_id = "mntgidmap",
+	 .field_init = ima_eventmnt_userns_gid_map_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 a191b861548b..bc4919d90c3a 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -609,3 +609,86 @@ int ima_eventinodegid_init(struct ima_event_data *event_data,
 {
 	return ima_eventinodedac_init_common(event_data, field_data, false);
 }
+
+int ima_eventmnt_userns_common_init(struct ima_event_data *event_data,
+				    struct ima_field_data *field_data,
+				    bool get_uid_map)
+{
+	struct user_namespace *mnt_userns;
+	u8 *buf, *buf_ptr;
+	struct uid_gid_map *map;
+	int rc, i;
+
+	if (!event_data->file)
+		return 0;
+
+	mnt_userns = file_mnt_user_ns(event_data->file);
+	if (mnt_userns == &init_user_ns)
+		return 0;
+
+	map = &mnt_userns->uid_map;
+	if (!get_uid_map)
+		map = &mnt_userns->gid_map;
+
+	buf_ptr = buf = kmalloc(sizeof(map->nr_extents) +
+				map->nr_extents * sizeof(*map->extent),
+				GFP_KERNEL);
+	if (!buf)
+		return 0;
+
+	memcpy(buf_ptr, &map->nr_extents, sizeof(map->nr_extents));
+
+	if (ima_canonical_fmt)
+		*(u32 *)buf_ptr = cpu_to_le32(*(u32 *)buf_ptr);
+
+	buf_ptr += sizeof(u32);
+
+	for (i = 0; i < map->nr_extents; i++) {
+		struct uid_gid_extent *extent;
+
+		/* Taken from m_start() in kernel/user_namespace.c. */
+		if (map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS)
+			extent = &map->extent[i];
+		else
+			extent = &map->forward[i];
+
+		memcpy(buf_ptr, &extent->first, sizeof(extent->first));
+		if (ima_canonical_fmt)
+			*(u32 *)buf_ptr = cpu_to_le32(*(u32 *)buf_ptr);
+		buf_ptr += sizeof(extent->first);
+		memcpy(buf_ptr, &extent->lower_first,
+		       sizeof(extent->lower_first));
+		if (ima_canonical_fmt)
+			*(u32 *)buf_ptr = cpu_to_le32(*(u32 *)buf_ptr);
+		buf_ptr += sizeof(extent->lower_first);
+		memcpy(buf_ptr, &extent->count, sizeof(extent->count));
+		if (ima_canonical_fmt)
+			*(u32 *)buf_ptr = cpu_to_le32(*(u32 *)buf_ptr);
+		buf_ptr += sizeof(extent->count);
+	}
+
+	rc = ima_write_template_field_data((char *)buf, buf_ptr - buf,
+					   DATA_FMT_HEX, field_data);
+	kfree(buf);
+	return rc;
+}
+
+/*
+ *  ima_eventmnt_userns_uid_map_init - include the UID mappings of the idmapped
+ *  mount as part of the template data
+ */
+int ima_eventmnt_userns_uid_map_init(struct ima_event_data *event_data,
+				     struct ima_field_data *field_data)
+{
+	return ima_eventmnt_userns_common_init(event_data, field_data, true);
+}
+
+/*
+ *  ima_eventmnt_userns_gid_map_init - include the GID mappings of the idmapped
+ *  mount as part of the template data
+ */
+int ima_eventmnt_userns_gid_map_init(struct ima_event_data *event_data,
+				     struct ima_field_data *field_data)
+{
+	return ima_eventmnt_userns_common_init(event_data, field_data, false);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index b0aaf109f386..51ee66fc7230 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -54,4 +54,8 @@ 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_eventmnt_userns_uid_map_init(struct ima_event_data *event_data,
+				     struct ima_field_data *field_data);
+int ima_eventmnt_userns_gid_map_init(struct ima_event_data *event_data,
+				     struct ima_field_data *field_data);
 #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
2.25.1


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

* [PATCH 4/7] ima: Introduce template field imode
  2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
                   ` (2 preceding siblings ...)
  2021-05-20  8:56 ` [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap Roberto Sassu
@ 2021-05-20  8:56 ` Roberto Sassu
  2021-05-20  8:56 ` [PATCH 5/7] evm: Verify portable signatures against all protected xattrs Roberto Sassu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:56 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	Roberto Sassu

This patch introduces 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 48a2df22a1a1..6e98bce20029 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -81,6 +81,7 @@ descriptors by adding their identifier to the format string
     [ uid_gid_extent1 ] ... [ uid_gid_extentN ], all u32 in canonical format);
  - 'mntgidmap': the GID mappings of the idmapped mount (same format as
    'mntuidmap');
+ - '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 19de115b985b..34674aef1cc5 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -57,6 +57,8 @@ static const struct ima_template_field supported_fields[] = {
 	{.field_id = "mntgidmap",
 	 .field_init = ima_eventmnt_userns_gid_map_init,
 	 .field_show = ima_show_template_sig},
+	{.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 bc4919d90c3a..b82fb8f35e5d 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -692,3 +692,25 @@ int ima_eventmnt_userns_gid_map_init(struct ima_event_data *event_data,
 {
 	return ima_eventmnt_userns_common_init(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 51ee66fc7230..dc3c16912f6d 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -58,4 +58,6 @@ int ima_eventmnt_userns_uid_map_init(struct ima_event_data *event_data,
 				     struct ima_field_data *field_data);
 int ima_eventmnt_userns_gid_map_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


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

* [PATCH 5/7] evm: Verify portable signatures against all protected xattrs
  2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
                   ` (3 preceding siblings ...)
  2021-05-20  8:56 ` [PATCH 4/7] ima: Introduce template field imode Roberto Sassu
@ 2021-05-20  8:56 ` Roberto Sassu
  2021-05-24 18:21   ` Mimi Zohar
  2021-05-20  8:57 ` [PATCH 6/7] ima: Introduce template field evmxattrs Roberto Sassu
  2021-05-20  8:57 ` [PATCH 7/7] evm: Don't return an error in evm_write_xattrs() if audit is not enabled Roberto Sassu
  6 siblings, 1 reply; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:56 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	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 in only 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 field evmxattrs has 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


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

* [PATCH 6/7] ima: Introduce template field evmxattrs
  2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
                   ` (4 preceding siblings ...)
  2021-05-20  8:56 ` [PATCH 5/7] evm: Verify portable signatures against all protected xattrs Roberto Sassu
@ 2021-05-20  8:57 ` Roberto Sassu
  2021-05-24 18:31   ` Mimi Zohar
  2021-05-20  8:57 ` [PATCH 7/7] evm: Don't return an error in evm_write_xattrs() if audit is not enabled Roberto Sassu
  6 siblings, 1 reply; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:57 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	Roberto Sassu

This patch introduces the new template field evmxattrs, which contains the
number of EVM protected xattrs (u32 in little endian), the xattr names
separated by \0, the xattr lengths (u32 in little endian) and the xattr
values. Xattrs can be used 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  |   3 +
 security/integrity/ima/ima_template.c     |   2 +
 security/integrity/ima/ima_template_lib.c | 121 ++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |   2 +
 4 files changed, 128 insertions(+)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 6e98bce20029..a9684fde3871 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -82,6 +82,9 @@ descriptors by adding their identifier to the format string
  - 'mntgidmap': the GID mappings of the idmapped mount (same format as
    'mntuidmap');
  - 'imode': the inode mode;
+ - 'evmxattrs': the EVM protected xattrs (num xattrs (u32 in canonical format),
+    xattr names separated by \0, xattr lengths (u32 in canonical format) and
+    xattr values);
 
 
 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 34674aef1cc5..b9dd900db0ff 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -59,6 +59,8 @@ static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_sig},
 	{.field_id = "imode", .field_init = ima_eventinodemode_init,
 	 .field_show = ima_show_template_uint},
+	{.field_id = "evmxattrs", .field_init = ima_eventinodeevmxattrs_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 b82fb8f35e5d..71e642d90e63 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)
 {
@@ -714,3 +715,123 @@ 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);
 }
+
+/*
+ *  ima_eventinodeevmxattrs_init - include the number of EVM protected xattrs,
+ *  the xattr names, lengths and values as part of the template data
+ */
+int ima_eventinodeevmxattrs_init(struct ima_event_data *event_data,
+				 struct ima_field_data *field_data)
+
+{
+	struct inode *inode;
+	u8 *buffer = NULL;
+	char *xattr_names, *xattr_names_ptr, *xattr_name;
+	size_t names_size, total_size;
+	u32 num_xattrs = 0, xattr_value_len;
+	loff_t names_offset, lengths_offset, values_offset;
+	int rc, evm_present = 0;
+
+	if (!event_data->file)
+		return 0;
+
+	inode = file_inode(event_data->file);
+	if (!inode->i_op->listxattr || !(inode->i_opflags & IOP_XATTR))
+		return 0;
+
+	names_size = inode->i_op->listxattr(file_dentry(event_data->file),
+					    NULL, 0);
+	if (names_size <= 0)
+		return 0;
+
+	xattr_names = kmalloc(names_size, GFP_KERNEL);
+	if (!xattr_names)
+		return 0;
+
+	names_size = inode->i_op->listxattr(file_dentry(event_data->file),
+					    xattr_names, names_size);
+	if (names_size <= 0)
+		goto out;
+
+	xattr_names_ptr = xattr_names;
+	total_size = sizeof(num_xattrs);
+	lengths_offset = total_size;
+
+	while (xattr_names_ptr < xattr_names + names_size) {
+		xattr_name = xattr_names_ptr;
+		xattr_names_ptr += strlen(xattr_names_ptr) + 1;
+
+		if (!strcmp(xattr_name, XATTR_NAME_EVM)) {
+			evm_present = 1;
+			continue;
+		}
+
+		if (!evm_protected_xattr_if_enabled(xattr_name))
+			continue;
+
+		total_size += xattr_names_ptr - xattr_name;
+		lengths_offset += xattr_names_ptr - xattr_name;
+		total_size += sizeof(xattr_value_len);
+		rc = __vfs_getxattr(file_dentry(event_data->file),
+				    file_inode(event_data->file), xattr_name,
+				    NULL, 0);
+		xattr_value_len = (rc >= 0) ? rc : 0;
+		total_size += xattr_value_len;
+		num_xattrs++;
+	}
+
+	/*
+	 * Don't provide data if security.evm is not found or there are no
+	 * protected xattrs.
+	 */
+	if (!evm_present || !num_xattrs)
+		return 0;
+
+	buffer = kmalloc(total_size, GFP_KERNEL);
+	if (!buffer)
+		goto out;
+
+	*(u32 *)buffer = num_xattrs;
+	if (ima_canonical_fmt)
+		*(u32 *)buffer = cpu_to_le32(*(u32 *)buffer);
+
+	names_offset = sizeof(num_xattrs);
+	values_offset = lengths_offset + num_xattrs * sizeof(xattr_value_len);
+
+	xattr_names_ptr = xattr_names;
+
+	while (xattr_names_ptr < xattr_names + names_size) {
+		xattr_name = xattr_names_ptr;
+		xattr_names_ptr += strlen(xattr_names_ptr) + 1;
+
+		if (!strcmp(xattr_name, XATTR_NAME_EVM))
+			continue;
+
+		if (!evm_protected_xattr_if_enabled(xattr_name))
+			continue;
+
+		memcpy(buffer + names_offset, xattr_name,
+		       xattr_names_ptr - xattr_name);
+		names_offset += xattr_names_ptr - xattr_name;
+
+		rc = __vfs_getxattr(file_dentry(event_data->file),
+				    file_inode(event_data->file), xattr_name,
+				    buffer + values_offset,
+				    total_size - values_offset);
+		xattr_value_len = (rc >= 0) ? rc : 0;
+		*(u32 *)(buffer + lengths_offset) = xattr_value_len;
+		if (ima_canonical_fmt)
+			*(u32 *)(buffer + lengths_offset) =
+				cpu_to_le32(*(u32 *)(buffer + lengths_offset));
+
+		lengths_offset += sizeof(xattr_value_len);
+		values_offset += xattr_value_len;
+	}
+
+	rc = ima_write_template_field_data((char *)buffer, total_size,
+					   DATA_FMT_HEX, field_data);
+out:
+	kfree(xattr_names);
+	kfree(buffer);
+	return rc;
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index dc3c16912f6d..ee8f53847305 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -60,4 +60,6 @@ int ima_eventmnt_userns_gid_map_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_eventinodeevmxattrs_init(struct ima_event_data *event_data,
+				 struct ima_field_data *field_data);
 #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
2.25.1


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

* [PATCH 7/7] evm: Don't return an error in evm_write_xattrs() if audit is not enabled
  2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
                   ` (5 preceding siblings ...)
  2021-05-20  8:57 ` [PATCH 6/7] ima: Introduce template field evmxattrs Roberto Sassu
@ 2021-05-20  8:57 ` Roberto Sassu
  6 siblings, 0 replies; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20  8:57 UTC (permalink / raw)
  To: zohar, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel,
	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


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

* Re: [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap
  2021-05-20  8:56 ` [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap Roberto Sassu
@ 2021-05-20  9:36   ` Christian Brauner
  2021-05-20  9:41     ` Christian Brauner
  0 siblings, 1 reply; 13+ messages in thread
From: Christian Brauner @ 2021-05-20  9:36 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: zohar, mjg59, linux-integrity, linux-security-module, linux-doc,
	linux-kernel

On Thu, May 20, 2021 at 10:56:57AM +0200, Roberto Sassu wrote:
> This patch introduces the new template fields mntuidmap and mntgidmap,
> which include respectively the UID and GID mappings of the idmapped mount,
> if the user namespace is not the initial one.
> 
> These template fields, which should be included whenever the iuid and the
> igid fields are included, allow remote verifiers to find the original UID
> and GID of the inode during signature verification. The iuid and igid
> fields include the mapped UID and GID when the inode is in an idmapped
> mount.
> 
> This solution has been preferred to providing always the original UID and
> GID, regardless of whether the inode is in an idmapped mount or not, as
> the mapped UID and GID are those seen by processes and matched with the IMA
> policy.

Hm, looking at the code this doesn't seem like a good idea to me. I
think we should avoid that and just rely on the original uid and gid.

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

* Re: [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap
  2021-05-20  9:36   ` Christian Brauner
@ 2021-05-20  9:41     ` Christian Brauner
  2021-05-20 11:54       ` Roberto Sassu
  0 siblings, 1 reply; 13+ messages in thread
From: Christian Brauner @ 2021-05-20  9:41 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: zohar, mjg59, linux-integrity, linux-security-module, linux-doc,
	linux-kernel

On Thu, May 20, 2021 at 11:37:07AM +0200, Christian Brauner wrote:
> On Thu, May 20, 2021 at 10:56:57AM +0200, Roberto Sassu wrote:
> > This patch introduces the new template fields mntuidmap and mntgidmap,
> > which include respectively the UID and GID mappings of the idmapped mount,
> > if the user namespace is not the initial one.
> > 
> > These template fields, which should be included whenever the iuid and the
> > igid fields are included, allow remote verifiers to find the original UID
> > and GID of the inode during signature verification. The iuid and igid
> > fields include the mapped UID and GID when the inode is in an idmapped
> > mount.
> > 
> > This solution has been preferred to providing always the original UID and
> > GID, regardless of whether the inode is in an idmapped mount or not, as
> > the mapped UID and GID are those seen by processes and matched with the IMA
> > policy.
> 
> Hm, looking at the code this doesn't seem like a good idea to me. I
> think we should avoid that and just rely on the original uid and gid.

It'd be ok to include the mapped uid/gid but don't copy the mapping
itself.

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

* RE: [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap
  2021-05-20  9:41     ` Christian Brauner
@ 2021-05-20 11:54       ` Roberto Sassu
  0 siblings, 0 replies; 13+ messages in thread
From: Roberto Sassu @ 2021-05-20 11:54 UTC (permalink / raw)
  To: Christian Brauner
  Cc: zohar, mjg59, linux-integrity, linux-security-module, linux-doc,
	linux-kernel

> From: Christian Brauner [mailto:christian.brauner@ubuntu.com]
> Sent: Thursday, May 20, 2021 11:41 AM
> On Thu, May 20, 2021 at 11:37:07AM +0200, Christian Brauner wrote:
> > On Thu, May 20, 2021 at 10:56:57AM +0200, Roberto Sassu wrote:
> > > This patch introduces the new template fields mntuidmap and mntgidmap,
> > > which include respectively the UID and GID mappings of the idmapped
> mount,
> > > if the user namespace is not the initial one.
> > >
> > > These template fields, which should be included whenever the iuid and the
> > > igid fields are included, allow remote verifiers to find the original UID
> > > and GID of the inode during signature verification. The iuid and igid
> > > fields include the mapped UID and GID when the inode is in an idmapped
> > > mount.
> > >
> > > This solution has been preferred to providing always the original UID and
> > > GID, regardless of whether the inode is in an idmapped mount or not, as
> > > the mapped UID and GID are those seen by processes and matched with
> the IMA
> > > policy.
> >
> > Hm, looking at the code this doesn't seem like a good idea to me. I
> > think we should avoid that and just rely on the original uid and gid.
> 
> It'd be ok to include the mapped uid/gid but don't copy the mapping
> itself.

Uhm, we would need a way to obtain the original UID and GID to
verify the portable signature.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

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

* Re: [PATCH 5/7] evm: Verify portable signatures against all protected xattrs
  2021-05-20  8:56 ` [PATCH 5/7] evm: Verify portable signatures against all protected xattrs Roberto Sassu
@ 2021-05-24 18:21   ` Mimi Zohar
  0 siblings, 0 replies; 13+ messages in thread
From: Mimi Zohar @ 2021-05-24 18:21 UTC (permalink / raw)
  To: Roberto Sassu, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel

On Thu, 2021-05-20 at 10:56 +0200, Roberto Sassu wrote:
> 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 in only 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 field evmxattrs has been included in
> the template format.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Nice, I really like this idea.

Mimi


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

* Re: [PATCH 6/7] ima: Introduce template field evmxattrs
  2021-05-20  8:57 ` [PATCH 6/7] ima: Introduce template field evmxattrs Roberto Sassu
@ 2021-05-24 18:31   ` Mimi Zohar
  0 siblings, 0 replies; 13+ messages in thread
From: Mimi Zohar @ 2021-05-24 18:31 UTC (permalink / raw)
  To: Roberto Sassu, mjg59
  Cc: linux-integrity, linux-security-module, linux-doc, linux-kernel

Hi Roberto,

On Thu, 2021-05-20 at 10:57 +0200, Roberto Sassu wrote:
> This patch introduces the new template field evmxattrs, which contains the
> number of EVM protected xattrs (u32 in little endian), the xattr names
> separated by \0, the xattr lengths (u32 in little endian) and the xattr
> values. Xattrs can be used to verify the EVM portable signature, if it was
> included with the template fields sig or evmsig.

Verifying the file data hash and the template data hash, the value
extended into the TPM,  are straight forward.  In the first case all
that is needed is the public key, and in the other case the length of
the template data.  Verifying the template data hash doesn't require
any knowledge of the template data format.   All that is needed is the
length of the template data.

This patch set provides all the necessary information for verifying the
EVM portable signature, but it is so much more difficult.  For example,
the security xattrs are listed in whatever order listxattr returns, not
the order in which the hash is calculated.  Does the attestation server
really need to know which xattrs are included or their length?  If that
information is important for the attestation server, then perhaps
provide it separately from the xattrs data.

I'm thinking the attestation server just needs the ability of verifying
the EVM portable signature.   As each field is prefixed with the field
data length, the attestation server should be able to re-calculate the
expected hash - xattrs, followed by the individual "misc" data fields.

thanks,

Mimi


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

end of thread, other threads:[~2021-05-24 18:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-20  8:56 [PATCH 0/7] ima: Add template fields to verify EVM portable signatures Roberto Sassu
2021-05-20  8:56 ` [PATCH 1/7] ima: Add ima_show_template_uint() template library function Roberto Sassu
2021-05-20  8:56 ` [PATCH 2/7] ima: Introduce template fields iuid and igid Roberto Sassu
2021-05-20  8:56 ` [PATCH 3/7] ima: Introduce template fields mntuidmap and mntgidmap Roberto Sassu
2021-05-20  9:36   ` Christian Brauner
2021-05-20  9:41     ` Christian Brauner
2021-05-20 11:54       ` Roberto Sassu
2021-05-20  8:56 ` [PATCH 4/7] ima: Introduce template field imode Roberto Sassu
2021-05-20  8:56 ` [PATCH 5/7] evm: Verify portable signatures against all protected xattrs Roberto Sassu
2021-05-24 18:21   ` Mimi Zohar
2021-05-20  8:57 ` [PATCH 6/7] ima: Introduce template field evmxattrs Roberto Sassu
2021-05-24 18:31   ` Mimi Zohar
2021-05-20  8:57 ` [PATCH 7/7] evm: Don't return an error in evm_write_xattrs() if audit is not enabled Roberto Sassu

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