[PATCH v2 0/7] fscrypt: key verification and KDF improvement

[PATCH v2 0/7] fscrypt: key verification and KDF improvement

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

This patch series solves two major problems which filesystem-level
encryption has currently.  First, the user-supplied master keys are not
verified, which means a malicious user can provide the wrong key for
another user's file and cause a DOS attack or worse.  This flaw has been
criticized in the past [1].  Second, the KDF (Key Derivation Function)
used to derive per-file keys is ad-hoc and nonstandard.  While it meets
the primary security requirement, it's inflexible and is missing some
useful properties such as non-reversibility, which is important under
some threat models.  This weakness was noted by Unterluggauer and
Mangard (2016) [2] who also demonstrated an EM attack against the
current AES-based KDF.

These problems are solved together by introducing a new encryption
policy version where the KDF is changed to HKDF-SHA512, i.e. RFC-5869
[3] with SHA-512 as the underlying hash function.  HKDF is used to
derive the per-file keys as well as to generate a "key hash" which is
stored on-disk to allow key verification.  The HMAC transform for each
master key is pre-keyed and cached, which in practice makes the new KDF
about as fast or even faster than the old one which did not use the
crypto API efficiently.

Please give special consideration to the choice and usage of crypto
algorithms and any other on-disk format and API changes, since we will
be locked into these once merged.

As a sanity check, I have verified the new on-disk format on ext4 by
dumping the encryption xattr and contents of an encrypted regular file
using debugfs, then reproducing the key_hash field and the encrypted
contents in userspace using the RFC-6234 example code and OpenSSL for
HKDF-SHA512 and AES-256-XTS respectively --- given the master key and
unencrypted file contents.

All these changes are independent of filesystem and encryption mode,
i.e. the "v2" encryption policies can be used on any fscrypt-capable
filesystem (ext4, f2fs, or ubifs currently) and with any of the
supported encryption modes.

Changes since v1:
    - Validate encryption modes and flags earlier in
      fscrypt_ioctl_set_policy(), since the modes are used in
      fscrypt_compute_key_hash().
    - Various cleanups
    - Base on v4.13-rc2

References:
  [1] https://blog.quarkslab.com/a-glimpse-of-ext4-filesystem-level-encryption.html

  [2] Unterluggauer and Mangard (2016).  "Exploiting the Physical
      Disparity: Side-Channel Attacks on Memory Encryption".
      https://eprint.iacr.org/2016/473.pdf

  [3] RFC 5869. "HMAC-based Extract-and-Expand Key Derivation Function
      (HKDF)".  https://tools.ietf.org/html/rfc5869

Eric Biggers (7):
  fscrypt: add v2 encryption context and policy
  fscrypt: rename ->ci_master_key to ->ci_master_key_descriptor
  fscrypt: use HKDF-SHA512 to derive the per-inode encryption keys
  fscrypt: validate modes and flags earlier when setting policy
  fscrypt: verify that the correct master key was supplied
  fscrypt: cache the HMAC transform for each master key
  fscrypt: for v2 policies, support "fscrypt:" key prefix only

 fs/crypto/Kconfig              |   2 +
 fs/crypto/fscrypt_private.h    | 113 +++++--
 fs/crypto/keyinfo.c            | 668 ++++++++++++++++++++++++++++++++++-------
 fs/crypto/policy.c             | 138 ++++++---
 fs/super.c                     |   4 +
 include/linux/fs.h             |   5 +
 include/linux/fscrypt_common.h |   2 +-
 include/uapi/linux/fs.h        |   6 +
 8 files changed, 779 insertions(+), 159 deletions(-)

-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH v2 1/7] fscrypt: add v2 encryption context and policy

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Currently, the fscrypt_context (i.e. the encryption xattr) does not
contain a cryptographically secure identifier for the master key's
payload.  Therefore it's not possible to verify that the correct key was
supplied, which is problematic in multi-user scenarios.  To make this
possible, define a new fscrypt_context version (v2) which includes a
key_hash field, and allow userspace to opt-in to it when setting an
encryption policy by setting fscrypt_policy.version to 2.  For now just
zero the new field; a later patch will start setting it for real.

Even though we aren't changing the layout of struct fscrypt_policy (i.e.
the struct used by the ioctls), the new context version still has to be
"opt-in" because old kernels will not recognize it, and the keyring key
will now need to be available when setting an encryption policy, which
is an API change.  We'll also be taking the opportunity to make another
API change (dropping support for the filesystem-specific key prefixes).

Previously, the version numbers were 0 in the fscrypt_policy and 1 in
the fscrypt_context.  Rather than incrementing them to 1 and 2, make
them both 2 to be consistent with each other.  It's not required that
these numbers match, but it should make things less confusing.

An alternative to adding a key_hash field would have been to reuse
master_key_descriptor.  However, master_key_descriptor is only 8 bytes,
which is too short to be a cryptographically secure hash.  Thus,
master_key_descriptor would have needed to be lengthened to 16+ bytes,
which would have required defining a fscrypt_policy_v2 structure and
adding a FS_IOC_GET_ENCRYPTION_POLICY_V2 ioctl.  It also would have
required userspace to start using a specific hash algorithm to create
the key descriptors, which would have made the API harder to use.
Perhaps it should have been done that way originally, but at this point
it seems better to keep the API simpler.

Acked-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h    | 84 ++++++++++++++++++++++++++++++++++--------
 fs/crypto/keyinfo.c            | 14 +++----
 fs/crypto/policy.c             | 73 ++++++++++++++++++++++++++----------
 include/linux/fscrypt_common.h |  2 +-
 include/uapi/linux/fs.h        |  6 +++
 5 files changed, 135 insertions(+), 44 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a1d5021c31ef..8329fb905ac6 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -24,40 +24,92 @@
 #define FS_AES_256_CTS_KEY_SIZE		32
 #define FS_AES_256_XTS_KEY_SIZE		64
 
-#define FS_KEY_DERIVATION_NONCE_SIZE		16
+#define FS_KEY_DERIVATION_NONCE_SIZE	16
+#define FSCRYPT_KEY_HASH_SIZE		16
 
 /**
- * Encryption context for inode
+ * fscrypt_context - the encryption context for an inode
  *
- * Protector format:
- *  1 byte: Protector format (1 = this version)
- *  1 byte: File contents encryption mode
- *  1 byte: File names encryption mode
- *  1 byte: Flags
- *  8 bytes: Master Key descriptor
- *  16 bytes: Encryption Key derivation nonce
+ * Filesystems usually store this in an extended attribute.  It identifies the
+ * encryption algorithm and key with which the file is encrypted.
+ *
+ * Since this is stored on-disk, be careful not to reorder fields or add any
+ * implicit padding bytes!
  */
 struct fscrypt_context {
-	u8 format;
+	/* v1+ */
+
+	/* Version of this structure */
+	u8 version;
+
+	/* Encryption mode for the contents of regular files */
 	u8 contents_encryption_mode;
+
+	/* Encryption mode for filenames in directories and symlink targets */
 	u8 filenames_encryption_mode;
+
+	/* Options that affect how encryption is done (e.g. padding amount) */
 	u8 flags;
+
+	/* Descriptor for this file's master key in the keyring */
 	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
+
+	/*
+	 * A unique value used in combination with the master key to derive the
+	 * file's actual encryption key
+	 */
 	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
-} __packed;
 
-#define FS_ENCRYPTION_CONTEXT_FORMAT_V1		1
+	/* v2+ */
+
+	/* Cryptographically secure hash of the master key */
+	u8 key_hash[FSCRYPT_KEY_HASH_SIZE];
+};
+
+#define FSCRYPT_CONTEXT_V1	1
+#define FSCRYPT_CONTEXT_V1_SIZE	offsetof(struct fscrypt_context, key_hash)
+
+#define FSCRYPT_CONTEXT_V2	2
+#define FSCRYPT_CONTEXT_V2_SIZE	sizeof(struct fscrypt_context)
+
+static inline int fscrypt_context_size(const struct fscrypt_context *ctx)
+{
+	switch (ctx->version) {
+	case FSCRYPT_CONTEXT_V1:
+		return FSCRYPT_CONTEXT_V1_SIZE;
+	case FSCRYPT_CONTEXT_V2:
+		return FSCRYPT_CONTEXT_V2_SIZE;
+	}
+	return 0;
+}
+
+static inline bool
+fscrypt_valid_context_format(const struct fscrypt_context *ctx, int size)
+{
+	return size >= 1 && size == fscrypt_context_size(ctx);
+}
 
 /*
- * A pointer to this structure is stored in the file system's in-core
- * representation of an inode.
+ * fscrypt_info - the "encryption key" for an inode
+ *
+ * When an encrypted file's key is made available, an instance of this struct is
+ * allocated and stored in ->i_crypt_info.  Once created, it remains until the
+ * inode is evicted.
  */
 struct fscrypt_info {
+
+	/* The actual crypto transforms needed for encryption and decryption */
+	struct crypto_skcipher *ci_ctfm;
+	struct crypto_cipher *ci_essiv_tfm;
+
+	/*
+	 * Cached fields from the fscrypt_context needed for encryption policy
+	 * inheritance and enforcement
+	 */
+	u8 ci_context_version;
 	u8 ci_data_mode;
 	u8 ci_filename_mode;
 	u8 ci_flags;
-	struct crypto_skcipher *ci_ctfm;
-	struct crypto_cipher *ci_essiv_tfm;
 	u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE];
 };
 
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 018c588c7ac3..7e664a11340a 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -272,29 +272,27 @@ int fscrypt_get_encryption_info(struct inode *inode)
 			return res;
 		/* Fake up a context for an unencrypted directory */
 		memset(&ctx, 0, sizeof(ctx));
-		ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
+		ctx.version = FSCRYPT_CONTEXT_V1;
 		ctx.contents_encryption_mode = FS_ENCRYPTION_MODE_AES_256_XTS;
 		ctx.filenames_encryption_mode = FS_ENCRYPTION_MODE_AES_256_CTS;
 		memset(ctx.master_key_descriptor, 0x42, FS_KEY_DESCRIPTOR_SIZE);
-	} else if (res != sizeof(ctx)) {
-		return -EINVAL;
+		res = FSCRYPT_CONTEXT_V1_SIZE;
 	}
 
-	if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
+	if (!fscrypt_valid_context_format(&ctx, res))
 		return -EINVAL;
 
 	if (ctx.flags & ~FS_POLICY_FLAGS_VALID)
 		return -EINVAL;
 
-	crypt_info = kmem_cache_alloc(fscrypt_info_cachep, GFP_NOFS);
+	crypt_info = kmem_cache_zalloc(fscrypt_info_cachep, GFP_NOFS);
 	if (!crypt_info)
 		return -ENOMEM;
 
-	crypt_info->ci_flags = ctx.flags;
+	crypt_info->ci_context_version = ctx.version;
 	crypt_info->ci_data_mode = ctx.contents_encryption_mode;
 	crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
-	crypt_info->ci_ctfm = NULL;
-	crypt_info->ci_essiv_tfm = NULL;
+	crypt_info->ci_flags = ctx.flags;
 	memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
 				sizeof(crypt_info->ci_master_key));
 
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index ce07a86200f3..d6beb48f08fc 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -13,6 +13,30 @@
 #include <linux/mount.h>
 #include "fscrypt_private.h"
 
+/* Note: requires a valid context version! */
+static u8 policy_version_for_context(const struct fscrypt_context *ctx)
+{
+	switch (ctx->version) {
+	case FSCRYPT_CONTEXT_V1:
+		return FS_POLICY_VERSION_ORIGINAL;
+	case FSCRYPT_CONTEXT_V2:
+		return FS_POLICY_VERSION_HKDF;
+	}
+	BUG();
+}
+
+/* Note: requires a valid policy version! */
+static u8 context_version_for_policy(const struct fscrypt_policy *policy)
+{
+	switch (policy->version) {
+	case FS_POLICY_VERSION_ORIGINAL:
+		return FSCRYPT_CONTEXT_V1;
+	case FS_POLICY_VERSION_HKDF:
+		return FSCRYPT_CONTEXT_V2;
+	}
+	BUG();
+}
+
 /*
  * check whether an encryption policy is consistent with an encryption context
  */
@@ -20,8 +44,10 @@ static bool is_encryption_context_consistent_with_policy(
 				const struct fscrypt_context *ctx,
 				const struct fscrypt_policy *policy)
 {
-	return memcmp(ctx->master_key_descriptor, policy->master_key_descriptor,
-		      FS_KEY_DESCRIPTOR_SIZE) == 0 &&
+	return (ctx->version == context_version_for_policy(policy)) &&
+		(memcmp(ctx->master_key_descriptor,
+			policy->master_key_descriptor,
+			FS_KEY_DESCRIPTOR_SIZE) == 0) &&
 		(ctx->flags == policy->flags) &&
 		(ctx->contents_encryption_mode ==
 		 policy->contents_encryption_mode) &&
@@ -34,10 +60,6 @@ static int create_encryption_context_from_policy(struct inode *inode,
 {
 	struct fscrypt_context ctx;
 
-	ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
-	memcpy(ctx.master_key_descriptor, policy->master_key_descriptor,
-					FS_KEY_DESCRIPTOR_SIZE);
-
 	if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
 				     policy->filenames_encryption_mode))
 		return -EINVAL;
@@ -45,13 +67,20 @@ static int create_encryption_context_from_policy(struct inode *inode,
 	if (policy->flags & ~FS_POLICY_FLAGS_VALID)
 		return -EINVAL;
 
+	ctx.version = context_version_for_policy(policy);
 	ctx.contents_encryption_mode = policy->contents_encryption_mode;
 	ctx.filenames_encryption_mode = policy->filenames_encryption_mode;
 	ctx.flags = policy->flags;
+	memcpy(ctx.master_key_descriptor, policy->master_key_descriptor,
+	       FS_KEY_DESCRIPTOR_SIZE);
 	BUILD_BUG_ON(sizeof(ctx.nonce) != FS_KEY_DERIVATION_NONCE_SIZE);
 	get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
+	if (ctx.version != FSCRYPT_CONTEXT_V1)
+		memset(ctx.key_hash, 0, FSCRYPT_KEY_HASH_SIZE);
 
-	return inode->i_sb->s_cop->set_context(inode, &ctx, sizeof(ctx), NULL);
+	return inode->i_sb->s_cop->set_context(inode, &ctx,
+					       fscrypt_context_size(&ctx),
+					       NULL);
 }
 
 int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
@@ -67,7 +96,8 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 	if (!inode_owner_or_capable(inode))
 		return -EACCES;
 
-	if (policy.version != 0)
+	if (policy.version != FS_POLICY_VERSION_ORIGINAL &&
+	    policy.version != FS_POLICY_VERSION_HKDF)
 		return -EINVAL;
 
 	ret = mnt_want_write_file(filp);
@@ -85,7 +115,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 		else
 			ret = create_encryption_context_from_policy(inode,
 								    &policy);
-	} else if (ret == sizeof(ctx) &&
+	} else if (ret >= 0 && fscrypt_valid_context_format(&ctx, ret) &&
 		   is_encryption_context_consistent_with_policy(&ctx,
 								&policy)) {
 		/* The file already uses the same encryption policy. */
@@ -113,14 +143,12 @@ int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
 		return -ENODATA;
 
 	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
-	if (res < 0 && res != -ERANGE)
-		return res;
-	if (res != sizeof(ctx))
-		return -EINVAL;
-	if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
+	if (res < 0)
+		return (res == -ERANGE) ? -EINVAL : res;
+	if (!fscrypt_valid_context_format(&ctx, res))
 		return -EINVAL;
 
-	policy.version = 0;
+	policy.version = policy_version_for_context(&ctx);
 	policy.contents_encryption_mode = ctx.contents_encryption_mode;
 	policy.filenames_encryption_mode = ctx.filenames_encryption_mode;
 	policy.flags = ctx.flags;
@@ -200,6 +228,8 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 	if (parent_ci && child_ci) {
 		return memcmp(parent_ci->ci_master_key, child_ci->ci_master_key,
 			      FS_KEY_DESCRIPTOR_SIZE) == 0 &&
+			(parent_ci->ci_context_version ==
+			 child_ci->ci_context_version) &&
 			(parent_ci->ci_data_mode == child_ci->ci_data_mode) &&
 			(parent_ci->ci_filename_mode ==
 			 child_ci->ci_filename_mode) &&
@@ -207,16 +237,17 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 	}
 
 	res = cops->get_context(parent, &parent_ctx, sizeof(parent_ctx));
-	if (res != sizeof(parent_ctx))
+	if (res < 0 || !fscrypt_valid_context_format(&parent_ctx, res))
 		return 0;
 
 	res = cops->get_context(child, &child_ctx, sizeof(child_ctx));
-	if (res != sizeof(child_ctx))
+	if (res < 0 || !fscrypt_valid_context_format(&child_ctx, res))
 		return 0;
 
 	return memcmp(parent_ctx.master_key_descriptor,
 		      child_ctx.master_key_descriptor,
 		      FS_KEY_DESCRIPTOR_SIZE) == 0 &&
+		(parent_ctx.version == child_ctx.version) &&
 		(parent_ctx.contents_encryption_mode ==
 		 child_ctx.contents_encryption_mode) &&
 		(parent_ctx.filenames_encryption_mode ==
@@ -249,16 +280,20 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 	if (ci == NULL)
 		return -ENOKEY;
 
-	ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
+	ctx.version = ci->ci_context_version;
 	ctx.contents_encryption_mode = ci->ci_data_mode;
 	ctx.filenames_encryption_mode = ci->ci_filename_mode;
 	ctx.flags = ci->ci_flags;
 	memcpy(ctx.master_key_descriptor, ci->ci_master_key,
 	       FS_KEY_DESCRIPTOR_SIZE);
 	get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
+	if (ctx.version != FSCRYPT_CONTEXT_V1)
+		memset(ctx.key_hash, 0, FSCRYPT_KEY_HASH_SIZE);
+
 	BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
 	res = parent->i_sb->s_cop->set_context(child, &ctx,
-						sizeof(ctx), fs_data);
+					       fscrypt_context_size(&ctx),
+					       fs_data);
 	if (res)
 		return res;
 	return preload ? fscrypt_get_encryption_info(child): 0;
diff --git a/include/linux/fscrypt_common.h b/include/linux/fscrypt_common.h
index 97f738628b36..c08e8ae63a02 100644
--- a/include/linux/fscrypt_common.h
+++ b/include/linux/fscrypt_common.h
@@ -84,7 +84,7 @@ struct fscrypt_operations {
 };
 
 /* Maximum value for the third parameter of fscrypt_operations.set_context(). */
-#define FSCRYPT_SET_CONTEXT_MAX_SIZE	28
+#define FSCRYPT_SET_CONTEXT_MAX_SIZE	44
 
 static inline bool fscrypt_dummy_context_enabled(struct inode *inode)
 {
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index b7495d05e8de..a5423ddd3b67 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -257,6 +257,12 @@ struct fsxattr {
  * File system encryption support
  */
 /* Policy provided via an ioctl on the topmost directory */
+
+/* original policy version, no key verification (potentially insecure) */
+#define FS_POLICY_VERSION_ORIGINAL	0
+/* new version w/ HKDF and key verification (recommended) */
+#define FS_POLICY_VERSION_HKDF		2
+
 #define FS_KEY_DESCRIPTOR_SIZE	8
 
 #define FS_POLICY_FLAGS_PAD_4		0x00
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH v2 2/7] fscrypt: rename ->ci_master_key to ->ci_master_key_descriptor

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

In struct fscrypt_info, ->ci_master_key is the master key descriptor,
not the master key itself.  In preparation for introducing a struct
fscrypt_master_key and making ->ci_master_key point to it, rename the
existing ->ci_master_key to ->ci_master_key_descriptor.

Acked-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h | 2 +-
 fs/crypto/keyinfo.c         | 4 ++--
 fs/crypto/policy.c          | 5 +++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 8329fb905ac6..4b0205accae9 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -110,7 +110,7 @@ struct fscrypt_info {
 	u8 ci_data_mode;
 	u8 ci_filename_mode;
 	u8 ci_flags;
-	u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE];
+	u8 ci_master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
 };
 
 typedef enum {
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 7e664a11340a..5591fd24e4b2 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -293,8 +293,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	crypt_info->ci_data_mode = ctx.contents_encryption_mode;
 	crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
 	crypt_info->ci_flags = ctx.flags;
-	memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
-				sizeof(crypt_info->ci_master_key));
+	memcpy(crypt_info->ci_master_key_descriptor, ctx.master_key_descriptor,
+	       FS_KEY_DESCRIPTOR_SIZE);
 
 	res = determine_cipher_type(crypt_info, inode, &cipher_str, &keysize);
 	if (res)
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index d6beb48f08fc..fe525da9e79c 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -226,7 +226,8 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 	child_ci = child->i_crypt_info;
 
 	if (parent_ci && child_ci) {
-		return memcmp(parent_ci->ci_master_key, child_ci->ci_master_key,
+		return memcmp(parent_ci->ci_master_key_descriptor,
+			      child_ci->ci_master_key_descriptor,
 			      FS_KEY_DESCRIPTOR_SIZE) == 0 &&
 			(parent_ci->ci_context_version ==
 			 child_ci->ci_context_version) &&
@@ -284,7 +285,7 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 	ctx.contents_encryption_mode = ci->ci_data_mode;
 	ctx.filenames_encryption_mode = ci->ci_filename_mode;
 	ctx.flags = ci->ci_flags;
-	memcpy(ctx.master_key_descriptor, ci->ci_master_key,
+	memcpy(ctx.master_key_descriptor, ci->ci_master_key_descriptor,
 	       FS_KEY_DESCRIPTOR_SIZE);
 	get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
 	if (ctx.version != FSCRYPT_CONTEXT_V1)
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH v2 3/7] fscrypt: use HKDF-SHA512 to derive the per-inode encryption keys

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

By design, the keys which userspace provides in the keyring are not used
to encrypt data directly.  Instead, a KDF (Key Derivation Function) is
used to derive a unique encryption key for each inode, given a "master"
key and a nonce.  The current KDF encrypts the master key with
AES-128-ECB using the nonce as the AES key.  This KDF is ad-hoc and is
not specified in any standard.  While it does generate unique derived
keys with sufficient entropy, it has several disadvantages:

- It's reversible: an attacker who compromises a derived key, e.g. using
  a side channel attack, can "decrypt" it to get back to the master key.

- It's not very extensible because it cannot easily be used to derive
  other key material that may be needed and it ties the length of the
  derived key closely to the length of the master key.

- It doesn't evenly distribute the entropy from the master key.  For
  example, the first 16 bytes of each derived key depend only on the
  first 16 bytes of the master key.

- It uses a public value as an AES key, which is unusual.  Ciphers are
  rarely evaluated under a threat model where the keys are public and
  the messages are secret.

Solve all these problems for v2 encryption policies by changing the KDF
to HKDF with SHA-512 as the underlying hash function.  To derive each
inode's encryption key, HKDF is executed with the master key as the
input key material, a fixed salt, and the per-inode nonce prefixed with
a context byte as the application-specific information string.  Unlike
the current KDF, HKDF has been formally published and standardized
[1][2], is nonreversible, can be used to derive any number and length of
secret and/or non-secret keys, and evenly distributes the entropy from
the master key (excepting limits inherent to not using a random salt).

Note that this introduces a dependency on the security and
implementation of SHA-512, whereas before we were using only AES for
both key derivation and encryption.  However, by using HMAC rather than
the hash function directly, HKDF is designed to remain secure even if
various classes of attacks, e.g. collision attacks, are found against
the underlying unkeyed hash function.  Even HMAC-MD5 is still considered
secure in practice, despite MD5 itself having been heavily compromised.

We *could* avoid introducing a hash function by instantiating
HKDF-Expand with CMAC-AES256 as the pseudorandom function rather than
HMAC-SHA512.  This would work; however, the HKDF specification doesn't
explicitly allow a non-HMAC pseudorandom function, so it would be less
standard.  It would also require skipping HKDF-Extract and changing the
API to accept only 32-byte master keys (since otherwise HKDF-Extract
using CMAC-AES would produce a pseudorandom key only 16 bytes long which
is only enough for AES-128, not AES-256).

HKDF-SHA512 can require more "crypto work" per key derivation when
compared to the current KDF.  However, later in this series, we'll start
caching the HMAC transform for each master key, which will actually make
the real-world performance about the same or even significantly better
than the AES-based KDF as currently implemented.  Also, each KDF can
actually be executed on the order of 1 million times per second, so KDF
performance probably isn't actually the bottleneck in practice anyway.

References:
  [1] Krawczyk (2010). "Cryptographic Extraction and Key Derivation: The
      HKDF Scheme".  https://eprint.iacr.org/2010/264.pdf

  [2] RFC 5869. "HMAC-based Extract-and-Expand Key Derivation Function
      (HKDF)".  https://tools.ietf.org/html/rfc5869

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/Kconfig           |   2 +
 fs/crypto/fscrypt_private.h |  13 ++
 fs/crypto/keyinfo.c         | 484 +++++++++++++++++++++++++++++++++++---------
 3 files changed, 403 insertions(+), 96 deletions(-)

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 02b7d91c9231..bbd4e38b293c 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -8,6 +8,8 @@ config FS_ENCRYPTION
 	select CRYPTO_CTS
 	select CRYPTO_CTR
 	select CRYPTO_SHA256
+	select CRYPTO_SHA512
+	select CRYPTO_HMAC
 	select KEYS
 	help
 	  Enable encryption of files and directories.  This
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4b0205accae9..b5881cb26e8f 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -89,6 +89,13 @@ fscrypt_valid_context_format(const struct fscrypt_context *ctx, int size)
 	return size >= 1 && size == fscrypt_context_size(ctx);
 }
 
+/*
+ * fscrypt_master_key - an in-use master key
+ */
+struct fscrypt_master_key {
+	struct crypto_shash	*mk_hmac;
+};
+
 /*
  * fscrypt_info - the "encryption key" for an inode
  *
@@ -102,6 +109,12 @@ struct fscrypt_info {
 	struct crypto_skcipher *ci_ctfm;
 	struct crypto_cipher *ci_essiv_tfm;
 
+	/*
+	 * The master key with which this inode was "unlocked"
+	 * (only set for inodes that use a v2+ encryption policy)
+	 */
+	struct fscrypt_master_key *ci_master_key;
+
 	/*
 	 * Cached fields from the fscrypt_context needed for encryption policy
 	 * inheritance and enforcement
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 5591fd24e4b2..3164e1c74196 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -6,17 +6,311 @@
  * This contains encryption key functions.
  *
  * Written by Michael Halcrow, Ildar Muslukhov, and Uday Savagaonkar, 2015.
+ * HKDF support added by Eric Biggers, 2017.
+ *
+ * The implementation and usage of HKDF should conform to RFC-5869 ("HMAC-based
+ * Extract-and-Expand Key Derivation Function").
  */
 
 #include <keys/user-type.h>
 #include <linux/scatterlist.h>
 #include <linux/ratelimit.h>
 #include <crypto/aes.h>
+#include <crypto/hash.h>
 #include <crypto/sha.h>
 #include "fscrypt_private.h"
 
 static struct crypto_shash *essiv_hash_tfm;
 
+/*
+ * Any unkeyed cryptographic hash algorithm can be used with HKDF, but we use
+ * SHA-512 because it is reasonably secure and efficient; and since it produces
+ * a 64-byte digest, deriving an AES-256-XTS key preserves all 64 bytes of
+ * entropy from the master key and requires only one iteration of HKDF-Expand.
+ */
+#define HKDF_HMAC_ALG		"hmac(sha512)"
+#define HKDF_HASHLEN		SHA512_DIGEST_SIZE
+
+/*
+ * The list of contexts in which we use HKDF to derive additional keys from a
+ * master key.  The values in this list are used as the first byte of the
+ * application-specific info string to guarantee that info strings are never
+ * repeated between contexts.
+ *
+ * Keys derived with different info strings are cryptographically isolated from
+ * each other --- knowledge of one derived key doesn't reveal any others.
+ */
+#define HKDF_CONTEXT_PER_FILE_KEY	1
+
+/*
+ * HKDF consists of two steps:
+ *
+ * 1. HKDF-Extract: extract a fixed-length pseudorandom key from the
+ *    input keying material and optional salt.
+ * 2. HKDF-Expand: expand the pseudorandom key into output keying material of
+ *    any length, parameterized by an application-specific info string.
+ *
+ * HKDF-Extract can be skipped if the input is already a good pseudorandom key
+ * that is at least as long as the hash.  While the fscrypt master keys should
+ * already be good pseudorandom keys, when using encryption algorithms that use
+ * short keys (e.g. AES-128-CBC) we'd like to permit the master key to be
+ * shorter than HKDF_HASHLEN bytes.  Thus, we still must do HKDF-Extract.
+ *
+ * Ideally, HKDF-Extract would be passed a random salt for each distinct input
+ * key.  Details about the advantages of a random salt can be found in the HKDF
+ * paper (Krawczyk, 2010; "Cryptographic Extraction and Key Derivation: The HKDF
+ * Scheme").  However, we do not have the ability to store a salt on a
+ * per-master-key basis.  Thus, we have to use a fixed salt.  This is sufficient
+ * as long as the master keys are already pseudorandom and are long enough to
+ * make dictionary attacks infeasible.  This should be the case if userspace
+ * used a cryptographically secure random number generator, e.g. /dev/urandom,
+ * to generate the master keys and it was initialized with sufficient entropy.
+ *
+ * For the fixed salt we use "fscrypt_hkdf_salt" rather than default of all 0's
+ * defined by RFC-5869.  This is only to be slightly more robust against
+ * userspace (unwisely) reusing the master keys for different purposes.
+ * Logically, it's more likely that the keys would be passed to unsalted
+ * HKDF-SHA512 than specifically to "fscrypt_hkdf_salt"-salted HKDF-SHA512.
+ * (Of course, a random salt would be better for this purpose.)
+ */
+
+#define HKDF_SALT		"fscrypt_hkdf_salt"
+#define HKDF_SALT_LEN		(sizeof(HKDF_SALT) - 1)
+
+/*
+ * HKDF-Extract (RFC-5869 section 2.2).  This extracts a pseudorandom key 'prk'
+ * from the input key material 'ikm' and a salt.  See explanation above for why
+ * we use a fixed salt.
+ */
+static int hkdf_extract(struct crypto_shash *hmac,
+			const u8 *ikm, unsigned int ikmlen,
+			u8 prk[HKDF_HASHLEN])
+{
+	SHASH_DESC_ON_STACK(desc, hmac);
+	int err;
+
+	desc->tfm = hmac;
+	desc->flags = 0;
+
+	err = crypto_shash_setkey(hmac, HKDF_SALT, HKDF_SALT_LEN);
+	if (err)
+		goto out;
+
+	err = crypto_shash_digest(desc, ikm, ikmlen, prk);
+out:
+	shash_desc_zero(desc);
+	return err;
+}
+
+/*
+ * HKDF-Expand (RFC-5869 section 2.3).  This expands the pseudorandom key, which
+ * has already been keyed into 'hmac', into 'okmlen' bytes of output keying
+ * material, parameterized by the application-specific information string of
+ * 'info' prefixed with the 'context' byte.  ('context' isn't part of the HKDF
+ * specification; it's just a prefix we add to our application-specific info
+ * strings to guarantee that we don't accidentally repeat an info string when
+ * using HKDF for different purposes.)
+ */
+static int hkdf_expand(struct crypto_shash *hmac, u8 context,
+		       const u8 *info, unsigned int infolen,
+		       u8 *okm, unsigned int okmlen)
+{
+	SHASH_DESC_ON_STACK(desc, hmac);
+	int err;
+	const u8 *prev = NULL;
+	unsigned int i;
+	u8 counter = 1;
+	u8 tmp[HKDF_HASHLEN];
+
+	desc->tfm = hmac;
+	desc->flags = 0;
+
+	if (unlikely(okmlen > 255 * HKDF_HASHLEN))
+		return -EINVAL;
+
+	for (i = 0; i < okmlen; i += HKDF_HASHLEN) {
+
+		err = crypto_shash_init(desc);
+		if (err)
+			goto out;
+
+		if (prev) {
+			err = crypto_shash_update(desc, prev, HKDF_HASHLEN);
+			if (err)
+				goto out;
+		}
+
+		err = crypto_shash_update(desc, &context, 1);
+		if (err)
+			goto out;
+
+		err = crypto_shash_update(desc, info, infolen);
+		if (err)
+			goto out;
+
+		if (okmlen - i < HKDF_HASHLEN) {
+			err = crypto_shash_finup(desc, &counter, 1, tmp);
+			if (err)
+				goto out;
+			memcpy(&okm[i], tmp, okmlen - i);
+			memzero_explicit(tmp, sizeof(tmp));
+		} else {
+			err = crypto_shash_finup(desc, &counter, 1, &okm[i]);
+			if (err)
+				goto out;
+		}
+		counter++;
+		prev = &okm[i];
+	}
+	err = 0;
+out:
+	shash_desc_zero(desc);
+	return err;
+}
+
+static void put_master_key(struct fscrypt_master_key *k)
+{
+	if (!k)
+		return;
+
+	crypto_free_shash(k->mk_hmac);
+	kzfree(k);
+}
+
+/*
+ * Allocate an fscrypt_master_key, given the keyring key payload.  This includes
+ * allocating and keying an HMAC transform so that we can efficiently derive the
+ * per-inode encryption keys with HKDF-Expand later.
+ */
+static struct fscrypt_master_key *
+alloc_master_key(const struct fscrypt_key *payload)
+{
+	struct fscrypt_master_key *k;
+	int err;
+	u8 prk[HKDF_HASHLEN];
+
+	k = kzalloc(sizeof(*k), GFP_NOFS);
+	if (!k)
+		return ERR_PTR(-ENOMEM);
+
+	k->mk_hmac = crypto_alloc_shash(HKDF_HMAC_ALG, 0, 0);
+	if (IS_ERR(k->mk_hmac)) {
+		err = PTR_ERR(k->mk_hmac);
+		k->mk_hmac = NULL;
+		pr_warn("fscrypt: error allocating " HKDF_HMAC_ALG ": %d\n",
+			err);
+		goto fail;
+	}
+
+	BUG_ON(crypto_shash_digestsize(k->mk_hmac) != sizeof(prk));
+
+	err = hkdf_extract(k->mk_hmac, payload->raw, payload->size, prk);
+	if (err)
+		goto fail;
+
+	err = crypto_shash_setkey(k->mk_hmac, prk, sizeof(prk));
+	if (err)
+		goto fail;
+out:
+	memzero_explicit(prk, sizeof(prk));
+	return k;
+
+fail:
+	put_master_key(k);
+	k = ERR_PTR(err);
+	goto out;
+}
+
+static void release_keyring_key(struct key *keyring_key)
+{
+	up_read(&keyring_key->sem);
+	key_put(keyring_key);
+}
+
+/*
+ * Find, lock, and validate the master key with the keyring description
+ * prefix:descriptor.  It must be released with release_keyring_key() later.
+ */
+static struct key *
+find_and_lock_keyring_key(const char *prefix,
+			  const u8 descriptor[FS_KEY_DESCRIPTOR_SIZE],
+			  unsigned int min_keysize,
+			  const struct fscrypt_key **payload_ret)
+{
+	char *description;
+	struct key *keyring_key;
+	const struct user_key_payload *ukp;
+	const struct fscrypt_key *payload;
+
+	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
+				FS_KEY_DESCRIPTOR_SIZE, descriptor);
+	if (!description)
+		return ERR_PTR(-ENOMEM);
+
+	keyring_key = request_key(&key_type_logon, description, NULL);
+	if (IS_ERR(keyring_key))
+		goto out;
+
+	down_read(&keyring_key->sem);
+	ukp = user_key_payload_locked(keyring_key);
+	payload = (const struct fscrypt_key *)ukp->data;
+
+	if (ukp->datalen != sizeof(struct fscrypt_key) ||
+	    payload->size == 0 || payload->size > FS_MAX_KEY_SIZE) {
+		pr_warn_ratelimited("fscrypt: key '%s' has invalid payload\n",
+				    description);
+		goto invalid;
+	}
+
+	/*
+	 * With the legacy AES-based KDF the master key must be at least as long
+	 * as the derived key.  With HKDF we could accept a shorter master key;
+	 * however, that would mean the derived key wouldn't contain as much
+	 * entropy as intended.  So don't allow it in either case.
+	 */
+	if (payload->size < min_keysize) {
+		pr_warn_ratelimited("fscrypt: key '%s' is too short (got %u bytes, wanted %u+ bytes)\n",
+				    description, payload->size, min_keysize);
+		goto invalid;
+	}
+
+	*payload_ret = payload;
+out:
+	kfree(description);
+	return keyring_key;
+
+invalid:
+	release_keyring_key(keyring_key);
+	keyring_key = ERR_PTR(-ENOKEY);
+	goto out;
+}
+
+static struct fscrypt_master_key *
+load_master_key_from_keyring(const struct inode *inode,
+			     const u8 descriptor[FS_KEY_DESCRIPTOR_SIZE],
+			     unsigned int min_keysize)
+{
+	struct key *keyring_key;
+	const struct fscrypt_key *payload;
+	struct fscrypt_master_key *master_key;
+
+	keyring_key = find_and_lock_keyring_key(FS_KEY_DESC_PREFIX, descriptor,
+						min_keysize, &payload);
+	if (keyring_key == ERR_PTR(-ENOKEY) && inode->i_sb->s_cop->key_prefix) {
+		keyring_key = find_and_lock_keyring_key(
+					inode->i_sb->s_cop->key_prefix,
+					descriptor, min_keysize, &payload);
+	}
+	if (IS_ERR(keyring_key))
+		return ERR_CAST(keyring_key);
+
+	master_key = alloc_master_key(payload);
+
+	release_keyring_key(keyring_key);
+
+	return master_key;
+}
+
 static void derive_crypt_complete(struct crypto_async_request *req, int rc)
 {
 	struct fscrypt_completion_result *ecr = req->data;
@@ -28,107 +322,100 @@ static void derive_crypt_complete(struct crypto_async_request *req, int rc)
 	complete(&ecr->completion);
 }
 
-/**
- * derive_key_aes() - Derive a key using AES-128-ECB
- * @deriving_key: Encryption key used for derivation.
- * @source_key:   Source key to which to apply derivation.
- * @derived_raw_key:  Derived raw key.
- *
- * Return: Zero on success; non-zero otherwise.
+/*
+ * Legacy key derivation function.  This generates the derived key by encrypting
+ * the master key with AES-128-ECB using the nonce as the AES key.  This
+ * provides a unique derived key for each inode, but it's nonstandard, isn't
+ * very extensible, and has the weakness that it's trivially reversible: an
+ * attacker who compromises a derived key, e.g. with a side channel attack, can
+ * "decrypt" it to get back to the master key, then derive any other key.
  */
-static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
-				const struct fscrypt_key *source_key,
-				u8 derived_raw_key[FS_MAX_KEY_SIZE])
+static int derive_key_aes(const struct fscrypt_key *master_key,
+			  const struct fscrypt_context *ctx,
+			  u8 *derived_key, unsigned int derived_keysize)
 {
-	int res = 0;
+	int err;
 	struct skcipher_request *req = NULL;
 	DECLARE_FS_COMPLETION_RESULT(ecr);
 	struct scatterlist src_sg, dst_sg;
-	struct crypto_skcipher *tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
+	struct crypto_skcipher *tfm;
+
+	tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
 
-	if (IS_ERR(tfm)) {
-		res = PTR_ERR(tfm);
-		tfm = NULL;
-		goto out;
-	}
 	crypto_skcipher_set_flags(tfm, CRYPTO_TFM_REQ_WEAK_KEY);
 	req = skcipher_request_alloc(tfm, GFP_NOFS);
 	if (!req) {
-		res = -ENOMEM;
+		err = -ENOMEM;
 		goto out;
 	}
 	skcipher_request_set_callback(req,
 			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
 			derive_crypt_complete, &ecr);
-	res = crypto_skcipher_setkey(tfm, deriving_key,
-					FS_AES_128_ECB_KEY_SIZE);
-	if (res < 0)
+
+	BUILD_BUG_ON(sizeof(ctx->nonce) != FS_AES_128_ECB_KEY_SIZE);
+	err = crypto_skcipher_setkey(tfm, ctx->nonce, sizeof(ctx->nonce));
+	if (err)
 		goto out;
 
-	sg_init_one(&src_sg, source_key->raw, source_key->size);
-	sg_init_one(&dst_sg, derived_raw_key, source_key->size);
-	skcipher_request_set_crypt(req, &src_sg, &dst_sg, source_key->size,
+	sg_init_one(&src_sg, master_key->raw, derived_keysize);
+	sg_init_one(&dst_sg, derived_key, derived_keysize);
+	skcipher_request_set_crypt(req, &src_sg, &dst_sg, derived_keysize,
 				   NULL);
-	res = crypto_skcipher_encrypt(req);
-	if (res == -EINPROGRESS || res == -EBUSY) {
+	err = crypto_skcipher_encrypt(req);
+	if (err == -EINPROGRESS || err == -EBUSY) {
 		wait_for_completion(&ecr.completion);
-		res = ecr.res;
+		err = ecr.res;
 	}
 out:
 	skcipher_request_free(req);
 	crypto_free_skcipher(tfm);
-	return res;
+	return err;
 }
 
-static int validate_user_key(struct fscrypt_info *crypt_info,
-			struct fscrypt_context *ctx, u8 *raw_key,
-			const char *prefix, int min_keysize)
+/*
+ * HKDF-based key derivation function.  This uses HKDF-SHA512 to derive a unique
+ * encryption key for each inode, using the inode's nonce prefixed with a
+ * context byte as the application-specific information string.  This is more
+ * flexible than the legacy AES-based KDF and has the advantage that it's
+ * non-reversible: an attacker who compromises a derived key cannot calculate
+ * the master key or any other derived keys.
+ */
+static int derive_key_hkdf(const struct fscrypt_master_key *master_key,
+			   const struct fscrypt_context *ctx,
+			   u8 *derived_key, unsigned int derived_keysize)
 {
-	char *description;
-	struct key *keyring_key;
-	struct fscrypt_key *master_key;
-	const struct user_key_payload *ukp;
-	int res;
+	return hkdf_expand(master_key->mk_hmac, HKDF_CONTEXT_PER_FILE_KEY,
+			   ctx->nonce, sizeof(ctx->nonce),
+			   derived_key, derived_keysize);
+}
 
-	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
-				FS_KEY_DESCRIPTOR_SIZE,
-				ctx->master_key_descriptor);
-	if (!description)
-		return -ENOMEM;
+static int find_and_derive_key_v1(const struct inode *inode,
+				  const struct fscrypt_context *ctx,
+				  u8 *derived_key, unsigned int derived_keysize)
+{
+	struct key *keyring_key;
+	const struct fscrypt_key *payload;
+	int err;
 
-	keyring_key = request_key(&key_type_logon, description, NULL);
-	kfree(description);
+	keyring_key = find_and_lock_keyring_key(FS_KEY_DESC_PREFIX,
+						ctx->master_key_descriptor,
+						derived_keysize, &payload);
+	if (keyring_key == ERR_PTR(-ENOKEY) && inode->i_sb->s_cop->key_prefix) {
+		keyring_key = find_and_lock_keyring_key(
+					inode->i_sb->s_cop->key_prefix,
+					ctx->master_key_descriptor,
+					derived_keysize, &payload);
+	}
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
-	down_read(&keyring_key->sem);
 
-	if (keyring_key->type != &key_type_logon) {
-		printk_once(KERN_WARNING
-				"%s: key type must be logon\n", __func__);
-		res = -ENOKEY;
-		goto out;
-	}
-	ukp = user_key_payload_locked(keyring_key);
-	if (ukp->datalen != sizeof(struct fscrypt_key)) {
-		res = -EINVAL;
-		goto out;
-	}
-	master_key = (struct fscrypt_key *)ukp->data;
-	BUILD_BUG_ON(FS_AES_128_ECB_KEY_SIZE != FS_KEY_DERIVATION_NONCE_SIZE);
-
-	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
-	    || master_key->size % AES_BLOCK_SIZE != 0) {
-		printk_once(KERN_WARNING
-				"%s: key size incorrect: %d\n",
-				__func__, master_key->size);
-		res = -ENOKEY;
-		goto out;
-	}
-	res = derive_key_aes(ctx->nonce, master_key, raw_key);
-out:
-	up_read(&keyring_key->sem);
-	key_put(keyring_key);
-	return res;
+	err = derive_key_aes(payload, ctx, derived_key, derived_keysize);
+
+	release_keyring_key(keyring_key);
+
+	return err;
 }
 
 static const struct {
@@ -179,6 +466,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
 
 	crypto_free_skcipher(ci->ci_ctfm);
 	crypto_free_cipher(ci->ci_essiv_tfm);
+	put_master_key(ci->ci_master_key);
 	kmem_cache_free(fscrypt_info_cachep, ci);
 }
 
@@ -254,8 +542,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	struct fscrypt_context ctx;
 	struct crypto_skcipher *ctfm;
 	const char *cipher_str;
-	int keysize;
-	u8 *raw_key = NULL;
+	unsigned int derived_keysize;
+	u8 *derived_key = NULL;
 	int res;
 
 	if (inode->i_crypt_info)
@@ -296,33 +584,40 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	memcpy(crypt_info->ci_master_key_descriptor, ctx.master_key_descriptor,
 	       FS_KEY_DESCRIPTOR_SIZE);
 
-	res = determine_cipher_type(crypt_info, inode, &cipher_str, &keysize);
+	res = determine_cipher_type(crypt_info, inode, &cipher_str,
+				    &derived_keysize);
 	if (res)
 		goto out;
 
 	/*
-	 * This cannot be a stack buffer because it is passed to the scatterlist
-	 * crypto API as part of key derivation.
+	 * This cannot be a stack buffer because it may be passed to the
+	 * scatterlist crypto API during key derivation.
 	 */
 	res = -ENOMEM;
-	raw_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
-	if (!raw_key)
+	derived_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
+	if (!derived_key)
 		goto out;
 
-	res = validate_user_key(crypt_info, &ctx, raw_key, FS_KEY_DESC_PREFIX,
-				keysize);
-	if (res && inode->i_sb->s_cop->key_prefix) {
-		int res2 = validate_user_key(crypt_info, &ctx, raw_key,
-					     inode->i_sb->s_cop->key_prefix,
-					     keysize);
-		if (res2) {
-			if (res2 == -ENOKEY)
-				res = -ENOKEY;
+	if (ctx.version == FSCRYPT_CONTEXT_V1) {
+		res = find_and_derive_key_v1(inode, &ctx, derived_key,
+					     derived_keysize);
+	} else {
+		crypt_info->ci_master_key =
+			load_master_key_from_keyring(inode,
+						     ctx.master_key_descriptor,
+						     derived_keysize);
+		if (IS_ERR(crypt_info->ci_master_key)) {
+			res = PTR_ERR(crypt_info->ci_master_key);
+			crypt_info->ci_master_key = NULL;
 			goto out;
 		}
-	} else if (res) {
-		goto out;
+
+		res = derive_key_hkdf(crypt_info->ci_master_key, &ctx,
+				      derived_key, derived_keysize);
 	}
+	if (res)
+		goto out;
+
 	ctfm = crypto_alloc_skcipher(cipher_str, 0, 0);
 	if (!ctfm || IS_ERR(ctfm)) {
 		res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
@@ -333,17 +628,14 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	crypt_info->ci_ctfm = ctfm;
 	crypto_skcipher_clear_flags(ctfm, ~0);
 	crypto_skcipher_set_flags(ctfm, CRYPTO_TFM_REQ_WEAK_KEY);
-	/*
-	 * if the provided key is longer than keysize, we use the first
-	 * keysize bytes of the derived key only
-	 */
-	res = crypto_skcipher_setkey(ctfm, raw_key, keysize);
+	res = crypto_skcipher_setkey(ctfm, derived_key, derived_keysize);
 	if (res)
 		goto out;
 
 	if (S_ISREG(inode->i_mode) &&
 	    crypt_info->ci_data_mode == FS_ENCRYPTION_MODE_AES_128_CBC) {
-		res = init_essiv_generator(crypt_info, raw_key, keysize);
+		res = init_essiv_generator(crypt_info, derived_key,
+					   derived_keysize);
 		if (res) {
 			pr_debug("%s: error %d (inode %lu) allocating essiv tfm\n",
 				 __func__, res, inode->i_ino);
@@ -356,7 +648,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	if (res == -ENOKEY)
 		res = 0;
 	put_crypt_info(crypt_info);
-	kzfree(raw_key);
+	kzfree(derived_key);
 	return res;
 }
 EXPORT_SYMBOL(fscrypt_get_encryption_info);
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH v2 4/7] fscrypt: validate modes and flags earlier when setting policy

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

For FS_IOC_SET_ENCRYPTION_POLICY, currently the encryption modes and
flags are only validated when a new encryption policy is being set, not
when an existing policy is being compared to the one specified.
However, we're going to start needing to compute the key_hash in both
cases, and for this it's helpful to validate that the master key has the
minimum length required by the specified encryption modes.  Therefore,
move the modes and flags validation earlier in the ioctl, next to where
we validate the policy version.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/policy.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index fe525da9e79c..d1e58798da3c 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -60,13 +60,6 @@ static int create_encryption_context_from_policy(struct inode *inode,
 {
 	struct fscrypt_context ctx;
 
-	if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
-				     policy->filenames_encryption_mode))
-		return -EINVAL;
-
-	if (policy->flags & ~FS_POLICY_FLAGS_VALID)
-		return -EINVAL;
-
 	ctx.version = context_version_for_policy(policy);
 	ctx.contents_encryption_mode = policy->contents_encryption_mode;
 	ctx.filenames_encryption_mode = policy->filenames_encryption_mode;
@@ -100,6 +93,13 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 	    policy.version != FS_POLICY_VERSION_HKDF)
 		return -EINVAL;
 
+	if (!fscrypt_valid_enc_modes(policy.contents_encryption_mode,
+				     policy.filenames_encryption_mode))
+		return -EINVAL;
+
+	if (policy.flags & ~FS_POLICY_FLAGS_VALID)
+		return -EINVAL;
+
 	ret = mnt_want_write_file(filp);
 	if (ret)
 		return ret;
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH v2 5/7] fscrypt: verify that the correct master key was supplied

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Currently, while a fscrypt master key is required to have a certain
description in the keyring, its payload is never verified to be correct.
While sufficient for well-behaved userspace, this is insecure in a
multi-user system where a user has been given only read-only access to
an encrypted file or directory.  Specifically, if an encrypted file or
directory does not yet have its key cached by the kernel, the first user
who accesses it can provide an arbitrary key in their own keyring, which
the kernel will then associate with the inode and use for read(),
write(), readdir(), etc. by other users as well.

Consequently, it's trivial for a user with *read-only* access to an
encrypted file or directory to make it appear as garbage to everyone.
Creative users might be able to accomplish more sophisticated attacks by
careful choice of the key, e.g. choosing a key causes certain bytes of
file contents to have certain values or that causes filenames containing
the '/' character to appear.

Solve the problem for v2 encryption policies by storing a "hash" of the
master encryption key in the encryption xattr and verifying it before
accepting the user-provided key.  We generate the "hash" using
HKDF-SHA512 by passing a distinct application-specific info string.
This produces a value which is cryptographically isolated and can be
stored in the clear without leaking any information about the master key
or any other derived keys (in a computational sense).  Reusing HKDF is
better than doing e.g. SHA-512(master_key) because it avoids passing the
same key into different cryptographic primitives.

We make the hash field 16 bytes long, as this should provide sufficient
collision and preimage resistance while not wasting too much space for
the encryption xattr.

Acked-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  4 ++++
 fs/crypto/keyinfo.c         | 46 +++++++++++++++++++++++++++++++++++++
 fs/crypto/policy.c          | 55 ++++++++++++++++++++++++++++++++++++---------
 3 files changed, 95 insertions(+), 10 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index b5881cb26e8f..f9469a71cc29 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -94,6 +94,7 @@ fscrypt_valid_context_format(const struct fscrypt_context *ctx, int size)
  */
 struct fscrypt_master_key {
 	struct crypto_shash	*mk_hmac;
+	u8			mk_hash[FSCRYPT_KEY_HASH_SIZE];
 };
 
 /*
@@ -157,6 +158,9 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
 					      gfp_t gfp_flags);
 
 /* keyinfo.c */
+extern int fscrypt_compute_key_hash(const struct inode *inode,
+				    const struct fscrypt_policy *policy,
+				    u8 hash[FSCRYPT_KEY_HASH_SIZE]);
 extern void __exit fscrypt_essiv_cleanup(void);
 
 #endif /* _FSCRYPT_PRIVATE_H */
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 3164e1c74196..a9514078cf9d 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -39,8 +39,11 @@ static struct crypto_shash *essiv_hash_tfm;
  *
  * Keys derived with different info strings are cryptographically isolated from
  * each other --- knowledge of one derived key doesn't reveal any others.
+ * (This property is particularly important for the derived key used as the
+ * "key hash", as that is stored in the clear.)
  */
 #define HKDF_CONTEXT_PER_FILE_KEY	1
+#define HKDF_CONTEXT_KEY_HASH		2
 
 /*
  * HKDF consists of two steps:
@@ -211,6 +214,12 @@ alloc_master_key(const struct fscrypt_key *payload)
 	err = crypto_shash_setkey(k->mk_hmac, prk, sizeof(prk));
 	if (err)
 		goto fail;
+
+	/* Calculate the "key hash" */
+	err = hkdf_expand(k->mk_hmac, HKDF_CONTEXT_KEY_HASH, NULL, 0,
+			  k->mk_hash, FSCRYPT_KEY_HASH_SIZE);
+	if (err)
+		goto fail;
 out:
 	memzero_explicit(prk, sizeof(prk));
 	return k;
@@ -536,6 +545,31 @@ void __exit fscrypt_essiv_cleanup(void)
 	crypto_free_shash(essiv_hash_tfm);
 }
 
+int fscrypt_compute_key_hash(const struct inode *inode,
+			     const struct fscrypt_policy *policy,
+			     u8 hash[FSCRYPT_KEY_HASH_SIZE])
+{
+	struct fscrypt_master_key *k;
+	unsigned int min_keysize;
+
+	/*
+	 * Require that the master key be long enough for both the
+	 * contents and filenames encryption modes.
+	 */
+	min_keysize =
+		max(available_modes[policy->contents_encryption_mode].keysize,
+		    available_modes[policy->filenames_encryption_mode].keysize);
+
+	k = load_master_key_from_keyring(inode, policy->master_key_descriptor,
+					 min_keysize);
+	if (IS_ERR(k))
+		return PTR_ERR(k);
+
+	memcpy(hash, k->mk_hash, FSCRYPT_KEY_HASH_SIZE);
+	put_master_key(k);
+	return 0;
+}
+
 int fscrypt_get_encryption_info(struct inode *inode)
 {
 	struct fscrypt_info *crypt_info;
@@ -612,6 +646,18 @@ int fscrypt_get_encryption_info(struct inode *inode)
 			goto out;
 		}
 
+		/*
+		 * Make sure the master key we found has the correct hash.
+		 * Buggy or malicious userspace may provide the wrong key.
+		 */
+		if (memcmp(crypt_info->ci_master_key->mk_hash, ctx.key_hash,
+			   FSCRYPT_KEY_HASH_SIZE)) {
+			pr_warn_ratelimited("fscrypt: wrong encryption key supplied for inode %lu\n",
+					    inode->i_ino);
+			res = -ENOKEY;
+			goto out;
+		}
+
 		res = derive_key_hkdf(crypt_info->ci_master_key, &ctx,
 				      derived_key, derived_keysize);
 	}
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index d1e58798da3c..c0a047559736 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -42,7 +42,8 @@ static u8 context_version_for_policy(const struct fscrypt_policy *policy)
  */
 static bool is_encryption_context_consistent_with_policy(
 				const struct fscrypt_context *ctx,
-				const struct fscrypt_policy *policy)
+				const struct fscrypt_policy *policy,
+				const u8 key_hash[FSCRYPT_KEY_HASH_SIZE])
 {
 	return (ctx->version == context_version_for_policy(policy)) &&
 		(memcmp(ctx->master_key_descriptor,
@@ -52,11 +53,14 @@ static bool is_encryption_context_consistent_with_policy(
 		(ctx->contents_encryption_mode ==
 		 policy->contents_encryption_mode) &&
 		(ctx->filenames_encryption_mode ==
-		 policy->filenames_encryption_mode);
+		 policy->filenames_encryption_mode) &&
+		(ctx->version == FSCRYPT_CONTEXT_V1 ||
+		 (memcmp(ctx->key_hash, key_hash, FSCRYPT_KEY_HASH_SIZE) == 0));
 }
 
 static int create_encryption_context_from_policy(struct inode *inode,
-				const struct fscrypt_policy *policy)
+				const struct fscrypt_policy *policy,
+				const u8 key_hash[FSCRYPT_KEY_HASH_SIZE])
 {
 	struct fscrypt_context ctx;
 
@@ -69,7 +73,7 @@ static int create_encryption_context_from_policy(struct inode *inode,
 	BUILD_BUG_ON(sizeof(ctx.nonce) != FS_KEY_DERIVATION_NONCE_SIZE);
 	get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
 	if (ctx.version != FSCRYPT_CONTEXT_V1)
-		memset(ctx.key_hash, 0, FSCRYPT_KEY_HASH_SIZE);
+		memcpy(ctx.key_hash, key_hash, FSCRYPT_KEY_HASH_SIZE);
 
 	return inode->i_sb->s_cop->set_context(inode, &ctx,
 					       fscrypt_context_size(&ctx),
@@ -82,6 +86,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 	struct inode *inode = file_inode(filp);
 	int ret;
 	struct fscrypt_context ctx;
+	u8 key_hash[FSCRYPT_KEY_HASH_SIZE];
 
 	if (copy_from_user(&policy, arg, sizeof(policy)))
 		return -EFAULT;
@@ -100,6 +105,25 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 	if (policy.flags & ~FS_POLICY_FLAGS_VALID)
 		return -EINVAL;
 
+	if (policy.version == FS_POLICY_VERSION_ORIGINAL) {
+		/*
+		 * Originally no key verification was implemented, which was
+		 * insufficient for scenarios where multiple users share
+		 * encrypted files.  The new encryption policy version fixes
+		 * this and also implements an improved key derivation function.
+		 * So as long as the key can be in the keyring at the time the
+		 * policy is set and compatibility with old kernels isn't
+		 * required, it's recommended to use the new policy version
+		 * (fscrypt_policy.version = 2).
+		 */
+		pr_warn_once("%s (pid %d) is setting less secure v0 encryption policy; recommend upgrading to v2.\n",
+			     current->comm, current->pid);
+	} else {
+		ret = fscrypt_compute_key_hash(inode, &policy, key_hash);
+		if (ret)
+			return ret;
+	}
+
 	ret = mnt_want_write_file(filp);
 	if (ret)
 		return ret;
@@ -114,10 +138,12 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 			ret = -ENOTEMPTY;
 		else
 			ret = create_encryption_context_from_policy(inode,
-								    &policy);
+								    &policy,
+								    key_hash);
 	} else if (ret >= 0 && fscrypt_valid_context_format(&ctx, ret) &&
 		   is_encryption_context_consistent_with_policy(&ctx,
-								&policy)) {
+								&policy,
+								key_hash)) {
 		/* The file already uses the same encryption policy. */
 		ret = 0;
 	} else if (ret >= 0 || ret == -ERANGE) {
@@ -234,7 +260,11 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 			(parent_ci->ci_data_mode == child_ci->ci_data_mode) &&
 			(parent_ci->ci_filename_mode ==
 			 child_ci->ci_filename_mode) &&
-			(parent_ci->ci_flags == child_ci->ci_flags);
+			(parent_ci->ci_flags == child_ci->ci_flags) &&
+			(parent_ci->ci_context_version == FSCRYPT_CONTEXT_V1 ||
+			 (memcmp(parent_ci->ci_master_key->mk_hash,
+				 child_ci->ci_master_key->mk_hash,
+				 FSCRYPT_KEY_HASH_SIZE) == 0));
 	}
 
 	res = cops->get_context(parent, &parent_ctx, sizeof(parent_ctx));
@@ -253,7 +283,10 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 		 child_ctx.contents_encryption_mode) &&
 		(parent_ctx.filenames_encryption_mode ==
 		 child_ctx.filenames_encryption_mode) &&
-		(parent_ctx.flags == child_ctx.flags);
+		(parent_ctx.flags == child_ctx.flags) &&
+		(parent_ctx.version == FSCRYPT_CONTEXT_V1 ||
+		 (memcmp(parent_ctx.key_hash, child_ctx.key_hash,
+			 FSCRYPT_KEY_HASH_SIZE) == 0));
 }
 EXPORT_SYMBOL(fscrypt_has_permitted_context);
 
@@ -288,8 +321,10 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 	memcpy(ctx.master_key_descriptor, ci->ci_master_key_descriptor,
 	       FS_KEY_DESCRIPTOR_SIZE);
 	get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
-	if (ctx.version != FSCRYPT_CONTEXT_V1)
-		memset(ctx.key_hash, 0, FSCRYPT_KEY_HASH_SIZE);
+	if (ctx.version != FSCRYPT_CONTEXT_V1) {
+		memcpy(ctx.key_hash, ci->ci_master_key->mk_hash,
+		       FSCRYPT_KEY_HASH_SIZE);
+	}
 
 	BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
 	res = parent->i_sb->s_cop->set_context(child, &ctx,
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH v2 6/7] fscrypt: cache the HMAC transform for each master key

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Now that we have a key_hash field which securely identifies a master key
payload, introduce a cache of the HMAC transforms for the master keys
currently in use for inodes using v2+ encryption policies.  The entries
in this cache are called 'struct fscrypt_master_key' and are identified
by key_hash.  The cache is per-superblock.  (It could be global, but
making it per-superblock should reduce the lock contention a bit, and we
may need to keep track of keys on a per-superblock basis for other
reasons later, e.g. to implement an ioctl for evicting keys.)

This results in a large efficiency gain: we now only have to allocate
and key an "hmac(sha512)" transformation, execute HKDF-Extract, and
compute key_hash once per master key rather than once per inode.  Note
that this optimization can't easily be applied to the original AES-based
KDF because that uses a different AES key for each KDF execution.  In
practice, this difference makes the HKDF per-inode encryption key
derivation performance comparable to or even faster than the old KDF,
which typically spends more time allocating an "ecb(aes)" transformation
from the crypto API than doing actual crypto work.

Note that it would have been possible to make the mapping be from
raw_key => fscrypt_master_key (where raw_key denotes the actual bytes of
the master key) rather than from key_hash => fscrypt_master_key.
However, an advantage of doing lookups by key_hash is that it replaces
the keyring lookup in most cases, which opens up the future
possibilities of not even having the master key in memory following an
initial provisioning step (if the HMAC-SHA512 implementation is
hardware-backed), or of introducing an ioctl to provision a key to the
filesystem directly, avoiding keyrings and their visibility problems
entirely.  Also, because key_hash is public information while raw_key is
secret information, it would have been very difficult to use raw_key as
a map key in a way that would prevent timing attacks while still being
scalable to a large number of entries.

Reviewed-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  11 ++++
 fs/crypto/keyinfo.c         | 134 +++++++++++++++++++++++++++++++++++++++++++-
 fs/crypto/policy.c          |   5 +-
 fs/super.c                  |   4 ++
 include/linux/fs.h          |   5 ++
 5 files changed, 152 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index f9469a71cc29..4221c5b23882 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -91,10 +91,21 @@ fscrypt_valid_context_format(const struct fscrypt_context *ctx, int size)
 
 /*
  * fscrypt_master_key - an in-use master key
+ *
+ * This is referenced from each in-core inode that has been "unlocked" using a
+ * particular master key.  It's primarily used to cache the HMAC transform so
+ * that the per-inode encryption keys can be derived efficiently with HKDF.  It
+ * is securely erased once all inodes referencing it have been evicted.
+ *
+ * If the same master key is used on different filesystems (unusual, but
+ * possible), we'll create one of these structs for each filesystem.
  */
 struct fscrypt_master_key {
 	struct crypto_shash	*mk_hmac;
 	u8			mk_hash[FSCRYPT_KEY_HASH_SIZE];
+	refcount_t		mk_refcount;
+	struct rb_node		mk_node;
+	struct super_block	*mk_sb;
 };
 
 /*
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index a9514078cf9d..2fca72826768 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -176,6 +176,14 @@ static void put_master_key(struct fscrypt_master_key *k)
 	if (!k)
 		return;
 
+	if (refcount_read(&k->mk_refcount) != 0) { /* in ->s_master_keys? */
+		if (!refcount_dec_and_lock(&k->mk_refcount,
+					   &k->mk_sb->s_master_keys_lock))
+			return;
+		rb_erase(&k->mk_node, &k->mk_sb->s_master_keys);
+		spin_unlock(&k->mk_sb->s_master_keys_lock);
+	}
+
 	crypto_free_shash(k->mk_hmac);
 	kzfree(k);
 }
@@ -230,6 +238,87 @@ alloc_master_key(const struct fscrypt_key *payload)
 	goto out;
 }
 
+/*
+ * ->s_master_keys is a map of master keys currently in use by in-core inodes on
+ * a given filesystem, identified by key_hash which is a cryptographically
+ * secure identifier for an actual key payload.
+ *
+ * Note that master_key_descriptor cannot be used to identify the keys because
+ * master_key_descriptor only identifies the "location" of a key in the keyring,
+ * not the actual key payload --- i.e., buggy or malicious userspace may provide
+ * different keys with the same master_key_descriptor.
+ */
+
+/*
+ * Search ->s_master_keys for the fscrypt_master_key having the specified hash.
+ * If found return it with a reference taken, otherwise return NULL.
+ */
+static struct fscrypt_master_key *
+get_cached_master_key(struct super_block *sb,
+		      const u8 hash[FSCRYPT_KEY_HASH_SIZE])
+{
+	struct rb_node *node;
+	struct fscrypt_master_key *k;
+	int res;
+
+	spin_lock(&sb->s_master_keys_lock);
+	node = sb->s_master_keys.rb_node;
+	while (node) {
+		k = rb_entry(node, struct fscrypt_master_key, mk_node);
+		res = memcmp(hash, k->mk_hash, FSCRYPT_KEY_HASH_SIZE);
+		if (res < 0)
+			node = node->rb_left;
+		else if (res > 0)
+			node = node->rb_right;
+		else {
+			refcount_inc(&k->mk_refcount);
+			goto out;
+		}
+	}
+	k = NULL;
+out:
+	spin_unlock(&sb->s_master_keys_lock);
+	return k;
+}
+
+/*
+ * Try to insert the specified fscrypt_master_key into ->s_master_keys.  If it
+ * already exists, then drop the key being inserted and take a reference to the
+ * existing one instead.
+ */
+static struct fscrypt_master_key *
+insert_master_key(struct super_block *sb, struct fscrypt_master_key *new)
+{
+	struct fscrypt_master_key *k;
+	struct rb_node *parent = NULL, **p;
+	int res;
+
+	spin_lock(&sb->s_master_keys_lock);
+	p = &sb->s_master_keys.rb_node;
+	while (*p) {
+		parent = *p;
+		k = rb_entry(parent, struct fscrypt_master_key, mk_node);
+		res = memcmp(new->mk_hash, k->mk_hash, FSCRYPT_KEY_HASH_SIZE);
+		if (res < 0)
+			p = &parent->rb_left;
+		else if (res > 0)
+			p = &parent->rb_right;
+		else {
+			refcount_inc(&k->mk_refcount);
+			spin_unlock(&sb->s_master_keys_lock);
+			put_master_key(new);
+			return k;
+		}
+	}
+
+	rb_link_node(&new->mk_node, parent, p);
+	rb_insert_color(&new->mk_node, &sb->s_master_keys);
+	refcount_set(&new->mk_refcount, 1);
+	new->mk_sb = sb;
+	spin_unlock(&sb->s_master_keys_lock);
+	return new;
+}
+
 static void release_keyring_key(struct key *keyring_key)
 {
 	up_read(&keyring_key->sem);
@@ -320,6 +409,47 @@ load_master_key_from_keyring(const struct inode *inode,
 	return master_key;
 }
 
+/*
+ * Get the fscrypt_master_key identified by the specified v2+ encryption
+ * context, or create it if not found.
+ *
+ * Returns the fscrypt_master_key with a reference taken, or an ERR_PTR().
+ */
+static struct fscrypt_master_key *
+find_or_create_master_key(const struct inode *inode,
+			  const struct fscrypt_context *ctx,
+			  unsigned int min_keysize)
+{
+	struct fscrypt_master_key *master_key;
+
+	if (WARN_ON(ctx->version < FSCRYPT_CONTEXT_V2))
+		return ERR_PTR(-EINVAL);
+
+	/*
+	 * First try looking up the master key by its cryptographically secure
+	 * key_hash.  If it's already in memory, there's no need to do a keyring
+	 * search.  (Note that we don't enforce access control based on which
+	 * processes "have the key" and which don't, as encryption is meant to
+	 * be orthogonal to operating-system level access control.  Hence, it's
+	 * sufficient for anyone on the system to have added the needed key.)
+	 */
+	master_key = get_cached_master_key(inode->i_sb, ctx->key_hash);
+	if (master_key)
+		return master_key;
+
+	/*
+	 * The needed master key isn't in memory yet.  Load it from the keyring.
+	 */
+	master_key = load_master_key_from_keyring(inode,
+						  ctx->master_key_descriptor,
+						  min_keysize);
+	if (IS_ERR(master_key))
+		return master_key;
+
+	/* Cache the key for later */
+	return insert_master_key(inode->i_sb, master_key);
+}
+
 static void derive_crypt_complete(struct crypto_async_request *req, int rc)
 {
 	struct fscrypt_completion_result *ecr = req->data;
@@ -637,9 +767,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
 					     derived_keysize);
 	} else {
 		crypt_info->ci_master_key =
-			load_master_key_from_keyring(inode,
-						     ctx.master_key_descriptor,
-						     derived_keysize);
+			find_or_create_master_key(inode, &ctx, derived_keysize);
 		if (IS_ERR(crypt_info->ci_master_key)) {
 			res = PTR_ERR(crypt_info->ci_master_key);
 			crypt_info->ci_master_key = NULL;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index c0a047559736..ece55350cee8 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -261,10 +261,7 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 			(parent_ci->ci_filename_mode ==
 			 child_ci->ci_filename_mode) &&
 			(parent_ci->ci_flags == child_ci->ci_flags) &&
-			(parent_ci->ci_context_version == FSCRYPT_CONTEXT_V1 ||
-			 (memcmp(parent_ci->ci_master_key->mk_hash,
-				 child_ci->ci_master_key->mk_hash,
-				 FSCRYPT_KEY_HASH_SIZE) == 0));
+			(parent_ci->ci_master_key == child_ci->ci_master_key);
 	}
 
 	res = cops->get_context(parent, &parent_ctx, sizeof(parent_ctx));
diff --git a/fs/super.c b/fs/super.c
index 6bc3352adcf3..de9be131b5ee 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -214,6 +214,10 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
 	spin_lock_init(&s->s_inode_list_lock);
 	INIT_LIST_HEAD(&s->s_inodes_wb);
 	spin_lock_init(&s->s_inode_wblist_lock);
+#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
+	s->s_master_keys = RB_ROOT;
+	spin_lock_init(&s->s_master_keys_lock);
+#endif
 
 	if (list_lru_init_memcg(&s->s_dentry_lru))
 		goto fail;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6e1fd5d21248..e6e494e6ef10 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1413,6 +1413,11 @@ struct super_block {
 
 	spinlock_t		s_inode_wblist_lock;
 	struct list_head	s_inodes_wb;	/* writeback inodes */
+
+#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
+	spinlock_t		s_master_keys_lock;
+	struct rb_root		s_master_keys;	/* master crypto keys in use */
+#endif
 } __randomize_layout;
 
 /* Helper functions so that in most cases filesystems will
-- 
2.14.0.rc0.400.g1c36432dff-goog

[PATCH v2 7/7] fscrypt: for v2 policies, support "fscrypt:" key prefix only

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Since v2 encryption policies are opt-in, take the opportunity to also
drop support for the legacy filesystem-specific key description prefixes
"ext4:", "f2fs:", and "ubifs:", instead requiring the generic prefix
"fscrypt:".  The generic prefix is preferred since it works for all
filesystems.  Also there is a performance benefit from not having to
search the keyrings twice.

The old prefixes remain supported for v1 encryption policies.

Reviewed-by: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  3 +--
 fs/crypto/keyinfo.c         | 16 ++++------------
 fs/crypto/policy.c          |  2 +-
 3 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4221c5b23882..c9de15d4b5b0 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -169,8 +169,7 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
 					      gfp_t gfp_flags);
 
 /* keyinfo.c */
-extern int fscrypt_compute_key_hash(const struct inode *inode,
-				    const struct fscrypt_policy *policy,
+extern int fscrypt_compute_key_hash(const struct fscrypt_policy *policy,
 				    u8 hash[FSCRYPT_KEY_HASH_SIZE]);
 extern void __exit fscrypt_essiv_cleanup(void);
 
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 2fca72826768..81b08d4a7efe 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -384,8 +384,7 @@ find_and_lock_keyring_key(const char *prefix,
 }
 
 static struct fscrypt_master_key *
-load_master_key_from_keyring(const struct inode *inode,
-			     const u8 descriptor[FS_KEY_DESCRIPTOR_SIZE],
+load_master_key_from_keyring(const u8 descriptor[FS_KEY_DESCRIPTOR_SIZE],
 			     unsigned int min_keysize)
 {
 	struct key *keyring_key;
@@ -394,11 +393,6 @@ load_master_key_from_keyring(const struct inode *inode,
 
 	keyring_key = find_and_lock_keyring_key(FS_KEY_DESC_PREFIX, descriptor,
 						min_keysize, &payload);
-	if (keyring_key == ERR_PTR(-ENOKEY) && inode->i_sb->s_cop->key_prefix) {
-		keyring_key = find_and_lock_keyring_key(
-					inode->i_sb->s_cop->key_prefix,
-					descriptor, min_keysize, &payload);
-	}
 	if (IS_ERR(keyring_key))
 		return ERR_CAST(keyring_key);
 
@@ -440,8 +434,7 @@ find_or_create_master_key(const struct inode *inode,
 	/*
 	 * The needed master key isn't in memory yet.  Load it from the keyring.
 	 */
-	master_key = load_master_key_from_keyring(inode,
-						  ctx->master_key_descriptor,
+	master_key = load_master_key_from_keyring(ctx->master_key_descriptor,
 						  min_keysize);
 	if (IS_ERR(master_key))
 		return master_key;
@@ -675,8 +668,7 @@ void __exit fscrypt_essiv_cleanup(void)
 	crypto_free_shash(essiv_hash_tfm);
 }
 
-int fscrypt_compute_key_hash(const struct inode *inode,
-			     const struct fscrypt_policy *policy,
+int fscrypt_compute_key_hash(const struct fscrypt_policy *policy,
 			     u8 hash[FSCRYPT_KEY_HASH_SIZE])
 {
 	struct fscrypt_master_key *k;
@@ -690,7 +682,7 @@ int fscrypt_compute_key_hash(const struct inode *inode,
 		max(available_modes[policy->contents_encryption_mode].keysize,
 		    available_modes[policy->filenames_encryption_mode].keysize);
 
-	k = load_master_key_from_keyring(inode, policy->master_key_descriptor,
+	k = load_master_key_from_keyring(policy->master_key_descriptor,
 					 min_keysize);
 	if (IS_ERR(k))
 		return PTR_ERR(k);
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index ece55350cee8..fa5599005a72 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -119,7 +119,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 		pr_warn_once("%s (pid %d) is setting less secure v0 encryption policy; recommend upgrading to v2.\n",
 			     current->comm, current->pid);
 	} else {
-		ret = fscrypt_compute_key_hash(inode, &policy, key_hash);
+		ret = fscrypt_compute_key_hash(&policy, key_hash);
 		if (ret)
 			return ret;
 	}
-- 
2.14.0.rc0.400.g1c36432dff-goog