Re: [PATCH 0/6] MODSIGN: Kernel module signing

2007-02-14 Thread Michael Halcrow
On Wed, Feb 14, 2007 at 09:59:37PM +, David Howells wrote:
 Michael Halcrow [EMAIL PROTECTED] wrote:
 
  Right now, eCryptfs just delegates its modular exponentiation
  operations to a userspace daemon. If RSA ever finds its way into the
  kernel, I might tweak eCryptfs to use that instead for some of the
  public key operations.
 
 Am I right in thinking that RSA uses many of the same MPI lib bits
 as DSA?

I would imagine so. Assuming we aren't having to hassle with key
generation (eCryptfs takes care of that in userspace), then RSA is,
more or less, a^b mod c (mpi_mulpowm() and friends).

Mike
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [CRYPTO] is it really optimized ?

2007-04-14 Thread Michael Halcrow
On Sun, Apr 15, 2007 at 05:34:19AM +1000, Herbert Xu wrote:
 Francis Moreau [EMAIL PROTECTED] wrote:
  
  hmm yes indeed it should do the job, but I don't see how you do that.
  For example, let say I want to use aes-foo with eCryptfs. I can give
  a higher priority to aes-foo than aes one. When eCryptfs asks for
  a aes cipher it will pass aes name and since aes-foo has a higher
  priority then the cypto core will return aes-foo cipher, right ? But
  in this scheme, eCryptfs has not a higher priority than other kernel
  users. How can I prevent others to use aes-foo ?
 
 You would assign aes-foo a lower priority and then tell eCryptfs to
 use aes-foo instead of aes.

Note that eCryptfs whitelists the cipher name (see
fs/ecryptfs/crypto.c::ecryptfs_cipher_code_str_map[] and associated
functions). This is because eCryptfs needs to pick a cipher code
(RFC2440-ish) to identify the cipher in the encrypted file
metadata. Shall I go ahead with a patch to add support for the '-'
qualifier?

Mike
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RSA support into kernel?

2007-07-06 Thread Michael Halcrow
On Fri, Jul 06, 2007 at 09:12:52PM +0800, Herbert Xu wrote:
 So far the only proposed user for RSA in-kernel seems to be module
 signing and I'm staying well away from that debate :)

eCryptfs uses RSA.

Right now it has to defer to a userspace daemon to perform the
operation.

Mike
.___.
 Michael A. Halcrow  
   Security Software Engineer, IBM Linux Technology Center   
GnuPG Fingerprint: 419C 5B1E 948A FA73 A54C  20F5 DB40 8531 6DCA 8769

This is about humans being human.  
 - Carl Sagan 


signature.asc
Description: Digital signature


Re: [PATCH 1/1]: Add countersize to CTR

2007-10-23 Thread Michael Halcrow
On Tue, Oct 23, 2007 at 03:26:29PM -0500, Joy Latten wrote:
 + unsigned int countersize;

It's somewhat nicer to just use size_t in the kernel for these sorts
of data types. If you care about the exact number of bytes used by the
variable, types like u32 make the code more parsable.

 + err = crypto_attr_u32(tb[4], countersize);
 + if (err)
 + goto out_put_alg;

It's also nice to have printk's along error paths. Syslogs down the
road tend to be less cryptic.

 - test_cipher(ctr(aes,4,8), ENCRYPT, aes_ctr_enc_tv_template,
 + test_cipher(ctr(aes,4,8,4), ENCRYPT, aes_ctr_enc_tv_template,
   AES_CTR_ENC_TEST_VECTORS);
 - test_cipher(ctr(aes,4,8), DECRYPT, aes_ctr_dec_tv_template,
 + test_cipher(ctr(aes,4,8,4), DECRYPT, aes_ctr_dec_tv_template,
   AES_CTR_DEC_TEST_VECTORS);

I have never been particularly thrilled about the the string-based
method of parameterizing block ciphers for in-kernel API calls.

Mike


signature.asc
Description: Digital signature


Re: [PATCH 1/1]: Add countersize to CTR

2007-10-23 Thread Michael Halcrow
On Wed, Oct 24, 2007 at 09:19:05AM +0800, Herbert Xu wrote:
 These paths can be triggered from user-space in future so printks
 are not appropriate.

I am familiar with CryptoDev and Cryproc. Will you be implementing
anything similar to what these projects are currently doing? Or do you
have something else entirely in mind?

Mike


signature.asc
Description: Digital signature


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

2017-07-13 Thread Michael Halcrow
On Wed, Jul 12, 2017 at 02:00:30PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebigg...@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.

The main concern that comes to mind is potentially blowing past the
inline xattr size limit and allocating a new inode block.  The
security benefit probably outweighs that concern in this case.

> 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 <ebigg...@google.com>

Acked-by: Michael Halcrow <mhalc...@google.com>

> ---
>  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_SIZE  64
>  
>  #define FS_KEY_DERIVATION_NONCE_SIZE 16

I'm seeing tab misalignment from all the other values here.  Maybe
remove the extra tab while you're at it?

> +#define FSCRYPT_KEY_HASH_SIZE16
>  
>  /**
> - * 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_V1  1
> + /* v2+ */
> +
> + /* Cryptographically s

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

2017-07-17 Thread Michael Halcrow
On Wed, Jul 12, 2017 at 02:00:35PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebigg...@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.
> 
> Signed-off-by: Eric Biggers <ebigg...@google.com>

Reviewed-by: Michael Halcrow <mhalc...@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 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
> 


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

2017-07-17 Thread Michael Halcrow
On Wed, Jul 12, 2017 at 02:00:34PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebigg...@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.
> 
> Signed-off-by: Eric Biggers <ebigg...@google.com>

Reviewed-by: Michael Halcrow <mhalc...@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 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;
>  }
>  
> +/*
> + *

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

2017-07-14 Thread Michael Halcrow
On Wed, Jul 12, 2017 at 02:00:33PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebigg...@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.
> 
> Signed-off-by: Eric Biggers <ebigg...@google.com>

Acked-by: Michael Halcrow <mhalc...@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 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_KEY1
> +#define HKDF_CONTEXT_KEY_HASH2
>  
>  /*
>   * 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 fs

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

2017-07-14 Thread Michael Halcrow
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;
>  
> + /*
> +  * 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..7ed1a7fb1308 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -6,17 +6,312 @@
>   * 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 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #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_KEY1
> +
> +/*
> + * HKDF consists of two steps:
> + *
> + * 1. HKDF-Extract: extract a fixed-length pseudorandom key from the
> + *input keying material and optional salt.
> + * 2. HDKF-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,

Modulo entropy gathered since boot.

> + * to generate the master keys.
> + *
> + * 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.)
> + */
> +
> +#defin

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

2017-07-14 Thread Michael Halcrow
On Wed, Jul 12, 2017 at 02:00:31PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebigg...@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.
> 
> Signed-off-by: Eric Biggers <ebigg...@google.com>

Acked-by: Michael Halcrow <mhalc...@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 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
>