fscrypt: improved logging and other cleanups

PATCH In review Eric Biggers 2 participants 17 messages Apr 30, 2018 – May 21, 2018

Subsystems (6)

VFS Memory technology devices (MTD) ext4 file system F2FS file system +2 more subsystems

lore

Share & tools

lore

Eric Biggers

author

2018-04-30 22:51 UTC #0 lore raw

v2 cover fscrypt: improved logging and other cleanups

From: Eric Biggers <[email protected]>

Hello, here's yet another round of cleanups for fscrypt. Please consider for v4.18. These are all patches I've sent out previously. The series is based on v4.17-rc3.

Patches 1-12 mostly remove unnecessary or redundant code from a number of different places; nothing particularly noteworthy. Note: to avoid a merge conflict with f2fs/dev, I've dropped the patch "fscrypt: remove WARN_ON_ONCE() when decryption fails".

Patch 13 switches the existing warning and error messages in fscrypt over to use a common logging function, which is ratelimited and includes the filesystem name (->s_id) when available as well as a consistent prefix of "fscrypt:".

Patches 14-15 are optimizations and cleanups for key lookup and key derivation. They are extracted from the patch "fscrypt: refactor finding and deriving key" I had originally sent as part of the series "fscrypt: filesystem-level keyring and v2 policy support". But I think they're useful changes by themselves, so no need to wait on them.

Eric Biggers (15):
fs, fscrypt: only define ->s_cop when FS_ENCRYPTION is enabled
fscrypt: clean up after fscrypt_prepare_lookup() conversions
fscrypt: remove unnecessary NULL check when allocating skcipher
fscrypt: remove error messages for skcipher_request_alloc() failure
fscrypt: remove stale comment from fscrypt_d_revalidate()
fscrypt: don't clear flags on crypto transform
fscrypt: don't special-case EOPNOTSUPP from
fscrypt_get_encryption_info()
fscrypt: drop max_namelen check from fname_decrypt()
fscrypt: drop empty name check from fname_decrypt()
fscrypt: make fscrypt_operations.max_namelen an integer
fscrypt: remove unnecessary check for non-logon key type
fscrypt: remove internal key size constants
fscrypt: use a common logging function
fscrypt: separate key lookup from key derivation
fscrypt: only derive the needed portion of the key

 fs/crypto/crypto.c              |  36 ++++--
 fs/crypto/fname.c               |  32 ++---
 fs/crypto/fscrypt_private.h     |  19 +--
 fs/crypto/hooks.c               |   5 +-
 fs/crypto/keyinfo.c             | 200 +++++++++++++++++---------------
 fs/ext4/super.c                 |   8 +-
 fs/f2fs/super.c                 |   8 +-
 fs/ubifs/crypto.c               |  10 +-
 include/linux/fs.h              |   4 +-
 include/linux/fscrypt_notsupp.h |  10 --
 include/linux/fscrypt_supp.h    |  16 +--
 11 files changed, 161 insertions(+), 187 deletions(-)

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #1 lore raw

PATCH v2 1/15 fs, fscrypt: only define ->s_cop when FS_ENCRYPTION is enabled

From: Eric Biggers <[email protected]>

Now that filesystems only set and use their fscrypt_operations when they are built with encryption support, we can remove ->s_cop from 'struct super_block' when FS_ENCRYPTION is disabled. This saves a few bytes on some kernels and also makes it consistent with ->i_crypt_info.

Signed-off-by Eric Biggers <[email protected]>

---
 include/linux/fs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/linux/fs.h +2 -2

index 760d8da1b6c7..8e2460694c3a 100644
@@ -1364,9 +1364,9 @@ struct super_block {
 	void                    *s_security;
 #endif
 	const struct xattr_handler **s_xattr;
-
+#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
 	const struct fscrypt_operations	*s_cop;
-
+#endif
 	struct hlist_bl_head	s_roots;	/* alternate root dentries for NFS */
 	struct list_head	s_mounts;	/* list of mounts; _not_ for fs use */
 	struct block_device	*s_bdev;

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Eric Biggers

author

2018-04-30 22:51 UTC #2 lore raw

PATCH v2 2/15 fscrypt: clean up after fscrypt_prepare_lookup() conversions

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Now that all filesystems have been converted to use fscrypt_prepare_lookup(), we can remove the fscrypt_set_d_op() and fscrypt_set_encrypted_dentry() functions as well as un-export fscrypt_d_ops.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/crypto.c              |  1 -
 fs/crypto/fscrypt_private.h     |  1 +
 include/linux/fscrypt_notsupp.h | 10 ----------
 include/linux/fscrypt_supp.h    | 14 --------------
 4 files changed, 1 insertion(+), 25 deletions(-)

fs/crypto/crypto.c +0 -1

index ce654526c0fb..f2b2d8408a01 100644
@@ -353,7 +353,6 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 const struct dentry_operations fscrypt_d_ops = {
 	.d_revalidate = fscrypt_d_revalidate,
 };
-EXPORT_SYMBOL(fscrypt_d_ops);
 
 void fscrypt_restore_control_page(struct page *page)
 {

fs/crypto/fscrypt_private.h +1 -0

index ad6722bae8b7..fb96e493167b 100644
@@ -106,6 +106,7 @@ extern int fscrypt_do_page_crypto(const struct inode *inode,
 				  gfp_t gfp_flags);
 extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
 					      gfp_t gfp_flags);
+extern const struct dentry_operations fscrypt_d_ops;
 
 /* fname.c */
 extern int fname_encrypt(struct inode *inode, const struct qstr *iname,

include/linux/fscrypt_notsupp.h +0 ~~-10~~

index 44b50c04bae9..25b6492de6e5 100644
@@ -64,16 +64,6 @@ static inline void fscrypt_restore_control_page(struct page *page)
 	return;
 }
 
-static inline void fscrypt_set_d_op(struct dentry *dentry)
-{
-	return;
-}
-
-static inline void fscrypt_set_encrypted_dentry(struct dentry *dentry)
-{
-	return;
-}
-
 /* policy.c */
 static inline int fscrypt_ioctl_set_policy(struct file *filp,
 					   const void __user *arg)

include/linux/fscrypt_supp.h +0 ~~-14~~

index 477a7a6504d2..c9c2cc26bc62 100644
@@ -74,20 +74,6 @@ static inline struct page *fscrypt_control_page(struct page *page)
 
 extern void fscrypt_restore_control_page(struct page *);
 
-extern const struct dentry_operations fscrypt_d_ops;
-
-static inline void fscrypt_set_d_op(struct dentry *dentry)
-{
-	d_set_d_op(dentry, &fscrypt_d_ops);
-}
-
-static inline void fscrypt_set_encrypted_dentry(struct dentry *dentry)
-{
-	spin_lock(&dentry->d_lock);
-	dentry->d_flags |= DCACHE_ENCRYPTED_WITH_KEY;
-	spin_unlock(&dentry->d_lock);
-}
-
 /* policy.c */
 extern int fscrypt_ioctl_set_policy(struct file *, const void __user *);
 extern int fscrypt_ioctl_get_policy(struct file *, void __user *);

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #3 lore raw

PATCH v2 3/15 fscrypt: remove unnecessary NULL check when allocating skcipher

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

crypto_alloc_skcipher() returns an ERR_PTR() on failure, not NULL. Remove the unnecessary check for NULL.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/keyinfo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/crypto/keyinfo.c +2 -2

index 05f5ee1f0705..d09df8f751df 100644
@@ -318,8 +318,8 @@ int fscrypt_get_encryption_info(struct inode *inode)
 		goto out;
 	}
 	ctfm = crypto_alloc_skcipher(cipher_str, 0, 0);
-	if (!ctfm || IS_ERR(ctfm)) {
-		res = ctfm ? PTR_ERR(ctfm) : -ENOMEM;
+	if (IS_ERR(ctfm)) {
+		res = PTR_ERR(ctfm);
 		pr_debug("%s: error %d (inode %lu) allocating crypto tfm\n",
 			 __func__, res, inode->i_ino);
 		goto out;

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #4 lore raw

PATCH v2 4/15 fscrypt: remove error messages for skcipher_request_alloc() failure

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

skcipher_request_alloc() can only fail due to lack of memory, and in that case the memory allocator will have already printed a detailed error message. Thus, remove the redundant error messages from fscrypt.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/crypto.c |  6 +-----
 fs/crypto/fname.c  | 10 ++--------
 2 files changed, 3 insertions(+), 13 deletions(-)

fs/crypto/crypto.c +1 -5

index f2b2d8408a01..58761d816751 100644
@@ -156,12 +156,8 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
 	}
 
 	req = skcipher_request_alloc(tfm, gfp_flags);
-	if (!req) {
-		printk_ratelimited(KERN_ERR
-				"%s: crypto_request_alloc() failed\n",
-				__func__);
+	if (!req)
 		return -ENOMEM;
-	}
 
 	skcipher_request_set_callback(
 		req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,

fs/crypto/fname.c +2 -8

index e33f3d3c5ade..3b5164b159cb 100644
@@ -59,11 +59,8 @@ int fname_encrypt(struct inode *inode, const struct qstr *iname,
 
 	/* Set up the encryption request */
 	req = skcipher_request_alloc(tfm, GFP_NOFS);
-	if (!req) {
-		printk_ratelimited(KERN_ERR
-			"%s: skcipher_request_alloc() failed\n", __func__);
+	if (!req)
 		return -ENOMEM;
-	}
 	skcipher_request_set_callback(req,
 			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
 			crypto_req_done, &wait);

@@ -108,11 +105,8 @@ static int fname_decrypt(struct inode *inode,
 
 	/* Allocate request */
 	req = skcipher_request_alloc(tfm, GFP_NOFS);
-	if (!req) {
-		printk_ratelimited(KERN_ERR
-			"%s: crypto_request_alloc() failed\n",  __func__);
+	if (!req)
 		return -ENOMEM;
-	}
 	skcipher_request_set_callback(req,
 		CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
 		crypto_req_done, &wait);

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #5 lore raw

PATCH v2 5/15 fscrypt: remove stale comment from fscrypt_d_revalidate()

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/crypto.c | 1 -
 1 file changed, 1 deletion(-)

fs/crypto/crypto.c +0 -1

index 58761d816751..d22887f689ad 100644
@@ -322,7 +322,6 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 		return 0;
 	}
 
-	/* this should eventually be an flag in d_flags */
 	spin_lock(&dentry->d_lock);
 	cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY;
 	spin_unlock(&dentry->d_lock);

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #6 lore raw

PATCH v2 6/15 fscrypt: don't clear flags on crypto transform

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

fscrypt is clearing the flags on the crypto_skcipher it allocates for each inode. But, this is unnecessary and may cause problems in the future because it will even clear flags that are meant to be internal to the crypto API, e.g. CRYPTO_TFM_NEED_KEY.

Remove the unnecessary flag clearing.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/keyinfo.c | 1 -
 1 file changed, 1 deletion(-)

fs/crypto/keyinfo.c +0 -1

index d09df8f751df..0f6a65c6483b 100644
@@ -325,7 +325,6 @@ int fscrypt_get_encryption_info(struct inode *inode)
 		goto out;
 	}
 	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

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Eric Biggers

author

2018-04-30 22:51 UTC #7 lore raw

PATCH v2 7/15 fscrypt: don't special-case EOPNOTSUPP from fscrypt_get_encryption_info()

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

In fscrypt_setup_filename(), remove the unnecessary check for fscrypt_get_encryption_info() returning EOPNOTSUPP. There's no reason to handle this error differently from any other. I think there may have been some confusion because the "notsupp" version of fscrypt_get_encryption_info() returns EOPNOTSUPP -- but that's not applicable from inside fs/crypto/.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/fname.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/crypto/fname.c +1 -1

index 3b5164b159cb..8088a606c0aa 100644
@@ -335,7 +335,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 		return 0;
 	}
 	ret = fscrypt_get_encryption_info(dir);
-	if (ret && ret != -EOPNOTSUPP)
+	if (ret)
 		return ret;
 
 	if (dir->i_crypt_info) {

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #8 lore raw

PATCH v2 8/15 fscrypt: drop max_namelen check from fname_decrypt()

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

fname_decrypt() returns an error if the input filename is longer than the inode's ->max_namelen() as given by the filesystem. But, this doesn't actually make sense because the filesystem provided the input filename in the first place, where it was subject to the filesystem's limits. And fname_decrypt() has no internal limit itself.

Thus, remove this unnecessary check.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/fname.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

fs/crypto/fname.c +2 -5

index 8088a606c0aa..cc9590b5f371 100644
@@ -93,14 +93,11 @@ static int fname_decrypt(struct inode *inode,
 	struct skcipher_request *req = NULL;
 	DECLARE_CRYPTO_WAIT(wait);
 	struct scatterlist src_sg, dst_sg;
-	struct fscrypt_info *ci = inode->i_crypt_info;
-	struct crypto_skcipher *tfm = ci->ci_ctfm;
+	struct crypto_skcipher *tfm = inode->i_crypt_info->ci_ctfm;
 	int res = 0;
 	char iv[FS_CRYPTO_BLOCK_SIZE];
-	unsigned lim;
 
-	lim = inode->i_sb->s_cop->max_namelen(inode);
-	if (iname->len <= 0 || iname->len > lim)
+	if (iname->len <= 0)
 		return -EIO;
 
 	/* Allocate request */

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Eric Biggers

author

2018-04-30 22:51 UTC #9 lore raw

PATCH v2 9/15 fscrypt: drop empty name check from fname_decrypt()

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

fname_decrypt() is validating that the encrypted filename is nonempty. However, earlier a stronger precondition was already enforced: the encrypted filename must be at least 16 (FS_CRYPTO_BLOCK_SIZE) bytes.

Drop the redundant check for an empty filename.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/fname.c | 3 ---
 1 file changed, 3 deletions(-)

fs/crypto/fname.c +0 -3

index cc9590b5f371..c4eb3a235ae4 100644
@@ -97,9 +97,6 @@ static int fname_decrypt(struct inode *inode,
 	int res = 0;
 	char iv[FS_CRYPTO_BLOCK_SIZE];
 
-	if (iname->len <= 0)
-		return -EIO;
-
 	/* Allocate request */
 	req = skcipher_request_alloc(tfm, GFP_NOFS);
 	if (!req)

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #10 lore raw

PATCH v2 10/15 fscrypt: make fscrypt_operations.max_namelen an integer

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Now ->max_namelen() is only called to limit the filename length when adding NUL padding, and only for real filenames -- not symlink targets. It also didn't give the correct length for symlink targets anyway since it forgot to subtract 'sizeof(struct fscrypt_symlink_data)'.

Thus, change ->max_namelen from a function to a simple 'unsigned int' that gives the filesystem's maximum filename length.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/fname.c            |  2 +-
 fs/ext4/super.c              |  8 +-------
 fs/f2fs/super.c              |  8 +-------
 fs/ubifs/crypto.c            | 10 +---------
 include/linux/fscrypt_supp.h |  2 +-
 5 files changed, 5 insertions(+), 25 deletions(-)

fs/crypto/fname.c +1 -1

index c4eb3a235ae4..39091fc31e98 100644
@@ -334,7 +334,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 
 	if (dir->i_crypt_info) {
 		if (!fscrypt_fname_encrypted_size(dir, iname->len,
-						  dir->i_sb->s_cop->max_namelen(dir),
+						  dir->i_sb->s_cop->max_namelen,
 						  &fname->crypto_buf.len))
 			return -ENAMETOOLONG;
 		fname->crypto_buf.name = kmalloc(fname->crypto_buf.len,

fs/ext4/super.c +1 -7

index eb104e8476f0..502c36da292c 100644
@@ -1237,19 +1237,13 @@ static bool ext4_dummy_context(struct inode *inode)
 	return DUMMY_ENCRYPTION_ENABLED(EXT4_SB(inode->i_sb));
 }
 
-static unsigned ext4_max_namelen(struct inode *inode)
-{
-	return S_ISLNK(inode->i_mode) ? inode->i_sb->s_blocksize :
-		EXT4_NAME_LEN;
-}
-
 static const struct fscrypt_operations ext4_cryptops = {
 	.key_prefix		= "ext4:",
 	.get_context		= ext4_get_context,
 	.set_context		= ext4_set_context,
 	.dummy_context		= ext4_dummy_context,
 	.empty_dir		= ext4_empty_dir,
-	.max_namelen		= ext4_max_namelen,
+	.max_namelen		= EXT4_NAME_LEN,
 };
 #endif

fs/f2fs/super.c +1 -7

index 42d564c5ccd0..970ae27f401c 100644
@@ -1930,19 +1930,13 @@ static bool f2fs_dummy_context(struct inode *inode)
 	return DUMMY_ENCRYPTION_ENABLED(F2FS_I_SB(inode));
 }
 
-static unsigned f2fs_max_namelen(struct inode *inode)
-{
-	return S_ISLNK(inode->i_mode) ?
-			inode->i_sb->s_blocksize : F2FS_NAME_LEN;
-}
-
 static const struct fscrypt_operations f2fs_cryptops = {
 	.key_prefix	= "f2fs:",
 	.get_context	= f2fs_get_context,
 	.set_context	= f2fs_set_context,
 	.dummy_context	= f2fs_dummy_context,
 	.empty_dir	= f2fs_empty_dir,
-	.max_namelen	= f2fs_max_namelen,
+	.max_namelen	= F2FS_NAME_LEN,
 };
 #endif

fs/ubifs/crypto.c +1 -9

index 616a688f5d8f..55c508fe8131 100644
@@ -24,14 +24,6 @@ static bool ubifs_crypt_empty_dir(struct inode *inode)
 	return ubifs_check_dir_empty(inode) == 0;
 }
 
-static unsigned int ubifs_crypt_max_namelen(struct inode *inode)
-{
-	if (S_ISLNK(inode->i_mode))
-		return UBIFS_MAX_INO_DATA;
-	else
-		return UBIFS_MAX_NLEN;
-}
-
 int ubifs_encrypt(const struct inode *inode, struct ubifs_data_node *dn,
 		  unsigned int in_len, unsigned int *out_len, int block)
 {

@@ -89,5 +81,5 @@ const struct fscrypt_operations ubifs_crypt_operations = {
 	.get_context		= ubifs_crypt_get_context,
 	.set_context		= ubifs_crypt_set_context,
 	.empty_dir		= ubifs_crypt_empty_dir,
-	.max_namelen		= ubifs_crypt_max_namelen,
+	.max_namelen		= UBIFS_MAX_NLEN,
 };

include/linux/fscrypt_supp.h +1 -1

index c9c2cc26bc62..5080cb1bec4c 100644
@@ -29,7 +29,7 @@ struct fscrypt_operations {
 	int (*set_context)(struct inode *, const void *, size_t, void *);
 	bool (*dummy_context)(struct inode *);
 	bool (*empty_dir)(struct inode *);
-	unsigned (*max_namelen)(struct inode *);
+	unsigned int max_namelen;
 };
 
 struct fscrypt_ctx {

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #11 lore raw

PATCH v2 11/15 fscrypt: remove unnecessary check for non-logon key type

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

We're passing 'key_type_logon' to request_key(), so the found key is guaranteed to be of type "logon". Thus, there is no reason to check later that the key is really a "logon" key.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/keyinfo.c | 6 ------
 1 file changed, 6 deletions(-)

fs/crypto/keyinfo.c +0 -6

index 0f6a65c6483b..0b48aa469453 100644
@@ -89,12 +89,6 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		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) {
 		/* key was revoked before we acquired its semaphore */

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #12 lore raw

PATCH v2 12/15 fscrypt: remove internal key size constants

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

With one exception, the internal key size constants such as FS_AES_256_XTS_KEY_SIZE are only used for the 'available_modes' array, where they really only serve to obfuscate what the values are. Also some of the constants are unused, and the key sizes tend to be in the names of the algorithms anyway. In the past these values were also misused, e.g. we used to have FS_AES_256_XTS_KEY_SIZE in places that technically should have been FS_MAX_KEY_SIZE.

The exception is that FS_AES_128_ECB_KEY_SIZE is used for key derivation. But it's more appropriate to use FS_KEY_DERIVATION_NONCE_SIZE for that instead.

Thus, just put the sizes directly in the 'available_modes' array.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/fscrypt_private.h | 10 +---------
 fs/crypto/keyinfo.c         | 17 ++++++-----------
 2 files changed, 7 insertions(+), 20 deletions(-)

fs/crypto/fscrypt_private.h +1 -9

index fb96e493167b..8358610d6558 100644
@@ -18,15 +18,7 @@
 
 /* Encryption parameters */
 #define FS_IV_SIZE			16
-#define FS_AES_128_ECB_KEY_SIZE		16
-#define FS_AES_128_CBC_KEY_SIZE		16
-#define FS_AES_128_CTS_KEY_SIZE		16
-#define FS_AES_256_GCM_KEY_SIZE		32
-#define FS_AES_256_CBC_KEY_SIZE		32
-#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
 
 /**
  * Encryption context for inode

fs/crypto/keyinfo.c +6 ~~-11~~

index 0b48aa469453..f6d6acd37b97 100644
@@ -27,7 +27,7 @@ static struct crypto_shash *essiv_hash_tfm;
  *
  * Return: Zero on success; non-zero otherwise.
  */
-static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
+static int derive_key_aes(u8 deriving_key[FS_KEY_DERIVATION_NONCE_SIZE],
 				const struct fscrypt_key *source_key,
 				u8 derived_raw_key[FS_MAX_KEY_SIZE])
 {

@@ -52,7 +52,7 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
 			crypto_req_done, &wait);
 	res = crypto_skcipher_setkey(tfm, deriving_key,
-					FS_AES_128_ECB_KEY_SIZE);
+				     FS_KEY_DERIVATION_NONCE_SIZE);
 	if (res < 0)
 		goto out;

@@ -100,7 +100,6 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		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) {

@@ -121,14 +120,10 @@ static const struct {
 	const char *cipher_str;
 	int keysize;
 } available_modes[] = {
-	[FS_ENCRYPTION_MODE_AES_256_XTS] = { "xts(aes)",
-					     FS_AES_256_XTS_KEY_SIZE },
-	[FS_ENCRYPTION_MODE_AES_256_CTS] = { "cts(cbc(aes))",
-					     FS_AES_256_CTS_KEY_SIZE },
-	[FS_ENCRYPTION_MODE_AES_128_CBC] = { "cbc(aes)",
-					     FS_AES_128_CBC_KEY_SIZE },
-	[FS_ENCRYPTION_MODE_AES_128_CTS] = { "cts(cbc(aes))",
-					     FS_AES_128_CTS_KEY_SIZE },
+	[FS_ENCRYPTION_MODE_AES_256_XTS]      = { "xts(aes)",		64 },
+	[FS_ENCRYPTION_MODE_AES_256_CTS]      = { "cts(cbc(aes))",	32 },
+	[FS_ENCRYPTION_MODE_AES_128_CBC]      = { "cbc(aes)",		16 },
+	[FS_ENCRYPTION_MODE_AES_128_CTS]      = { "cts(cbc(aes))",	16 },
 };
 
 static int determine_cipher_type(struct fscrypt_info *ci, struct inode *inode,

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Eric Biggers

author

2018-04-30 22:51 UTC #13 lore raw

PATCH v2 13/15 fscrypt: use a common logging function

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Use a common function for fscrypt warning and error messages so that all the messages are consistently ratelimited, include the "fscrypt:" prefix, and include the filesystem name if applicable.

Also fix up a few of the log messages to be more descriptive.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/crypto.c          | 28 +++++++++++++++++++++++++---
 fs/crypto/fname.c           | 10 ++++++----
 fs/crypto/fscrypt_private.h |  8 ++++++++
 fs/crypto/hooks.c           |  5 +++--
 fs/crypto/keyinfo.c         | 27 +++++++++++++++------------
 5 files changed, 57 insertions(+), 21 deletions(-)

fs/crypto/crypto.c +25 -3

index d22887f689ad..a40d7d73b633 100644
@@ -174,9 +174,10 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
 		res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
 	skcipher_request_free(req);
 	if (res) {
-		printk_ratelimited(KERN_ERR
-			"%s: crypto_skcipher_encrypt() returned %d\n",
-			__func__, res);
+		fscrypt_err(inode->i_sb,
+			    "%scryption failed for inode %lu, block %llu: %d",
+			    (rw == FS_DECRYPT ? "de" : "en"),
+			    inode->i_ino, lblk_num, res);
 		return res;
 	}
 	return 0;

@@ -416,6 +417,27 @@ int fscrypt_initialize(unsigned int cop_flags)
 	return res;
 }
 
+void fscrypt_msg(struct super_block *sb, const char *level,
+		 const char *fmt, ...)
+{
+	static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
+				      DEFAULT_RATELIMIT_BURST);
+	struct va_format vaf;
+	va_list args;
+
+	if (!__ratelimit(&rs))
+		return;
+
+	va_start(args, fmt);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+	if (sb)
+		printk("%sfscrypt (%s): %pV\n", level, sb->s_id, &vaf);
+	else
+		printk("%sfscrypt: %pV\n", level, &vaf);
+	va_end(args);
+}
+
 /**
  * fscrypt_init() - Set up for fs encryption.
  */

fs/crypto/fname.c +6 -4

index 39091fc31e98..d7a0f682ca12 100644
@@ -71,8 +71,9 @@ int fname_encrypt(struct inode *inode, const struct qstr *iname,
 	res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
 	skcipher_request_free(req);
 	if (res < 0) {
-		printk_ratelimited(KERN_ERR
-				"%s: Error (error code %d)\n", __func__, res);
+		fscrypt_err(inode->i_sb,
+			    "Filename encryption failed for inode %lu: %d",
+			    inode->i_ino, res);
 		return res;
 	}

@@ -115,8 +116,9 @@ static int fname_decrypt(struct inode *inode,
 	res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
 	skcipher_request_free(req);
 	if (res < 0) {
-		printk_ratelimited(KERN_ERR
-				"%s: Error (error code %d)\n", __func__, res);
+		fscrypt_err(inode->i_sb,
+			    "Filename decryption failed for inode %lu: %d",
+			    inode->i_ino, res);
 		return res;
 	}

fs/crypto/fscrypt_private.h +8 -0

index 8358610d6558..09d6c72635b6 100644
@@ -100,6 +100,14 @@ extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
 					      gfp_t gfp_flags);
 extern const struct dentry_operations fscrypt_d_ops;
 
+extern void __printf(3, 4) __cold
+fscrypt_msg(struct super_block *sb, const char *level, const char *fmt, ...);
+
+#define fscrypt_warn(sb, fmt, ...)		\
+	fscrypt_msg(sb, KERN_WARNING, fmt, ##__VA_ARGS__)
+#define fscrypt_err(sb, fmt, ...)		\
+	fscrypt_msg(sb, KERN_ERR, fmt, ##__VA_ARGS__)
+
 /* fname.c */
 extern int fname_encrypt(struct inode *inode, const struct qstr *iname,
 			 u8 *out, unsigned int olen);

fs/crypto/hooks.c +3 -2

index bec06490fb13..926e5df20ec3 100644
@@ -39,8 +39,9 @@ int fscrypt_file_open(struct inode *inode, struct file *filp)
 	dir = dget_parent(file_dentry(filp));
 	if (IS_ENCRYPTED(d_inode(dir)) &&
 	    !fscrypt_has_permitted_context(d_inode(dir), inode)) {
-		pr_warn_ratelimited("fscrypt: inconsistent encryption contexts: %lu/%lu",
-				    d_inode(dir)->i_ino, inode->i_ino);
+		fscrypt_warn(inode->i_sb,
+			     "inconsistent encryption contexts: %lu/%lu",
+			     d_inode(dir)->i_ino, inode->i_ino);
 		err = -EPERM;
 	}
 	dput(dir);

fs/crypto/keyinfo.c +15 ~~-12~~

index f6d6acd37b97..f63bfd6dffd6 100644
@@ -103,9 +103,8 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 
 	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);
+		fscrypt_warn(NULL, "key size incorrect: %u",
+			     master_key->size);
 		res = -ENOKEY;
 		goto out;
 	}

@@ -132,9 +131,10 @@ static int determine_cipher_type(struct fscrypt_info *ci, struct inode *inode,
 	u32 mode;
 
 	if (!fscrypt_valid_enc_modes(ci->ci_data_mode, ci->ci_filename_mode)) {
-		pr_warn_ratelimited("fscrypt: inode %lu uses unsupported encryption modes (contents mode %d, filenames mode %d)\n",
-				    inode->i_ino,
-				    ci->ci_data_mode, ci->ci_filename_mode);
+		fscrypt_warn(inode->i_sb,
+			     "inode %lu uses unsupported encryption modes (contents mode %d, filenames mode %d)",
+			     inode->i_ino, ci->ci_data_mode,
+			     ci->ci_filename_mode);
 		return -EINVAL;
 	}

@@ -173,8 +173,9 @@ static int derive_essiv_salt(const u8 *key, int keysize, u8 *salt)
 
 		tfm = crypto_alloc_shash("sha256", 0, 0);
 		if (IS_ERR(tfm)) {
-			pr_warn_ratelimited("fscrypt: error allocating SHA-256 transform: %ld\n",
-					    PTR_ERR(tfm));
+			fscrypt_warn(NULL,
+				     "error allocating SHA-256 transform: %ld",
+				     PTR_ERR(tfm));
 			return PTR_ERR(tfm);
 		}
 		prev_tfm = cmpxchg(&essiv_hash_tfm, NULL, tfm);

@@ -309,8 +310,9 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	ctfm = crypto_alloc_skcipher(cipher_str, 0, 0);
 	if (IS_ERR(ctfm)) {
 		res = PTR_ERR(ctfm);
-		pr_debug("%s: error %d (inode %lu) allocating crypto tfm\n",
-			 __func__, res, inode->i_ino);
+		fscrypt_warn(inode->i_sb,
+			     "error allocating '%s' transform for inode %lu: %d",
+			     cipher_str, inode->i_ino, res);
 		goto out;
 	}
 	crypt_info->ci_ctfm = ctfm;

@@ -327,8 +329,9 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	    crypt_info->ci_data_mode == FS_ENCRYPTION_MODE_AES_128_CBC) {
 		res = init_essiv_generator(crypt_info, raw_key, keysize);
 		if (res) {
-			pr_debug("%s: error %d (inode %lu) allocating essiv tfm\n",
-				 __func__, res, inode->i_ino);
+			fscrypt_warn(inode->i_sb,
+				     "error initializing ESSIV generator for inode %lu: %d",
+				     inode->i_ino, res);
 			goto out;
 		}
 	}

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Eric Biggers

author

2018-04-30 22:51 UTC #14 lore raw

PATCH v2 14/15 fscrypt: separate key lookup from key derivation

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Refactor the confusingly-named function 'validate_user_key()' into a new function 'find_and_derive_key()' which first finds the keyring key, then does the key derivation. Among other benefits this avoids the strange behavior we had previously where if key derivation failed for some reason, then we would fall back to the alternate key prefix. Now, we'll only fall back to the alternate key prefix if a valid key isn't found.

This patch also improves the warning messages that are logged when the keyring key's payload is invalid.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/keyinfo.c | 122 +++++++++++++++++++++++++++-----------------
 1 file changed, 74 insertions(+), 48 deletions(-)

fs/crypto/keyinfo.c +74 ~~-48~~

index f63bfd6dffd6..f248ee9974fa 100644
@@ -27,7 +27,7 @@ static struct crypto_shash *essiv_hash_tfm;
  *
  * Return: Zero on success; non-zero otherwise.
  */
-static int derive_key_aes(u8 deriving_key[FS_KEY_DERIVATION_NONCE_SIZE],
+static int derive_key_aes(const u8 deriving_key[FS_KEY_DERIVATION_NONCE_SIZE],
 				const struct fscrypt_key *source_key,
 				u8 derived_raw_key[FS_MAX_KEY_SIZE])
 {

@@ -67,52 +67,88 @@ static int derive_key_aes(u8 deriving_key[FS_KEY_DERIVATION_NONCE_SIZE],
 	return res;
 }
 
-static int validate_user_key(struct fscrypt_info *crypt_info,
-			struct fscrypt_context *ctx, u8 *raw_key,
-			const char *prefix, int min_keysize)
+/*
+ * Search the current task's subscribed keyrings for a "logon" key with
+ * description prefix:descriptor, and if found acquire a read lock on it and
+ * return a pointer to its validated payload in *payload_ret.
+ */
+static struct key *
+find_and_lock_process_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;
-	struct fscrypt_key *master_key;
+	struct key *key;
 	const struct user_key_payload *ukp;
-	int res;
+	const struct fscrypt_key *payload;
 
 	description = kasprintf(GFP_NOFS, "%s%*phN", prefix,
-				FS_KEY_DESCRIPTOR_SIZE,
-				ctx->master_key_descriptor);
+				FS_KEY_DESCRIPTOR_SIZE, descriptor);
 	if (!description)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
-	keyring_key = request_key(&key_type_logon, description, NULL);
+	key = request_key(&key_type_logon, description, NULL);
 	kfree(description);
-	if (IS_ERR(keyring_key))
-		return PTR_ERR(keyring_key);
-	down_read(&keyring_key->sem);
-
-	ukp = user_key_payload_locked(keyring_key);
-	if (!ukp) {
-		/* key was revoked before we acquired its semaphore */
-		res = -EKEYREVOKED;
-		goto out;
+	if (IS_ERR(key))
+		return key;
+
+	down_read(&key->sem);
+	ukp = user_key_payload_locked(key);
+
+	if (!ukp) /* was the key revoked before we acquired its semaphore? */
+		goto invalid;
+
+	payload = (const struct fscrypt_key *)ukp->data;
+
+	if (ukp->datalen != sizeof(struct fscrypt_key) ||
+	    payload->size < 1 || payload->size > FS_MAX_KEY_SIZE) {
+		fscrypt_warn(NULL,
+			     "key with description '%s' has invalid payload",
+			     key->description);
+		goto invalid;
 	}
-	if (ukp->datalen != sizeof(struct fscrypt_key)) {
-		res = -EINVAL;
-		goto out;
+
+	if (payload->size < min_keysize ||
+	    payload->size % AES_BLOCK_SIZE != 0) {
+		fscrypt_warn(NULL,
+			     "key with description '%s' is too short or is misaligned (got %u bytes, need %u+ bytes)",
+			     key->description, payload->size, min_keysize);
+		goto invalid;
 	}
-	master_key = (struct fscrypt_key *)ukp->data;
 
-	if (master_key->size < min_keysize || master_key->size > FS_MAX_KEY_SIZE
-	    || master_key->size % AES_BLOCK_SIZE != 0) {
-		fscrypt_warn(NULL, "key size incorrect: %u",
-			     master_key->size);
-		res = -ENOKEY;
-		goto out;
+	*payload_ret = payload;
+	return key;
+
+invalid:
+	up_read(&key->sem);
+	key_put(key);
+	return ERR_PTR(-ENOKEY);
+}
+
+/* Find the master key, then derive the inode's actual encryption key */
+static int find_and_derive_key(const struct inode *inode,
+			       const struct fscrypt_context *ctx,
+			       u8 *derived_key, unsigned int derived_keysize)
+{
+	struct key *key;
+	const struct fscrypt_key *payload;
+	int err;
+
+	key = find_and_lock_process_key(FS_KEY_DESC_PREFIX,
+					ctx->master_key_descriptor,
+					derived_keysize, &payload);
+	if (key == ERR_PTR(-ENOKEY) && inode->i_sb->s_cop->key_prefix) {
+		key = find_and_lock_process_key(inode->i_sb->s_cop->key_prefix,
+						ctx->master_key_descriptor,
+						derived_keysize, &payload);
 	}
-	res = derive_key_aes(ctx->nonce, master_key, raw_key);
-out:
-	up_read(&keyring_key->sem);
-	key_put(keyring_key);
-	return res;
+	if (IS_ERR(key))
+		return PTR_ERR(key);
+	err = derive_key_aes(ctx->nonce, payload, derived_key);
+	up_read(&key->sem);
+	key_put(key);
+	return err;
 }
 
 static const struct {

@@ -293,20 +329,10 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	if (!raw_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;
-			goto out;
-		}
-	} else if (res) {
+	res = find_and_derive_key(inode, &ctx, raw_key, keysize);
+	if (res)
 		goto out;
-	}
+
 	ctfm = crypto_alloc_skcipher(cipher_str, 0, 0);
 	if (IS_ERR(ctfm)) {
 		res = PTR_ERR(ctfm);

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Eric Biggers

author

2018-04-30 22:51 UTC #15 lore raw

PATCH v2 15/15 fscrypt: only derive the needed portion of the key

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Currently the key derivation function in fscrypt uses the master key length as the amount of output key material to derive. This works, but it means we can waste time deriving more key material than is actually used, e.g. most commonly, deriving 64 bytes for directories which only take a 32-byte AES-256-CTS-CBC key. It also forces us to validate that the master key length is a multiple of AES_BLOCK_SIZE, which wouldn't otherwise be necessary.

Fix it to only derive the needed length key.

Signed-off-by Eric Biggers <[email protected]>

---
 fs/crypto/keyinfo.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

fs/crypto/keyinfo.c +16 ~~-23~~

index f248ee9974fa..c4d1388fc9b4 100644
@@ -19,17 +19,16 @@
 
 static struct crypto_shash *essiv_hash_tfm;
 
-/**
- * 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.
+/*
+ * Key derivation function.  This generates the derived key by encrypting the
+ * master key with AES-128-ECB using the inode's nonce as the AES key.
  *
- * Return: Zero on success; non-zero otherwise.
+ * The master key must be at least as long as the derived key.  If the master
+ * key is longer, then only the first 'derived_keysize' bytes are used.
  */
-static int derive_key_aes(const u8 deriving_key[FS_KEY_DERIVATION_NONCE_SIZE],
-				const struct fscrypt_key *source_key,
-				u8 derived_raw_key[FS_MAX_KEY_SIZE])
+static int derive_key_aes(const u8 *master_key,
+			  const struct fscrypt_context *ctx,
+			  u8 *derived_key, unsigned int derived_keysize)
 {
 	int res = 0;
 	struct skcipher_request *req = NULL;

@@ -51,14 +50,13 @@ static int derive_key_aes(const u8 deriving_key[FS_KEY_DERIVATION_NONCE_SIZE],
 	skcipher_request_set_callback(req,
 			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
 			crypto_req_done, &wait);
-	res = crypto_skcipher_setkey(tfm, deriving_key,
-				     FS_KEY_DERIVATION_NONCE_SIZE);
+	res = crypto_skcipher_setkey(tfm, ctx->nonce, sizeof(ctx->nonce));
 	if (res < 0)
 		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, 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_wait_req(crypto_skcipher_encrypt(req), &wait);
 out:

@@ -109,10 +107,9 @@ find_and_lock_process_key(const char *prefix,
 		goto invalid;
 	}
 
-	if (payload->size < min_keysize ||
-	    payload->size % AES_BLOCK_SIZE != 0) {
+	if (payload->size < min_keysize) {
 		fscrypt_warn(NULL,
-			     "key with description '%s' is too short or is misaligned (got %u bytes, need %u+ bytes)",
+			     "key with description '%s' is too short (got %u bytes, need %u+ bytes)",
 			     key->description, payload->size, min_keysize);
 		goto invalid;
 	}

@@ -145,7 +142,7 @@ static int find_and_derive_key(const struct inode *inode,
 	}
 	if (IS_ERR(key))
 		return PTR_ERR(key);
-	err = derive_key_aes(ctx->nonce, payload, derived_key);
+	err = derive_key_aes(payload->raw, ctx, derived_key, derived_keysize);
 	up_read(&key->sem);
 	key_put(key);
 	return err;

@@ -325,7 +322,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	 * crypto API as part of key derivation.
 	 */
 	res = -ENOMEM;
-	raw_key = kmalloc(FS_MAX_KEY_SIZE, GFP_NOFS);
+	raw_key = kmalloc(keysize, GFP_NOFS);
 	if (!raw_key)
 		goto out;

@@ -343,10 +340,6 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	}
 	crypt_info->ci_ctfm = ctfm;
 	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);
 	if (res)
 		goto out;

Signature · 6 lines

2.17.0.441.gb46fe60e1d-goog


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Theodore Y. Ts'o

maintainer

2018-05-21 00:55 UTC #16 lore raw

In reply to Eric Biggers

Quoted 16 lines · Eric Biggers

From: Eric Biggers <[email protected]>
Hello, here's yet another round of cleanups for fscrypt. Please consider for v4.18. These are all patches I've sent out previously. The series is based on v4.17-rc3.
Patches 1-12 mostly remove unnecessary or redundant code from a number of different places; nothing particularly noteworthy. Note: to avoid a merge conflict with f2fs/dev, I've dropped the patch "fscrypt: remove WARN_ON_ONCE() when decryption fails".
Patch 13 switches the existing warning and error messages in fscrypt over to use a common logging function, which is ratelimited and includes the filesystem name (->s_id) when available as well as a consistent prefix of "fscrypt:".

Quoted 5 lines

Patches 14-15 are optimizations and cleanups for key lookup and key derivation. They are extracted from the patch "fscrypt: refactor finding and deriving key" I had originally sent as part of the series "fscrypt: filesystem-level keyring and v2 policy support". But I think they're useful changes by themselves, so no need to wait on them.

Thanks, I've applied this patch series.

- Ted

Quoted 18 lines

Eric Biggers (15):
fs, fscrypt: only define ->s_cop when FS_ENCRYPTION is enabled
fscrypt: clean up after fscrypt_prepare_lookup() conversions
fscrypt: remove unnecessary NULL check when allocating skcipher
fscrypt: remove error messages for skcipher_request_alloc() failure
fscrypt: remove stale comment from fscrypt_d_revalidate()
fscrypt: don't clear flags on crypto transform
fscrypt: don't special-case EOPNOTSUPP from
fscrypt_get_encryption_info()
fscrypt: drop max_namelen check from fname_decrypt()
fscrypt: drop empty name check from fname_decrypt()
fscrypt: make fscrypt_operations.max_namelen an integer
fscrypt: remove unnecessary check for non-logon key type
fscrypt: remove internal key size constants
fscrypt: use a common logging function
fscrypt: separate key lookup from key derivation
fscrypt: only derive the needed portion of the key

Quoted 16 lines

 fs/crypto/crypto.c              |  36 ++++--
 fs/crypto/fname.c               |  32 ++---
 fs/crypto/fscrypt_private.h     |  19 +--
 fs/crypto/hooks.c               |   5 +-
 fs/crypto/keyinfo.c             | 200 +++++++++++++++++---------------
 fs/ext4/super.c                 |   8 +-
 fs/f2fs/super.c                 |   8 +-
 fs/ubifs/crypto.c               |  10 +-
 include/linux/fs.h              |   4 +-
 include/linux/fscrypt_notsupp.h |  10 --
 include/linux/fscrypt_supp.h    |  16 +--
 11 files changed, 161 insertions(+), 187 deletions(-)

Signature · 1 line

2.17.0.441.gb46fe60e1d-goog

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot