linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] ima: digest list feature
@ 2017-11-07 10:36 Roberto Sassu
  2017-11-07 10:36 ` [PATCH v2 01/15] ima: generalize ima_read_policy() Roberto Sassu
                   ` (17 more replies)
  0 siblings, 18 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:36 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

IMA is a security module with the objective of reporting or enforcing the
integrity of a system, by measuring files accessed with the execve(),
mmap() and open() system calls. For reporting, it takes advantage of the
TPM and extends a PCR with the digest of an evaluated event. For enforcing,
it returns a value which is zero if the operation should be allowed,
negative if it should be denied.

Measuring files of an operating system introduces three main issues. First,
since the overhead introduced by the TPM is noticeable, the performance of
the system decreases linearly with the number of measurements taken. This
can be seen especially at boot time. Second, managing large measurement
lists requires computation power and network bandwidth. Third, it is
necessary to obtain reference measurements (i.e. digests of software known
to be good) to evaluate/enforce the integrity of the system. If file
signatures are used to enforce access, Linux distribution vendors have to
modify their building systems in order to include signatures in their
packages.

Digest lists aim at mitigating these issues. A digest list is a list of
digests that are taken by IMA as reference measurements and loaded before
files are accessed. Then, IMA compares calculated digests of accessed files
with digests from loaded digest lists. If the digest is found, measurement,
appraisal and audit are not performed.

Multiple digest lists can be loaded at the same time, by providing to IMA
metadata for each list: digest, signature and path. The digest is specified
so that loaded digest lists can be identified only with the measurement of
metadata. The signature is used for appraisal. If the verification
succeeds, IMA loads the digest list even if security.ima is missing.

Digest lists address the first issue because the TPM is used only if the
digest of a measured file is unknown. On a minimal system, 10 of 1400
measurements are unknown because of mutable files (e.g. log files).

Digest lists mitigate the second issue because, since digest lists do not
change, they don't have to be sent at every remote attestation. Sending
unknown measurements and a reference to digest lists would be sufficient.

Finally, digest lists address also the third issue because Linux
distribution vendors already provide the digests of files included in each
RPM package. The digest list is stored in the RPM header, signed by the
vendor.

When using digest lists, a limitation must be considered. Since a
measurement is not reported if the digest of an accessed file is found in a
digest list, the measurement list does not show which files have been
actually accessed, and in which sequence.

A possible solution would be to load a list with digest of files which are
usually accessed. Also, it is possible to selectively enable digest list
lookup only for a subset of IMA policy rules. For example, a policy could
enable digest lookup only for file accesses from the TCB and disable it
for execve() and mmap() from regular users.

Changelog

v1:
- added new policy option digest_list to selectively enable digest lookup
- added support for appraisal
- added support for immutable/mutable files

Roberto Sassu (15):
  ima: generalize ima_read_policy()
  ima: generalize ima_write_policy()
  ima: generalize policy file operations
  ima: use ima_show_htable_value to show hash table data
  ima: add functions to manage digest lists
  ima: add parser of digest lists metadata
  ima: add parser of compact digest list
  ima: add parser of RPM package headers
  ima: introduce securityfs interfaces for digest lists
  ima: disable digest lookup if digest lists are not checked
  ima: add policy action digest_list
  ima: do not update security.ima if appraisal status is not
    INTEGRITY_PASS
  evm: add kernel command line option to select protected xattrs
  ima: add support for appraisal with digest lists
  ima: add Documentation/security/IMA-digest-lists.txt

 Documentation/admin-guide/kernel-parameters.txt |   4 +
 Documentation/security/IMA-digest-lists.txt     | 161 ++++++++++++
 include/linux/evm.h                             |   6 +
 include/linux/fs.h                              |   2 +
 security/integrity/evm/evm_main.c               |  36 +++
 security/integrity/iint.c                       |   1 +
 security/integrity/ima/Kconfig                  |  19 ++
 security/integrity/ima/Makefile                 |   1 +
 security/integrity/ima/ima.h                    |  33 ++-
 security/integrity/ima/ima_api.c                |   7 +-
 security/integrity/ima/ima_appraise.c           |  52 +++-
 security/integrity/ima/ima_digest_list.c        | 326 ++++++++++++++++++++++++
 security/integrity/ima/ima_fs.c                 | 181 ++++++++-----
 security/integrity/ima/ima_main.c               |  47 +++-
 security/integrity/ima/ima_policy.c             |  33 ++-
 security/integrity/ima/ima_queue.c              |  42 +++
 security/integrity/integrity.h                  |  11 +
 17 files changed, 877 insertions(+), 85 deletions(-)
 create mode 100644 Documentation/security/IMA-digest-lists.txt
 create mode 100644 security/integrity/ima/ima_digest_list.c

-- 
2.11.0

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

* [PATCH v2 01/15] ima: generalize ima_read_policy()
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
@ 2017-11-07 10:36 ` Roberto Sassu
  2017-11-07 10:36 ` [PATCH v2 02/15] ima: generalize ima_write_policy() Roberto Sassu
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:36 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

Rename ima_read_policy() to ima_read_file(), and add file_id as new
parameter. If file_id is equal to READING_POLICY, ima_read_file() behavior
remains unchanged. ima_read_file() will be used to read digest list
metadata.

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

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index fa540c0469da..27de4558303e 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -272,7 +272,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.release = seq_release,
 };
 
-static ssize_t ima_read_policy(char *path)
+static ssize_t ima_read_file(char *path, enum kernel_read_file_id file_id)
 {
 	void *data;
 	char *datap;
@@ -285,16 +285,22 @@ static ssize_t ima_read_policy(char *path)
 	datap = path;
 	strsep(&datap, "\n");
 
-	rc = kernel_read_file_from_path(path, &data, &size, 0, READING_POLICY);
+	rc = kernel_read_file_from_path(path, &data, &size, 0, file_id);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
 		return rc;
 	}
 
 	datap = data;
-	while (size > 0 && (p = strsep(&datap, "\n"))) {
-		pr_debug("rule: %s\n", p);
-		rc = ima_parse_add_rule(p);
+	while (size > 0) {
+		if (file_id == READING_POLICY) {
+			p = strsep(&datap, "\n");
+			if (p == NULL)
+				break;
+
+			pr_debug("rule: %s\n", p);
+			rc = ima_parse_add_rule(p);
+		}
 		if (rc < 0)
 			break;
 		size -= rc;
@@ -334,7 +340,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 		goto out_free;
 
 	if (data[0] == '/') {
-		result = ima_read_policy(data);
+		result = ima_read_file(data, READING_POLICY);
 	} else if (ima_appraise & IMA_APPRAISE_POLICY) {
 		pr_err("IMA: signed policy file (specified as an absolute pathname) required\n");
 		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
-- 
2.11.0

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

* [PATCH v2 02/15] ima: generalize ima_write_policy()
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
  2017-11-07 10:36 ` [PATCH v2 01/15] ima: generalize ima_read_policy() Roberto Sassu
@ 2017-11-07 10:36 ` Roberto Sassu
  2017-11-07 10:36 ` [PATCH v2 03/15] ima: generalize policy file operations Roberto Sassu
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:36 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch renames ima_write_policy() to ima_write_data(). Also, it
determines the kernel_read_file_id from the dentry associated to the file,
and passes it to ima_read_file().

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

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 27de4558303e..864d34581081 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -28,6 +28,21 @@
 
 static DEFINE_MUTEX(ima_write_mutex);
 
+static struct dentry *ima_dir;
+static struct dentry *binary_runtime_measurements;
+static struct dentry *ascii_runtime_measurements;
+static struct dentry *runtime_measurements_count;
+static struct dentry *violations;
+static struct dentry *ima_policy;
+
+static enum kernel_read_file_id ima_get_file_id(struct dentry *dentry)
+{
+	if (dentry == ima_policy)
+		return READING_POLICY;
+
+	return READING_UNKNOWN;
+}
+
 bool ima_canonical_fmt;
 static int __init default_canonical_fmt_setup(char *str)
 {
@@ -315,11 +330,12 @@ static ssize_t ima_read_file(char *path, enum kernel_read_file_id file_id)
 		return pathlen;
 }
 
-static ssize_t ima_write_policy(struct file *file, const char __user *buf,
-				size_t datalen, loff_t *ppos)
+static ssize_t ima_write_data(struct file *file, const char __user *buf,
+			      size_t datalen, loff_t *ppos)
 {
 	char *data;
 	ssize_t result;
+	enum kernel_read_file_id file_id = ima_get_file_id(file->f_path.dentry);
 
 	if (datalen >= PAGE_SIZE)
 		datalen = PAGE_SIZE - 1;
@@ -340,34 +356,33 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 		goto out_free;
 
 	if (data[0] == '/') {
-		result = ima_read_file(data, READING_POLICY);
-	} else if (ima_appraise & IMA_APPRAISE_POLICY) {
-		pr_err("IMA: signed policy file (specified as an absolute pathname) required\n");
-		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
-				    "policy_update", "signed policy required",
-				    1, 0);
-		if (ima_appraise & IMA_APPRAISE_ENFORCE)
-			result = -EACCES;
+		result = ima_read_file(data, file_id);
+	} else if (file_id == READING_POLICY) {
+		if (ima_appraise & IMA_APPRAISE_POLICY) {
+			pr_err("IMA: signed policy file (specified "
+			       "as an absolute pathname) required\n");
+			integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
+				"policy_update", "signed policy required",
+				1, 0);
+			if (ima_appraise & IMA_APPRAISE_ENFORCE)
+				result = -EACCES;
+		} else {
+			result = ima_parse_add_rule(data);
+		}
 	} else {
-		result = ima_parse_add_rule(data);
+		pr_err("Unknown data type\n");
+		result = -EINVAL;
 	}
 	mutex_unlock(&ima_write_mutex);
 out_free:
 	kfree(data);
 out:
-	if (result < 0)
+	if (file_id == READING_POLICY && result < 0)
 		valid_policy = 0;
 
 	return result;
 }
 
-static struct dentry *ima_dir;
-static struct dentry *binary_runtime_measurements;
-static struct dentry *ascii_runtime_measurements;
-static struct dentry *runtime_measurements_count;
-static struct dentry *violations;
-static struct dentry *ima_policy;
-
 enum ima_fs_flags {
 	IMA_FS_BUSY,
 };
@@ -446,7 +461,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 
 static const struct file_operations ima_measure_policy_ops = {
 	.open = ima_open_policy,
-	.write = ima_write_policy,
+	.write = ima_write_data,
 	.read = seq_read,
 	.release = ima_release_policy,
 	.llseek = generic_file_llseek,
-- 
2.11.0

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

* [PATCH v2 03/15] ima: generalize policy file operations
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
  2017-11-07 10:36 ` [PATCH v2 01/15] ima: generalize ima_read_policy() Roberto Sassu
  2017-11-07 10:36 ` [PATCH v2 02/15] ima: generalize ima_write_policy() Roberto Sassu
@ 2017-11-07 10:36 ` Roberto Sassu
  2017-11-07 10:36 ` [PATCH v2 04/15] ima: use ima_show_htable_value to show hash table data Roberto Sassu
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:36 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch renames ima_open_policy() and ima_release_policy() respectively
to ima_open_data_upload() and ima_release_data_upload(). They will be used
to implement file operations for interfaces allowing to upload and read
provided data.

Also, the new flag IMA_POLICY_BUSY has been defined specifically for the
policy, as it might not be cleared at file release. This would prevent
userspace applications from uploading files after a policy has been loaded.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Changelog

v1:
- clear correct flag in ima_release_data_upload()
---
 security/integrity/ima/ima_fs.c | 48 ++++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 864d34581081..a5b82e075ec8 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -384,6 +384,7 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf,
 }
 
 enum ima_fs_flags {
+	IMA_POLICY_BUSY,
 	IMA_FS_BUSY,
 };
 
@@ -399,22 +400,33 @@ static const struct seq_operations ima_policy_seqops = {
 #endif
 
 /*
- * ima_open_policy: sequentialize access to the policy file
+ * ima_open_data_upload: sequentialize access to the data upload interface
  */
-static int ima_open_policy(struct inode *inode, struct file *filp)
+static int ima_open_data_upload(struct inode *inode, struct file *filp)
 {
+	enum kernel_read_file_id file_id = ima_get_file_id(filp->f_path.dentry);
+	const struct seq_operations *seq_ops = NULL;
+	enum ima_fs_flags flag = IMA_FS_BUSY;
+	bool read_allowed = false;
+
+	if (file_id == READING_POLICY) {
+		flag = IMA_POLICY_BUSY;
+#ifdef	CONFIG_IMA_READ_POLICY
+		read_allowed = true;
+		seq_ops = &ima_policy_seqops;
+#endif
+	}
+
 	if (!(filp->f_flags & O_WRONLY)) {
-#ifndef	CONFIG_IMA_READ_POLICY
-		return -EACCES;
-#else
+		if (!read_allowed)
+			return -EACCES;
 		if ((filp->f_flags & O_ACCMODE) != O_RDONLY)
 			return -EACCES;
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
-		return seq_open(filp, &ima_policy_seqops);
-#endif
+		return seq_open(filp, seq_ops);
 	}
-	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+	if (test_and_set_bit(flag, &ima_fs_flags))
 		return -EBUSY;
 	return 0;
 }
@@ -426,13 +438,19 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
  * point to the new policy rules, and remove the securityfs policy file,
  * assuming a valid policy.
  */
-static int ima_release_policy(struct inode *inode, struct file *file)
+static int ima_release_data_upload(struct inode *inode, struct file *file)
 {
+	enum kernel_read_file_id file_id = ima_get_file_id(file->f_path.dentry);
 	const char *cause = valid_policy ? "completed" : "failed";
 
 	if ((file->f_flags & O_ACCMODE) == O_RDONLY)
 		return seq_release(inode, file);
 
+	if (file_id != READING_POLICY) {
+		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+		return 0;
+	}
+
 	if (valid_policy && ima_check_policy() < 0) {
 		cause = "failed";
 		valid_policy = 0;
@@ -445,7 +463,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 	if (!valid_policy) {
 		ima_delete_rules();
 		valid_policy = 1;
-		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+		clear_bit(IMA_POLICY_BUSY, &ima_fs_flags);
 		return 0;
 	}
 
@@ -454,16 +472,16 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 	securityfs_remove(ima_policy);
 	ima_policy = NULL;
 #elif defined(CONFIG_IMA_WRITE_POLICY)
-	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+	clear_bit(IMA_POLICY_BUSY, &ima_fs_flags);
 #endif
 	return 0;
 }
 
-static const struct file_operations ima_measure_policy_ops = {
-	.open = ima_open_policy,
+static const struct file_operations ima_data_upload_ops = {
+	.open = ima_open_data_upload,
 	.write = ima_write_data,
 	.read = seq_read,
-	.release = ima_release_policy,
+	.release = ima_release_data_upload,
 	.llseek = generic_file_llseek,
 };
 
@@ -502,7 +520,7 @@ int __init ima_fs_init(void)
 
 	ima_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS,
 					    ima_dir, NULL,
-					    &ima_measure_policy_ops);
+					    &ima_data_upload_ops);
 	if (IS_ERR(ima_policy))
 		goto out;
 
-- 
2.11.0

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

* [PATCH v2 04/15] ima: use ima_show_htable_value to show hash table data
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (2 preceding siblings ...)
  2017-11-07 10:36 ` [PATCH v2 03/15] ima: generalize policy file operations Roberto Sassu
@ 2017-11-07 10:36 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 05/15] ima: add functions to manage digest lists Roberto Sassu
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:36 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch removes ima_show_htable_violations() and
ima_show_measurements_count(). ima_show_htable_value(), called by those
functions, determines which hash table data should be copied to the buffer
depending on the dentry of the file passed as argument.

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

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index a5b82e075ec8..4158ced5d3c9 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -55,38 +55,24 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
 
 static int valid_policy = 1;
 #define TMPBUFLEN 12
-static ssize_t ima_show_htable_value(char __user *buf, size_t count,
-				     loff_t *ppos, atomic_long_t *val)
+static ssize_t ima_show_htable_value(struct file *filp, char __user *buf,
+				     size_t count, loff_t *ppos)
 {
+	atomic_long_t *val = NULL;
 	char tmpbuf[TMPBUFLEN];
 	ssize_t len;
 
+	if (filp->f_path.dentry == violations)
+		val = &ima_htable.violations;
+	else if (filp->f_path.dentry == runtime_measurements_count)
+		val = &ima_htable.len;
+
 	len = scnprintf(tmpbuf, TMPBUFLEN, "%li\n", atomic_long_read(val));
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
 }
 
-static ssize_t ima_show_htable_violations(struct file *filp,
-					  char __user *buf,
-					  size_t count, loff_t *ppos)
-{
-	return ima_show_htable_value(buf, count, ppos, &ima_htable.violations);
-}
-
-static const struct file_operations ima_htable_violations_ops = {
-	.read = ima_show_htable_violations,
-	.llseek = generic_file_llseek,
-};
-
-static ssize_t ima_show_measurements_count(struct file *filp,
-					   char __user *buf,
-					   size_t count, loff_t *ppos)
-{
-	return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
-
-}
-
-static const struct file_operations ima_measurements_count_ops = {
-	.read = ima_show_measurements_count,
+static const struct file_operations ima_htable_value_ops = {
+	.read = ima_show_htable_value,
 	.llseek = generic_file_llseek,
 };
 
@@ -508,13 +494,13 @@ int __init ima_fs_init(void)
 	runtime_measurements_count =
 	    securityfs_create_file("runtime_measurements_count",
 				   S_IRUSR | S_IRGRP, ima_dir, NULL,
-				   &ima_measurements_count_ops);
+				   &ima_htable_value_ops);
 	if (IS_ERR(runtime_measurements_count))
 		goto out;
 
 	violations =
 	    securityfs_create_file("violations", S_IRUSR | S_IRGRP,
-				   ima_dir, NULL, &ima_htable_violations_ops);
+				   ima_dir, NULL, &ima_htable_value_ops);
 	if (IS_ERR(violations))
 		goto out;
 
-- 
2.11.0

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

* [PATCH v2 05/15] ima: add functions to manage digest lists
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (3 preceding siblings ...)
  2017-11-07 10:36 ` [PATCH v2 04/15] ima: use ima_show_htable_value to show hash table data Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 06/15] ima: add parser of digest lists metadata Roberto Sassu
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch first introduces a new structure called ima_digest, which
contains a digest parsed from a digest list. It has been preferred to
ima_queue_entry, as the existing structure includes an additional member
(a list head), which is not necessary for digest lookup. It also introduces
the is_mutable field, which indicates if a file with a given digest can be
updated or not.

Finally, this patch introduces functions to lookup and add a digest to the
new ima_digests_htable hash table.

Changelog

v1:
- added support for immutable/mutable files

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

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..1f6591a57fea 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -107,6 +107,12 @@ struct ima_queue_entry {
 };
 extern struct list_head ima_measurements;	/* list of all measurements */
 
+struct ima_digest {
+	struct hlist_node hnext;
+	u8 is_mutable;
+	u8 digest[0];
+};
+
 /* Some details preceding the binary serialized measurement list */
 struct ima_kexec_hdr {
 	u16 version;
@@ -150,6 +156,8 @@ void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
 struct ima_template_desc *ima_template_desc_current(void);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
+struct ima_digest *ima_lookup_loaded_digest(u8 *digest);
+int ima_add_digest_data_entry(u8 *digest, u8 is_mutable);
 int ima_measurements_show(struct seq_file *m, void *v);
 unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
@@ -166,6 +174,7 @@ struct ima_h_table {
 	struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
 };
 extern struct ima_h_table ima_htable;
+extern struct ima_h_table ima_digests_htable;
 
 static inline unsigned long ima_hash_key(u8 *digest)
 {
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index a02a86d51102..96c91c413430 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -42,6 +42,11 @@ struct ima_h_table ima_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+struct ima_h_table ima_digests_htable = {
+	.len = ATOMIC_LONG_INIT(0),
+	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
+};
+
 /* mutex protects atomicity of extending measurement list
  * and extending the TPM PCR aggregate. Since tpm_extend can take
  * long (and the tpm driver uses a mutex), we can't use the spinlock.
@@ -212,3 +217,40 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry)
 	mutex_unlock(&ima_extend_list_mutex);
 	return result;
 }
+
+struct ima_digest *ima_lookup_loaded_digest(u8 *digest)
+{
+	struct ima_digest *d = NULL;
+	int digest_len = hash_digest_size[ima_hash_algo];
+	unsigned int key = ima_hash_key(digest);
+
+	rcu_read_lock();
+	hlist_for_each_entry_rcu(d, &ima_digests_htable.queue[key], hnext) {
+		if (memcmp(d->digest, digest, digest_len) == 0)
+			break;
+	}
+	rcu_read_unlock();
+	return d;
+}
+
+int ima_add_digest_data_entry(u8 *digest, u8 is_mutable)
+{
+	struct ima_digest *d = ima_lookup_loaded_digest(digest);
+	int digest_len = hash_digest_size[ima_hash_algo];
+	unsigned int key = ima_hash_key(digest);
+
+	if (d) {
+		d->is_mutable = is_mutable;
+		return -EEXIST;
+	}
+
+	d = kmalloc(sizeof(*d) + digest_len, GFP_KERNEL);
+	if (d == NULL)
+		return -ENOMEM;
+
+	d->is_mutable = is_mutable;
+	memcpy(d->digest, digest, digest_len);
+	hlist_add_head_rcu(&d->hnext, &ima_digests_htable.queue[key]);
+	atomic_long_inc(&ima_digests_htable.len);
+	return 0;
+}
-- 
2.11.0

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

* [PATCH v2 06/15] ima: add parser of digest lists metadata
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (4 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 05/15] ima: add functions to manage digest lists Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-18  4:20   ` Serge E. Hallyn
  2017-11-07 10:37 ` [PATCH v2 07/15] ima: add parser of compact digest list Roberto Sassu
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

Digest lists can be uploaded to IMA by supplying the path of their
metadata.

Digest list metadata are:

- DATA_ALGO: algorithm of the digests to be uploaded
- DATA_DIGEST: digest of the file containing the digest list
- DATA_SIGNATURE: signature of the file containing the digest list
- DATA_FILE_PATH: pathname
- DATA_REF_ID: reference ID of the digest list
- DATA_TYPE: type of digest list

The new function ima_load_digest_list_metadata() loads digest list metadata
from a predefined position (/etc/ima/digest_lists/metadata), when rootfs
becomes available. Digest lists must be loaded before IMA appraisal is in
enforcing mode.

The new function ima_parse_digest_list_metadata() parses the metadata and
loads each digest list individually. Then, it parses the data according to
the data type specified.

To avoid the delay due to extending a PCR for each digest list, digests of
digest lists are added to the hash table. If appraisal is in enforcing
mode, this is done only if the signature verification succeeds. IMA does
not add the digest of an accessed file to the measurement list if the
digest is found in the hash table.

Changelog

v1:
- Verify signature of digest lists if appraisal is enabled
- Load digest lists when rootfs is available
- Ignore digest lists if no policy is loaded

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/fs.h                       |   2 +
 security/integrity/iint.c                |   1 +
 security/integrity/ima/Kconfig           |  19 ++++
 security/integrity/ima/Makefile          |   1 +
 security/integrity/ima/ima.h             |  12 +++
 security/integrity/ima/ima_digest_list.c | 152 +++++++++++++++++++++++++++++++
 security/integrity/integrity.h           |   8 ++
 7 files changed, 195 insertions(+)
 create mode 100644 security/integrity/ima/ima_digest_list.c

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8d7d2850963c..06737235665b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2793,6 +2793,8 @@ extern int do_pipe_flags(int *, int);
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
 	id(POLICY, security-policy)		\
 	id(X509_CERTIFICATE, x509-certificate)	\
+	id(DIGEST_LIST_METADATA, digest-list-metadata)	\
+	id(DIGEST_LIST, digest-list)	\
 	id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c84e05866052..68c14d1dfc0c 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -209,4 +209,5 @@ void __init integrity_load_keys(void)
 {
 	ima_load_x509();
 	evm_load_x509();
+	ima_load_digest_list_metadata();
 }
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 35ef69312811..fa40cee1e1a2 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -227,3 +227,22 @@ config IMA_APPRAISE_SIGNED_INIT
 	default n
 	help
 	   This option requires user-space init to be signed.
+
+config IMA_DIGEST_LIST
+	bool "Measure/appraise/audit files depending on uploaded digest lists"
+	depends on IMA
+	default n
+	help
+	   This option allows users to load digest lists. If a measured file
+	   has the same digest of one from loaded lists, IMA will not create
+	   a new measurement entry or an audit log. They will be created only
+	   when digest lists are loaded. If appraisal is enabled, access will
+	   be permitted if the digest is in the digest list. File updates
+	   will be permitted if, in addition, the digest is marked as mutable.
+
+config IMA_DIGEST_LIST_METADATA_PATH
+	string "IMA digest list metadata path"
+	depends on IMA_DIGEST_LIST
+	default "/etc/ima/digest_lists/metadata"
+	help
+	   This option defines IMA digest list metadata path.
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 29f198bde02b..00dbe3a8cb71 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -9,4 +9,5 @@ ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
 	 ima_policy.o ima_template.o ima_template_lib.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
 ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
+ima-$(CONFIG_IMA_DIGEST_LIST) += ima_digest_list.o
 obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 1f6591a57fea..1f43284788eb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -158,6 +158,18 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 struct ima_digest *ima_lookup_loaded_digest(u8 *digest);
 int ima_add_digest_data_entry(u8 *digest, u8 is_mutable);
+#ifdef CONFIG_IMA_DIGEST_LIST
+void __init ima_load_digest_list_metadata(void);
+ssize_t ima_parse_digest_list_metadata(loff_t size, void *buf);
+#else
+static inline void ima_load_digest_list_metadata(void)
+{
+}
+static inline ssize_t ima_parse_digest_list_metadata(loff_t size, void *buf)
+{
+	return -ENOTSUPP;
+}
+#endif
 int ima_measurements_show(struct seq_file *m, void *v);
 unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
new file mode 100644
index 000000000000..28172424e5a2
--- /dev/null
+++ b/security/integrity/ima/ima_digest_list.c
@@ -0,0 +1,152 @@
+/*
+ * Copyright (C) 2017 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: ima_digest_list.c
+ *      Functions to manage digest lists.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/vmalloc.h>
+
+#include "ima.h"
+#include "ima_template_lib.h"
+
+enum digest_metadata_fields {DATA_ALGO, DATA_DIGEST, DATA_SIGNATURE,
+			     DATA_FILE_PATH, DATA_REF_ID, DATA_TYPE,
+			     DATA__LAST};
+
+static int ima_parse_digest_list_data(struct ima_field_data *data)
+{
+	void *digest_list;
+	loff_t digest_list_size;
+	u16 data_algo = le16_to_cpu(*(u16 *)data[DATA_ALGO].data);
+	u16 data_type = le16_to_cpu(*(u16 *)data[DATA_TYPE].data);
+	int ret;
+
+	if (data_algo != ima_hash_algo) {
+		pr_err("Incompatible digest algorithm, expected %s\n",
+		       hash_algo_name[ima_hash_algo]);
+		return -EINVAL;
+	}
+
+	ret = kernel_read_file_from_path(data[DATA_FILE_PATH].data,
+					 &digest_list, &digest_list_size,
+					 0, READING_DIGEST_LIST);
+	if (ret < 0) {
+		pr_err("Unable to open file: %s (%d)",
+		       data[DATA_FILE_PATH].data, ret);
+		return ret;
+	}
+
+	switch (data_type) {
+	default:
+		pr_err("Parser for data type %d not implemented\n", data_type);
+		ret = -EINVAL;
+	}
+
+	if (ret < 0) {
+		pr_err("Error parsing file: %s (%d)\n",
+		       data[DATA_FILE_PATH].data, ret);
+		return ret;
+	}
+
+	vfree(digest_list);
+	return ret;
+}
+
+ssize_t ima_parse_digest_list_metadata(loff_t size, void *buf)
+{
+	struct ima_field_data entry;
+
+	struct ima_field_data entry_data[DATA__LAST] = {
+		[DATA_ALGO] = {.len = sizeof(u16)},
+		[DATA_TYPE] = {.len = sizeof(u16)},
+	};
+
+	DECLARE_BITMAP(data_mask, DATA__LAST);
+	void *bufp = buf, *bufendp = buf + size;
+	int ret;
+
+	bitmap_zero(data_mask, DATA__LAST);
+	bitmap_set(data_mask, DATA_ALGO, 1);
+	bitmap_set(data_mask, DATA_TYPE, 1);
+
+	ret = ima_parse_buf(bufp, bufendp, &bufp, 1, &entry, NULL, NULL,
+			    ENFORCE_FIELDS, "metadata list entry");
+	if (ret < 0)
+		return ret;
+
+	ret = ima_parse_buf(entry.data, entry.data + entry.len, NULL,
+			    DATA__LAST, entry_data, NULL, data_mask,
+			    ENFORCE_FIELDS | ENFORCE_BUFEND,
+			    "metadata entry data");
+	if (ret < 0)
+		goto out;
+
+	if (ima_policy_flag & IMA_APPRAISE) {
+		ret = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+				(const char *)entry_data[DATA_SIGNATURE].data,
+				entry_data[DATA_SIGNATURE].len,
+				entry_data[DATA_DIGEST].data,
+				entry_data[DATA_DIGEST].len);
+		if (ret < 0) {
+			pr_err("Failed signature verification of: %s (%d)",
+				entry_data[DATA_FILE_PATH].data, ret);
+			goto out_parse_digest_list;
+		}
+	}
+
+	ret = ima_add_digest_data_entry(entry_data[DATA_DIGEST].data, 0);
+	if (ret < 0) {
+		if (ret == -EEXIST)
+			ret = 0;
+
+		goto out;
+	}
+
+out_parse_digest_list:
+	ret = ima_parse_digest_list_data(entry_data);
+out:
+	return ret < 0 ? ret : bufp - buf;
+}
+
+void __init ima_load_digest_list_metadata(void)
+{
+	void *datap;
+	loff_t size;
+	int ret;
+
+	int unset_flags = ima_policy_flag & IMA_APPRAISE;
+
+	if (!ima_policy_flag)
+		return;
+
+	ima_policy_flag &= ~unset_flags;
+	ret = kernel_read_file_from_path(CONFIG_IMA_DIGEST_LIST_METADATA_PATH,
+					 &datap, &size, 0,
+					 READING_DIGEST_LIST_METADATA);
+	if (ret < 0)
+		pr_err("Unable to open file: %s (%d)",
+		       CONFIG_IMA_DIGEST_LIST_METADATA_PATH, ret);
+
+	ima_policy_flag |= unset_flags;
+
+	while (size > 0) {
+		ret = ima_parse_digest_list_metadata(size, datap);
+		if (ret < 0) {
+			pr_err("Unable to parse file: %s (%d)",
+			       CONFIG_IMA_DIGEST_LIST_METADATA_PATH, ret);
+			break;
+		}
+		datap += ret;
+		size -= ret;
+	}
+}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e1bf040fb110..a5951879c15c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -174,6 +174,14 @@ static inline void evm_load_x509(void)
 }
 #endif
 
+#ifdef CONFIG_IMA_DIGEST_LIST
+void __init ima_load_digest_list_metadata(void);
+#else
+static inline void ima_load_digest_list_metadata(void)
+{
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_AUDIT
 /* declarations */
 void integrity_audit_msg(int audit_msgno, struct inode *inode,
-- 
2.11.0

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

* [PATCH v2 07/15] ima: add parser of compact digest list
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (5 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 06/15] ima: add parser of digest lists metadata Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 08/15] ima: add parser of RPM package headers Roberto Sassu
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch introduces the parser for the compact digest list.
Its format is:

entry_id[2] count[4] data_len[4]
data[data_len]
entry_id[2] count[4] data_len[4]
data[data_len]
...

entry_id, count and data_len are in little endian.

This format is suitable to store a large number of digests, as there is no
metadata provided for each. Digests (which have all the same size) are
concatenated together and placed after the header.

COMPACT_DIGEST (0) and COMPACT_DIGEST_MUTABLE (1) entry IDs are supported.

If the entry ID is COMPACT_DIGEST and appraisal is in enforcing mode, file
updates are denied. If the entry ID is COMPACT_DIGEST_MUTABLE, file updates
are permitted.

Changelog

v1:
- Renamed COMPACT_LIST_ID_DIGEST to COMPACT_DIGEST
- Added support for immutable/mutable files

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

diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
index 28172424e5a2..6ad00ba32c94 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -23,6 +23,69 @@ enum digest_metadata_fields {DATA_ALGO, DATA_DIGEST, DATA_SIGNATURE,
 			     DATA_FILE_PATH, DATA_REF_ID, DATA_TYPE,
 			     DATA__LAST};
 
+enum digest_data_types {DATA_TYPE_COMPACT_LIST};
+
+enum compact_list_entry_ids {COMPACT_DIGEST, COMPACT_DIGEST_MUTABLE};
+
+struct compact_list_hdr {
+	u16 entry_id;
+	u32 count;
+	u32 datalen;
+} __packed;
+
+static int ima_parse_compact_list(loff_t size, void *buf)
+{
+	void *bufp = buf, *bufendp = buf + size;
+	int digest_len = hash_digest_size[ima_hash_algo];
+	struct compact_list_hdr *hdr;
+	u8 is_mutable = 0;
+	int ret, i;
+
+	while (bufp < bufendp) {
+		if (bufp + sizeof(*hdr) > bufendp) {
+			pr_err("compact list, missing header\n");
+			return -EINVAL;
+		}
+
+		hdr = bufp;
+
+		if (ima_canonical_fmt) {
+			hdr->entry_id = le16_to_cpu(hdr->entry_id);
+			hdr->count = le32_to_cpu(hdr->count);
+			hdr->datalen = le32_to_cpu(hdr->datalen);
+		}
+
+		switch (hdr->entry_id) {
+		case COMPACT_DIGEST_MUTABLE:
+			is_mutable = 1;
+		case COMPACT_DIGEST:
+			break;
+		default:
+			pr_err("compact list, invalid data type\n");
+			return -EINVAL;
+		}
+
+		bufp += sizeof(*hdr);
+
+		for (i = 0; i < hdr->count &&
+		     bufp + digest_len <= bufendp; i++) {
+			ret = ima_add_digest_data_entry(bufp, is_mutable);
+			if (ret < 0 && ret != -EEXIST)
+				return ret;
+
+			bufp += digest_len;
+		}
+
+		if (i != hdr->count ||
+		    bufp != (void *)hdr + sizeof(*hdr) + hdr->datalen) {
+			pr_err("compact list, invalid data\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int ima_parse_digest_list_data(struct ima_field_data *data)
 {
 	void *digest_list;
@@ -47,6 +110,9 @@ static int ima_parse_digest_list_data(struct ima_field_data *data)
 	}
 
 	switch (data_type) {
+	case DATA_TYPE_COMPACT_LIST:
+		ret = ima_parse_compact_list(digest_list_size, digest_list);
+		break;
 	default:
 		pr_err("Parser for data type %d not implemented\n", data_type);
 		ret = -EINVAL;
-- 
2.11.0

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

* [PATCH v2 08/15] ima: add parser of RPM package headers
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (6 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 07/15] ima: add parser of compact digest list Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 09/15] ima: introduce securityfs interfaces for digest lists Roberto Sassu
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch introduces a parser of RPM package headers. It extracts the
digests from the RPMTAG_FILEDIGESTS header section and converts them to
binary data before adding them to the hash table.

The advantage of this data type is that verifiers can determine who
produced that data, as headers are signed by Linux distribution vendors.
RPM header signatures can be provided as digest list metadata.

The parser also checks the RPMTAG_FILEMODES section. If the file is not
executable, the setuid/setgid/sticky bits are not set and has write
permission, the digest is marked as mutable (file updates are permitted if
appraisal is in enforcing mode).

Changelog

v1:
- Moved parser of file digests outside the first loop
- Added support for immutable/mutable files

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

diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
index 6ad00ba32c94..664a4994efbb 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -19,11 +19,14 @@
 #include "ima.h"
 #include "ima_template_lib.h"
 
+#define RPMTAG_FILEDIGESTS 1035
+#define RPMTAG_FILEMODES 1030
+
 enum digest_metadata_fields {DATA_ALGO, DATA_DIGEST, DATA_SIGNATURE,
 			     DATA_FILE_PATH, DATA_REF_ID, DATA_TYPE,
 			     DATA__LAST};
 
-enum digest_data_types {DATA_TYPE_COMPACT_LIST};
+enum digest_data_types {DATA_TYPE_COMPACT_LIST, DATA_TYPE_RPM};
 
 enum compact_list_entry_ids {COMPACT_DIGEST, COMPACT_DIGEST_MUTABLE};
 
@@ -33,6 +36,20 @@ struct compact_list_hdr {
 	u32 datalen;
 } __packed;
 
+struct rpm_hdr {
+	u32 magic;
+	u32 reserved;
+	u32 tags;
+	u32 datasize;
+} __packed;
+
+struct rpm_entryinfo {
+	int32_t tag;
+	u32 type;
+	int32_t offset;
+	u32 count;
+} __packed;
+
 static int ima_parse_compact_list(loff_t size, void *buf)
 {
 	void *bufp = buf, *bufendp = buf + size;
@@ -86,6 +103,94 @@ static int ima_parse_compact_list(loff_t size, void *buf)
 	return 0;
 }
 
+static int ima_parse_rpm(loff_t size, void *buf)
+{
+	void *bufp = buf, *bufendp = buf + size;
+	struct rpm_hdr *hdr = bufp;
+	u32 tags = be32_to_cpu(hdr->tags);
+	struct rpm_entryinfo *entry;
+	void *datap = bufp + sizeof(*hdr) + tags * sizeof(struct rpm_entryinfo);
+	void *digests = NULL, *modes = NULL;
+	u32 digests_count, modes_count;
+	int digest_len = hash_digest_size[ima_hash_algo];
+	u8 digest[digest_len];
+	int ret, i;
+
+	const unsigned char rpm_header_magic[8] = {
+		0x8e, 0xad, 0xe8, 0x01, 0x00, 0x00, 0x00, 0x00
+	};
+
+	if (size < sizeof(*hdr)) {
+		pr_err("Missing RPM header\n");
+		return -EINVAL;
+	}
+
+	if (memcmp(bufp, rpm_header_magic, sizeof(rpm_header_magic))) {
+		pr_err("Invalid RPM header\n");
+		return -EINVAL;
+	}
+
+	bufp += sizeof(*hdr);
+
+	for (i = 0; i < tags && (bufp + sizeof(*entry)) <= bufendp;
+	     i++, bufp += sizeof(*entry)) {
+		entry = bufp;
+
+		if (be32_to_cpu(entry->tag) == RPMTAG_FILEDIGESTS) {
+			digests = datap + be32_to_cpu(entry->offset);
+			digests_count = be32_to_cpu(entry->count);
+		}
+		if (be32_to_cpu(entry->tag) == RPMTAG_FILEMODES) {
+			modes = datap + be32_to_cpu(entry->offset);
+			modes_count = be32_to_cpu(entry->count);
+		}
+		if (digests && modes)
+			break;
+	}
+
+	if (digests == NULL)
+		return 0;
+
+	for (i = 0; i < digests_count && digests < bufendp; i++) {
+		u8 is_mutable = 0;
+		u16 mode;
+
+		if (strlen(digests) == 0) {
+			digests++;
+			continue;
+		}
+
+		if (modes) {
+			if (modes + (i + 1) * sizeof(mode) > bufendp) {
+				pr_err("RPM header read at invalid offset\n");
+				return -EINVAL;
+			}
+
+			mode = be16_to_cpu(*(u16 *)(modes + i * sizeof(mode)));
+			if (!(mode & (S_IXUGO | S_ISUID | S_ISGID | S_ISVTX)) &&
+			    (mode & S_IWUGO))
+				is_mutable = 1;
+		}
+
+		if (digests + digest_len * 2 + 1 > bufendp) {
+			pr_err("RPM header read at invalid offset\n");
+			return -EINVAL;
+		}
+
+		ret = hex2bin(digest, digests, digest_len);
+		if (ret < 0)
+			return -EINVAL;
+
+		ret = ima_add_digest_data_entry(digest, is_mutable);
+		if (ret < 0 && ret != -EEXIST)
+			return ret;
+
+		digests += digest_len * 2 + 1;
+	}
+
+	return 0;
+}
+
 static int ima_parse_digest_list_data(struct ima_field_data *data)
 {
 	void *digest_list;
@@ -113,6 +218,9 @@ static int ima_parse_digest_list_data(struct ima_field_data *data)
 	case DATA_TYPE_COMPACT_LIST:
 		ret = ima_parse_compact_list(digest_list_size, digest_list);
 		break;
+	case DATA_TYPE_RPM:
+		ret = ima_parse_rpm(digest_list_size, digest_list);
+		break;
 	default:
 		pr_err("Parser for data type %d not implemented\n", data_type);
 		ret = -EINVAL;
-- 
2.11.0

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

* [PATCH v2 09/15] ima: introduce securityfs interfaces for digest lists
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (7 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 08/15] ima: add parser of RPM package headers Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 10/15] ima: disable digest lookup if digest lists are not checked Roberto Sassu
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch introduces the file 'digest_lists' in the securityfs filesystem,
to load digest lists metadata. IMA will parse the metadata and load the
digest lists from the path provided.

It also introduces 'digests_count', to show the number of digests stored in
the ima_digests_htable hash table.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Changelog

v1:
- Deny upload of digest lists if no policy is loaded
---
 security/integrity/ima/ima_fs.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 4158ced5d3c9..1ed717d94487 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -34,11 +34,15 @@ static struct dentry *ascii_runtime_measurements;
 static struct dentry *runtime_measurements_count;
 static struct dentry *violations;
 static struct dentry *ima_policy;
+static struct dentry *digest_lists;
+static struct dentry *digests_count;
 
 static enum kernel_read_file_id ima_get_file_id(struct dentry *dentry)
 {
 	if (dentry == ima_policy)
 		return READING_POLICY;
+	else if (dentry == digest_lists)
+		return READING_DIGEST_LIST_METADATA;
 
 	return READING_UNKNOWN;
 }
@@ -66,6 +70,8 @@ static ssize_t ima_show_htable_value(struct file *filp, char __user *buf,
 		val = &ima_htable.violations;
 	else if (filp->f_path.dentry == runtime_measurements_count)
 		val = &ima_htable.len;
+	else if (filp->f_path.dentry == digests_count)
+		val = &ima_digests_htable.len;
 
 	len = scnprintf(tmpbuf, TMPBUFLEN, "%li\n", atomic_long_read(val));
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
@@ -301,6 +307,9 @@ static ssize_t ima_read_file(char *path, enum kernel_read_file_id file_id)
 
 			pr_debug("rule: %s\n", p);
 			rc = ima_parse_add_rule(p);
+		} else if (file_id == READING_DIGEST_LIST_METADATA) {
+			rc = ima_parse_digest_list_metadata(size, datap);
+			datap += rc;
 		}
 		if (rc < 0)
 			break;
@@ -401,7 +410,8 @@ static int ima_open_data_upload(struct inode *inode, struct file *filp)
 		read_allowed = true;
 		seq_ops = &ima_policy_seqops;
 #endif
-	}
+	} else if (file_id == READING_DIGEST_LIST_METADATA && !ima_policy_flag)
+		return -EACCES;
 
 	if (!(filp->f_flags & O_WRONLY)) {
 		if (!read_allowed)
@@ -510,8 +520,22 @@ int __init ima_fs_init(void)
 	if (IS_ERR(ima_policy))
 		goto out;
 
+#ifdef CONFIG_IMA_DIGEST_LIST
+	digest_lists = securityfs_create_file("digest_lists", S_IWUSR, ima_dir,
+					      NULL, &ima_data_upload_ops);
+	if (IS_ERR(digest_lists))
+		goto out;
+
+	digests_count = securityfs_create_file("digests_count",
+					       S_IRUSR | S_IRGRP, ima_dir,
+					       NULL, &ima_htable_value_ops);
+	if (IS_ERR(digests_count))
+		goto out;
+#endif
 	return 0;
 out:
+	securityfs_remove(digests_count);
+	securityfs_remove(digest_lists);
 	securityfs_remove(violations);
 	securityfs_remove(runtime_measurements_count);
 	securityfs_remove(ascii_runtime_measurements);
-- 
2.11.0

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

* [PATCH v2 10/15] ima: disable digest lookup if digest lists are not checked
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (8 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 09/15] ima: introduce securityfs interfaces for digest lists Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 11/15] ima: add policy action digest_list Roberto Sassu
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch introduces two new hooks DIGEST_LIST_METADATA_CHECK and
DIGEST_LIST_CHECK, which are called respectively when parsing digest list
metadata and digest lists.

It also checks that rules for these two hooks are always specified in the
current policy. Without them, digest lists could be uploaded to IMA without
adding a new entry to the measurement list, without verifying the
signature, or without auditing the operation. Digest lookup is disabled for
each missing policy action (measure, appraise, audit).

Digest lookup is also disabled if CONFIG_IMA_DIGEST_LIST is not defined.

Changelog

v1:
- clear IMA_MEASURE action flag only if it was in the policy
- check if at least one action is allowed before searching the file digest
- retrieve ima_digest structure
- set IMA action flags in ima_disable_digest_lookup
- added DIGEST_LIST_METADATA_CHECK hook
- update MEASURE/APPRAISE policies

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

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 1f43284788eb..4b3b1ca5c09a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -204,6 +204,8 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	hook(KEXEC_KERNEL_CHECK)	\
 	hook(KEXEC_INITRAMFS_CHECK)	\
 	hook(POLICY_CHECK)		\
+	hook(DIGEST_LIST_METADATA_CHECK)		\
+	hook(DIGEST_LIST_CHECK)		\
 	hook(MAX_CHECK)
 #define __ima_hook_enumify(ENUM)	ENUM,
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 766fe2e77419..840362734f91 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -29,6 +29,12 @@
 
 int ima_initialized;
 
+#ifdef CONFIG_IMA_DIGEST_LIST
+static int ima_disable_digest_lookup;
+#else
+static int ima_disable_digest_lookup = IMA_DO_MASK & ~IMA_APPRAISE_SUBMASK;
+#endif
+
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise = IMA_APPRAISE_ENFORCE;
 #else
@@ -168,12 +174,20 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	char *pathbuf = NULL;
 	char filename[NAME_MAX];
 	const char *pathname = NULL;
-	int rc = -ENOMEM, action, must_appraise;
+	int rc = -ENOMEM, action, action_done, must_appraise, digest_lookup;
 	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+	struct ima_digest *found_digest = NULL;
 	struct evm_ima_xattr_data *xattr_value = NULL;
 	int xattr_len = 0;
 	bool violation_check;
 	enum hash_algo hash_algo;
+	int disable_mask = (func == DIGEST_LIST_CHECK) ?
+			   IMA_DO_MASK & ~IMA_APPRAISE_SUBMASK :
+			   IMA_DO_MASK & ~(IMA_APPRAISE | IMA_APPRAISE_SUBMASK);
+
+	if ((func == DIGEST_LIST_METADATA_CHECK || func == DIGEST_LIST_CHECK) &&
+	    !ima_policy_flag)
+		ima_disable_digest_lookup = disable_mask;
 
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;
@@ -185,6 +199,9 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	action = ima_get_action(inode, mask, func, &pcr);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
 			   (ima_policy_flag & IMA_MEASURE));
+	if (func == DIGEST_LIST_METADATA_CHECK || func == DIGEST_LIST_CHECK)
+		ima_disable_digest_lookup |= (~action & disable_mask);
+
 	if (!action && !violation_check)
 		return 0;
 
@@ -242,6 +259,21 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_digsig;
 
+	digest_lookup = action & ~ima_disable_digest_lookup;
+	if (digest_lookup) {
+		found_digest = ima_lookup_loaded_digest(iint->ima_hash->digest);
+		if (found_digest) {
+			action_done = digest_lookup & (IMA_MEASURE | IMA_AUDIT);
+			action &= ~action_done;
+			iint->flags |= (action_done << 1);
+
+			if (!(digest_lookup & IMA_APPRAISE))
+				found_digest = NULL;
+			if (digest_lookup & IMA_MEASURE)
+				iint->measured_pcrs |= (0x1 << pcr);
+		}
+	}
+
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
@@ -378,7 +410,9 @@ static int read_idmap[READING_MAX_ID] = {
 	[READING_MODULE] = MODULE_CHECK,
 	[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
 	[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
-	[READING_POLICY] = POLICY_CHECK
+	[READING_POLICY] = POLICY_CHECK,
+	[READING_DIGEST_LIST_METADATA] = DIGEST_LIST_METADATA_CHECK,
+	[READING_DIGEST_LIST] = DIGEST_LIST_CHECK
 };
 
 /**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index ee4613fa5840..2767f7901f94 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -127,9 +127,20 @@ static struct ima_rule_entry default_measurement_rules[] __ro_after_init = {
 	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
 	{.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC},
 	{.action = MEASURE, .func = POLICY_CHECK, .flags = IMA_FUNC},
+#ifdef CONFIG_IMA_DIGEST_LIST
+	{.action = MEASURE, .func = DIGEST_LIST_METADATA_CHECK,
+	 .flags = IMA_FUNC},
+	{.action = MEASURE, .func = DIGEST_LIST_CHECK, .flags = IMA_FUNC},
+#endif
 };
 
 static struct ima_rule_entry default_appraise_rules[] __ro_after_init = {
+#ifdef CONFIG_IMA_DIGEST_LIST
+	{.action = APPRAISE, .func = DIGEST_LIST_METADATA_CHECK,
+	 .flags = IMA_FUNC},
+	{.action = APPRAISE, .func = DIGEST_LIST_CHECK,
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
 	{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
@@ -699,6 +710,11 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = KEXEC_INITRAMFS_CHECK;
 			else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
 				entry->func = POLICY_CHECK;
+			else if (strcmp(args[0].from, "DIGEST_LIST_CHECK") == 0)
+				entry->func = DIGEST_LIST_CHECK;
+			else if (strcmp(args[0].from,
+				 "DIGEST_LIST_METADATA_CHECK") == 0)
+				entry->func = DIGEST_LIST_METADATA_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)
-- 
2.11.0

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

* [PATCH v2 11/15] ima: add policy action digest_list
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (9 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 10/15] ima: disable digest lookup if digest lists are not checked Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 12/15] ima: do not update security.ima if appraisal status is not INTEGRITY_PASS Roberto Sassu
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

The new policy action 'digest_list' has been added to selectively search a
digest in the ima_digests_htable hash table only for specific rules.

The main use case would be to use digest lists to measure/appraise the TCB,
so that the PCR 10 value is predictable, and to extend a different PCR if
binaries and libraries are accessed by regular users. The policy should be:

measure func=BPRM_CHECK uid=0 digest_list
measure func=BPRM_CHECK pcr=11
measure func=MMAP_CHECK uid=0 digest_list
measure func=MMAP_CHECK pcr=11
measure func=FILE_CHECK uid=0 digest_list mask=^MAY_READ

appraise uid=0 digest_list

Digest lookup is enabled if the digest_list policy action is not specified
in the policy.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h          |  4 ++--
 security/integrity/ima/ima_api.c      |  7 +++++--
 security/integrity/ima/ima_appraise.c |  2 +-
 security/integrity/ima/ima_main.c     |  5 ++++-
 security/integrity/ima/ima_policy.c   | 17 ++++++++++++++---
 security/integrity/integrity.h        |  1 +
 6 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 4b3b1ca5c09a..ddd0e1e7e99b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -215,7 +215,7 @@ enum ima_hooks {
 
 /* LIM API function definitions */
 int ima_get_action(struct inode *inode, int mask,
-		   enum ima_hooks func, int *pcr);
+		   enum ima_hooks func, int *pcr, int *digest_mask);
 int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
 			    struct file *file, void *buf, loff_t size,
@@ -236,7 +236,7 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
 
 /* IMA policy related functions */
 int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags, int *pcr);
+		     int flags, int *pcr, int *digest_mask);
 void ima_init_policy(void);
 void ima_update_policy(void);
 void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c7e8db0ea4c0..01dfab95b6ac 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -161,6 +161,8 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  *        MAY_APPEND)
  * @func: caller identifier
  * @pcr: pointer filled in if matched measure policy sets pcr=
+ * @digest_mask: pointer filled with actions for which digest lookup
+ *               must be disabled
  *
  * The policy is defined in terms of keypairs:
  *		subj=, obj=, type=, func=, mask=, fsmagic=
@@ -172,13 +174,14 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  * Returns IMA_MEASURE, IMA_APPRAISE mask.
  *
  */
-int ima_get_action(struct inode *inode, int mask, enum ima_hooks func, int *pcr)
+int ima_get_action(struct inode *inode, int mask, enum ima_hooks func, int *pcr,
+		   int *digest_mask)
 {
 	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE;
 
 	flags &= ima_policy_flag;
 
-	return ima_match_policy(inode, func, mask, flags, pcr);
+	return ima_match_policy(inode, func, mask, flags, pcr, digest_mask);
 }
 
 /*
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index ec7dfa02c051..285a53452fb5 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -53,7 +53,7 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
 	if (!ima_appraise)
 		return 0;
 
-	return ima_match_policy(inode, func, mask, IMA_APPRAISE, NULL);
+	return ima_match_policy(inode, func, mask, IMA_APPRAISE, NULL, NULL);
 }
 
 static int ima_fix_xattr(struct dentry *dentry,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 840362734f91..d58199c8435c 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -184,6 +184,8 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	int disable_mask = (func == DIGEST_LIST_CHECK) ?
 			   IMA_DO_MASK & ~IMA_APPRAISE_SUBMASK :
 			   IMA_DO_MASK & ~(IMA_APPRAISE | IMA_APPRAISE_SUBMASK);
+	int disable_mask_policy = (ima_policy_flag & IMA_SEARCH_DIGEST_LIST) ?
+				  IMA_DO_MASK & ~IMA_APPRAISE_SUBMASK : 0;
 
 	if ((func == DIGEST_LIST_METADATA_CHECK || func == DIGEST_LIST_CHECK) &&
 	    !ima_policy_flag)
@@ -196,7 +198,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	 * bitmask based on the appraise/audit/measurement policy.
 	 * Included is the appraise submask.
 	 */
-	action = ima_get_action(inode, mask, func, &pcr);
+	action = ima_get_action(inode, mask, func, &pcr, &disable_mask_policy);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
 			   (ima_policy_flag & IMA_MEASURE));
 	if (func == DIGEST_LIST_METADATA_CHECK || func == DIGEST_LIST_CHECK)
@@ -260,6 +262,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		goto out_digsig;
 
 	digest_lookup = action & ~ima_disable_digest_lookup;
+	digest_lookup &= ~disable_mask_policy;
 	if (digest_lookup) {
 		found_digest = ima_lookup_loaded_digest(iint->ima_hash->digest);
 		if (found_digest) {
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 2767f7901f94..b9d38a0d45a6 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -365,6 +365,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  * @func: IMA hook identifier
  * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
  * @pcr: set the pcr to extend
+ * @digest_mask: unset actions for which digest lookup should be enabled
  *
  * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
  * conditions.
@@ -374,7 +375,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  * than writes so ima_match_policy() is classical RCU candidate.
  */
 int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags, int *pcr)
+		     int flags, int *pcr, int *digest_mask)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
@@ -401,6 +402,8 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
 
 		if ((pcr) && (entry->flags & IMA_PCR))
 			*pcr = entry->pcr;
+		if (digest_mask && (entry->flags & IMA_SEARCH_DIGEST_LIST))
+			*digest_mask &= (~entry->action & IMA_DO_MASK);
 
 		if (!actmask)
 			break;
@@ -421,8 +424,10 @@ void ima_update_policy_flag(void)
 	struct ima_rule_entry *entry;
 
 	list_for_each_entry(entry, ima_rules, list) {
+		int digest_list = entry->flags & IMA_SEARCH_DIGEST_LIST;
+
 		if (entry->action & IMA_DO_MASK)
-			ima_policy_flag |= entry->action;
+			ima_policy_flag |= (entry->action | digest_list);
 	}
 
 	ima_appraise |= temp_ima_appraise;
@@ -540,7 +545,7 @@ enum {
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
 	Opt_appraise_type, Opt_permit_directio,
-	Opt_pcr
+	Opt_pcr, Opt_digest_list
 };
 
 static match_table_t policy_tokens = {
@@ -571,6 +576,7 @@ static match_table_t policy_tokens = {
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_permit_directio, "permit_directio"},
 	{Opt_pcr, "pcr=%s"},
+	{Opt_digest_list, "digest_list"},
 	{Opt_err, NULL}
 };
 
@@ -889,6 +895,9 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->flags |= IMA_PCR;
 
 			break;
+		case Opt_digest_list:
+			entry->flags |= IMA_SEARCH_DIGEST_LIST;
+			break;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
 			result = -EINVAL;
@@ -1158,6 +1167,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, "appraise_type=imasig ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
 		seq_puts(m, "permit_directio ");
+	if (entry->flags & IMA_SEARCH_DIGEST_LIST)
+		seq_puts(m, "digest_list ");
 	rcu_read_unlock();
 	seq_puts(m, "\n");
 	return 0;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a5951879c15c..b46461a5f43f 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -33,6 +33,7 @@
 #define IMA_DIGSIG_REQUIRED	0x02000000
 #define IMA_PERMIT_DIRECTIO	0x04000000
 #define IMA_NEW_FILE		0x08000000
+#define IMA_SEARCH_DIGEST_LIST	0x10000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_APPRAISE_SUBMASK)
-- 
2.11.0

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

* [PATCH v2 12/15] ima: do not update security.ima if appraisal status is not INTEGRITY_PASS
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (10 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 11/15] ima: add policy action digest_list Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-18  4:25   ` Serge E. Hallyn
  2017-11-07 10:37 ` [PATCH v2 13/15] evm: add kernel command line option to select protected xattrs Roberto Sassu
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters")
moved the call of ima_file_check() from may_open() to do_filp_open() at a
point where the file descriptor is already opened.

This breaks the assumption made by IMA that file descriptors being closed
belong to files whose access was granted by ima_file_check(). The
consequence is that security.ima and security.evm are updated with good
values, regardless of the current appraisal status.

For example, if a file does not have security.ima, IMA will create it after
opening the file for writing, even if access is denied. Access to the file
will be allowed afterwards.

Avoid this issue by checking the appraisal status before updating
security.ima.

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

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 285a53452fb5..1b2236e637ff 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -320,6 +320,9 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
 	if (iint->flags & IMA_DIGSIG)
 		return;
 
+	if (iint->ima_file_status != INTEGRITY_PASS)
+		return;
+
 	rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo);
 	if (rc < 0)
 		return;
-- 
2.11.0

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

* [PATCH v2 13/15] evm: add kernel command line option to select protected xattrs
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (11 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 12/15] ima: do not update security.ima if appraisal status is not INTEGRITY_PASS Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 14/15] ima: add support for appraisal with digest lists Roberto Sassu
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

EVM protects all extended attributes defined by LSMs, if LSMs are enabled
in the kernel configuration.

It is desirable to select a subset of extended attributes at run-time, so
that setting remaining extended attributes is allowed if they should not be
protected. At the moment, this option can be used to select security.ima
and ignore other extended attributes.

Protecting only security.ima is useful when the IMA policy does not rely on
the correctness of file metadata (e.g. when only rules based on subject
criteria are specified).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  4 +++
 include/linux/evm.h                             |  6 +++++
 security/integrity/evm/evm_main.c               | 36 +++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 05496622b4ef..ac292121bd90 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1149,6 +1149,10 @@
 			Permit 'security.evm' to be updated regardless of
 			current integrity status.
 
+	evm_xattrs=	[EVM]
+			Format: { "security.ima" }
+			Protect only security.ima extended attribute.
+
 	failslab=
 	fail_page_alloc=
 	fail_make_request=[KNL]
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 35ed9a8a403a..d31c02fb2ac1 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -14,6 +14,7 @@
 struct integrity_iint_cache;
 
 #ifdef CONFIG_EVM
+extern int evm_set_includes_protected_xattrs(char **set, int count);
 extern int evm_set_key(void *key, size_t keylen);
 extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
 					     const char *xattr_name,
@@ -44,6 +45,11 @@ static inline int posix_xattr_acl(const char *xattrname)
 #endif
 #else
 
+static inline int evm_set_includes_protected_xattrs(char **set, int count)
+{
+	return 1;
+}
+
 static inline int evm_set_key(void *key, size_t keylen)
 {
 	return -EOPNOTSUPP;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 9826c02e2db8..afd6ee027250 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -68,6 +68,18 @@ static int __init evm_set_fixmode(char *str)
 }
 __setup("evm=", evm_set_fixmode);
 
+static int __init evm_select_xattrs(char *str)
+{
+#ifdef CONFIG_IMA_APPRAISE
+	if (strcmp(str, XATTR_NAME_IMA) == 0) {
+		evm_config_xattrnames[0] = XATTR_NAME_IMA;
+		evm_config_xattrnames[1] = NULL;
+	}
+#endif
+	return 0;
+}
+__setup("evm_xattrs=", evm_select_xattrs);
+
 static void __init evm_init_config(void)
 {
 #ifdef CONFIG_EVM_ATTR_FSUUID
@@ -221,6 +233,30 @@ static int evm_protected_xattr(const char *req_xattr_name)
 }
 
 /**
+ * evm_set_includes_protected_xattrs - check if set includes protected xattrs
+ * @set: array of extended attribute names to check
+ * @count: size of array
+ *
+ * Check if the provided set includes all EVM protected extended attributes.
+ *
+ * Return: 1 if set includes all protected xattrs, 0 otherwise.
+ */
+int evm_set_includes_protected_xattrs(char **set, int count)
+{
+	char **xattr;
+	int i, found = 1;
+
+	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
+		for (i = 0; i < count; i++)
+			if (strcmp(set[i], *xattr) == 0)
+				break;
+		if (i == count)
+			return 0;
+	}
+	return found;
+}
+
+/**
  * evm_verifyxattr - verify the integrity of the requested xattr
  * @dentry: object of the verify xattr
  * @xattr_name: requested xattr
-- 
2.11.0

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

* [PATCH v2 14/15] ima: add support for appraisal with digest lists
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (12 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 13/15] evm: add kernel command line option to select protected xattrs Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 10:37 ` [PATCH v2 15/15] ima: add Documentation/security/IMA-digest-lists.txt Roberto Sassu
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

Appraisal verification consists on comparing the calculated digest of an
accessed file with the value of the security.ima extended attribute. With
digest lists, appraisal verification succeeds if the calculated digest is
included in a list. Since the digital signature of each digest list is
verified, it is not possible to allow access of unauthorized files.

For mutable files, IMA writes the current digest to security.ima so that
next file accesses are allowed even if the files have been modified. For
immutable files, IMA writes security.ima only if also additional extended
attributes should be protected by EVM. Otherwise, security.ima would be
redundant, as digest lists provide reference values.

When IMA writes security.ima, EVM calculates the HMAC based on the current
value of protected extended attributes. Without file signatures, initial
extended attribute values will not checked until digest lists include them.
When extended attribute values are available, IMA will check them as the
same as the digest, and will not write security.ima for immutable files if
values are provided for all extended attributes protected by EVM.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h          |  6 +++--
 security/integrity/ima/ima_appraise.c | 47 +++++++++++++++++++++++++++++++----
 security/integrity/ima/ima_main.c     |  6 +++--
 security/integrity/integrity.h        |  2 ++
 4 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ddd0e1e7e99b..5f8e0740a33e 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -261,7 +261,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, int opened);
+			     int xattr_len, int opened,
+			     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,
@@ -277,7 +278,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func,
 					   struct file *file,
 					   const unsigned char *filename,
 					   struct evm_ima_xattr_data *xattr_value,
-					   int xattr_len, int opened)
+					   int xattr_len, int opened,
+					   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 1b2236e637ff..fd03a0278fba 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -201,17 +201,27 @@ 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, int opened)
+			     int xattr_len, int opened,
+			     struct ima_digest *found_digest)
 {
 	static const char op[] = "appraise_data";
 	char *cause = "unknown";
 	struct dentry *dentry = file_dentry(file);
 	struct inode *inode = d_backing_inode(dentry);
 	enum integrity_status status = INTEGRITY_UNKNOWN;
-	int rc = xattr_len, hash_start = 0;
+	struct evm_ima_xattr_data digest_list_value;
+	char *list_metadata = XATTR_NAME_IMA;
+	int rc = xattr_len, hash_start = 0, cache_flags_disabled = 0;
 
 	if (!(inode->i_opflags & IOP_XATTR))
-		return INTEGRITY_UNKNOWN;
+		return found_digest ? INTEGRITY_PASS : INTEGRITY_UNKNOWN;
+
+	if (found_digest && (!rc || rc == -ENODATA)) {
+		digest_list_value.type = found_digest->is_mutable ?
+			IMA_DIGEST_LIST_MUTABLE : IMA_DIGEST_LIST_IMMUTABLE;
+		xattr_value = &digest_list_value;
+		rc = sizeof(*xattr_value);
+	}
 
 	if (rc <= 0) {
 		if (rc && rc != -ENODATA)
@@ -228,6 +238,9 @@ int ima_appraise_measurement(enum ima_hooks func,
 		goto out;
 	}
 
+	if (xattr_value == &digest_list_value)
+		goto no_evm_check;
+
 	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
 	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
 		if ((status == INTEGRITY_NOLABEL)
@@ -237,13 +250,17 @@ int ima_appraise_measurement(enum ima_hooks func,
 			cause = "invalid-HMAC";
 		goto out;
 	}
+
+no_evm_check:
 	switch (xattr_value->type) {
 	case IMA_XATTR_DIGEST_NG:
 		/* first byte contains algorithm id */
 		hash_start = 1;
 		/* fall through */
 	case IMA_XATTR_DIGEST:
-		if (iint->flags & IMA_DIGSIG_REQUIRED) {
+		if (found_digest && !found_digest->is_mutable)
+			iint->flags |= IMA_DIGSIG;
+		else if (iint->flags & IMA_DIGSIG_REQUIRED) {
 			cause = "IMA-signature-required";
 			status = INTEGRITY_FAIL;
 			break;
@@ -280,6 +297,25 @@ int ima_appraise_measurement(enum ima_hooks func,
 			status = INTEGRITY_PASS;
 		}
 		break;
+	case IMA_DIGEST_LIST_MUTABLE:
+		if (iint->flags & IMA_DIGSIG_REQUIRED) {
+			cause = "IMA-signature-required";
+			status = INTEGRITY_FAIL;
+			break;
+		}
+		if (ima_fix_xattr(dentry, iint) == -EROFS)
+			cache_flags_disabled = 1;
+
+		status = INTEGRITY_PASS;
+		break;
+	case IMA_DIGEST_LIST_IMMUTABLE:
+		iint->flags |= IMA_DIGSIG;
+		if (!evm_set_includes_protected_xattrs(&list_metadata, 1))
+			if (ima_fix_xattr(dentry, iint) == -EROFS)
+				cache_flags_disabled = 1;
+
+		status = INTEGRITY_PASS;
+		break;
 	default:
 		status = INTEGRITY_UNKNOWN;
 		cause = "unknown-ima-data";
@@ -302,7 +338,8 @@ int ima_appraise_measurement(enum ima_hooks func,
 		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
 				    op, cause, rc, 0);
 	} else {
-		ima_cache_flags(iint, func);
+		if (!cache_flags_disabled)
+			ima_cache_flags(iint, func);
 	}
 	ima_set_cache_status(iint, func, status);
 	return status;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d58199c8435c..a6cd414b46e3 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -270,7 +270,8 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 			action &= ~action_done;
 			iint->flags |= (action_done << 1);
 
-			if (!(digest_lookup & IMA_APPRAISE))
+			if (!(digest_lookup & IMA_APPRAISE) ||
+			    opened & FILE_CREATED)
 				found_digest = NULL;
 			if (digest_lookup & IMA_MEASURE)
 				iint->measured_pcrs |= (0x1 << pcr);
@@ -282,7 +283,8 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK))
 		rc = ima_appraise_measurement(func, iint, file, pathname,
-					      xattr_value, xattr_len, opened);
+					      xattr_value, xattr_len, opened,
+					      found_digest);
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
 				      xattr_value, xattr_len, pcr);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index b46461a5f43f..f6b3c15dc57f 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -59,6 +59,8 @@ enum evm_ima_xattr_type {
 	EVM_XATTR_HMAC,
 	EVM_IMA_XATTR_DIGSIG,
 	IMA_XATTR_DIGEST_NG,
+	IMA_DIGEST_LIST_MUTABLE,
+	IMA_DIGEST_LIST_IMMUTABLE,
 	IMA_XATTR_LAST
 };
 
-- 
2.11.0

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

* [PATCH v2 15/15] ima: add Documentation/security/IMA-digest-lists.txt
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (13 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 14/15] ima: add support for appraisal with digest lists Roberto Sassu
@ 2017-11-07 10:37 ` Roberto Sassu
  2017-11-07 13:37 ` [PATCH v2 00/15] ima: digest list feature Mimi Zohar
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 10:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu, Roberto Sassu

This patch adds the documentation of digest lists.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/security/IMA-digest-lists.txt | 161 ++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)
 create mode 100644 Documentation/security/IMA-digest-lists.txt

diff --git a/Documentation/security/IMA-digest-lists.txt b/Documentation/security/IMA-digest-lists.txt
new file mode 100644
index 000000000000..afa860bbe53e
--- /dev/null
+++ b/Documentation/security/IMA-digest-lists.txt
@@ -0,0 +1,161 @@
+============
+Digest Lists
+============
+
+
+INTRODUCTION
+============
+
+IMA is a security module with the objective of reporting or enforcing the
+integrity of a system, by measuring files accessed with the execve(),
+mmap() and open() system calls. For reporting, it takes advantage of the
+TPM and extends a PCR with the digest of an evaluated event. For enforcing,
+it returns a value which is zero if the operation should be allowed,
+negative if it should be denied.
+
+Measuring files of an operating system introduces three main issues. First,
+since the overhead introduced by the TPM is noticeable, the performance of
+the system decreases linearly with the number of measurements taken. This
+can be seen especially at boot time. Second, managing large measurement
+lists requires computation power and network bandwidth. Third, it is
+necessary to obtain reference measurements (i.e. digests of software known
+to be good) to evaluate/enforce the integrity of the system. If file
+signatures are used to enforce access, Linux distribution vendors have to
+modify their building systems in order to include signatures in their
+packages.
+
+Digest lists aim at mitigating these issues. A digest list is a list of
+digests that are taken by IMA as reference measurements and loaded before
+files are accessed. Then, IMA compares calculated digests of accessed files
+with digests from loaded digest lists. If the digest is found, measurement,
+appraisal and audit are not performed.
+
+Multiple digest lists can be loaded at the same time, by providing to IMA
+metadata for each list: digest, signature and path. The digest is specified
+so that loaded digest lists can be identified only with the measurement of
+metadata. The signature is used for appraisal. If the verification
+succeeds, IMA loads the digest list even if security.ima is missing.
+
+Digest lists address the first issue because the TPM is used only if the
+digest of a measured file is unknown. On a minimal system, 10 of 1400
+measurements are unknown because of mutable files (e.g. log files).
+
+Digest lists mitigate the second issue because, since digest lists do not
+change, they don't have to be sent at every remote attestation. Sending
+unknown measurements and a reference to digest lists would be sufficient.
+
+Finally, digest lists address also the third issue because Linux
+distribution vendors already provide the digests of files included in each
+RPM package. The digest list is stored in the RPM header, signed by the
+vendor.
+
+When using digest lists, a limitation must be considered. Since a
+measurement is not reported if the digest of an accessed file is found in a
+digest list, the measurement list does not show which files have been
+actually accessed, and in which sequence.
+
+A possible solution would be to load a list with digest of files which are
+usually accessed. Also, it is possible to selectively enable digest list
+lookup only for a subset of IMA policy rules. For example, a policy could
+enable digest lookup only for file accesses from the TCB and disable it
+for execve() and mmap() from regular users.
+
+
+
+SETUP
+=====
+
+Digest lists should be placed in the /etc/ima/digest_lists directory and
+metadata should be written to /etc/ima/digest_lists/metadata.
+
+If digest lists are included in the initial ram disk, IMA will load them
+early in the boot process. Otherwise, a patched systemd can check if the
+file with digest list metadata exists in the filesystem and, if yes, send
+the path to IMA through the 'digest_lists' securityfs interface. The main
+use case for the patched systemd is to load digest lists of newly installed
+packages, which are not included in the initial ram disk.
+
+
+
+FORMATS
+=======
+
+The format of digest list metadata is:
+
+algo[2]
+digest_len[4] digest[digest_len]
+signature_len[4] signature[signature_len]
+path_len[4] path[path_len]
+ref_id_len[4] ref_id[ref_id_len]
+list_type[2]
+
+algo and list_type are in little endian.
+
+algo values are defined in include/uapi/linux/hash_info.h. The algorithms
+in the list metadata must be the same of ima_hash_algo (algorithm used by
+IMA to calculate the file digest).
+
+list type values:
+
+0: compact digest list
+1: RPM package header
+
+
+The format of the compact digest list is:
+
+entry_id[2] count[4] data_len[4]
+data[data_len]
+[...]
+entry_id[2] count[4] data_len[4]
+data[data_len]
+
+entry_id, count and data_len are in little endian.
+
+entry_id can have values 0 or 1. If entry_id is 0, files with provided
+digests are immutable. If entry_id is 1, files are mutable. 'data' contains
+'count' digests concatenated together.
+
+For example, a compact digest list with 10 SHA256 digests will look like:
+
+0 10 320
+digest1..digest10
+
+
+
+MEASUREMENT LIST
+================
+
+If IMA loads the digest lists from the initial ram disk, the measurement
+list should look like:
+
+10 <template digest> ima-ng sha1:<digest> boot_aggregate
+10 <template digest> ima-ng sha1:<digest> /etc/ima/digest_lists/metadata
+
+For the integrity evaluation, metadata and digest lists must be provided to
+verifiers. The digest of digest lists must be compared with the digest
+included in the metadata, and the digest of metadata with the digest in the
+measurement list.
+
+
+
+APPRAISAL
+=========
+
+Appraisal verification consists on comparing the calculated digest of an
+accessed file with the value of the security.ima extended attribute. With
+digest lists, appraisal verification succeeds if the calculated digest is
+included in a list. Since the digital signature of each digest list is
+verified, it is not possible to allow access of unauthorized files.
+
+For mutable files, IMA writes the current digest to security.ima so that
+next file accesses are allowed even if the files have been modified. For
+immutable files, IMA writes security.ima only if also additional extended
+attributes should be protected by EVM. Otherwise, security.ima would be
+redundant, as digest lists provide reference values.
+
+When IMA writes security.ima, EVM calculates the HMAC based on the current
+value of protected extended attributes. Without file signatures, initial
+extended attribute values will not checked until digest lists include them.
+When extended attribute values are available, IMA will check them as the
+same as the digest, and will not write security.ima for immutable files if
+values are provided for all extended attributes protected by EVM.
-- 
2.11.0

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (14 preceding siblings ...)
  2017-11-07 10:37 ` [PATCH v2 15/15] ima: add Documentation/security/IMA-digest-lists.txt Roberto Sassu
@ 2017-11-07 13:37 ` Mimi Zohar
  2017-11-07 16:45   ` Roberto Sassu
  2017-11-07 14:49 ` Matthew Garrett
  2017-11-07 18:03 ` Safford, David (GE Global Research, US)
  17 siblings, 1 reply; 40+ messages in thread
From: Mimi Zohar @ 2017-11-07 13:37 UTC (permalink / raw)
  To: Roberto Sassu, linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu

Hi Roberto,

On Tue, 2017-11-07 at 11:36 +0100, Roberto Sassu wrote:
> IMA is a security module with the objective of reporting or enforcing the
> integrity of a system, by measuring files accessed with the execve(),
> mmap() and open() system calls. For reporting, it takes advantage of the
> TPM and extends a PCR with the digest of an evaluated event. For enforcing,
> it returns a value which is zero if the operation should be allowed,
> negative if it should be denied.
> 
> Measuring files of an operating system introduces three main issues. First,
> since the overhead introduced by the TPM is noticeable, the performance of
> the system decreases linearly with the number of measurements taken. This
> can be seen especially at boot time.

I've said this previously.  The solution IS FIRST to improve the
performance of the TPM device driver, before finding solutions around
it.

TPM performance patches:
a233a0289cf9 "tpm: msleep() delays - replace with usleep_range() in i2c nuvoton driver"
0afb7118ae02 "tpm: add sleep only for retry in i2c_nuvoton_write_status()"
9f3fc7bcddcb "tpm: replace msleep() with  usleep_range() in TPM 1.2/2.0 generic drivers"

Nayna Jain submitted additional performance improvements, that were posted
https://www.spinics.net/lists/linux-integrity/msg00238.html and are
currently being tested.

Even after these TPM performance improvements, there are still more
TPM performance improvements.

> Second, managing large measurement
> lists requires computation power and network bandwidth.

"Large" for whom?  Large for the attestation server?  Large for the
client?  Smaller devices would have fewer measurements than larger
devices.  We're not discussing gigabytes/terabytes of data here.
 Attestation servers should be able to handle the bandwidth.  If it
becomes a problem, then the attestation server/client communication
could be optimized to send just the recent measurements, not the
entire measurement list.

> Third, it is
> necessary to obtain reference measurements (i.e. digests of software known
> to be good) to evaluate/enforce the integrity of the system. If file
> signatures are used to enforce access, Linux distribution vendors have to
> modify their building systems in order to include signatures in their
> packages.

Although IMA-appraisal verifies file integrity based on either a file
hash or signature, they are not equivalent.  File signatures provide
file provenance.  Not only does the file hash have to match, but a
certificate used to sign the file data must be loaded onto the IMA
keyring.  File hashes should be limited to mutable files.

Instead of working around the problem of a lack of file signatures in
software packages, help promote including them so that there are
measurement and signature chains of trust anchored in hardware.

> Digest lists aim at mitigating these issues. A digest list is a list of
> digests that are taken by IMA as reference measurements and loaded before
> files are accessed. Then, IMA compares calculated digests of accessed files
> with digests from loaded digest lists. If the digest is found, measurement,
> appraisal and audit are not performed.

Although the previous patch set did not break userspace per-se, it
changed the existing meaning of the IMA measurement list.  Without
taking into account my previous comments, this patch set makes similar
changes to IMA-appraisal and IMA-measurement.

Instead of including the individual file measurements, the previous
version of this patch set, I assume it hasn't changed, includes a hash
of a file containing a list of all potential file measurements, not
the actual file measurements.

Instead of changing the meaning of the IMA measurement list, I
previously suggested defining a new securityfs file for this purpose.

> Multiple digest lists can be loaded at the same time, by providing to IMA
> metadata for each list: digest, signature and path. The digest is specified
> so that loaded digest lists can be identified only with the measurement of
> metadata. The signature is used for appraisal. If the verification
> succeeds, IMA loads the digest list even if security.ima is missing.

Previously IMA-appraisal verified the file signature of the file
containing the file hashes.  It now sounds like even this guarantee is
gone.

Normally, the protection of kernel memory is out of scope for IMA.
This patch set introduces an in kernel white list, which would be a
prime target for attackers looking for ways of by-passing IMA-
measurement, IMA-appraisal and IMA-audit.  Others might disagree, but
from my perspective, this risk is too high.

Mimi

> Digest lists address the first issue because the TPM is used only if the
> digest of a measured file is unknown. On a minimal system, 10 of 1400
> measurements are unknown because of mutable files (e.g. log files).
> 
> Digest lists mitigate the second issue because, since digest lists do not
> change, they don't have to be sent at every remote attestation. Sending
> unknown measurements and a reference to digest lists would be sufficient.
> 
> Finally, digest lists address also the third issue because Linux
> distribution vendors already provide the digests of files included in each
> RPM package. The digest list is stored in the RPM header, signed by the
> vendor.
> 
> When using digest lists, a limitation must be considered. Since a
> measurement is not reported if the digest of an accessed file is found in a
> digest list, the measurement list does not show which files have been
> actually accessed, and in which sequence.
> 
> A possible solution would be to load a list with digest of files which are
> usually accessed. Also, it is possible to selectively enable digest list
> lookup only for a subset of IMA policy rules. For example, a policy could
> enable digest lookup only for file accesses from the TCB and disable it
> for execve() and mmap() from regular users.
> 
> Changelog
> 
> v1:
> - added new policy option digest_list to selectively enable digest lookup
> - added support for appraisal
> - added support for immutable/mutable files
> 
> Roberto Sassu (15):
>   ima: generalize ima_read_policy()
>   ima: generalize ima_write_policy()
>   ima: generalize policy file operations
>   ima: use ima_show_htable_value to show hash table data
>   ima: add functions to manage digest lists
>   ima: add parser of digest lists metadata
>   ima: add parser of compact digest list
>   ima: add parser of RPM package headers
>   ima: introduce securityfs interfaces for digest lists
>   ima: disable digest lookup if digest lists are not checked
>   ima: add policy action digest_list
>   ima: do not update security.ima if appraisal status is not
>     INTEGRITY_PASS
>   evm: add kernel command line option to select protected xattrs
>   ima: add support for appraisal with digest lists
>   ima: add Documentation/security/IMA-digest-lists.txt
> 
>  Documentation/admin-guide/kernel-parameters.txt |   4 +
>  Documentation/security/IMA-digest-lists.txt     | 161 ++++++++++++
>  include/linux/evm.h                             |   6 +
>  include/linux/fs.h                              |   2 +
>  security/integrity/evm/evm_main.c               |  36 +++
>  security/integrity/iint.c                       |   1 +
>  security/integrity/ima/Kconfig                  |  19 ++
>  security/integrity/ima/Makefile                 |   1 +
>  security/integrity/ima/ima.h                    |  33 ++-
>  security/integrity/ima/ima_api.c                |   7 +-
>  security/integrity/ima/ima_appraise.c           |  52 +++-
>  security/integrity/ima/ima_digest_list.c        | 326 ++++++++++++++++++++++++
>  security/integrity/ima/ima_fs.c                 | 181 ++++++++-----
>  security/integrity/ima/ima_main.c               |  47 +++-
>  security/integrity/ima/ima_policy.c             |  33 ++-
>  security/integrity/ima/ima_queue.c              |  42 +++
>  security/integrity/integrity.h                  |  11 +
>  17 files changed, 877 insertions(+), 85 deletions(-)
>  create mode 100644 Documentation/security/IMA-digest-lists.txt
>  create mode 100644 security/integrity/ima/ima_digest_list.c
> 

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (15 preceding siblings ...)
  2017-11-07 13:37 ` [PATCH v2 00/15] ima: digest list feature Mimi Zohar
@ 2017-11-07 14:49 ` Matthew Garrett
  2017-11-07 17:53   ` Roberto Sassu
  2017-11-07 18:03 ` Safford, David (GE Global Research, US)
  17 siblings, 1 reply; 40+ messages in thread
From: Matthew Garrett @ 2017-11-07 14:49 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Tue, Nov 7, 2017 at 2:36 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
> Finally, digest lists address also the third issue because Linux
> distribution vendors already provide the digests of files included in each
> RPM package. The digest list is stored in the RPM header, signed by the
> vendor.

RPM's hardly universal, and distributions are in the process of moving
away from using it for distributing non-core applications (Flatpak and
Snap are becoming increasingly popular here). I think this needs to be
a generic solution rather than having the kernel tied to a specific
package format.

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 13:37 ` [PATCH v2 00/15] ima: digest list feature Mimi Zohar
@ 2017-11-07 16:45   ` Roberto Sassu
  2017-11-17  1:08     ` Kees Cook
  0 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 16:45 UTC (permalink / raw)
  To: Mimi Zohar, linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu

On 11/7/2017 2:37 PM, Mimi Zohar wrote:
> Hi Roberto,
> 
> On Tue, 2017-11-07 at 11:36 +0100, Roberto Sassu wrote:
>> IMA is a security module with the objective of reporting or enforcing the
>> integrity of a system, by measuring files accessed with the execve(),
>> mmap() and open() system calls. For reporting, it takes advantage of the
>> TPM and extends a PCR with the digest of an evaluated event. For enforcing,
>> it returns a value which is zero if the operation should be allowed,
>> negative if it should be denied.
>>
>> Measuring files of an operating system introduces three main issues. First,
>> since the overhead introduced by the TPM is noticeable, the performance of
>> the system decreases linearly with the number of measurements taken. This
>> can be seen especially at boot time.
> 
> I've said this previously.  The solution IS FIRST to improve the
> performance of the TPM device driver, before finding solutions around
> it.
> 
> TPM performance patches:
> a233a0289cf9 "tpm: msleep() delays - replace with usleep_range() in i2c nuvoton driver"
> 0afb7118ae02 "tpm: add sleep only for retry in i2c_nuvoton_write_status()"
> 9f3fc7bcddcb "tpm: replace msleep() with  usleep_range() in TPM 1.2/2.0 generic drivers"
> 
> Nayna Jain submitted additional performance improvements, that were posted
> https://www.spinics.net/lists/linux-integrity/msg00238.html and are
> currently being tested.
> 
> Even after these TPM performance improvements, there are still more
> TPM performance improvements.

Hi Mimi

I applied the patches you mentioned, and indeed the performance
improvement is great, from 1 minute and 30 seconds to 24 seconds
compared to the normal boot time of 8.5 seconds. However, especially for
embedded devices performances requirements might be more strict, and
much more files might be measured. For desktops, also it would be
desirable to have low latency.


>> Second, managing large measurement
>> lists requires computation power and network bandwidth.
> 
> "Large" for whom?  Large for the attestation server?  Large for the
> client?  Smaller devices would have fewer measurements than larger
> devices.  We're not discussing gigabytes/terabytes of data here.
>   Attestation servers should be able to handle the bandwidth.  If it
> becomes a problem, then the attestation server/client communication
> could be optimized to send just the recent measurements, not the
> entire measurement list.

I'm not considering an individual system, but a datacenter with several
nodes. An attestation server processing for example 200 measurement
lists with 5000 entries must be much more powerful than one that
processes the same number of lists with 10 entries.


>> Third, it is
>> necessary to obtain reference measurements (i.e. digests of software known
>> to be good) to evaluate/enforce the integrity of the system. If file
>> signatures are used to enforce access, Linux distribution vendors have to
>> modify their building systems in order to include signatures in their
>> packages.
> 
> Although IMA-appraisal verifies file integrity based on either a file
> hash or signature, they are not equivalent.  File signatures provide
> file provenance.  Not only does the file hash have to match, but a
> certificate used to sign the file data must be loaded onto the IMA
> keyring.  File hashes should be limited to mutable files.

Digest lists work the same. If appraisal is in enforcing mode, digest
lists must have a valid digital signature.


> Instead of working around the problem of a lack of file signatures in
> software packages, help promote including them so that there are
> measurement and signature chains of trust anchored in hardware.

One of the key point of digest lists feature is that it reuses
information that is already available, while providing the same security
properties. I find it difficult to promote a solution that would
introduce redundant information and complicate the management of Linux
distributions.


>> Digest lists aim at mitigating these issues. A digest list is a list of
>> digests that are taken by IMA as reference measurements and loaded before
>> files are accessed. Then, IMA compares calculated digests of accessed files
>> with digests from loaded digest lists. If the digest is found, measurement,
>> appraisal and audit are not performed.
> 
> Although the previous patch set did not break userspace per-se, it
> changed the existing meaning of the IMA measurement list.  Without
> taking into account my previous comments, this patch set makes similar
> changes to IMA-appraisal and IMA-measurement.

For appraisal, I think only the verification method changes: instead of
verifying file signatures individually, IMA verifies one signature per
many file digests.


> Instead of including the individual file measurements, the previous
> version of this patch set, I assume it hasn't changed, includes a hash
> of a file containing a list of all potential file measurements, not
> the actual file measurements.

Digest lists do not introduce false positives due to not including a
measurement. Files whose digest is included in a digest list must be
considered as accessed. This can be improved later, for example by
introducing a new digest list type contaning digests of files that must
be accessed. IMA would create a new measurement entry when the last file
is accessed.


> Instead of changing the meaning of the IMA measurement list, I
> previously suggested defining a new securityfs file for this purpose.

The meaning of the measurement list is determined by the policy, which
is measured. As the same, the meaning of the measurement list can be
determined from the digest of digest list metadata. Either a verifier
detects that digest lists have been loaded (if he supports them), or
must consider the system as compromised, as the digest of digest list
metadata is unknown (he does not support digest lists).


>> Multiple digest lists can be loaded at the same time, by providing to IMA
>> metadata for each list: digest, signature and path. The digest is specified
>> so that loaded digest lists can be identified only with the measurement of
>> metadata. The signature is used for appraisal. If the verification
>> succeeds, IMA loads the digest list even if security.ima is missing.
> 
> Previously IMA-appraisal verified the file signature of the file
> containing the file hashes.  It now sounds like even this guarantee is
> gone.

IMA obtains reference values from digest lists instead of security.ima.
This is comparable to file signatures, because digest lists must be
signed for appraisal.


> Normally, the protection of kernel memory is out of scope for IMA.
> This patch set introduces an in kernel white list, which would be a
> prime target for attackers looking for ways of by-passing IMA-
> measurement, IMA-appraisal and IMA-audit.  Others might disagree, but
> from my perspective, this risk is too high.

It would be much easier for an attacker to just set ima_policy_flag to
zero.

Roberto


>> Digest lists address the first issue because the TPM is used only if the
>> digest of a measured file is unknown. On a minimal system, 10 of 1400
>> measurements are unknown because of mutable files (e.g. log files).
>>
>> Digest lists mitigate the second issue because, since digest lists do not
>> change, they don't have to be sent at every remote attestation. Sending
>> unknown measurements and a reference to digest lists would be sufficient.
>>
>> Finally, digest lists address also the third issue because Linux
>> distribution vendors already provide the digests of files included in each
>> RPM package. The digest list is stored in the RPM header, signed by the
>> vendor.
>>
>> When using digest lists, a limitation must be considered. Since a
>> measurement is not reported if the digest of an accessed file is found in a
>> digest list, the measurement list does not show which files have been
>> actually accessed, and in which sequence.
>>
>> A possible solution would be to load a list with digest of files which are
>> usually accessed. Also, it is possible to selectively enable digest list
>> lookup only for a subset of IMA policy rules. For example, a policy could
>> enable digest lookup only for file accesses from the TCB and disable it
>> for execve() and mmap() from regular users.
>>
>> Changelog
>>
>> v1:
>> - added new policy option digest_list to selectively enable digest lookup
>> - added support for appraisal
>> - added support for immutable/mutable files
>>
>> Roberto Sassu (15):
>>    ima: generalize ima_read_policy()
>>    ima: generalize ima_write_policy()
>>    ima: generalize policy file operations
>>    ima: use ima_show_htable_value to show hash table data
>>    ima: add functions to manage digest lists
>>    ima: add parser of digest lists metadata
>>    ima: add parser of compact digest list
>>    ima: add parser of RPM package headers
>>    ima: introduce securityfs interfaces for digest lists
>>    ima: disable digest lookup if digest lists are not checked
>>    ima: add policy action digest_list
>>    ima: do not update security.ima if appraisal status is not
>>      INTEGRITY_PASS
>>    evm: add kernel command line option to select protected xattrs
>>    ima: add support for appraisal with digest lists
>>    ima: add Documentation/security/IMA-digest-lists.txt
>>
>>   Documentation/admin-guide/kernel-parameters.txt |   4 +
>>   Documentation/security/IMA-digest-lists.txt     | 161 ++++++++++++
>>   include/linux/evm.h                             |   6 +
>>   include/linux/fs.h                              |   2 +
>>   security/integrity/evm/evm_main.c               |  36 +++
>>   security/integrity/iint.c                       |   1 +
>>   security/integrity/ima/Kconfig                  |  19 ++
>>   security/integrity/ima/Makefile                 |   1 +
>>   security/integrity/ima/ima.h                    |  33 ++-
>>   security/integrity/ima/ima_api.c                |   7 +-
>>   security/integrity/ima/ima_appraise.c           |  52 +++-
>>   security/integrity/ima/ima_digest_list.c        | 326 ++++++++++++++++++++++++
>>   security/integrity/ima/ima_fs.c                 | 181 ++++++++-----
>>   security/integrity/ima/ima_main.c               |  47 +++-
>>   security/integrity/ima/ima_policy.c             |  33 ++-
>>   security/integrity/ima/ima_queue.c              |  42 +++
>>   security/integrity/integrity.h                  |  11 +
>>   17 files changed, 877 insertions(+), 85 deletions(-)
>>   create mode 100644 Documentation/security/IMA-digest-lists.txt
>>   create mode 100644 security/integrity/ima/ima_digest_list.c
>>
> 

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 14:49 ` Matthew Garrett
@ 2017-11-07 17:53   ` Roberto Sassu
  2017-11-07 18:06     ` Matthew Garrett
  0 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-07 17:53 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On 11/7/2017 3:49 PM, Matthew Garrett wrote:
> On Tue, Nov 7, 2017 at 2:36 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>> Finally, digest lists address also the third issue because Linux
>> distribution vendors already provide the digests of files included in each
>> RPM package. The digest list is stored in the RPM header, signed by the
>> vendor.
> 
> RPM's hardly universal, and distributions are in the process of moving
> away from using it for distributing non-core applications (Flatpak and
> Snap are becoming increasingly popular here). I think this needs to be
> a generic solution rather than having the kernel tied to a specific
> package format.

Support for new digest list formats can be easily added. Digest list
metadata includes the digest list type, so that the appropriate parser
is selected.

I defined a new generic format for digest lists in Patch 7/15. I would
appreciate if we can discuss this format, and if you can give me
suggestions about how to improve it. I think it would not be a problem
to support your use case and associate metadata to each digest.

Digest lists should be parsed directly by the kernel, because processing
the lists in userspace would increase the chances that a compromised
tool does not upload to the kernel the expected digests. Also, digest
lists must be processed before init, otherwise appraisal will deny the
execution. Lastly, the mechanism of parsing files from the kernel is
already used to parse the IMA policy.

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* RE: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
                   ` (16 preceding siblings ...)
  2017-11-07 14:49 ` Matthew Garrett
@ 2017-11-07 18:03 ` Safford, David (GE Global Research, US)
  2017-11-08 10:16   ` Roberto Sassu
  17 siblings, 1 reply; 40+ messages in thread
From: Safford, David (GE Global Research, US) @ 2017-11-07 18:03 UTC (permalink / raw)
  To: Roberto Sassu, linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu

> -----Original Message-----
> From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
> owner@vger.kernel.org] On Behalf Of Roberto Sassu
> Sent: Tuesday, November 07, 2017 5:37 AM
> To: linux-integrity@vger.kernel.org
> Cc: linux-security-module@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
> silviu.vlasceanu@huawei.com; Roberto Sassu <roberto.sassu@huawei.com>
> Subject: EXT: [PATCH v2 00/15] ima: digest list feature
> 
> IMA is a security module with the objective of reporting or enforcing the
> integrity of a system, by measuring files accessed with the execve(),
> mmap() and open() system calls. For reporting, it takes advantage of the
> TPM and extends a PCR with the digest of an evaluated event. For enforcing,
> it returns a value which is zero if the operation should be allowed, negative if
> it should be denied.
> 
> Measuring files of an operating system introduces three main issues. First,
> since the overhead introduced by the TPM is noticeable, the performance of
> the system decreases linearly with the number of measurements taken. 
> This can be seen especially at boot time.

If you want the measurement chain of trust, every link must be extended in the TPM.
This is inherent in the model. Doing local verification of TCB files is really no substitute.
Not to mention that leaving out "known" hashes from attestation eliminates the 
ability to do analytics on the patterns of usage of the good files. Local appraisal is a 
good thing, but not a complete substitute for remote attestation.

> Second, managing large measurement
> lists requires computation power and network bandwidth. 

So 200 nodes with 5000 entries, 100bytes per entry average (that's a pretty large TCB, but OK):
that's roughly .8 seconds total on a single Gb link.

> Third, it is
> necessary to obtain reference measurements (i.e. digests of software
> known to be good) to evaluate/enforce the integrity of the system. If file
> signatures are used to enforce access, Linux distribution vendors have to
> modify their building systems in order to include signatures in their packages.

Or you can use the initial enrollment to transfer a reference manifest.
Or you can use SWIDS. Or you can sign everything yourself. (That's what we do.)

> Digest lists aim at mitigating these issues. A digest list is a list of digests that
> are taken by IMA as reference measurements and loaded before files are
> accessed. Then, IMA compares calculated digests of accessed files with
> digests from loaded digest lists. If the digest is found, measurement,
> appraisal and audit are not performed.

So who manages the "good" hash lists? They have to go into the initramfs,
and be updated with every package update. And Leaving out attestation of 
good TCB files reduces the potential power of analytics.

> Multiple digest lists can be loaded at the same time, by providing to IMA
> metadata for each list: digest, signature and path. The digest is specified so
> that loaded digest lists can be identified only with the measurement of
> metadata. The signature is used for appraisal. If the verification succeeds,
> IMA loads the digest list even if security.ima is missing.
> 
> Digest lists address the first issue because the TPM is used only if the digest
> of a measured file is unknown. On a minimal system, 10 of 1400
> measurements are unknown because of mutable files (e.g. log files).

At 5ms per extend, you at most save 7 seconds at boot. But the savings are
actually much less, as the extends run simultaneously with most of the
other boot operations. I typically can't tell the difference without a 
stopwatch.

> Digest lists mitigate the second issue because, since digest lists do not
> change, they don't have to be sent at every remote attestation. Sending
> unknown measurements and a reference to digest lists would be sufficient.

The .8 second isn't a problem, and even that can be pretty much eliminated by
sending just the delta measurements.

> Finally, digest lists address also the third issue because Linux distribution
> vendors already provide the digests of files included in each RPM package.
> The digest list is stored in the RPM header, signed by the vendor.

But then tooling is needed to select the desired hashes and put them in
the initramfs for loading.

I guess I don't see the problem, and think the cure introduces issues of its own.

dave

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 17:53   ` Roberto Sassu
@ 2017-11-07 18:06     ` Matthew Garrett
  2017-11-08 12:00       ` Roberto Sassu
  0 siblings, 1 reply; 40+ messages in thread
From: Matthew Garrett @ 2017-11-07 18:06 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Tue, Nov 7, 2017 at 12:53 PM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
> On 11/7/2017 3:49 PM, Matthew Garrett wrote:
>> RPM's hardly universal, and distributions are in the process of moving
>> away from using it for distributing non-core applications (Flatpak and
>> Snap are becoming increasingly popular here). I think this needs to be
>> a generic solution rather than having the kernel tied to a specific
>> package format.
>
>
> Support for new digest list formats can be easily added. Digest list
> metadata includes the digest list type, so that the appropriate parser
> is selected.

But we're still left in a state where the kernel has to end up
supporting a number of very niche formats, and userland agility is
tied to the kernel. I think it makes significantly more sense to push
the problem out to userland.

> Digest lists should be parsed directly by the kernel, because processing
> the lists in userspace would increase the chances that a compromised
> tool does not upload to the kernel the expected digests. Also, digest
> lists must be processed before init, otherwise appraisal will deny the
> execution. Lastly, the mechanism of parsing files from the kernel is
> already used to parse the IMA policy.

Isn't failing to upload the expected digest list just a DoS? We
already expect to load keys from initramfs, so it seems fine to parse
stuff there - what's the problem with extracting information from
RPMs, translating them to the generic format and pushing that into the
kernel?

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 18:03 ` Safford, David (GE Global Research, US)
@ 2017-11-08 10:16   ` Roberto Sassu
  0 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-08 10:16 UTC (permalink / raw)
  To: Safford, David (GE Global Research, US), linux-integrity
  Cc: linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu

On 11/7/2017 7:03 PM, Safford, David (GE Global Research, US) wrote:
>> -----Original Message-----
>> From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity-
>> owner@vger.kernel.org] On Behalf Of Roberto Sassu
>> Sent: Tuesday, November 07, 2017 5:37 AM
>> To: linux-integrity@vger.kernel.org
>> Cc: linux-security-module@vger.kernel.org; linux-fsdevel@vger.kernel.org;
>> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>> silviu.vlasceanu@huawei.com; Roberto Sassu <roberto.sassu@huawei.com>
>> Subject: EXT: [PATCH v2 00/15] ima: digest list feature
>>
>> IMA is a security module with the objective of reporting or enforcing the
>> integrity of a system, by measuring files accessed with the execve(),
>> mmap() and open() system calls. For reporting, it takes advantage of the
>> TPM and extends a PCR with the digest of an evaluated event. For enforcing,
>> it returns a value which is zero if the operation should be allowed, negative if
>> it should be denied.
>>
>> Measuring files of an operating system introduces three main issues. First,
>> since the overhead introduced by the TPM is noticeable, the performance of
>> the system decreases linearly with the number of measurements taken.
>> This can be seen especially at boot time.
> 
> If you want the measurement chain of trust, every link must be extended in the TPM.
> This is inherent in the model. Doing local verification of TCB files is really no substitute.

Digest list measurement is extended in the TPM. If not, digest lookup
is disabled until reboot. The same happens for appraisal: if the digital
signature of digest lists is invalid, digest list cannot be used for
local verification. This check is done in patch 10/15.

The Trusted Computing paradigm measure and load is respected: either the
digest list has been measured and local verification can be done, or
individual digests are reported.

I would compare the digest list measurement as the same as the
measurement of a security policy containing allowed permissions.
Trusting the measurement of the policy and the system means trusting the
system to enforce the security policy.


> Not to mention that leaving out "known" hashes from attestation eliminates the
> ability to do analytics on the patterns of usage of the good files. Local appraisal is a
> good thing, but not a complete substitute for remote attestation.

Patch 11/15 gives the possibility to selectively enable digest list
lookup for specific rules. This means that, for example, digest lookup
can be enabled for PCR 10, so that its value is predictable and can be
used for sealing policies, and disabled for a different PCR, which can
be used to report more information.


>> Second, managing large measurement
>> lists requires computation power and network bandwidth.
> 
> So 200 nodes with 5000 entries, 100bytes per entry average (that's a pretty large TCB, but OK):
> that's roughly .8 seconds total on a single Gb link.

I was also considering the overhead due to using the TCG format (XML).


>> Third, it is
>> necessary to obtain reference measurements (i.e. digests of software
>> known to be good) to evaluate/enforce the integrity of the system. If file
>> signatures are used to enforce access, Linux distribution vendors have to
>> modify their building systems in order to include signatures in their packages.
> 
> Or you can use the initial enrollment to transfer a reference manifest.
> Or you can use SWIDS. Or you can sign everything yourself. (That's what we do.)

The first two suggestions look similar to what I'm proposing, having a
list of digests signed by the vendor.


>> Digest lists aim at mitigating these issues. A digest list is a list of digests that
>> are taken by IMA as reference measurements and loaded before files are
>> accessed. Then, IMA compares calculated digests of accessed files with
>> digests from loaded digest lists. If the digest is found, measurement,
>> appraisal and audit are not performed.
> 
> So who manages the "good" hash lists? They have to go into the initramfs,
> and be updated with every package update. And Leaving out attestation of
> good TCB files reduces the potential power of analytics.

The initial ram disk should contain at least (for appraisal) the digests
of files that will be accessed before the root filesystem is mounted.
Then, the remaining digest lists can be loaded by systemd, for example.

Previously, I mentioned the possibility of using different PCRs for a
more complete report. But, if a sealing policy for PCR 10 should provide
more guarantees than just which set of files can be possibly accessed
(for example, which subset of files have been actually accessed), this
can be achieved by extending the TPM with the digest of a policy to 
check that condition, and creating a measurement entry when the
condition becomes true. This will keep the PCR 10 value predictable.

I have an use case for the predictable state: I want to seal the EVM key
to PCR 10, use the key to protect mutable files and enforce the Biba 
strict policy in order to allow writes to mutable files only by the TCB.
Then, if the EVM key is securely generated (sensitiveDataOrigin is set),
and the system proves during remote attestation that the expected key
has been loaded and PCR values match those used to seal the key, the
measurement of mutable files can be avoided.


>> Multiple digest lists can be loaded at the same time, by providing to IMA
>> metadata for each list: digest, signature and path. The digest is specified so
>> that loaded digest lists can be identified only with the measurement of
>> metadata. The signature is used for appraisal. If the verification succeeds,
>> IMA loads the digest list even if security.ima is missing.
>>
>> Digest lists address the first issue because the TPM is used only if the digest
>> of a measured file is unknown. On a minimal system, 10 of 1400
>> measurements are unknown because of mutable files (e.g. log files).
> 
> At 5ms per extend, you at most save 7 seconds at boot. But the savings are
> actually much less, as the extends run simultaneously with most of the
> other boot operations. I typically can't tell the difference without a
> stopwatch.

I performed some tests on a HP EliteDesk 800 G2. The boot time without
an IMA policy is 8.5 seconds. With the IMA policy and without the
TPM performance patches is 1 minute and 30 seconds. With the TPM
performance patches is 24 seconds. 1400 measurements are recorded.

Roberto


>> Digest lists mitigate the second issue because, since digest lists do not
>> change, they don't have to be sent at every remote attestation. Sending
>> unknown measurements and a reference to digest lists would be sufficient.
> 
> The .8 second isn't a problem, and even that can be pretty much eliminated by
> sending just the delta measurements.
> 
>> Finally, digest lists address also the third issue because Linux distribution
>> vendors already provide the digests of files included in each RPM package.
>> The digest list is stored in the RPM header, signed by the vendor.
> 
> But then tooling is needed to select the desired hashes and put them in
> the initramfs for loading.
> 
> I guess I don't see the problem, and think the cure introduces issues of its own.
> 
> dave
> 

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 18:06     ` Matthew Garrett
@ 2017-11-08 12:00       ` Roberto Sassu
  2017-11-08 15:48         ` Matthew Garrett
  0 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-08 12:00 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On 11/7/2017 7:06 PM, Matthew Garrett wrote:
> On Tue, Nov 7, 2017 at 12:53 PM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>> On 11/7/2017 3:49 PM, Matthew Garrett wrote:
>>> RPM's hardly universal, and distributions are in the process of moving
>>> away from using it for distributing non-core applications (Flatpak and
>>> Snap are becoming increasingly popular here). I think this needs to be
>>> a generic solution rather than having the kernel tied to a specific
>>> package format.
>>
>>
>> Support for new digest list formats can be easily added. Digest list
>> metadata includes the digest list type, so that the appropriate parser
>> is selected.
> 
> But we're still left in a state where the kernel has to end up
> supporting a number of very niche formats, and userland agility is
> tied to the kernel. I think it makes significantly more sense to push
> the problem out to userland.

At least for appraisal, digest lists must be parsed by the kernel. If
the parser is moved to userspace, I don't know if we are able to provide
the same guarantee, that the correct set of digests has been uploaded to
IMA. A new measurement can be added, when IMA receives the digests, but
a verifier has to verify the signature of the original file, perform
format conversion, calculate the digest and compare it with that in the
new IMA measurement. If digest lists are parsed directly by the kernel,
then the signature can be verified directly.


>> Digest lists should be parsed directly by the kernel, because processing
>> the lists in userspace would increase the chances that a compromised
>> tool does not upload to the kernel the expected digests. Also, digest
>> lists must be processed before init, otherwise appraisal will deny the
>> execution. Lastly, the mechanism of parsing files from the kernel is
>> already used to parse the IMA policy.
> 
> Isn't failing to upload the expected digest list just a DoS? We
> already expect to load keys from initramfs, so it seems fine to parse
> stuff there - what's the problem with extracting information from
> RPMs, translating them to the generic format and pushing that into the
> kernel?

The main problem is that the digest list measurement, performed when the
parser accesses the file containing the RPM header, might not reflect
what IMA uses for digest lookup.

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-08 12:00       ` Roberto Sassu
@ 2017-11-08 15:48         ` Matthew Garrett
  2017-11-09  9:51           ` Roberto Sassu
  0 siblings, 1 reply; 40+ messages in thread
From: Matthew Garrett @ 2017-11-08 15:48 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Wed, Nov 8, 2017 at 7:00 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
> On 11/7/2017 7:06 PM, Matthew Garrett wrote:
>> But we're still left in a state where the kernel has to end up
>> supporting a number of very niche formats, and userland agility is
>> tied to the kernel. I think it makes significantly more sense to push
>> the problem out to userland.
>
>
> At least for appraisal, digest lists must be parsed by the kernel. If
> the parser is moved to userspace, I don't know if we are able to provide
> the same guarantee, that the correct set of digests has been uploaded to
> IMA. A new measurement can be added, when IMA receives the digests, but
> a verifier has to verify the signature of the original file, perform
> format conversion, calculate the digest and compare it with that in the
> new IMA measurement. If digest lists are parsed directly by the kernel,
> then the signature can be verified directly.

The code doing the parsing is in the initramfs, which has already been
measured at boot time. You can guarantee that it's being done by
trusted code.

>> Isn't failing to upload the expected digest list just a DoS? We
>> already expect to load keys from initramfs, so it seems fine to parse
>> stuff there - what's the problem with extracting information from
>> RPMs, translating them to the generic format and pushing that into the
>> kernel?
>
>
> The main problem is that the digest list measurement, performed when the
> parser accesses the file containing the RPM header, might not reflect
> what IMA uses for digest lookup.

Why not?

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-08 15:48         ` Matthew Garrett
@ 2017-11-09  9:51           ` Roberto Sassu
  2017-11-09 14:47             ` Matthew Garrett
  0 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-09  9:51 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On 11/8/2017 4:48 PM, Matthew Garrett wrote:
> On Wed, Nov 8, 2017 at 7:00 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>> On 11/7/2017 7:06 PM, Matthew Garrett wrote:
>>> But we're still left in a state where the kernel has to end up
>>> supporting a number of very niche formats, and userland agility is
>>> tied to the kernel. I think it makes significantly more sense to push
>>> the problem out to userland.
>>
>>
>> At least for appraisal, digest lists must be parsed by the kernel. If
>> the parser is moved to userspace, I don't know if we are able to provide
>> the same guarantee, that the correct set of digests has been uploaded to
>> IMA. A new measurement can be added, when IMA receives the digests, but
>> a verifier has to verify the signature of the original file, perform
>> format conversion, calculate the digest and compare it with that in the
>> new IMA measurement. If digest lists are parsed directly by the kernel,
>> then the signature can be verified directly.
> 
> The code doing the parsing is in the initramfs, which has already been
> measured at boot time. You can guarantee that it's being done by
> trusted code.

The parser can be executed in the initial ram disk, but everything
accessed before the parser is executed will be measured/appraised
without digest lists. To do signature-based remote attestation, where
the verification consists on checking the signature of digests of
measured files, it would be necessary to sign systemd, libraries,
everything accessed before the parser, and the parser. If RPM headers
are parsed by the kernel, measurement/appraisal will be done directly
with digest lists.


>>> Isn't failing to upload the expected digest list just a DoS? We
>>> already expect to load keys from initramfs, so it seems fine to parse
>>> stuff there - what's the problem with extracting information from
>>> RPMs, translating them to the generic format and pushing that into the
>>> kernel?
>>
>>
>> The main problem is that the digest list measurement, performed when the
>> parser accesses the file containing the RPM header, might not reflect
>> what IMA uses for digest lookup.
> 
> Why not?

I assumed you wanted to measure digest lists only at the time they are
read by the parser, and not when they are read by IMA. If instead digest
lists are verified again after conversion, the new workflow should be:

1) the kernel parses digest list metadata before systemd is executed
2) the kernel verifies the signature of digest lists (RPM headers) and
    add the digest of digest lists to the hash table, so that appraisal
    succeeds
3) systemd (with file signature) is executed
4) the parser (with file signature) is executed
5) the parser reads and converts the digest lists to the generic format,
    and writes them to a tmpfs filesystem
6) the parser generates a new digest list metadata file with the path of
    converted digest lists and sends the path of the new metadata to IMA
7) IMA reads the generic digest lists

The measurement list should look like:

10 <digest> ima-sig <digest> boot_aggregate
10 <digest> ima-sig <digest> /etc/ima/digest_lists/metadata
10 <digest> ima-sig <digest> /usr/lib/systemd/systemd <signature>
...
10 <digest> ima-sig <digest> <parser> <signature>
10 <digest> ima-sig <digest> /tmp/metadata


If parsing of RPM headers is done by the kernel, the measurement list
will look like:

10 <digest> ima-ng <digest> boot_aggregate
10 <digest> ima-ng <digest> /etc/ima/digest_lists/metadata


A built-in policy should enable appraisal of tmpfs. If not, patch 11/15
disables digest lookup for appraisal. Since generic digest lists will
have a security.ima extended attribute (they are mutable files),
appraisal verification will succeed.

With this solution, digital signatures cannot be required, because
generic digest lists will have a HMAC. For appraisal, it becomes
necessary to ensure that only digest lists written by the parser can be
processed by IMA.

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-09  9:51           ` Roberto Sassu
@ 2017-11-09 14:47             ` Matthew Garrett
  2017-11-09 16:13               ` Roberto Sassu
  2017-11-09 16:17               ` Mimi Zohar
  0 siblings, 2 replies; 40+ messages in thread
From: Matthew Garrett @ 2017-11-09 14:47 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Thu, Nov 9, 2017 at 4:51 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
> On 11/8/2017 4:48 PM, Matthew Garrett wrote:
>> The code doing the parsing is in the initramfs, which has already been
>> measured at boot time. You can guarantee that it's being done by
>> trusted code.
>
>
> The parser can be executed in the initial ram disk, but everything
> accessed before the parser is executed will be measured/appraised
> without digest lists. To do signature-based remote attestation, where
> the verification consists on checking the signature of digests of
> measured files, it would be necessary to sign systemd, libraries,
> everything accessed before the parser, and the parser. If RPM headers
> are parsed by the kernel, measurement/appraisal will be done directly
> with digest lists.

There's no need to have a policy that measures those files, because
they're part of the already-measured initramfs. Just set the IMA
policy after you've loaded the digest list.

>>> The main problem is that the digest list measurement, performed when the
>>> parser accesses the file containing the RPM header, might not reflect
>>> what IMA uses for digest lookup.
>>
>>
>> Why not?
>
>
> I assumed you wanted to measure digest lists only at the time they are
> read by the parser, and not when they are read by IMA. If instead digest
> lists are verified again after conversion, the new workflow should be:
>
> 1) the kernel parses digest list metadata before systemd is executed
> 2) the kernel verifies the signature of digest lists (RPM headers) and
>    add the digest of digest lists to the hash table, so that appraisal
>    succeeds
> 3) systemd (with file signature) is executed
> 4) the parser (with file signature) is executed
> 5) the parser reads and converts the digest lists to the generic format,
>    and writes them to a tmpfs filesystem
> 6) the parser generates a new digest list metadata file with the path of
>    converted digest lists and sends the path of the new metadata to IMA
> 7) IMA reads the generic digest lists
>
> The measurement list should look like:
>
> 10 <digest> ima-sig <digest> boot_aggregate
> 10 <digest> ima-sig <digest> /etc/ima/digest_lists/metadata
> 10 <digest> ima-sig <digest> /usr/lib/systemd/systemd <signature>
> ...
> 10 <digest> ima-sig <digest> <parser> <signature>
> 10 <digest> ima-sig <digest> /tmp/metadata
>
>
> If parsing of RPM headers is done by the kernel, the measurement list
> will look like:
>
> 10 <digest> ima-ng <digest> boot_aggregate
> 10 <digest> ima-ng <digest> /etc/ima/digest_lists/metadata
>
>
> A built-in policy should enable appraisal of tmpfs. If not, patch 11/15
> disables digest lookup for appraisal. Since generic digest lists will
> have a security.ima extended attribute (they are mutable files),
> appraisal verification will succeed.
>
> With this solution, digital signatures cannot be required, because
> generic digest lists will have a HMAC. For appraisal, it becomes
> necessary to ensure that only digest lists written by the parser can be
> processed by IMA.

This seems very over-complicated, and it's unclear why the kernel
needs to open the file itself. You *know* that all of userland is
trustworthy at this point even in the absence of signatures. It seems
reasonable to provide a interface that allows userland to pass a
digest list to the kernel, in the same way that userland can pass an
IMA policy to the kernel. You can then restrict access to that
interface via an LSM.

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-09 14:47             ` Matthew Garrett
@ 2017-11-09 16:13               ` Roberto Sassu
  2017-11-09 16:46                 ` Matthew Garrett
  2017-11-09 16:17               ` Mimi Zohar
  1 sibling, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-09 16:13 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On 11/9/2017 3:47 PM, Matthew Garrett wrote:
> On Thu, Nov 9, 2017 at 4:51 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>> On 11/8/2017 4:48 PM, Matthew Garrett wrote:
>>> The code doing the parsing is in the initramfs, which has already been
>>> measured at boot time. You can guarantee that it's being done by
>>> trusted code.
>>
>>
>> The parser can be executed in the initial ram disk, but everything
>> accessed before the parser is executed will be measured/appraised
>> without digest lists. To do signature-based remote attestation, where
>> the verification consists on checking the signature of digests of
>> measured files, it would be necessary to sign systemd, libraries,
>> everything accessed before the parser, and the parser. If RPM headers
>> are parsed by the kernel, measurement/appraisal will be done directly
>> with digest lists.
> 
> There's no need to have a policy that measures those files, because
> they're part of the already-measured initramfs. Just set the IMA
> policy after you've loaded the digest list.

The default IMA policy measures files accessed from the initial ram
disk. It is easier to verify individual files, rather than the whole
image.


>>>> The main problem is that the digest list measurement, performed when the
>>>> parser accesses the file containing the RPM header, might not reflect
>>>> what IMA uses for digest lookup.
>>>
>>>
>>> Why not?
>>
>>
>> I assumed you wanted to measure digest lists only at the time they are
>> read by the parser, and not when they are read by IMA. If instead digest
>> lists are verified again after conversion, the new workflow should be:
>>
>> 1) the kernel parses digest list metadata before systemd is executed
>> 2) the kernel verifies the signature of digest lists (RPM headers) and
>>     add the digest of digest lists to the hash table, so that appraisal
>>     succeeds
>> 3) systemd (with file signature) is executed
>> 4) the parser (with file signature) is executed
>> 5) the parser reads and converts the digest lists to the generic format,
>>     and writes them to a tmpfs filesystem
>> 6) the parser generates a new digest list metadata file with the path of
>>     converted digest lists and sends the path of the new metadata to IMA
>> 7) IMA reads the generic digest lists
>>
>> The measurement list should look like:
>>
>> 10 <digest> ima-sig <digest> boot_aggregate
>> 10 <digest> ima-sig <digest> /etc/ima/digest_lists/metadata
>> 10 <digest> ima-sig <digest> /usr/lib/systemd/systemd <signature>
>> ...
>> 10 <digest> ima-sig <digest> <parser> <signature>
>> 10 <digest> ima-sig <digest> /tmp/metadata
>>
>>
>> If parsing of RPM headers is done by the kernel, the measurement list
>> will look like:
>>
>> 10 <digest> ima-ng <digest> boot_aggregate
>> 10 <digest> ima-ng <digest> /etc/ima/digest_lists/metadata
>>
>>
>> A built-in policy should enable appraisal of tmpfs. If not, patch 11/15
>> disables digest lookup for appraisal. Since generic digest lists will
>> have a security.ima extended attribute (they are mutable files),
>> appraisal verification will succeed.
>>
>> With this solution, digital signatures cannot be required, because
>> generic digest lists will have a HMAC. For appraisal, it becomes
>> necessary to ensure that only digest lists written by the parser can be
>> processed by IMA.
> 
> This seems very over-complicated, and it's unclear why the kernel
> needs to open the file itself. You *know* that all of userland is

You can have a look at ima_fs.c. If appraisal is in enforcing mode,
direct upload of a policy is not permitted. The kernel reads the policy,
calculates the digest, and verifies the signature.


> trustworthy at this point even in the absence of signatures. It seem > reasonable to provide a interface that allows userland to pass a
> digest list to the kernel, in the same way that userland can pass an
> IMA policy to the kernel. You can then restrict access to that
> interface via an LSM.

Then digest lists cannot be used alone, without an LSM. Also, verifiers
have to check the LSM policy to ensure that only the parser was able to
upload the digest lists.

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-09 14:47             ` Matthew Garrett
  2017-11-09 16:13               ` Roberto Sassu
@ 2017-11-09 16:17               ` Mimi Zohar
  1 sibling, 0 replies; 40+ messages in thread
From: Mimi Zohar @ 2017-11-09 16:17 UTC (permalink / raw)
  To: Matthew Garrett, Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Thu, 2017-11-09 at 09:47 -0500, Matthew Garrett wrote:

> This seems very over-complicated, and it's unclear why the kernel
> needs to open the file itself. You *know* that all of userland is
> trustworthy at this point even in the absence of signatures.

Assuming the initramfs is signed, then yes the rootfs files would be
trusted.  rootfs can still access files from real root, which is where
policies are normally stored.

> It seems
> reasonable to provide a interface that allows userland to pass a
> digest list to the kernel, in the same way that userland can pass an
> IMA policy to the kernel. You can then restrict access to that
> interface via an LSM.

IMA can and should be configured to require signed policies.

Mimi

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-09 16:13               ` Roberto Sassu
@ 2017-11-09 16:46                 ` Matthew Garrett
  2017-11-09 17:23                   ` Roberto Sassu
  0 siblings, 1 reply; 40+ messages in thread
From: Matthew Garrett @ 2017-11-09 16:46 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Thu, Nov 9, 2017 at 11:13 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
> On 11/9/2017 3:47 PM, Matthew Garrett wrote:
>> There's no need to have a policy that measures those files, because
>> they're part of the already-measured initramfs. Just set the IMA
>> policy after you've loaded the digest list.
>
>
> The default IMA policy measures files accessed from the initial ram
> disk. It is easier to verify individual files, rather than the whole
> image.

That's a matter of implementation. You're not forced to use the default policy.

>> This seems very over-complicated, and it's unclear why the kernel
>> needs to open the file itself. You *know* that all of userland is
>
>
> You can have a look at ima_fs.c. If appraisal is in enforcing mode,
> direct upload of a policy is not permitted. The kernel reads the policy,
> calculates the digest, and verifies the signature.

Is there an expectation that you'll load additional digest lists at runtime?

>> trustworthy at this point even in the absence of signatures. It seem >
>> reasonable to provide a interface that allows userland to pass a
>> digest list to the kernel, in the same way that userland can pass an
>> IMA policy to the kernel. You can then restrict access to that
>> interface via an LSM.
>
>
> Then digest lists cannot be used alone, without an LSM. Also, verifiers
> have to check the LSM policy to ensure that only the parser was able to
> upload the digest lists.

Only if you want to add additional digest lists at runtime, but yes,
you really want to be verifying the LSM policy in any case.

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-09 16:46                 ` Matthew Garrett
@ 2017-11-09 17:23                   ` Roberto Sassu
  0 siblings, 0 replies; 40+ messages in thread
From: Roberto Sassu @ 2017-11-09 17:23 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On 11/9/2017 5:46 PM, Matthew Garrett wrote:
> On Thu, Nov 9, 2017 at 11:13 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>> On 11/9/2017 3:47 PM, Matthew Garrett wrote:
>>> There's no need to have a policy that measures those files, because
>>> they're part of the already-measured initramfs. Just set the IMA
>>> policy after you've loaded the digest list.
>>
>>
>> The default IMA policy measures files accessed from the initial ram
>> disk. It is easier to verify individual files, rather than the whole
>> image.
> 
> That's a matter of implementation. You're not forced to use the default policy.
> 
>>> This seems very over-complicated, and it's unclear why the kernel
>>> needs to open the file itself. You *know* that all of userland is
>>
>>
>> You can have a look at ima_fs.c. If appraisal is in enforcing mode,
>> direct upload of a policy is not permitted. The kernel reads the policy,
>> calculates the digest, and verifies the signature.
> 
> Is there an expectation that you'll load additional digest lists at runtime?

Yes, before new packages are installed.


>>> trustworthy at this point even in the absence of signatures. It seem >
>>> reasonable to provide a interface that allows userland to pass a
>>> digest list to the kernel, in the same way that userland can pass an
>>> IMA policy to the kernel. You can then restrict access to that
>>> interface via an LSM.
>>
>>
>> Then digest lists cannot be used alone, without an LSM. Also, verifiers
>> have to check the LSM policy to ensure that only the parser was able to
>> upload the digest lists.
> 
> Only if you want to add additional digest lists at runtime, but yes,
> you really want to be verifying the LSM policy in any case.

If the trustworthiness of the initial ram disk can be guaranteed with
low effort, for example if the image is distributed by the vendor, it
would be easier to guarantee that digest lists were uploaded by the
parser.

But, likely, the initial ram disk is generated by the local system.
Relying on the assumption that the vendor distributes the initial ram
disk would limit, in my opinion, the applicability of the digest list
feature. Relying on the enforcement by an LSM would make the integrity
evaluation much more complex.

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-07 16:45   ` Roberto Sassu
@ 2017-11-17  1:08     ` Kees Cook
  2017-11-17  8:55       ` Roberto Sassu
  0 siblings, 1 reply; 40+ messages in thread
From: Kees Cook @ 2017-11-17  1:08 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Mimi Zohar, linux-integrity, linux-security-module,
	linux-fsdevel, linux-doc, LKML, silviu.vlasceanu

On Tue, Nov 7, 2017 at 8:45 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
> On 11/7/2017 2:37 PM, Mimi Zohar wrote:
>> Normally, the protection of kernel memory is out of scope for IMA.
>> This patch set introduces an in kernel white list, which would be a
>> prime target for attackers looking for ways of by-passing IMA-
>> measurement, IMA-appraisal and IMA-audit.  Others might disagree, but
>> from my perspective, this risk is too high.

BTW, which part of the series does the whitelist? I'd agree generally,
though: we don't want to make things writable if they're normally
read-only.

> It would be much easier for an attacker to just set ima_policy_flag to
> zero.

That's a fair point. I wonder if ima_policy_flag could be marked
__ro_after_init? Most of the writes are from __init sections, but I
haven't looked closely at when ima_update_policy() gets called.

-Kees

-- 
Kees Cook
Pixel Security

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-17  1:08     ` Kees Cook
@ 2017-11-17  8:55       ` Roberto Sassu
  2017-11-17 12:21         ` Mimi Zohar
  0 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-17  8:55 UTC (permalink / raw)
  To: Kees Cook
  Cc: Mimi Zohar, linux-integrity, linux-security-module,
	linux-fsdevel, linux-doc, LKML, silviu.vlasceanu

On 11/17/2017 2:08 AM, Kees Cook wrote:
> On Tue, Nov 7, 2017 at 8:45 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>> On 11/7/2017 2:37 PM, Mimi Zohar wrote:
>>> Normally, the protection of kernel memory is out of scope for IMA.
>>> This patch set introduces an in kernel white list, which would be a
>>> prime target for attackers looking for ways of by-passing IMA-
>>> measurement, IMA-appraisal and IMA-audit.  Others might disagree, but
>>> from my perspective, this risk is too high.
> 
> BTW, which part of the series does the whitelist? I'd agree generally,
> though: we don't want to make things writable if they're normally
> read-only.

Patch 5/15 introduces the hash table ima_digests_htable and the
functions to add/search file digests

Patches 6-7-8/15 add file digests to ima_digests_htable

Patch 10/15 searches file digests in ima_digests_htable


Original files containing digest lists are discarded after being parsed.


>> It would be much easier for an attacker to just set ima_policy_flag to
>> zero.
> 
> That's a fair point. I wonder if ima_policy_flag could be marked
> __ro_after_init? Most of the writes are from __init sections, but I
> haven't looked closely at when ima_update_policy() gets called.

Unfortunately not. New policies can be loaded by writing to a file in
the securityfs filesystem. They could enable different actions
(measurement/appraisal/audit).

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 00/15] ima: digest list feature
  2017-11-17  8:55       ` Roberto Sassu
@ 2017-11-17 12:21         ` Mimi Zohar
  0 siblings, 0 replies; 40+ messages in thread
From: Mimi Zohar @ 2017-11-17 12:21 UTC (permalink / raw)
  To: Roberto Sassu, Kees Cook
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	LKML, silviu.vlasceanu

On Fri, 2017-11-17 at 09:55 +0100, Roberto Sassu wrote:
> On 11/17/2017 2:08 AM, Kees Cook wrote:
> > On Tue, Nov 7, 2017 at 8:45 AM, Roberto Sassu <roberto.sassu@huawei.com> wrote:
> >> On 11/7/2017 2:37 PM, Mimi Zohar wrote:
> >>> Normally, the protection of kernel memory is out of scope for IMA.
> >>> This patch set introduces an in kernel white list, which would be a
> >>> prime target for attackers looking for ways of by-passing IMA-
> >>> measurement, IMA-appraisal and IMA-audit.  Others might disagree, but
> >>> from my perspective, this risk is too high.
> > 
> > BTW, which part of the series does the whitelist? I'd agree generally,
> > though: we don't want to make things writable if they're normally
> > read-only.

The white list is a proposed new feature.

> Patch 5/15 introduces the hash table ima_digests_htable and the
> functions to add/search file digests
> 
> Patches 6-7-8/15 add file digests to ima_digests_htable
> 
> Patch 10/15 searches file digests in ima_digests_htable
> 
> 
> Original files containing digest lists are discarded after being parsed.
> 
> 
> >> It would be much easier for an attacker to just set ima_policy_flag to
> >> zero.
> > 
> > That's a fair point. I wonder if ima_policy_flag could be marked
> > __ro_after_init? Most of the writes are from __init sections, but I
> > haven't looked closely at when ima_update_policy() gets called.
> 
> Unfortunately not. New policies can be loaded by writing to a file in
> the securityfs filesystem. They could enable different actions
> (measurement/appraisal/audit).

The ima_policy_flag is an optimization indicating which actions -
MEASURE, APPRAISE, AUDIT - the policy contains.  The IMA policy,
itself, can be replaced with a signed custom policy just once.  This
is normally done in the initramfs, after the LSM policies have been
loaded, in order to define policy rules in terms of LSM labels.  Once
the new policy is loaded, the ima_policy_flag is set.

A Kconfig option allows additional signed rules to be added to the IMA
policy.  After adding these new rules, additional actions can be added
to the policy flag, but not cleared.

The system admin/owner knows, prior to loading the custom policy,
which actions will be defined.  Instead of waiting for the policy to
be written, the ima_policy_flag could be set at init.  (We could
extend the existing "ima_policy=" boot command line option.)  If not
the ima_policy_flag, itself, then a shadow of the ima_policy_flag,
which is OR'ed with the ima_policy_flag.

Mimi

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

* Re: [PATCH v2 06/15] ima: add parser of digest lists metadata
  2017-11-07 10:37 ` [PATCH v2 06/15] ima: add parser of digest lists metadata Roberto Sassu
@ 2017-11-18  4:20   ` Serge E. Hallyn
  2017-11-18 23:23     ` Mimi Zohar
  0 siblings, 1 reply; 40+ messages in thread
From: Serge E. Hallyn @ 2017-11-18  4:20 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Tue, Nov 07, 2017 at 11:37:01AM +0100, Roberto Sassu wrote:
> from a predefined position (/etc/ima/digest_lists/metadata), when rootfs
> becomes available. Digest lists must be loaded before IMA appraisal is in
> enforcing mode.

I'm sure there's a good reason for it, but this seems weird to me.
Why read it from a file on disk instead of accepting it through say
a securityfile write?

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

* Re: [PATCH v2 12/15] ima: do not update security.ima if appraisal status is not INTEGRITY_PASS
  2017-11-07 10:37 ` [PATCH v2 12/15] ima: do not update security.ima if appraisal status is not INTEGRITY_PASS Roberto Sassu
@ 2017-11-18  4:25   ` Serge E. Hallyn
  0 siblings, 0 replies; 40+ messages in thread
From: Serge E. Hallyn @ 2017-11-18  4:25 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Tue, Nov 07, 2017 at 11:37:07AM +0100, Roberto Sassu wrote:
> Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters")
> moved the call of ima_file_check() from may_open() to do_filp_open() at a
> point where the file descriptor is already opened.
> 
> This breaks the assumption made by IMA that file descriptors being closed
> belong to files whose access was granted by ima_file_check(). The
> consequence is that security.ima and security.evm are updated with good
> values, regardless of the current appraisal status.
> 
> For example, if a file does not have security.ima, IMA will create it after
> opening the file for writing, even if access is denied. Access to the file
> will be allowed afterwards.
> 
> Avoid this issue by checking the appraisal status before updating
> security.ima.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

IIUC this seems like a huge deal.  Shouldn't this go in separately, asap?

> ---
>  security/integrity/ima/ima_appraise.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 285a53452fb5..1b2236e637ff 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -320,6 +320,9 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
>  	if (iint->flags & IMA_DIGSIG)
>  		return;
>  
> +	if (iint->ima_file_status != INTEGRITY_PASS)
> +		return;
> +
>  	rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo);
>  	if (rc < 0)
>  		return;
> -- 
> 2.11.0

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

* Re: [PATCH v2 06/15] ima: add parser of digest lists metadata
  2017-11-18  4:20   ` Serge E. Hallyn
@ 2017-11-18 23:23     ` Mimi Zohar
  2017-11-20  9:40       ` Roberto Sassu
  0 siblings, 1 reply; 40+ messages in thread
From: Mimi Zohar @ 2017-11-18 23:23 UTC (permalink / raw)
  To: Serge E. Hallyn, Roberto Sassu
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

Hi Serge,

On Fri, 2017-11-17 at 22:20 -0600, Serge E. Hallyn wrote:
> On Tue, Nov 07, 2017 at 11:37:01AM +0100, Roberto Sassu wrote:
> > from a predefined position (/etc/ima/digest_lists/metadata), when rootfs
> > becomes available. Digest lists must be loaded before IMA appraisal is in
> > enforcing mode.
> 
> I'm sure there's a good reason for it, but this seems weird to me.
> Why read it from a file on disk instead of accepting it through say
> a securityfile write?

Assuming that the concept of a white list is something we want to
support, then at minimum the list needs to be signed and verified.
Instead of defining a new Kconfig pathname option, a securityfs file
could read it, like the IMA policy.

Mimi

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

* Re: [PATCH v2 06/15] ima: add parser of digest lists metadata
  2017-11-18 23:23     ` Mimi Zohar
@ 2017-11-20  9:40       ` Roberto Sassu
  2017-11-20 13:53         ` Mimi Zohar
  0 siblings, 1 reply; 40+ messages in thread
From: Roberto Sassu @ 2017-11-20  9:40 UTC (permalink / raw)
  To: Mimi Zohar, Serge E. Hallyn
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On 11/19/2017 12:23 AM, Mimi Zohar wrote:
> Hi Serge,
> 
> On Fri, 2017-11-17 at 22:20 -0600, Serge E. Hallyn wrote:
>> On Tue, Nov 07, 2017 at 11:37:01AM +0100, Roberto Sassu wrote:
>>> from a predefined position (/etc/ima/digest_lists/metadata), when rootfs
>>> becomes available. Digest lists must be loaded before IMA appraisal is in
>>> enforcing mode.
>>
>> I'm sure there's a good reason for it, but this seems weird to me.
>> Why read it from a file on disk instead of accepting it through say
>> a securityfile write?

There are two reasons.

Digest lists must be loaded before any file is accessed, otherwise IMA
will deny the operation if appraisal is in enforcing mode. With digest
lists it is possible to appraise files in the initial ram disk without
including extended attributes (the default policy excludes those files).

The second reason is that appraisal has to be temporarily disabled
because the file containing digest list metadata is not signed. The same
happens when loading a public key (check ima_load_x509() in ima_init.c).

The file containing digest list metadata is not signed because its
content depends on the list of installed packages. I thought it is
acceptable to load it without verification, as providing the path of
digest lists is similar to writing the path of a policy to a securityfs
file. The important point is that no digest is added to the hash table
without verifying the signature first.

The alternative would be to load signed digest lists directly. But, the
main issue is that there would be a PCR extend for each digest list,
while with digest list metadata there is only one.


> Assuming that the concept of a white list is something we want to
> support, then at minimum the list needs to be signed and verified.
> Instead of defining a new Kconfig pathname option, a securityfs file
> could read it, like the IMA policy.

Both methods are supported (patch 9/15 introduces 'digest_lists' in the
securityfs filesystem). The securityfs file can be used to load digest
lists for files not in the initial ram disk, and for system updates.

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

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

* Re: [PATCH v2 06/15] ima: add parser of digest lists metadata
  2017-11-20  9:40       ` Roberto Sassu
@ 2017-11-20 13:53         ` Mimi Zohar
  2017-11-20 16:52           ` Serge E. Hallyn
  0 siblings, 1 reply; 40+ messages in thread
From: Mimi Zohar @ 2017-11-20 13:53 UTC (permalink / raw)
  To: Roberto Sassu, Serge E. Hallyn
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu

On Mon, 2017-11-20 at 10:40 +0100, Roberto Sassu wrote:
> On 11/19/2017 12:23 AM, Mimi Zohar wrote:
> > Hi Serge,
> > 
> > On Fri, 2017-11-17 at 22:20 -0600, Serge E. Hallyn wrote:
> >> On Tue, Nov 07, 2017 at 11:37:01AM +0100, Roberto Sassu wrote:
> >>> from a predefined position (/etc/ima/digest_lists/metadata), when rootfs
> >>> becomes available. Digest lists must be loaded before IMA appraisal is in
> >>> enforcing mode.
> >>
> >> I'm sure there's a good reason for it, but this seems weird to me.
> >> Why read it from a file on disk instead of accepting it through say
> >> a securityfile write?
> 
> There are two reasons.
> 
> Digest lists must be loaded before any file is accessed, otherwise IMA
> will deny the operation if appraisal is in enforcing mode. With digest
> lists it is possible to appraise files in the initial ram disk without
> including extended attributes (the default policy excludes those files).

There are a number of people interested in extending CPIO to support
extended attributes, not just for IMA-appraisal. (Years ago I started
but unfortunately haven't had time to finish it.)  Isn't the right
solution to add extended attribute support to CPIO?

> The second reason is that appraisal has to be temporarily disabled
> because the file containing digest list metadata is not signed. The same
> happens when loading a public key (check ima_load_x509() in ima_init.c).

In terms of the x509 certificate, in order to validate the file
containing the x509 certificate, there needs to be a key on the IMA
keyring.

Since x509 certificates themselves are signed by a key on the builtin
keyring, it is safe to load them on the IMA keyring without first
validating the file signature.

Once a public key is loaded on the IMA keyring, all other file
signatures can be appraised.  There's no need for treating the digest
list file any differently than other files, as there is for the x509
certificate.

Mimi

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

* Re: [PATCH v2 06/15] ima: add parser of digest lists metadata
  2017-11-20 13:53         ` Mimi Zohar
@ 2017-11-20 16:52           ` Serge E. Hallyn
  0 siblings, 0 replies; 40+ messages in thread
From: Serge E. Hallyn @ 2017-11-20 16:52 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Roberto Sassu, Serge E. Hallyn, linux-integrity,
	linux-security-module, linux-fsdevel, linux-doc, linux-kernel,
	silviu.vlasceanu

Quoting Mimi Zohar (zohar@linux.vnet.ibm.com):
> On Mon, 2017-11-20 at 10:40 +0100, Roberto Sassu wrote:
> > On 11/19/2017 12:23 AM, Mimi Zohar wrote:
> > > Hi Serge,
> > > 
> > > On Fri, 2017-11-17 at 22:20 -0600, Serge E. Hallyn wrote:
> > >> On Tue, Nov 07, 2017 at 11:37:01AM +0100, Roberto Sassu wrote:
> > >>> from a predefined position (/etc/ima/digest_lists/metadata), when rootfs
> > >>> becomes available. Digest lists must be loaded before IMA appraisal is in
> > >>> enforcing mode.
> > >>
> > >> I'm sure there's a good reason for it, but this seems weird to me.
> > >> Why read it from a file on disk instead of accepting it through say
> > >> a securityfile write?
> > 
> > There are two reasons.
> > 
> > Digest lists must be loaded before any file is accessed, otherwise IMA
> > will deny the operation if appraisal is in enforcing mode. With digest
> > lists it is possible to appraise files in the initial ram disk without
> > including extended attributes (the default policy excludes those files).
> 
> There are a number of people interested in extending CPIO to support
> extended attributes, not just for IMA-appraisal. (Years ago I started
> but unfortunately haven't had time to finish it.)  Isn't the right
> solution to add extended attribute support to CPIO?

(For the record) Yes, yes it is. 

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

end of thread, other threads:[~2017-11-20 16:52 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 10:36 [PATCH v2 00/15] ima: digest list feature Roberto Sassu
2017-11-07 10:36 ` [PATCH v2 01/15] ima: generalize ima_read_policy() Roberto Sassu
2017-11-07 10:36 ` [PATCH v2 02/15] ima: generalize ima_write_policy() Roberto Sassu
2017-11-07 10:36 ` [PATCH v2 03/15] ima: generalize policy file operations Roberto Sassu
2017-11-07 10:36 ` [PATCH v2 04/15] ima: use ima_show_htable_value to show hash table data Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 05/15] ima: add functions to manage digest lists Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 06/15] ima: add parser of digest lists metadata Roberto Sassu
2017-11-18  4:20   ` Serge E. Hallyn
2017-11-18 23:23     ` Mimi Zohar
2017-11-20  9:40       ` Roberto Sassu
2017-11-20 13:53         ` Mimi Zohar
2017-11-20 16:52           ` Serge E. Hallyn
2017-11-07 10:37 ` [PATCH v2 07/15] ima: add parser of compact digest list Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 08/15] ima: add parser of RPM package headers Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 09/15] ima: introduce securityfs interfaces for digest lists Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 10/15] ima: disable digest lookup if digest lists are not checked Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 11/15] ima: add policy action digest_list Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 12/15] ima: do not update security.ima if appraisal status is not INTEGRITY_PASS Roberto Sassu
2017-11-18  4:25   ` Serge E. Hallyn
2017-11-07 10:37 ` [PATCH v2 13/15] evm: add kernel command line option to select protected xattrs Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 14/15] ima: add support for appraisal with digest lists Roberto Sassu
2017-11-07 10:37 ` [PATCH v2 15/15] ima: add Documentation/security/IMA-digest-lists.txt Roberto Sassu
2017-11-07 13:37 ` [PATCH v2 00/15] ima: digest list feature Mimi Zohar
2017-11-07 16:45   ` Roberto Sassu
2017-11-17  1:08     ` Kees Cook
2017-11-17  8:55       ` Roberto Sassu
2017-11-17 12:21         ` Mimi Zohar
2017-11-07 14:49 ` Matthew Garrett
2017-11-07 17:53   ` Roberto Sassu
2017-11-07 18:06     ` Matthew Garrett
2017-11-08 12:00       ` Roberto Sassu
2017-11-08 15:48         ` Matthew Garrett
2017-11-09  9:51           ` Roberto Sassu
2017-11-09 14:47             ` Matthew Garrett
2017-11-09 16:13               ` Roberto Sassu
2017-11-09 16:46                 ` Matthew Garrett
2017-11-09 17:23                   ` Roberto Sassu
2017-11-09 16:17               ` Mimi Zohar
2017-11-07 18:03 ` Safford, David (GE Global Research, US)
2017-11-08 10:16   ` 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).