On Mon, Oct 23, 2017 at 02:40:39PM -0700, Eric Biggers wrote: > By having an API to add a key to the *filesystem* we'll be able to > eliminate all the above hacks and better express the intended semantics: > the "locked/unlocked" status of an encrypted directory is global. And > orthogonally to encryption, existing mechanisms such as file permissions > and LSMs can and should continue to be used for the purpose of *access > control*. At some point I'd like to try to tackle the problem of making the encryption policy somehow *reflect* the access control policy. For now this change cleans up a real mess and makes things much more manageable and predictable. > Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx> Reviewed-by: Michael Halcrow <mhalcrow@xxxxxxxxxx> > --- > fs/crypto/crypto.c | 12 +- > fs/crypto/fscrypt_private.h | 3 + > fs/crypto/keyinfo.c | 351 +++++++++++++++++++++++++++++++++++++++- > include/linux/fscrypt_notsupp.h | 5 + > include/linux/fscrypt_supp.h | 1 + > include/uapi/linux/fscrypt.h | 41 +++-- > 6 files changed, 397 insertions(+), 16 deletions(-) > > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c > index 608f6bbe0f31..489c504ac20d 100644 > --- a/fs/crypto/crypto.c > +++ b/fs/crypto/crypto.c > @@ -24,6 +24,7 @@ > #include <linux/module.h> > #include <linux/scatterlist.h> > #include <linux/ratelimit.h> > +#include <linux/key-type.h> > #include <linux/dcache.h> > #include <linux/namei.h> > #include <crypto/aes.h> > @@ -449,6 +450,8 @@ int fscrypt_initialize(unsigned int cop_flags) > */ > static int __init fscrypt_init(void) > { > + int err = -ENOMEM; > + > fscrypt_read_workqueue = alloc_workqueue("fscrypt_read_queue", > WQ_HIGHPRI, 0); > if (!fscrypt_read_workqueue) > @@ -462,14 +465,20 @@ static int __init fscrypt_init(void) > if (!fscrypt_info_cachep) > goto fail_free_ctx; > > + err = register_key_type(&key_type_fscrypt_mk); > + if (err) > + goto fail_free_info; > + > return 0; > > +fail_free_info: > + kmem_cache_destroy(fscrypt_info_cachep); > fail_free_ctx: > kmem_cache_destroy(fscrypt_ctx_cachep); > fail_free_queue: > destroy_workqueue(fscrypt_read_workqueue); > fail: > - return -ENOMEM; > + return err; > } > module_init(fscrypt_init) > > @@ -484,6 +493,7 @@ static void __exit fscrypt_exit(void) > destroy_workqueue(fscrypt_read_workqueue); > kmem_cache_destroy(fscrypt_ctx_cachep); > kmem_cache_destroy(fscrypt_info_cachep); > + unregister_key_type(&key_type_fscrypt_mk); > > fscrypt_essiv_cleanup(); > } > diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h > index 5cb80a2d39ea..b2fad12eeedb 100644 > --- a/fs/crypto/fscrypt_private.h > +++ b/fs/crypto/fscrypt_private.h > @@ -27,6 +27,8 @@ > > #define FS_KEY_DERIVATION_NONCE_SIZE 16 > > +#define FSCRYPT_MIN_KEY_SIZE 16 > + > /** > * Encryption context for inode > * > @@ -93,6 +95,7 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx, > gfp_t gfp_flags); > > /* keyinfo.c */ > +extern struct key_type key_type_fscrypt_mk; > extern void __exit fscrypt_essiv_cleanup(void); > > #endif /* _FSCRYPT_PRIVATE_H */ > diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c > index d3a97c2cd4dd..3f1cb8bbc1e5 100644 > --- a/fs/crypto/keyinfo.c > +++ b/fs/crypto/keyinfo.c > @@ -9,14 +9,307 @@ > */ > > #include <keys/user-type.h> > -#include <linux/scatterlist.h> > +#include <linux/key-type.h> > #include <linux/ratelimit.h> > +#include <linux/scatterlist.h> > +#include <linux/seq_file.h> > #include <crypto/aes.h> > #include <crypto/sha.h> > #include "fscrypt_private.h" > > static struct crypto_shash *essiv_hash_tfm; > > +/* > + * fscrypt_master_key_secret - secret key material of an in-use master key > + */ > +struct fscrypt_master_key_secret { > + > + /* Size of the raw key in bytes */ > + u32 size; > + > + /* The raw key */ > + u8 raw[FSCRYPT_MAX_KEY_SIZE]; > +}; With structs fscrypt introduces, I suggest __randomize_layout wherever feasible. > +#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \ > + (sizeof("fscrypt-") - 1 + sizeof(((struct super_block > *)0)->s_id) + 1) What function do the " - 1" and " + 1" parts serve here? Readability? > + key = find_master_key(inode->i_sb, &mk_spec); > + if (IS_ERR(key)) { > + if (key != ERR_PTR(-ENOKEY)) > + return PTR_ERR(key); > + /* > + * As a legacy fallback, we search the current task's subscribed > + * keyrings in addition to ->s_master_keys. Please add an explicit comment that it's important for security that the ordering of these two searches be preserved. > + */ > + return find_and_derive_key_legacy(inode, ctx, derived_key, > + derived_keysize); > + } > + mk = key->payload.data[0]; > + > + /* > + * Require that the master key be at least as long as the derived key. > + * Otherwise, the derived key cannot possibly contain as much entropy as > + * that required by the encryption mode it will be used for. > + */ > + if (mk->mk_secret.size < derived_keysize) { As I've mentioned in a previous patch in this set, if we're going to get opinionated about source entropy, there's more we could measure/estimate than just the length. -- To unsubscribe from this list: send the line "unsubscribe linux-fscrypt" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |