evm: Allow non-SHA1 digital signatures
Subsystems (4)
PATCH v3 1/1 evm: Allow non-SHA1 digital signatures
SHA1 is reasonable in HMAC constructs, but it's desirable to be able to use stronger hashes in digital signatures. Modify the EVM crypto code so the hash type is imported from the digital signature and passed down to the hash calculation code, and return the digest size to higher layers for validation.
Signed-off-by Matthew Garrett <[email protected]>
---
Added a new struct type equivalent to ima_digest_data but with the digest field actually populated in order to make it practical to allocate on the stack, and reworked stuff to use that.
security/integrity/evm/evm.h | 5 +- security/integrity/evm/evm_crypto.c | 59 ++++++++++++----------- security/integrity/evm/evm_main.c | 20 +++++--- security/integrity/ima/ima_api.c | 14 +++--- security/integrity/ima/ima_appraise.c | 20 ++++---- security/integrity/ima/ima_crypto.c | 28 +++++------ security/integrity/ima/ima_init.c | 4 +- security/integrity/ima/ima_template_lib.c | 12 ++--- security/integrity/integrity.h | 11 ++++- 9 files changed, 96 insertions(+), 77 deletions(-)
security/integrity/evm/evm.h +3 -2
@@ -49,10 +49,11 @@ int evm_update_evmxattr(struct dentry *dentry, size_t req_xattr_value_len);int evm_calc_hmac (struct dentry *dentry,const char *req_xattr_name,const char *req_xattr_value, -size_t req_xattr_value_len,char *digest); +size_t req_xattr_value_len,struct ima_xattr *data);int evm_calc_hash (struct dentry *dentry,const char *req_xattr_name,const char *req_xattr_value, -size_t req_xattr_value_len,char type,char *digest); +size_t req_xattr_value_len,char type, +struct ima_xattr *data);int evm_init_hmac (struct inode *inode,const struct xattr *xattr,char *hmac_val);int evm_init_secfs (void );
security/integrity/evm/evm_crypto.c +31 -28
149 lines not on this page · raw message
security/integrity/evm/evm_main.c +13 -7
@@ -25,6 +25,7 @@#include <linux/magic.h> #include <crypto/hash.h> +#include <crypto/hash_info.h> #include <crypto/algapi.h> #include "evm.h"
@@ -122,8 +123,9 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, struct integrity_iint_cache *iint) {struct evm_ima_xattr_data *xattr_data =NULL ; -struct evm_ima_xattr_data calc; +struct signature_v2_hdr *hdr;enum integrity_status evm_status =INTEGRITY_PASS ; +struct ima_xattr digest;struct inode *inode;int rc, xattr_len;
@@ -159,25 +161,29 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, evm_status =INTEGRITY_FAIL ; goto out; } + + digest.header.algo =HASH_ALGO_SHA1 ; rc =evm_calc_hmac (dentry, xattr_name, xattr_value, - xattr_value_len, calc.digest); + xattr_value_len, &digest);if (rc)break ; - rc =crypto_memneq (xattr_data->digest, calc.digest, -sizeof (calc.digest)); + rc =crypto_memneq (xattr_data->digest, &digest.digest, +SHA1_DIGEST_SIZE );if (rc) rc = -EINVAL ;break ;case EVM_IMA_XATTR_DIGSIG :case EVM_XATTR_PORTABLE_DIGSIG : + hdr = (struct signature_v2_hdr *)xattr_data; + + digest.header.algo = hdr->hash_algo; rc =evm_calc_hash (dentry, xattr_name, xattr_value, - xattr_value_len, xattr_data->type, - calc.digest); + xattr_value_len, xattr_data->type, &digest);if (rc)break ; rc =integrity_digsig_verify (INTEGRITY_KEYRING_EVM , (const char *)xattr_data, xattr_len, - calc.digest,sizeof (calc.digest)); + digest.digest, digest.header.length);if (!rc) { inode =d_backing_inode (dentry);
security/integrity/ima/ima_api.c +7 -7
@@ -103,7 +103,7 @@ int ima_store_template(struct ima_template_entry *entry,int num_fields = entry->template_desc->num_fields;/* this function uses default algo */ - hash.hdr.algo =HASH_ALGO_SHA1 ; + hash.hdr.header.algo =HASH_ALGO_SHA1 ; result =ima_calc_field_array_hash (&entry->template_data[0 ], entry->template_desc, num_fields, &hash.hdr);
@@ -113,7 +113,7 @@ int ima_store_template(struct ima_template_entry *entry, audit_cause, result, 0);return result; } -memcpy (entry->digest, hash.hdr.digest, hash.hdr.length); +memcpy (entry->digest, hash.hdr.digest, hash.hdr.header.length); } entry->pcr = pcr; result =ima_add_template_entry (entry, violation, op, inode, filename);
@@ -220,7 +220,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,* measurement/appraisal/audit. */ i_version =inode_query_iversion (inode); - hash.hdr.algo = algo; + hash.hdr.header.algo = algo;/* Initialize hash digest to 0's in case of failure */ memset (&hash.digest,0 ,sizeof (hash.digest));
@@ -233,7 +233,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,if (result && result != -EBADF && result != -EINVAL ) goto out; - length =sizeof (hash.hdr) + hash.hdr.length; + length =sizeof (hash.hdr) + hash.hdr.header.length; tmpbuf =krealloc (iint->ima_hash, length,GFP_NOFS );if (!tmpbuf) { result = -ENOMEM ;
@@ -312,17 +312,17 @@ void ima_audit_measurement(struct integrity_iint_cache *iint, {struct audit_buffer *ab;char *hash; -const char *algo_name = hash_algo_name[iint->ima_hash->algo]; +const char *algo_name = hash_algo_name[iint->ima_hash->header.algo];int i;if (iint->flags &IMA_AUDITED )return ; - hash =kzalloc ((iint->ima_hash->length *2 ) +1 ,GFP_KERNEL ); + hash =kzalloc ((iint->ima_hash->header.length *2 ) +1 ,GFP_KERNEL );if (!hash)return ; -for (i =0 ; i < iint->ima_hash->length; i++) +for (i =0 ; i < iint->ima_hash->header.length; i++)hex_byte_pack (hash + (i *2 ), iint->ima_hash->digest[i]); hash[i *2 ] ='\0' ;
security/integrity/ima/ima_appraise.c +10 -10
@@ -64,20 +64,20 @@ static int ima_fix_xattr(struct dentry *dentry, struct integrity_iint_cache *iint) {int rc, offset; -u8 algo = iint->ima_hash->algo; +u8 algo = iint->ima_hash->header.algo;if (algo <=HASH_ALGO_SHA1 ) { offset =1 ; - iint->ima_hash->xattr.sha1.type =IMA_XATTR_DIGEST ; + iint->ima_hash->header.xattr.sha1.type =IMA_XATTR_DIGEST ; }else { offset =0 ; - iint->ima_hash->xattr.ng.type =IMA_XATTR_DIGEST_NG ; - iint->ima_hash->xattr.ng.algo = algo; + iint->ima_hash->header.xattr.ng.type =IMA_XATTR_DIGEST_NG ; + iint->ima_hash->header.xattr.ng.algo = algo; } rc =__vfs_setxattr_noperm (dentry,XATTR_NAME_IMA , - &iint->ima_hash->xattr.data[offset], - (sizeof (iint->ima_hash->xattr) - offset) + - iint->ima_hash->length,0 ); + &iint->ima_hash->header.xattr.data[offset], + (sizeof (iint->ima_hash->header.xattr) - offset) + + iint->ima_hash->header.length,0 );return rc; }
@@ -270,13 +270,13 @@ int ima_appraise_measurement(enum ima_hooks func, }clear_bit (IMA_DIGSIG , &iint->atomic_flags);if (xattr_len -sizeof (xattr_value->type) - hash_start >= - iint->ima_hash->length) + iint->ima_hash->header.length)/* xattr length may be longer. md5 hash in previous version occupied 20 bytes in xattr, instead of 16 */ rc =memcmp (&xattr_value->digest[hash_start], iint->ima_hash->digest, - iint->ima_hash->length); + iint->ima_hash->header.length);else rc = -EINVAL ;if (rc) {
@@ -291,7 +291,7 @@ int ima_appraise_measurement(enum ima_hooks func, rc =integrity_digsig_verify (INTEGRITY_KEYRING_IMA , (const char *)xattr_value, rc, iint->ima_hash->digest, - iint->ima_hash->length); + iint->ima_hash->header.length);if (rc == -EOPNOTSUPP ) { status =INTEGRITY_UNKNOWN ; }else if (rc) {
security/integrity/ima/ima_crypto.c +14 -14
@@ -216,7 +216,7 @@ static int ima_calc_file_hash_atfm(struct file *file,struct crypto_wait wait;size_t rbuf_size[2 ]; - hash->length =crypto_ahash_digestsize (tfm); + hash->header.length =crypto_ahash_digestsize (tfm); req =ahash_request_alloc (tfm,GFP_KERNEL );if (!req)
@@ -319,7 +319,7 @@ static int ima_calc_file_ahash(struct file *file, struct ima_digest_data *hash)struct crypto_ahash *tfm;int rc; - tfm =ima_alloc_atfm (hash->algo); + tfm =ima_alloc_atfm (hash->header.algo);if (IS_ERR (tfm))return PTR_ERR (tfm);
@@ -342,7 +342,7 @@ static int ima_calc_file_hash_tfm(struct file *file, shash->tfm = tfm; shash->flags =0 ; - hash->length =crypto_shash_digestsize (tfm); + hash->header.length =crypto_shash_digestsize (tfm); rc =crypto_shash_init (shash);if (rc !=0 )
@@ -392,7 +392,7 @@ static int ima_calc_file_shash(struct file *file, struct ima_digest_data *hash)struct crypto_shash *tfm;int rc; - tfm =ima_alloc_tfm (hash->algo); + tfm =ima_alloc_tfm (hash->header.algo);if (IS_ERR (tfm))return PTR_ERR (tfm);
@@ -426,8 +426,8 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)* filesystems mounted with/without DAX option. */ if (file->f_flags &O_DIRECT ) { - hash->length = hash_digest_size[ima_hash_algo]; - hash->algo = ima_hash_algo; + hash->header.length = hash_digest_size[ima_hash_algo]; + hash->header.algo = ima_hash_algo;return -EINVAL ; }
@@ -457,7 +457,7 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data, shash->tfm = tfm; shash->flags =0 ; - hash->length =crypto_shash_digestsize (tfm); + hash->header.length =crypto_shash_digestsize (tfm); rc =crypto_shash_init (shash);if (rc !=0 )
@@ -499,7 +499,7 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,struct crypto_shash *tfm;int rc; - tfm =ima_alloc_tfm (hash->algo); + tfm =ima_alloc_tfm (hash->header.algo);if (IS_ERR (tfm))return PTR_ERR (tfm);
@@ -520,7 +520,7 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,struct crypto_wait wait;int rc, ahash_rc =0 ; - hash->length =crypto_ahash_digestsize (tfm); + hash->header.length =crypto_ahash_digestsize (tfm); req =ahash_request_alloc (tfm,GFP_KERNEL );if (!req)
@@ -557,7 +557,7 @@ static int calc_buffer_ahash(const void *buf, loff_t len,struct crypto_ahash *tfm;int rc; - tfm =ima_alloc_atfm (hash->algo); + tfm =ima_alloc_atfm (hash->header.algo);if (IS_ERR (tfm))return PTR_ERR (tfm);
@@ -579,7 +579,7 @@ static int calc_buffer_shash_tfm(const void *buf, loff_t size, shash->tfm = tfm; shash->flags =0 ; - hash->length =crypto_shash_digestsize (tfm); + hash->header.length =crypto_shash_digestsize (tfm); rc =crypto_shash_init (shash);if (rc !=0 )
@@ -605,7 +605,7 @@ static int calc_buffer_shash(const void *buf, loff_t len,struct crypto_shash *tfm;int rc; - tfm =ima_alloc_tfm (hash->algo); + tfm =ima_alloc_tfm (hash->header.algo);if (IS_ERR (tfm))return PTR_ERR (tfm);
@@ -671,11 +671,11 @@ int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)struct crypto_shash *tfm;int rc; - tfm =ima_alloc_tfm (hash->algo); + tfm =ima_alloc_tfm (hash->header.algo);if (IS_ERR (tfm))return PTR_ERR (tfm); - hash->length =crypto_shash_digestsize (tfm); + hash->header.length =crypto_shash_digestsize (tfm); rc =ima_calc_boot_aggregate_tfm (hash->digest, tfm);ima_free_tfm (tfm);
security/integrity/ima/ima_init.c +2 -2
@@ -61,8 +61,8 @@ static int __init ima_add_boot_aggregate(void)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 ; + iint->ima_hash->header.algo =HASH_ALGO_SHA1 ; + iint->ima_hash->header.length =SHA1_DIGEST_SIZE ;if (ima_used_chip) { result =ima_calc_boot_aggregate (&hash.hdr);
security/integrity/ima/ima_template_lib.c +6 -6
@@ -276,9 +276,9 @@ int ima_eventdigest_init(struct ima_event_data *event_data,if (event_data->violation)/* recording a violation. */ goto out; -if (ima_template_hash_algo_allowed (event_data->iint->ima_hash->algo)) { +if (ima_template_hash_algo_allowed (event_data->iint->ima_hash->header.algo)) { cur_digest = event_data->iint->ima_hash->digest; - cur_digestsize = event_data->iint->ima_hash->length; + cur_digestsize = event_data->iint->ima_hash->header.length; goto out; }
@@ -286,7 +286,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,return -EINVAL ; inode =file_inode (event_data->file); - hash.hdr.algo =ima_template_hash_algo_allowed (ima_hash_algo) ? + hash.hdr.header.algo =ima_template_hash_algo_allowed (ima_hash_algo) ? ima_hash_algo :HASH_ALGO_SHA1 ; result =ima_calc_file_hash (event_data->file, &hash.hdr);if (result) {
@@ -296,7 +296,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,return result; } cur_digest = hash.hdr.digest; - cur_digestsize = hash.hdr.length; + cur_digestsize = hash.hdr.header.length; out:return ima_eventdigest_init_common (cur_digest, cur_digestsize,HASH_ALGO__LAST , field_data);
@@ -315,9 +315,9 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data, goto out; cur_digest = event_data->iint->ima_hash->digest; - cur_digestsize = event_data->iint->ima_hash->length; + cur_digestsize = event_data->iint->ima_hash->header.length; - hash_algo = event_data->iint->ima_hash->algo; + hash_algo = event_data->iint->ima_hash->header.algo; out:return ima_eventdigest_init_common (cur_digest, cur_digestsize, hash_algo, field_data);
security/integrity/integrity.h +10 -1
@@ -83,7 +83,7 @@ struct evm_ima_xattr_data {#define IMA_MAX_DIGEST_SIZE 64 -struct ima_digest_data { +struct ima_digest_header {u8 algo;u8 length;union {
@@ -97,9 +97,18 @@ struct ima_digest_data { } ng;u8 data[2 ]; } xattr; +} __packed; + +struct ima_digest_data { +struct ima_digest_header header;u8 digest[0 ]; } __packed; +struct ima_xattr { +struct ima_digest_header header; +u8 digest[IMA_MAX_DIGEST_SIZE ]; +} __packed; +/* * signature format v2 - for using with asymmetric keys */
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
Quoted 13 lines · Matthew Garrett
SHA1 is reasonable in HMAC constructs, but it's desirable to be able to use stronger hashes in digital signatures. Modify the EVM crypto code so the hash type is imported from the digital signature and passed down to the hash calculation code, and return the digest size to higher layers for validation.
Signed-off-by Matthew Garrett <[email protected]>---
Added a new struct type equivalent to
ima_digest_databut with the digest field actually populated in order to make it practical to allocate on the stack, and reworked stuff to use that.
Quoted 10 lines
security/integrity/evm/evm.h | 5 +- security/integrity/evm/evm_crypto.c | 59 ++++++++++++----------- security/integrity/evm/evm_main.c | 20 +++++--- security/integrity/ima/ima_api.c | 14 +++--- security/integrity/ima/ima_appraise.c | 20 ++++---- security/integrity/ima/ima_crypto.c | 28 +++++------ security/integrity/ima/ima_init.c | 4 +- security/integrity/ima/ima_template_lib.c | 12 ++--- security/integrity/integrity.h | 11 ++++- 9 files changed, 96 insertions(+), 77 deletions(-)
Wow! This is a lot more change than I expected.
Quoted 19 lines
security/integrity/integrity.h +3
-1@@ -83,7 +83,7 @@ struct evm_ima_xattr_data {#define IMA_MAX_DIGEST_SIZE 64 -struct ima_digest_data { +struct ima_digest_header {u8 algo;u8 length;union {@@ -97,9 +97,18 @@ struct ima_digest_data { } ng;u8 data[2 ]; } xattr; +} __packed; +
security/integrity/integrity.h struct ima_digest_data { · line 111
Quoted 10 lines
security/integrity/integrity.h +7
-0+struct ima_digest_data { +struct ima_digest_header header;u8 digest[0 ]; } __packed; +struct ima_xattr { +struct ima_digest_header header; +u8 digest[IMA_MAX_DIGEST_SIZE ]; +} __packed; +
security.ima can be either a digest or a signature. Calling this struct "ima_xattr" will be confusing. The only thing common between ima_digest_data and signature_v2_hdr is the first byte - the 'type' field.
The IMA xattr is contained within this struct, but it is prefixed with some additional info. The original IMA xattr format starts with ima_digest_header.xattr[1]; the IMA xattr-ng format starts with ima_digest_header.xattr[0]. Even for security.ima containing digests, naming this struct ima_xattr is confusing.
We need to calculate hashes based on hash algorithms with larger digests, but at least for EVM we do not need to write them out as an xattr. There are a number of IMA examples where a struct, like below, are defined.
struct {
struct ima_digest_data hdr;
char digest[IMA_MAX_DIGEST_SIZE];
} hash;If a common struct was to be defined, I would appreciate separating the IMA from the EVM changes.
Mimi
Quoted 3 lines
security/integrity/integrity.h +0
-0/* * signature format v2 - for using with asymmetric keys */