Re: [PATCHv3 4/6] evm: provide a function to set EVM key from the kernel

[Mimi Zohar <zohar@linux.vnet.ibm.com>]

On Thu, 2015-10-22 at 21:49 +0300, Dmitry Kasatkin wrote:
> Crypto HW kernel module can possibly initialize EVM key from the
> kernel __init code to enable EVM before calling 'init' process.
> This patch provide a function evm_set_key() which can be used to
> set custom key directly to EVM without using KEY subsystem.

Thanks, Dmitry.  There's a minor comment inline.
> 
> Changes in v3:
> * error reporting moved to evm_set_key
> * EVM_INIT_HMAC moved to evm_set_key
> * added bitop to prevent key setting race
> 
> Changes in v2:
> * use size_t for key size instead of signed int
> * provide EVM_MAX_KEY_SIZE macro in <linux/evm.h>
> * provide EVM_MIN_KEY_SIZE macro in <linux/evm.h>
> 
> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> ---
>  include/linux/evm.h                 |  7 ++++++
>  security/integrity/evm/evm_crypto.c | 47 +++++++++++++++++++++++++++++++------
>  security/integrity/evm/evm_secfs.c  | 10 +++-----
>  3 files changed, 50 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/evm.h b/include/linux/evm.h
> index 1fcb88c..35ed9a8 100644
> --- a/include/linux/evm.h
> +++ b/include/linux/evm.h
> @@ -14,6 +14,7 @@
>  struct integrity_iint_cache;
> 
>  #ifdef CONFIG_EVM
> +extern int evm_set_key(void *key, size_t keylen);
>  extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  					     const char *xattr_name,
>  					     void *xattr_value,
> @@ -42,6 +43,12 @@ static inline int posix_xattr_acl(const char *xattrname)
>  }
>  #endif
>  #else
> +
> +static inline int evm_set_key(void *key, size_t keylen)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  #ifdef CONFIG_INTEGRITY
>  static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  						    const char *xattr_name,
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index 34e1a6f..7aec93e 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -18,6 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/crypto.h>
>  #include <linux/xattr.h>
> +#include <linux/evm.h>
>  #include <keys/encrypted-type.h>
>  #include <crypto/hash.h>
>  #include "evm.h"
> @@ -32,6 +33,41 @@ struct crypto_shash *hash_tfm;
> 
>  static DEFINE_MUTEX(mutex);
> 
> +#define EVM_SET_KEY_BUSY 0
> +
> +static unsigned long evm_set_key_flags;
> +
> +/* evm_set_key - set EVM HMAC key from the kernel
> + *

For exported functions this should be "kernel-doc" format.

Mimi  

> + * This function allows to set EVM HMAC key from the kernel
> + * without using key subsystem 'encrypted' keys. It can be used
> + * by the crypto HW kernel module which has own way of managing
> + * keys.
> + *
> + * key length should be between 32 and 128 bytes long
> + */
> +int evm_set_key(void *key, size_t keylen)
> +{
> +	int rc;
> +
> +	rc = -EBUSY;
> +	if (test_and_set_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags))
> +		goto busy;
> +	rc = -EINVAL;
> +	if (keylen > MAX_KEY_SIZE)
> +		goto inval;
> +	memcpy(evmkey, key, keylen);
> +	evm_initialized |= EVM_INIT_HMAC;
> +	pr_info("key initialized\n");
> +	return 0;
> +inval:
> +	clear_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags);
> +busy:
> +	pr_err("key initialization failed\n");
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(evm_set_key);
> +
>  static struct shash_desc *init_desc(char type)
>  {
>  	long rc;
> @@ -242,7 +278,7 @@ int evm_init_key(void)
>  {
>  	struct key *evm_key;
>  	struct encrypted_key_payload *ekp;
> -	int rc = 0;
> +	int rc;
> 
>  	evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
>  	if (IS_ERR(evm_key))
> @@ -250,12 +286,9 @@ int evm_init_key(void)
> 
>  	down_read(&evm_key->sem);
>  	ekp = evm_key->payload.data;
> -	if (ekp->decrypted_datalen > MAX_KEY_SIZE) {
> -		rc = -EINVAL;
> -		goto out;
> -	}
> -	memcpy(evmkey, ekp->decrypted_data, ekp->decrypted_datalen);
> -out:
> +
> +	rc = evm_set_key(ekp->decrypted_data, ekp->decrypted_datalen);
> +
>  	/* burn the original key contents */
>  	memset(ekp->decrypted_data, 0, ekp->decrypted_datalen);
>  	up_read(&evm_key->sem);
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index 3f775df..c8dccd5 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -62,7 +62,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
>  			     size_t count, loff_t *ppos)
>  {
>  	char temp[80];
> -	int i, error;
> +	int i;
> 
>  	if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_INIT_HMAC))
>  		return -EPERM;
> @@ -78,12 +78,8 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
>  	if ((sscanf(temp, "%d", &i) != 1) || (i != 1))
>  		return -EINVAL;
> 
> -	error = evm_init_key();
> -	if (!error) {
> -		evm_initialized |= EVM_INIT_HMAC;
> -		pr_info("initialized\n");
> -	} else
> -		pr_err("initialization failed\n");
> +	evm_init_key();
> +
>  	return count;
>  }
>