All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/3] ima: Digest Lists extension
@ 2020-08-03 15:13 Roberto Sassu
  2020-08-03 15:13 ` [RFC][PATCH 1/3] ima: Add support for measurement with digest lists Roberto Sassu
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Roberto Sassu @ 2020-08-03 15:13 UTC (permalink / raw)
  To: zohar; +Cc: linux-integrity, silviu.vlasceanu

Hi everyone

before sending the full patch set which adds support for digest lists,
I would like to have an early review of the patches which introduce new
behavior for measurement, appraisal and EVM, so that we can identify
potential problems. The patches wouldn't alter existing behavior unless
explicitly requested by the user with additional kernel options.


========
Overview
========

Digest lists are a list of reference values for files and metadata, which
are preloaded early in the boot process so that can be compared with
calculated digest of files and metadata, when they are verified. If there
is a match, and the user enabled the feature, IMA and EVM behaviors are
modified in the following way:

- Measurement: adding ima_digest_list_pcr=<PCR> will cause the creation of
  an alternative measurement list, with the PCR specified, which includes
  only the measurement of the digest lists and of the files for which the
  digest has not be found among the preloaded ones. Both the standard and
  the alternative measurement list can be created by adding '+' before PCR.

- Appraisal: adding ima_appraise_digest_list=digest will grant access to
  the files whose digest is found, until EVM is initialized. Adding
  ima_appraise_digest_list=digest-nometadata will extend the usability of
  digest lists also after EVM is initialized. It is the least secure
  option, as it grants access to files without verifying metadata and would
  require the user to trust xattr values at first use. These files can be
  easily distinguished from others, as IMA at file close assigns to them a
  different security.ima type. Choosing this option might be useful if the
  only available reference values are for file content.

- EVM: a new type for security.evm has been introduced to calculate the
  metadata digest in the same way as for portable signatures and to search
  it in the digest lists. EVM reports successful verification if metadata
  digest is found.


========
Benefits
========

The main benefit of this extension is to maintain a stable PCR which can be
used for sealing of data and keys. The PCR maintained by the extension
changes, after the initial measurement of digest lists, only if an unknown
file is measured, and would work as an effective way to revoke access to
sensitive information protected by the TPM.

If a TPM key is bound to the stable PCR, and that key is used to establish
a TLS communication, remote peers would know that the system accessed
unknown files as, after revocation, the TPM would prevent the system from
performing the TLS protocol.

On the other end, recording only unknown files means losing information
about those that are in the digest lists. A verifier would not known
whether or not those files have been accessed. Given that both the standard
and the alternative measurement list can be created at the same time, one
can use the latter for normal operation and the former for a more precise
assessment for example due to suspicious activity.

Compared to the approach of measuring signing keys, digest lists have finer
granularity. The former represents all files signed with the measured key,
the latter only the files in a digest list, e.g. those belonging to a
package, container image, etc. Due to finer granularity, revocation would
be easier. In the future, a rollback prevention mechanism will prevent
digest lists of old package versions to be loaded again in the kernel. If
an old file will be accessed, it will be treated like an unknown file and a
new measurement entry will be created.

Given that software updates would also change the stable PCR, both
solutions can be combined together: measure the signing key used to verify
digest lists, and use digest lists to decide whether files should be added
to the measurement list. 

For appraisal and metadata verification, having a signed list of many
digests is more efficient than signing individual files, in terms of space
required to store the signatures and computation required for verification.


=========
Lifecycle
=========

From the lifecycle point of view, managing digest lists is feasible and
could be achieved without modification of existing building
infrastructures. I created a new project in the SUSE build service:

https://build.opensuse.org/project/show/home:roberto.sassu:branches:openSUSE:Leap:15.2

which automatically generates digest lists every time a package is built.
To achieve this, I modified some packages, such as rpm and
pesign-obs-integration, and added new ones, such as digest-list-tools and
brp-digest-list.

Then, I created a new project to generate an image suitable for execution
with KVM:

https://build.opensuse.org/project/show/home:roberto.sassu:branches:openSUSE:Templates:Images:15.2

This project generates an openSUSE Leap 15.2 JeOS image, which can be
downloaded at:

https://download.opensuse.org/repositories/home:/roberto.sassu:/branches:/openSUSE:/Templates:/Images:/15.2/images/

As for IMA signatures, an rpm plugin is responsible to perform additional
configuration when packages are installed. It extracts the header from the
package and converts the PGP signature to an IMA signature, so it can be
verified without additional modification of the kernel.


=======
Testing
=======

With the image, it is possible to easily evaluate the new functionality.
The second boot menu entry should be selected to enable usage of digest
lists for measurement and appraisal.


1. Show measurement list with predictable PCR:

# cat /sys/kernel/security/ima/ascii_runtime_measurements

11 <digest> ima-sig sha256:<digest> boot_aggregate
11 <digest> ima-sig sha256:<digest> .../0-parser_list-compact-libexec <sig>
11 <digest> ima-sig sha256:<digest> .../0-metadata_list-compact-digest-list-tools-0.3.93-lp152.20.5.x86_64 <sig>
11 <digest> ima-sig sha256:<digest> .../0-metadata_list-rpm-xz-5.2.3-lp152.6.1.x86_64 <sig>
...

The filename format is 0-<digest type>-<format>-<dir/package>

Every file is recognized, the measurement list contains only digest lists.
With appraisal enabled, unknown files cannot be accessed (with a patch) and
they are not added to the measurement list.

# echo test > script.sh
# chmod +x script.sh
# ./script.sh 
-bash: ./script.sh: Permission denied

# cat /sys/kernel/security/ima/ascii_runtime_measurements


2. Show measurement entry creation for unknown files:

Replace ima_appraise=enforce-evm with ima_appraise=log-evm in the kernel
command line of the second boot menu entry (the -evm suffix means that
uninitialized EVM is considered as an error).

If the unknown script is executed again, this time a new measurement entry
is created.

# cat /sys/kernel/security/ima/ascii_runtime_measurements
...
11 <digest> ima-sig sha256:<digest> /root/script.sh


3. Show file metadata protection

Reboot the system with ima_appraise=enforce-evm in the kernel command line.

A new policy option called 'metadata_immutable' has been introduced to
ensure that only binaries with verified and immutable metadata are
executed.

# cat
^C
# chmod 777 /bin/cat
# cat
-bash: /usr/bin/cat: Permission denied
# chmod 755 /bin/cat
# cat
^C



Please have a look at the patches or try the image. Any feedback is very
much appreciated.

Roberto Sassu (3):
  ima: Add support for measurement with digest lists
  ima: Add support for appraisal with digest lists
  evm: Add support for digest lists of metadata

 .../admin-guide/kernel-parameters.txt         | 18 ++++
 security/integrity/evm/evm_crypto.c           |  9 +-
 security/integrity/evm/evm_main.c             | 82 +++++++++++++++++--
 security/integrity/ima/ima.h                  | 15 ++--
 security/integrity/ima/ima_api.c              | 42 ++++++++--
 security/integrity/ima/ima_appraise.c         | 74 +++++++++++++++--
 security/integrity/ima/ima_digest_list.c      | 25 ++++++
 security/integrity/ima/ima_init.c             |  2 +-
 security/integrity/ima/ima_main.c             | 24 +++++-
 security/integrity/ima/ima_policy.c           |  3 +-
 security/integrity/integrity.h                |  2 +
 11 files changed, 263 insertions(+), 33 deletions(-)

-- 
2.27.GIT


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

* [RFC][PATCH 1/3] ima: Add support for measurement with digest lists
  2020-08-03 15:13 [RFC][PATCH 0/3] ima: Digest Lists extension Roberto Sassu
@ 2020-08-03 15:13 ` Roberto Sassu
  2020-08-03 15:13 ` [RFC][PATCH 2/3] ima: Add support for appraisal " Roberto Sassu
  2020-08-03 15:13 ` [RFC][PATCH 3/3] evm: Add support for digest lists of metadata Roberto Sassu
  2 siblings, 0 replies; 4+ messages in thread
From: Roberto Sassu @ 2020-08-03 15:13 UTC (permalink / raw)
  To: zohar; +Cc: linux-integrity, silviu.vlasceanu

IMA-Measure creates a new measurement entry every time a file is measured,
unless the same entry is already in the measurement list.

This patch introduces a new type of measurement list, recognizable by the
PCR number specified with the new ima_digest_list_pcr= kernel option. This
type of measurement list includes measurements of digest lists and files
not found in those lists.

The benefit of this patch is the availability of a predictable PCR that
can be used to seal data or TPM keys to the OS software. Unlike standard
measurements, digest list measurements only indicate that files with a
digest in those lists could have been accessed, but not if and when. With
standard measurements, however, the chosen PCR is unlikely predictable.

Both standard and digest list measurements can be generated at the same
time by adding '+' as a prefix to the value of ima_digest_list_pcr=
(example: with ima_digest_list_pcr=+11, IMA generates standard measurements
with PCR 10 and digest list measurements with PCR 11).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  9 ++++
 security/integrity/ima/ima.h                  |  9 ++--
 security/integrity/ima/ima_api.c              | 42 +++++++++++++++----
 security/integrity/ima/ima_digest_list.c      | 25 +++++++++++
 security/integrity/ima/ima_init.c             |  2 +-
 security/integrity/ima/ima_main.c             | 18 +++++++-
 security/integrity/ima/ima_policy.c           |  3 +-
 security/integrity/integrity.h                |  1 +
 8 files changed, 95 insertions(+), 14 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2647c5622f0f..6a44594c10d7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1696,6 +1696,15 @@
 			Use the canonical format for the binary runtime
 			measurements, instead of host native format.
 
+	ima_digest_list_pcr=
+			[IMA]
+			Specify which PCR is extended when file digests are
+			not found in the loaded digest lists. If '+' is not
+			specified, no measurement entry is created if the
+			digest is found. Otherwise, IMA creates also entries
+			with PCR 10, according to the existing behavior.
+			Format: { [+]<unsigned int> }
+
 	ima_hash=	[IMA]
 			Format: { md5 | sha1 | rmd160 | sha256 | sha384
 				   | sha512 | ... }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 0a5d1f813f36..7af7620b770a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -57,6 +57,8 @@ extern int ima_hash_algo_idx __ro_after_init;
 extern int ima_extra_slots __ro_after_init;
 extern int ima_appraise;
 extern struct tpm_chip *ima_tpm_chip;
+extern int ima_digest_list_pcr;
+extern bool ima_plus_standard_pcr;
 extern const char boot_aggregate_name[];
 extern int ima_digest_list_actions;
 
@@ -250,7 +252,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, const struct modsig *modsig, int pcr,
-			   struct ima_template_desc *template_desc);
+			   struct ima_template_desc *template_desc,
+			   struct ima_digest *digest);
 void process_buffer_measurement(const void *buf, int size,
 				const char *eventname, enum ima_hooks func,
 				int pcr, const char *keyring);
@@ -260,8 +263,8 @@ int ima_alloc_init_template(struct ima_event_data *event_data,
 			    struct ima_template_entry **entry,
 			    struct ima_template_desc *template_desc);
 int ima_store_template(struct ima_template_entry *entry, int violation,
-		       struct inode *inode,
-		       const unsigned char *filename, int pcr);
+		       struct inode *inode, const unsigned char *filename,
+		       int pcr, struct ima_digest *digest);
 void ima_free_template_entry(struct ima_template_entry *entry);
 const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
 
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index bf22de8b7ce0..45a2ce2f1fe2 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -101,11 +101,13 @@ int ima_alloc_init_template(struct ima_event_data *event_data,
  */
 int ima_store_template(struct ima_template_entry *entry,
 		       int violation, struct inode *inode,
-		       const unsigned char *filename, int pcr)
+		       const unsigned char *filename, int pcr,
+		       struct ima_digest *digest)
 {
 	static const char op[] = "add_template_measure";
 	static const char audit_cause[] = "hashing_error";
 	char *template_name = entry->template_desc->name;
+	struct ima_template_entry *duplicated_entry = NULL;
 	int result;
 
 	if (!violation) {
@@ -118,8 +120,26 @@ int ima_store_template(struct ima_template_entry *entry,
 			return result;
 		}
 	}
+
+	if (ima_plus_standard_pcr && !digest) {
+		duplicated_entry = kmemdup(entry,
+			sizeof(*entry) + entry->template_desc->num_fields *
+			sizeof(struct ima_field_data), GFP_KERNEL);
+		if (duplicated_entry)
+			duplicated_entry->pcr = ima_digest_list_pcr;
+	} else if (!ima_plus_standard_pcr && ima_digest_list_pcr >= 0) {
+		pcr = ima_digest_list_pcr;
+	}
+
 	entry->pcr = pcr;
 	result = ima_add_template_entry(entry, violation, op, inode, filename);
+	if (!result && duplicated_entry) {
+		result = ima_add_template_entry(duplicated_entry, violation, op,
+						inode, filename);
+		if (result < 0)
+			kfree(duplicated_entry);
+	}
+
 	return result;
 }
 
@@ -151,8 +171,8 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 		result = -ENOMEM;
 		goto err_out;
 	}
-	result = ima_store_template(entry, violation, inode,
-				    filename, CONFIG_IMA_MEASURE_PCR_IDX);
+	result = ima_store_template(entry, violation, inode, filename,
+				    CONFIG_IMA_MEASURE_PCR_IDX, NULL);
 	if (result < 0)
 		ima_free_template_entry(entry);
 err_out:
@@ -297,13 +317,14 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 			   struct file *file, const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, const struct modsig *modsig, int pcr,
-			   struct ima_template_desc *template_desc)
+			   struct ima_template_desc *template_desc,
+			   struct ima_digest *digest)
 {
 	static const char op[] = "add_template_measure";
 	static const char audit_cause[] = "ENOMEM";
 	int result = -ENOMEM;
 	struct inode *inode = file_inode(file);
-	struct ima_template_entry *entry;
+	struct ima_template_entry *entry = NULL;
 	struct ima_event_data event_data = { .iint = iint,
 					     .file = file,
 					     .filename = filename,
@@ -321,6 +342,11 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 	if (iint->measured_pcrs & (0x1 << pcr) && !modsig)
 		return;
 
+	if (digest && !ima_plus_standard_pcr && ima_digest_list_pcr >= 0) {
+		result = -EEXIST;
+		goto out;
+	}
+
 	result = ima_alloc_init_template(&event_data, &entry, template_desc);
 	if (result < 0) {
 		integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
@@ -328,12 +354,14 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 		return;
 	}
 
-	result = ima_store_template(entry, violation, inode, filename, pcr);
+	result = ima_store_template(entry, violation, inode, filename, pcr,
+				    digest);
+out:
 	if ((!result || result == -EEXIST) && !(file->f_flags & O_DIRECT)) {
 		iint->flags |= IMA_MEASURED;
 		iint->measured_pcrs |= (0x1 << pcr);
 	}
-	if (result < 0)
+	if (result < 0 && entry)
 		ima_free_template_entry(entry);
 }
 
diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
index 607a33ba52ee..7e901bdb4340 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -28,6 +28,31 @@ struct ima_h_table ima_digests_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+static int __init digest_list_pcr_setup(char *str)
+{
+	int pcr, ret;
+
+	ret = kstrtouint(str, 10, &pcr);
+	if (ret) {
+		pr_err("Invalid PCR number %s\n", str);
+		return 1;
+	}
+
+	if (pcr == CONFIG_IMA_MEASURE_PCR_IDX) {
+		pr_err("Default PCR cannot be used for digest lists\n");
+		return 1;
+	}
+
+	ima_digest_list_pcr = pcr;
+	ima_digest_list_actions |= IMA_MEASURE;
+
+	if (*str == '+')
+		ima_plus_standard_pcr = true;
+
+	return 1;
+}
+__setup("ima_digest_list_pcr=", digest_list_pcr_setup);
+
 /*************************
  * Get/add/del functions *
  *************************/
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 4902fe7bd570..5b5edd3a6b9e 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -86,7 +86,7 @@ static int __init ima_add_boot_aggregate(void)
 
 	result = ima_store_template(entry, violation, NULL,
 				    boot_aggregate_name,
-				    CONFIG_IMA_MEASURE_PCR_IDX);
+				    CONFIG_IMA_MEASURE_PCR_IDX, NULL);
 	if (result < 0) {
 		ima_free_template_entry(entry);
 		audit_cause = "store_entry";
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index e14632cf5c1b..ca80e7c5b885 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -27,6 +27,7 @@
 #include <linux/fs.h>
 
 #include "ima.h"
+#include "ima_digest_list.h"
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise = IMA_APPRAISE_ENFORCE;
@@ -38,6 +39,10 @@ int ima_hash_algo = HASH_ALGO_SHA1;
 
 /* Actions (measure/appraisal) for which digest lists can be used */
 int ima_digest_list_actions;
+/* PCR used for digest list measurements */
+int ima_digest_list_pcr = -1;
+/* Flag to include standard measurement if digest list PCR is specified */
+bool ima_plus_standard_pcr;
 
 static int hash_setup_done;
 
@@ -162,6 +167,8 @@ static enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
 			return ima_hash_algo;
 		return sig->hash_algo;
 		break;
+	case EVM_IMA_XATTR_DIGEST_LIST:
+		fallthrough;
 	case IMA_XATTR_DIGEST_NG:
 		/* first byte contains algorithm id */
 		ret = xattr_value->data[0];
@@ -255,6 +262,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	const char *pathname = NULL;
 	int rc = 0, action, must_appraise = 0;
 	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+	struct ima_digest *found_digest;
 	struct evm_ima_xattr_data *xattr_value = NULL;
 	struct modsig *modsig = NULL;
 	int xattr_len = 0;
@@ -384,10 +392,16 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
+	found_digest = ima_lookup_digest(iint->ima_hash->digest, hash_algo,
+					 COMPACT_FILE);
+
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
 				      xattr_value, xattr_len, modsig, pcr,
-				      template_desc);
+				      template_desc,
+				      ima_digest_allow(found_digest,
+						       IMA_MEASURE));
+
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
 		rc = ima_check_blacklist(iint, modsig, pcr);
 		if (rc != -EPERM) {
@@ -858,7 +872,7 @@ void process_buffer_measurement(const void *buf, int size,
 	if (ret < 0)
 		goto out;
 
-	ret = ima_store_template(entry, violation, NULL, buf, pcr);
+	ret = ima_store_template(entry, violation, NULL, buf, pcr, NULL);
 
 	if (ret < 0)
 		ima_free_template_entry(entry);
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index dd7f22a846a2..4af9c1613c8e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1305,7 +1305,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			ima_log_string(ab, "pcr", args[0].from);
 
 			result = kstrtoint(args[0].from, 10, &entry->pcr);
-			if (result || INVALID_PCR(entry->pcr))
+			if (result || INVALID_PCR(entry->pcr) ||
+			    entry->pcr == ima_digest_list_pcr)
 				result = -EINVAL;
 			else
 				entry->flags |= IMA_PCR;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 041edd217829..3525f2d185b3 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -78,6 +78,7 @@ enum evm_ima_xattr_type {
 	EVM_IMA_XATTR_DIGSIG,
 	IMA_XATTR_DIGEST_NG,
 	EVM_XATTR_PORTABLE_DIGSIG,
+	EVM_IMA_XATTR_DIGEST_LIST,
 	IMA_XATTR_LAST
 };
 
-- 
2.27.GIT


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

* [RFC][PATCH 2/3] ima: Add support for appraisal with digest lists
  2020-08-03 15:13 [RFC][PATCH 0/3] ima: Digest Lists extension Roberto Sassu
  2020-08-03 15:13 ` [RFC][PATCH 1/3] ima: Add support for measurement with digest lists Roberto Sassu
@ 2020-08-03 15:13 ` Roberto Sassu
  2020-08-03 15:13 ` [RFC][PATCH 3/3] evm: Add support for digest lists of metadata Roberto Sassu
  2 siblings, 0 replies; 4+ messages in thread
From: Roberto Sassu @ 2020-08-03 15:13 UTC (permalink / raw)
  To: zohar; +Cc: linux-integrity, silviu.vlasceanu

IMA-Appraise grants access to files with a valid signature or with actual
file digest equal to the digest included in security.ima.

This patch adds support for appraisal based on digest lists. Instead of
using the reference value from security.ima, this patch checks if the
calculated file digest is included in the uploaded digest lists.

This functionality must be explicitly enabled by providing one of the
following values for the ima_appraise_digest_list= kernel option:

- digest: this mode enables appraisal verification with digest lists until
  EVM is initialized; after that, EVM verification must be successful even
  if the file digest is found in a digest list;

- digest-nometadata: this mode enables appraisal verification with digest
  lists even after EVM has been initialized; files without security.evm are
  allowed if the digest of the content is found in the digest list, and
  security.evm is created with current values of xattrs (trust at first
  use); all files created in this way will have the new security.ima type
  EVM_IMA_XATTR_DIGEST_LIST; they can be accessed later only if this mode
  has been selected.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  9 +++
 security/integrity/ima/ima.h                  |  6 +-
 security/integrity/ima/ima_appraise.c         | 74 +++++++++++++++++--
 security/integrity/ima/ima_main.c             |  6 +-
 security/integrity/integrity.h                |  1 +
 5 files changed, 87 insertions(+), 9 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 6a44594c10d7..b2f899233bef 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1688,6 +1688,15 @@
 				  "enforce-evm" | "log-evm" }
 			default: "enforce"
 
+	ima_appraise_digest_list= [IMA]
+			Format: { "digest" | "digest-nometadata" }
+
+			digest: enables appraisal of files with digest lists
+			until EVM is initialized.
+
+			digest-nometadata: enables appraisal of files with
+			digest lists even after EVM is initialized.
+
 	ima_appraise_tcb [IMA] Deprecated.  Use ima_policy= instead.
 			The builtin appraise policy appraises all files
 			owned by uid=0.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 7af7620b770a..ca1e1fcd84d2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -301,7 +301,8 @@ int ima_appraise_measurement(enum ima_hooks func,
 			     struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len, const struct modsig *modsig);
+			     int xattr_len, const struct modsig *modsig,
+			     struct ima_digest *found_digest);
 int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
 void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
 enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
@@ -319,7 +320,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func,
 					   const unsigned char *filename,
 					   struct evm_ima_xattr_data *xattr_value,
 					   int xattr_len,
-					   const struct modsig *modsig)
+					   const struct modsig *modsig,
+					   struct ima_digest *found_digest)
 {
 	return INTEGRITY_UNKNOWN;
 }
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 433d547ff773..4ea107db32e2 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -15,6 +15,7 @@
 #include <keys/system_keyring.h>
 
 #include "ima.h"
+#include "ima_digest_list.h"
 
 static bool ima_appraise_req_evm __ro_after_init;
 static int __init default_appraise_setup(char *str)
@@ -35,6 +36,22 @@ static int __init default_appraise_setup(char *str)
 
 __setup("ima_appraise=", default_appraise_setup);
 
+static bool ima_appraise_no_metadata __ro_after_init;
+#ifdef CONFIG_IMA_DIGEST_LIST
+static int __init appraise_digest_list_setup(char *str)
+{
+	if (!strncmp(str, "digest", 6)) {
+		ima_digest_list_actions |= IMA_APPRAISE;
+
+		if (!strcmp(str + 6, "-nometadata"))
+			ima_appraise_no_metadata = true;
+	}
+
+	return 1;
+}
+__setup("ima_appraise_digest_list=", appraise_digest_list_setup);
+#endif
+
 /*
  * is_ima_appraise_enabled - return appraise status
  *
@@ -74,6 +91,9 @@ static int ima_fix_xattr(struct dentry *dentry,
 	} else {
 		offset = 0;
 		iint->ima_hash->xattr.ng.type = IMA_XATTR_DIGEST_NG;
+		if (test_bit(IMA_DIGEST_LIST, &iint->atomic_flags))
+			iint->ima_hash->xattr.ng.type =
+						EVM_IMA_XATTR_DIGEST_LIST;
 		iint->ima_hash->xattr.ng.algo = algo;
 	}
 	rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
@@ -161,17 +181,32 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
  */
 static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 			struct evm_ima_xattr_data *xattr_value, int xattr_len,
-			enum integrity_status *status, const char **cause)
+			enum integrity_status *status, const char **cause,
+			struct ima_digest *found_digest)
 {
 	int rc = -EINVAL, hash_start = 0;
 
+	if (found_digest && *status != INTEGRITY_PASS &&
+	    *status != INTEGRITY_PASS_IMMUTABLE)
+		set_bit(IMA_DIGEST_LIST, &iint->atomic_flags);
+
 	switch (xattr_value->type) {
+	case EVM_IMA_XATTR_DIGEST_LIST:
+		set_bit(IMA_DIGEST_LIST, &iint->atomic_flags);
+
+		if (!ima_appraise_no_metadata) {
+			*cause = "IMA-xattr-untrusted";
+			*status = INTEGRITY_FAIL;
+			break;
+		}
+		fallthrough;
 	case IMA_XATTR_DIGEST_NG:
 		/* first byte contains algorithm id */
 		hash_start = 1;
 		/* fall through */
 	case IMA_XATTR_DIGEST:
-		if (*status != INTEGRITY_PASS_IMMUTABLE) {
+		if (*status != INTEGRITY_PASS_IMMUTABLE &&
+		    (!found_digest || !ima_digest_is_immutable(found_digest))) {
 			if (iint->flags & IMA_DIGSIG_REQUIRED) {
 				*cause = "IMA-signature-required";
 				*status = INTEGRITY_FAIL;
@@ -304,7 +339,8 @@ int ima_appraise_measurement(enum ima_hooks func,
 			     struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len, const struct modsig *modsig)
+			     int xattr_len, const struct modsig *modsig,
+			     struct ima_digest *found_digest)
 {
 	static const char op[] = "appraise_data";
 	const char *cause = "unknown";
@@ -312,12 +348,23 @@ int ima_appraise_measurement(enum ima_hooks func,
 	struct inode *inode = d_backing_inode(dentry);
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	int rc = xattr_len;
+	char _buf[sizeof(struct evm_ima_xattr_data) + SHA512_DIGEST_SIZE];
 	bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
 
 	/* If not appraising a modsig, we need an xattr. */
 	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig)
 		return INTEGRITY_UNKNOWN;
 
+	if (rc == -ENODATA && found_digest &&
+	    !(file->f_mode & FMODE_CREATED)) {
+		xattr_value = (struct evm_ima_xattr_data *)_buf;
+		xattr_value->type = IMA_XATTR_DIGEST_NG;
+		xattr_value->data[0] = found_digest->algo;
+		memcpy(&xattr_value->data[1], found_digest->digest,
+		       hash_digest_size[found_digest->algo]);
+		rc = hash_digest_size[found_digest->algo] + 2;
+	}
+
 	/* If reading the xattr failed and there's no modsig, error out. */
 	if (rc <= 0 && !try_modsig) {
 		if (rc && rc != -ENODATA)
@@ -342,7 +389,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 		break;
 	case INTEGRITY_UNKNOWN:
 		if (ima_appraise_req_evm &&
-		    xattr_value->type != EVM_IMA_XATTR_DIGSIG)
+		    xattr_value->type != EVM_IMA_XATTR_DIGSIG && !found_digest)
 			goto out;
 		break;
 	case INTEGRITY_NOXATTRS:	/* No EVM protected xattrs. */
@@ -351,6 +398,23 @@ int ima_appraise_measurement(enum ima_hooks func,
 			break;
 		/* fall through */
 	case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
+		/*
+		 * If the digest-nometadata mode is selected, allow access
+		 * without metadata check. EVM will eventually create an HMAC
+		 * based on current xattr values.
+		 */
+		if (ima_appraise_no_metadata && found_digest)
+			break;
+		/* Allow access to digest lists without metadata, only if they
+		 * are signed or found in a digest list (immutable)
+		 */
+		if (func == DIGEST_LIST_CHECK) {
+			if (xattr_value->type == EVM_IMA_XATTR_DIGSIG)
+				break;
+			if (found_digest &&
+			    ima_digest_is_immutable(found_digest))
+				break;
+		}
 		cause = "missing-HMAC";
 		goto out;
 	case INTEGRITY_FAIL_IMMUTABLE:
@@ -365,7 +429,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 
 	if (xattr_value)
 		rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
-				  &cause);
+				  &cause, found_digest);
 
 	/*
 	 * If we have a modsig and either no imasig or the imasig's key isn't
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index ca80e7c5b885..031d3ce44e58 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -407,8 +407,10 @@ static int process_measurement(struct file *file, const struct cred *cred,
 		if (rc != -EPERM) {
 			inode_lock(inode);
 			rc = ima_appraise_measurement(func, iint, file,
-						      pathname, xattr_value,
-						      xattr_len, modsig);
+					      pathname, xattr_value,
+					      xattr_len, modsig,
+					      ima_digest_allow(found_digest,
+							       IMA_APPRAISE));
 			inode_unlock(inode);
 		}
 		if (!rc)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 3525f2d185b3..de68d816505d 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -71,6 +71,7 @@
 #define IMA_CHANGE_ATTR		2
 #define IMA_DIGSIG		3
 #define IMA_MUST_MEASURE	4
+#define IMA_DIGEST_LIST		5
 
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,
-- 
2.27.GIT


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

* [RFC][PATCH 3/3] evm: Add support for digest lists of metadata
  2020-08-03 15:13 [RFC][PATCH 0/3] ima: Digest Lists extension Roberto Sassu
  2020-08-03 15:13 ` [RFC][PATCH 1/3] ima: Add support for measurement with digest lists Roberto Sassu
  2020-08-03 15:13 ` [RFC][PATCH 2/3] ima: Add support for appraisal " Roberto Sassu
@ 2020-08-03 15:13 ` Roberto Sassu
  2 siblings, 0 replies; 4+ messages in thread
From: Roberto Sassu @ 2020-08-03 15:13 UTC (permalink / raw)
  To: zohar; +Cc: linux-integrity, silviu.vlasceanu

This patch adds support in EVM to verify file metadata digest with digest
lists. Metadata digest, calculated in the same way as for portable
signatures, is searched in the digest lists only if the file has the
security.evm xattr with type EVM_IMA_XATTR_DIGEST_LIST.

If the found digest is marked as immutable, content and xattr/attr updates
are not allowed. Otherwise, after verification, the existing security.evm
with the new type will be replaced with an HMAC, similarly to non-portable
signatures.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/evm/evm_crypto.c |  9 ++--
 security/integrity/evm/evm_main.c   | 82 ++++++++++++++++++++++++++---
 2 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 168c3b78ac47..9b4b07811e97 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -152,7 +152,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	/* Don't include the inode or generation number in portable
 	 * signatures
 	 */
-	if (type != EVM_XATTR_PORTABLE_DIGSIG) {
+	if (type != EVM_XATTR_PORTABLE_DIGSIG &&
+	    type != EVM_IMA_XATTR_DIGEST_LIST) {
 		hmac_misc.ino = inode->i_ino;
 		hmac_misc.generation = inode->i_generation;
 	}
@@ -169,7 +170,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	hmac_misc.mode = inode->i_mode;
 	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
 	if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
-	    type != EVM_XATTR_PORTABLE_DIGSIG)
+	    type != EVM_XATTR_PORTABLE_DIGSIG &&
+	    type != EVM_IMA_XATTR_DIGEST_LIST)
 		crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
 	crypto_shash_final(desc, digest);
 }
@@ -282,7 +284,8 @@ static int evm_is_immutable(struct dentry *dentry, struct inode *inode)
 			return 0;
 		return rc;
 	}
-	if (xattr_data->type == EVM_XATTR_PORTABLE_DIGSIG)
+	if (xattr_data->type == EVM_XATTR_PORTABLE_DIGSIG ||
+	    xattr_data->type == EVM_IMA_XATTR_DIGEST_LIST)
 		rc = 1;
 	else
 		rc = 0;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index d4d918183094..3c5247154811 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -89,7 +89,7 @@ static bool evm_key_loaded(void)
 	return (bool)(evm_initialized & EVM_KEY_MASK);
 }
 
-static int evm_find_protected_xattrs(struct dentry *dentry)
+static int evm_find_protected_xattrs(struct dentry *dentry, int *ima_present)
 {
 	struct inode *inode = d_backing_inode(dentry);
 	struct xattr_list *xattr;
@@ -106,6 +106,8 @@ static int evm_find_protected_xattrs(struct dentry *dentry)
 				continue;
 			return error;
 		}
+		if (!strcmp(xattr->name, XATTR_NAME_IMA))
+			*ima_present = 1;
 		count++;
 	}
 
@@ -134,9 +136,14 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	struct evm_ima_xattr_data *xattr_data = NULL;
 	struct signature_v2_hdr *hdr;
 	enum integrity_status evm_status = INTEGRITY_PASS;
+	enum integrity_status saved_evm_status = INTEGRITY_UNKNOWN;
 	struct evm_digest digest;
+	struct ima_digest *found_digest;
 	struct inode *inode;
-	int rc, xattr_len, evm_immutable = 0;
+	struct signature_v2_hdr evm_fake_xattr = {
+				.type = EVM_IMA_XATTR_DIGEST_LIST,
+				.version = 2, .hash_algo = HASH_ALGO_SHA256 };
+	int rc, xattr_len, evm_immutable = 0, ima_present = 0;
 
 	if (iint && (iint->evm_status == INTEGRITY_PASS ||
 		     iint->evm_status == INTEGRITY_PASS_IMMUTABLE))
@@ -150,7 +157,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	if (rc <= 0) {
 		evm_status = INTEGRITY_FAIL;
 		if (rc == -ENODATA) {
-			rc = evm_find_protected_xattrs(dentry);
+			rc = evm_find_protected_xattrs(dentry, &ima_present);
 			if (rc > 0)
 				evm_status = INTEGRITY_NOLABEL;
 			else if (rc == 0)
@@ -158,7 +165,20 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 		} else if (rc == -EOPNOTSUPP) {
 			evm_status = INTEGRITY_UNKNOWN;
 		}
-		goto out;
+		/* IMA added a fake xattr, set also EVM fake xattr */
+		if (!ima_present && xattr_name &&
+		    !strcmp(xattr_name, XATTR_NAME_IMA) &&
+		    xattr_value_len >= sizeof(struct evm_ima_xattr_data)) {
+			evm_fake_xattr.hash_algo =
+			  ((struct evm_ima_xattr_data *)xattr_value)->data[0];
+			xattr_data =
+			  (struct evm_ima_xattr_data *)&evm_fake_xattr;
+			rc = sizeof(evm_fake_xattr);
+		}
+		if (xattr_data != (struct evm_ima_xattr_data *)&evm_fake_xattr)
+			goto out;
+
+		saved_evm_status = evm_status;
 	}
 
 	xattr_len = rc;
@@ -216,19 +236,66 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 			}
 		}
 		break;
+	case EVM_IMA_XATTR_DIGEST_LIST:
+		if (xattr_len < offsetof(struct signature_v2_hdr, keyid)) {
+			evm_status = INTEGRITY_FAIL;
+			goto out;
+		}
+
+		hdr = (struct signature_v2_hdr *)xattr_data;
+		digest.hdr.algo = hdr->hash_algo;
+		rc = evm_calc_hash(dentry, xattr_name, xattr_value,
+				   xattr_value_len, xattr_data->type, &digest);
+		if (rc)
+			break;
+
+		found_digest = ima_lookup_digest(digest.digest, hdr->hash_algo,
+						 COMPACT_METADATA);
+		if (!found_digest) {
+			rc = -ENOENT;
+			break;
+		}
+
+		if (!ima_digest_allow(found_digest, IMA_APPRAISE)) {
+			rc = -EACCES;
+			break;
+		}
+
+		if (ima_digest_is_immutable(found_digest)) {
+			evm_immutable = 1;
+
+			if (iint)
+				iint->flags |= EVM_IMMUTABLE_DIGSIG;
+			evm_status = INTEGRITY_PASS_IMMUTABLE;
+		} else {
+			inode = d_backing_inode(dentry);
+			if (!IS_RDONLY(inode) &&
+			    !(inode->i_sb->s_readonly_remount) &&
+			    !IS_IMMUTABLE(inode)) {
+				rc = __vfs_removexattr(dentry, XATTR_NAME_EVM);
+				if (!rc)
+					evm_update_evmxattr(dentry, xattr_name,
+							    xattr_value,
+							    xattr_value_len);
+			}
+		}
+		break;
 	default:
 		rc = -EINVAL;
 		break;
 	}
 
-	if (rc)
+	if (rc && xattr_data == (struct evm_ima_xattr_data *)&evm_fake_xattr)
+		evm_status = saved_evm_status;
+	else if (rc)
 		evm_status = (rc == -ENODATA) ?
 				INTEGRITY_NOXATTRS : evm_immutable ?
 				INTEGRITY_FAIL_IMMUTABLE : INTEGRITY_FAIL;
 out:
 	if (iint)
 		iint->evm_status = evm_status;
-	kfree(xattr_data);
+	if (xattr_data != (struct evm_ima_xattr_data *)&evm_fake_xattr)
+		kfree(xattr_data);
 	return evm_status;
 }
 
@@ -452,7 +519,8 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name,
 		if (!xattr_value_len)
 			return -EINVAL;
 		if (xattr_data->type != EVM_IMA_XATTR_DIGSIG &&
-		    xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG)
+		    xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG &&
+		    xattr_data->type != EVM_IMA_XATTR_DIGEST_LIST)
 			return -EPERM;
 	}
 	return evm_protect_xattr(dentry, xattr_name, xattr_value,
-- 
2.27.GIT


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

end of thread, other threads:[~2020-08-03 15:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-03 15:13 [RFC][PATCH 0/3] ima: Digest Lists extension Roberto Sassu
2020-08-03 15:13 ` [RFC][PATCH 1/3] ima: Add support for measurement with digest lists Roberto Sassu
2020-08-03 15:13 ` [RFC][PATCH 2/3] ima: Add support for appraisal " Roberto Sassu
2020-08-03 15:13 ` [RFC][PATCH 3/3] evm: Add support for digest lists of metadata Roberto Sassu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.