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

[Eric Biggers]

From: Eric Biggers 

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.

Signed-off-by: Eric Biggers 
---
 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 a7baeac92575..4b158717a8c3 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -88,11 +88,22 @@ 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;
unsigned intmk_size;
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 12a60eacf819..bf60e76f9599 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(>mk_refcount) != 0) { /* in ->s_master_keys? */
+   if (!refcount_dec_and_lock(>mk_refcount,
+  >mk_sb->s_master_keys_lock))
+   return;
+   rb_erase(>mk_node, >mk_sb->s_master_keys);
+   spin_unlock(>mk_sb->s_master_keys_lock);
+   }
+
crypto_free_shash(k->mk_hmac);
kzfree(k);
 }
@@ -231,6 +239,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.
+ 

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

[Eric Biggers]

From: Eric Biggers 

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.

Signed-off-by: Eric Biggers 
---
 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 ef6909035823..5470aac82cab 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -107,7 +107,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, _str, );
if (res)
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 044f23fadb5a..81c59f8e45c0 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -224,7 +224,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) &&
@@ -282,7 +283,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.13.2.932.g7449e964c-goog

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

[Eric Biggers]

From: Eric Biggers 

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 
---
 fs/crypto/Kconfig   |   2 +
 fs/crypto/fscrypt_private.h |  14 ++
 fs/crypto/keyinfo.c | 485 +++-
 3 files changed, 405 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 5470aac82cab..095e7c16483a 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -86,6 +86,14 @@ 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;
+   unsigned intmk_size;
+};
+
 /*
  * fscrypt_info - the "encryption key" for an inode
  *
@@ -99,6 +107,12 @@ struct fscrypt_info {
struct crypto_skcipher *ci_ctfm;
struct crypto_cipher *ci_essiv_tfm;

[PATCH 4/6] fscrypt: verify that the correct master key was supplied

[Eric Biggers]

From: Eric Biggers 

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.

Signed-off-by: Eric Biggers 
---
 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 095e7c16483a..a7baeac92575 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -92,6 +92,7 @@ fscrypt_valid_context_format(const struct fscrypt_context 
*ctx, int size)
 struct fscrypt_master_key {
struct crypto_shash *mk_hmac;
unsigned intmk_size;
+   u8  mk_hash[FSCRYPT_KEY_HASH_SIZE];
 };
 
 /*
@@ -155,6 +156,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 7ed1a7fb1308..12a60eacf819 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:
@@ -212,6 +215,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;
@@ -537,6 +546,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))
+   

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

[Eric Biggers]

From: Eric Biggers 

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.

Signed-off-by: Eric Biggers 
---
 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 4b158717a8c3..201906ff7033 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -167,8 +167,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 bf60e76f9599..e20b5e85c1b3 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -385,8 +385,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;
@@ -395,11 +394,6 @@ load_master_key_from_keyring(const struct inode *inode,
 
keyring_key = find_and_lock_keyring_key(FS_KEY_DESC_PREFIX, descriptor,
min_keysize, );
-   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, );
-   }
if (IS_ERR(keyring_key))
return ERR_CAST(keyring_key);
 
@@ -441,8 +435,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;
@@ -676,8 +669,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;
@@ -691,7 +683,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 7661c66a3533..cd8c9c7cc9a9 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -117,7 +117,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, , key_hash);
+   ret = fscrypt_compute_key_hash(, key_hash);
if (ret)
return ret;
}
-- 
2.13.2.932.g7449e964c-goog

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

[Eric Biggers]

From: Eric Biggers 

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.

Signed-off-by: Eric Biggers 
---
 fs/crypto/fscrypt_private.h| 79 ++
 fs/crypto/keyinfo.c| 14 
 fs/crypto/policy.c | 67 ++-
 include/linux/fscrypt_common.h |  2 +-
 include/uapi/linux/fs.h|  6 
 5 files changed, 127 insertions(+), 41 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a1d5021c31ef..ef6909035823 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -25,39 +25,88 @@
 #define FS_AES_256_XTS_KEY_SIZE64
 
 #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.
  */
 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_V11
+   /* v2+ */
+
+   /* Cryptographically secure hash of the master key */
+   u8 key_hash[FSCRYPT_KEY_HASH_SIZE];
+};
+
+#define FSCRYPT_CONTEXT_V1 1
+#define FSCRYPT_CONTEXT_V1_SIZEoffsetof(struct fscrypt_context, 
key_hash)
+
+#define FSCRYPT_CONTEXT_V2 2
+#define FSCRYPT_CONTEXT_V2_SIZEsizeof(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.
+ * 

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

[Eric Biggers]

From: Eric Biggers 

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.

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.

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 (6):
  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: 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| 109 ++-
 fs/crypto/keyinfo.c| 669 ++---
 fs/crypto/policy.c | 118 ++--
 fs/super.c |   4 +
 include/linux/fs.h |   5 +
 include/linux/fscrypt_common.h |   2 +-
 include/uapi/linux/fs.h|   6 +
 8 files changed, 766 insertions(+), 149 deletions(-)

-- 
2.13.2.932.g7449e964c-goog

Re: [PATCH v3 net-next 3/4] tls: kernel TLS support

[Dave Watson]

On 07/12/17 09:20 AM, Steffen Klassert wrote:
> On Tue, Jul 11, 2017 at 11:53:11AM -0700, Dave Watson wrote:
> > On 07/11/17 08:29 AM, Steffen Klassert wrote:
> > > Sorry for replying to old mail...
> > > > +int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
> > > > +{
> > > 
> > > ...
> > > 
> > > > +
> > > > +   if (!sw_ctx->aead_send) {
> > > > +   sw_ctx->aead_send = crypto_alloc_aead("gcm(aes)", 0, 0);
> > > > +   if (IS_ERR(sw_ctx->aead_send)) {
> > > > +   rc = PTR_ERR(sw_ctx->aead_send);
> > > > +   sw_ctx->aead_send = NULL;
> > > > +   goto free_rec_seq;
> > > > +   }
> > > > +   }
> > > > +
> > > 
> > > When I look on how you allocate the aead transformation, it seems
> > > that you should either register an asynchronous callback with
> > > aead_request_set_callback(), or request for a synchronous algorithm.
> > > 
> > > Otherwise you will crash on an asynchronous crypto return, no?
> > 
> > The intention is for it to be synchronous, and gather directly from
> > userspace buffers.  It looks like calling
> > crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC) is the correct way
> > to request synchronous algorithms only?
> 
> Yes, but then you loose the aes-ni based algorithms because they are
> asynchronous. If you want to have good crypto performance, it is
> better to implement the asynchronous callbacks.

Right, the trick is we want both aesni, and to guarantee that we are
done using the input buffers before sendmsg() returns.  For now I can
set a callback, and wait on a completion.  The initial use case of
userspace openssl integration shouldn't hit the aesni async case
anyway (!irq_fpu_usable())

Re: Decreasing time for `rsa_init`

[Paul Menzel]

Dear Stephan,


Thank you for the quick response.


On 07/12/17 19:28, Stephan Müller wrote:

Am Mittwoch, 12. Juli 2017, 12:59:58 CEST schrieb Paul Menzel:



Building CRYPTO_RSA not as module, but into the Linux kernel,
`rsa_init()` takes 130 ms on an ASRock E350M1.

(Timings are shown by adding `initcall_debug` to Linux command line [1].
The times are visualized by `analyze_boot.py` from pm-graph [2] or
`systemd-bootchart`.)

This is quite a lot of time compared to other modules, and I wonder if
there are ways to decrease that time other than building it as a module,
and not signing modules?


Is the testmgr compiled? If yes, the self test may take that time.


It looks like it is, as the tests are not disabled.

```
$ grep MANAGER_DISABLE_TESTS .config
# CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
```

I’ll try an image without the tests, and will report back.


Kind regards,

Paul



[1] http://elinux.org/Initcall_Debug
[2] https://github.com/01org/pm-graph

Re: Decreasing time for `rsa_init`

[Stephan Müller]

Am Mittwoch, 12. Juli 2017, 12:59:58 CEST schrieb Paul Menzel:

Hi Paul,

> Dear Linux crypto folks,
> 
> 
> Building CRYPTO_RSA not as module, but into the Linux kernel,
> `rsa_init()` takes 130 ms on an ASRock E350M1.
> 
> (Timings are shown by adding `initcall_debug` to Linux command line [1].
> The times are visualized by `analyze_boot.py` from pm-graph [2] or
> `systemd-bootchart`.)
> 
> This is quite a lot of time compared to other modules, and I wonder if
> there are ways to decrease that time other than building it as a module,
> and not signing modules?

Is the testmgr compiled? If yes, the self test may take that time.
> 
> 
> Kind regards,
> 
> Paul
> 
> 
> [1] http://elinux.org/Initcall_Debug
> [2] https://github.com/01org/pm-graph



Ciao
Stephan

Decreasing time for `rsa_init`

[Paul Menzel]

Dear Linux crypto folks,


Building CRYPTO_RSA not as module, but into the Linux kernel, 
`rsa_init()` takes 130 ms on an ASRock E350M1.


(Timings are shown by adding `initcall_debug` to Linux command line [1]. 
The times are visualized by `analyze_boot.py` from pm-graph [2] or 
`systemd-bootchart`.)


This is quite a lot of time compared to other modules, and I wonder if 
there are ways to decrease that time other than building it as a module, 
and not signing modules?



Kind regards,

Paul


[1] http://elinux.org/Initcall_Debug
[2] https://github.com/01org/pm-graph

Re: [PATCH] crypto: cavium: make several functions static

[Herbert Xu]

On Tue, Jun 20, 2017 at 11:35:50AM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> The functions cvm_encrypt, cvm_decrypt, cvm_xts_setkey and
> cvm_enc_dec_init does not need to be in global scope, so make
> them static.
> 
> Signed-off-by: Colin Ian King 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: caam - properly set IV after {en,de}crypt

[Herbert Xu]

On Wed, Jun 28, 2017 at 03:27:10PM +0200, David Gstir wrote:
> Certain cipher modes like CTS expect the IV (req->info) of
> ablkcipher_request (or equivalently req->iv of skcipher_request) to
> contain the last ciphertext block when the {en,de}crypt operation is done.
> This is currently not the case for the CAAM driver which in turn breaks
> e.g. cts(cbc(aes)) when the CAAM driver is enabled.
> 
> This patch fixes the CAAM driver to properly set the IV after the
> {en,de}crypt operation of ablkcipher finishes.
> 
> This issue was revealed by the changes in the SW CTS mode in commit
> 0605c41cc53ca ("crypto: cts - Convert to skcipher")
> 
> Cc:  # 4.8+
> Signed-off-by: David Gstir 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: chcr: Avoid algo allocation in softirq.

[Herbert Xu]

On Fri, Jun 23, 2017 at 07:45:11PM +0530, Harsh Jain wrote:
> Thsi patch fixes calling "crypto_alloc_cipher" call in bottom halves.
> Pre allocate aes cipher required to update Tweak value for XTS.
> 
> Signed-off-by: Harsh Jain 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC PATCH] crypto: caam - fix signals handling

[Herbert Xu]

On Fri, Jul 07, 2017 at 04:57:06PM +0300, Horia Geantă wrote:
> Driver does not properly handle the case when signals interrupt
> wait_for_completion_interruptible():
> -it does not check for return value
> -completion structure is allocated on stack; in case a signal interrupts
> the sleep, it will go out of scope, causing the worker thread
> (caam_jr_dequeue) to fail when it accesses it
> 
> wait_for_completion_interruptible() is replaced with uninterruptable
> wait_for_completion().
> We choose to block all signals while waiting for I/O (device executing
> the split key generation job descriptor) since the alternative - in
> order to have a deterministic device state - would be to flush the job
> ring (aborting *all* in-progress jobs).
> 
> Cc: 
> Fixes: 045e36780f115 ("crypto: caam - ahash hmac support")
> Fixes: 4c1ec1f930154 ("crypto: caam - refactor key_gen, sg")
> Signed-off-by: Horia Geantă 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [BUGFIX] crypto: atmel: only treat EBUSY as transient if backlog

[Herbert Xu]

On Wed, Jun 28, 2017 at 10:22:03AM +0300, Gilad Ben-Yossef wrote:
> The Atmel SHA driver was treating -EBUSY as indication of queueing
> to backlog without checking that backlog is enabled for the request.
> 
> Fix it by checking request flags.
> 
> Signed-off-by: Gilad Ben-Yossef 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: change hwrng device default permissions to 0444

[Herbert Xu]

On Mon, Jul 03, 2017 at 12:37:59PM +0200, Harald Freudenberger wrote:
> Currently /dev/hwrng uses default device node permissions
> which is 0600. So by default the device node is not accessible
> by an ordinary user. Some distros do rewrite the device node
> permissions via udev rule, others don't. This patch provides
> 0444 as the new mode value and so makes the device node
> accessible for all users without the need to have udev rules
> rewriting the access rights.
> 
> Signed-off-by: Harald Freudenberger 

Hmm, one usage scenario for /dev/hwrng is to feed rngd which then
feeds into /dev/random.  In that case it may not be desirable to
allow arbitrary access to hwrgn since it may cause the rate of
entropy going into /dev/random to go down.

In any case, as you noted userspace can change this anyway so I
don't see why we need to make this policy change in the kernel.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v3 net-next 3/4] tls: kernel TLS support

[Steffen Klassert]

On Tue, Jul 11, 2017 at 11:53:11AM -0700, Dave Watson wrote:
> On 07/11/17 08:29 AM, Steffen Klassert wrote:
> > Sorry for replying to old mail...
> > > +int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
> > > +{
> > 
> > ...
> > 
> > > +
> > > + if (!sw_ctx->aead_send) {
> > > + sw_ctx->aead_send = crypto_alloc_aead("gcm(aes)", 0, 0);
> > > + if (IS_ERR(sw_ctx->aead_send)) {
> > > + rc = PTR_ERR(sw_ctx->aead_send);
> > > + sw_ctx->aead_send = NULL;
> > > + goto free_rec_seq;
> > > + }
> > > + }
> > > +
> > 
> > When I look on how you allocate the aead transformation, it seems
> > that you should either register an asynchronous callback with
> > aead_request_set_callback(), or request for a synchronous algorithm.
> > 
> > Otherwise you will crash on an asynchronous crypto return, no?
> 
> The intention is for it to be synchronous, and gather directly from
> userspace buffers.  It looks like calling
> crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC) is the correct way
> to request synchronous algorithms only?

Yes, but then you loose the aes-ni based algorithms because they are
asynchronous. If you want to have good crypto performance, it is
better to implement the asynchronous callbacks.

> 
> > Also, it seems that you have your scatterlists on a per crypto
> > transformation base istead of per crypto request. Is this intentional?
> 
> We hold the socket lock and only one crypto op can happen at a time,
> so we reuse the scatterlists.

This is OK as long as the crypto happens synchronous. But as said above,
I think this is not what you want.