Linux cryptographic layer development
 help / color / mirror / Atom feed

* [RFC][PATCH 04/20] ima: read and use signature hash algorithm
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Dmitry Kasatkin, linux-crypto, David Howells, Dmitry Kasatkin,
	Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>

All files on the filesystem, currently, are hashed using the same hash
algorithm.  In preparation for files from different packages being
signed using different hash algorithms, this patch adds support for
reading the signature hash algorithm from the 'security.ima' extended
attribute and calculates the appropriate file data hash based on it.

Changelog:
- fix scripts Lindent and checkpatch msgs - Mimi

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/digsig_asymmetric.c | 11 ---------
 security/integrity/ima/ima.h           | 29 ++++++++++++++++++++---
 security/integrity/ima/ima_api.c       | 12 +++++++++-
 security/integrity/ima/ima_appraise.c  | 42 +++++++++++++++++++++++++++-------
 security/integrity/ima/ima_main.c      | 11 +++++++--
 security/integrity/integrity.h         | 11 +++++++++
 6 files changed, 91 insertions(+), 25 deletions(-)

diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index b475466..9eae480 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -20,17 +20,6 @@
 #include "integrity.h"
 
 /*
- * signature format v2 - for using with asymmetric keys
- */
-struct signature_v2_hdr {
-	uint8_t version;	/* signature format version */
-	uint8_t	hash_algo;	/* Digest algorithm [enum pkey_hash_algo] */
-	uint32_t keyid;		/* IMA key identifier - not X509/PGP specific*/
-	uint16_t sig_size;	/* signature size */
-	uint8_t sig[0];		/* signature payload */
-} __packed;
-
-/*
  * Request an asymmetric key.
  */
 static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index eb86032..efcdef2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -99,7 +99,9 @@ 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 file *file,
+			    struct evm_ima_xattr_data **xattr_value,
+			    int *xattr_len);
 void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   const unsigned char *filename);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
@@ -132,17 +134,25 @@ void ima_delete_rules(void);
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
-			     struct file *file, const unsigned char *filename);
+			     struct file *file, const unsigned char *filename,
+			     struct evm_ima_xattr_data *xattr_value,
+			     int xattr_len);
 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);
+int ima_read_xattr(struct dentry *dentry,
+		   struct evm_ima_xattr_data **xattr_value);
 
 #else
 static inline int ima_appraise_measurement(int func,
 					   struct integrity_iint_cache *iint,
 					   struct file *file,
-					   const unsigned char *filename)
+					   const unsigned char *filename,
+					   struct evm_ima_xattr_data *xattr_value,
+					   int xattr_len)
 {
 	return INTEGRITY_UNKNOWN;
 }
@@ -163,6 +173,19 @@ 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 int ima_read_xattr(struct dentry *dentry,
+				 struct evm_ima_xattr_data **xattr_value)
+{
+	return 0;
+}
+
 #endif
 
 /* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index e531fe2..1dba98e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -139,17 +139,27 @@ int ima_must_measure(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 file *file,
+			    struct evm_ima_xattr_data **xattr_value,
+			    int *xattr_len)
 {
 	struct inode *inode = file_inode(file);
 	const char *filename = file->f_dentry->d_name.name;
 	int result = 0;
 
+	if (xattr_value)
+		*xattr_len = ima_read_xattr(file->f_dentry, xattr_value);
+
 	if (!(iint->flags & IMA_COLLECTED)) {
 		u64 i_version = file_inode(file)->i_version;
 
 		/* use default hash algorithm */
 		iint->ima_hash.algo = ima_hash_algo;
+
+		if (xattr_value)
+			ima_get_hash_algo(*xattr_value, *xattr_len,
+					  &iint->ima_hash);
+
 		result = ima_calc_file_hash(file, &iint->ima_hash);
 		if (!result) {
 			iint->version = i_version;
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f2a3bda..5b59b43 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -107,6 +107,34 @@ 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)
+{
+	struct signature_v2_hdr *sig;
+
+	if (!xattr_value || xattr_len < 0 || xattr_len <= 1 + sizeof(*sig))
+		return;
+
+	sig = (typeof(sig)) xattr_value->digest;
+
+	if (xattr_value->type != EVM_IMA_XATTR_DIGSIG || sig->version != 2)
+		return;
+
+	hash->algo = sig->hash_algo;
+}
+
+int ima_read_xattr(struct dentry *dentry,
+		   struct evm_ima_xattr_data **xattr_value)
+{
+	struct inode *inode = dentry->d_inode;
+
+	if (!inode->i_op->getxattr)
+		return 0;
+
+	return vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value,
+				  0, GFP_NOFS);
+}
+
 /*
  * ima_appraise_measurement - appraise file measurement
  *
@@ -116,23 +144,22 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func)
  * Return 0 on success, error code otherwise
  */
 int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
-			     struct file *file, const unsigned char *filename)
+			     struct file *file, const unsigned char *filename,
+			     struct evm_ima_xattr_data *xattr_value,
+			     int xattr_len)
 {
 	struct dentry *dentry = file->f_dentry;
 	struct inode *inode = dentry->d_inode;
-	struct evm_ima_xattr_data *xattr_value = NULL;
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	const char *op = "appraise_data";
 	char *cause = "unknown";
-	int rc;
+	int rc = xattr_len;
 
 	if (!ima_appraise)
 		return 0;
 	if (!inode->i_op->getxattr)
 		return INTEGRITY_UNKNOWN;
 
-	rc = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)&xattr_value,
-				0, GFP_NOFS);
 	if (rc <= 0) {
 		if (rc && rc != -ENODATA)
 			goto out;
@@ -159,7 +186,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 			status = INTEGRITY_FAIL;
 			break;
 		}
-		if (rc - 1 == iint->ima_hash.length)
+		if (xattr_len - 1 == iint->ima_hash.length)
 			rc = memcmp(xattr_value->digest,
 				    iint->ima_hash.digest,
 				    iint->ima_hash.length);
@@ -207,7 +234,6 @@ out:
 		ima_cache_flags(iint, func);
 	}
 	ima_set_cache_status(iint, func, status);
-	kfree(xattr_value);
 	return status;
 }
 
@@ -223,7 +249,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);
+	rc = ima_collect_measurement(iint, file, NULL, NULL);
 	if (rc < 0)
 		return;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a8ae99b..c22027e 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -149,6 +149,8 @@ static int process_measurement(struct file *file, const char *filename,
 	char *pathbuf = NULL;
 	const char *pathname = NULL;
 	int rc = -ENOMEM, action, must_appraise, _func;
+	struct evm_ima_xattr_data *xattr_value = NULL, **xattr_ptr = NULL;
+	int xattr_len = 0;
 
 	if (!ima_initialized || !S_ISREG(inode->i_mode))
 		return 0;
@@ -187,7 +189,10 @@ static int process_measurement(struct file *file, const char *filename,
 		goto out_digsig;
 	}
 
-	rc = ima_collect_measurement(iint, file);
+	if (action & IMA_APPRAISE_SUBMASK)
+		xattr_ptr = &xattr_value;
+
+	rc = ima_collect_measurement(iint, file, xattr_ptr, &xattr_len);
 	if (rc != 0)
 		goto out_digsig;
 
@@ -198,7 +203,8 @@ static int process_measurement(struct file *file, const char *filename,
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname);
 	if (action & IMA_APPRAISE_SUBMASK)
-		rc = ima_appraise_measurement(_func, iint, file, pathname);
+		rc = ima_appraise_measurement(_func, iint, file, pathname,
+					      xattr_value, xattr_len);
 	if (action & IMA_AUDIT)
 		ima_audit_measurement(iint, pathname);
 	kfree(pathbuf);
@@ -207,6 +213,7 @@ out_digsig:
 		rc = -EACCES;
 out:
 	mutex_unlock(&inode->i_mutex);
+	kfree(xattr_value);
 	if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE))
 		return -EACCES;
 	return 0;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a97f1091..d519a9c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -70,6 +70,17 @@ struct ima_digest_data {
 	u8 digest[IMA_MAX_DIGEST_SIZE];
 } __packed;
 
+/*
+ * signature format v2 - for using with asymmetric keys
+ */
+struct signature_v2_hdr {
+	uint8_t version;	/* signature format version */
+	uint8_t	hash_algo;	/* Digest algorithm [enum pkey_hash_algo] */
+	uint32_t keyid;		/* IMA key identifier - not X509/PGP specific */
+	uint16_t sig_size;	/* signature size */
+	uint8_t sig[0];		/* signature payload */
+} __packed;
+
 /* integrity data associated with an inode */
 struct integrity_iint_cache {
 	struct rb_node rb_node;	/* rooted in integrity_iint_tree */
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 05/20] ima: use dynamically allocated hash storage
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Dmitry Kasatkin, linux-crypto, David Howells, Dmitry Kasatkin,
	Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>

For each inode in the IMA policy, an iint is allocated.  To support
larger hash digests, the iint digest size changed from 20 bytes to
the maximum supported hash digest size.  Instead of allocating the
maximum size, which most likely is not needed, this patch dynamically
allocates the needed hash storage.

Changelog:
- fix krealloc bug

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/iint.c             |  2 ++
 security/integrity/ima/ima_api.c      | 57 +++++++++++++++++++++++------------
 security/integrity/ima/ima_appraise.c | 16 +++++-----
 security/integrity/integrity.h        |  4 +--
 4 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 74522db..c49d3f1 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -70,6 +70,8 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode)
 
 static void iint_free(struct integrity_iint_cache *iint)
 {
+	kfree(iint->ima_hash);
+	iint->ima_hash = NULL;
 	iint->version = 0;
 	iint->flags = 0UL;
 	iint->ima_file_status = INTEGRITY_UNKNOWN;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 1dba98e..5a7942e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -44,7 +44,10 @@ int ima_store_template(struct ima_template_entry *entry,
 	const char *op = "add_template_measure";
 	const char *audit_cause = "hashing_error";
 	int result;
-	struct ima_digest_data hash;
+	struct {
+		struct ima_digest_data hdr;
+		char digest[IMA_MAX_DIGEST_SIZE];
+	} hash;
 
 	memset(entry->digest, 0, sizeof(entry->digest));
 	entry->template_name = IMA_TEMPLATE_NAME;
@@ -52,14 +55,14 @@ int ima_store_template(struct ima_template_entry *entry,
 
 	if (!violation) {
 		result = ima_calc_buffer_hash(&entry->template,
-					      entry->template_len, &hash);
+					      entry->template_len, &hash.hdr);
 		if (result < 0) {
 			integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode,
 					    entry->template_name, op,
 					    audit_cause, result, 0);
 			return result;
 		}
-		memcpy(entry->digest, hash.digest, hash.length);
+		memcpy(entry->digest, hash.hdr.digest, hash.hdr.length);
 	}
 	result = ima_add_template_entry(entry, violation, op, inode);
 	return result;
@@ -146,6 +149,10 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	struct inode *inode = file_inode(file);
 	const char *filename = file->f_dentry->d_name.name;
 	int result = 0;
+	struct {
+		struct ima_digest_data hdr;
+		char digest[IMA_MAX_DIGEST_SIZE];
+	} hash;
 
 	if (xattr_value)
 		*xattr_len = ima_read_xattr(file->f_dentry, xattr_value);
@@ -154,16 +161,23 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 		u64 i_version = file_inode(file)->i_version;
 
 		/* use default hash algorithm */
-		iint->ima_hash.algo = ima_hash_algo;
+		hash.hdr.algo = ima_hash_algo;
 
 		if (xattr_value)
-			ima_get_hash_algo(*xattr_value, *xattr_len,
-					  &iint->ima_hash);
+			ima_get_hash_algo(*xattr_value, *xattr_len, &hash.hdr);
 
-		result = ima_calc_file_hash(file, &iint->ima_hash);
+		result = ima_calc_file_hash(file, &hash.hdr);
 		if (!result) {
-			iint->version = i_version;
-			iint->flags |= IMA_COLLECTED;
+			int length = sizeof(hash.hdr) + hash.hdr.length;
+			void *tmpbuf = krealloc(iint->ima_hash, length,
+						GFP_NOFS);
+			if (tmpbuf) {
+				iint->ima_hash = tmpbuf;
+				memcpy(iint->ima_hash, &hash, length);
+				iint->version = i_version;
+				iint->flags |= IMA_COLLECTED;
+			} else
+				result = -ENOMEM;
 		}
 	}
 	if (result)
@@ -208,21 +222,24 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 		return;
 	}
 	memset(&entry->template, 0, sizeof(entry->template));
-	if (iint->ima_hash.algo != ima_hash_algo) {
-		struct ima_digest_data hash;
+	if (iint->ima_hash->algo != ima_hash_algo) {
+		struct {
+			struct ima_digest_data hdr;
+			char digest[IMA_MAX_DIGEST_SIZE];
+		} hash;
 
-		hash.algo = ima_hash_algo;
-		result = ima_calc_file_hash(file, &hash);
+		hash.hdr.algo = ima_hash_algo;
+		result = ima_calc_file_hash(file, &hash.hdr);
 		if (result)
 			integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
 					    filename, "collect_data", "failed",
 					    result, 0);
 		else
-			memcpy(entry->template.digest, hash.digest,
-			       hash.length);
+			memcpy(entry->template.digest, hash.hdr.digest,
+			       hash.hdr.length);
 	} else
-		memcpy(entry->template.digest, iint->ima_hash.digest,
-		       iint->ima_hash.length);
+		memcpy(entry->template.digest, iint->ima_hash->digest,
+		       iint->ima_hash->length);
 	strcpy(entry->template.file_name,
 	       (strlen(filename) > IMA_EVENT_NAME_LEN_MAX) ?
 	       file->f_dentry->d_name.name : filename);
@@ -238,14 +255,14 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename)
 {
 	struct audit_buffer *ab;
-	char hash[(iint->ima_hash.length * 2) + 1];
+	char hash[(iint->ima_hash->length * 2) + 1];
 	int i;
 
 	if (iint->flags & IMA_AUDITED)
 		return;
 
-	for (i = 0; i < iint->ima_hash.length; i++)
-		hex_byte_pack(hash + (i * 2), iint->ima_hash.digest[i]);
+	for (i = 0; i < iint->ima_hash->length; i++)
+		hex_byte_pack(hash + (i * 2), iint->ima_hash->digest[i]);
 	hash[i * 2] = '\0';
 
 	ab = audit_log_start(current->audit_context, GFP_KERNEL,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 5b59b43..b60c511d 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -45,10 +45,10 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
 static int ima_fix_xattr(struct dentry *dentry,
 			 struct integrity_iint_cache *iint)
 {
-	iint->ima_hash.type = IMA_XATTR_DIGEST;
+	iint->ima_hash->type = IMA_XATTR_DIGEST;
 	return __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
-				     &iint->ima_hash.type,
-				     1 + iint->ima_hash.length, 0);
+				     &iint->ima_hash->type,
+				     1 + iint->ima_hash->length, 0);
 }
 
 /* Return specific func appraised cached result */
@@ -186,10 +186,10 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 			status = INTEGRITY_FAIL;
 			break;
 		}
-		if (xattr_len - 1 == iint->ima_hash.length)
+		if (xattr_len - 1 == iint->ima_hash->length)
 			rc = memcmp(xattr_value->digest,
-				    iint->ima_hash.digest,
-				    iint->ima_hash.length);
+				    iint->ima_hash->digest,
+				    iint->ima_hash->length);
 		else
 			rc = -EINVAL;
 		if (rc) {
@@ -203,8 +203,8 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 		iint->flags |= IMA_DIGSIG;
 		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
 					     xattr_value->digest, rc - 1,
-					     iint->ima_hash.digest,
-					     iint->ima_hash.length);
+					     iint->ima_hash->digest,
+					     iint->ima_hash->length);
 		if (rc == -EOPNOTSUPP) {
 			status = INTEGRITY_UNKNOWN;
 		} else if (rc) {
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index d519a9c..fadb83a 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -67,7 +67,7 @@ struct ima_digest_data {
 	u8 algo;
 	u8 length;
 	u8 type;
-	u8 digest[IMA_MAX_DIGEST_SIZE];
+	u8 digest[0];
 } __packed;
 
 /*
@@ -92,7 +92,7 @@ struct integrity_iint_cache {
 	enum integrity_status ima_bprm_status:4;
 	enum integrity_status ima_module_status:4;
 	enum integrity_status evm_status:4;
-	struct ima_digest_data ima_hash;
+	struct ima_digest_data *ima_hash;
 };
 
 /* rbtree tree calls to lookup, insert, delete
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 06/20] ima: differentiate between template hash and file data hash sizes
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module; +Cc: Mimi Zohar, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

The TPM v1.2 limits the template hash size to 20 bytes.  This
patch differentiates between the template hash size, as defined
in the ima_template_entry, and the file data hash size, as
defined in the ima_template_data.  Subsequent patches add support
for different file data hash algorithms.

Change log:
- hash digest definition in ima_store_template() should be TPM_DIGEST_SIZE

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
 security/integrity/ima/ima.h        |  2 +-
 security/integrity/ima/ima_api.c    |  2 +-
 security/integrity/ima/ima_crypto.c |  4 ++--
 security/integrity/ima/ima_fs.c     | 10 +++++-----
 security/integrity/ima/ima_init.c   |  2 +-
 security/integrity/ima/ima_queue.c  |  4 ++--
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index efcdef2..52393ed 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -49,7 +49,7 @@ struct ima_template_data {
 };
 
 struct ima_template_entry {
-	u8 digest[IMA_DIGEST_SIZE];	/* sha1 or md5 measurement hash */
+	u8 digest[TPM_DIGEST_SIZE];	/* sha1 or md5 measurement hash */
 	const char *template_name;
 	int template_len;
 	struct ima_template_data template;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 5a7942e..2cc5dcc 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -46,7 +46,7 @@ int ima_store_template(struct ima_template_entry *entry,
 	int result;
 	struct {
 		struct ima_digest_data hdr;
-		char digest[IMA_MAX_DIGEST_SIZE];
+		char digest[TPM_DIGEST_SIZE];
 	} hash;
 
 	memset(entry->digest, 0, sizeof(entry->digest));
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 2fd1786..872c669 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -155,7 +155,7 @@ static void __init ima_pcrread(int idx, u8 *pcr)
  */
 int __init ima_calc_boot_aggregate(char *digest)
 {
-	u8 pcr_i[IMA_DIGEST_SIZE];
+	u8 pcr_i[TPM_DIGEST_SIZE];
 	int rc, i;
 	struct {
 		struct shash_desc shash;
@@ -173,7 +173,7 @@ int __init ima_calc_boot_aggregate(char *digest)
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
 		ima_pcrread(i, pcr_i);
 		/* now accumulate with current aggregate */
-		rc = crypto_shash_update(&desc.shash, pcr_i, IMA_DIGEST_SIZE);
+		rc = crypto_shash_update(&desc.shash, pcr_i, TPM_DIGEST_SIZE);
 	}
 	if (!rc)
 		crypto_shash_final(&desc.shash, digest);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 38477c9..89adf1d 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -134,7 +134,7 @@ static int ima_measurements_show(struct seq_file *m, void *v)
 	ima_putc(m, &pcr, sizeof pcr);
 
 	/* 2nd: template digest */
-	ima_putc(m, e->digest, IMA_DIGEST_SIZE);
+	ima_putc(m, e->digest, TPM_DIGEST_SIZE);
 
 	/* 3rd: template name size */
 	namelen = strlen(e->template_name);
@@ -168,11 +168,11 @@ static const struct file_operations ima_measurements_ops = {
 	.release = seq_release,
 };
 
-static void ima_print_digest(struct seq_file *m, u8 *digest)
+static void ima_print_digest(struct seq_file *m, u8 *digest, int size)
 {
 	int i;
 
-	for (i = 0; i < IMA_DIGEST_SIZE; i++)
+	for (i = 0; i < size; i++)
 		seq_printf(m, "%02x", *(digest + i));
 }
 
@@ -183,7 +183,7 @@ void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show)
 
 	switch (show) {
 	case IMA_SHOW_ASCII:
-		ima_print_digest(m, entry->digest);
+		ima_print_digest(m, entry->digest, IMA_DIGEST_SIZE);
 		seq_printf(m, " %s\n", entry->file_name);
 		break;
 	case IMA_SHOW_BINARY:
@@ -213,7 +213,7 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
 	seq_printf(m, "%2d ", CONFIG_IMA_MEASURE_PCR_IDX);
 
 	/* 2nd: SHA1 template hash */
-	ima_print_digest(m, e->digest);
+	ima_print_digest(m, e->digest, TPM_DIGEST_SIZE);
 
 	/* 3th:  template name */
 	seq_printf(m, " %s ", e->template_name);
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 162ea72..9d0243c 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -74,7 +74,7 @@ err_out:
 
 int __init ima_init(void)
 {
-	u8 pcr_i[IMA_DIGEST_SIZE];
+	u8 pcr_i[TPM_DIGEST_SIZE];
 	int rc;
 
 	ima_used_chip = 0;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index ff63fe0..e63ff33 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -50,7 +50,7 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value)
 	key = ima_hash_key(digest_value);
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(qe, &ima_htable.queue[key], hnext) {
-		rc = memcmp(qe->entry->digest, digest_value, IMA_DIGEST_SIZE);
+		rc = memcmp(qe->entry->digest, digest_value, TPM_DIGEST_SIZE);
 		if (rc == 0) {
 			ret = qe;
 			break;
@@ -106,7 +106,7 @@ static int ima_pcr_extend(const u8 *hash)
 int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode)
 {
-	u8 digest[IMA_DIGEST_SIZE];
+	u8 digest[TPM_DIGEST_SIZE];
 	const char *audit_cause = "hash_added";
 	char tpm_audit_cause[AUDIT_CAUSE_LEN_MAX];
 	int audit_info = 1;
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 11/20] ima: pass the filename argument up to ima_add_template_entry()
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

Pass the filename argument to ima_add_template_entry() in order to
eliminate a dependency on template specific data (third argument of
integrity_audit_msg).

This change is required because, with the new template management
mechanism, the generation of a new measurement entry will be performed
by new specific functions (introduced in next patches) and the current IMA
code will not be aware anymore of how data is stored in the entry payload.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h       | 5 +++--
 security/integrity/ima/ima_api.c   | 9 +++++----
 security/integrity/ima/ima_init.c  | 3 ++-
 security/integrity/ima/ima_queue.c | 6 +++---
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d7bec6f..27d2ffb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -69,7 +69,8 @@ int ima_fs_init(void);
 void ima_fs_cleanup(void);
 int ima_inode_alloc(struct inode *inode);
 int ima_add_template_entry(struct ima_template_entry *entry, int violation,
-			   const char *op, struct inode *inode);
+			   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 *data, int len,
 			 struct ima_digest_data *hash);
@@ -107,7 +108,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename);
 int ima_store_template(struct ima_template_entry *entry, int violation,
-		       struct inode *inode);
+		       struct inode *inode, const unsigned char *filename);
 void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show);
 const char *ima_d_path(struct path *path, char **pathbuf);
 
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 98160a3..a0fe504 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -40,7 +40,8 @@ static const char *IMA_TEMPLATE_NAME = "ima";
  * Returns 0 on success, error code otherwise
  */
 int ima_store_template(struct ima_template_entry *entry,
-		       int violation, struct inode *inode)
+		       int violation, struct inode *inode,
+		       const unsigned char *filename)
 {
 	const char *op = "add_template_measure";
 	const char *audit_cause = "hashing_error";
@@ -67,7 +68,7 @@ int ima_store_template(struct ima_template_entry *entry,
 		}
 		memcpy(entry->digest, hash.hdr.digest, hash.hdr.length);
 	}
-	result = ima_add_template_entry(entry, violation, op, inode);
+	result = ima_add_template_entry(entry, violation, op, inode, filename);
 	return result;
 }
 
@@ -96,7 +97,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 	}
 	memset(&entry->template, 0, sizeof(entry->template));
 	strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX);
-	result = ima_store_template(entry, violation, inode);
+	result = ima_store_template(entry, violation, inode, filename);
 	if (result < 0)
 		kfree(entry);
 err_out:
@@ -248,7 +249,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 	       (strlen(filename) > IMA_EVENT_NAME_LEN_MAX) ?
 	       file->f_dentry->d_name.name : filename);
 
-	result = ima_store_template(entry, violation, inode);
+	result = ima_store_template(entry, violation, inode, filename);
 	if (!result || result == -EEXIST)
 		iint->flags |= IMA_MEASURED;
 	if (result < 0)
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 77cd500..d42fac3 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -71,7 +71,8 @@ static void __init ima_add_boot_aggregate(void)
 		memcpy(entry->template.digest, hash.hdr.digest,
 		       hash.hdr.length);
 	}
-	result = ima_store_template(entry, violation, NULL);
+	result = ima_store_template(entry, violation, NULL,
+				    boot_aggregate_name);
 	if (result < 0)
 		kfree(entry);
 	return;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index e63ff33..d85e997 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -104,7 +104,8 @@ static int ima_pcr_extend(const u8 *hash)
  * and extend the pcr.
  */
 int ima_add_template_entry(struct ima_template_entry *entry, int violation,
-			   const char *op, struct inode *inode)
+			   const char *op, struct inode *inode,
+			   const unsigned char *filename)
 {
 	u8 digest[TPM_DIGEST_SIZE];
 	const char *audit_cause = "hash_added";
@@ -141,8 +142,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 	}
 out:
 	mutex_unlock(&ima_extend_list_mutex);
-	integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode,
-			    entry->template.file_name,
+	integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
 			    op, audit_cause, result, audit_info);
 	return result;
 }
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 15/20] ima: define new template ima-ng and template fields d-ng and n-ng
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

This patch adds support for the new template 'ima-ng', whose format
is defined as 'd-ng|n-ng'.  These new field definitions remove the
size limitations of the original 'ima' template.  Further, the 'd-ng'
field prefixes the inode digest with the hash algorithim, when
displaying the new larger digest sizes.

Change log:
- scripts/Lindent fixes  - Mimi
- "always true comparison" - reported by Fengguang Wu, resolved Dmitry

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_template.c     |   7 +-
 security/integrity/ima/ima_template_lib.c | 155 ++++++++++++++++++++++++++----
 security/integrity/ima/ima_template_lib.h |   8 ++
 3 files changed, 152 insertions(+), 18 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 8100422..bf38d1a 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -16,7 +16,8 @@
 #include "ima_template_lib.h"
 
 static struct ima_template_desc defined_templates[] = {
-	{.name = IMA_TEMPLATE_IMA_NAME,.fmt = IMA_TEMPLATE_IMA_FMT},
+	{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
+	{.name = "ima-ng",.fmt = "d-ng|n-ng"},
 };
 
 static struct ima_template_field supported_fields[] = {
@@ -24,6 +25,10 @@ static struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_digest},
 	{.field_id = "n",.field_init = ima_eventname_init,
 	 .field_show = ima_show_template_string},
+	{.field_id = "d-ng",.field_init = ima_eventdigest_ng_init,
+	 .field_show = ima_show_template_digest_ng},
+	{.field_id = "n-ng",.field_init = ima_eventname_ng_init,
+	 .field_show = ima_show_template_string},
 };
 
 static struct ima_template_field *lookup_template_field(const char *field_id)
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index f1a2fcb..f0bc14f 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -12,9 +12,25 @@
  * File: ima_template_lib.c
  *      Library of supported template fields.
  */
+#include <crypto/hash_info.h>
+
 #include "ima_template_lib.h"
 
-enum data_formats { DATA_FMT_DIGEST = 0, DATA_FMT_EVENT_NAME, DATA_FMT_STRING };
+static bool ima_template_hash_algo_allowed(u8 algo)
+{
+	if (algo == HASH_ALGO_SHA1 || algo == HASH_ALGO_MD5)
+		return true;
+
+	return false;
+}
+
+enum data_formats {
+	DATA_FMT_DIGEST = 0,
+	DATA_FMT_DIGEST_WITH_ALGO,
+	DATA_FMT_EVENT_NAME,
+	DATA_FMT_STRING
+};
+
 static int ima_write_template_field_data(const void *data, const u32 datalen,
 					 enum data_formats datafmt,
 					 struct ima_field_data *field_data)
@@ -62,12 +78,22 @@ static void ima_show_template_data_ascii(struct seq_file *m,
 					 enum data_formats datafmt,
 					 struct ima_field_data *field_data)
 {
+	u8 *buf_ptr = field_data->data, buflen = field_data->len;
+
 	switch (datafmt) {
+	case DATA_FMT_DIGEST_WITH_ALGO:
+		buf_ptr = strnchr(field_data->data, buflen, ':');
+		if (buf_ptr != field_data->data)
+			seq_printf(m, "%s", field_data->data);
+
+		/* skip ':' and '\0' */
+		buf_ptr += 2;
+		buflen -= buf_ptr - field_data->data;
 	case DATA_FMT_DIGEST:
-		ima_print_digest(m, field_data->data, field_data->len);
+		ima_print_digest(m, buf_ptr, buflen);
 		break;
 	case DATA_FMT_STRING:
-		seq_printf(m, "%s", field_data->data);
+		seq_printf(m, "%s", buf_ptr);
 		break;
 	default:
 		break;
@@ -79,7 +105,7 @@ static void ima_show_template_data_binary(struct seq_file *m,
 					  enum data_formats datafmt,
 					  struct ima_field_data *field_data)
 {
-	if (datafmt != DATA_FMT_DIGEST)
+	if (datafmt != DATA_FMT_DIGEST && datafmt != DATA_FMT_DIGEST_WITH_ALGO)
 		ima_putc(m, &field_data->len, sizeof(u32));
 	ima_putc(m, field_data->data, field_data->len);
 }
@@ -107,14 +133,59 @@ void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
 	ima_show_template_field_data(m, show, DATA_FMT_DIGEST, field_data);
 }
 
+void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
+				 struct ima_field_data *field_data)
+{
+	ima_show_template_field_data(m, show, DATA_FMT_DIGEST_WITH_ALGO,
+				     field_data);
+}
+
 void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data)
 {
 	ima_show_template_field_data(m, show, DATA_FMT_STRING, field_data);
 }
 
+static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo,
+				       struct ima_field_data *field_data,
+				       bool size_limit)
+{
+	/*
+	 * digest formats:
+	 *  - DATA_FMT_DIGEST: digest
+	 *  - DATA_FMT_DIGEST_WITH_ALGO: [<hash algo>] + ':' + '\0' + digest,
+	 *    where <hash algo> is provided if the hash algoritm is not
+	 *    SHA1 or MD5
+	 */
+	u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
+	enum data_formats fmt = DATA_FMT_DIGEST;
+	u32 offset = 0;
+
+	if (!size_limit) {
+		fmt = DATA_FMT_DIGEST_WITH_ALGO;
+		if (hash_algo < HASH_ALGO__LAST)
+			offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1,
+					   "%s", hash_algo_name[hash_algo]);
+		buffer[offset] = ':';
+		offset += 2;
+	}
+
+	if (digest)
+		memcpy(buffer + offset, digest, digestsize);
+	else
+		/*
+		 * If digest is NULL, the event being recorded is a violation.
+		 * Make room for the digest by increasing the offset of
+		 * IMA_DIGEST_SIZE.
+		 */
+		offset += IMA_DIGEST_SIZE;
+
+	return ima_write_template_field_data(buffer, offset + digestsize,
+					     fmt, field_data);
+}
+
 /*
- * This function writes the digest of an event.
+ * This function writes the digest of an event (with size limit).
  */
 int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
 			 const unsigned char *filename,
@@ -124,8 +195,8 @@ int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
 		struct ima_digest_data hdr;
 		char digest[IMA_MAX_DIGEST_SIZE];
 	} hash;
-	u8 *cur_digest = hash.hdr.digest;
-	u32 cur_digestsize = IMA_DIGEST_SIZE;
+	u8 *cur_digest = NULL;
+	u32 cur_digestsize = 0;
 	struct inode *inode;
 	int result;
 
@@ -134,7 +205,7 @@ int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
 	if (!iint)		/* recording a violation. */
 		goto out;
 
-	if (iint->ima_hash->algo == ima_hash_algo) {
+	if (ima_template_hash_algo_allowed(iint->ima_hash->algo)) {
 		cur_digest = iint->ima_hash->digest;
 		cur_digestsize = iint->ima_hash->length;
 		goto out;
@@ -144,7 +215,8 @@ int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
 		return -EINVAL;
 
 	inode = file_inode(file);
-	hash.hdr.algo = ima_hash_algo;
+	hash.hdr.algo = ima_template_hash_algo_allowed(ima_hash_algo) ?
+	    ima_hash_algo : HASH_ALGO_SHA1;
 	result = ima_calc_file_hash(file, &hash.hdr);
 	if (result) {
 		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
@@ -152,20 +224,47 @@ int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
 				    "failed", result, 0);
 		return result;
 	}
+	cur_digest = hash.hdr.digest;
+	cur_digestsize = hash.hdr.length;
 out:
-	return ima_write_template_field_data(cur_digest, cur_digestsize,
-					     DATA_FMT_DIGEST, field_data);
+	return ima_eventdigest_init_common(cur_digest, cur_digestsize, -1,
+					   field_data, true);
 }
 
 /*
- * This function writes the name of an event.
+ * This function writes the digest of an event (without size limit).
  */
-int ima_eventname_init(struct integrity_iint_cache *iint, struct file *file,
-		       const unsigned char *filename,
-		       struct ima_field_data *field_data)
+int ima_eventdigest_ng_init(struct integrity_iint_cache *iint,
+			    struct file *file, const unsigned char *filename,
+			    struct ima_field_data *field_data)
+{
+	u8 *cur_digest = NULL, hash_algo = -1;
+	u32 cur_digestsize = 0;
+
+	/* If iint is NULL, we are recording a violation. */
+	if (!iint)
+		goto out;
+
+	cur_digest = iint->ima_hash->digest;
+	cur_digestsize = iint->ima_hash->length;
+
+	if (!ima_template_hash_algo_allowed(iint->ima_hash->algo))
+		hash_algo = iint->ima_hash->algo;
+out:
+	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
+					   hash_algo, field_data, false);
+}
+
+static int ima_eventname_init_common(struct integrity_iint_cache *iint,
+				     struct file *file,
+				     const unsigned char *filename,
+				     struct ima_field_data *field_data,
+				     bool size_limit)
 {
 	const char *cur_filename = NULL;
 	u32 cur_filename_len = 0;
+	enum data_formats fmt = size_limit ?
+	    DATA_FMT_EVENT_NAME : DATA_FMT_STRING;
 
 	BUG_ON(filename == NULL && file == NULL);
 
@@ -173,7 +272,7 @@ int ima_eventname_init(struct integrity_iint_cache *iint, struct file *file,
 		cur_filename = filename;
 		cur_filename_len = strlen(filename);
 
-		if (cur_filename_len <= IMA_EVENT_NAME_LEN_MAX)
+		if (!size_limit || cur_filename_len <= IMA_EVENT_NAME_LEN_MAX)
 			goto out;
 	}
 
@@ -188,5 +287,27 @@ int ima_eventname_init(struct integrity_iint_cache *iint, struct file *file,
 		cur_filename_len = IMA_EVENT_NAME_LEN_MAX;
 out:
 	return ima_write_template_field_data(cur_filename, cur_filename_len,
-					     DATA_FMT_EVENT_NAME, field_data);
+					     fmt, field_data);
+}
+
+/*
+ * This function writes the name of an event (with size limit).
+ */
+int ima_eventname_init(struct integrity_iint_cache *iint, struct file *file,
+		       const unsigned char *filename,
+		       struct ima_field_data *field_data)
+{
+	return ima_eventname_init_common(iint, file, filename,
+					 field_data, true);
+}
+
+/*
+ * This function writes the name of an event (without size limit).
+ */
+int ima_eventname_ng_init(struct integrity_iint_cache *iint, struct file *file,
+			  const unsigned char *filename,
+			  struct ima_field_data *field_data)
+{
+	return ima_eventname_init_common(iint, file, filename,
+					 field_data, false);
 }
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 2cecc83..16c5e78 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -20,6 +20,8 @@
 
 void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data);
+void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
+				 struct ima_field_data *field_data);
 void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data);
 int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
@@ -28,4 +30,10 @@ int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
 int ima_eventname_init(struct integrity_iint_cache *iint, struct file *file,
 		       const unsigned char *filename,
 		       struct ima_field_data *field_data);
+int ima_eventdigest_ng_init(struct integrity_iint_cache *iint,
+			    struct file *file, const unsigned char *filename,
+			    struct ima_field_data *field_data);
+int ima_eventname_ng_init(struct integrity_iint_cache *iint, struct file *file,
+			  const unsigned char *filename,
+			  struct ima_field_data *field_data);
 #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 16/20] ima: switch to new template management mechanism
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

This patch performs the switch to the new template mechanism by modifying
the functions ima_alloc_init_template(), ima_measurements_show() and
ima_ascii_measurements_show(). The old function ima_template_show() was
removed as it is no longer needed. Also, if the template descriptor used
to generate a measurement entry is not 'ima', the whole length of field
data stored for an entry is provided before the data itself through the
binary_runtime_measurement interface.

Changelog:
- unnecessary to use strncmp() (Mimi Zohar)
- create new variable 'field' in ima_alloc_init_template() (Roberto Sassu)
- use GFP_NOFS flag in ima_alloc_init_template() (Roberto Sassu)
- new variable 'num_fields' in ima_store_template() (Roberto Sassu,
  proposed by Mimi Zohar)
- rename ima_calc_buffer_hash/template_hash() to ima_calc_field_array_hash(),
  something more generic (Mimi, requested by Dmitry)
- sparse error fix - Fengguang Wu

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h          | 19 ++++-----
 security/integrity/ima/ima_api.c      | 73 +++++++++++------------------------
 security/integrity/ima/ima_crypto.c   | 31 +++++++++++----
 security/integrity/ima/ima_fs.c       | 54 ++++++++++++--------------
 security/integrity/ima/ima_template.c | 22 +++++++++++
 5 files changed, 101 insertions(+), 98 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e1f081d..72d013e 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -72,17 +72,11 @@ struct ima_template_desc {
 	struct ima_template_field **fields;
 };
 
-/* IMA inode template definition */
-struct ima_template_data {
-	u8 digest[IMA_DIGEST_SIZE];	/* sha1/md5 measurement hash */
-	char file_name[IMA_EVENT_NAME_LEN_MAX + 1];	/* name + \0 */
-};
-
 struct ima_template_entry {
 	u8 digest[TPM_DIGEST_SIZE];	/* sha1 or md5 measurement hash */
-	const char *template_name;
-	int template_len;
-	struct ima_template_data template;
+	struct ima_template_desc *template_desc; /* template descriptor */
+	u32 template_data_len;
+	struct ima_field_data template_data[0];	/* template related data */
 };
 
 struct ima_queue_entry {
@@ -102,14 +96,16 @@ 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 *data, int len,
-			 struct ima_digest_data *hash);
+int ima_calc_field_array_hash(struct ima_field_data *field_data, int num_fields,
+			      struct ima_digest_data *hash);
 int __init ima_calc_boot_aggregate(struct ima_digest_data *hash);
 void ima_add_violation(struct file *file, const unsigned char *filename,
 		       const char *op, const char *cause);
 int ima_init_crypto(void);
 void ima_putc(struct seq_file *m, void *data, int datalen);
 void ima_print_digest(struct seq_file *m, u8 *digest, int size);
+struct ima_template_desc *ima_template_desc_current(void);
+int ima_init_template(void);
 
 int ima_init_template(void);
 
@@ -146,7 +142,6 @@ int ima_alloc_init_template(struct integrity_iint_cache *iint,
 			    struct ima_template_entry **entry);
 int ima_store_template(struct ima_template_entry *entry, int violation,
 		       struct inode *inode, const unsigned char *filename);
-void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show);
 const char *ima_d_path(struct path *path, char **pathbuf);
 
 /* rbtree tree calls to lookup, insert, delete
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 29dd43d..974fbab 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -21,8 +21,6 @@
 #include <crypto/hash_info.h>
 #include "ima.h"
 
-static const char *IMA_TEMPLATE_NAME = "ima";
-
 /*
  * ima_alloc_init_template - create and initialize a new template entry
  */
@@ -30,52 +28,28 @@ int ima_alloc_init_template(struct integrity_iint_cache *iint,
 			    struct file *file, const unsigned char *filename,
 			    struct ima_template_entry **entry)
 {
-	struct ima_template_entry *e;
-	int result = 0;
+	struct ima_template_desc *template_desc = ima_template_desc_current();
+	int i, result = 0;
 
-	e = kzalloc(sizeof(**entry), GFP_NOFS);
-	if (!e)
+	*entry = kzalloc(sizeof(**entry) + template_desc->num_fields *
+				sizeof(struct ima_field_data), GFP_NOFS);
+	if (!*entry)
 		return -ENOMEM;
 
-	memset(&(e)->template, 0, sizeof(e->template));
-	if (!iint)		/* IMA measurement violation entry */
-		goto out;
-
-	if (iint->ima_hash->algo != ima_hash_algo) {
-		struct inode *inode;
-		struct {
-			struct ima_digest_data hdr;
-			char digest[IMA_MAX_DIGEST_SIZE];
-		} hash;
+	for (i = 0; i < template_desc->num_fields; i++) {
+		struct ima_template_field *field = template_desc->fields[i];
+		result = field->field_init(iint, file, filename,
+					   &((*entry)->template_data[i]));
+		if (result != 0)
+			goto out;
 
-		if (!file) {
-			result = -EINVAL;
-			goto out_free;
-		}
-
-		inode = file_inode(file);
-		hash.hdr.algo = ima_hash_algo;
-		hash.hdr.length = SHA1_DIGEST_SIZE;
-		result = ima_calc_file_hash(file, &hash.hdr);
-		if (result) {
-			integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
-					    filename, "collect_data",
-					    "failed", result, 0);
-			goto out_free;
-		} else
-			memcpy(e->template.digest, hash.hdr.digest,
-			       hash.hdr.length);
-	} else
-		memcpy(e->template.digest, iint->ima_hash->digest,
-		       iint->ima_hash->length);
-out:
-	strcpy(e->template.file_name,
-	       (strlen(filename) > IMA_EVENT_NAME_LEN_MAX && file != NULL) ?
-	       file->f_dentry->d_name.name : filename);
-	*entry = e;
+		(*entry)->template_data_len += (*entry)->template_data[i].len;
+	}
+	(*entry)->template_desc = template_desc;
 	return 0;
-out_free:
-	kfree(e);
+out:
+	kfree(*entry);
+	*entry = NULL;
 	return result;
 }
 
@@ -101,24 +75,23 @@ int ima_store_template(struct ima_template_entry *entry,
 {
 	const char *op = "add_template_measure";
 	const char *audit_cause = "hashing_error";
+	char *template_name = entry->template_desc->name;
 	int result;
 	struct {
 		struct ima_digest_data hdr;
 		char digest[TPM_DIGEST_SIZE];
 	} hash;
 
-	memset(entry->digest, 0, sizeof(entry->digest));
-	entry->template_name = IMA_TEMPLATE_NAME;
-	entry->template_len = sizeof(entry->template);
-
 	if (!violation) {
+		int num_fields = entry->template_desc->num_fields;
+
 		/* this function uses default algo */
 		hash.hdr.algo = HASH_ALGO_SHA1;
-		result = ima_calc_buffer_hash(&entry->template,
-					      entry->template_len, &hash.hdr);
+		result = ima_calc_field_array_hash(&entry->template_data[0],
+						   num_fields, &hash.hdr);
 		if (result < 0) {
 			integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode,
-					    entry->template_name, op,
+					    template_name, op,
 					    audit_cause, result, 0);
 			return result;
 		}
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 22be23f..84a6a4c 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -137,26 +137,43 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 }
 
 /*
- * Calculate the hash of a given buffer
+ * Calculate the hash of template data
  */
-static int ima_calc_buffer_hash_tfm(const void *buf, int len,
-				    struct ima_digest_data *hash,
-				    struct crypto_shash *tfm)
+static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
+					 int num_fields,
+					 struct ima_digest_data *hash,
+					 struct crypto_shash *tfm)
 {
 	struct {
 		struct shash_desc shash;
 		char ctx[crypto_shash_descsize(tfm)];
 	} desc;
+	int rc, i;
 
 	desc.shash.tfm = tfm;
 	desc.shash.flags = 0;
 
 	hash->length = crypto_shash_digestsize(tfm);
 
-	return crypto_shash_digest(&desc.shash, buf, len, hash->digest);
+	rc = crypto_shash_init(&desc.shash);
+	if (rc != 0)
+		return rc;
+
+	for (i = 0; i < num_fields; i++) {
+		rc = crypto_shash_update(&desc.shash, field_data[i].data,
+					 field_data[i].len);
+		if (rc)
+			break;
+	}
+
+	if (!rc)
+		rc = crypto_shash_final(&desc.shash, hash->digest);
+
+	return rc;
 }
 
-int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
+int ima_calc_field_array_hash(struct ima_field_data *field_data, int num_fields,
+			      struct ima_digest_data *hash)
 {
 	struct crypto_shash *tfm;
 	int rc;
@@ -165,7 +182,7 @@ int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
 	if (IS_ERR(tfm))
 		return PTR_ERR(tfm);
 
-	rc = ima_calc_buffer_hash_tfm(buf, len, hash, tfm);
+	rc = ima_calc_field_array_hash_tfm(field_data, num_fields, hash, tfm);
 
 	ima_free_tfm(tfm);
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index f9d8d05..0575d65 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -111,6 +111,7 @@ void ima_putc(struct seq_file *m, void *data, int datalen)
  *       char[20]=template digest
  *       32bit-le=template name size
  *       char[n]=template name
+ *       [eventdata length]
  *       eventdata[n]=template specific data
  */
 static int ima_measurements_show(struct seq_file *m, void *v)
@@ -120,6 +121,7 @@ static int ima_measurements_show(struct seq_file *m, void *v)
 	struct ima_template_entry *e;
 	int namelen;
 	u32 pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+	int i;
 
 	/* get entry */
 	e = qe->entry;
@@ -137,15 +139,22 @@ static int ima_measurements_show(struct seq_file *m, void *v)
 	ima_putc(m, e->digest, TPM_DIGEST_SIZE);
 
 	/* 3rd: template name size */
-	namelen = strlen(e->template_name);
+	namelen = strlen(e->template_desc->name);
 	ima_putc(m, &namelen, sizeof namelen);
 
 	/* 4th:  template name */
-	ima_putc(m, (void *)e->template_name, namelen);
+	ima_putc(m, e->template_desc->name, namelen);
+
+	/* 5th:  template length (except for 'ima' template) */
+	if (strcmp(e->template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
+		ima_putc(m, &e->template_data_len,
+			 sizeof(e->template_data_len));
 
-	/* 5th:  template specific data */
-	ima_template_show(m, (struct ima_template_data *)&e->template,
-			  IMA_SHOW_BINARY);
+	/* 6th:  template specific data */
+	for (i = 0; i < e->template_desc->num_fields; i++) {
+		e->template_desc->fields[i]->field_show(m, IMA_SHOW_BINARY,
+							&e->template_data[i]);
+	}
 	return 0;
 }
 
@@ -176,33 +185,13 @@ void ima_print_digest(struct seq_file *m, u8 *digest, int size)
 		seq_printf(m, "%02x", *(digest + i));
 }
 
-void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show)
-{
-	struct ima_template_data *entry = e;
-	int namelen;
-
-	switch (show) {
-	case IMA_SHOW_ASCII:
-		ima_print_digest(m, entry->digest, IMA_DIGEST_SIZE);
-		seq_printf(m, " %s\n", entry->file_name);
-		break;
-	case IMA_SHOW_BINARY:
-		ima_putc(m, entry->digest, IMA_DIGEST_SIZE);
-
-		namelen = strlen(entry->file_name);
-		ima_putc(m, &namelen, sizeof namelen);
-		ima_putc(m, entry->file_name, namelen);
-	default:
-		break;
-	}
-}
-
 /* print in ascii */
 static int ima_ascii_measurements_show(struct seq_file *m, void *v)
 {
 	/* the list never shrinks, so we don't need a lock here */
 	struct ima_queue_entry *qe = v;
 	struct ima_template_entry *e;
+	int i;
 
 	/* get entry */
 	e = qe->entry;
@@ -216,11 +205,18 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
 	ima_print_digest(m, e->digest, TPM_DIGEST_SIZE);
 
 	/* 3th:  template name */
-	seq_printf(m, " %s ", e->template_name);
+	seq_printf(m, " %s", e->template_desc->name);
 
 	/* 4th:  template specific data */
-	ima_template_show(m, (struct ima_template_data *)&e->template,
-			  IMA_SHOW_ASCII);
+	for (i = 0; i < e->template_desc->num_fields; i++) {
+		seq_puts(m, " ");
+		if (e->template_data[i].len == 0)
+			continue;
+
+		e->template_desc->fields[i]->field_show(m, IMA_SHOW_ASCII,
+							&e->template_data[i]);
+	}
+	seq_puts(m, "\n");
 	return 0;
 }
 
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index bf38d1a..1c4cf19 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -31,6 +31,20 @@ static struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_string},
 };
 
+static struct ima_template_desc *ima_template;
+
+static struct ima_template_desc *lookup_template_desc(const char *name)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(defined_templates); i++) {
+		if (strcmp(defined_templates[i].name, name) == 0)
+			return defined_templates + i;
+	}
+
+	return NULL;
+}
+
 static struct ima_template_field *lookup_template_field(const char *field_id)
 {
 	int i;
@@ -110,6 +124,14 @@ static int init_defined_templates(void)
 	return result;
 }
 
+struct ima_template_desc *ima_template_desc_current(void)
+{
+	if (!ima_template)
+		ima_template = lookup_template_desc(IMA_TEMPLATE_IMA_NAME);
+
+	return ima_template;
+}
+
 int ima_init_template(void)
 {
 	int result;
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 17/20] ima: defer determining the appraisal hash algorithm for 'ima' template
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

The same hash algorithm should be used for calculating the file
data hash for the IMA measurement list, as for appraising the file
data integrity.  (The appraise hash algorithm is stored in the
'security.ima' extended attribute.)  The exception is when the
reference file data hash digest, stored in the extended attribute,
is larger than the one supported by the template.  In this case,
the file data hash needs to be calculated twice, once for the
measurement list and, again, for appraisal.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d9965bf..c2f6d96 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -145,6 +145,7 @@ static int process_measurement(struct file *file, const char *filename,
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint;
+	struct ima_template_desc *template_desc = ima_template_desc_current();
 	char *pathbuf = NULL;
 	const char *pathname = NULL;
 	int rc = -ENOMEM, action, must_appraise, _func;
@@ -188,7 +189,10 @@ static int process_measurement(struct file *file, const char *filename,
 		goto out_digsig;
 	}
 
-	if (action & IMA_APPRAISE_SUBMASK)
+	if (strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) == 0) {
+		if (action & IMA_APPRAISE_SUBMASK)
+			xattr_ptr = &xattr_value;
+	} else
 		xattr_ptr = &xattr_value;
 
 	rc = ima_collect_measurement(iint, file, xattr_ptr, &xattr_len);
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 19/20] ima: define kernel parameter 'ima_template=' to change configured default
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

This patch allows users to specify from the kernel command line the
template descriptor, among those defined, that will be used to generate
and display measurement entries. If an user specifies a wrong template,
IMA reverts to the template descriptor set in the kernel configuration.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 Documentation/kernel-parameters.txt   |  5 +++++
 security/integrity/ima/ima_template.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index cb5daa1..7c60db3 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1139,6 +1139,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			programs exec'd, files mmap'd for exec, and all files
 			opened for read by uid=0.
 
+	ima_template=   [IMA]
+			Select one of defined IMA measurements template formats.
+			Formats: { "ima" | "ima-ng" }
+			Default: "ima-ng"
+
 	init=		[KNL]
 			Format: <full_path>
 			Run specified binary instead of /sbin/init as init
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index c28ff9b..0002214 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -12,6 +12,8 @@
  * File: ima_template.c
  *      Helpers to manage template descriptors.
  */
+#include <crypto/hash_info.h>
+
 #include "ima.h"
 #include "ima_template_lib.h"
 
@@ -32,6 +34,35 @@ static struct ima_template_field supported_fields[] = {
 };
 
 static struct ima_template_desc *ima_template;
+static struct ima_template_desc *lookup_template_desc(const char *name);
+
+static int __init ima_template_setup(char *str)
+{
+	struct ima_template_desc *template_desc;
+	int template_len = strlen(str);
+
+	/*
+	 * Verify that a template with the supplied name exists.
+	 * If not, use CONFIG_IMA_DEFAULT_TEMPLATE.
+	 */
+	template_desc = lookup_template_desc(str);
+	if (!template_desc)
+		return 1;
+
+	/*
+	 * Verify whether the current hash algorithm is supported
+	 * by the 'ima' template.
+	 */
+	if (template_len == 3 && strcmp(str, IMA_TEMPLATE_IMA_NAME) == 0 &&
+	    ima_hash_algo != HASH_ALGO_SHA1 && ima_hash_algo != HASH_ALGO_MD5) {
+		pr_err("IMA: template does not support hash alg\n");
+		return 1;
+	}
+
+	ima_template = template_desc;
+	return 1;
+}
+__setup("ima_template=", ima_template_setup);
 
 static struct ima_template_desc *lookup_template_desc(const char *name)
 {
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 20/20] ima: enable support for larger default filedata hash algorithms
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, linux-crypto, David Howells, Mimi Zohar,
	Roberto Sassu
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

The IMA measurement list contains two hashes - a template data hash
and a filedata hash.  The template data hash is committed to the TPM,
which is limited, by the TPM v1.2 specification, to 20 bytes.  The
filedata hash is defined as 20 bytes as well.

Now that support for variable length measurement list templates was
added, the filedata hash is not limited to 20 bytes.  This patch adds
Kconfig support for defining larger default filedata hash algorithms
and replacing the builtin default with one specified on the kernel
command line.

<uapi/linux/hash_info.h> contains a list of hash algorithms.  The
Kconfig default hash algorithm is a subset of this list, but any hash
algorithm included in the list can be specified at boot, using the
'ima_hash=' kernel command line option.

Changelog:
- support hashes that are configured
- use generic HASH_ALGO_ definitions
- add Kconfig support
- hash_setup must be called only once (Dmitry)
- removed trailing whitespaces (Roberto Sassu)

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 Documentation/kernel-parameters.txt |  6 +++++-
 security/integrity/ima/Kconfig      | 35 +++++++++++++++++++++++++++++++++++
 security/integrity/ima/ima_main.c   | 26 ++++++++++++++++++++++++--
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7c60db3..f328a60 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1130,9 +1130,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			owned by uid=0.
 
 	ima_hash=	[IMA]
-			Format: { "sha1" | "md5" }
+			Format: { md5 | sha1 | rmd160 | sha256 | sha384
+				   | sha512 | ... }
 			default: "sha1"
 
+			The list of supported hash algorithms is defined
+			in crypto/hash_info.h.
+
 	ima_tcb		[IMA]
 			Load a policy which meets the needs of the Trusted
 			Computing Base.  This means IMA will measure all
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index de26cc8..dab1898 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -71,6 +71,41 @@ config IMA_DEFAULT_TEMPLATE
 	default "ima" if IMA_TEMPLATE
 	default "ima-ng" if IMA_NG_TEMPLATE
 
+choice
+	prompt "Default integrity hash algorithm"
+	default IMA_DEFAULT_HASH_SHA1
+	depends on IMA
+	help
+	   Select the default hash algorithm used for the measurement
+	   list, integrity appraisal and audit log.  The compiled default
+	   hash algorithm can be overwritten using the kernel command
+	   line 'ima_hash=' option.
+
+	config IMA_DEFAULT_HASH_SHA1
+		bool "SHA1 (default)"
+		depends on CRYPTO_SHA1
+
+	config IMA_DEFAULT_HASH_SHA256
+		bool "SHA256"
+		depends on CRYPTO_SHA256 && IMA_NG_TEMPLATE
+
+	config IMA_DEFAULT_HASH_SHA512
+		bool "SHA512"
+		depends on CRYPTO_SHA512 && IMA_NG_TEMPLATE
+
+	config IMA_DEFAULT_HASH_WP512
+		bool "WP512"
+		depends on CRYPTO_WP512 && IMA_NG_TEMPLATE
+endchoice
+
+config IMA_DEFAULT_HASH
+	string
+	depends on IMA
+	default "sha1" if IMA_DEFAULT_HASH_SHA1
+	default "sha256" if IMA_DEFAULT_HASH_SHA256
+	default "sha512" if IMA_DEFAULT_HASH_SHA512
+	default "wp512" if IMA_DEFAULT_HASH_WP512
+
 config IMA_APPRAISE
 	bool "Appraise integrity measurements"
 	depends on IMA
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c2f6d96..fc1ff82 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -37,11 +37,32 @@ int ima_appraise;
 #endif
 
 int ima_hash_algo = HASH_ALGO_SHA1;
+static int hash_setup_done;
 
 static int __init hash_setup(char *str)
 {
-	if (strncmp(str, "md5", 3) == 0)
-		ima_hash_algo = HASH_ALGO_MD5;
+	struct ima_template_desc *template_desc = ima_template_desc_current();
+	int i;
+
+	if (hash_setup_done)
+		return 1;
+
+	if (strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) == 0) {
+		if (strncmp(str, "sha1", 4) == 0)
+			ima_hash_algo = HASH_ALGO_SHA1;
+		else if (strncmp(str, "md5", 3) == 0)
+			ima_hash_algo = HASH_ALGO_MD5;
+		goto out;
+	}
+
+	for (i = 0; i < HASH_ALGO__LAST; i++) {
+		if (strcmp(str, hash_algo_name[i]) == 0) {
+			ima_hash_algo = i;
+			break;
+		}
+	}
+out:
+	hash_setup_done = 1;
 	return 1;
 }
 __setup("ima_hash=", hash_setup);
@@ -306,6 +327,7 @@ static int __init init_ima(void)
 {
 	int error;
 
+	hash_setup(CONFIG_IMA_DEFAULT_HASH);
 	error = ima_init();
 	if (!error)
 		ima_initialized = 1;
-- 
1.8.1.4


^ permalink raw reply related

* [RFC][PATCH 08/20] ima: support arbitrary hash algorithms in ima_calc_buffer_hash
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Dmitry Kasatkin, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>

ima_calc_buffer_hash will be used with different hash algorithms.
This patch provides support for arbitrary hash algorithms in
ima_calc_buffer_hash.

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_api.c    |  3 +++
 security/integrity/ima/ima_crypto.c | 28 ++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 2cc5dcc..bc1d128 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 #include <linux/xattr.h>
 #include <linux/evm.h>
+#include <crypto/hash_info.h>
 #include "ima.h"
 
 static const char *IMA_TEMPLATE_NAME = "ima";
@@ -54,6 +55,8 @@ int ima_store_template(struct ima_template_entry *entry,
 	entry->template_len = sizeof(entry->template);
 
 	if (!violation) {
+		/* this function uses default algo */
+		hash.hdr.algo = HASH_ALGO_SHA1;
 		result = ima_calc_buffer_hash(&entry->template,
 					      entry->template_len, &hash.hdr);
 		if (result < 0) {
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index e5d3ebf..e2be252 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -139,23 +139,39 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 /*
  * Calculate the hash of a given buffer
  */
-int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
+static int ima_calc_buffer_hash_tfm(const void *buf, int len,
+				    struct ima_digest_data *hash,
+				    struct crypto_shash *tfm)
 {
 	struct {
 		struct shash_desc shash;
-		char ctx[crypto_shash_descsize(ima_shash_tfm)];
+		char ctx[crypto_shash_descsize(tfm)];
 	} desc;
 
-	desc.shash.tfm = ima_shash_tfm;
+	desc.shash.tfm = tfm;
 	desc.shash.flags = 0;
 
-	/* this function uses default algo */
-	hash->algo = ima_hash_algo;
-	hash->length = crypto_shash_digestsize(ima_shash_tfm);
+	hash->length = crypto_shash_digestsize(tfm);
 
 	return crypto_shash_digest(&desc.shash, buf, len, hash->digest);
 }
 
+int ima_calc_buffer_hash(const void *buf, int 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 = ima_calc_buffer_hash_tfm(buf, len, hash, tfm);
+
+	ima_free_tfm(tfm);
+
+	return rc;
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
 	if (!ima_used_chip)
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 07/20] ima: provide dedicated hash algo allocation function
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Dmitry Kasatkin, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Dmitry Kasatkin <d.kasatkin@samsung.com>

This patch provides dedicated hash algo allocation and
deallocation function which can be used by different clients.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_crypto.c | 43 +++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 872c669..e5d3ebf 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -39,6 +39,28 @@ int ima_init_crypto(void)
 	return 0;
 }
 
+static struct crypto_shash *ima_alloc_tfm(enum hash_algo algo)
+{
+	struct crypto_shash *tfm = ima_shash_tfm;
+	int rc;
+
+	if (algo != ima_hash_algo && algo < HASH_ALGO__LAST) {
+		tfm = crypto_alloc_shash(hash_algo_name[algo], 0, 0);
+		if (IS_ERR(tfm)) {
+			rc = PTR_ERR(tfm);
+			pr_err("Can not allocate %s (reason: %d)\n",
+			       hash_algo_name[algo], rc);
+		}
+	}
+	return tfm;
+}
+
+static void ima_free_tfm(struct crypto_shash *tfm)
+{
+	if (tfm != ima_shash_tfm)
+		crypto_free_shash(tfm);
+}
+
 /*
  * Calculate the MD5/SHA1 file digest
  */
@@ -57,6 +79,8 @@ static int ima_calc_file_hash_tfm(struct file *file,
 	desc.shash.tfm = tfm;
 	desc.shash.flags = 0;
 
+	hash->length = crypto_shash_digestsize(tfm);
+
 	rc = crypto_shash_init(&desc.shash);
 	if (rc != 0)
 		return rc;
@@ -98,25 +122,16 @@ out:
 
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 {
-	struct crypto_shash *tfm = ima_shash_tfm;
+	struct crypto_shash *tfm;
 	int rc;
 
-	if (hash->algo != ima_hash_algo && hash->algo < HASH_ALGO__LAST) {
-		tfm = crypto_alloc_shash(hash_algo_name[hash->algo], 0, 0);
-		if (IS_ERR(tfm)) {
-			rc = PTR_ERR(tfm);
-			pr_err("Can not allocate %s (reason: %d)\n",
-			       hash_algo_name[hash->algo], rc);
-			return rc;
-		}
-	}
-
-	hash->length = crypto_shash_digestsize(tfm);
+	tfm = ima_alloc_tfm(hash->algo);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
 
 	rc = ima_calc_file_hash_tfm(file, hash, tfm);
 
-	if (tfm != ima_shash_tfm)
-		crypto_free_shash(tfm);
+	ima_free_tfm(tfm);
 
 	return rc;
 }
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 09/20] ima: ima_calc_boot_agregate must use SHA1
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Dmitry Kasatkin, linux-crypto, David Howells, Roberto Sassu,
	Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>

With multiple hash algorithms, ima_hash_tfm is no longer guarantied to be sha1.
Need to force to use sha1.

Changelog:
- pass ima_digest_data to ima_calc_boot_aggregate() instead of char *
  (Roberto Sassu);
- create an ima_digest_data structure in ima_add_boot_aggregate()
  (Roberto Sassu);
- pass hash->algo to ima_alloc_tfm() (Roberto Sassu, reported by Dmitry).
- "move hash definition in ima_add_boot_aggregate()" commit hunk to here.
- sparse warning fix - Fengguang Wu

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h        |  2 +-
 security/integrity/ima/ima_crypto.c | 24 +++++++++++++++++++++---
 security/integrity/ima/ima_init.c   | 10 +++++++++-
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 52393ed..e0e1cde 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -73,7 +73,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
 int ima_calc_buffer_hash(const void *data, int len,
 			 struct ima_digest_data *hash);
-int ima_calc_boot_aggregate(char *digest);
+int __init ima_calc_boot_aggregate(struct ima_digest_data *hash);
 void ima_add_violation(struct inode *inode, const unsigned char *filename,
 		       const char *op, const char *cause);
 int ima_init_crypto(void);
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index e2be252..22be23f 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -184,16 +184,17 @@ static void __init ima_pcrread(int idx, u8 *pcr)
 /*
  * Calculate the boot aggregate hash
  */
-int __init ima_calc_boot_aggregate(char *digest)
+static int __init ima_calc_boot_aggregate_tfm(char *digest,
+					      struct crypto_shash *tfm)
 {
 	u8 pcr_i[TPM_DIGEST_SIZE];
 	int rc, i;
 	struct {
 		struct shash_desc shash;
-		char ctx[crypto_shash_descsize(ima_shash_tfm)];
+		char ctx[crypto_shash_descsize(tfm)];
 	} desc;
 
-	desc.shash.tfm = ima_shash_tfm;
+	desc.shash.tfm = tfm;
 	desc.shash.flags = 0;
 
 	rc = crypto_shash_init(&desc.shash);
@@ -210,3 +211,20 @@ int __init ima_calc_boot_aggregate(char *digest)
 		crypto_shash_final(&desc.shash, digest);
 	return rc;
 }
+
+int __init ima_calc_boot_aggregate(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);
+
+	hash->length = crypto_shash_digestsize(tfm);
+	rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm);
+
+	ima_free_tfm(tfm);
+
+	return rc;
+}
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 9d0243c..77cd500 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -18,6 +18,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 */
@@ -46,6 +47,10 @@ static void __init ima_add_boot_aggregate(void)
 	const char *audit_cause = "ENOMEM";
 	int result = -ENOMEM;
 	int violation = 1;
+	struct {
+		struct ima_digest_data hdr;
+		char digest[TPM_DIGEST_SIZE];
+	} hash;
 
 	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
@@ -56,12 +61,15 @@ static void __init ima_add_boot_aggregate(void)
 		IMA_EVENT_NAME_LEN_MAX);
 	if (ima_used_chip) {
 		violation = 0;
-		result = ima_calc_boot_aggregate(entry->template.digest);
+		hash.hdr.algo = HASH_ALGO_SHA1;
+		result = ima_calc_boot_aggregate(&hash.hdr);
 		if (result < 0) {
 			audit_cause = "hashing_error";
 			kfree(entry);
 			goto err_out;
 		}
+		memcpy(entry->template.digest, hash.hdr.digest,
+		       hash.hdr.length);
 	}
 	result = ima_store_template(entry, violation, NULL);
 	if (result < 0)
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 10/20] ima: pass the file descriptor to ima_add_violation()
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

Pass the file descriptor instead of the inode to ima_add_violation(),
to make the latter consistent with ima_store_measurement() in
preparation for the new template architecture.

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h      | 2 +-
 security/integrity/ima/ima_api.c  | 3 ++-
 security/integrity/ima/ima_main.c | 5 ++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e0e1cde..d7bec6f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -74,7 +74,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
 int ima_calc_buffer_hash(const void *data, int len,
 			 struct ima_digest_data *hash);
 int __init ima_calc_boot_aggregate(struct ima_digest_data *hash);
-void ima_add_violation(struct inode *inode, const unsigned char *filename,
+void ima_add_violation(struct file *file, const unsigned char *filename,
 		       const char *op, const char *cause);
 int ima_init_crypto(void);
 
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index bc1d128..98160a3 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -78,10 +78,11 @@ int ima_store_template(struct ima_template_entry *entry,
  * By extending the PCR with 0xFF's instead of with zeroes, the PCR
  * value is invalidated.
  */
-void ima_add_violation(struct inode *inode, const unsigned char *filename,
+void ima_add_violation(struct file *file, const unsigned char *filename,
 		       const char *op, const char *cause)
 {
 	struct ima_template_entry *entry;
+	struct inode *inode = file->f_dentry->d_inode;
 	int violation = 1;
 	int result;
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c22027e..d9965bf 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -94,10 +94,9 @@ out:
 		pathname = dentry->d_name.name;
 
 	if (send_tomtou)
-		ima_add_violation(inode, pathname,
-				  "invalid_pcr", "ToMToU");
+		ima_add_violation(file, pathname, "invalid_pcr", "ToMToU");
 	if (send_writers)
-		ima_add_violation(inode, pathname,
+		ima_add_violation(file, pathname,
 				  "invalid_pcr", "open_writers");
 	kfree(pathbuf);
 }
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 12/20] ima: define new function ima_alloc_init_template() to API
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

Instead of allocating and initializing the template entry from multiple
places (eg. boot aggregate, violation, and regular measurements), this
patch defines a new function called ima_alloc_init_template().  The new
function allocates and initializes the measurement entry with the inode
digest and the filename.

In respect to the current behavior, it truncates the file name passed
in the 'filename' argument if the latter's size is greater than 255 bytes
and the passed file descriptor is NULL.

Changelog:
- initialize 'hash' variable for non TPM case - Mimi
- conform to expectation for 'iint' to be defined as a pointer. - Mimi
- add missing 'file' dependency for recalculating file hash. - Mimi

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h      |  3 ++
 security/integrity/ima/ima_api.c  | 88 ++++++++++++++++++++++++++-------------
 security/integrity/ima/ima_init.c | 24 ++++++-----
 3 files changed, 76 insertions(+), 39 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 27d2ffb..da03d33 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -107,6 +107,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   const unsigned char *filename);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename);
+int ima_alloc_init_template(struct integrity_iint_cache *iint,
+			    struct file *file, const unsigned char *filename,
+			    struct ima_template_entry **entry);
 int ima_store_template(struct ima_template_entry *entry, int violation,
 		       struct inode *inode, const unsigned char *filename);
 void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index a0fe504..29dd43d 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -24,6 +24,62 @@
 static const char *IMA_TEMPLATE_NAME = "ima";
 
 /*
+ * ima_alloc_init_template - create and initialize a new template entry
+ */
+int ima_alloc_init_template(struct integrity_iint_cache *iint,
+			    struct file *file, const unsigned char *filename,
+			    struct ima_template_entry **entry)
+{
+	struct ima_template_entry *e;
+	int result = 0;
+
+	e = kzalloc(sizeof(**entry), GFP_NOFS);
+	if (!e)
+		return -ENOMEM;
+
+	memset(&(e)->template, 0, sizeof(e->template));
+	if (!iint)		/* IMA measurement violation entry */
+		goto out;
+
+	if (iint->ima_hash->algo != ima_hash_algo) {
+		struct inode *inode;
+		struct {
+			struct ima_digest_data hdr;
+			char digest[IMA_MAX_DIGEST_SIZE];
+		} hash;
+
+		if (!file) {
+			result = -EINVAL;
+			goto out_free;
+		}
+
+		inode = file_inode(file);
+		hash.hdr.algo = ima_hash_algo;
+		hash.hdr.length = SHA1_DIGEST_SIZE;
+		result = ima_calc_file_hash(file, &hash.hdr);
+		if (result) {
+			integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
+					    filename, "collect_data",
+					    "failed", result, 0);
+			goto out_free;
+		} else
+			memcpy(e->template.digest, hash.hdr.digest,
+			       hash.hdr.length);
+	} else
+		memcpy(e->template.digest, iint->ima_hash->digest,
+		       iint->ima_hash->length);
+out:
+	strcpy(e->template.file_name,
+	       (strlen(filename) > IMA_EVENT_NAME_LEN_MAX && file != NULL) ?
+	       file->f_dentry->d_name.name : filename);
+	*entry = e;
+	return 0;
+out_free:
+	kfree(e);
+	return result;
+}
+
+/*
  * ima_store_template - store ima template measurements
  *
  * Calculate the hash of a template entry, add the template entry
@@ -90,13 +146,11 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 	/* can overflow, only indicator */
 	atomic_long_inc(&ima_htable.violations);
 
-	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
-	if (!entry) {
+	result = ima_alloc_init_template(NULL, file, filename, &entry);
+	if (result < 0) {
 		result = -ENOMEM;
 		goto err_out;
 	}
-	memset(&entry->template, 0, sizeof(entry->template));
-	strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX);
 	result = ima_store_template(entry, violation, inode, filename);
 	if (result < 0)
 		kfree(entry);
@@ -220,34 +274,12 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 	if (iint->flags & IMA_MEASURED)
 		return;
 
-	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
-	if (!entry) {
+	result = ima_alloc_init_template(iint, file, filename, &entry);
+	if (result < 0) {
 		integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
 				    op, audit_cause, result, 0);
 		return;
 	}
-	memset(&entry->template, 0, sizeof(entry->template));
-	if (iint->ima_hash->algo != ima_hash_algo) {
-		struct {
-			struct ima_digest_data hdr;
-			char digest[IMA_MAX_DIGEST_SIZE];
-		} hash;
-
-		hash.hdr.algo = ima_hash_algo;
-		result = ima_calc_file_hash(file, &hash.hdr);
-		if (result)
-			integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
-					    filename, "collect_data", "failed",
-					    result, 0);
-		else
-			memcpy(entry->template.digest, hash.hdr.digest,
-			       hash.hdr.length);
-	} else
-		memcpy(entry->template.digest, iint->ima_hash->digest,
-		       iint->ima_hash->length);
-	strcpy(entry->template.file_name,
-	       (strlen(filename) > IMA_EVENT_NAME_LEN_MAX) ?
-	       file->f_dentry->d_name.name : filename);
 
 	result = ima_store_template(entry, violation, inode, filename);
 	if (!result || result == -EEXIST)
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index d42fac3..50e15e6 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -43,34 +43,36 @@ int ima_used_chip;
 static void __init ima_add_boot_aggregate(void)
 {
 	struct ima_template_entry *entry;
+	struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
 	const char *op = "add_boot_aggregate";
 	const char *audit_cause = "ENOMEM";
 	int result = -ENOMEM;
-	int violation = 1;
+	int violation = 0;
 	struct {
 		struct ima_digest_data hdr;
 		char digest[TPM_DIGEST_SIZE];
 	} hash;
 
-	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
-	if (!entry)
-		goto err_out;
+	memset(iint, 0, sizeof(*iint));
+	memset(&hash, 0, sizeof(hash));
+	iint->ima_hash = &hash.hdr;
+	iint->ima_hash->algo = HASH_ALGO_SHA1;
+	iint->ima_hash->length = SHA1_DIGEST_SIZE;
 
-	memset(&entry->template, 0, sizeof(entry->template));
-	strncpy(entry->template.file_name, boot_aggregate_name,
-		IMA_EVENT_NAME_LEN_MAX);
 	if (ima_used_chip) {
-		violation = 0;
-		hash.hdr.algo = HASH_ALGO_SHA1;
 		result = ima_calc_boot_aggregate(&hash.hdr);
 		if (result < 0) {
 			audit_cause = "hashing_error";
 			kfree(entry);
 			goto err_out;
 		}
-		memcpy(entry->template.digest, hash.hdr.digest,
-		       hash.hdr.length);
 	}
+
+	result = ima_alloc_init_template(iint, NULL, boot_aggregate_name,
+					 &entry);
+	if (result < 0)
+		return;
+
 	result = ima_store_template(entry, violation, NULL,
 				    boot_aggregate_name);
 	if (result < 0)
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 14/20] ima: define template fields library and new helpers
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

This patch defines a library containing two initial template fields,
inode digest (d) and file name (n), the 'ima' template descriptor,
whose format is 'd|n', and two helper functions,
ima_write_template_field_data() and ima_show_template_field_data().

Changelog:
- replace ima_eventname_init() parameter NULL checking with BUG_ON.
  (suggested by Mimi)
- include "new template fields for inode digest (d) and file name (n)"
  definitions to fix a compiler warning.  - Mimi
- unnecessary to prefix static function names with 'ima_'. remove
  prefix to resolve Lindent formatting changes. - Mimi
- abbreviated/removed inline comments - Mimi

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/Makefile           |   2 +-
 security/integrity/ima/ima.h              |   5 +
 security/integrity/ima/ima_fs.c           |   4 +-
 security/integrity/ima/ima_template.c     |  15 ++-
 security/integrity/ima/ima_template_lib.c | 192 ++++++++++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  31 +++++
 6 files changed, 241 insertions(+), 8 deletions(-)
 create mode 100644 security/integrity/ima/ima_template_lib.c
 create mode 100644 security/integrity/ima/ima_template_lib.h

diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 7fe4ae3..d79263d 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -6,5 +6,5 @@
 obj-$(CONFIG_IMA) += ima.o
 
 ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
-	 ima_policy.o ima_template.o
+	 ima_policy.o ima_template.o ima_template_lib.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c85718f..e1f081d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -39,6 +39,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
 #define IMA_TEMPLATE_FIELD_ID_MAX_LEN	16
 #define IMA_TEMPLATE_NUM_FIELDS_MAX	15
 
+#define IMA_TEMPLATE_IMA_NAME "ima"
+#define IMA_TEMPLATE_IMA_FMT "d|n"
+
 /* set during initialization */
 extern int ima_initialized;
 extern int ima_used_chip;
@@ -105,6 +108,8 @@ int __init ima_calc_boot_aggregate(struct ima_digest_data *hash);
 void ima_add_violation(struct file *file, const unsigned char *filename,
 		       const char *op, const char *cause);
 int ima_init_crypto(void);
+void ima_putc(struct seq_file *m, void *data, int datalen);
+void ima_print_digest(struct seq_file *m, u8 *digest, int size);
 
 int ima_init_template(void);
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 89adf1d..f9d8d05 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -100,7 +100,7 @@ static void ima_measurements_stop(struct seq_file *m, void *v)
 {
 }
 
-static void ima_putc(struct seq_file *m, void *data, int datalen)
+void ima_putc(struct seq_file *m, void *data, int datalen)
 {
 	while (datalen--)
 		seq_putc(m, *(char *)data++);
@@ -168,7 +168,7 @@ static const struct file_operations ima_measurements_ops = {
 	.release = seq_release,
 };
 
-static void ima_print_digest(struct seq_file *m, u8 *digest, int size)
+void ima_print_digest(struct seq_file *m, u8 *digest, int size)
 {
 	int i;
 
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 7e86783..8100422 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -13,15 +13,20 @@
  *      Helpers to manage template descriptors.
  */
 #include "ima.h"
+#include "ima_template_lib.h"
 
 static struct ima_template_desc defined_templates[] = {
+	{.name = IMA_TEMPLATE_IMA_NAME,.fmt = IMA_TEMPLATE_IMA_FMT},
 };
 
 static struct ima_template_field supported_fields[] = {
+	{.field_id = "d",.field_init = ima_eventdigest_init,
+	 .field_show = ima_show_template_digest},
+	{.field_id = "n",.field_init = ima_eventname_init,
+	 .field_show = ima_show_template_string},
 };
 
-static struct ima_template_field *ima_lookup_template_field(
-							const char *field_id)
+static struct ima_template_field *lookup_template_field(const char *field_id)
 {
 	int i;
 
@@ -32,7 +37,7 @@ static struct ima_template_field *ima_lookup_template_field(
 	return NULL;
 }
 
-static int ima_template_fmt_size(char *template_fmt)
+static int template_fmt_size(char *template_fmt)
 {
 	char c;
 	int template_fmt_len = strlen(template_fmt);
@@ -53,7 +58,7 @@ static int template_desc_init_fields(char *template_fmt,
 				     int *num_fields)
 {
 	char *c, *template_fmt_ptr = template_fmt;
-	int template_num_fields = ima_template_fmt_size(template_fmt);
+	int template_num_fields = template_fmt_size(template_fmt);
 	int i, result = 0;
 
 	if (template_num_fields > IMA_TEMPLATE_NUM_FIELDS_MAX)
@@ -66,7 +71,7 @@ static int template_desc_init_fields(char *template_fmt,
 	}
 	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
 	     i < template_num_fields; i++) {
-		struct ima_template_field *f = ima_lookup_template_field(c);
+		struct ima_template_field *f = lookup_template_field(c);
 
 		if (!f) {
 			result = -ENOENT;
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
new file mode 100644
index 0000000..f1a2fcb
--- /dev/null
+++ b/security/integrity/ima/ima_template_lib.c
@@ -0,0 +1,192 @@
+/*
+ * Copyright (C) 2013 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Author: Roberto Sassu <roberto.sassu@polito.it>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: ima_template_lib.c
+ *      Library of supported template fields.
+ */
+#include "ima_template_lib.h"
+
+enum data_formats { DATA_FMT_DIGEST = 0, DATA_FMT_EVENT_NAME, DATA_FMT_STRING };
+static int ima_write_template_field_data(const void *data, const u32 datalen,
+					 enum data_formats datafmt,
+					 struct ima_field_data *field_data)
+{
+	u8 *buf, *buf_ptr;
+	u32 buflen;
+
+	switch (datafmt) {
+	case DATA_FMT_EVENT_NAME:
+		buflen = IMA_EVENT_NAME_LEN_MAX + 1;
+		break;
+	case DATA_FMT_STRING:
+		buflen = datalen + 1;
+		break;
+	default:
+		buflen = datalen;
+	}
+
+	buf = kzalloc(buflen, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	memcpy(buf, data, datalen);
+
+	/*
+	 * Replace all space characters with underscore for event names and
+	 * strings. This avoid that, during the parsing of a measurements list,
+	 * filenames with spaces or that end with the suffix ' (deleted)' are
+	 * split into multiple template fields (the space is the delimitator
+	 * character for measurements lists in ASCII format).
+	 */
+	if (datafmt == DATA_FMT_EVENT_NAME || datafmt == DATA_FMT_STRING) {
+		for (buf_ptr = buf; buf_ptr - buf < datalen; buf_ptr++)
+			if (*buf_ptr == ' ')
+				*buf_ptr = '_';
+	}
+
+	field_data->data = buf;
+	field_data->len = buflen;
+	return 0;
+}
+
+static void ima_show_template_data_ascii(struct seq_file *m,
+					 enum ima_show_type show,
+					 enum data_formats datafmt,
+					 struct ima_field_data *field_data)
+{
+	switch (datafmt) {
+	case DATA_FMT_DIGEST:
+		ima_print_digest(m, field_data->data, field_data->len);
+		break;
+	case DATA_FMT_STRING:
+		seq_printf(m, "%s", field_data->data);
+		break;
+	default:
+		break;
+	}
+}
+
+static void ima_show_template_data_binary(struct seq_file *m,
+					  enum ima_show_type show,
+					  enum data_formats datafmt,
+					  struct ima_field_data *field_data)
+{
+	if (datafmt != DATA_FMT_DIGEST)
+		ima_putc(m, &field_data->len, sizeof(u32));
+	ima_putc(m, field_data->data, field_data->len);
+}
+
+static void ima_show_template_field_data(struct seq_file *m,
+					 enum ima_show_type show,
+					 enum data_formats datafmt,
+					 struct ima_field_data *field_data)
+{
+	switch (show) {
+	case IMA_SHOW_ASCII:
+		ima_show_template_data_ascii(m, show, datafmt, field_data);
+		break;
+	case IMA_SHOW_BINARY:
+		ima_show_template_data_binary(m, show, datafmt, field_data);
+		break;
+	default:
+		break;
+	}
+}
+
+void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
+			      struct ima_field_data *field_data)
+{
+	ima_show_template_field_data(m, show, DATA_FMT_DIGEST, field_data);
+}
+
+void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
+			      struct ima_field_data *field_data)
+{
+	ima_show_template_field_data(m, show, DATA_FMT_STRING, field_data);
+}
+
+/*
+ * This function writes the digest of an event.
+ */
+int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
+			 const unsigned char *filename,
+			 struct ima_field_data *field_data)
+{
+	struct {
+		struct ima_digest_data hdr;
+		char digest[IMA_MAX_DIGEST_SIZE];
+	} hash;
+	u8 *cur_digest = hash.hdr.digest;
+	u32 cur_digestsize = IMA_DIGEST_SIZE;
+	struct inode *inode;
+	int result;
+
+	memset(&hash, 0, sizeof(hash));
+
+	if (!iint)		/* recording a violation. */
+		goto out;
+
+	if (iint->ima_hash->algo == ima_hash_algo) {
+		cur_digest = iint->ima_hash->digest;
+		cur_digestsize = iint->ima_hash->length;
+		goto out;
+	}
+
+	if (!file)		/* missing info to re-calculate the digest */
+		return -EINVAL;
+
+	inode = file_inode(file);
+	hash.hdr.algo = ima_hash_algo;
+	result = ima_calc_file_hash(file, &hash.hdr);
+	if (result) {
+		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
+				    filename, "collect_data",
+				    "failed", result, 0);
+		return result;
+	}
+out:
+	return ima_write_template_field_data(cur_digest, cur_digestsize,
+					     DATA_FMT_DIGEST, field_data);
+}
+
+/*
+ * This function writes the name of an event.
+ */
+int ima_eventname_init(struct integrity_iint_cache *iint, struct file *file,
+		       const unsigned char *filename,
+		       struct ima_field_data *field_data)
+{
+	const char *cur_filename = NULL;
+	u32 cur_filename_len = 0;
+
+	BUG_ON(filename == NULL && file == NULL);
+
+	if (filename) {
+		cur_filename = filename;
+		cur_filename_len = strlen(filename);
+
+		if (cur_filename_len <= IMA_EVENT_NAME_LEN_MAX)
+			goto out;
+	}
+
+	if (file) {
+		cur_filename = file->f_dentry->d_name.name;
+		cur_filename_len = strlen(cur_filename);
+	} else
+		/*
+		 * Truncate filename if the latter is too long and
+		 * the file descriptor is not available.
+		 */
+		cur_filename_len = IMA_EVENT_NAME_LEN_MAX;
+out:
+	return ima_write_template_field_data(cur_filename, cur_filename_len,
+					     DATA_FMT_EVENT_NAME, field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
new file mode 100644
index 0000000..2cecc83
--- /dev/null
+++ b/security/integrity/ima/ima_template_lib.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2013 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Author: Roberto Sassu <roberto.sassu@polito.it>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: ima_template_lib.h
+ *      Header for the library of supported template fields.
+ */
+#ifndef __LINUX_IMA_TEMPLATE_LIB_H
+#define __LINUX_IMA_TEMPLATE_LIB_H
+
+#include <linux/seq_file.h>
+#include "ima.h"
+
+void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
+			      struct ima_field_data *field_data);
+void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
+			      struct ima_field_data *field_data);
+int ima_eventdigest_init(struct integrity_iint_cache *iint, struct file *file,
+			 const unsigned char *filename,
+			 struct ima_field_data *field_data);
+int ima_eventname_init(struct integrity_iint_cache *iint, struct file *file,
+		       const unsigned char *filename,
+		       struct ima_field_data *field_data);
+#endif /* __LINUX_IMA_TEMPLATE_LIB_H */
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 13/20] ima: new templates management mechanism
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Roberto Sassu, linux-crypto, David Howells, Mimi Zohar
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

From: Roberto Sassu <roberto.sassu@polito.it>

The original 'ima' template is fixed length, containing the filedata hash
and pathname.  The filedata hash is limited to 20 bytes (md5/sha1).  The
pathname is a null terminated string, limited to 255 characters.  To
overcome these limitations and to add additional file metadata, it is
necessary to extend the current version of IMA by defining additional
templates.

The main reason to introduce this feature is that, each time a new
template is defined, the functions that generate and display the
measurement list would include the code for handling a new format and,
thus, would significantly grow over time.

This patch set solves this problem by separating the template management
from the remaining IMA code. The core of this solution is the definition
of two new data structures: a template descriptor, to determine which
information should be included in the measurement list, and a template
field, to generate and display data of a given type.

To define a new template field, developers define the field identifier
and implement two functions, init() and show(), respectively to generate
and display measurement entries.  Initially, this patch set defines the
following template fields (support for additional data types will be
added later):
 - 'd': the digest of the event (i.e. the digest of a measured file),
        calculated with the SHA1 or MD5 hash algorithm;
 - 'n': the name of the event (i.e. the file name), with size up to
        255 bytes;
 - 'd-ng': the digest of the event, calculated with an arbitrary hash
           algorithm (field format: [<hash algo>:]digest, where the digest
           prefix is shown only if the hash algorithm is not SHA1 or MD5);
 - 'n-ng': the name of the event, without size limitations.

Defining a new template descriptor requires specifying the template format,
a string of field identifiers separated by the '|' character.  This patch
set defines the following template descriptors:
 - "ima": its format is 'd|n';
 - "ima-ng" (default): its format is 'd-ng|n-ng'

Further details about the new template architecture can be found in
Documentation/security/IMA-templates.txt.

Changelog:
- don't defer calling ima_init_template() - Mimi
- don't define ima_lookup_template_desc() until used - Mimi
- squashed with documentation patch - Mimi

Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 Documentation/security/00-INDEX          |   2 +
 Documentation/security/IMA-templates.txt |  87 ++++++++++++++++++++++++
 security/integrity/ima/Makefile          |   2 +-
 security/integrity/ima/ima.h             |  29 ++++++++
 security/integrity/ima/ima_init.c        |   4 ++
 security/integrity/ima/ima_template.c    | 112 +++++++++++++++++++++++++++++++
 6 files changed, 235 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/security/IMA-templates.txt
 create mode 100644 security/integrity/ima/ima_template.c

diff --git a/Documentation/security/00-INDEX b/Documentation/security/00-INDEX
index 414235c..45c82fd 100644
--- a/Documentation/security/00-INDEX
+++ b/Documentation/security/00-INDEX
@@ -22,3 +22,5 @@ keys.txt
 	- description of the kernel key retention service.
 tomoyo.txt
 	- documentation on the TOMOYO Linux Security Module.
+IMA-templates.txt
+	- documentation on the template management mechanism for IMA.
diff --git a/Documentation/security/IMA-templates.txt b/Documentation/security/IMA-templates.txt
new file mode 100644
index 0000000..a777e5f
--- /dev/null
+++ b/Documentation/security/IMA-templates.txt
@@ -0,0 +1,87 @@
+                       IMA Template Management Mechanism
+
+
+==== INTRODUCTION ====
+
+The original 'ima' template is fixed length, containing the filedata hash
+and pathname. The filedata hash is limited to 20 bytes (md5/sha1).
+The pathname is a null terminated string, limited to 255 characters.
+To overcome these limitations and to add additional file metadata, it is
+necessary to extend the current version of IMA by defining additional
+templates. For example, information that could be possibly reported are
+the inode UID/GID or the LSM labels either of the inode and of the process
+that is accessing it.
+
+However, the main problem to introduce this feature is that, each time
+a new template is defined, the functions that generate and display
+the measurements list would include the code for handling a new format
+and, thus, would significantly grow over the time.
+
+The proposed solution solves this problem by separating the template
+management from the remaining IMA code. The core of this solution is the
+definition of two new data structures: a template descriptor, to determine
+which information should be included in the measurement list; a template
+field, to generate and display data of a given type.
+
+Managing templates with these structures is very simple. To support
+a new data type, developers define the field identifier and implement
+two functions, init() and show(), respectively to generate and display
+measurement entries. Defining a new template descriptor requires
+specifying the template format, a string of field identifiers separated
+by the '|' character. While in the current implementation it is possible
+to define new template descriptors only by adding their definition in the
+template specific code (ima_template.c), in a future version it will be
+possible to register a new template on a running kernel by supplying to IMA
+the desired format string. In this version, IMA initializes at boot time
+all defined template descriptors by translating the format into an array
+of template fields structures taken from the set of the supported ones.
+
+After the initialization step, IMA will call ima_alloc_init_template()
+(new function defined within the patches for the new template management
+mechanism) to generate a new measurement entry by using the template
+descriptor chosen through the kernel configuration or through the newly
+introduced 'ima_template=' kernel command line parameter. It is during this
+phase that the advantages of the new architecture are clearly shown:
+the latter function will not contain specific code to handle a given template
+but, instead, it simply calls the init() method of the template fields
+associated to the chosen template descriptor and store the result (pointer
+to allocated data and data length) in the measurement entry structure.
+
+The same mechanism is employed to display measurements entries.
+The functions ima[_ascii]_measurements_show() retrieve, for each entry,
+the template descriptor used to produce that entry and call the show()
+method for each item of the array of template fields structures.
+
+
+
+==== SUPPORTED TEMPLATE FIELDS AND DESCRIPTORS ====
+
+In the following, there is the list of supported template fields
+('<identifier>': description), that can be used to define new template
+descriptors by adding their identifier to the format string
+(support for more data types will be added later):
+
+ - 'd': the digest of the event (i.e. the digest of a measured file),
+        calculated with the SHA1 or MD5 hash algorithm;
+ - 'n': the name of the event (i.e. the file name), with size up to 255 bytes;
+ - 'd-ng': the digest of the event, calculated with an arbitrary hash
+           algorithm (field format: [<hash algo>:]digest, where the digest
+           prefix is shown only if the hash algorithm is not SHA1 or MD5);
+ - 'n-ng': the name of the event, without size limitations.
+
+
+Below, there is the list of defined template descriptors:
+ - "ima": its format is 'd|n';
+ - "ima-ng" (default): its format is 'd-ng|n-ng'.
+
+
+
+==== USE ====
+
+To specify the template descriptor to be used to generate measurement entries,
+currently the following methods are supported:
+
+ - select a template descriptor among those supported in the kernel
+   configuration ('ima-ng' is the default choice);
+ - specify a template descriptor name from the kernel command line through
+   the 'ima_template=' parameter.
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 56dfee7..7fe4ae3 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -6,5 +6,5 @@
 obj-$(CONFIG_IMA) += ima.o
 
 ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
-	 ima_policy.o
+	 ima_policy.o ima_template.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index da03d33..c85718f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -36,12 +36,39 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
 #define IMA_HASH_BITS 9
 #define IMA_MEASURE_HTABLE_SIZE (1 << IMA_HASH_BITS)
 
+#define IMA_TEMPLATE_FIELD_ID_MAX_LEN	16
+#define IMA_TEMPLATE_NUM_FIELDS_MAX	15
+
 /* set during initialization */
 extern int ima_initialized;
 extern int ima_used_chip;
 extern int ima_hash_algo;
 extern int ima_appraise;
 
+/* IMA template field data definition */
+struct ima_field_data {
+	u8 *data;
+	u32 len;
+};
+
+/* IMA template field definition */
+struct ima_template_field {
+	const char field_id[IMA_TEMPLATE_FIELD_ID_MAX_LEN];
+	int (*field_init) (struct integrity_iint_cache *iint, struct file *file,
+			   const unsigned char *filename,
+			   struct ima_field_data *field_data);
+	void (*field_show) (struct seq_file *m, enum ima_show_type show,
+			    struct ima_field_data *field_data);
+};
+
+/* IMA template descriptor definition */
+struct ima_template_desc {
+	char *name;
+	char *fmt;
+	int num_fields;
+	struct ima_template_field **fields;
+};
+
 /* IMA inode template definition */
 struct ima_template_data {
 	u8 digest[IMA_DIGEST_SIZE];	/* sha1/md5 measurement hash */
@@ -79,6 +106,8 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 		       const char *op, const char *cause);
 int ima_init_crypto(void);
 
+int ima_init_template(void);
+
 /*
  * used to protect h_table and sha_table
  */
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 50e15e6..f84aec5 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -99,6 +99,10 @@ int __init ima_init(void)
 	rc = ima_init_crypto();
 	if (rc)
 		return rc;
+	rc = ima_init_template();
+	if (rc != 0)
+		return rc;
+
 	ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	ima_init_policy();
 
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
new file mode 100644
index 0000000..7e86783
--- /dev/null
+++ b/security/integrity/ima/ima_template.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (C) 2013 Politecnico di Torino, Italy
+ *                    TORSEC group -- http://security.polito.it
+ *
+ * Author: Roberto Sassu <roberto.sassu@polito.it>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: ima_template.c
+ *      Helpers to manage template descriptors.
+ */
+#include "ima.h"
+
+static struct ima_template_desc defined_templates[] = {
+};
+
+static struct ima_template_field supported_fields[] = {
+};
+
+static struct ima_template_field *ima_lookup_template_field(
+							const char *field_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(supported_fields); i++)
+		if (strncmp(supported_fields[i].field_id, field_id,
+			    IMA_TEMPLATE_FIELD_ID_MAX_LEN) == 0)
+			return &supported_fields[i];
+	return NULL;
+}
+
+static int ima_template_fmt_size(char *template_fmt)
+{
+	char c;
+	int template_fmt_len = strlen(template_fmt);
+	int i = 0, j = 0;
+
+	while (i < template_fmt_len) {
+		c = template_fmt[i];
+		if (c == '|')
+			j++;
+		i++;
+	}
+
+	return j + 1;
+}
+
+static int template_desc_init_fields(char *template_fmt,
+				     struct ima_template_field ***fields,
+				     int *num_fields)
+{
+	char *c, *template_fmt_ptr = template_fmt;
+	int template_num_fields = ima_template_fmt_size(template_fmt);
+	int i, result = 0;
+
+	if (template_num_fields > IMA_TEMPLATE_NUM_FIELDS_MAX)
+		return -EINVAL;
+
+	*fields = kzalloc(template_num_fields * sizeof(*fields), GFP_KERNEL);
+	if (*fields == NULL) {
+		result = -ENOMEM;
+		goto out;
+	}
+	for (i = 0; (c = strsep(&template_fmt_ptr, "|")) != NULL &&
+	     i < template_num_fields; i++) {
+		struct ima_template_field *f = ima_lookup_template_field(c);
+
+		if (!f) {
+			result = -ENOENT;
+			goto out;
+		}
+		(*fields)[i] = f;
+	}
+	*num_fields = i;
+	return 0;
+out:
+	kfree(*fields);
+	*fields = NULL;
+	return result;
+}
+
+static int init_defined_templates(void)
+{
+	int i = 0;
+	int result = 0;
+
+	/* Init defined templates. */
+	for (i = 0; i < ARRAY_SIZE(defined_templates); i++) {
+		struct ima_template_desc *template = &defined_templates[i];
+
+		result = template_desc_init_fields(template->fmt,
+						   &(template->fields),
+						   &(template->num_fields));
+		if (result < 0)
+			return result;
+	}
+	return result;
+}
+
+int ima_init_template(void)
+{
+	int result;
+
+	result = init_defined_templates();
+	if (result < 0)
+		return result;
+
+	return 0;
+}
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 18/20] ima: add Kconfig default measurement list template
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, linux-crypto, David Howells, Mimi Zohar,
	Roberto Sassu
In-Reply-To: <1374105100-25880-1-git-send-email-zohar@linux.vnet.ibm.com>

This patch adds a Kconfig option to select the default IMA
measurement list template.  The 'ima' template limited the
filedata hash to 20 bytes and the pathname to 255 charaters.
The 'ima-ng' measurement list template permits larger hash
digests and longer pathnames.

Changelog:
- keep 'select CRYPTO_HASH_INFO' in 'config IMA' section (Kconfig)
  (Roberto Sassu);
- removed trailing whitespaces (Roberto Sassu).
- Lindent fixes

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
---
 security/integrity/ima/Kconfig        | 25 +++++++++++++++++++++++++
 security/integrity/ima/ima_template.c |  4 ++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index e6628e7..de26cc8 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -46,6 +46,31 @@ config IMA_LSM_RULES
 	help
 	  Disabling this option will disregard LSM based policy rules.
 
+choice
+	prompt "Default template"
+	default IMA_NG_TEMPLATE
+	depends on IMA
+	help
+	  Select the default IMA measurement template.
+
+	  The original 'ima' measurement list template contains a
+	  hash, defined as 20 bytes, and a null terminated pathname,
+	  limited to 255 characters.  The 'ima-ng' measurement list
+	  template permits both larger hash digests and longer
+	  pathnames.
+
+	config IMA_TEMPLATE
+		bool "ima"
+	config IMA_NG_TEMPLATE
+		bool "ima-ng (default)"
+endchoice
+
+config IMA_DEFAULT_TEMPLATE
+	string
+	depends on IMA
+	default "ima" if IMA_TEMPLATE
+	default "ima-ng" if IMA_NG_TEMPLATE
+
 config IMA_APPRAISE
 	bool "Appraise integrity measurements"
 	depends on IMA
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 1c4cf19..c28ff9b 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -127,8 +127,8 @@ static int init_defined_templates(void)
 struct ima_template_desc *ima_template_desc_current(void)
 {
 	if (!ima_template)
-		ima_template = lookup_template_desc(IMA_TEMPLATE_IMA_NAME);
-
+		ima_template =
+		    lookup_template_desc(CONFIG_IMA_DEFAULT_TEMPLATE);
 	return ima_template;
 }
 
-- 
1.8.1.4

^ permalink raw reply related

* [RFC][PATCH 00/20] ima: larger digests and template support
From: Mimi Zohar @ 2013-07-17 23:51 UTC (permalink / raw)
  To: linux-security-module; +Cc: Mimi Zohar, linux-crypto, David Howells

Support for additional hash algorithms with larger digests, as well as
support for additional file metadata, in the IMA measurement list, have
been proposed for a long time.  Unfortunately, the IMA measurement list
entries, which are exposed to userspace via the securityfs
ascii/binary_runtime_measurement lists, are fixed length.  Any changes
to the measurement list would impact existing userspace tools.

This patch set defines an extensible template architecture and support
for larger hash algorithms.  A description of the new template
architecture is described in the "ima: new templates management
mechanism" patch description and, with more detail, in
Documentation/security/IMA-templates.txt.  The two initial templates
defined are: the original 'ima', for backwards compatibility, and
'ima-ng', which eliminates the digest and pathname size limitations.
Future templates will include file metadata, such as uid/gid and LSM
object/subject labels.

Larger hash algorithms, used for file data signature verification, is
also required to conform to various standards.

The first 9 patches add support for additional, larger hash
algorithms used in both the IMA measurement list and for file signature
verification, but isn't enabled until support for the new template
architecture is enabled.

Patches from 10 to 12 are preparatory for the subsequent changes.

Patches from 13 to 16 introduce the new template architecture
and the last group effectively enables it by allowing users to
choose their desired template either from the kernel configuration
and from the kernel command line.

Mimi Zohar

Dmitry Kasatkin (8):
  crypto: provide single place for hash algo information
  keys: change asymmetric keys to use common hash definitions
  ima: provide support for arbitrary hash algorithms
  ima: read and use signature hash algorithm
  ima: use dynamically allocated hash storage
  ima: provide dedicated hash algo allocation function
  ima: support arbitrary hash algorithms in ima_calc_buffer_hash
  ima: ima_calc_boot_agregate must use SHA1

Mimi Zohar (3):
  ima: differentiate between template hash and file data hash sizes
  ima: add Kconfig default measurement list template
  ima: enable support for larger default filedata hash algorithms

Roberto Sassu (9):
  ima: pass the file descriptor to ima_add_violation()
  ima: pass the filename argument up to ima_add_template_entry()
  ima: define new function ima_alloc_init_template() to API
  ima: new templates management mechanism
  ima: define template fields library and new helpers
  ima: define new template ima-ng and template fields d-ng and n-ng
  ima: switch to new template management mechanism
  ima: defer determining the appraisal hash algorithm for 'ima' template
  ima: define kernel parameter 'ima_template=' to change configured
    default

 Documentation/kernel-parameters.txt       |  11 +-
 Documentation/security/00-INDEX           |   2 +
 Documentation/security/IMA-templates.txt  |  87 +++++++++
 crypto/Kconfig                            |   3 +
 crypto/Makefile                           |   1 +
 crypto/asymmetric_keys/Kconfig            |   1 +
 crypto/asymmetric_keys/public_key.c       |  12 --
 crypto/asymmetric_keys/rsa.c              |  14 +-
 crypto/asymmetric_keys/x509_cert_parser.c |  12 +-
 crypto/asymmetric_keys/x509_parser.h      |   2 +-
 crypto/asymmetric_keys/x509_public_key.c  |   4 +-
 crypto/hash_info.c                        |  56 ++++++
 include/crypto/hash_info.h                |  40 ++++
 include/crypto/public_key.h               |  18 +-
 include/uapi/linux/hash_info.h            |  37 ++++
 kernel/module_signing.c                   |   8 +-
 security/integrity/digsig_asymmetric.c    |  11 --
 security/integrity/iint.c                 |   2 +
 security/integrity/ima/Kconfig            |  61 ++++++
 security/integrity/ima/Makefile           |   2 +-
 security/integrity/ima/ima.h              |  95 +++++++--
 security/integrity/ima/ima_api.c          | 119 ++++++++----
 security/integrity/ima/ima_appraise.c     |  60 ++++--
 security/integrity/ima/ima_crypto.c       | 131 +++++++++++--
 security/integrity/ima/ima_fs.c           |  64 +++---
 security/integrity/ima/ima_init.c         |  37 ++--
 security/integrity/ima/ima_main.c         |  50 ++++-
 security/integrity/ima/ima_queue.c        |  10 +-
 security/integrity/ima/ima_template.c     | 175 +++++++++++++++++
 security/integrity/ima/ima_template_lib.c | 313 ++++++++++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.h |  39 ++++
 security/integrity/integrity.h            |  26 ++-
 32 files changed, 1300 insertions(+), 203 deletions(-)
 create mode 100644 Documentation/security/IMA-templates.txt
 create mode 100644 crypto/hash_info.c
 create mode 100644 include/crypto/hash_info.h
 create mode 100644 include/uapi/linux/hash_info.h
 create mode 100644 security/integrity/ima/ima_template.c
 create mode 100644 security/integrity/ima/ima_template_lib.c
 create mode 100644 security/integrity/ima/ima_template_lib.h

-- 
1.8.1.4

^ permalink raw reply

* Suggestion for 3DES driver
From: Joel Fernandes @ 2013-07-18  3:08 UTC (permalink / raw)
  To: linux-crypto

Hi,

I am working on a driver for OMAP 3DES hardware similar to OMAP AES
(drivers/crypto/omap-aes). The hardware is similar in many ways and a lot of it
can be reused from first look.

Can anyone comment if its ok to add to omap-aes, calling it
drivers/crypto/omap-aesdes. That will ensure maximum reuse and reduce LOC too.
Any other suggestions?

Thanks,

-Joel

^ permalink raw reply

* Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.
From: Tetsuo Handa @ 2013-07-18  3:47 UTC (permalink / raw)
  To: Tim Chen; +Cc: herbert, linux-kernel, linux-crypto
In-Reply-To: <1374098936.22432.322.camel@schen9-DESK>

Tim Chen wrote:
> > > Your approach is quite complicated.  I think something simpler like the
> > > following will work:
> >
> > We cannot benefit from PCLMULQDQ. Is it acceptable for you?
> 
> 
> The following code in crct10dif-pclmul_glue.c
> 
> static const struct x86_cpu_id crct10dif_cpu_id[] = {
>         X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
>         {}
> };
> MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
> 
> will put the module in the device table and get the module
> loaded, as long as the cpu support PCLMULQDQ. So we should be able
> to benefit.

Excuse me, how can crct10dif-pclmul.ko get loaded automatically?
Did you test CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m with below debug message?

diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c b/arch/x86/crypto/crct10dif-pclmul_glue.c
index 7845d7f..a8a95aa 100644
--- a/arch/x86/crypto/crct10dif-pclmul_glue.c
+++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
@@ -129,9 +129,10 @@ MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
 
 static int __init crct10dif_intel_mod_init(void)
 {
+	printk(KERN_WARNING "********** Checking for X86_FEATURE_PCLMULQDQ\n");
 	if (!x86_match_cpu(crct10dif_cpu_id))
 		return -ENODEV;
-
+	printk(KERN_WARNING "********** Registering crct10dif-pclmul\n");
 	return crypto_register_shash(&alg);
 }

As far as I tested, crct10dif-pclmul.ko will not be loaded unless manually
adding "modprobe crct10dif-pclmul" to initramfs's /init or choosing
CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.

> So as long as the crct10dif.ko and crct10dif-pclmul.ko are loaded,
> the pclmulqdq t10dif will have a higher priority and get allocated
> and used.

What I'm talking are

  (1) Since mkinitramfs is unable to know that crct10dif-pclmul.ko has higher
      priority than crct10dif.ko , mkinitramfs will not include
      "modprobe crct10dif-pclmul" line in the generated initramfs.

  (2) In order to get benefit from PCLMULQDQ, users have to manually make sure
      that "modprobe crct10dif-pclmul" is called before crc-t10dif.ko (which is
      loaded before sd_mod.ko is loaded) is loaded by their initramfs's /init
      script.

  (3) The cause of (1) is that crct10dif-pclmul.ko will not be loaded
      automatically unless choosing CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.

  (4) The cause of (3) is that modules.dep does not describe that users will
      benefit by loading crct10dif-pclmul.ko before loading crc-t10dif.ko .

  (5) Currently crct10dif-pclmul.ko cannot be loaded if PCLMULQDQ is not
      supported. This leads to boot failure (since sd_mod.ko cannot be loaded)
      if modules.dep says that "crct10dif-pclmul.ko is required by
      crc-t10dif.ko".

  (6) To solve (4) and (5), modules.dep should say "crct10dif-pclmul.ko is
      preferred for crc-t10dif.ko but is not required by crc-t10dif.ko".
      But there is no such mechanism. Thus, currently available choice is
      "allow loading crct10dif-pclmul.ko even if PCLMULQDQ is not supported"
      or "ignore errors by built-in the crct10dif-pclmul.ko module".

My patch (b) seems to be complicated but is required in order to solve (4)
without asking users to manually add "modprobe crct10dif-pclmul" into their
initramfs. If we choose patch (b) rather than patch (a), we need to solve (5).

^ permalink raw reply related

* [PATCH] crypto: remove double execution of the same test suite
From: Cristian Stoica @ 2013-07-18 15:57 UTC (permalink / raw)
  To: herbert; +Cc: davem, linux-crypto, linux-kernel, horia.geanta, Cristian Stoica

This patch removes redundant execution of the same test suite in cases
where alg and driver variables are the same (e.g. when alg_test is
called from tcrypt_test)

Signed-off-by: Cristian Stoica <cristian.stoica@freescale.com>
---
 crypto/testmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 2f00607..e091ef6 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -3234,7 +3234,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 	if (i >= 0)
 		rc |= alg_test_descs[i].test(alg_test_descs + i, driver,
 					     type, mask);
-	if (j >= 0)
+	if (j >= 0 && j != i)
 		rc |= alg_test_descs[j].test(alg_test_descs + j, driver,
 					     type, mask);
 
-- 
1.8.1.5

^ permalink raw reply related

* Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.
From: Tim Chen @ 2013-07-18 21:00 UTC (permalink / raw)
  To: Tetsuo Handa, Rafael J. Wysocki; +Cc: herbert, linux-kernel, linux-crypto, ak
In-Reply-To: <201307180347.r6I3l5e9077577@www262.sakura.ne.jp>

On Thu, 2013-07-18 at 12:47 +0900, Tetsuo Handa wrote:
> Tim Chen wrote:
> > > > Your approach is quite complicated.  I think something simpler like the
> > > > following will work:
> > >
> > > We cannot benefit from PCLMULQDQ. Is it acceptable for you?
> > 
> > 
> > The following code in crct10dif-pclmul_glue.c
> > 
> > static const struct x86_cpu_id crct10dif_cpu_id[] = {
> >         X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
> >         {}
> > };
> > MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
> > 
> > will put the module in the device table and get the module
> > loaded, as long as the cpu support PCLMULQDQ. So we should be able
> > to benefit.
> 
> Excuse me, how can crct10dif-pclmul.ko get loaded automatically?
> Did you test CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m with below debug message?

The code:

static const struct x86_cpu_id crct10dif_cpu_id[] = {
         X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
         {}
};
MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);

causes the following line to be added to modules.alias file:

alias x86cpu:vendor:*:family:*:model:*:feature:*0081* crct10dif_pclmul

This should cause udev to load the crct10dif_pclml module when cpu
support the PCLMULQDQ (feature code 0081).  I did my testing during
development on 3.10 and the module was indeed loaded.

However, I found that the following commit under 3.11-rc1 broke 
the mechanism after some bisection.

commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Date:   Fri May 3 00:26:22 2013 +0200

    ACPI / processor: Use common hotplug infrastructure
    
    Split the ACPI processor driver into two parts, one that is
    non-modular, resides in the ACPI core and handles the enumeration
    and hotplug of processors and one that implements the rest of the
    existing processor driver functionality.
    
Rafael, can you check and see if this can be fixed so those optimized 
crypto modules for Intel cpu that support them can be loaded?

Herbert, we had a discussion earlier on a separate issue regarding the 
module load order.  We wanted to load all supported crypto t10dif modules
before the crc-t10dif module.  You mentioned that the MODULE_ALIAS also
need some fixing and wonder if those changes have been merged into
3.11-rc1?  See https://lkml.org/lkml/2013/5/21/216 .  Theoretically, that
fix should also get all the crypto modules support t10dif be loaded.

Thanks.

Tim

> 
> diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c b/arch/x86/crypto/crct10dif-pclmul_glue.c
> index 7845d7f..a8a95aa 100644
> --- a/arch/x86/crypto/crct10dif-pclmul_glue.c
> +++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
> @@ -129,9 +129,10 @@ MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
>  
>  static int __init crct10dif_intel_mod_init(void)
>  {
> +	printk(KERN_WARNING "********** Checking for X86_FEATURE_PCLMULQDQ\n");
>  	if (!x86_match_cpu(crct10dif_cpu_id))
>  		return -ENODEV;
> -
> +	printk(KERN_WARNING "********** Registering crct10dif-pclmul\n");
>  	return crypto_register_shash(&alg);
>  }
> 
> As far as I tested, crct10dif-pclmul.ko will not be loaded unless manually
> adding "modprobe crct10dif-pclmul" to initramfs's /init or choosing
> CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.
> 
> > So as long as the crct10dif.ko and crct10dif-pclmul.ko are loaded,
> > the pclmulqdq t10dif will have a higher priority and get allocated
> > and used.
> 
> What I'm talking are
> 
>   (1) Since mkinitramfs is unable to know that crct10dif-pclmul.ko has higher
>       priority than crct10dif.ko , mkinitramfs will not include
>       "modprobe crct10dif-pclmul" line in the generated initramfs.
> 
>   (2) In order to get benefit from PCLMULQDQ, users have to manually make sure
>       that "modprobe crct10dif-pclmul" is called before crc-t10dif.ko (which is
>       loaded before sd_mod.ko is loaded) is loaded by their initramfs's /init
>       script.
> 
>   (3) The cause of (1) is that crct10dif-pclmul.ko will not be loaded
>       automatically unless choosing CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.
> 
>   (4) The cause of (3) is that modules.dep does not describe that users will
>       benefit by loading crct10dif-pclmul.ko before loading crc-t10dif.ko .
> 
>   (5) Currently crct10dif-pclmul.ko cannot be loaded if PCLMULQDQ is not
>       supported. This leads to boot failure (since sd_mod.ko cannot be loaded)
>       if modules.dep says that "crct10dif-pclmul.ko is required by
>       crc-t10dif.ko".
> 
>   (6) To solve (4) and (5), modules.dep should say "crct10dif-pclmul.ko is
>       preferred for crc-t10dif.ko but is not required by crc-t10dif.ko".
>       But there is no such mechanism. Thus, currently available choice is
>       "allow loading crct10dif-pclmul.ko even if PCLMULQDQ is not supported"
>       or "ignore errors by built-in the crct10dif-pclmul.ko module".
> 
> My patch (b) seems to be complicated but is required in order to solve (4)
> without asking users to manually add "modprobe crct10dif-pclmul" into their
> initramfs. If we choose patch (b) rather than patch (a), we need to solve (5).
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.
From: Rafael J. Wysocki @ 2013-07-18 22:17 UTC (permalink / raw)
  To: Tim Chen; +Cc: Tetsuo Handa, herbert, linux-kernel, linux-crypto, ak, rjw
In-Reply-To: <1374181200.22432.350.camel@schen9-DESK>

On 7/18/2013 11:00 PM, Tim Chen wrote:
> On Thu, 2013-07-18 at 12:47 +0900, Tetsuo Handa wrote:
>> Tim Chen wrote:
>>>>> Your approach is quite complicated.  I think something simpler like the
>>>>> following will work:
>>>> We cannot benefit from PCLMULQDQ. Is it acceptable for you?
>>>
>>> The following code in crct10dif-pclmul_glue.c
>>>
>>> static const struct x86_cpu_id crct10dif_cpu_id[] = {
>>>          X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
>>>          {}
>>> };
>>> MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
>>>
>>> will put the module in the device table and get the module
>>> loaded, as long as the cpu support PCLMULQDQ. So we should be able
>>> to benefit.
>> Excuse me, how can crct10dif-pclmul.ko get loaded automatically?
>> Did you test CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m with below debug message?
> The code:
>
> static const struct x86_cpu_id crct10dif_cpu_id[] = {
>           X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
>           {}
> };
> MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
>
> causes the following line to be added to modules.alias file:
>
> alias x86cpu:vendor:*:family:*:model:*:feature:*0081* crct10dif_pclmul
>
> This should cause udev to load the crct10dif_pclml module when cpu
> support the PCLMULQDQ (feature code 0081).  I did my testing during
> development on 3.10 and the module was indeed loaded.
>
> However, I found that the following commit under 3.11-rc1 broke
> the mechanism after some bisection.
>
> commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
> Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Date:   Fri May 3 00:26:22 2013 +0200
>
>      ACPI / processor: Use common hotplug infrastructure
>      
>      Split the ACPI processor driver into two parts, one that is
>      non-modular, resides in the ACPI core and handles the enumeration
>      and hotplug of processors and one that implements the rest of the
>      existing processor driver functionality.
>      
> Rafael, can you check and see if this can be fixed so those optimized
> crypto modules for Intel cpu that support them can be loaded?

I think this is an ordering issue between udev startup and the time when 
devices are registered.

I wonder what happens if you put those modules into the initramfs image?

Rafael


> Herbert, we had a discussion earlier on a separate issue regarding the
> module load order.  We wanted to load all supported crypto t10dif modules
> before the crc-t10dif module.  You mentioned that the MODULE_ALIAS also
> need some fixing and wonder if those changes have been merged into
> 3.11-rc1?  See https://lkml.org/lkml/2013/5/21/216 .  Theoretically, that
> fix should also get all the crypto modules support t10dif be loaded.
>
> Thanks.
>
> Tim
>
>> diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c b/arch/x86/crypto/crct10dif-pclmul_glue.c
>> index 7845d7f..a8a95aa 100644
>> --- a/arch/x86/crypto/crct10dif-pclmul_glue.c
>> +++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
>> @@ -129,9 +129,10 @@ MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
>>   
>>   static int __init crct10dif_intel_mod_init(void)
>>   {
>> +	printk(KERN_WARNING "********** Checking for X86_FEATURE_PCLMULQDQ\n");
>>   	if (!x86_match_cpu(crct10dif_cpu_id))
>>   		return -ENODEV;
>> -
>> +	printk(KERN_WARNING "********** Registering crct10dif-pclmul\n");
>>   	return crypto_register_shash(&alg);
>>   }
>>
>> As far as I tested, crct10dif-pclmul.ko will not be loaded unless manually
>> adding "modprobe crct10dif-pclmul" to initramfs's /init or choosing
>> CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.
>>
>>> So as long as the crct10dif.ko and crct10dif-pclmul.ko are loaded,
>>> the pclmulqdq t10dif will have a higher priority and get allocated
>>> and used.
>> What I'm talking are
>>
>>    (1) Since mkinitramfs is unable to know that crct10dif-pclmul.ko has higher
>>        priority than crct10dif.ko , mkinitramfs will not include
>>        "modprobe crct10dif-pclmul" line in the generated initramfs.
>>
>>    (2) In order to get benefit from PCLMULQDQ, users have to manually make sure
>>        that "modprobe crct10dif-pclmul" is called before crc-t10dif.ko (which is
>>        loaded before sd_mod.ko is loaded) is loaded by their initramfs's /init
>>        script.
>>
>>    (3) The cause of (1) is that crct10dif-pclmul.ko will not be loaded
>>        automatically unless choosing CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.
>>
>>    (4) The cause of (3) is that modules.dep does not describe that users will
>>        benefit by loading crct10dif-pclmul.ko before loading crc-t10dif.ko .
>>
>>    (5) Currently crct10dif-pclmul.ko cannot be loaded if PCLMULQDQ is not
>>        supported. This leads to boot failure (since sd_mod.ko cannot be loaded)
>>        if modules.dep says that "crct10dif-pclmul.ko is required by
>>        crc-t10dif.ko".
>>
>>    (6) To solve (4) and (5), modules.dep should say "crct10dif-pclmul.ko is
>>        preferred for crc-t10dif.ko but is not required by crc-t10dif.ko".
>>        But there is no such mechanism. Thus, currently available choice is
>>        "allow loading crct10dif-pclmul.ko even if PCLMULQDQ is not supported"
>>        or "ignore errors by built-in the crct10dif-pclmul.ko module".
>>
>> My patch (b) seems to be complicated but is required in order to solve (4)
>> without asking users to manually add "modprobe crct10dif-pclmul" into their
>> initramfs. If we choose patch (b) rather than patch (a), we need to solve (5).
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply

* Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.
From: Tim Chen @ 2013-07-18 23:08 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Tetsuo Handa, herbert, linux-kernel, linux-crypto, ak, rjw
In-Reply-To: <51E8696A.7090406@intel.com>

On Fri, 2013-07-19 at 00:17 +0200, Rafael J. Wysocki wrote:
> On 7/18/2013 11:00 PM, Tim Chen wrote:
> > On Thu, 2013-07-18 at 12:47 +0900, Tetsuo Handa wrote:
> >> Tim Chen wrote:
> >>>>> Your approach is quite complicated.  I think something simpler like the
> >>>>> following will work:
> >>>> We cannot benefit from PCLMULQDQ. Is it acceptable for you?
> >>>
> >>> The following code in crct10dif-pclmul_glue.c
> >>>
> >>> static const struct x86_cpu_id crct10dif_cpu_id[] = {
> >>>          X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
> >>>          {}
> >>> };
> >>> MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
> >>>
> >>> will put the module in the device table and get the module
> >>> loaded, as long as the cpu support PCLMULQDQ. So we should be able
> >>> to benefit.
> >> Excuse me, how can crct10dif-pclmul.ko get loaded automatically?
> >> Did you test CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m with below debug message?
> > The code:
> >
> > static const struct x86_cpu_id crct10dif_cpu_id[] = {
> >           X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
> >           {}
> > };
> > MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
> >
> > causes the following line to be added to modules.alias file:
> >
> > alias x86cpu:vendor:*:family:*:model:*:feature:*0081* crct10dif_pclmul
> >
> > This should cause udev to load the crct10dif_pclml module when cpu
> > support the PCLMULQDQ (feature code 0081).  I did my testing during
> > development on 3.10 and the module was indeed loaded.
> >
> > However, I found that the following commit under 3.11-rc1 broke
> > the mechanism after some bisection.
> >
> > commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
> > Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Date:   Fri May 3 00:26:22 2013 +0200
> >
> >      ACPI / processor: Use common hotplug infrastructure
> >      
> >      Split the ACPI processor driver into two parts, one that is
> >      non-modular, resides in the ACPI core and handles the enumeration
> >      and hotplug of processors and one that implements the rest of the
> >      existing processor driver functionality.
> >      
> > Rafael, can you check and see if this can be fixed so those optimized
> > crypto modules for Intel cpu that support them can be loaded?
> 
> I think this is an ordering issue between udev startup and the time when 
> devices are registered.

Something that can be fixed?

> 
> I wonder what happens if you put those modules into the initramfs image?

Under initramfs image's /lib/modules/3.11.0-rc1/kernel/arch/x86/crypto
directory?  Any files in /lib/modules/3.11.0-rc1/modules.* need to be
modified?

Tim

^ permalink raw reply

* Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.
From: H. Peter Anvin @ 2013-07-18 23:44 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Tim Chen, Tetsuo Handa, herbert, linux-kernel, linux-crypto, ak,
	rjw
In-Reply-To: <51E8696A.7090406@intel.com>

On 07/18/2013 03:17 PM, Rafael J. Wysocki wrote:
>>
>> alias x86cpu:vendor:*:family:*:model:*:feature:*0081* crct10dif_pclmul
>>
>> This should cause udev to load the crct10dif_pclml module when cpu
>> support the PCLMULQDQ (feature code 0081).  I did my testing during
>> development on 3.10 and the module was indeed loaded.
>>
>> However, I found that the following commit under 3.11-rc1 broke
>> the mechanism after some bisection.
>>
>> commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
>> Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Date:   Fri May 3 00:26:22 2013 +0200
>>
>>      ACPI / processor: Use common hotplug infrastructure
>>           Split the ACPI processor driver into two parts, one that is
>>      non-modular, resides in the ACPI core and handles the enumeration
>>      and hotplug of processors and one that implements the rest of the
>>      existing processor driver functionality.
>>      Rafael, can you check and see if this can be fixed so those
>> optimized
>> crypto modules for Intel cpu that support them can be loaded?
> 
> I think this is an ordering issue between udev startup and the time when
> devices are registered.
> 
> I wonder what happens if you put those modules into the initramfs image?
> 

OK, this bothers me on some pretty deep level... a set of changes
exclusively in drivers/acpi breaking functionality which had nothing to
do with ACPI, specifically CPU-feature-based module loading.

Please let me know what the investigation comes up with, or if I need to
get more directly involved.

	-hpa

^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox