[RFC PATCH v2 00/11] vfss: support for a common kernel file loader

STALE3857d

50 messages, 8 authors, 2016-01-27 · open the first message on its own page

[RFC PATCH v2 00/11] vfss: support for a common kernel file loader

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:24:42

For a while it was looked down upon to directly read files from Linux.
These days there exists a few mechanisms in the kernel that do just this
though to load a file into a local buffer. There are minor but important
checks differences on each, we should take all the best practices from
each of them, generalize them and make all places in the kernel that
read a file use it.[1]

One difference is the method for opening the file.  In some cases we
have a file, while in other cases we have a pathname or a file descriptor.

Another difference is the security hook calls, or lack of them.  In
some versions there is a post file read hook, while in others there
is a pre file read hook.

This patch set is the first attempt at resolving these differences.  It
does not attempt to merge the different methods of opening a file, but
defines a single common kernel file read function with two wrappers.
Although this patch set defines two new security hooks for pre and post
file read, it does not attempt to merge the existing security hooks.
That is left as future work.

Changelog v2:
- Combined the "ima: measuring/appraising files read by the kernel" patches
with this patch set to simplify review.
- Split the "ima: measure and appraise kexec image and initramfs" patch to
separate IMA from the kexec changes.

The latest version of these patches can be found in the next-kernel-read-v2
branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

[1] Taken from Luis Rodriguez's wiki -
http://kernelnewbies.org/KernelProjects/common-kernel-loader

Mimi

Dmitry Kasatkin (3):
  ima: separate 'security.ima' reading functionality from collect
  ima: provide buffer hash calculation function
  ima: load policy using path

Mimi Zohar (8):
  vfs: define a generic function to read a file from the kernel
  ima: calculate the hash of a buffer using aynchronous hash(ahash)
  ima: define a new hook to measure and appraise a file already in
    memory
  kexec: replace call to copy_file_from_fd() with kernel version
  firmware: replace call to fw_read_file_contents() with kernel version
  module: replace copy_module_from_fd with kernel version
  ima: measure and appraise the IMA policy itself
  ima: require signed IMA policy

 Documentation/ABI/testing/ima_policy      |   2 +-
 drivers/base/firmware_class.c             |  48 ++++--------
 fs/exec.c                                 |  93 +++++++++++++++++++++++
 include/linux/fs.h                        |   3 +
 include/linux/ima.h                       |  17 ++++-
 include/linux/lsm_hooks.h                 |  19 +++++
 include/linux/security.h                  |  14 ++--
 kernel/kexec_file.c                       |  72 ++----------------
 kernel/module.c                           |  67 ++--------------
 security/integrity/iint.c                 |   1 +
 security/integrity/ima/ima.h              |  35 +++++----
 security/integrity/ima/ima_api.c          |  19 ++---
 security/integrity/ima/ima_appraise.c     |  45 +++++------
 security/integrity/ima/ima_crypto.c       | 120 ++++++++++++++++++++++++++++-
 security/integrity/ima/ima_fs.c           |  50 +++++++++++-
 security/integrity/ima/ima_init.c         |   2 +-
 security/integrity/ima/ima_main.c         |  52 +++++++++----
 security/integrity/ima/ima_policy.c       | 122 +++++++++++++++++++-----------
 security/integrity/ima/ima_template.c     |   2 -
 security/integrity/ima/ima_template_lib.c |   1 -
 security/integrity/integrity.h            |  14 ++--
 security/security.c                       |  46 +++++++----
 22 files changed, 540 insertions(+), 304 deletions(-)

-- 
2.1.0

[RFC PATCH v2 04/11] ima: calculate the hash of a buffer using aynchronous hash(ahash)

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:22:00

Setting up ahash has some overhead.  Only use ahash to calculate the
hash of a buffer, if the buffer is larger than ima_ahash_minsize.

Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima_crypto.c | 75 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 8d86281..82c4d3c 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c

@@ -519,6 +519,63 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
 	return rc;
 }
 
+static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
+				  struct ima_digest_data *hash,
+				  struct crypto_ahash *tfm)
+{
+	struct ahash_request *req;
+	struct scatterlist sg;
+	struct ahash_completion res;
+	int rc, ahash_rc = 0;
+
+	hash->length = crypto_ahash_digestsize(tfm);
+
+	req = ahash_request_alloc(tfm, GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	init_completion(&res.completion);
+	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+				   CRYPTO_TFM_REQ_MAY_SLEEP,
+				   ahash_complete, &res);
+
+	rc = ahash_wait(crypto_ahash_init(req), &res);
+	if (rc)
+		goto out;
+
+	sg_init_one(&sg, buf, len);
+	ahash_request_set_crypt(req, &sg, NULL, len);
+
+	ahash_rc = crypto_ahash_update(req);
+
+	/* wait for the update request to complete */
+	rc = ahash_wait(ahash_rc, &res);
+	if (!rc) {
+		ahash_request_set_crypt(req, NULL, hash->digest, 0);
+		rc = ahash_wait(crypto_ahash_final(req), &res);
+	}
+out:
+	ahash_request_free(req);
+	return rc;
+}
+
+static int calc_buffer_ahash(const void *buf, loff_t len,
+			     struct ima_digest_data *hash)
+{
+	struct crypto_ahash *tfm;
+	int rc;
+
+	tfm = ima_alloc_atfm(hash->algo);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	rc = calc_buffer_ahash_atfm(buf, len, hash, tfm);
+
+	ima_free_atfm(tfm);
+
+	return rc;
+}
+
 static int calc_buffer_shash_tfm(const void *buf, loff_t size,
 				struct ima_digest_data *hash,
 				struct crypto_shash *tfm)

@@ -550,8 +607,8 @@ static int calc_buffer_shash_tfm(const void *buf, loff_t size,
 	return rc;
 }
 
-int ima_calc_buffer_hash(const void *buf, loff_t len,
-			 struct ima_digest_data *hash)
+static int calc_buffer_shash(const void *buf, loff_t len,
+			     struct ima_digest_data *hash)
 {
 	struct crypto_shash *tfm;
 	int rc;

@@ -566,6 +623,20 @@ int ima_calc_buffer_hash(const void *buf, loff_t len,
 	return rc;
 }
 
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+			 struct ima_digest_data *hash)
+{
+	int rc;
+
+	if (ima_ahash_minsize && len >= ima_ahash_minsize) {
+		rc = calc_buffer_ahash(buf, len, hash);
+		if (!rc)
+			return 0;
+	}
+
+	return calc_buffer_shash(buf, len, hash);
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
 	if (!ima_used_chip)

-- 
2.1.0

[RFC PATCH v2 03/11] ima: provide buffer hash calculation function

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:22:36

From: Dmitry Kasatkin <redacted>

This patch provides convenient buffer hash calculation function.

Changelog:
- rewrite to support loff_t sized buffers - Mimi
  (based on Fenguang Wu's testing)

Signed-off-by: Dmitry Kasatkin <redacted>
Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima.h        |  2 ++
 security/integrity/ima/ima_crypto.c | 47 +++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fb8da36..de53631 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -107,6 +107,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode,
 			   const unsigned char *filename);
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+			 struct ima_digest_data *hash);
 int ima_calc_field_array_hash(struct ima_field_data *field_data,
 			      struct ima_template_desc *desc, int num_fields,
 			      struct ima_digest_data *hash);

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index fb30ce4..8d86281 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c

@@ -519,6 +519,53 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
 	return rc;
 }
 
+static int calc_buffer_shash_tfm(const void *buf, loff_t size,
+				struct ima_digest_data *hash,
+				struct crypto_shash *tfm)
+{
+	SHASH_DESC_ON_STACK(shash, tfm);
+	unsigned int len;
+	loff_t offset = 0;
+	int rc;
+
+	shash->tfm = tfm;
+	shash->flags = 0;
+
+	hash->length = crypto_shash_digestsize(tfm);
+
+	rc = crypto_shash_init(shash);
+	if (rc != 0)
+		return rc;
+
+	len = size < PAGE_SIZE ? size : PAGE_SIZE;
+	while (offset < size) {
+		rc = crypto_shash_update(shash, buf + offset, len);
+		if (rc)
+			break;
+		offset += len;
+	}
+
+	if (!rc)
+		rc = crypto_shash_final(shash, hash->digest);
+	return rc;
+}
+
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+			 struct ima_digest_data *hash)
+{
+	struct crypto_shash *tfm;
+	int rc;
+
+	tfm = ima_alloc_tfm(hash->algo);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	rc = calc_buffer_shash_tfm(buf, len, hash, tfm);
+
+	ima_free_tfm(tfm);
+	return rc;
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
 	if (!ima_used_chip)

-- 
2.1.0

[RFC PATCH v2 01/11] ima: separate 'security.ima' reading functionality from collect

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:23:04

From: Dmitry Kasatkin <redacted>

Instead of passing pointers to pointers to ima_collect_measurent() to
read and return the 'security.ima' xattr value, this patch moves the
functionality to the calling process_measurement() to directly read
the xattr and pass only the hash algo to the ima_collect_measurement().

Signed-off-by: Dmitry Kasatkin <redacted>
Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima.h              | 15 +++++++--------
 security/integrity/ima/ima_api.c          | 15 +++------------
 security/integrity/ima/ima_appraise.c     | 25 ++++++++++++++-----------
 security/integrity/ima/ima_crypto.c       |  2 +-
 security/integrity/ima/ima_init.c         |  2 +-
 security/integrity/ima/ima_main.c         | 11 +++++++----
 security/integrity/ima/ima_template.c     |  2 --
 security/integrity/ima/ima_template_lib.c |  1 -
 8 files changed, 33 insertions(+), 40 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 585af61..fb8da36 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -23,6 +23,7 @@
 #include <linux/hash.h>
 #include <linux/tpm.h>
 #include <linux/audit.h>
+#include <crypto/hash_info.h>
 
 #include "../integrity.h"

@@ -140,9 +141,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 int ima_get_action(struct inode *inode, int mask, int function);
 int ima_must_measure(struct inode *inode, int mask, int function);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
-			    struct file *file,
-			    struct evm_ima_xattr_data **xattr_value,
-			    int *xattr_len);
+			    struct file *file, enum hash_algo algo);
 void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,

@@ -188,8 +187,8 @@ 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,
 					   int func);
-void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len,
-		       struct ima_digest_data *hash);
+enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
+				 int xattr_len);
 int ima_read_xattr(struct dentry *dentry,
 		   struct evm_ima_xattr_data **xattr_value);

@@ -221,10 +220,10 @@ static inline enum integrity_status ima_get_cache_status(struct integrity_iint_c
 	return INTEGRITY_UNKNOWN;
 }
 
-static inline void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
-				     int xattr_len,
-				     struct ima_digest_data *hash)
+static inline enum hash_algo
+ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
 {
+	return ima_hash_algo;
 }
 
 static inline int ima_read_xattr(struct dentry *dentry,

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 1d950fb..e7c7a5d 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c

@@ -18,7 +18,7 @@
 #include <linux/fs.h>
 #include <linux/xattr.h>
 #include <linux/evm.h>
-#include <crypto/hash_info.h>
+
 #include "ima.h"
 
 /*

@@ -188,9 +188,7 @@ int ima_get_action(struct inode *inode, int mask, int function)
  * Return 0 on success, error code otherwise
  */
 int ima_collect_measurement(struct integrity_iint_cache *iint,
-			    struct file *file,
-			    struct evm_ima_xattr_data **xattr_value,
-			    int *xattr_len)
+			    struct file *file, enum hash_algo algo)
 {
 	const char *audit_cause = "failed";
 	struct inode *inode = file_inode(file);

@@ -201,9 +199,6 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 		char digest[IMA_MAX_DIGEST_SIZE];
 	} hash;
 
-	if (xattr_value)
-		*xattr_len = ima_read_xattr(file->f_path.dentry, xattr_value);
-
 	if (!(iint->flags & IMA_COLLECTED)) {
 		u64 i_version = file_inode(file)->i_version;

@@ -213,11 +208,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 			goto out;
 		}
 
-		/* use default hash algorithm */
-		hash.hdr.algo = ima_hash_algo;
-
-		if (xattr_value)
-			ima_get_hash_algo(*xattr_value, *xattr_len, &hash.hdr);
+		hash.hdr.algo = algo;
 
 		result = ima_calc_file_hash(file, &hash.hdr);
 		if (!result) {

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 1873b55..9c2b46b 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -15,7 +15,6 @@
 #include <linux/magic.h>
 #include <linux/ima.h>
 #include <linux/evm.h>
-#include <crypto/hash_info.h>
 
 #include "ima.h"

@@ -130,36 +129,40 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
 	}
 }
 
-void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len,
-		       struct ima_digest_data *hash)
+enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
+				 int xattr_len)
 {
 	struct signature_v2_hdr *sig;
 
 	if (!xattr_value || xattr_len < 2)
-		return;
+		/* return default hash algo */
+		return ima_hash_algo;
 
 	switch (xattr_value->type) {
 	case EVM_IMA_XATTR_DIGSIG:
 		sig = (typeof(sig))xattr_value;
 		if (sig->version != 2 || xattr_len <= sizeof(*sig))
-			return;
-		hash->algo = sig->hash_algo;
+			return ima_hash_algo;
+		return sig->hash_algo;
 		break;
 	case IMA_XATTR_DIGEST_NG:
-		hash->algo = xattr_value->digest[0];
+		return xattr_value->digest[0];
 		break;
 	case IMA_XATTR_DIGEST:
 		/* this is for backward compatibility */
 		if (xattr_len == 21) {
 			unsigned int zero = 0;
 			if (!memcmp(&xattr_value->digest[16], &zero, 4))
-				hash->algo = HASH_ALGO_MD5;
+				return HASH_ALGO_MD5;
 			else
-				hash->algo = HASH_ALGO_SHA1;
+				return HASH_ALGO_SHA1;
 		} else if (xattr_len == 17)
-			hash->algo = HASH_ALGO_MD5;
+			return HASH_ALGO_MD5;
 		break;
 	}
+
+	/* return default hash algo */
+	return ima_hash_algo;
 }
 
 int ima_read_xattr(struct dentry *dentry,

@@ -296,7 +299,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
 	if (iint->flags & IMA_DIGSIG)
 		return;
 
-	rc = ima_collect_measurement(iint, file, NULL, NULL);
+	rc = ima_collect_measurement(iint, file, ima_hash_algo);
 	if (rc < 0)
 		return;

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 6eb6293..fb30ce4 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c

@@ -24,7 +24,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <crypto/hash.h>
-#include <crypto/hash_info.h>
+
 #include "ima.h"
 
 struct ahash_completion {

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index bd79f25..5d679a6 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c

@@ -21,7 +21,7 @@
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/err.h>
-#include <crypto/hash_info.h>
+
 #include "ima.h"
 
 /* name for boot aggregate entry */

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c21f09b..d9fc463 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -24,7 +24,6 @@
 #include <linux/slab.h>
 #include <linux/xattr.h>
 #include <linux/ima.h>
-#include <crypto/hash_info.h>
 
 #include "ima.h"

@@ -163,9 +162,10 @@ static int process_measurement(struct file *file, int mask, int function,
 	char *pathbuf = NULL;
 	const char *pathname = NULL;
 	int rc = -ENOMEM, action, must_appraise;
-	struct evm_ima_xattr_data *xattr_value = NULL, **xattr_ptr = NULL;
+	struct evm_ima_xattr_data *xattr_value = NULL;
 	int xattr_len = 0;
 	bool violation_check;
+	enum hash_algo hash_algo;
 
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;

@@ -221,9 +221,12 @@ static int process_measurement(struct file *file, int mask, int function,
 	template_desc = ima_template_desc_current();
 	if ((action & IMA_APPRAISE_SUBMASK) ||
 		    strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
-		xattr_ptr = &xattr_value;
+		/* read 'security.ima' */
+		xattr_len = ima_read_xattr(file->f_path.dentry, &xattr_value);
 
-	rc = ima_collect_measurement(iint, file, xattr_ptr, &xattr_len);
+	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
+
+	rc = ima_collect_measurement(iint, file, hash_algo);
 	if (rc != 0) {
 		if (file->f_flags & O_DIRECT)
 			rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 0b7404e..febd12e 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c

@@ -15,8 +15,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <crypto/hash_info.h>
-
 #include "ima.h"
 #include "ima_template_lib.h"

diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 2934e3d..f9bae04 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c

@@ -12,7 +12,6 @@
  * File: ima_template_lib.c
  *      Library of supported template fields.
  */
-#include <crypto/hash_info.h>
 
 #include "ima_template_lib.h"

-- 
2.1.0

[RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:25:06

Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function.  A benefit of calling kernel_read_file()
to read the firmware is the firmware is read only once, instead of once
for measuring/appraising the firmware and again for reading the file
contents into memory.

This patch retains the kernel_fw_from_file() hook, which is called from
security_kernel_post_read_file(), but removes the
sercurity_kernel_fw_from_file() function.

Changelog:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)

Signed-off-by: Mimi Zohar <redacted>
---
 drivers/base/firmware_class.c         | 48 ++++++++++-------------------------
 include/linux/ima.h                   |  7 +----
 include/linux/security.h              |  8 +-----
 security/integrity/ima/ima.h          |  1 -
 security/integrity/ima/ima_appraise.c |  8 ------
 security/integrity/ima/ima_main.c     | 18 +++++--------
 security/integrity/ima/ima_policy.c   | 26 +++++++++----------
 security/integrity/integrity.h        | 11 +++-----
 security/security.c                   | 28 ++++++++++----------
 9 files changed, 54 insertions(+), 101 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8524450..cc175f1 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c

@@ -29,6 +29,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/reboot.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 
 #include <generated/utsrelease.h>

@@ -291,40 +292,10 @@ static const char * const fw_path[] = {
 module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
 
-static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
-{
-	int size;
-	char *buf;
-	int rc;
-
-	if (!S_ISREG(file_inode(file)->i_mode))
-		return -EINVAL;
-	size = i_size_read(file_inode(file));
-	if (size <= 0)
-		return -EINVAL;
-	buf = vmalloc(size);
-	if (!buf)
-		return -ENOMEM;
-	rc = kernel_read(file, 0, buf, size);
-	if (rc != size) {
-		if (rc > 0)
-			rc = -EIO;
-		goto fail;
-	}
-	rc = security_kernel_fw_from_file(file, buf, size);
-	if (rc)
-		goto fail;
-	fw_buf->data = buf;
-	fw_buf->size = size;
-	return 0;
-fail:
-	vfree(buf);
-	return rc;
-}
-
 static int fw_get_filesystem_firmware(struct device *device,
 				       struct firmware_buf *buf)
 {
+	loff_t size;
 	int i, len;
 	int rc = -ENOENT;
 	char *path;

@@ -350,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
 		file = filp_open(path, O_RDONLY, 0);
 		if (IS_ERR(file))
 			continue;
-		rc = fw_read_file_contents(file, buf);
+
+		buf->size = 0;
+		rc = kernel_read_file(file, &buf->data, &size, INT_MAX,
+				      FIRMWARE_CHECK);
 		fput(file);
 		if (rc)
 			dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n",
 				path, rc);
-		else
+		else {
+			buf->size = (size_t) size;
 			break;
+		}
 	}
 	__putname(path);

@@ -685,8 +661,10 @@ static ssize_t firmware_loading_store(struct device *dev,
 				dev_err(dev, "%s: map pages failed\n",
 					__func__);
 			else
-				rc = security_kernel_fw_from_file(NULL,
-						fw_buf->data, fw_buf->size);
+				rc = security_kernel_post_read_file(NULL,
+							       fw_buf->data,
+							       fw_buf->size,
+							       FIRMWARE_CHECK);
 
 			/*
 			 * Same logic as fw_load_abort, only the DONE bit

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ae91938..0a7f039 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -16,6 +16,7 @@ struct linux_binprm;
 enum ima_policy_id {
 	KEXEC_CHECK = 1,
 	INITRAMFS_CHECK,
+	FIRMWARE_CHECK,
 	IMA_MAX_READ_CHECK
 };

@@ -25,7 +26,6 @@ extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_module_check(struct file *file);
-extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
 extern int ima_hash_and_process_file(struct file *file,
 				     void *buf, loff_t size,
 				     enum ima_policy_id policy_id);

@@ -56,11 +56,6 @@ static inline int ima_module_check(struct file *file)
 	return 0;
 }
 
-static inline int ima_fw_from_file(struct file *file, char *buf, size_t size)
-{
-	return 0;
-}
-
 static inline int ima_hash_and_process_file(struct file *file,
 					    void *buf, loff_t size,
 					    enum ima_policy_id policy_id)

diff --git a/include/linux/security.h b/include/linux/security.h
index 44d8832..51f3bc6 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -28,6 +28,7 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/mm.h>
+#include <linux/ima.h>
 
 struct linux_binprm;
 struct cred;

@@ -298,7 +299,6 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
 void security_transfer_creds(struct cred *new, const struct cred *old);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
-int security_kernel_fw_from_file(struct file *file, char *buf, size_t size);
 int security_kernel_module_request(char *kmod_name);
 int security_kernel_module_from_file(struct file *file);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,

@@ -852,12 +852,6 @@ static inline int security_kernel_create_files_as(struct cred *cred,
 	return 0;
 }
 
-static inline int security_kernel_fw_from_file(struct file *file,
-					       char *buf, size_t size)
-{
-	return 0;
-}
-
 static inline int security_kernel_module_request(char *kmod_name)
 {
 	return 0;

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b98dbd5..520c7b4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -165,7 +165,6 @@ enum ima_hooks {
 	MMAP_CHECK,
 	BPRM_CHECK,
 	MODULE_CHECK,
-	FIRMWARE_CHECK,
 	POST_SETATTR
 };

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 3adf937..57b1ad1 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -76,8 +76,6 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_bprm_status;
 	case MODULE_CHECK:
 		return iint->ima_module_status;
-	case FIRMWARE_CHECK:
-		return iint->ima_firmware_status;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		return iint->ima_read_status;
 	case FILE_CHECK:

@@ -99,9 +97,6 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case MODULE_CHECK:
 		iint->ima_module_status = status;
 		break;
-	case FIRMWARE_CHECK:
-		iint->ima_firmware_status = status;
-		break;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		iint->ima_read_status = status;
 		break;

@@ -124,9 +119,6 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
 	case MODULE_CHECK:
 		iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED);
 		break;
-	case FIRMWARE_CHECK:
-		iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED);
-		break;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		break;
 	case FILE_CHECK:

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 668cbc6..1251882 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -337,17 +337,6 @@ int ima_module_check(struct file *file)
 	return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
 }
 
-int ima_fw_from_file(struct file *file, char *buf, size_t size)
-{
-	if (!file) {
-		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE))
-			return -EACCES;	/* INTEGRITY_UNKNOWN */
-		return 0;
-	}
-	return process_measurement(file, NULL, 0, MAY_EXEC, FIRMWARE_CHECK, 0);
-}
-
 /**
  * ima_hash_and_process_file - in memory collect/appraise/audit measurement
  * @file: pointer to the file to be measured/appraised/audit

@@ -361,6 +350,13 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size)
 int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
 			      enum ima_policy_id policy_id)
 {
+	if (!file && policy_id == FIRMWARE_CHECK) {
+		if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE))
+			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		return 0;
+	}
+
 	if (!file || !buf || size == 0) { /* should never happen */
 		if (ima_appraise & IMA_APPRAISE_ENFORCE)
 			return -EACCES;

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 4711083..dbd7aa1 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -102,7 +102,8 @@ static struct ima_rule_entry original_measurement_rules[] = {
 	{.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID},
 	{.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
-	{.action = MEASURE, .hooks.func = FIRMWARE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
+	 .flags = IMA_FUNC},
 };
 
 static struct ima_rule_entry default_measurement_rules[] = {

@@ -115,7 +116,8 @@ static struct ima_rule_entry default_measurement_rules[] = {
 	{.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_INMASK | IMA_UID},
 	{.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
-	{.action = MEASURE, .hooks.func = FIRMWARE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
+	 .flags = IMA_FUNC},
 };
 
 static struct ima_rule_entry default_appraise_rules[] = {

@@ -304,8 +306,6 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
 		return IMA_BPRM_APPRAISE;
 	case MODULE_CHECK:
 		return IMA_MODULE_APPRAISE;
-	case FIRMWARE_CHECK:
-		return IMA_FIRMWARE_APPRAISE;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		return IMA_READ_APPRAISE;
 	case FILE_CHECK:

@@ -609,8 +609,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->hooks.func = FILE_CHECK;
 			else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
 				entry->hooks.func = MODULE_CHECK;
-			else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0)
-				entry->hooks.func = FIRMWARE_CHECK;
 			else if ((strcmp(args[0].from, "FILE_MMAP") == 0)
 				|| (strcmp(args[0].from, "MMAP_CHECK") == 0))
 				entry->hooks.func = MMAP_CHECK;

@@ -620,6 +618,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->hooks.policy_id = KEXEC_CHECK;
 			else if (strcmp(args[0].from, "INITRAMFS_CHECK") == 0)
 				entry->hooks.policy_id = INITRAMFS_CHECK;
+			else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0)
+				entry->hooks.policy_id = FIRMWARE_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)

@@ -776,7 +776,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 		result = -EINVAL;
 	else if (entry->hooks.func == MODULE_CHECK)
 		temp_ima_appraise |= IMA_APPRAISE_MODULES;
-	else if (entry->hooks.func == FIRMWARE_CHECK)
+	else if (entry->hooks.policy_id == FIRMWARE_CHECK)
 		temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
 	audit_log_format(ab, "res=%d", !result);
 	audit_log_end(ab);

@@ -863,8 +863,8 @@ static char *mask_tokens[] = {
 
 enum {
 	func_file = 0, func_mmap, func_bprm,
-	func_module, func_firmware, func_post,
-	func_kexec, func_initramfs
+	func_module, func_post,
+	func_kexec, func_initramfs, func_firmware
 };
 
 static char *func_tokens[] = {

@@ -872,10 +872,10 @@ static char *func_tokens[] = {
 	"MMAP_CHECK",
 	"BPRM_CHECK",
 	"MODULE_CHECK",
-	"FIRMWARE_CHECK",
 	"POST_SETATTR",
 	"KEXEC_CHECK",
 	"INITRAMFS_CHECK",
+	"FIRMWARE_CHECK"
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)

@@ -949,9 +949,6 @@ int ima_policy_show(struct seq_file *m, void *v)
 		case MODULE_CHECK:
 			seq_printf(m, pt(Opt_func), ft(func_module));
 			break;
-		case FIRMWARE_CHECK:
-			seq_printf(m, pt(Opt_func), ft(func_firmware));
-			break;
 		case POST_SETATTR:
 			seq_printf(m, pt(Opt_func), ft(func_post));
 			break;

@@ -963,6 +960,9 @@ int ima_policy_show(struct seq_file *m, void *v)
 			case INITRAMFS_CHECK:
 				seq_printf(m, pt(Opt_func), ft(func_initramfs));
 				break;
+			case FIRMWARE_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_firmware));
+				break;
 			default:
 				snprintf(tbuf, sizeof(tbuf), "%d",
 					 entry->hooks.func);

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 9a0ea4c..75334cd 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h

@@ -47,16 +47,14 @@
 #define IMA_BPRM_APPRAISED	0x00002000
 #define IMA_MODULE_APPRAISE	0x00004000
 #define IMA_MODULE_APPRAISED	0x00008000
-#define IMA_FIRMWARE_APPRAISE	0x00010000
-#define IMA_FIRMWARE_APPRAISED	0x00020000
-#define IMA_READ_APPRAISE	0x00040000
-#define IMA_READ_APPRAISED	0x00080000
+#define IMA_READ_APPRAISE	0x00010000
+#define IMA_READ_APPRAISED	0x00020000
 #define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
 				 IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE | \
-				 IMA_FIRMWARE_APPRAISE | IMA_READ_APPRAISE)
+				 IMA_READ_APPRAISE)
 #define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
 				 IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED | \
-				 IMA_FIRMWARE_APPRAISED | IMA_READ_APPRAISED)
+				 IMA_READ_APPRAISED)
 
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,

@@ -112,7 +110,6 @@ struct integrity_iint_cache {
 	enum integrity_status ima_mmap_status:4;
 	enum integrity_status ima_bprm_status:4;
 	enum integrity_status ima_module_status:4;
-	enum integrity_status ima_firmware_status:4;
 	enum integrity_status ima_read_status:4;
 	enum integrity_status evm_status:4;
 	struct ima_digest_data *ima_hash;

diff --git a/security/security.c b/security/security.c
index 49cacae..a391ce4 100644
--- a/security/security.c
+++ b/security/security.c

@@ -884,17 +884,6 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
 	return call_int_hook(kernel_create_files_as, 0, new, inode);
 }
 
-int security_kernel_fw_from_file(struct file *file, char *buf, size_t size)
-{
-	int ret;
-
-	ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
-	if (ret)
-		return ret;
-	return ima_fw_from_file(file, buf, size);
-}
-EXPORT_SYMBOL_GPL(security_kernel_fw_from_file);
-
 int security_kernel_module_request(char *kmod_name)
 {
 	return call_int_hook(kernel_module_request, 0, kmod_name);

@@ -913,8 +902,21 @@ int security_kernel_module_from_file(struct file *file)
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   int policy_id)
 {
-	return = call_int_hook(kernel_post_read_file, 0, file, buf, size,
-			       policy_id);
+	int ret = 0;
+
+	switch (policy_id) {
+	case FIRMWARE_CHECK:
+		ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
+		break;
+	default:
+		ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
+				    policy_id);
+		break;
+	}
+	if (ret)
+		return ret;
+
+	return ima_hash_and_process_file(file, buf, size, policy_id);
 }
 EXPORT_SYMBOL_GPL(security_kernel_post_read_file);

-- 
2.1.0

[RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:25:10

This patch defines kernel_read_file_from_fd(), a wrapper for the VFS
common kernel_read_file(), and replaces the kexec copy_file_from_fd()
calls with the kernel_read_file_from_fd() wrapper.

Two new IMA policy identifiers named KEXEC_CHECK and INITRAMFS_CHECK
are defined for measuring, appraising or auditing the kexec image
and initramfs.

Changelog v1:
- re-order and squash the kexec patches

v3: ima: measure and appraise kexec image and initramfs (squashed)
- rename ima_read_hooks enumeration to ima_policy_id
- use kstat file size type loff_t, not size_t
- add union name "hooks" to fix sparse warning

v2:
- Calculate the file hash from the in memory buffer
(suggested by Dave Young)
- Rename ima_read_and_process_file() to ima_hash_and_process_file()
- replace individual case statements with range:
        KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1
v1:
- Instead of ima_read_and_process_file() allocating memory, the caller
allocates and frees the memory.
- Moved the kexec measurement/appraisal call to copy_file_from_fd(). The
same call now measures and appraises both the kexec image and initramfs.

Signed-off-by: Mimi Zohar <redacted>
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 fs/exec.c                             | 15 ++++++++
 include/linux/fs.h                    |  1 +
 include/linux/ima.h                   |  2 +
 kernel/kexec_file.c                   | 72 ++++-------------------------------
 security/integrity/ima/ima.h          |  9 ++++-
 security/integrity/ima/ima_appraise.c |  7 ++++
 security/integrity/ima/ima_policy.c   | 27 ++++++++++---
 8 files changed, 64 insertions(+), 71 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 0a378a8..e80f767 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy

@@ -26,7 +26,7 @@ Description:
 			option:	[[appraise_type=]] [permit_directio]
 
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
-				[FIRMWARE_CHECK]
+				[FIRMWARE_CHECK] [KEXEC_CHECK] [INITRAMFS_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value

diff --git a/fs/exec.c b/fs/exec.c
index 211b81c..a5ae51e 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -884,6 +884,21 @@ out:
 }
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+			     int policy_id)
+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;
+
+	if (!f.file)
+		goto out;
+
+	ret = kernel_read_file(f.file, buf, size, max_size, policy_id);
+out:
+	fdput(f);
+	return ret;
+}
+
 ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
 {
 	ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9b1468c..9642623 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -2528,6 +2528,7 @@ extern int do_pipe_flags(int *, int);
 
 extern int kernel_read(struct file *, loff_t, char *, unsigned long);
 extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
 extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ca76f60..ae91938 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -14,6 +14,8 @@
 struct linux_binprm;
 
 enum ima_policy_id {
+	KEXEC_CHECK = 1,
+	INITRAMFS_CHECK,
 	IMA_MAX_READ_CHECK
 };

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b70ada0..f7c3ce4 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c

@@ -18,6 +18,7 @@
 #include <linux/kexec.h>
 #include <linux/mutex.h>
 #include <linux/list.h>
+#include <linux/ima.h>
 #include <crypto/hash.h>
 #include <crypto/sha.h>
 #include <linux/syscalls.h>

@@ -33,65 +34,6 @@ size_t __weak kexec_purgatory_size = 0;
 
 static int kexec_calculate_store_digests(struct kimage *image);
 
-static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len)
-{
-	struct fd f = fdget(fd);
-	int ret;
-	struct kstat stat;
-	loff_t pos;
-	ssize_t bytes = 0;
-
-	if (!f.file)
-		return -EBADF;
-
-	ret = vfs_getattr(&f.file->f_path, &stat);
-	if (ret)
-		goto out;
-
-	if (stat.size > INT_MAX) {
-		ret = -EFBIG;
-		goto out;
-	}
-
-	/* Don't hand 0 to vmalloc, it whines. */
-	if (stat.size == 0) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	*buf = vmalloc(stat.size);
-	if (!*buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	pos = 0;
-	while (pos < stat.size) {
-		bytes = kernel_read(f.file, pos, (char *)(*buf) + pos,
-				    stat.size - pos);
-		if (bytes < 0) {
-			vfree(*buf);
-			ret = bytes;
-			goto out;
-		}
-
-		if (bytes == 0)
-			break;
-		pos += bytes;
-	}
-
-	if (pos != stat.size) {
-		ret = -EBADF;
-		vfree(*buf);
-		goto out;
-	}
-
-	*buf_len = pos;
-out:
-	fdput(f);
-	return ret;
-}
-
 /* Architectures can provide this probe function */
 int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
 					 unsigned long buf_len)

@@ -180,16 +122,17 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 {
 	int ret = 0;
 	void *ldata;
+	loff_t size;
 
-	ret = copy_file_from_fd(kernel_fd, &image->kernel_buf,
-				&image->kernel_buf_len);
+	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
+				       &size, INT_MAX, KEXEC_CHECK);
 	if (ret)
 		return ret;
+	image->kernel_buf_len = size;
 
 	/* Call arch image probe handlers */
 	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
 					    image->kernel_buf_len);
-
 	if (ret)
 		goto out;

@@ -204,10 +147,11 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 #endif
 	/* It is possible that there no initramfs is being loaded */
 	if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
-		ret = copy_file_from_fd(initrd_fd, &image->initrd_buf,
-					&image->initrd_buf_len);
+		ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
+					       &size, INT_MAX, INITRAMFS_CHECK);
 		if (ret)
 			goto out;
+		image->initrd_buf_len = size;
 	}
 
 	if (cmdline_len) {

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 06bcc24..b98dbd5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -160,7 +160,14 @@ void ima_free_template_entry(struct ima_template_entry *entry);
 const char *ima_d_path(struct path *path, char **pathbuf);
 
 /* IMA policy related functions */
-enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, FIRMWARE_CHECK, POST_SETATTR };
+enum ima_hooks {
+	FILE_CHECK = IMA_MAX_READ_CHECK,
+	MMAP_CHECK,
+	BPRM_CHECK,
+	MODULE_CHECK,
+	FIRMWARE_CHECK,
+	POST_SETATTR
+};
 
 int ima_match_policy(struct inode *inode, int func, int mask, int flags);
 void ima_init_policy(void);

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 4edf47f..3adf937 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -78,6 +78,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_module_status;
 	case FIRMWARE_CHECK:
 		return iint->ima_firmware_status;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		return iint->ima_read_status;
 	case FILE_CHECK:
 	default:
 		return iint->ima_file_status;

@@ -100,6 +102,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case FIRMWARE_CHECK:
 		iint->ima_firmware_status = status;
 		break;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		iint->ima_read_status = status;
+		break;
 	case FILE_CHECK:
 	default:
 		iint->ima_file_status = status;

@@ -122,6 +127,8 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
 	case FIRMWARE_CHECK:
 		iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED);
 		break;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		break;
 	case FILE_CHECK:
 	default:
 		iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 595e038..4711083 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -306,6 +306,8 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
 		return IMA_MODULE_APPRAISE;
 	case FIRMWARE_CHECK:
 		return IMA_FIRMWARE_APPRAISE;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		return IMA_READ_APPRAISE;
 	case FILE_CHECK:
 	default:
 		return IMA_FILE_APPRAISE;

@@ -614,6 +616,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->hooks.func = MMAP_CHECK;
 			else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
 				entry->hooks.func = BPRM_CHECK;
+			else if (strcmp(args[0].from, "KEXEC_CHECK") == 0)
+				entry->hooks.policy_id = KEXEC_CHECK;
+			else if (strcmp(args[0].from, "INITRAMFS_CHECK") == 0)
+				entry->hooks.policy_id = INITRAMFS_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)

@@ -867,7 +873,9 @@ static char *func_tokens[] = {
 	"BPRM_CHECK",
 	"MODULE_CHECK",
 	"FIRMWARE_CHECK",
-	"POST_SETATTR"
+	"POST_SETATTR",
+	"KEXEC_CHECK",
+	"INITRAMFS_CHECK",
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)

@@ -948,10 +956,19 @@ int ima_policy_show(struct seq_file *m, void *v)
 			seq_printf(m, pt(Opt_func), ft(func_post));
 			break;
 		default:
-			snprintf(tbuf, sizeof(tbuf), "%d",
-				 entry->hooks.func);
-			seq_printf(m, pt(Opt_func), tbuf);
-			break;
+			switch (entry->hooks.policy_id) {
+			case KEXEC_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_kexec));
+				break;
+			case INITRAMFS_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_initramfs));
+				break;
+			default:
+				snprintf(tbuf, sizeof(tbuf), "%d",
+					 entry->hooks.func);
+				seq_printf(m, pt(Opt_func), tbuf);
+				break;
+			}
 		}
 		seq_puts(m, " ");
 	}

-- 
2.1.0

[RFC PATCH v2 09/11] ima: load policy using path

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:25:15

From: Dmitry Kasatkin <redacted>

We currently cannot do appraisal or signature vetting of IMA policies
since we currently can only load IMA policies by writing the contents
of the policy directly in, as follows:

cat policy-file > <securityfs>/ima/policy

If we provide the kernel the path to the IMA policy so it can load
the policy itself it'd be able to later appraise or vet the file
signature if it has one.  This patch adds support to load the IMA
policy with a given path as follows:

echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy

This patch defines kernel_read_file_from_path(), a wrapper for the VFS
common kernel_read_file(), and replaces the integrity_read_file() with
a call to the kernel_read_file_from_path() wrapper.

Changelog v3:
- after re-ordering the patches, replace calling integrity_kernel_read()
  to read the file with kernel_read_file_from_path() (Mimi)

Changelog v2:
- Patch description re-written by Luis R. Rodriguez

Signed-off-by: Dmitry Kasatkin <redacted>
Signed-off-by: Mimi Zohar <redacted>
---
 fs/exec.c                       | 21 ++++++++++++++++++++
 include/linux/fs.h              |  1 +
 include/linux/ima.h             |  1 +
 security/integrity/ima/ima_fs.c | 43 +++++++++++++++++++++++++++++++++++++++--
 4 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 3524e5f..5731b40 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -903,6 +903,27 @@ out:
 	return ret;
 }
 
+int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+			       loff_t max_size, int policy_id)
+{
+	struct file *file;
+	int ret;
+
+	if (!path || !*path)
+		return -EINVAL;
+
+	file = filp_open(path, O_RDONLY, 0);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		pr_err("Unable to open file: %s (%d)", path, ret);
+		return ret;
+	}
+
+	ret = kernel_read_file(file, buf, size, max_size, policy_id);
+	fput(file);
+	return ret;
+}
+
 ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
 {
 	ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9642623..79b1172 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -2529,6 +2529,7 @@ extern int do_pipe_flags(int *, int);
 extern int kernel_read(struct file *, loff_t, char *, unsigned long);
 extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
 extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t, int);
 extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);

diff --git a/include/linux/ima.h b/include/linux/ima.h
index eec5e2b..164d918 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -18,6 +18,7 @@ enum ima_policy_id {
 	INITRAMFS_CHECK,
 	FIRMWARE_CHECK,
 	MODULE_CHECK,
+	POLICY_CHECK,
 	IMA_MAX_READ_CHECK
 };

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index f355231..fe8b16b 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c

@@ -22,6 +22,7 @@
 #include <linux/rculist.h>
 #include <linux/rcupdate.h>
 #include <linux/parser.h>
+#include <linux/vmalloc.h>
 
 #include "ima.h"

@@ -258,6 +259,41 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.release = seq_release,
 };
 
+static ssize_t ima_read_policy(char *path)
+{
+	void *data;
+	char *datap;
+	loff_t size;
+	int rc, pathlen = strlen(path);
+
+	char *p;
+
+	/* remove \n */
+	datap = path;
+	strsep(&datap, "\n");
+
+	rc = kernel_read_file_from_path(path, &data, &size, 0, POLICY_CHECK);
+	if (rc < 0)
+		return rc;
+
+	datap = data;
+	while (size > 0 && (p = strsep(&datap, "\n"))) {
+		pr_debug("rule: %s\n", p);
+		rc = ima_parse_add_rule(p);
+		if (rc < 0)
+			break;
+		size -= rc;
+	}
+
+	vfree(data);
+	if (rc < 0)
+		return rc;
+	else if (size)
+		return -EINVAL;
+	else
+		return pathlen;
+}
+
 static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 				size_t datalen, loff_t *ppos)
 {

@@ -286,9 +322,12 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 	result = mutex_lock_interruptible(&ima_write_mutex);
 	if (result < 0)
 		goto out_free;
-	result = ima_parse_add_rule(data);
-	mutex_unlock(&ima_write_mutex);
 
+	if (data[0] == '/')
+		result = ima_read_policy(data);
+	else
+		result = ima_parse_add_rule(data);
+	mutex_unlock(&ima_write_mutex);
 out_free:
 	kfree(data);
 out:

-- 
2.1.0

[RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:25:48

This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function.  Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.

This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file.  For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.

This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.

Signed-off-by: Mimi Zohar <redacted>
---
 fs/exec.c                             |  4 +++
 include/linux/ima.h                   |  1 +
 include/linux/lsm_hooks.h             |  8 +++++
 include/linux/security.h              |  3 +-
 kernel/module.c                       | 67 ++++-------------------------------
 security/integrity/ima/ima.h          |  1 -
 security/integrity/ima/ima_appraise.c |  7 ----
 security/integrity/ima/ima_main.c     |  5 ++-
 security/integrity/ima/ima_policy.c   | 16 ++++-----
 security/integrity/integrity.h        | 12 +++----
 security/security.c                   | 12 +++++--
 11 files changed, 47 insertions(+), 89 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index a5ae51e..3524e5f 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -842,6 +842,10 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 	if (!S_ISREG(file_inode(file)->i_mode))
 		return -EINVAL;
 
+	ret = security_kernel_read_file(file, policy_id);
+	if (ret)
+		return ret;
+
 	i_size = i_size_read(file_inode(file));
 	if (max_size > 0 && i_size > max_size)
 		return -EFBIG;

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0a7f039..eec5e2b 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -17,6 +17,7 @@ enum ima_policy_id {
 	KEXEC_CHECK = 1,
 	INITRAMFS_CHECK,
 	FIRMWARE_CHECK,
+	MODULE_CHECK,
 	IMA_MAX_READ_CHECK
 };

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4e6e2af..9915310 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -561,6 +561,12 @@
  *	the kernel module to load. If the module is being loaded from a blob,
  *	this argument will be NULL.
  *	Return 0 if permission is granted.
+ * @kernel_read_file:
+ *      Read a file specified by userspace.
+ *	@file contains the file structure pointing to the file being read
+ *	by the kernel.
+ *	@policy_id contains IMA policy identifier.
+ *	Return 0 if permission is granted.
  * @kernel_post_read_file:
  *	Read a file specified by userspace.
  *	@file contains the file structure pointing to the file being read

@@ -1465,6 +1471,7 @@ union security_list_options {
 	int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
 	int (*kernel_module_request)(char *kmod_name);
 	int (*kernel_module_from_file)(struct file *file);
+	int (*kernel_read_file)(struct file *file, int policy_id);
 	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
 				     int policy_id);
 	int (*task_fix_setuid)(struct cred *new, const struct cred *old,

@@ -1726,6 +1733,7 @@ struct security_hook_heads {
 	struct list_head kernel_act_as;
 	struct list_head kernel_create_files_as;
 	struct list_head kernel_fw_from_file;
+	struct list_head kernel_read_file;
 	struct list_head kernel_post_read_file;
 	struct list_head kernel_module_request;
 	struct list_head kernel_module_from_file;

diff --git a/include/linux/security.h b/include/linux/security.h
index 51f3bc6..6d005b3 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -301,6 +301,7 @@ int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
 int security_kernel_module_from_file(struct file *file);
+int security_kernel_read_file(struct file *file, int policy_id);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
 				   int policy_id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,

@@ -857,7 +858,7 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
-static inline int security_kernel_module_from_file(struct file *file)
+static inline int security_kernel_read_file(struct file *file, int policy_id)
 {
 	return 0;
 }

diff --git a/kernel/module.c b/kernel/module.c
index 8f051a1..7398d12 100644
--- a/kernel/module.c
+++ b/kernel/module.c

@@ -2665,7 +2665,7 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	if (info->len < sizeof(*(info->hdr)))
 		return -ENOEXEC;
 
-	err = security_kernel_module_from_file(NULL);
+	err = security_kernel_read_file(NULL, MODULE_CHECK);
 	if (err)
 		return err;

@@ -2683,63 +2683,6 @@ static int copy_module_from_user(const void __user *umod, unsigned long len,
 	return 0;
 }
 
-/* Sets info->hdr and info->len. */
-static int copy_module_from_fd(int fd, struct load_info *info)
-{
-	struct fd f = fdget(fd);
-	int err;
-	struct kstat stat;
-	loff_t pos;
-	ssize_t bytes = 0;
-
-	if (!f.file)
-		return -ENOEXEC;
-
-	err = security_kernel_module_from_file(f.file);
-	if (err)
-		goto out;
-
-	err = vfs_getattr(&f.file->f_path, &stat);
-	if (err)
-		goto out;
-
-	if (stat.size > INT_MAX) {
-		err = -EFBIG;
-		goto out;
-	}
-
-	/* Don't hand 0 to vmalloc, it whines. */
-	if (stat.size == 0) {
-		err = -EINVAL;
-		goto out;
-	}
-
-	info->hdr = vmalloc(stat.size);
-	if (!info->hdr) {
-		err = -ENOMEM;
-		goto out;
-	}
-
-	pos = 0;
-	while (pos < stat.size) {
-		bytes = kernel_read(f.file, pos, (char *)(info->hdr) + pos,
-				    stat.size - pos);
-		if (bytes < 0) {
-			vfree(info->hdr);
-			err = bytes;
-			goto out;
-		}
-		if (bytes == 0)
-			break;
-		pos += bytes;
-	}
-	info->len = pos;
-
-out:
-	fdput(f);
-	return err;
-}
-
 static void free_copy(struct load_info *info)
 {
 	vfree(info->hdr);

@@ -3602,8 +3545,10 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
 
 SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 {
-	int err;
 	struct load_info info = { };
+	loff_t size;
+	void *hdr;
+	int err;
 
 	err = may_init_module();
 	if (err)

@@ -3615,9 +3560,11 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 		      |MODULE_INIT_IGNORE_VERMAGIC))
 		return -EINVAL;
 
-	err = copy_module_from_fd(fd, &info);
+	err = kernel_read_file_from_fd(fd, &hdr, &size, INT_MAX, MODULE_CHECK);
 	if (err)
 		return err;
+	info.hdr = hdr;
+	info.len = size;
 
 	return load_module(&info, uargs, flags);
 }

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 520c7b4..fc31ba2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -164,7 +164,6 @@ enum ima_hooks {
 	FILE_CHECK = IMA_MAX_READ_CHECK,
 	MMAP_CHECK,
 	BPRM_CHECK,
-	MODULE_CHECK,
 	POST_SETATTR
 };

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 57b1ad1..6b3e30a 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -74,8 +74,6 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_mmap_status;
 	case BPRM_CHECK:
 		return iint->ima_bprm_status;
-	case MODULE_CHECK:
-		return iint->ima_module_status;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		return iint->ima_read_status;
 	case FILE_CHECK:

@@ -94,8 +92,6 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case BPRM_CHECK:
 		iint->ima_bprm_status = status;
 		break;
-	case MODULE_CHECK:
-		iint->ima_module_status = status;
 		break;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		iint->ima_read_status = status;

@@ -116,9 +112,6 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
 	case BPRM_CHECK:
 		iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
 		break;
-	case MODULE_CHECK:
-		iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED);
-		break;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		break;
 	case FILE_CHECK:

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 1251882..107e6a7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -334,7 +334,7 @@ int ima_module_check(struct file *file)
 #endif
 		return 0;	/* We rely on module signature checking */
 	}
-	return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
+	return 0;
 }
 
 /**

@@ -357,6 +357,9 @@ int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
 		return 0;
 	}
 
+	if (!file && policy_id == MODULE_CHECK) /* MODULE_SIG_FORCE enabled */
+		return 0;
+
 	if (!file || !buf || size == 0) { /* should never happen */
 		if (ima_appraise & IMA_APPRAISE_ENFORCE)
 			return -EACCES;

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index dbd7aa1..dbfd26b 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -101,7 +101,7 @@ static struct ima_rule_entry original_measurement_rules[] = {
 	 .flags = IMA_FUNC | IMA_MASK},
 	{.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID},
-	{.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.policy_id = MODULE_CHECK, .flags = IMA_FUNC},
 	{.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC},
 };

@@ -304,8 +304,6 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
 		return IMA_MMAP_APPRAISE;
 	case BPRM_CHECK:
 		return IMA_BPRM_APPRAISE;
-	case MODULE_CHECK:
-		return IMA_MODULE_APPRAISE;
 	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
 		return IMA_READ_APPRAISE;
 	case FILE_CHECK:

@@ -607,8 +605,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			/* PATH_CHECK is for backwards compat */
 			else if (strcmp(args[0].from, "PATH_CHECK") == 0)
 				entry->hooks.func = FILE_CHECK;
-			else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
-				entry->hooks.func = MODULE_CHECK;
 			else if ((strcmp(args[0].from, "FILE_MMAP") == 0)
 				|| (strcmp(args[0].from, "MMAP_CHECK") == 0))
 				entry->hooks.func = MMAP_CHECK;

@@ -620,6 +616,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->hooks.policy_id = INITRAMFS_CHECK;
 			else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0)
 				entry->hooks.policy_id = FIRMWARE_CHECK;
+			else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
+				entry->hooks.policy_id = MODULE_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)

@@ -774,7 +772,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 	}
 	if (!result && (entry->action == UNKNOWN))
 		result = -EINVAL;
-	else if (entry->hooks.func == MODULE_CHECK)
+	else if (entry->hooks.policy_id == MODULE_CHECK)
 		temp_ima_appraise |= IMA_APPRAISE_MODULES;
 	else if (entry->hooks.policy_id == FIRMWARE_CHECK)
 		temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;

@@ -946,9 +944,6 @@ int ima_policy_show(struct seq_file *m, void *v)
 		case BPRM_CHECK:
 			seq_printf(m, pt(Opt_func), ft(func_bprm));
 			break;
-		case MODULE_CHECK:
-			seq_printf(m, pt(Opt_func), ft(func_module));
-			break;
 		case POST_SETATTR:
 			seq_printf(m, pt(Opt_func), ft(func_post));
 			break;

@@ -963,6 +958,9 @@ int ima_policy_show(struct seq_file *m, void *v)
 			case FIRMWARE_CHECK:
 				seq_printf(m, pt(Opt_func), ft(func_firmware));
 				break;
+			case MODULE_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_module));
+				break;
 			default:
 				snprintf(tbuf, sizeof(tbuf), "%d",
 					 entry->hooks.func);

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 75334cd..97fb5c2 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h

@@ -45,16 +45,12 @@
 #define IMA_MMAP_APPRAISED	0x00000800
 #define IMA_BPRM_APPRAISE	0x00001000
 #define IMA_BPRM_APPRAISED	0x00002000
-#define IMA_MODULE_APPRAISE	0x00004000
-#define IMA_MODULE_APPRAISED	0x00008000
-#define IMA_READ_APPRAISE	0x00010000
-#define IMA_READ_APPRAISED	0x00020000
+#define IMA_READ_APPRAISE	0x00004000
+#define IMA_READ_APPRAISED	0x00008000
 #define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
-				 IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE | \
-				 IMA_READ_APPRAISE)
+				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE)
 #define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
-				 IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED | \
-				 IMA_READ_APPRAISED)
+				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED)
 
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,

diff --git a/security/security.c b/security/security.c
index a391ce4..fa8a9e8 100644
--- a/security/security.c
+++ b/security/security.c

@@ -889,11 +889,17 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, 0, kmod_name);
 }
 
-int security_kernel_module_from_file(struct file *file)
+int security_kernel_read_file(struct file *file, int policy_id)
 {
 	int ret;
 
-	ret = call_int_hook(kernel_module_from_file, 0, file);
+	switch (policy_id) {
+	case MODULE_CHECK:
+		ret = call_int_hook(kernel_module_from_file, 0, file);
+		break;
+	default:
+		ret = call_int_hook(kernel_read_file, 0, file, policy_id);
+	}
 	if (ret)
 		return ret;
 	return ima_module_check(file);

@@ -1707,6 +1713,8 @@ struct security_hook_heads security_hook_heads = {
 		LIST_HEAD_INIT(security_hook_heads.kernel_module_request),
 	.kernel_module_from_file =
 		LIST_HEAD_INIT(security_hook_heads.kernel_module_from_file),
+	.kernel_read_file =
+		LIST_HEAD_INIT(security_hook_heads.kernel_read_file),
 	.kernel_post_read_file =
 		LIST_HEAD_INIT(security_hook_heads.kernel_post_read_file),
 	.task_fix_setuid =

-- 
2.1.0

[RFC PATCH v2 05/11] ima: define a new hook to measure and appraise a file already in memory

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:25:58

This patch defines a new IMA hook ima_hash_and_process_file() for
measuring and appraising files read by the kernel. The caller loads
the file into memory before calling this function, which calculates
the hash followed by the normal IMA policy based processing.

To differentiate between callers of ima_hash_and_process_file() this
patch introducees a policy identifier enumation named ima_policy_id.

Changelog v1:
- To simplify patch review, separate the IMA changes from the kexec
and initramfs changes in "ima: measure and appraise kexec image and
initramfs" patch.  This patch contains just the IMA changes.  The
kexec and initramfs changes are with the rest of the kexec changes
in "kexec: replace call to copy_file_from_fd() with kernel version".

Signed-off-by: Mimi Zohar <redacted>
---
 fs/exec.c                             |  4 +--
 include/linux/fs.h                    |  2 +-
 include/linux/ima.h                   | 14 ++++++++
 include/linux/lsm_hooks.h             |  4 ++-
 include/linux/security.h              |  6 ++--
 security/integrity/iint.c             |  1 +
 security/integrity/ima/ima.h          | 12 +++----
 security/integrity/ima/ima_api.c      |  6 ++--
 security/integrity/ima/ima_appraise.c |  4 +--
 security/integrity/ima/ima_main.c     | 40 ++++++++++++++++++-----
 security/integrity/ima/ima_policy.c   | 60 +++++++++++++++++++----------------
 security/integrity/integrity.h        |  7 ++--
 security/security.c                   |  6 ++--
 13 files changed, 110 insertions(+), 56 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 6d623c2..211b81c 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -833,7 +833,7 @@ int kernel_read(struct file *file, loff_t offset,
 EXPORT_SYMBOL(kernel_read);
 
 int kernel_read_file(struct file *file, void **buf, loff_t *size,
-		     loff_t max_size)
+		     loff_t max_size, int policy_id)
 {
 	loff_t i_size, pos;
 	ssize_t bytes = 0;

@@ -871,7 +871,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 		goto out;
 	}
 
-	ret = security_kernel_post_read_file(file, *buf, i_size);
+	ret = security_kernel_post_read_file(file, *buf, i_size, policy_id);
 	if (!ret)
 		*size = pos;

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 93ca379..9b1468c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -2527,7 +2527,7 @@ static inline void i_readcount_inc(struct inode *inode)
 extern int do_pipe_flags(int *, int);
 
 extern int kernel_read(struct file *, loff_t, char *, unsigned long);
-extern int kernel_read_file(struct file *, void **, loff_t *, loff_t);
+extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
 extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 120ccc5..ca76f60 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -13,6 +13,10 @@
 #include <linux/fs.h>
 struct linux_binprm;
 
+enum ima_policy_id {
+	IMA_MAX_READ_CHECK
+};
+
 #ifdef CONFIG_IMA
 extern int ima_bprm_check(struct linux_binprm *bprm);
 extern int ima_file_check(struct file *file, int mask, int opened);

@@ -20,6 +24,9 @@ extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_module_check(struct file *file);
 extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
+extern int ima_hash_and_process_file(struct file *file,
+				     void *buf, loff_t size,
+				     enum ima_policy_id policy_id);
 
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)

@@ -52,6 +59,13 @@ static inline int ima_fw_from_file(struct file *file, char *buf, size_t size)
 	return 0;
 }
 
+static inline int ima_hash_and_process_file(struct file *file,
+					    void *buf, loff_t size,
+					    enum ima_policy_id policy_id)
+{
+	return 0;
+}
+
 #endif /* CONFIG_IMA */
 
 #ifdef CONFIG_IMA_APPRAISE

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index f82631c..4e6e2af 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -567,6 +567,7 @@
  *	by the kernel.
  *	@buf pointer to buffer containing the file contents.
  *	@size length of the file contents.
+ *	@policy-id IMA policy identifier
  *	Return 0 if permission is granted.
  * @task_fix_setuid:
  *	Update the module's state after setting one or more of the user

@@ -1464,7 +1465,8 @@ union security_list_options {
 	int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
 	int (*kernel_module_request)(char *kmod_name);
 	int (*kernel_module_from_file)(struct file *file);
-	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size);
+	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
+				     int policy_id);
 	int (*task_fix_setuid)(struct cred *new, const struct cred *old,
 				int flags);
 	int (*task_setpgid)(struct task_struct *p, pid_t pgid);

diff --git a/include/linux/security.h b/include/linux/security.h
index f30f564..44d8832 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -301,7 +301,8 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_fw_from_file(struct file *file, char *buf, size_t size);
 int security_kernel_module_request(char *kmod_name);
 int security_kernel_module_from_file(struct file *file);
-int security_kernel_post_read_file(struct file *file, char *buf, loff_t size);
+int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
+				   int policy_id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
 			     int flags);
 int security_task_setpgid(struct task_struct *p, pid_t pgid);

@@ -868,7 +869,8 @@ static inline int security_kernel_module_from_file(struct file *file)
 }
 
 static inline int security_kernel_post_read_file(struct file *file,
-						 char *buf, loff_t size)
+						 char *buf, loff_t size,
+						 int policy_id)
 {
 	return 0;
 }

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 8f1ab37..a41e073 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c

@@ -158,6 +158,7 @@ static void init_once(void *foo)
 	iint->ima_mmap_status = INTEGRITY_UNKNOWN;
 	iint->ima_bprm_status = INTEGRITY_UNKNOWN;
 	iint->ima_module_status = INTEGRITY_UNKNOWN;
+	iint->ima_read_status = INTEGRITY_UNKNOWN;
 	iint->evm_status = INTEGRITY_UNKNOWN;
 }

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index de53631..06bcc24 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -20,6 +20,7 @@
 #include <linux/types.h>
 #include <linux/crypto.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 #include <linux/hash.h>
 #include <linux/tpm.h>
 #include <linux/audit.h>

@@ -143,7 +144,8 @@ static inline unsigned long ima_hash_key(u8 *digest)
 int ima_get_action(struct inode *inode, int mask, int function);
 int ima_must_measure(struct inode *inode, int mask, int function);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
-			    struct file *file, enum hash_algo algo);
+			    struct file *file, void *buf, loff_t size,
+			    enum hash_algo algo);
 void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,

@@ -160,8 +162,7 @@ const char *ima_d_path(struct path *path, char **pathbuf);
 /* IMA policy related functions */
 enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, FIRMWARE_CHECK, POST_SETATTR };
 
-int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags);
+int ima_match_policy(struct inode *inode, int func, int mask, int flags);
 void ima_init_policy(void);
 void ima_update_policy(void);
 void ima_update_policy_flag(void);

@@ -185,7 +186,7 @@ int ima_appraise_measurement(int 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 ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
+int ima_must_appraise(struct inode *inode, int mask, int 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,
 					   int func);

@@ -205,8 +206,7 @@ static inline int ima_appraise_measurement(int func,
 	return INTEGRITY_UNKNOWN;
 }
 
-static inline int ima_must_appraise(struct inode *inode, int mask,
-				    enum ima_hooks func)
+static inline int ima_must_appraise(struct inode *inode, int mask, int func)
 {
 	return 0;
 }

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index e7c7a5d..b702e4b 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c

@@ -188,7 +188,8 @@ int ima_get_action(struct inode *inode, int mask, int function)
  * Return 0 on success, error code otherwise
  */
 int ima_collect_measurement(struct integrity_iint_cache *iint,
-			    struct file *file, enum hash_algo algo)
+			    struct file *file, void *buf, loff_t size,
+			    enum hash_algo algo)
 {
 	const char *audit_cause = "failed";
 	struct inode *inode = file_inode(file);

@@ -210,7 +211,8 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 
 		hash.hdr.algo = algo;
 
-		result = ima_calc_file_hash(file, &hash.hdr);
+		result = (!buf) ?  ima_calc_file_hash(file, &hash.hdr) :
+			ima_calc_buffer_hash(buf, size, &hash.hdr);
 		if (!result) {
 			int length = sizeof(hash.hdr) + hash.hdr.length;
 			void *tmpbuf = krealloc(iint->ima_hash, length,

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 9c2b46b..4edf47f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -36,7 +36,7 @@ __setup("ima_appraise=", default_appraise_setup);
  *
  * Return 1 to appraise
  */
-int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
+int ima_must_appraise(struct inode *inode, int mask, int func)
 {
 	if (!ima_appraise)
 		return 0;

@@ -299,7 +299,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
 	if (iint->flags & IMA_DIGSIG)
 		return;
 
-	rc = ima_collect_measurement(iint, file, ima_hash_algo);
+	rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo);
 	if (rc < 0)
 		return;

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d9fc463..668cbc6 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -153,8 +153,8 @@ void ima_file_free(struct file *file)
 	ima_check_last_writer(iint, inode, file);
 }
 
-static int process_measurement(struct file *file, int mask, int function,
-			       int opened)
+static int process_measurement(struct file *file, char *buf, loff_t size,
+			       int mask, int function, int opened)
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint = NULL;

@@ -226,7 +226,7 @@ static int process_measurement(struct file *file, int mask, int function,
 
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
 
-	rc = ima_collect_measurement(iint, file, hash_algo);
+	rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
 	if (rc != 0) {
 		if (file->f_flags & O_DIRECT)
 			rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;

@@ -273,7 +273,8 @@ out:
 int ima_file_mmap(struct file *file, unsigned long prot)
 {
 	if (file && (prot & PROT_EXEC))
-		return process_measurement(file, MAY_EXEC, MMAP_CHECK, 0);
+		return process_measurement(file, NULL, 0, MAY_EXEC,
+					   MMAP_CHECK, 0);
 	return 0;
 }

@@ -292,7 +293,8 @@ int ima_file_mmap(struct file *file, unsigned long prot)
  */
 int ima_bprm_check(struct linux_binprm *bprm)
 {
-	return process_measurement(bprm->file, MAY_EXEC, BPRM_CHECK, 0);
+	return process_measurement(bprm->file, NULL, 0, MAY_EXEC,
+				   BPRM_CHECK, 0);
 }
 
 /**

@@ -307,7 +309,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
  */
 int ima_file_check(struct file *file, int mask, int opened)
 {
-	return process_measurement(file,
+	return process_measurement(file, NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC),
 				   FILE_CHECK, opened);
 }

@@ -332,7 +334,7 @@ int ima_module_check(struct file *file)
 #endif
 		return 0;	/* We rely on module signature checking */
 	}
-	return process_measurement(file, MAY_EXEC, MODULE_CHECK, 0);
+	return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
 }
 
 int ima_fw_from_file(struct file *file, char *buf, size_t size)

@@ -343,7 +345,29 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size)
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
 		return 0;
 	}
-	return process_measurement(file, MAY_EXEC, FIRMWARE_CHECK, 0);
+	return process_measurement(file, NULL, 0, MAY_EXEC, FIRMWARE_CHECK, 0);
+}
+
+/**
+ * ima_hash_and_process_file - in memory collect/appraise/audit measurement
+ * @file: pointer to the file to be measured/appraised/audit
+ * @buf: pointer to in memory file contents
+ * @size: size of in memory file contents
+ * @policy_id: caller identifier
+ *
+ * Measure/appraise/audit in memory file based on policy.  Policy rules
+ * are written in terms of a policy identifier.
+ */
+int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
+			      enum ima_policy_id policy_id)
+{
+	if (!file || !buf || size == 0) { /* should never happen */
+		if (ima_appraise & IMA_APPRAISE_ENFORCE)
+			return -EACCES;
+		return 0;
+	}
+
+	return process_measurement(file, buf, size, MAY_READ, policy_id, 0);
 }
 
 static int __init init_ima(void)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 0a3b781..595e038 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -53,7 +53,10 @@ struct ima_rule_entry {
 	struct list_head list;
 	int action;
 	unsigned int flags;
-	enum ima_hooks func;
+	union {
+		enum ima_hooks func;
+		enum ima_policy_id policy_id;
+	} hooks;
 	int mask;
 	unsigned long fsmagic;
 	u8 fsuuid[16];

@@ -92,27 +95,27 @@ static struct ima_rule_entry dont_measure_rules[] = {
 };
 
 static struct ima_rule_entry original_measurement_rules[] = {
-	{.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC,
+	{.action = MEASURE, .hooks.func = MMAP_CHECK, .mask = MAY_EXEC,
 	 .flags = IMA_FUNC | IMA_MASK},
-	{.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC,
+	{.action = MEASURE, .hooks.func = BPRM_CHECK, .mask = MAY_EXEC,
 	 .flags = IMA_FUNC | IMA_MASK},
-	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ,
+	{.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID},
-	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
-	{.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.func = FIRMWARE_CHECK, .flags = IMA_FUNC},
 };
 
 static struct ima_rule_entry default_measurement_rules[] = {
-	{.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC,
+	{.action = MEASURE, .hooks.func = MMAP_CHECK, .mask = MAY_EXEC,
 	 .flags = IMA_FUNC | IMA_MASK},
-	{.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC,
+	{.action = MEASURE, .hooks.func = BPRM_CHECK, .mask = MAY_EXEC,
 	 .flags = IMA_FUNC | IMA_MASK},
-	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ,
+	{.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_INMASK | IMA_EUID},
-	{.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ,
+	{.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
 	 .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_INMASK | IMA_UID},
-	{.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
-	{.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.func = FIRMWARE_CHECK, .flags = IMA_FUNC},
 };
 
 static struct ima_rule_entry default_appraise_rules[] = {

@@ -208,14 +211,14 @@ static void ima_lsm_update_rules(void)
  * Returns true on rule match, false on failure.
  */
 static bool ima_match_rules(struct ima_rule_entry *rule,
-			    struct inode *inode, enum ima_hooks func, int mask)
+			    struct inode *inode, int func, int mask)
 {
 	struct task_struct *tsk = current;
 	const struct cred *cred = current_cred();
 	int i;
 
 	if ((rule->flags & IMA_FUNC) &&
-	    (rule->func != func && func != POST_SETATTR))
+	    (rule->hooks.func != func && func != POST_SETATTR))
 		return false;
 	if ((rule->flags & IMA_MASK) &&
 	    (rule->mask != mask && func != POST_SETATTR))

@@ -322,8 +325,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
  * list when walking it.  Reads are many orders of magnitude more numerous
  * 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 ima_match_policy(struct inode *inode, int func, int mask, int flags)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);

@@ -595,23 +597,23 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 		case Opt_func:
 			ima_log_string(ab, "func", args[0].from);
 
-			if (entry->func)
+			if (entry->hooks.func)
 				result = -EINVAL;
 
 			if (strcmp(args[0].from, "FILE_CHECK") == 0)
-				entry->func = FILE_CHECK;
+				entry->hooks.func = FILE_CHECK;
 			/* PATH_CHECK is for backwards compat */
 			else if (strcmp(args[0].from, "PATH_CHECK") == 0)
-				entry->func = FILE_CHECK;
+				entry->hooks.func = FILE_CHECK;
 			else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
-				entry->func = MODULE_CHECK;
+				entry->hooks.func = MODULE_CHECK;
 			else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0)
-				entry->func = FIRMWARE_CHECK;
+				entry->hooks.func = FIRMWARE_CHECK;
 			else if ((strcmp(args[0].from, "FILE_MMAP") == 0)
 				|| (strcmp(args[0].from, "MMAP_CHECK") == 0))
-				entry->func = MMAP_CHECK;
+				entry->hooks.func = MMAP_CHECK;
 			else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
-				entry->func = BPRM_CHECK;
+				entry->hooks.func = BPRM_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)

@@ -766,9 +768,9 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 	}
 	if (!result && (entry->action == UNKNOWN))
 		result = -EINVAL;
-	else if (entry->func == MODULE_CHECK)
+	else if (entry->hooks.func == MODULE_CHECK)
 		temp_ima_appraise |= IMA_APPRAISE_MODULES;
-	else if (entry->func == FIRMWARE_CHECK)
+	else if (entry->hooks.func == FIRMWARE_CHECK)
 		temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
 	audit_log_format(ab, "res=%d", !result);
 	audit_log_end(ab);

@@ -855,7 +857,8 @@ static char *mask_tokens[] = {
 
 enum {
 	func_file = 0, func_mmap, func_bprm,
-	func_module, func_firmware, func_post
+	func_module, func_firmware, func_post,
+	func_kexec, func_initramfs
 };
 
 static char *func_tokens[] = {

@@ -925,7 +928,7 @@ int ima_policy_show(struct seq_file *m, void *v)
 	seq_puts(m, " ");
 
 	if (entry->flags & IMA_FUNC) {
-		switch (entry->func) {
+		switch (entry->hooks.func) {
 		case FILE_CHECK:
 			seq_printf(m, pt(Opt_func), ft(func_file));
 			break;

@@ -945,7 +948,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 			seq_printf(m, pt(Opt_func), ft(func_post));
 			break;
 		default:
-			snprintf(tbuf, sizeof(tbuf), "%d", entry->func);
+			snprintf(tbuf, sizeof(tbuf), "%d",
+				 entry->hooks.func);
 			seq_printf(m, pt(Opt_func), tbuf);
 			break;
 		}

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 5efe2ec..9a0ea4c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h

@@ -49,12 +49,14 @@
 #define IMA_MODULE_APPRAISED	0x00008000
 #define IMA_FIRMWARE_APPRAISE	0x00010000
 #define IMA_FIRMWARE_APPRAISED	0x00020000
+#define IMA_READ_APPRAISE	0x00040000
+#define IMA_READ_APPRAISED	0x00080000
 #define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
 				 IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE | \
-				 IMA_FIRMWARE_APPRAISE)
+				 IMA_FIRMWARE_APPRAISE | IMA_READ_APPRAISE)
 #define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
 				 IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED | \
-				 IMA_FIRMWARE_APPRAISED)
+				 IMA_FIRMWARE_APPRAISED | IMA_READ_APPRAISED)
 
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,

@@ -111,6 +113,7 @@ struct integrity_iint_cache {
 	enum integrity_status ima_bprm_status:4;
 	enum integrity_status ima_module_status:4;
 	enum integrity_status ima_firmware_status:4;
+	enum integrity_status ima_read_status:4;
 	enum integrity_status evm_status:4;
 	struct ima_digest_data *ima_hash;
 };

diff --git a/security/security.c b/security/security.c
index 293bb4b..49cacae 100644
--- a/security/security.c
+++ b/security/security.c

@@ -910,9 +910,11 @@ int security_kernel_module_from_file(struct file *file)
 	return ima_module_check(file);
 }
 
-int security_kernel_post_read_file(struct file *file, char *buf, loff_t size)
+int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
+				   int policy_id)
 {
-	return = call_int_hook(kernel_post_read_file, 0, file, buf, size);
+	return = call_int_hook(kernel_post_read_file, 0, file, buf, size,
+			       policy_id);
 }
 EXPORT_SYMBOL_GPL(security_kernel_post_read_file);

-- 
2.1.0

[RFC PATCH v2 02/11] vfs: define a generic function to read a file from the kernel

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:26:16

For a while it was looked down upon to directly read files from Linux.
These days there exists a few mechanisms in the kernel that do just
this though to load a file into a local buffer.  There are minor but
important checks differences on each.  This patch set is the first
attempt at resolving some of these differences.

This patch introduces a common function for reading files from the kernel
with the corresponding security post-read hook and function.

Changelog v1:
- To simplify patch review, re-ordered patches

Signed-off-by: Mimi Zohar <redacted>
---
 fs/exec.c                 | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h        |  1 +
 include/linux/lsm_hooks.h |  9 ++++++++
 include/linux/security.h  |  7 +++++++
 security/security.c       |  8 +++++++
 5 files changed, 78 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index b06623a..6d623c2 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -56,6 +56,7 @@
 #include <linux/pipe_fs_i.h>
 #include <linux/oom.h>
 #include <linux/compat.h>
+#include <linux/vmalloc.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>

@@ -831,6 +832,58 @@ int kernel_read(struct file *file, loff_t offset,
 
 EXPORT_SYMBOL(kernel_read);
 
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+		     loff_t max_size)
+{
+	loff_t i_size, pos;
+	ssize_t bytes = 0;
+	int ret;
+
+	if (!S_ISREG(file_inode(file)->i_mode))
+		return -EINVAL;
+
+	i_size = i_size_read(file_inode(file));
+	if (max_size > 0 && i_size > max_size)
+		return -EFBIG;
+	if (i_size == 0)
+		return -EINVAL;
+
+	*buf = vmalloc(i_size);
+	if (!*buf)
+		return -ENOMEM;
+
+	pos = 0;
+	while (pos < i_size) {
+		bytes = kernel_read(file, pos, (char *)(*buf) + pos,
+				    i_size - pos);
+		if (bytes < 0) {
+			ret = bytes;
+			goto out;
+		}
+
+		if (bytes == 0)
+			break;
+		pos += bytes;
+	}
+
+	if (pos != i_size) {
+		ret = -EIO;
+		goto out;
+	}
+
+	ret = security_kernel_post_read_file(file, *buf, i_size);
+	if (!ret)
+		*size = pos;
+
+out:
+	if (ret < 0) {
+		vfree(*buf);
+		*buf = NULL;
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
 ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
 {
 	ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3aa5142..93ca379 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -2527,6 +2527,7 @@ static inline void i_readcount_inc(struct inode *inode)
 extern int do_pipe_flags(int *, int);
 
 extern int kernel_read(struct file *, loff_t, char *, unsigned long);
+extern int kernel_read_file(struct file *, void **, loff_t *, loff_t);
 extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 71969de..f82631c 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -561,6 +561,13 @@
  *	the kernel module to load. If the module is being loaded from a blob,
  *	this argument will be NULL.
  *	Return 0 if permission is granted.
+ * @kernel_post_read_file:
+ *	Read a file specified by userspace.
+ *	@file contains the file structure pointing to the file being read
+ *	by the kernel.
+ *	@buf pointer to buffer containing the file contents.
+ *	@size length of the file contents.
+ *	Return 0 if permission is granted.
  * @task_fix_setuid:
  *	Update the module's state after setting one or more of the user
  *	identity attributes of the current process.  The @flags parameter

@@ -1457,6 +1464,7 @@ union security_list_options {
 	int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
 	int (*kernel_module_request)(char *kmod_name);
 	int (*kernel_module_from_file)(struct file *file);
+	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size);
 	int (*task_fix_setuid)(struct cred *new, const struct cred *old,
 				int flags);
 	int (*task_setpgid)(struct task_struct *p, pid_t pgid);

@@ -1716,6 +1724,7 @@ struct security_hook_heads {
 	struct list_head kernel_act_as;
 	struct list_head kernel_create_files_as;
 	struct list_head kernel_fw_from_file;
+	struct list_head kernel_post_read_file;
 	struct list_head kernel_module_request;
 	struct list_head kernel_module_from_file;
 	struct list_head task_fix_setuid;

diff --git a/include/linux/security.h b/include/linux/security.h
index 4824a4c..f30f564 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -301,6 +301,7 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_fw_from_file(struct file *file, char *buf, size_t size);
 int security_kernel_module_request(char *kmod_name);
 int security_kernel_module_from_file(struct file *file);
+int security_kernel_post_read_file(struct file *file, char *buf, loff_t size);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
 			     int flags);
 int security_task_setpgid(struct task_struct *p, pid_t pgid);

@@ -866,6 +867,12 @@ static inline int security_kernel_module_from_file(struct file *file)
 	return 0;
 }
 
+static inline int security_kernel_post_read_file(struct file *file,
+						 char *buf, loff_t size)
+{
+	return 0;
+}
+
 static inline int security_task_fix_setuid(struct cred *new,
 					   const struct cred *old,
 					   int flags)

diff --git a/security/security.c b/security/security.c
index e8ffd92..293bb4b 100644
--- a/security/security.c
+++ b/security/security.c

@@ -910,6 +910,12 @@ int security_kernel_module_from_file(struct file *file)
 	return ima_module_check(file);
 }
 
+int security_kernel_post_read_file(struct file *file, char *buf, loff_t size)
+{
+	return = call_int_hook(kernel_post_read_file, 0, file, buf, size);
+}
+EXPORT_SYMBOL_GPL(security_kernel_post_read_file);
+
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
 			     int flags)
 {

@@ -1697,6 +1703,8 @@ struct security_hook_heads security_hook_heads = {
 		LIST_HEAD_INIT(security_hook_heads.kernel_module_request),
 	.kernel_module_from_file =
 		LIST_HEAD_INIT(security_hook_heads.kernel_module_from_file),
+	.kernel_post_read_file =
+		LIST_HEAD_INIT(security_hook_heads.kernel_post_read_file),
 	.task_fix_setuid =
 		LIST_HEAD_INIT(security_hook_heads.task_fix_setuid),
 	.task_setpgid =	LIST_HEAD_INIT(security_hook_heads.task_setpgid),

-- 
2.1.0

[RFC PATCH v2 11/11] ima: require signed IMA policy

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:28:30

Require the IMA policy to be signed when additional rules can be added.

Changelog v2:
- add union name "hooks" to fix sparse warning
v1:
- initialize the policy flag
- include IMA_APPRAISE_POLICY in the policy flag

Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima_policy.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 7a63760..327e691 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -133,6 +133,10 @@ static struct ima_rule_entry default_appraise_rules[] = {
 	{.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = NSFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
+#ifdef CONFIG_IMA_WRITE_POLICY
+	{.action = APPRAISE, .hooks.policy_id = POLICY_CHECK,
+	.flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+#endif
 #ifndef CONFIG_IMA_APPRAISE_SIGNED_INIT
 	{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
 #else

@@ -414,9 +418,12 @@ void __init ima_init_policy(void)
 	for (i = 0; i < appraise_entries; i++) {
 		list_add_tail(&default_appraise_rules[i].list,
 			      &ima_default_rules);
+		if (default_appraise_rules[i].hooks.policy_id == POLICY_CHECK)
+			temp_ima_appraise |= IMA_APPRAISE_POLICY;
 	}
 
 	ima_rules = &ima_default_rules;
+	ima_update_policy_flag();
 }
 
 /* Make sure we have a valid policy, at least containing some rules. */

-- 
2.1.0

[RFC PATCH v2 10/11] ima: measure and appraise the IMA policy itself

From: Mimi Zohar <hidden>
Date: 2016-01-18 15:29:52

This patch adds support for measuring and appraising the IMA policy
itself.

Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_fs.c     |  9 ++++++++-
 security/integrity/ima/ima_policy.c | 14 ++++++++++++--
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fc31ba2..e8f111b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -185,6 +185,7 @@ int ima_policy_show(struct seq_file *m, void *v);
 #define IMA_APPRAISE_LOG	0x04
 #define IMA_APPRAISE_MODULES	0x08
 #define IMA_APPRAISE_FIRMWARE	0x10
+#define IMA_APPRAISE_POLICY	0x20
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index fe8b16b..57c6b2e 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c

@@ -325,7 +325,14 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 
 	if (data[0] == '/')
 		result = ima_read_policy(data);
-	else
+	else if (ima_appraise & IMA_APPRAISE_POLICY) {
+		pr_err("IMA: signed policy 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);
 	mutex_unlock(&ima_write_mutex);
 out_free:

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index dbfd26b..7a63760 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -118,6 +118,7 @@ static struct ima_rule_entry default_measurement_rules[] = {
 	{.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
 	{.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC},
+	{.action = MEASURE, .hooks.policy_id = POLICY_CHECK, .flags = IMA_FUNC},
 };
 
 static struct ima_rule_entry default_appraise_rules[] = {

@@ -618,6 +619,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->hooks.policy_id = FIRMWARE_CHECK;
 			else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
 				entry->hooks.policy_id = MODULE_CHECK;
+			else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
+				entry->hooks.policy_id = POLICY_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)

@@ -776,6 +779,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 		temp_ima_appraise |= IMA_APPRAISE_MODULES;
 	else if (entry->hooks.policy_id == FIRMWARE_CHECK)
 		temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
+	else if (entry->hooks.policy_id == POLICY_CHECK)
+		temp_ima_appraise |= IMA_APPRAISE_POLICY;
 	audit_log_format(ab, "res=%d", !result);
 	audit_log_end(ab);
 	return result;

@@ -862,7 +867,8 @@ static char *mask_tokens[] = {
 enum {
 	func_file = 0, func_mmap, func_bprm,
 	func_module, func_post,
-	func_kexec, func_initramfs, func_firmware
+	func_kexec, func_initramfs, func_firmware,
+	func_policy
 };
 
 static char *func_tokens[] = {

@@ -873,7 +879,8 @@ static char *func_tokens[] = {
 	"POST_SETATTR",
 	"KEXEC_CHECK",
 	"INITRAMFS_CHECK",
-	"FIRMWARE_CHECK"
+	"FIRMWARE_CHECK",
+	"POLICY_CHECK"
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)

@@ -961,6 +968,9 @@ int ima_policy_show(struct seq_file *m, void *v)
 			case MODULE_CHECK:
 				seq_printf(m, pt(Opt_func), ft(func_module));
 				break;
+			case POLICY_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_policy));
+				break;
 			default:
 				snprintf(tbuf, sizeof(tbuf), "%d",
 					 entry->hooks.func);

-- 
2.1.0

Re: [RFC PATCH v2 03/11] ima: provide buffer hash calculation function

From: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Date: 2016-01-19 19:26:27

On Mon, Jan 18, 2016 at 5:11 PM, Mimi Zohar [off-list ref] wrote:

quoted hunk

From: Dmitry Kasatkin <redacted>

This patch provides convenient buffer hash calculation function.

Changelog:
- rewrite to support loff_t sized buffers - Mimi
  (based on Fenguang Wu's testing)

Signed-off-by: Dmitry Kasatkin <redacted>
Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima.h        |  2 ++
 security/integrity/ima/ima_crypto.c | 47 +++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fb8da36..de53631 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -107,6 +107,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
                           const char *op, struct inode *inode,
                           const unsigned char *filename);
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+                        struct ima_digest_data *hash);
 int ima_calc_field_array_hash(struct ima_field_data *field_data,
                              struct ima_template_desc *desc, int num_fields,
                              struct ima_digest_data *hash);

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index fb30ce4..8d86281 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c

@@ -519,6 +519,53 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
        return rc;
 }

+static int calc_buffer_shash_tfm(const void *buf, loff_t size,
+                               struct ima_digest_data *hash,
+                               struct crypto_shash *tfm)
+{
+       SHASH_DESC_ON_STACK(shash, tfm);
+       unsigned int len;
+       loff_t offset = 0;
+       int rc;
+
+       shash->tfm = tfm;
+       shash->flags = 0;
+
+       hash->length = crypto_shash_digestsize(tfm);
+
+       rc = crypto_shash_init(shash);
+       if (rc != 0)
+               return rc;
+
+       len = size < PAGE_SIZE ? size : PAGE_SIZE;
+       while (offset < size) {
+               rc = crypto_shash_update(shash, buf + offset, len);
+               if (rc)
+                       break;
+               offset += len;
+       }
+

Hello Mimi,

May be this was my earlier patch, but it seems to have a problem of
accessing beyond end of buffer using the same len.
When buffer always padded by zeros it is not a problem, but it is a bug.

This seems to be better version.

   while (size) {
               len = size < PAGE_SIZE ? size : PAGE_SIZE;
               rc = crypto_shash_update(shash, buf, len);
               if (rc)
                       break;
               buf += len;
               size -= len;
    }

Please change my sign-of to: dmitry.kasatkin@huawei.com

Thanks.
Dmitry

+       if (!rc)
+               rc = crypto_shash_final(shash, hash->digest);
+       return rc;
+}
+
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+                        struct ima_digest_data *hash)
+{
+       struct crypto_shash *tfm;
+       int rc;
+
+       tfm = ima_alloc_tfm(hash->algo);
+       if (IS_ERR(tfm))
+               return PTR_ERR(tfm);
+
+       rc = calc_buffer_shash_tfm(buf, len, hash, tfm);
+
+       ima_free_tfm(tfm);
+       return rc;
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
        if (!ima_used_chip)
--
2.1.0



-- 
Thanks,
Dmitry

Re: [RFC PATCH v2 01/11] ima: separate 'security.ima' reading functionality from collect

From: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Date: 2016-01-19 20:01:03

Hi Mimi,

Please change

Signed-off-by: Dmitry Kasatkin <redacted>

Thanks

Dmitry


On Mon, Jan 18, 2016 at 5:11 PM, Mimi Zohar [off-list ref] wrote:

quoted hunk

From: Dmitry Kasatkin <redacted>

Instead of passing pointers to pointers to ima_collect_measurent() to
read and return the 'security.ima' xattr value, this patch moves the
functionality to the calling process_measurement() to directly read
the xattr and pass only the hash algo to the ima_collect_measurement().

Signed-off-by: Dmitry Kasatkin <redacted>
Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima.h              | 15 +++++++--------
 security/integrity/ima/ima_api.c          | 15 +++------------
 security/integrity/ima/ima_appraise.c     | 25 ++++++++++++++-----------
 security/integrity/ima/ima_crypto.c       |  2 +-
 security/integrity/ima/ima_init.c         |  2 +-
 security/integrity/ima/ima_main.c         | 11 +++++++----
 security/integrity/ima/ima_template.c     |  2 --
 security/integrity/ima/ima_template_lib.c |  1 -
 8 files changed, 33 insertions(+), 40 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 585af61..fb8da36 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -23,6 +23,7 @@
 #include <linux/hash.h>
 #include <linux/tpm.h>
 #include <linux/audit.h>
+#include <crypto/hash_info.h>

 #include "../integrity.h"

@@ -140,9 +141,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 int ima_get_action(struct inode *inode, int mask, int function);
 int ima_must_measure(struct inode *inode, int mask, int function);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
-                           struct file *file,
-                           struct evm_ima_xattr_data **xattr_value,
-                           int *xattr_len);
+                           struct file *file, enum hash_algo algo);
 void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
                           const unsigned char *filename,
                           struct evm_ima_xattr_data *xattr_value,

@@ -188,8 +187,8 @@ 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,
                                           int func);
-void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len,
-                      struct ima_digest_data *hash);
+enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
+                                int xattr_len);
 int ima_read_xattr(struct dentry *dentry,
                   struct evm_ima_xattr_data **xattr_value);

@@ -221,10 +220,10 @@ static inline enum integrity_status ima_get_cache_status(struct integrity_iint_c
        return INTEGRITY_UNKNOWN;
 }

-static inline void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
-                                    int xattr_len,
-                                    struct ima_digest_data *hash)
+static inline enum hash_algo
+ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
 {
+       return ima_hash_algo;
 }

 static inline int ima_read_xattr(struct dentry *dentry,

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 1d950fb..e7c7a5d 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c

@@ -18,7 +18,7 @@
 #include <linux/fs.h>
 #include <linux/xattr.h>
 #include <linux/evm.h>
-#include <crypto/hash_info.h>
+
 #include "ima.h"

 /*

@@ -188,9 +188,7 @@ int ima_get_action(struct inode *inode, int mask, int function)
  * Return 0 on success, error code otherwise
  */
 int ima_collect_measurement(struct integrity_iint_cache *iint,
-                           struct file *file,
-                           struct evm_ima_xattr_data **xattr_value,
-                           int *xattr_len)
+                           struct file *file, enum hash_algo algo)
 {
        const char *audit_cause = "failed";
        struct inode *inode = file_inode(file);

@@ -201,9 +199,6 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
                char digest[IMA_MAX_DIGEST_SIZE];
        } hash;

-       if (xattr_value)
-               *xattr_len = ima_read_xattr(file->f_path.dentry, xattr_value);
-
        if (!(iint->flags & IMA_COLLECTED)) {
                u64 i_version = file_inode(file)->i_version;

@@ -213,11 +208,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
                        goto out;
                }

-               /* use default hash algorithm */
-               hash.hdr.algo = ima_hash_algo;
-
-               if (xattr_value)
-                       ima_get_hash_algo(*xattr_value, *xattr_len, &hash.hdr);
+               hash.hdr.algo = algo;

                result = ima_calc_file_hash(file, &hash.hdr);
                if (!result) {

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 1873b55..9c2b46b 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -15,7 +15,6 @@
 #include <linux/magic.h>
 #include <linux/ima.h>
 #include <linux/evm.h>
-#include <crypto/hash_info.h>

 #include "ima.h"

@@ -130,36 +129,40 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
        }
 }

-void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len,
-                      struct ima_digest_data *hash)
+enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
+                                int xattr_len)
 {
        struct signature_v2_hdr *sig;

        if (!xattr_value || xattr_len < 2)
-               return;
+               /* return default hash algo */
+               return ima_hash_algo;

        switch (xattr_value->type) {
        case EVM_IMA_XATTR_DIGSIG:
                sig = (typeof(sig))xattr_value;
                if (sig->version != 2 || xattr_len <= sizeof(*sig))
-                       return;
-               hash->algo = sig->hash_algo;
+                       return ima_hash_algo;
+               return sig->hash_algo;
                break;
        case IMA_XATTR_DIGEST_NG:
-               hash->algo = xattr_value->digest[0];
+               return xattr_value->digest[0];
                break;
        case IMA_XATTR_DIGEST:
                /* this is for backward compatibility */
                if (xattr_len == 21) {
                        unsigned int zero = 0;
                        if (!memcmp(&xattr_value->digest[16], &zero, 4))
-                               hash->algo = HASH_ALGO_MD5;
+                               return HASH_ALGO_MD5;
                        else
-                               hash->algo = HASH_ALGO_SHA1;
+                               return HASH_ALGO_SHA1;
                } else if (xattr_len == 17)
-                       hash->algo = HASH_ALGO_MD5;
+                       return HASH_ALGO_MD5;
                break;
        }
+
+       /* return default hash algo */
+       return ima_hash_algo;
 }

 int ima_read_xattr(struct dentry *dentry,

@@ -296,7 +299,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
        if (iint->flags & IMA_DIGSIG)
                return;

-       rc = ima_collect_measurement(iint, file, NULL, NULL);
+       rc = ima_collect_measurement(iint, file, ima_hash_algo);
        if (rc < 0)
                return;

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 6eb6293..fb30ce4 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c

@@ -24,7 +24,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <crypto/hash.h>
-#include <crypto/hash_info.h>
+
 #include "ima.h"

 struct ahash_completion {

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index bd79f25..5d679a6 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c

@@ -21,7 +21,7 @@
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/err.h>
-#include <crypto/hash_info.h>
+
 #include "ima.h"

 /* name for boot aggregate entry */

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c21f09b..d9fc463 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -24,7 +24,6 @@
 #include <linux/slab.h>
 #include <linux/xattr.h>
 #include <linux/ima.h>
-#include <crypto/hash_info.h>

 #include "ima.h"

@@ -163,9 +162,10 @@ static int process_measurement(struct file *file, int mask, int function,
        char *pathbuf = NULL;
        const char *pathname = NULL;
        int rc = -ENOMEM, action, must_appraise;
-       struct evm_ima_xattr_data *xattr_value = NULL, **xattr_ptr = NULL;
+       struct evm_ima_xattr_data *xattr_value = NULL;
        int xattr_len = 0;
        bool violation_check;
+       enum hash_algo hash_algo;

        if (!ima_policy_flag || !S_ISREG(inode->i_mode))
                return 0;

@@ -221,9 +221,12 @@ static int process_measurement(struct file *file, int mask, int function,
        template_desc = ima_template_desc_current();
        if ((action & IMA_APPRAISE_SUBMASK) ||
                    strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
-               xattr_ptr = &xattr_value;
+               /* read 'security.ima' */
+               xattr_len = ima_read_xattr(file->f_path.dentry, &xattr_value);

-       rc = ima_collect_measurement(iint, file, xattr_ptr, &xattr_len);
+       hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
+
+       rc = ima_collect_measurement(iint, file, hash_algo);
        if (rc != 0) {
                if (file->f_flags & O_DIRECT)
                        rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 0b7404e..febd12e 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c

@@ -15,8 +15,6 @@

 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

-#include <crypto/hash_info.h>
-
 #include "ima.h"
 #include "ima_template_lib.h"

diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 2934e3d..f9bae04 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c

@@ -12,7 +12,6 @@
  * File: ima_template_lib.c
  *      Library of supported template fields.
  */
-#include <crypto/hash_info.h>

 #include "ima_template_lib.h"

--

2.1.0



-- 
Thanks,
Dmitry

Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version

From: Kees Cook <hidden>
Date: 2016-01-20 00:10:37

On Mon, Jan 18, 2016 at 7:11 AM, Mimi Zohar [off-list ref] wrote:

Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function.  A benefit of calling kernel_read_file()
to read the firmware is the firmware is read only once, instead of once
for measuring/appraising the firmware and again for reading the file
contents into memory.

This patch retains the kernel_fw_from_file() hook, which is called from
security_kernel_post_read_file(), but removes the
sercurity_kernel_fw_from_file() function.

Changelog:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)

Signed-off-by: Mimi Zohar <redacted>

Reviewed-by: Kees Cook <redacted>

-Kees

quoted hunk

---
 drivers/base/firmware_class.c         | 48 ++++++++++-------------------------
 include/linux/ima.h                   |  7 +----
 include/linux/security.h              |  8 +-----
 security/integrity/ima/ima.h          |  1 -
 security/integrity/ima/ima_appraise.c |  8 ------
 security/integrity/ima/ima_main.c     | 18 +++++--------
 security/integrity/ima/ima_policy.c   | 26 +++++++++----------
 security/integrity/integrity.h        | 11 +++-----
 security/security.c                   | 28 ++++++++++----------
 9 files changed, 54 insertions(+), 101 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8524450..cc175f1 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c

@@ -29,6 +29,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/reboot.h>
 #include <linux/security.h>
+#include <linux/ima.h>

 #include <generated/utsrelease.h>

@@ -291,40 +292,10 @@ static const char * const fw_path[] = {
 module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");

-static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
-{
-       int size;
-       char *buf;
-       int rc;
-
-       if (!S_ISREG(file_inode(file)->i_mode))
-               return -EINVAL;
-       size = i_size_read(file_inode(file));
-       if (size <= 0)
-               return -EINVAL;
-       buf = vmalloc(size);
-       if (!buf)
-               return -ENOMEM;
-       rc = kernel_read(file, 0, buf, size);
-       if (rc != size) {
-               if (rc > 0)
-                       rc = -EIO;
-               goto fail;
-       }
-       rc = security_kernel_fw_from_file(file, buf, size);
-       if (rc)
-               goto fail;
-       fw_buf->data = buf;
-       fw_buf->size = size;
-       return 0;
-fail:
-       vfree(buf);
-       return rc;
-}
-
 static int fw_get_filesystem_firmware(struct device *device,
                                       struct firmware_buf *buf)
 {
+       loff_t size;
        int i, len;
        int rc = -ENOENT;
        char *path;

@@ -350,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
                file = filp_open(path, O_RDONLY, 0);
                if (IS_ERR(file))
                        continue;
-               rc = fw_read_file_contents(file, buf);
+
+               buf->size = 0;
+               rc = kernel_read_file(file, &buf->data, &size, INT_MAX,
+                                     FIRMWARE_CHECK);
                fput(file);
                if (rc)
                        dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n",
                                path, rc);
-               else
+               else {
+                       buf->size = (size_t) size;
                        break;
+               }
        }
        __putname(path);

@@ -685,8 +661,10 @@ static ssize_t firmware_loading_store(struct device *dev,
                                dev_err(dev, "%s: map pages failed\n",
                                        __func__);
                        else
-                               rc = security_kernel_fw_from_file(NULL,
-                                               fw_buf->data, fw_buf->size);
+                               rc = security_kernel_post_read_file(NULL,
+                                                              fw_buf->data,
+                                                              fw_buf->size,
+                                                              FIRMWARE_CHECK);

                        /*
                         * Same logic as fw_load_abort, only the DONE bit

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ae91938..0a7f039 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -16,6 +16,7 @@ struct linux_binprm;
 enum ima_policy_id {
        KEXEC_CHECK = 1,
        INITRAMFS_CHECK,
+       FIRMWARE_CHECK,
        IMA_MAX_READ_CHECK
 };

@@ -25,7 +26,6 @@ extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_module_check(struct file *file);
-extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
 extern int ima_hash_and_process_file(struct file *file,
                                     void *buf, loff_t size,
                                     enum ima_policy_id policy_id);

@@ -56,11 +56,6 @@ static inline int ima_module_check(struct file *file)
        return 0;
 }

-static inline int ima_fw_from_file(struct file *file, char *buf, size_t size)
-{
-       return 0;
-}
-
 static inline int ima_hash_and_process_file(struct file *file,
                                            void *buf, loff_t size,
                                            enum ima_policy_id policy_id)

diff --git a/include/linux/security.h b/include/linux/security.h
index 44d8832..51f3bc6 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h

@@ -28,6 +28,7 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/mm.h>
+#include <linux/ima.h>

 struct linux_binprm;
 struct cred;

@@ -298,7 +299,6 @@ int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
 void security_transfer_creds(struct cred *new, const struct cred *old);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
-int security_kernel_fw_from_file(struct file *file, char *buf, size_t size);
 int security_kernel_module_request(char *kmod_name);
 int security_kernel_module_from_file(struct file *file);
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,

@@ -852,12 +852,6 @@ static inline int security_kernel_create_files_as(struct cred *cred,
        return 0;
 }

-static inline int security_kernel_fw_from_file(struct file *file,
-                                              char *buf, size_t size)
-{
-       return 0;
-}
-
 static inline int security_kernel_module_request(char *kmod_name)
 {
        return 0;

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b98dbd5..520c7b4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -165,7 +165,6 @@ enum ima_hooks {
        MMAP_CHECK,
        BPRM_CHECK,
        MODULE_CHECK,
-       FIRMWARE_CHECK,
        POST_SETATTR
 };

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 3adf937..57b1ad1 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -76,8 +76,6 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
                return iint->ima_bprm_status;
        case MODULE_CHECK:
                return iint->ima_module_status;
-       case FIRMWARE_CHECK:
-               return iint->ima_firmware_status;
        case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
                return iint->ima_read_status;
        case FILE_CHECK:

@@ -99,9 +97,6 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
        case MODULE_CHECK:
                iint->ima_module_status = status;
                break;
-       case FIRMWARE_CHECK:
-               iint->ima_firmware_status = status;
-               break;
        case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
                iint->ima_read_status = status;
                break;

@@ -124,9 +119,6 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
        case MODULE_CHECK:
                iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED);
                break;
-       case FIRMWARE_CHECK:
-               iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED);
-               break;
        case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
                break;
        case FILE_CHECK:

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 668cbc6..1251882 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -337,17 +337,6 @@ int ima_module_check(struct file *file)
        return process_measurement(file, NULL, 0, MAY_EXEC, MODULE_CHECK, 0);
 }

-int ima_fw_from_file(struct file *file, char *buf, size_t size)
-{
-       if (!file) {
-               if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
-                   (ima_appraise & IMA_APPRAISE_ENFORCE))
-                       return -EACCES; /* INTEGRITY_UNKNOWN */
-               return 0;
-       }
-       return process_measurement(file, NULL, 0, MAY_EXEC, FIRMWARE_CHECK, 0);
-}
-
 /**
  * ima_hash_and_process_file - in memory collect/appraise/audit measurement
  * @file: pointer to the file to be measured/appraised/audit

@@ -361,6 +350,13 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size)
 int ima_hash_and_process_file(struct file *file, void *buf, loff_t size,
                              enum ima_policy_id policy_id)
 {
+       if (!file && policy_id == FIRMWARE_CHECK) {
+               if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
+                   (ima_appraise & IMA_APPRAISE_ENFORCE))
+                       return -EACCES; /* INTEGRITY_UNKNOWN */
+               return 0;
+       }
+
        if (!file || !buf || size == 0) { /* should never happen */
                if (ima_appraise & IMA_APPRAISE_ENFORCE)
                        return -EACCES;

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 4711083..dbd7aa1 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -102,7 +102,8 @@ static struct ima_rule_entry original_measurement_rules[] = {
        {.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
         .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID},
        {.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
-       {.action = MEASURE, .hooks.func = FIRMWARE_CHECK, .flags = IMA_FUNC},
+       {.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
+        .flags = IMA_FUNC},
 };

 static struct ima_rule_entry default_measurement_rules[] = {

@@ -115,7 +116,8 @@ static struct ima_rule_entry default_measurement_rules[] = {
        {.action = MEASURE, .hooks.func = FILE_CHECK, .mask = MAY_READ,
         .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_INMASK | IMA_UID},
        {.action = MEASURE, .hooks.func = MODULE_CHECK, .flags = IMA_FUNC},
-       {.action = MEASURE, .hooks.func = FIRMWARE_CHECK, .flags = IMA_FUNC},
+       {.action = MEASURE, .hooks.policy_id = FIRMWARE_CHECK,
+        .flags = IMA_FUNC},
 };

 static struct ima_rule_entry default_appraise_rules[] = {

@@ -304,8 +306,6 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
                return IMA_BPRM_APPRAISE;
        case MODULE_CHECK:
                return IMA_MODULE_APPRAISE;
-       case FIRMWARE_CHECK:
-               return IMA_FIRMWARE_APPRAISE;
        case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
                return IMA_READ_APPRAISE;
        case FILE_CHECK:

@@ -609,8 +609,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
                                entry->hooks.func = FILE_CHECK;
                        else if (strcmp(args[0].from, "MODULE_CHECK") == 0)
                                entry->hooks.func = MODULE_CHECK;
-                       else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0)
-                               entry->hooks.func = FIRMWARE_CHECK;
                        else if ((strcmp(args[0].from, "FILE_MMAP") == 0)
                                || (strcmp(args[0].from, "MMAP_CHECK") == 0))
                                entry->hooks.func = MMAP_CHECK;

@@ -620,6 +618,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
                                entry->hooks.policy_id = KEXEC_CHECK;
                        else if (strcmp(args[0].from, "INITRAMFS_CHECK") == 0)
                                entry->hooks.policy_id = INITRAMFS_CHECK;
+                       else if (strcmp(args[0].from, "FIRMWARE_CHECK") == 0)
+                               entry->hooks.policy_id = FIRMWARE_CHECK;
                        else
                                result = -EINVAL;
                        if (!result)

@@ -776,7 +776,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
                result = -EINVAL;
        else if (entry->hooks.func == MODULE_CHECK)
                temp_ima_appraise |= IMA_APPRAISE_MODULES;
-       else if (entry->hooks.func == FIRMWARE_CHECK)
+       else if (entry->hooks.policy_id == FIRMWARE_CHECK)
                temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
        audit_log_format(ab, "res=%d", !result);
        audit_log_end(ab);

@@ -863,8 +863,8 @@ static char *mask_tokens[] = {

 enum {
        func_file = 0, func_mmap, func_bprm,
-       func_module, func_firmware, func_post,
-       func_kexec, func_initramfs
+       func_module, func_post,
+       func_kexec, func_initramfs, func_firmware
 };

 static char *func_tokens[] = {

@@ -872,10 +872,10 @@ static char *func_tokens[] = {
        "MMAP_CHECK",
        "BPRM_CHECK",
        "MODULE_CHECK",
-       "FIRMWARE_CHECK",
        "POST_SETATTR",
        "KEXEC_CHECK",
        "INITRAMFS_CHECK",
+       "FIRMWARE_CHECK"
 };

 void *ima_policy_start(struct seq_file *m, loff_t *pos)

@@ -949,9 +949,6 @@ int ima_policy_show(struct seq_file *m, void *v)
                case MODULE_CHECK:
                        seq_printf(m, pt(Opt_func), ft(func_module));
                        break;
-               case FIRMWARE_CHECK:
-                       seq_printf(m, pt(Opt_func), ft(func_firmware));
-                       break;
                case POST_SETATTR:
                        seq_printf(m, pt(Opt_func), ft(func_post));
                        break;

@@ -963,6 +960,9 @@ int ima_policy_show(struct seq_file *m, void *v)
                        case INITRAMFS_CHECK:
                                seq_printf(m, pt(Opt_func), ft(func_initramfs));
                                break;
+                       case FIRMWARE_CHECK:
+                               seq_printf(m, pt(Opt_func), ft(func_firmware));
+                               break;
                        default:
                                snprintf(tbuf, sizeof(tbuf), "%d",
                                         entry->hooks.func);

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 9a0ea4c..75334cd 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h

@@ -47,16 +47,14 @@
 #define IMA_BPRM_APPRAISED     0x00002000
 #define IMA_MODULE_APPRAISE    0x00004000
 #define IMA_MODULE_APPRAISED   0x00008000
-#define IMA_FIRMWARE_APPRAISE  0x00010000
-#define IMA_FIRMWARE_APPRAISED 0x00020000
-#define IMA_READ_APPRAISE      0x00040000
-#define IMA_READ_APPRAISED     0x00080000
+#define IMA_READ_APPRAISE      0x00010000
+#define IMA_READ_APPRAISED     0x00020000
 #define IMA_APPRAISE_SUBMASK   (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
                                 IMA_BPRM_APPRAISE | IMA_MODULE_APPRAISE | \
-                                IMA_FIRMWARE_APPRAISE | IMA_READ_APPRAISE)
+                                IMA_READ_APPRAISE)
 #define IMA_APPRAISED_SUBMASK  (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
                                 IMA_BPRM_APPRAISED | IMA_MODULE_APPRAISED | \
-                                IMA_FIRMWARE_APPRAISED | IMA_READ_APPRAISED)
+                                IMA_READ_APPRAISED)

 enum evm_ima_xattr_type {
        IMA_XATTR_DIGEST = 0x01,

@@ -112,7 +110,6 @@ struct integrity_iint_cache {
        enum integrity_status ima_mmap_status:4;
        enum integrity_status ima_bprm_status:4;
        enum integrity_status ima_module_status:4;
-       enum integrity_status ima_firmware_status:4;
        enum integrity_status ima_read_status:4;
        enum integrity_status evm_status:4;
        struct ima_digest_data *ima_hash;

diff --git a/security/security.c b/security/security.c
index 49cacae..a391ce4 100644
--- a/security/security.c
+++ b/security/security.c

@@ -884,17 +884,6 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
        return call_int_hook(kernel_create_files_as, 0, new, inode);
 }

-int security_kernel_fw_from_file(struct file *file, char *buf, size_t size)
-{
-       int ret;
-
-       ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
-       if (ret)
-               return ret;
-       return ima_fw_from_file(file, buf, size);
-}
-EXPORT_SYMBOL_GPL(security_kernel_fw_from_file);
-
 int security_kernel_module_request(char *kmod_name)
 {
        return call_int_hook(kernel_module_request, 0, kmod_name);

@@ -913,8 +902,21 @@ int security_kernel_module_from_file(struct file *file)
 int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
                                   int policy_id)
 {
-       return = call_int_hook(kernel_post_read_file, 0, file, buf, size,
-                              policy_id);
+       int ret = 0;
+
+       switch (policy_id) {
+       case FIRMWARE_CHECK:
+               ret = call_int_hook(kernel_fw_from_file, 0, file, buf, size);
+               break;
+       default:
+               ret = call_int_hook(kernel_post_read_file, 0, file, buf, size,
+                                   policy_id);
+               break;
+       }
+       if (ret)
+               return ret;
+
+       return ima_hash_and_process_file(file, buf, size, policy_id);
 }
 EXPORT_SYMBOL_GPL(security_kernel_post_read_file);

--

2.1.0



-- 
Kees Cook
Chrome OS & Brillo Security

Re: [RFC PATCH v2 02/11] vfs: define a generic function to read a file from the kernel

From: Luis R. Rodriguez <hidden>
Date: 2016-01-20 01:10:00

On Mon, Jan 18, 2016 at 10:11:17AM -0500, Mimi Zohar wrote:

quoted hunk

diff --git a/fs/exec.c b/fs/exec.c
index b06623a..6d623c2 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -831,6 +832,58 @@ int kernel_read(struct file *file, loff_t offset,
 
 EXPORT_SYMBOL(kernel_read);
 
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+		     loff_t max_size)
+{
+	loff_t i_size, pos;
+	ssize_t bytes = 0;
+	int ret;
+
+	if (!S_ISREG(file_inode(file)->i_mode))
+		return -EINVAL;
+
+	i_size = i_size_read(file_inode(file));
+	if (max_size > 0 && i_size > max_size)
+		return -EFBIG;

loff_t is a __kernel_loff_t, which in turn is a long long, and that's
signed. We don't catch a negative value here, for max_size, we could
return -EINVAL if its < 0.

+	if (i_size == 0)
+		return -EINVAL;

Likewise for i_size. The setter of the size will depend on how the
code calling this routine setup the struct file passed.

So how about adding a i_size <= 0 check here as well here?
At least fw_read_file_contents() has historically done this,
so if this generic read is going to skip that I'd like to
see why. We're unifying so I rather be more pedantic.

Provided this is addressed feel free to peg:

Reviewed-by: Luis R. Rodriguez <redacted>

  Luis

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Minfei Huang <hidden>
Date: 2016-01-20 03:19:58

On 01/18/16 at 10:11am, Mimi Zohar wrote:

quoted hunk

diff --git a/fs/exec.c b/fs/exec.c
index 211b81c..a5ae51e 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -884,6 +884,21 @@ out:
 }
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+			     int policy_id)
+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;
+
+	if (!f.file)
+		goto out;

It is no need to call the fdput, if f.file is NULL. It is better to
return it directly.

+
+	ret = kernel_read_file(f.file, buf, size, max_size, policy_id);
+out:
+	fdput(f);
+	return ret;
+}
+

Thanks
Minfei

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-20 23:13:04

On Mon, Jan 18, 2016 at 10:11:21AM -0500, Mimi Zohar wrote:

quoted hunk

diff --git a/fs/exec.c b/fs/exec.c
index 211b81c..a5ae51e 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -884,6 +884,21 @@ out:
 }
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+			     int policy_id)
+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;
+
+	if (!f.file)
+		goto out;
+
+	ret = kernel_read_file(f.file, buf, size, max_size, policy_id);
+out:
+	fdput(f);
+	return ret;
+}
+

Don't you need to EXPORT_SYMBOL_GPL() here as well?

quoted hunk

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 4edf47f..3adf937 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -78,6 +78,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_module_status;
 	case FIRMWARE_CHECK:
 		return iint->ima_firmware_status;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		return iint->ima_read_status;

I didn't get the memo that we're OK to use compiler specific extensions
like this. I'm sure if you are using it its not the first case, just
want to be sure we are aware of possible issues if some compiler doesn't
support this.

If we don't have a precedence can we just avoid its use?

Cc'd Julia in case this might be of interest for Coccinelle to grok.

quoted hunk

 	case FILE_CHECK:
 	default:
 		return iint->ima_file_status;

@@ -100,6 +102,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case FIRMWARE_CHECK:
 		iint->ima_firmware_status = status;
 		break;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		iint->ima_read_status = status;
+		break;
 	case FILE_CHECK:
 	default:
 		iint->ima_file_status = status;

Likewise.

quoted hunk

@@ -122,6 +127,8 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
 	case FIRMWARE_CHECK:
 		iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED);
 		break;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		break;
 	case FILE_CHECK:
 	default:
 		iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);

Likewise.

quoted hunk

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 595e038..4711083 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -306,6 +306,8 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
 		return IMA_MODULE_APPRAISE;
 	case FIRMWARE_CHECK:
 		return IMA_FIRMWARE_APPRAISE;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		return IMA_READ_APPRAISE;
 	case FILE_CHECK:
 	default:
 		return IMA_FILE_APPRAISE;

Likewise.

quoted hunk

@@ -948,10 +956,19 @@ int ima_policy_show(struct seq_file *m, void *v)
 			seq_printf(m, pt(Opt_func), ft(func_post));
 			break;
 		default:
-			snprintf(tbuf, sizeof(tbuf), "%d",
-				 entry->hooks.func);
-			seq_printf(m, pt(Opt_func), tbuf);
-			break;
+			switch (entry->hooks.policy_id) {
+			case KEXEC_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_kexec));
+				break;
+			case INITRAMFS_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_initramfs));
+				break;
+			default:
+				snprintf(tbuf, sizeof(tbuf), "%d",
+					 entry->hooks.func);
+				seq_printf(m, pt(Opt_func), tbuf);
+				break;
+			}
 		}
 		seq_puts(m, " ");
 	}

Two switches wrapped tend to lead to messy and hard to read code,
is using a function here better?

  Luis

Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-20 23:39:42

On Mon, Jan 18, 2016 at 10:11:22AM -0500, Mimi Zohar wrote:

quoted hunk

Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function.  A benefit of calling kernel_read_file()
to read the firmware is the firmware is read only once, instead of once
for measuring/appraising the firmware and again for reading the file
contents into memory.

This patch retains the kernel_fw_from_file() hook, which is called from
security_kernel_post_read_file(), but removes the
sercurity_kernel_fw_from_file() function.

Changelog:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)

Signed-off-by: Mimi Zohar <redacted>
---
 drivers/base/firmware_class.c         | 48 ++++++++++-------------------------
 include/linux/ima.h                   |  7 +----
 include/linux/security.h              |  8 +-----
 security/integrity/ima/ima.h          |  1 -
 security/integrity/ima/ima_appraise.c |  8 ------
 security/integrity/ima/ima_main.c     | 18 +++++--------
 security/integrity/ima/ima_policy.c   | 26 +++++++++----------
 security/integrity/integrity.h        | 11 +++-----
 security/security.c                   | 28 ++++++++++----------
 9 files changed, 54 insertions(+), 101 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8524450..cc175f1 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c

@@ -29,6 +29,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/reboot.h>
 #include <linux/security.h>
+#include <linux/ima.h>
 
 #include <generated/utsrelease.h>

@@ -291,40 +292,10 @@ static const char * const fw_path[] = {
 module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
 
-static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
-{
-	int size;
-	char *buf;
-	int rc;
-
-	if (!S_ISREG(file_inode(file)->i_mode))
-		return -EINVAL;
-	size = i_size_read(file_inode(file));
-	if (size <= 0)
-		return -EINVAL;
-	buf = vmalloc(size);
-	if (!buf)
-		return -ENOMEM;
-	rc = kernel_read(file, 0, buf, size);
-	if (rc != size) {
-		if (rc > 0)
-			rc = -EIO;
-		goto fail;
-	}
-	rc = security_kernel_fw_from_file(file, buf, size);
-	if (rc)
-		goto fail;
-	fw_buf->data = buf;
-	fw_buf->size = size;
-	return 0;
-fail:
-	vfree(buf);
-	return rc;
-}
-
 static int fw_get_filesystem_firmware(struct device *device,
 				       struct firmware_buf *buf)
 {
+	loff_t size;
 	int i, len;
 	int rc = -ENOENT;
 	char *path;

@@ -350,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
 		file = filp_open(path, O_RDONLY, 0);
 		if (IS_ERR(file))
 			continue;
-		rc = fw_read_file_contents(file, buf);
+
+		buf->size = 0;
+		rc = kernel_read_file(file, &buf->data, &size, INT_MAX,
+				      FIRMWARE_CHECK);

The way kernel firmware signing was implemented was that we'd first read the
foo.sig (or whatever extension we use). The same kernel_read_file() would be
used if this gets applied so this would still works well with that. Of course
folks using IMA and their own security policy would just disable the kernel
fw signing facility.

quoted hunk

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ae91938..0a7f039 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -16,6 +16,7 @@ struct linux_binprm;
 enum ima_policy_id {
 	KEXEC_CHECK = 1,
 	INITRAMFS_CHECK,
+	FIRMWARE_CHECK,
 	IMA_MAX_READ_CHECK
 };

The only thing that is worth questioning here in light of kernel fw signing is
what int policy_id do we use? Would we be OK to add FIRMWARE_SIG_CHECK later 
While at it, perhaps kernel_read_file() last argument should be enum
ima_policy_id then. If the policy_id is going to be used elsewhere outside of
IMA then perhaps ima.h is not the best place for it ? Would fs.h for type of
file be OK ? Then we'd have a list of known file types the kernel reads.

  Luis

Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-20 23:57:05

On Wed, Jan 20, 2016 at 3:39 PM, Luis R. Rodriguez [off-list ref] wrote:

On Mon, Jan 18, 2016 at 10:11:22AM -0500, Mimi Zohar wrote:

quoted

Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function.  A benefit of calling kernel_read_file()
to read the firmware is the firmware is read only once, instead of once
for measuring/appraising the firmware and again for reading the file
contents into memory.

This patch retains the kernel_fw_from_file() hook, which is called from
security_kernel_post_read_file(), but removes the
sercurity_kernel_fw_from_file() function.

Changelog:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)

Signed-off-by: Mimi Zohar <redacted>
---
 drivers/base/firmware_class.c         | 48 ++++++++++-------------------------
 include/linux/ima.h                   |  7 +----
 include/linux/security.h              |  8 +-----
 security/integrity/ima/ima.h          |  1 -
 security/integrity/ima/ima_appraise.c |  8 ------
 security/integrity/ima/ima_main.c     | 18 +++++--------
 security/integrity/ima/ima_policy.c   | 26 +++++++++----------
 security/integrity/integrity.h        | 11 +++-----
 security/security.c                   | 28 ++++++++++----------
 9 files changed, 54 insertions(+), 101 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8524450..cc175f1 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c

@@ -29,6 +29,7 @@
 #include <linux/syscore_ops.h>
 #include <linux/reboot.h>
 #include <linux/security.h>
+#include <linux/ima.h>

 #include <generated/utsrelease.h>

@@ -291,40 +292,10 @@ static const char * const fw_path[] = {
 module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");

-static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
-{
-     int size;
-     char *buf;
-     int rc;
-
-     if (!S_ISREG(file_inode(file)->i_mode))
-             return -EINVAL;
-     size = i_size_read(file_inode(file));
-     if (size <= 0)
-             return -EINVAL;
-     buf = vmalloc(size);
-     if (!buf)
-             return -ENOMEM;
-     rc = kernel_read(file, 0, buf, size);
-     if (rc != size) {
-             if (rc > 0)
-                     rc = -EIO;
-             goto fail;
-     }
-     rc = security_kernel_fw_from_file(file, buf, size);
-     if (rc)
-             goto fail;
-     fw_buf->data = buf;
-     fw_buf->size = size;
-     return 0;
-fail:
-     vfree(buf);
-     return rc;
-}
-
 static int fw_get_filesystem_firmware(struct device *device,
                                     struct firmware_buf *buf)
 {
+     loff_t size;
      int i, len;
      int rc = -ENOENT;
      char *path;

@@ -350,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
              file = filp_open(path, O_RDONLY, 0);
              if (IS_ERR(file))
                      continue;
-             rc = fw_read_file_contents(file, buf);
+
+             buf->size = 0;
+             rc = kernel_read_file(file, &buf->data, &size, INT_MAX,
+                                   FIRMWARE_CHECK);

The way kernel firmware signing was implemented was that we'd first read the
foo.sig (or whatever extension we use). The same kernel_read_file() would be
used if this gets applied so this would still works well with that. Of course
folks using IMA and their own security policy would just disable the kernel
fw signing facility.

quoted

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ae91938..0a7f039 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -16,6 +16,7 @@ struct linux_binprm;
 enum ima_policy_id {
      KEXEC_CHECK = 1,
      INITRAMFS_CHECK,
+     FIRMWARE_CHECK,
      IMA_MAX_READ_CHECK
 };

The only thing that is worth questioning here in light of kernel fw signing is
what int policy_id do we use? Would we be OK to add FIRMWARE_SIG_CHECK later
While at it, perhaps kernel_read_file() last argument should be enum
ima_policy_id then. If the policy_id is going to be used elsewhere outside of
IMA then perhaps ima.h is not the best place for it ? Would fs.h for type of
file be OK ? Then we'd have a list of known file types the kernel reads.

Actually your patch #9 "ima: load policy using path" defines
kernel_read_file_from_path and since the firmware code uses a path
this code would be much cleaner if instead you used that. It'd mean
more code sharing and would make firmware code cleaner. Could you
re-order that to go first and then later this as its first user?
Perhaps add the helper for the firmware patch.

 Luis

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-21 00:03:23

On Mon, Jan 18, 2016 at 10:11:23AM -0500, Mimi Zohar wrote:

This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function.  Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.

This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file.  For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.

This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.

I think it would help if your cover letter and this patch described
a bit that some LSMs either prefer to read / check / appraise files
prior to loading and some other prefer to do that later. You could
explain the LSM hook preferences and what they do. Then here you
can explain how this one prefers a hook early, but acknowledge that
the other one still exists.

So:

kernel_read_file() {
	...
	security_kernel_read_file();
	...
	security_kernel_post_read_file();
	...
}

quoted hunk

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4e6e2af..9915310 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -1465,6 +1471,7 @@ union security_list_options {
 	int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
 	int (*kernel_module_request)(char *kmod_name);
 	int (*kernel_module_from_file)(struct file *file);
+	int (*kernel_read_file)(struct file *file, int policy_id);
 	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
 				     int policy_id);
 	int (*task_fix_setuid)(struct cred *new, const struct cred *old,

Is the goal to eventually kill the other LSM hooks and just keep the
file one? If so where is that done in this series? It was not clear.

  Luis

Re: [RFC PATCH v2 09/11] ima: load policy using path

From: Luis R. Rodriguez <hidden>
Date: 2016-01-21 00:05:59

On Mon, Jan 18, 2016 at 10:11:24AM -0500, Mimi Zohar wrote:

quoted hunk

--- a/fs/exec.c
+++ b/fs/exec.c

@@ -903,6 +903,27 @@ out:
 	return ret;
 }
 
+int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+			       loff_t max_size, int policy_id)
+{
+	struct file *file;
+	int ret;
+
+	if (!path || !*path)
+		return -EINVAL;
+
+	file = filp_open(path, O_RDONLY, 0);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		pr_err("Unable to open file: %s (%d)", path, ret);
+		return ret;
+	}
+
+	ret = kernel_read_file(file, buf, size, max_size, policy_id);
+	fput(file);
+	return ret;
+}
+

EXPORT_SYMBOL_GPL() needed.

  Luis

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2016-01-21 00:27:53

On Thu, Jan 21, 2016 at 12:12:40AM +0100, Luis R. Rodriguez wrote:

On Mon, Jan 18, 2016 at 10:11:21AM -0500, Mimi Zohar wrote:

quoted

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 4edf47f..3adf937 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -78,6 +78,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_module_status;
 	case FIRMWARE_CHECK:
 		return iint->ima_firmware_status;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		return iint->ima_read_status;

I didn't get the memo that we're OK to use compiler specific extensions
like this. I'm sure if you are using it its not the first case, just
want to be sure we are aware of possible issues if some compiler doesn't
support this.

If we don't have a precedence can we just avoid its use?

This has sailed:

dtor@dtor-ws:~$ grep -rl 'case.*\.\.\..*:' kernel/work/drivers/ | wc -l
98

Thanks.

-- 
Dmitry

Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-21 12:06:15

On Tue, 2016-01-19 at 16:10 -0800, Kees Cook wrote:

On Mon, Jan 18, 2016 at 7:11 AM, Mimi Zohar [off-list ref] wrote:

quoted

Replace fw_read_file_contents() for reading a file with the common VFS
kernel_read_file() function.  A benefit of calling kernel_read_file()
to read the firmware is the firmware is read only once, instead of once
for measuring/appraising the firmware and again for reading the file
contents into memory.

This patch retains the kernel_fw_from_file() hook, which is called from
security_kernel_post_read_file(), but removes the
sercurity_kernel_fw_from_file() function.

Changelog:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)

Signed-off-by: Mimi Zohar <redacted>

Reviewed-by: Kees Cook <redacted>

Thanks!

Mimi

Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-21 12:07:18

On Wed, 2016-01-20 at 15:56 -0800, Luis R. Rodriguez wrote:

On Wed, Jan 20, 2016 at 3:39 PM, Luis R. Rodriguez [off-list ref] wrote:

quoted

@@ -350,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
              file = filp_open(path, O_RDONLY, 0);
              if (IS_ERR(file))
                      continue;
-             rc = fw_read_file_contents(file, buf);
+
+             buf->size = 0;
+             rc = kernel_read_file(file, &buf->data, &size, INT_MAX,
+                                   FIRMWARE_CHECK);

The way kernel firmware signing was implemented was that we'd first read the
foo.sig (or whatever extension we use).

Was there a reason for using a detached signature and not using the same
method as kernel modules?

The same kernel_read_file() would be

quoted

used if this gets applied so this would still works well with that. Of course
folks using IMA and their own security policy would just disable the kernel
fw signing facility.

Right, support for not measuring/appraising the firmware and sig would
be supported in the policy.

quoted

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ae91938..0a7f039 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -16,6 +16,7 @@ struct linux_binprm;
 enum ima_policy_id {
      KEXEC_CHECK = 1,
      INITRAMFS_CHECK,
+     FIRMWARE_CHECK,
      IMA_MAX_READ_CHECK
 };

The only thing that is worth questioning here in light of kernel fw signing is
what int policy_id do we use? Would we be OK to add FIRMWARE_SIG_CHECK later
While at it, perhaps kernel_read_file() last argument should be enum
ima_policy_id then. If the policy_id is going to be used elsewhere outside of
IMA then perhaps ima.h is not the best place for it ? Would fs.h for type of
file be OK ? Then we'd have a list of known file types the kernel reads.

I would definitely prefer the enumeration be defined at the VFS layer.
For example, 

enum kernel_read_file_id {
        READING_KEXEC_IMAGE,
        READING_KEXEC_INITRAMFS,
        READING_FIRMWARE,
        READING_FIRMWARE_SIG,
        READING_MAX_ID
};

Agreed, the last field of kernel_read_file() and the wrappers should be
the enumeration.

Actually your patch #9 "ima: load policy using path" defines
kernel_read_file_from_path and since the firmware code uses a path
this code would be much cleaner if instead you used that. It'd mean
more code sharing and would make firmware code cleaner. Could you
re-order that to go first and then later this as its first user?
Perhaps add the helper for the firmware patch.

Thanks, I missed that.  I'll include this change in the next version.

Mimi

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-21 13:13:34

On Thu, 2016-01-21 at 01:03 +0100, Luis R. Rodriguez wrote:

On Mon, Jan 18, 2016 at 10:11:23AM -0500, Mimi Zohar wrote:

quoted

This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function.  Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.

This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file.  For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.

This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.

I think it would help if your cover letter and this patch described
a bit that some LSMs either prefer to read / check / appraise files
prior to loading and some other prefer to do that later. You could
explain the LSM hook preferences and what they do. Then here you
can explain how this one prefers a hook early, but acknowledge that
the other one still exists.

Before this patch set, IMA measured/appraised/audited a file before
allowing it to be accessed, causing the file in some cases to be read
twice.   This patch set changes that.  Files are read into memory and
then measured/appraised/audited.
 
It's been a while since this hook was added.  As I recall, Kees added
the pre module hook to limit loading kernel modules to only those
filesystems that were mounted read-only.  I would have to look at each
of the LSMs to see how they're using the hooks.

So:

kernel_read_file() {
	...
	security_kernel_read_file();
	...
	security_kernel_post_read_file();
	...
}

quoted

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4e6e2af..9915310 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -1465,6 +1471,7 @@ union security_list_options {
 	int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
 	int (*kernel_module_request)(char *kmod_name);
 	int (*kernel_module_from_file)(struct file *file);
+	int (*kernel_read_file)(struct file *file, int policy_id);
 	int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
 				     int policy_id);
 	int (*task_fix_setuid)(struct cred *new, const struct cred *old,

Is the goal to eventually kill the other LSM hooks and just keep the
file one? If so where is that done in this series? It was not clear.

As mentioned in the cover letter, consolidating the LSM hooks is not
covered in this patch set.  I was under the impression that not only
were we defining a common kernel read file function, but that we were
also consolidating the pre and post security hooks as well.  By defining
the pre and post security hooks in this patch set, it permits each of
the LSMs to migrate to the new hooks independently of each other.   Lets
ask the LSM maintainers what they think.

Paul, Casey, Kees, Jon, Tetsuo does it make sense to consolidate the
module, firmware, and kexec pre and post security hooks and have just
one set of pre and post security kernel_read_file hook instead?   Does
it make sense for this patch set to define the new hooks to allow the
LSMs to migrate to it independently of each other?

Mimi

Re: [RFC PATCH v2 03/11] ima: provide buffer hash calculation function

From: Mimi Zohar <hidden>
Date: 2016-01-21 13:19:32

On Tue, 2016-01-19 at 21:26 +0200, Dmitry Kasatkin wrote:

On Mon, Jan 18, 2016 at 5:11 PM, Mimi Zohar [off-list ref] wrote:

quoted

From: Dmitry Kasatkin <redacted>

This patch provides convenient buffer hash calculation function.

Changelog:
- rewrite to support loff_t sized buffers - Mimi
  (based on Fenguang Wu's testing)

Signed-off-by: Dmitry Kasatkin <redacted>
Signed-off-by: Mimi Zohar <redacted>
---
 security/integrity/ima/ima.h        |  2 ++
 security/integrity/ima/ima_crypto.c | 47 +++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fb8da36..de53631 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -107,6 +107,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
                           const char *op, struct inode *inode,
                           const unsigned char *filename);
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, loff_t len,
+                        struct ima_digest_data *hash);
 int ima_calc_field_array_hash(struct ima_field_data *field_data,
                              struct ima_template_desc *desc, int num_fields,
                              struct ima_digest_data *hash);

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index fb30ce4..8d86281 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c

@@ -519,6 +519,53 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
        return rc;
 }

+static int calc_buffer_shash_tfm(const void *buf, loff_t size,
+                               struct ima_digest_data *hash,
+                               struct crypto_shash *tfm)
+{
+       SHASH_DESC_ON_STACK(shash, tfm);
+       unsigned int len;
+       loff_t offset = 0;
+       int rc;
+
+       shash->tfm = tfm;
+       shash->flags = 0;
+
+       hash->length = crypto_shash_digestsize(tfm);
+
+       rc = crypto_shash_init(shash);
+       if (rc != 0)
+               return rc;
+
+       len = size < PAGE_SIZE ? size : PAGE_SIZE;
+       while (offset < size) {
+               rc = crypto_shash_update(shash, buf + offset, len);
+               if (rc)
+                       break;
+               offset += len;
+       }
+

Hello Mimi,

May be this was my earlier patch, but it seems to have a problem of
accessing beyond end of buffer using the same len.
When buffer always padded by zeros it is not a problem, but it is a bug.

This seems to be better version.

   while (size) {
               len = size < PAGE_SIZE ? size : PAGE_SIZE;
               rc = crypto_shash_update(shash, buf, len);
               if (rc)
                       break;
               buf += len;
               size -= len;
    }

Please change my sign-of to: dmitry.kasatkin@huawei.com

Good catch!  I think I unfortunately introduce this bug.  Thank you for
the review!

Mimi

Re: [RFC PATCH v2 01/11] ima: separate 'security.ima' reading functionality from collect

From: Mimi Zohar <hidden>
Date: 2016-01-21 13:20:55

On Tue, 2016-01-19 at 22:00 +0200, Dmitry Kasatkin wrote:

Hi Mimi,

Please change

Signed-off-by: Dmitry Kasatkin <redacted>

I'll make the change here and in the other patches as well.

Mimi

Re: [RFC PATCH v2 02/11] vfs: define a generic function to read a file from the kernel

From: Mimi Zohar <hidden>
Date: 2016-01-21 13:25:33

On Wed, 2016-01-20 at 02:09 +0100, Luis R. Rodriguez wrote:

On Mon, Jan 18, 2016 at 10:11:17AM -0500, Mimi Zohar wrote:

quoted

diff --git a/fs/exec.c b/fs/exec.c
index b06623a..6d623c2 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -831,6 +832,58 @@ int kernel_read(struct file *file, loff_t offset,
 
 EXPORT_SYMBOL(kernel_read);
 
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+		     loff_t max_size)
+{
+	loff_t i_size, pos;
+	ssize_t bytes = 0;
+	int ret;
+
+	if (!S_ISREG(file_inode(file)->i_mode))
+		return -EINVAL;
+
+	i_size = i_size_read(file_inode(file));
+	if (max_size > 0 && i_size > max_size)
+		return -EFBIG;

loff_t is a __kernel_loff_t, which in turn is a long long, and that's
signed. We don't catch a negative value here, for max_size, we could
return -EINVAL if its < 0.

quoted

+	if (i_size == 0)
+		return -EINVAL;

Likewise for i_size. The setter of the size will depend on how the
code calling this routine setup the struct file passed.

So how about adding a i_size <= 0 check here as well here?
At least fw_read_file_contents() has historically done this,
so if this generic read is going to skip that I'd like to
see why. We're unifying so I rather be more pedantic.

Provided this is addressed feel free to peg:

Reviewed-by: Luis R. Rodriguez <redacted>

Don't know how I missed that.  Thanks!

Mimi

Re: [RFC PATCH v2 09/11] ima: load policy using path

From: Mimi Zohar <hidden>
Date: 2016-01-21 13:25:34

On Thu, 2016-01-21 at 01:05 +0100, Luis R. Rodriguez wrote:

On Mon, Jan 18, 2016 at 10:11:24AM -0500, Mimi Zohar wrote:

quoted

--- a/fs/exec.c
+++ b/fs/exec.c

@@ -903,6 +903,27 @@ out:
 	return ret;
 }
 
+int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+			       loff_t max_size, int policy_id)
+{
+	struct file *file;
+	int ret;
+
+	if (!path || !*path)
+		return -EINVAL;
+
+	file = filp_open(path, O_RDONLY, 0);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		pr_err("Unable to open file: %s (%d)", path, ret);
+		return ret;
+	}
+
+	ret = kernel_read_file(file, buf, size, max_size, policy_id);
+	fput(file);
+	return ret;
+}
+

EXPORT_SYMBOL_GPL() needed.

Yes.  Thank you for reviewing all the patches!

Mimi

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Paul Moore <hidden>
Date: 2016-01-21 15:46:01

On Thursday, January 21, 2016 08:12:12 AM Mimi Zohar wrote:

Paul, Casey, Kees, Jon, Tetsuo does it make sense to consolidate the
module, firmware, and kexec pre and post security hooks and have just
one set of pre and post security kernel_read_file hook instead?   Does
it make sense for this patch set to define the new hooks to allow the
LSMs to migrate to it independently of each other?

Well, as usual, the easiest way to both get solid feedback and actually get a 
change accepted is to post patches to the affected LSMs.  Probably not what 
you wanted to hear, but at least I'm honest :)

-- 
paul moore
security @ redhat

Re: [RFC PATCH v2 07/11] firmware: replace call to fw_read_file_contents() with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-21 16:50:20

On Thu, Jan 21, 2016 at 4:05 AM, Mimi Zohar [off-list ref] wrote:

On Wed, 2016-01-20 at 15:56 -0800, Luis R. Rodriguez wrote:

quoted

On Wed, Jan 20, 2016 at 3:39 PM, Luis R. Rodriguez [off-list ref] wrote:

quoted

@@ -350,13 +321,18 @@ static int fw_get_filesystem_firmware(struct device *device,
              file = filp_open(path, O_RDONLY, 0);
              if (IS_ERR(file))
                      continue;
-             rc = fw_read_file_contents(file, buf);
+
+             buf->size = 0;
+             rc = kernel_read_file(file, &buf->data, &size, INT_MAX,
+                                   FIRMWARE_CHECK);

The way kernel firmware signing was implemented was that we'd first read the
foo.sig (or whatever extension we use).

Was there a reason for using a detached signature and not using the same
method as kernel modules?

Yes, the firmware might have different license. Its also easier for
users to use standard mechanisms to verify.

quoted

The same kernel_read_file() would be

quoted

used if this gets applied so this would still works well with that. Of course
folks using IMA and their own security policy would just disable the kernel
fw signing facility.

Right, support for not measuring/appraising the firmware and sig would
be supported in the policy.

OK!

quoted

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ae91938..0a7f039 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -16,6 +16,7 @@ struct linux_binprm;
 enum ima_policy_id {
      KEXEC_CHECK = 1,
      INITRAMFS_CHECK,
+     FIRMWARE_CHECK,
      IMA_MAX_READ_CHECK
 };

The only thing that is worth questioning here in light of kernel fw signing is
what int policy_id do we use? Would we be OK to add FIRMWARE_SIG_CHECK later
While at it, perhaps kernel_read_file() last argument should be enum
ima_policy_id then. If the policy_id is going to be used elsewhere outside of
IMA then perhaps ima.h is not the best place for it ? Would fs.h for type of
file be OK ? Then we'd have a list of known file types the kernel reads.

I would definitely prefer the enumeration be defined at the VFS layer.
For example,

enum kernel_read_file_id {
        READING_KEXEC_IMAGE,
        READING_KEXEC_INITRAMFS,
        READING_FIRMWARE,
        READING_FIRMWARE_SIG,
        READING_MAX_ID
};

Looks good to me. Please add a kdoc section to force us to have to
document each one.

Agreed, the last field of kernel_read_file() and the wrappers should be
the enumeration.

Great.

quoted

Actually your patch #9 "ima: load policy using path" defines
kernel_read_file_from_path and since the firmware code uses a path
this code would be much cleaner if instead you used that. It'd mean
more code sharing and would make firmware code cleaner. Could you
re-order that to go first and then later this as its first user?
Perhaps add the helper for the firmware patch.

Thanks, I missed that.  I'll include this change in the next version.

Great.

  Luis

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-21 16:57:33

On Thu, Jan 21, 2016 at 5:12 AM, Mimi Zohar [off-list ref] wrote:

On Thu, 2016-01-21 at 01:03 +0100, Luis R. Rodriguez wrote:

quoted

On Mon, Jan 18, 2016 at 10:11:23AM -0500, Mimi Zohar wrote:

quoted

This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function.  Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.

This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file.  For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.

This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.

I think it would help if your cover letter and this patch described
a bit that some LSMs either prefer to read / check / appraise files
prior to loading and some other prefer to do that later. You could
explain the LSM hook preferences and what they do. Then here you
can explain how this one prefers a hook early, but acknowledge that
the other one still exists.

Before this patch set, IMA measured/appraised/audited a file before
allowing it to be accessed, causing the file in some cases to be read
twice.   This patch set changes that.  Files are read into memory and
then measured/appraised/audited.

Sounds like this could help also with performance, has any preliminary
benchmarking been done to see the effect ?

It's been a while since this hook was added.  As I recall, Kees added
the pre module hook to limit loading kernel modules to only those
filesystems that were mounted read-only.  I would have to look at each
of the LSMs to see how they're using the hooks.

Sure.

quoted

So:

kernel_read_file() {
      ...
      security_kernel_read_file();
      ...
      security_kernel_post_read_file();
      ...
}

quoted

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4e6e2af..9915310 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h

@@ -1465,6 +1471,7 @@ union security_list_options {
    int (*kernel_fw_from_file)(struct file *file, char *buf, size_t size);
    int (*kernel_module_request)(char *kmod_name);
    int (*kernel_module_from_file)(struct file *file);
+   int (*kernel_read_file)(struct file *file, int policy_id);
    int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
                                 int policy_id);
    int (*task_fix_setuid)(struct cred *new, const struct cred *old,

Is the goal to eventually kill the other LSM hooks and just keep the
file one? If so where is that done in this series? It was not clear.

As mentioned in the cover letter, consolidating the LSM hooks is not
covered in this patch set.

Sorry I missed that after I started reviewing.

I was under the impression that not only
were we defining a common kernel read file function, but that we were
also consolidating the pre and post security hooks as well.

Sure.

By defining
the pre and post security hooks in this patch set, it permits each of
the LSMs to migrate to the new hooks independently of each other.   Lets
ask the LSM maintainers what they think.

I see -- yeah making this a 2 step thing makes sense, so long as the
maintainers can later expect / understand what would be done in a
second patch set. Breaking this down in two patch sets makes sense. It
should also mean there might be fun benchmarks on gains provided there
were considerable IO savings by not opening files twice.


  Luis

Re: [RFC PATCH v2 01/11] ima: separate 'security.ima' reading functionality from collect

From: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Date: 2016-01-21 18:19:07

On Thu, Jan 21, 2016 at 3:19 PM, Mimi Zohar [off-list ref] wrote:

On Tue, 2016-01-19 at 22:00 +0200, Dmitry Kasatkin wrote:

quoted

Hi Mimi,

Please change

Signed-off-by: Dmitry Kasatkin <redacted>

I'll make the change here and in the other patches as well.

Mimi

Thanks.

-- 
Thanks,
Dmitry

Re: [RFC PATCH v2 00/11] vfss: support for a common kernel file loader

From: Luis R. Rodriguez <hidden>
Date: 2016-01-21 20:17:17

On Mon, Jan 18, 2016 at 10:11:15AM -0500, Mimi Zohar wrote:

For a while it was looked down upon to directly read files from Linux.
These days there exists a few mechanisms in the kernel that do just this
though to load a file into a local buffer. There are minor but important
checks differences on each, we should take all the best practices from
each of them, generalize them and make all places in the kernel that
read a file use it.[1]

One difference is the method for opening the file.  In some cases we
have a file, while in other cases we have a pathname or a file descriptor.

Another difference is the security hook calls, or lack of them.  In
some versions there is a post file read hook, while in others there
is a pre file read hook.

This patch set is the first attempt at resolving these differences.  It
does not attempt to merge the different methods of opening a file, but
defines a single common kernel file read function with two wrappers.
Although this patch set defines two new security hooks for pre and post
file read, it does not attempt to merge the existing security hooks.
That is left as future work.

Changelog v2:
- Combined the "ima: measuring/appraising files read by the kernel" patches
with this patch set to simplify review.
- Split the "ima: measure and appraise kexec image and initramfs" patch to
separate IMA from the kexec changes.

The latest version of these patches can be found in the next-kernel-read-v2
branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

[1] Taken from Luis Rodriguez's wiki -
http://kernelnewbies.org/KernelProjects/common-kernel-loader

Did 0-day bot get a chance to test this tree? If not can it
be added ?

  Luis

Re: [RFC PATCH v2 00/11] vfss: support for a common kernel file loader

From: Mimi Zohar <hidden>
Date: 2016-01-21 20:20:32

On Thu, 2016-01-21 at 21:16 +0100, Luis R. Rodriguez wrote:

On Mon, Jan 18, 2016 at 10:11:15AM -0500, Mimi Zohar wrote:

quoted

The latest version of these patches can be found in the next-kernel-read-v2
branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

[1] Taken from Luis Rodriguez's wiki -
http://kernelnewbies.org/KernelProjects/common-kernel-loader

Did 0-day bot get a chance to test this tree? If not can it
be added ?

Yes, I get the notifications.

Mimi

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-21 20:39:24

On Thu, 2016-01-21 at 08:56 -0800, Luis R. Rodriguez wrote:

On Thu, Jan 21, 2016 at 5:12 AM, Mimi Zohar [off-list ref] wrote:

quoted

On Thu, 2016-01-21 at 01:03 +0100, Luis R. Rodriguez wrote:

quoted

On Mon, Jan 18, 2016 at 10:11:23AM -0500, Mimi Zohar wrote:

quoted

This patch replaces the module copy_module_from_fd() call with the VFS
common kernel_read_file_from_fd() function.  Instead of reading the
kernel module twice, once for measuring/appraising and then loading
the kernel module, the file is read once.

This patch defines a new security hook named security_kernel_read_file(),
which is called before reading the file.  For now, call the module
security hook from security_kernel_read_file until the LSMs have been
converted to use the kernel_read_file hook.

This patch retains the kernel_module_from_file hook, but removes the
security_kernel_module_from_file() function.

I think it would help if your cover letter and this patch described
a bit that some LSMs either prefer to read / check / appraise files
prior to loading and some other prefer to do that later. You could
explain the LSM hook preferences and what they do. Then here you
can explain how this one prefers a hook early, but acknowledge that
the other one still exists.

Before this patch set, IMA measured/appraised/audited a file before
allowing it to be accessed, causing the file in some cases to be read
twice.   This patch set changes that.  Files are read into memory and
then measured/appraised/audited.

Sounds like this could help also with performance, has any preliminary
benchmarking been done to see the effect ?

In general, IMA's pre-reading a file has negligible performance impact,
if any.   Dmitry's LinuxCon 2013 Europe talk "Integrity Protection
Solutions in Linux" had some performance statistics.  I'm not sure this
change will have much of a performance impact.

quoted

By defining
the pre and post security hooks in this patch set, it permits each of
the LSMs to migrate to the new hooks independently of each other.   Lets
ask the LSM maintainers what they think.

I see -- yeah making this a 2 step thing makes sense, so long as the
maintainers can later expect / understand what would be done in a
second patch set. Breaking this down in two patch sets makes sense.

I'll defer adding the pre and post security hooks to the subsequent
patch set.

Mimi

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-21 21:16:35

On Thu, 2016-01-21 at 10:45 -0500, Paul Moore wrote:

On Thursday, January 21, 2016 08:12:12 AM Mimi Zohar wrote:

quoted

Paul, Casey, Kees, Jon, Tetsuo does it make sense to consolidate the
module, firmware, and kexec pre and post security hooks and have just
one set of pre and post security kernel_read_file hook instead?   Does
it make sense for this patch set to define the new hooks to allow the
LSMs to migrate to it independently of each other?

Well, as usual, the easiest way to both get solid feedback and actually get a 
change accepted is to post patches to the affected LSMs.  Probably not what 
you wanted to hear, but at least I'm honest :)

Unless I'm misreading the code, it might be a lot simpler than I
thought.  Of the three LSM hooks kernel_module_request,
kernel_module_from_file, and kernel_fw_from_file, the only upstreamed
LSM on any of these hooks is SELinux, which is only on the
kernel_module_request hook.

After converting the SELinux kernel_module_request hook to use the new
kernel_read_file(),  do I then remove the three hooks?   Are we
concerned about "minor" LSMs that have not been upstreamed that might be
using these hooks?

Mimi

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Paul Moore <hidden>
Date: 2016-01-21 21:27:19

On Thursday, January 21, 2016 04:15:02 PM Mimi Zohar wrote:

On Thu, 2016-01-21 at 10:45 -0500, Paul Moore wrote:

quoted

On Thursday, January 21, 2016 08:12:12 AM Mimi Zohar wrote:

quoted

Paul, Casey, Kees, Jon, Tetsuo does it make sense to consolidate the
module, firmware, and kexec pre and post security hooks and have just
one set of pre and post security kernel_read_file hook instead?   Does
it make sense for this patch set to define the new hooks to allow the
LSMs to migrate to it independently of each other?

Well, as usual, the easiest way to both get solid feedback and actually
get a change accepted is to post patches to the affected LSMs.  Probably
not what you wanted to hear, but at least I'm honest :)

Unless I'm misreading the code, it might be a lot simpler than I
thought.  Of the three LSM hooks kernel_module_request,
kernel_module_from_file, and kernel_fw_from_file, the only upstreamed
LSM on any of these hooks is SELinux, which is only on the
kernel_module_request hook.

After converting the SELinux kernel_module_request hook to use the new
kernel_read_file(),  do I then remove the three hooks?   Are we
concerned about "minor" LSMs that have not been upstreamed that might be
using these hooks?

You can't worry about code that isn't upstream; if this change breaks 
something that hasn't been merged, then the burden lies on the out-of-tree 
developers to change their code.

-- 
paul moore
security @ redhat

Re: [RFC PATCH v2 08/11] module: replace copy_module_from_fd with kernel version

From: Kees Cook <hidden>
Date: 2016-01-21 21:59:12

On Thu, Jan 21, 2016 at 1:15 PM, Mimi Zohar [off-list ref] wrote:

On Thu, 2016-01-21 at 10:45 -0500, Paul Moore wrote:

quoted

On Thursday, January 21, 2016 08:12:12 AM Mimi Zohar wrote:

quoted

Paul, Casey, Kees, Jon, Tetsuo does it make sense to consolidate the
module, firmware, and kexec pre and post security hooks and have just
one set of pre and post security kernel_read_file hook instead?   Does
it make sense for this patch set to define the new hooks to allow the
LSMs to migrate to it independently of each other?

Well, as usual, the easiest way to both get solid feedback and actually get a
change accepted is to post patches to the affected LSMs.  Probably not what
you wanted to hear, but at least I'm honest :)

Unless I'm misreading the code, it might be a lot simpler than I
thought.  Of the three LSM hooks kernel_module_request,
kernel_module_from_file, and kernel_fw_from_file, the only upstreamed
LSM on any of these hooks is SELinux, which is only on the
kernel_module_request hook.

After converting the SELinux kernel_module_request hook to use the new
kernel_read_file(),  do I then remove the three hooks?   Are we
concerned about "minor" LSMs that have not been upstreamed that might be
using these hooks?

It should be easy for me to port my LSM to use the new hook. No
objections in consolidating things.

(Which reminds me to resend my LSM again...)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

Re: [RFC PATCH v2 09/11] ima: load policy using path

From: Luis R. Rodriguez <hidden>
Date: 2016-01-23 02:59:53

On Mon, Jan 18, 2016 at 10:11:24AM -0500, Mimi Zohar wrote:

From: Dmitry Kasatkin <redacted>

echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy

 fs/exec.c                       | 21 ++++++++++++++++++++

quoted hunk

diff --git a/fs/exec.c b/fs/exec.c
index 3524e5f..5731b40 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -903,6 +903,27 @@ out:
 	return ret;
 }
 
+int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+			       loff_t max_size, int policy_id)
+{
+	struct file *file;
+	int ret;
+
+	if (!path || !*path)
+		return -EINVAL;
+
+	file = filp_open(path, O_RDONLY, 0);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		pr_err("Unable to open file: %s (%d)", path, ret);
+		return ret;
+	}
+
+	ret = kernel_read_file(file, buf, size, max_size, policy_id);
+	fput(file);
+	return ret;
+}
+

I like this strategy, and can see it being further generalized in
areas of the kernel where we may want self policy / file loading
appraisal. Those policies are now defined through LSMs, in IMA
here you do your own vetting but -- it seems we might better
want instead to enable easily *any* LSM to do its vetting. In this
case I suppose that's still possible given kernel_read_file() is
used and that in turn would have a pre LSM hook and post-LSM hook.

Is that right? If so then it should just be a matter of all other
areas of the kernel to use kernel_read_file_from_path() or
kernel_read_file() to take advantage of similar strategies. Or
is there more work? The other obvious alternative for the heavy
handed users would be to just use the future sysdata helpers, that
in turn would use kernel_read_file_from_path() and still if a
distribution wants can also in the future get kernel signature
verification.

  Luis

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Dave Young <hidden>
Date: 2016-01-25 06:37:55

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?
  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

Do you have a simple usage documentation about how to test it?

On 01/18/16 at 10:11am, Mimi Zohar wrote:

quoted hunk

This patch defines kernel_read_file_from_fd(), a wrapper for the VFS
common kernel_read_file(), and replaces the kexec copy_file_from_fd()
calls with the kernel_read_file_from_fd() wrapper.

Two new IMA policy identifiers named KEXEC_CHECK and INITRAMFS_CHECK
are defined for measuring, appraising or auditing the kexec image
and initramfs.

Changelog v1:
- re-order and squash the kexec patches

v3: ima: measure and appraise kexec image and initramfs (squashed)
- rename ima_read_hooks enumeration to ima_policy_id
- use kstat file size type loff_t, not size_t
- add union name "hooks" to fix sparse warning

v2:
- Calculate the file hash from the in memory buffer
(suggested by Dave Young)
- Rename ima_read_and_process_file() to ima_hash_and_process_file()
- replace individual case statements with range:
        KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1
v1:
- Instead of ima_read_and_process_file() allocating memory, the caller
allocates and frees the memory.
- Moved the kexec measurement/appraisal call to copy_file_from_fd(). The
same call now measures and appraises both the kexec image and initramfs.

Signed-off-by: Mimi Zohar <redacted>
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 fs/exec.c                             | 15 ++++++++
 include/linux/fs.h                    |  1 +
 include/linux/ima.h                   |  2 +
 kernel/kexec_file.c                   | 72 ++++-------------------------------
 security/integrity/ima/ima.h          |  9 ++++-
 security/integrity/ima/ima_appraise.c |  7 ++++
 security/integrity/ima/ima_policy.c   | 27 ++++++++++---
 8 files changed, 64 insertions(+), 71 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 0a378a8..e80f767 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy

@@ -26,7 +26,7 @@ Description:
 			option:	[[appraise_type=]] [permit_directio]
 
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
-				[FIRMWARE_CHECK]
+				[FIRMWARE_CHECK] [KEXEC_CHECK] [INITRAMFS_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value

diff --git a/fs/exec.c b/fs/exec.c
index 211b81c..a5ae51e 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -884,6 +884,21 @@ out:
 }
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+			     int policy_id)

Though this is only used in kexec now, it looks more a general function, move it
to general code should be fine along with kernel_read_file

+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;

-EBADF looks better?

quoted hunk

+
+	if (!f.file)
+		goto out;
+
+	ret = kernel_read_file(f.file, buf, size, max_size, policy_id);
+out:
+	fdput(f);
+	return ret;
+}
+
 ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
 {
 	ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9b1468c..9642623 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -2528,6 +2528,7 @@ extern int do_pipe_flags(int *, int);
 
 extern int kernel_read(struct file *, loff_t, char *, unsigned long);
 extern int kernel_read_file(struct file *, void **, loff_t *, loff_t, int);
+extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t, int);
 extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);

diff --git a/include/linux/ima.h b/include/linux/ima.h
index ca76f60..ae91938 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h

@@ -14,6 +14,8 @@
 struct linux_binprm;
 
 enum ima_policy_id {
+	KEXEC_CHECK = 1,
+	INITRAMFS_CHECK,

Change to below should be better:
KEXEC_KERNEL_CHECK
KEXEC_INITRAMFS_CHECK

quoted hunk

 	IMA_MAX_READ_CHECK
 };

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b70ada0..f7c3ce4 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c

@@ -18,6 +18,7 @@
 #include <linux/kexec.h>
 #include <linux/mutex.h>
 #include <linux/list.h>
+#include <linux/ima.h>
 #include <crypto/hash.h>
 #include <crypto/sha.h>
 #include <linux/syscalls.h>

@@ -33,65 +34,6 @@ size_t __weak kexec_purgatory_size = 0;
 
 static int kexec_calculate_store_digests(struct kimage *image);
 
-static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len)
-{
-	struct fd f = fdget(fd);
-	int ret;
-	struct kstat stat;
-	loff_t pos;
-	ssize_t bytes = 0;
-
-	if (!f.file)
-		return -EBADF;
-
-	ret = vfs_getattr(&f.file->f_path, &stat);
-	if (ret)
-		goto out;
-
-	if (stat.size > INT_MAX) {
-		ret = -EFBIG;
-		goto out;
-	}
-
-	/* Don't hand 0 to vmalloc, it whines. */
-	if (stat.size == 0) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	*buf = vmalloc(stat.size);
-	if (!*buf) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	pos = 0;
-	while (pos < stat.size) {
-		bytes = kernel_read(f.file, pos, (char *)(*buf) + pos,
-				    stat.size - pos);
-		if (bytes < 0) {
-			vfree(*buf);
-			ret = bytes;
-			goto out;
-		}
-
-		if (bytes == 0)
-			break;
-		pos += bytes;
-	}
-
-	if (pos != stat.size) {
-		ret = -EBADF;
-		vfree(*buf);
-		goto out;
-	}
-
-	*buf_len = pos;
-out:
-	fdput(f);
-	return ret;
-}
-
 /* Architectures can provide this probe function */
 int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
 					 unsigned long buf_len)

@@ -180,16 +122,17 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 {
 	int ret = 0;
 	void *ldata;
+	loff_t size;
 
-	ret = copy_file_from_fd(kernel_fd, &image->kernel_buf,
-				&image->kernel_buf_len);
+	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
+				       &size, INT_MAX, KEXEC_CHECK);
 	if (ret)
 		return ret;
+	image->kernel_buf_len = size;
 
 	/* Call arch image probe handlers */
 	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
 					    image->kernel_buf_len);
-
 	if (ret)
 		goto out;

@@ -204,10 +147,11 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 #endif
 	/* It is possible that there no initramfs is being loaded */
 	if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
-		ret = copy_file_from_fd(initrd_fd, &image->initrd_buf,
-					&image->initrd_buf_len);
+		ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
+					       &size, INT_MAX, INITRAMFS_CHECK);
 		if (ret)
 			goto out;
+		image->initrd_buf_len = size;
 	}
 
 	if (cmdline_len) {

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 06bcc24..b98dbd5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h

@@ -160,7 +160,14 @@ void ima_free_template_entry(struct ima_template_entry *entry);
 const char *ima_d_path(struct path *path, char **pathbuf);
 
 /* IMA policy related functions */
-enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, FIRMWARE_CHECK, POST_SETATTR };
+enum ima_hooks {
+	FILE_CHECK = IMA_MAX_READ_CHECK,
+	MMAP_CHECK,
+	BPRM_CHECK,
+	MODULE_CHECK,
+	FIRMWARE_CHECK,
+	POST_SETATTR
+};
 
 int ima_match_policy(struct inode *inode, int func, int mask, int flags);
 void ima_init_policy(void);

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 4edf47f..3adf937 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -78,6 +78,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_module_status;
 	case FIRMWARE_CHECK:
 		return iint->ima_firmware_status;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		return iint->ima_read_status;
 	case FILE_CHECK:
 	default:
 		return iint->ima_file_status;

@@ -100,6 +102,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case FIRMWARE_CHECK:
 		iint->ima_firmware_status = status;
 		break;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		iint->ima_read_status = status;
+		break;
 	case FILE_CHECK:
 	default:
 		iint->ima_file_status = status;

@@ -122,6 +127,8 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
 	case FIRMWARE_CHECK:
 		iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED);
 		break;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		break;
 	case FILE_CHECK:
 	default:
 		iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 595e038..4711083 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -306,6 +306,8 @@ static int get_subaction(struct ima_rule_entry *rule, int func)
 		return IMA_MODULE_APPRAISE;
 	case FIRMWARE_CHECK:
 		return IMA_FIRMWARE_APPRAISE;
+	case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1:
+		return IMA_READ_APPRAISE;
 	case FILE_CHECK:
 	default:
 		return IMA_FILE_APPRAISE;

@@ -614,6 +616,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->hooks.func = MMAP_CHECK;
 			else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
 				entry->hooks.func = BPRM_CHECK;
+			else if (strcmp(args[0].from, "KEXEC_CHECK") == 0)
+				entry->hooks.policy_id = KEXEC_CHECK;
+			else if (strcmp(args[0].from, "INITRAMFS_CHECK") == 0)
+				entry->hooks.policy_id = INITRAMFS_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)

@@ -867,7 +873,9 @@ static char *func_tokens[] = {
 	"BPRM_CHECK",
 	"MODULE_CHECK",
 	"FIRMWARE_CHECK",
-	"POST_SETATTR"
+	"POST_SETATTR",
+	"KEXEC_CHECK",
+	"INITRAMFS_CHECK",
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)

@@ -948,10 +956,19 @@ int ima_policy_show(struct seq_file *m, void *v)
 			seq_printf(m, pt(Opt_func), ft(func_post));
 			break;
 		default:
-			snprintf(tbuf, sizeof(tbuf), "%d",
-				 entry->hooks.func);
-			seq_printf(m, pt(Opt_func), tbuf);
-			break;
+			switch (entry->hooks.policy_id) {
+			case KEXEC_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_kexec));
+				break;
+			case INITRAMFS_CHECK:
+				seq_printf(m, pt(Opt_func), ft(func_initramfs));
+				break;
+			default:
+				snprintf(tbuf, sizeof(tbuf), "%d",
+					 entry->hooks.func);
+				seq_printf(m, pt(Opt_func), tbuf);
+				break;
+			}
 		}
 		seq_puts(m, " ");
 	}

-- 
2.1.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Dave Young <hidden>
Date: 2016-01-25 07:03:24

quoted

diff --git a/fs/exec.c b/fs/exec.c
index 211b81c..a5ae51e 100644
--- a/fs/exec.c
+++ b/fs/exec.c

@@ -884,6 +884,21 @@ out:
 }
 EXPORT_SYMBOL_GPL(kernel_read_file);
 
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+			     int policy_id)

Though this is only used in kexec now, it looks more a general function, move it
to general code should be fine along with kernel_read_file

Oops, seems I misread exec.c as kexec.c

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-25 15:05:38

On Mon, 2016-01-25 at 14:37 +0800, Dave Young wrote:

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?

Yes, without IMA configured or an IMA policy, it is the same as before.

  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

IMA provides a uniform method of measuring and appraising all files on
the system, based on policy.  The IMA policy could prevent the original
kexec syscall.  On systems without MODULE_SIG_FORCE, the IMA policy
would require an IMA signature as well.  (The current patch would
require both, even when MODULE_SIG_FORCE is enabled.)

The pe format is supported on x86.  Why require the pe file signature
format on all platforms?

Do you have a simple usage documentation about how to test it?

The wiki[1] and ima-evm-ctl package[2] have directions for enabling
IMA/IMA-appraisal.

To include just the kexec image and initramfs file hashes in the IMA
measurement list, create a file containing the following IMA policy
rules.  "cat" the policy and redirect the output
to /sys/kernel/security/ima/policy.   After loading the kexec image and
initramfs, the IMA measurements will be included in the measurement list
(/sys/kernel/security/ima/ascii_runtime_measurements)

IMA policy: 
measure func=KEXEC_CHECK
measure func=INITRAMFS_CHECK

Appraising the kexec image and initramfs is a bit more complicated as it
requires creating a key, which is signed by a key on the system keyring,
and loading the key onto the trusted IMA keyring.  To simplify testing,
without CONFIG_IMA_TRUSTED_KEYRING enabled, the key being loaded onto
the IMA keyring does not need to be signed.  The evmctl man page[2]
contains directions for creating and loading the key onto the IMA
keyring. 

To appraise just the kexec image and initramfs files, add the following
two rules to the IMA policy and load the policy as before.  (The policy
can only be loaded once per boot, unless IMA_WRITE_POLICY is configured.
With the default appraisal policy, the policy would need to signed.)
Sign the kexec image and initramfs with evmctl before loading them.

# evmctl ima_sign -k <private key> -a sha256 <VM image>
# evmctl ima_sign -k <private key> -a sha256 <initramfs>

IMA appraise policy:
appraise func=KEXEC_CHECK appraise_type=imasig
appraise func=INITRAMFS_CHECK appraise_type=imasig

[1] http://sourceforge.net/p/linux-ima/wiki/Home
[2] http://linux-ima.sourceforge.net/evmctl.1.html

quoted

+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;

-EBADF looks better?

Sure.

Mimi

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-25 20:34:38

On Mon, Jan 25, 2016 at 10:04:18AM -0500, Mimi Zohar wrote:

On Mon, 2016-01-25 at 14:37 +0800, Dave Young wrote:

quoted

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?

Yes, without IMA configured or an IMA policy, it is the same as before.

That's a bit unfair to your work here, this actually paves the way
for not only IMA but also other LSMs to vet for the kexec/initramfs
given LSM hooks are used now on a common kernel read set of functions.

quoted

  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

IMA provides a uniform method of measuring and appraising all files on
the system, based on policy.  The IMA policy could prevent the original
kexec syscall.  On systems without MODULE_SIG_FORCE, the IMA policy
would require an IMA signature as well.  (The current patch would
require both, even when MODULE_SIG_FORCE is enabled.)


Right, so what this approach has revealed really is that architecturally
MODULE_SIG_FORCE should have been an LSM but its not, its also hard to make it
an LSM.  Now with LSM stacking this might make more sense but that requires
work and who knows when and if that will happen. In the meantime we'll live
with the fact that enabling MODULE_SIG_FORCE means you want to stack on
top of the LSMs you have enabled, the MODULE_SIG_FORCE functionality being
all kernel related and perhaps easier to manage / set.

  Luis

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-25 23:49:32

On Mon, 2016-01-25 at 21:34 +0100, Luis R. Rodriguez wrote:

On Mon, Jan 25, 2016 at 10:04:18AM -0500, Mimi Zohar wrote:

quoted

On Mon, 2016-01-25 at 14:37 +0800, Dave Young wrote:

quoted

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?

Yes, without IMA configured or an IMA policy, it is the same as before.

That's a bit unfair to your work here, this actually paves the way
for not only IMA but also other LSMs to vet for the kexec/initramfs
given LSM hooks are used now on a common kernel read set of functions.

Right, I responded to his questions about IMA and not the bigger picture.

quoted

  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

IMA provides a uniform method of measuring and appraising all files on
the system, based on policy.  The IMA policy could prevent the original
kexec syscall.

Sorry for jumping back and forth between security hooks.  Similarly, for
the kernel module hook, it prevents the original kernel module syscall
as well.

On systems without MODULE_SIG_FORCE, the IMA policy

quoted

would require an IMA signature as well.  (The current patch would
require both, even when MODULE_SIG_FORCE is enabled.)


Right, so what this approach has revealed really is that architecturally
MODULE_SIG_FORCE should have been an LSM but its not, its also hard to make it
an LSM.  Now with LSM stacking this might make more sense but that requires
work and who knows when and if that will happen.

I kind of lost you here.  A new mini LSM would require file signatures
of an existing type or would it be a new method for verifying file
signatures?

In the meantime we'll live
with the fact that enabling MODULE_SIG_FORCE means you want to stack on
top of the LSMs you have enabled, the MODULE_SIG_FORCE functionality being
all kernel related and perhaps easier to manage / set.

As I see it, with MODULE_SIG_FORCE, IMA-appraisal could relax its own
policy knowing that only signed kernel modules are loaded.  Without
MODULE_SIG_FORCE enabled, then IMA-appraisal needs to do the enforcing,
of course based on policy.

Mimi

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Dave Young <hidden>
Date: 2016-01-26 01:21:09

Hi, Mimi

On 01/25/16 at 10:04am, Mimi Zohar wrote:

On Mon, 2016-01-25 at 14:37 +0800, Dave Young wrote:

quoted

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?

Yes, without IMA configured or an IMA policy, it is the same as before.

quoted

  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

IMA provides a uniform method of measuring and appraising all files on
the system, based on policy.  The IMA policy could prevent the original
kexec syscall.  On systems without MODULE_SIG_FORCE, the IMA policy
would require an IMA signature as well.  (The current patch would
require both, even when MODULE_SIG_FORCE is enabled.)

Hmm, enabling policy is in userspace (initramfs?) so it may not be good
enough for secure boot case. IMA can be used as a uniform method for kexec
kernel signature verification for !UEFI or !secure-boot case.

The pe format is supported on x86.  Why require the pe file signature
format on all platforms?

For secure boot purpose, an uefi bootable kernel (as an uefi applicatioin)
require it to be a pe file.

But for !secure-boot it is not mandatory.

quoted

Do you have a simple usage documentation about how to test it?

The wiki[1] and ima-evm-ctl package[2] have directions for enabling
IMA/IMA-appraisal.

To include just the kexec image and initramfs file hashes in the IMA
measurement list, create a file containing the following IMA policy
rules.  "cat" the policy and redirect the output
to /sys/kernel/security/ima/policy.   After loading the kexec image and
initramfs, the IMA measurements will be included in the measurement list
(/sys/kernel/security/ima/ascii_runtime_measurements)

IMA policy: 
measure func=KEXEC_CHECK
measure func=INITRAMFS_CHECK

Appraising the kexec image and initramfs is a bit more complicated as it
requires creating a key, which is signed by a key on the system keyring,
and loading the key onto the trusted IMA keyring.  To simplify testing,
without CONFIG_IMA_TRUSTED_KEYRING enabled, the key being loaded onto
the IMA keyring does not need to be signed.  The evmctl man page[2]
contains directions for creating and loading the key onto the IMA
keyring. 

To appraise just the kexec image and initramfs files, add the following
two rules to the IMA policy and load the policy as before.  (The policy
can only be loaded once per boot, unless IMA_WRITE_POLICY is configured.
With the default appraisal policy, the policy would need to signed.)
Sign the kexec image and initramfs with evmctl before loading them.

# evmctl ima_sign -k <private key> -a sha256 <VM image>
# evmctl ima_sign -k <private key> -a sha256 <initramfs>

IMA appraise policy:
appraise func=KEXEC_CHECK appraise_type=imasig
appraise func=INITRAMFS_CHECK appraise_type=imasig

[1] http://sourceforge.net/p/linux-ima/wiki/Home
[2] http://linux-ima.sourceforge.net/evmctl.1.html

Thank you, will try

quoted

+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;

-EBADF looks better?

Sure.

Seems you missed another comment about the policy id name?
can the name be like below?
KEXEC_KERNEL_CHECK
KEXEC_INITRAMFS_CHECK

Thanks
Dave

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Mimi Zohar <hidden>
Date: 2016-01-26 16:41:24

Hi Dave,

On Tue, 2016-01-26 at 09:20 +0800, Dave Young wrote:

Hi, Mimi

On 01/25/16 at 10:04am, Mimi Zohar wrote:

quoted

On Mon, 2016-01-25 at 14:37 +0800, Dave Young wrote:

quoted

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?

Yes, without IMA configured or an IMA policy, it is the same as before.

quoted

  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

IMA provides a uniform method of measuring and appraising all files on
the system, based on policy.  The IMA policy could prevent the original
kexec syscall.  On systems without MODULE_SIG_FORCE, the IMA policy
would require an IMA signature as well.  (The current patch would
require both, even when MODULE_SIG_FORCE is enabled.)

Hmm, enabling policy is in userspace (initramfs?) so it may not be good
enough for secure boot case. IMA can be used as a uniform method for kexec
kernel signature verification for !UEFI or !secure-boot case.

Normally, the kernel is booted with a builtin policy.   The policy, if
it is being replaced, is normally replaced in the initramfs.  This patch
set introduces the concept of a signed policy.   Refer to the last 3
patches.

quoted

The pe format is supported on x86.  Why require the pe file signature
format on all platforms?

For secure boot purpose, an uefi bootable kernel (as an uefi applicatioin)
require it to be a pe file.

But for !secure-boot it is not mandatory.

It would be more appropriate to say that "UEFI secure boot" requires a
pe file, as opposed to "secure boot" in general.

quoted

Do you have a simple usage documentation about how to test it?

The wiki[1] and ima-evm-ctl package[2] have directions for enabling
IMA/IMA-appraisal.

To include just the kexec image and initramfs file hashes in the IMA
measurement list, create a file containing the following IMA policy
rules.  "cat" the policy and redirect the output
to /sys/kernel/security/ima/policy.   After loading the kexec image and
initramfs, the IMA measurements will be included in the measurement list
(/sys/kernel/security/ima/ascii_runtime_measurements)

IMA policy: 
measure func=KEXEC_CHECK
measure func=INITRAMFS_CHECK

Appraising the kexec image and initramfs is a bit more complicated as it
requires creating a key, which is signed by a key on the system keyring,
and loading the key onto the trusted IMA keyring.  To simplify testing,
without CONFIG_IMA_TRUSTED_KEYRING enabled, the key being loaded onto
the IMA keyring does not need to be signed.  The evmctl man page[2]
contains directions for creating and loading the key onto the IMA
keyring. 

To appraise just the kexec image and initramfs files, add the following
two rules to the IMA policy and load the policy as before.  (The policy
can only be loaded once per boot, unless IMA_WRITE_POLICY is configured.
With the default appraisal policy, the policy would need to signed.)
Sign the kexec image and initramfs with evmctl before loading them.

# evmctl ima_sign -k <private key> -a sha256 <VM image>
# evmctl ima_sign -k <private key> -a sha256 <initramfs>

IMA appraise policy:
appraise func=KEXEC_CHECK appraise_type=imasig
appraise func=INITRAMFS_CHECK appraise_type=imasig

[1] http://sourceforge.net/p/linux-ima/wiki/Home
[2] http://linux-ima.sourceforge.net/evmctl.1.html

Thank you, will try

quoted

+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;

-EBADF looks better?

Sure.

Seems you missed another comment about the policy id name?
can the name be like below?
KEXEC_KERNEL_CHECK
KEXEC_INITRAMFS_CHECK

Luis suggested making the enumeration more generic, not IMA specific.  I
suggested the following:

enum kernel_read_file_id {
        READING_KEXEC_IMAGE = 1,
        READING_KEXEC_INITRAMFS,
        READING_FIRMWARE,
        READING_MODULE,
        READING_POLICY,
        READING_MAX_ID
};

Mimi

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Luis R. Rodriguez <hidden>
Date: 2016-01-26 20:48:45

On Mon, Jan 25, 2016 at 06:48:12PM -0500, Mimi Zohar wrote:

On Mon, 2016-01-25 at 21:34 +0100, Luis R. Rodriguez wrote:

quoted

On Mon, Jan 25, 2016 at 10:04:18AM -0500, Mimi Zohar wrote:

quoted

On Mon, 2016-01-25 at 14:37 +0800, Dave Young wrote:

quoted

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?

Yes, without IMA configured or an IMA policy, it is the same as before.

That's a bit unfair to your work here, this actually paves the way
for not only IMA but also other LSMs to vet for the kexec/initramfs
given LSM hooks are used now on a common kernel read set of functions.

Right, I responded to his questions about IMA and not the bigger picture.

Indeed, its just the bigger picture helps validate your work further.

quoted

  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

IMA provides a uniform method of measuring and appraising all files on
the system, based on policy.  The IMA policy could prevent the original
kexec syscall.

Sorry for jumping back and forth between security hooks.  Similarly, for
the kernel module hook, it prevents the original kernel module syscall
as well.

This is indeed an important feature. I know people who don't use IMA won't
care, but people who do should, likewise its also important for the prospects
in design of LSMs if they wanted to consider the prospects of this evaluation
for any of the file types we are now clearly defining.

quoted

On systems without MODULE_SIG_FORCE, the IMA policy

quoted

would require an IMA signature as well.  (The current patch would
require both, even when MODULE_SIG_FORCE is enabled.)


Right, so what this approach has revealed really is that architecturally
MODULE_SIG_FORCE should have been an LSM but its not, its also hard to make it
an LSM.  Now with LSM stacking this might make more sense but that requires
work and who knows when and if that will happen.

I kind of lost you here.  A new mini LSM would require file signatures
of an existing type or would it be a new method for verifying file
signatures?

No the way I see it is this is a feature for signature verification with all
security mechanisms built-in to the kernel. Clearly there is a difference for
how signatures are defined between file types: modules have signatures built-in
to the file, meanwhile for other files this might be detached (that will be the
case for firmware, sysdata). This has yet to be considered for kexec /
initramfs but it shouldn't' be too hard now to consider that. Its also why
firmware signatures is using a separate kconfig option.

quoted

In the meantime we'll live
with the fact that enabling MODULE_SIG_FORCE means you want to stack on
top of the LSMs you have enabled, the MODULE_SIG_FORCE functionality being
all kernel related and perhaps easier to manage / set.

As I see it, with MODULE_SIG_FORCE, IMA-appraisal could relax its own
policy knowing that only signed kernel modules are loaded.  Without
MODULE_SIG_FORCE enabled, then IMA-appraisal needs to do the enforcing,
of course based on policy.

Sure, up to the LSMs, which begs the question of how an LSM would codify
such stacking assumptions. IIRC stacking will just let the code stack, but
I am not sure if we have a clean way to know: hey the kernel vetted this
file's signature itself, just fyi. Other than #idef'ery or relying on the
.config.  What I just described is the case for say IMA and kernel module
firmware signing (note both have separate kconfigs), the same question still
applies to ways LSM stacking can provide a set of security requirements each
could have addressed. All in a clean way.

  Luis

Re: [RFC PATCH v2 06/11] kexec: replace call to copy_file_from_fd() with kernel version

From: Dave Young <hidden>
Date: 2016-01-27 01:51:36

On 01/26/16 at 11:40am, Mimi Zohar wrote:

Hi Dave,

On Tue, 2016-01-26 at 09:20 +0800, Dave Young wrote:

quoted

Hi, Mimi

On 01/25/16 at 10:04am, Mimi Zohar wrote:

quoted

On Mon, 2016-01-25 at 14:37 +0800, Dave Young wrote:

quoted

Hi, Mimi

Besides of code issues, I have several thing to be understand:

What is the effect to kexec behavior with this patchset?
  - without IMA enabled (kconfig or kernel cmdline) it will be same as before?

Yes, without IMA configured or an IMA policy, it is the same as before.

quoted

  - with IMA enabled for kernel bzImage, kexec_file_load will check both ima
    signature and original pe file signature, those two mechanisms are
    somehow duplicated. I'm not sure if we need both for bzImage.

IMA provides a uniform method of measuring and appraising all files on
the system, based on policy.  The IMA policy could prevent the original
kexec syscall.  On systems without MODULE_SIG_FORCE, the IMA policy
would require an IMA signature as well.  (The current patch would
require both, even when MODULE_SIG_FORCE is enabled.)

Hmm, enabling policy is in userspace (initramfs?) so it may not be good
enough for secure boot case. IMA can be used as a uniform method for kexec
kernel signature verification for !UEFI or !secure-boot case.

Normally, the kernel is booted with a builtin policy.   The policy, if
it is being replaced, is normally replaced in the initramfs.  This patch
set introduces the concept of a signed policy.   Refer to the last 3
patches.

But one can still disable ima via kernel cmdline. I'm not objecting this
patch I just think that it can not replace the kexec signature verification
that we are using for UEFI secure boot.

I think they can coexist consider their different design.

quoted

The pe format is supported on x86.  Why require the pe file signature
format on all platforms?

For secure boot purpose, an uefi bootable kernel (as an uefi applicatioin)
require it to be a pe file.

But for !secure-boot it is not mandatory.

It would be more appropriate to say that "UEFI secure boot" requires a
pe file, as opposed to "secure boot" in general.

Fair enough and agreed.

quoted

Do you have a simple usage documentation about how to test it?

The wiki[1] and ima-evm-ctl package[2] have directions for enabling
IMA/IMA-appraisal.

To include just the kexec image and initramfs file hashes in the IMA
measurement list, create a file containing the following IMA policy
rules.  "cat" the policy and redirect the output
to /sys/kernel/security/ima/policy.   After loading the kexec image and
initramfs, the IMA measurements will be included in the measurement list
(/sys/kernel/security/ima/ascii_runtime_measurements)

IMA policy: 
measure func=KEXEC_CHECK
measure func=INITRAMFS_CHECK

Appraising the kexec image and initramfs is a bit more complicated as it
requires creating a key, which is signed by a key on the system keyring,
and loading the key onto the trusted IMA keyring.  To simplify testing,
without CONFIG_IMA_TRUSTED_KEYRING enabled, the key being loaded onto
the IMA keyring does not need to be signed.  The evmctl man page[2]
contains directions for creating and loading the key onto the IMA
keyring. 

To appraise just the kexec image and initramfs files, add the following
two rules to the IMA policy and load the policy as before.  (The policy
can only be loaded once per boot, unless IMA_WRITE_POLICY is configured.
With the default appraisal policy, the policy would need to signed.)
Sign the kexec image and initramfs with evmctl before loading them.

# evmctl ima_sign -k <private key> -a sha256 <VM image>
# evmctl ima_sign -k <private key> -a sha256 <initramfs>

IMA appraise policy:
appraise func=KEXEC_CHECK appraise_type=imasig
appraise func=INITRAMFS_CHECK appraise_type=imasig

[1] http://sourceforge.net/p/linux-ima/wiki/Home
[2] http://linux-ima.sourceforge.net/evmctl.1.html

Thank you, will try

quoted

+{
+	struct fd f = fdget(fd);
+	int ret = -ENOEXEC;

-EBADF looks better?

Sure.

Seems you missed another comment about the policy id name?
can the name be like below?
KEXEC_KERNEL_CHECK
KEXEC_INITRAMFS_CHECK

Luis suggested making the enumeration more generic, not IMA specific.  I
suggested the following:

enum kernel_read_file_id {
        READING_KEXEC_IMAGE = 1,
        READING_KEXEC_INITRAMFS,
        READING_FIRMWARE,
        READING_MODULE,
        READING_POLICY,
        READING_MAX_ID
};

It is even better.

Mimi

Thanks
Dave

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help