Re: [PATCH-v2 08/20] ext4 crypto: add encryption key management facilities

[Dmitry Monakhov <dmonlist@gmail.com>]

Theodore Ts'o <tytso@mit.edu> writes:

> From: Michael Halcrow <mhalcrow@google.com>
>
> Change-Id: I0cb5711a628554d3b05cefd15740d8346115cbaa
> Signed-off-by: Michael Halcrow <mhalcrow@google.com>
> Signed-off-by: Ildar Muslukhov <muslukhovi@gmail.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Hi, hope that it is not too late for discussion minor things

I'm wondering whenever derivation key algo (AEC-CBC) is too weak because allow
 attacker get master key once attacker find key for any inode in a filesystem.
 IMHO sane key derivation should be done via HMAC (HMACSHA256) or any
 other one direction procedure. 

Read file connect w/o key. 
   Currently this is prohibited which breaks a lot of applications. For
   example my backup scenario looks like rsync like script which copy
   content of home directory to some archive. Script has no
   access to my keys, so encrypted files will be backed-up. This is bad.
   IMHO it is reasonable to allow to read content of the file even w/o key but
   return encrypted content (as it is stored on disk). This is very
   similar to what we do with filenames. AFAIU this will no break our
   security assumptions 'steal once' because even if attacker has access
   to several versions of the encrypted file this simply equals to
   several files with same ctx.nonce (effectively equals encrypted file
   with size = sum of all files)
   If everybody are ok with that I'll send a draft patch.
> ---
>  fs/ext4/Makefile      |   2 +-
>  fs/ext4/crypto_key.c  | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/ext4/ext4.h        |  13 ++++
>  fs/ext4/ext4_crypto.h |   3 +
>  4 files changed, 179 insertions(+), 1 deletion(-)
>  create mode 100644 fs/ext4/crypto_key.c
>
> diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
> index 1b1c561..4e5af21 100644
> --- a/fs/ext4/Makefile
> +++ b/fs/ext4/Makefile
> @@ -12,4 +12,4 @@ ext4-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o page-io.o \
>  
>  ext4-$(CONFIG_EXT4_FS_POSIX_ACL)	+= acl.o
>  ext4-$(CONFIG_EXT4_FS_SECURITY)		+= xattr_security.o
> -ext4-$(CONFIG_EXT4_FS_ENCRYPTION)	+= crypto_policy.o crypto.o
> +ext4-$(CONFIG_EXT4_FS_ENCRYPTION)	+= crypto_policy.o crypto.o crypto_key.o
> diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
> new file mode 100644
> index 0000000..572bd97
> --- /dev/null
> +++ b/fs/ext4/crypto_key.c
> @@ -0,0 +1,162 @@
> +/*
> + * linux/fs/ext4/crypto_key.c
> + *
> + * Copyright (C) 2015, Google, Inc.
> + *
> + * This contains encryption key functions for ext4
> + *
> + * Written by Michael Halcrow, Ildar Muslukhov, and Uday Savagaonkar, 2015.
> + */
> +
> +#include <keys/encrypted-type.h>
> +#include <keys/user-type.h>
> +#include <linux/random.h>
> +#include <linux/scatterlist.h>
> +#include <uapi/linux/keyctl.h>
> +
> +#include "ext4.h"
> +#include "xattr.h"
> +
> +static void derive_crypt_complete(struct crypto_async_request *req, int rc)
> +{
> +	struct ext4_completion_result *ecr = req->data;
> +
> +	if (rc == -EINPROGRESS)
> +		return;
> +
> +	ecr->res = rc;
> +	complete(&ecr->completion);
> +}
> +
> +/**
> + * ext4_derive_key_aes() - Derive a key using AES-128-ECB
> + * @deriving_key: Encryption key used for derivatio.
> + * @source_key:   Source key to which to apply derivation.
> + * @derived_key:  Derived key.
> + *
> + * Return: Zero on success; non-zero otherwise.
> + */
> +static int ext4_derive_key_aes(char deriving_key[EXT4_AES_128_ECB_KEY_SIZE],
> +			       char source_key[EXT4_AES_256_XTS_KEY_SIZE],
> +			       char derived_key[EXT4_AES_256_XTS_KEY_SIZE])
> +{
> +	int res = 0;
> +	struct ablkcipher_request *req = NULL;
> +	DECLARE_EXT4_COMPLETION_RESULT(ecr);
> +	struct scatterlist src_sg, dst_sg;
> +	struct crypto_ablkcipher *tfm = crypto_alloc_ablkcipher("ecb(aes)", 0,
> +								0);
> +
> +	if (IS_ERR(tfm)) {
> +		res = PTR_ERR(tfm);
> +		tfm = NULL;
> +		goto out;
> +	}
> +	crypto_ablkcipher_set_flags(tfm, CRYPTO_TFM_REQ_WEAK_KEY);
> +	req = ablkcipher_request_alloc(tfm, GFP_NOFS);
> +	if (!req) {
> +		res = -ENOMEM;
> +		goto out;
> +	}
> +	ablkcipher_request_set_callback(req,
> +			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
> +			derive_crypt_complete, &ecr);
> +	res = crypto_ablkcipher_setkey(tfm, deriving_key,
> +				       EXT4_AES_128_ECB_KEY_SIZE);
> +	if (res < 0)
> +		goto out;
> +	sg_init_one(&src_sg, source_key, EXT4_AES_256_XTS_KEY_SIZE);
> +	sg_init_one(&dst_sg, derived_key, EXT4_AES_256_XTS_KEY_SIZE);
> +	ablkcipher_request_set_crypt(req, &src_sg, &dst_sg,
> +				     EXT4_AES_256_XTS_KEY_SIZE, NULL);
> +	res = crypto_ablkcipher_encrypt(req);
> +	if (res == -EINPROGRESS || res == -EBUSY) {
> +		BUG_ON(req->base.data != &ecr);
> +		wait_for_completion(&ecr.completion);
> +		res = ecr.res;
> +	}
> +
> +out:
> +	if (req)
> +		ablkcipher_request_free(req);
> +	if (tfm)
> +		crypto_free_ablkcipher(tfm);
> +	return res;
> +}
> +
> +/**
> + * ext4_generate_encryption_key() - generates an encryption key
> + * @inode: The inode to generate the encryption key for.
> + */
> +int ext4_generate_encryption_key(struct inode *inode)
> +{
> +	struct ext4_inode_info *ei = EXT4_I(inode);
> +	struct ext4_encryption_key *crypt_key = &ei->i_encryption_key;
> +	char full_key_descriptor[EXT4_KEY_DESC_PREFIX_SIZE +
> +				 (EXT4_KEY_DESCRIPTOR_SIZE * 2) + 1];
> +	struct key *keyring_key = NULL;
> +	struct ext4_encryption_key *master_key;
> +	struct ext4_encryption_context ctx;
> +	struct user_key_payload *ukp;
> +	int res = ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
> +				 EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> +				 &ctx, sizeof(ctx));
> +
> +	if (res != sizeof(ctx)) {
> +		if (res > 0)
> +			res = -EINVAL;
> +		goto out;
> +	}
> +	res = 0;
> +
> +	memcpy(full_key_descriptor, EXT4_KEY_DESC_PREFIX,
> +	       EXT4_KEY_DESC_PREFIX_SIZE);
> +	sprintf(full_key_descriptor + EXT4_KEY_DESC_PREFIX_SIZE,
> +		"%*phN", EXT4_KEY_DESCRIPTOR_SIZE,
> +		ctx.master_key_descriptor);
> +	full_key_descriptor[EXT4_KEY_DESC_PREFIX_SIZE +
> +			    (2 * EXT4_KEY_DESCRIPTOR_SIZE)] = '\0';
> +	keyring_key = request_key(&key_type_logon, full_key_descriptor, NULL);
> +	if (IS_ERR(keyring_key)) {
> +		res = PTR_ERR(keyring_key);
> +		keyring_key = NULL;
> +		goto out;
> +	}
> +	BUG_ON(keyring_key->type != &key_type_logon);
> +	ukp = ((struct user_key_payload *)keyring_key->payload.data);
> +	if (ukp->datalen != sizeof(struct ext4_encryption_key)) {
> +		res = -EINVAL;
> +		goto out;
> +	}
> +	master_key = (struct ext4_encryption_key *)ukp->data;
> +
> +	if (S_ISREG(inode->i_mode))
> +		crypt_key->mode = ctx.contents_encryption_mode;
> +	else if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
> +		crypt_key->mode = ctx.filenames_encryption_mode;
> +	else {
> +		printk(KERN_ERR "ext4 crypto: Unsupported inode type.\n");
> +		BUG();
> +	}
> +	crypt_key->size = ext4_encryption_key_size(crypt_key->mode);
> +	BUG_ON(!crypt_key->size);
> +	BUILD_BUG_ON(EXT4_AES_128_ECB_KEY_SIZE !=
> +		     EXT4_KEY_DERIVATION_NONCE_SIZE);
> +	BUG_ON(master_key->size != EXT4_AES_256_XTS_KEY_SIZE);
> +	BUG_ON(crypt_key->size < EXT4_AES_256_CBC_KEY_SIZE);
> +	res = ext4_derive_key_aes(ctx.nonce, master_key->raw, crypt_key->raw);
> +out:
> +	if (keyring_key)
> +		key_put(keyring_key);
> +	if (res < 0)
> +		crypt_key->mode = EXT4_ENCRYPTION_MODE_INVALID;
> +	return res;
> +}
> +
> +int ext4_has_encryption_key(struct inode *inode)
> +{
> +	struct ext4_inode_info *ei = EXT4_I(inode);
> +	struct ext4_encryption_key *crypt_key = &ei->i_encryption_key;
> +
> +	return (crypt_key->mode != EXT4_ENCRYPTION_MODE_INVALID);
> +}
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index c4a51636..89b999f 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2065,6 +2065,19 @@ static inline int ext4_sb_has_crypto(struct super_block *sb)
>  }
>  #endif
>  
> +/* crypto_key.c */
> +int ext4_generate_encryption_key(struct inode *inode);
> +
> +#ifdef CONFIG_EXT4_FS_ENCRYPTION
> +int ext4_has_encryption_key(struct inode *inode);
> +#else
> +static inline int ext4_has_encryption_key(struct inode *inode)
> +{
> +	return 0;
> +}
> +#endif
> +
> +
>  /* dir.c */
>  extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *,
>  				  struct file *,
> diff --git a/fs/ext4/ext4_crypto.h b/fs/ext4/ext4_crypto.h
> index 9d5d2e5..6a7c0c0 100644
> --- a/fs/ext4/ext4_crypto.h
> +++ b/fs/ext4/ext4_crypto.h
> @@ -55,6 +55,9 @@ struct ext4_encryption_context {
>  #define EXT4_AES_256_XTS_KEY_SIZE 64
>  #define EXT4_MAX_KEY_SIZE 64
>  
> +#define EXT4_KEY_DESC_PREFIX "ext4:"
> +#define EXT4_KEY_DESC_PREFIX_SIZE 5
> +
>  struct ext4_encryption_key {
>  	uint32_t mode;
>  	char raw[EXT4_MAX_KEY_SIZE];
> -- 
> 2.3.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html