All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] fscrypt: improved logging and other cleanups
@ 2018-03-23 19:23 ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

Hi, here's yet another round of cleanups for fscrypt.  Please consider
for v4.17.

Patches 1-11 remove some unnecessary or redundant code from a number of
different places; nothing particularly noteworthy.

Patch 12 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 13-14 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.

This series can also be retrieved from git at
https://github.com/ebiggers/linux, branch "fscrypt_2018-03-23".  Note
that it's on top of a few other fscrypt patches I've sent out this
cycle:

    fscrypt: clean up after fscrypt_prepare_lookup() conversions
    fs, fscrypt: only define ->s_cop when FS_ENCRYPTION is enabled
    fscrypt: add Speck128/256 support

Eric Biggers (14):
  fscrypt: remove unnecessary NULL check when allocating skcipher
  fscrypt: remove error messages for skcipher_request_alloc() failure
  fscrypt: remove WARN_ON_ONCE() when decryption fails
  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/bio.c              |   6 +-
 fs/crypto/crypto.c           |  35 ++++--
 fs/crypto/fname.c            |  32 ++----
 fs/crypto/fscrypt_private.h  |  18 ++--
 fs/crypto/hooks.c            |   5 +-
 fs/crypto/keyinfo.c          | 202 ++++++++++++++++++-----------------
 fs/ext4/super.c              |   8 +-
 fs/f2fs/super.c              |   8 +-
 fs/ubifs/crypto.c            |  10 +-
 include/linux/fscrypt_supp.h |   2 +-
 10 files changed, 161 insertions(+), 165 deletions(-)

-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 00/14] fscrypt: improved logging and other cleanups
@ 2018-03-23 19:23 ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

Hi, here's yet another round of cleanups for fscrypt.  Please consider
for v4.17.

Patches 1-11 remove some unnecessary or redundant code from a number of
different places; nothing particularly noteworthy.

Patch 12 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 13-14 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.

This series can also be retrieved from git at
https://github.com/ebiggers/linux, branch "fscrypt_2018-03-23".  Note
that it's on top of a few other fscrypt patches I've sent out this
cycle:

    fscrypt: clean up after fscrypt_prepare_lookup() conversions
    fs, fscrypt: only define ->s_cop when FS_ENCRYPTION is enabled
    fscrypt: add Speck128/256 support

Eric Biggers (14):
  fscrypt: remove unnecessary NULL check when allocating skcipher
  fscrypt: remove error messages for skcipher_request_alloc() failure
  fscrypt: remove WARN_ON_ONCE() when decryption fails
  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/bio.c              |   6 +-
 fs/crypto/crypto.c           |  35 ++++--
 fs/crypto/fname.c            |  32 ++----
 fs/crypto/fscrypt_private.h  |  18 ++--
 fs/crypto/hooks.c            |   5 +-
 fs/crypto/keyinfo.c          | 202 ++++++++++++++++++-----------------
 fs/ext4/super.c              |   8 +-
 fs/f2fs/super.c              |   8 +-
 fs/ubifs/crypto.c            |  10 +-
 include/linux/fscrypt_supp.h |   2 +-
 10 files changed, 161 insertions(+), 165 deletions(-)

-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 01/14] fscrypt: remove unnecessary NULL check when allocating skcipher
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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

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

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 96a1eae9e51d..8099388f5581 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -320,8 +320,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;
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 01/14] fscrypt: remove unnecessary NULL check when allocating skcipher
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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

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

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 96a1eae9e51d..8099388f5581 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -320,8 +320,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;
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 02/14] fscrypt: remove error messages for skcipher_request_alloc() failure
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/crypto.c |  6 +-----
 fs/crypto/fname.c  | 10 ++--------
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index f2b2d8408a01..58761d816751 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -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,
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index e33f3d3c5ade..3b5164b159cb 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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);
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 02/14] fscrypt: remove error messages for skcipher_request_alloc() failure
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/crypto.c |  6 +-----
 fs/crypto/fname.c  | 10 ++--------
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index f2b2d8408a01..58761d816751 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -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,
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index e33f3d3c5ade..3b5164b159cb 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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);
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 03/14] fscrypt: remove WARN_ON_ONCE() when decryption fails
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

If decrypting a block fails, fscrypt did a WARN_ON_ONCE().  But WARN is
meant for kernel bugs, which this isn't; this could be hit by fuzzers
using fault injection, for example.  Also, there is already a proper
warning message logged in fscrypt_do_page_crypto(), so the WARN doesn't
add much.

Just remove the unnessary WARN.

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

diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 0d5e6a569d58..771c8684826b 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -43,12 +43,10 @@ static void completion_pages(struct work_struct *work)
 		int ret = fscrypt_decrypt_page(page->mapping->host, page,
 				PAGE_SIZE, 0, page->index);
 
-		if (ret) {
-			WARN_ON_ONCE(1);
+		if (ret)
 			SetPageError(page);
-		} else {
+		else
 			SetPageUptodate(page);
-		}
 		unlock_page(page);
 	}
 	fscrypt_release_ctx(ctx);
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 03/14] fscrypt: remove WARN_ON_ONCE() when decryption fails
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

If decrypting a block fails, fscrypt did a WARN_ON_ONCE().  But WARN is
meant for kernel bugs, which this isn't; this could be hit by fuzzers
using fault injection, for example.  Also, there is already a proper
warning message logged in fscrypt_do_page_crypto(), so the WARN doesn't
add much.

Just remove the unnessary WARN.

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

diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 0d5e6a569d58..771c8684826b 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -43,12 +43,10 @@ static void completion_pages(struct work_struct *work)
 		int ret = fscrypt_decrypt_page(page->mapping->host, page,
 				PAGE_SIZE, 0, page->index);
 
-		if (ret) {
-			WARN_ON_ONCE(1);
+		if (ret)
 			SetPageError(page);
-		} else {
+		else
 			SetPageUptodate(page);
-		}
 		unlock_page(page);
 	}
 	fscrypt_release_ctx(ctx);
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 04/14] fscrypt: remove stale comment from fscrypt_d_revalidate()
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/crypto.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 58761d816751..d22887f689ad 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -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);
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 04/14] fscrypt: remove stale comment from fscrypt_d_revalidate()
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/crypto.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 58761d816751..d22887f689ad 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -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);
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 05/14] fscrypt: don't clear flags on crypto transform
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 8099388f5581..1435a1c81010 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -327,7 +327,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
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 05/14] fscrypt: don't clear flags on crypto transform
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 8099388f5581..1435a1c81010 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -327,7 +327,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
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 06/14] fscrypt: don't special-case EOPNOTSUPP from fscrypt_get_encryption_info()
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/fname.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 3b5164b159cb..8088a606c0aa 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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) {
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 06/14] fscrypt: don't special-case EOPNOTSUPP from fscrypt_get_encryption_info()
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/fname.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 3b5164b159cb..8088a606c0aa 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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) {
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 07/14] fscrypt: drop max_namelen check from fname_decrypt()
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 fscrypt_decrypt() has no internal limit itself.

Thus, remove this unnecessary check.

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

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 8088a606c0aa..cc9590b5f371 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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 */
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 07/14] fscrypt: drop max_namelen check from fname_decrypt()
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 fscrypt_decrypt() has no internal limit itself.

Thus, remove this unnecessary check.

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

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 8088a606c0aa..cc9590b5f371 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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 */
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 08/14] fscrypt: drop empty name check from fname_decrypt()
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/fname.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index cc9590b5f371..c4eb3a235ae4 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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)
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 08/14] fscrypt: drop empty name check from fname_decrypt()
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/fname.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index cc9590b5f371..c4eb3a235ae4 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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)
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 09/14] fscrypt: make fscrypt_operations.max_namelen an integer
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 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(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index c4eb3a235ae4..39091fc31e98 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 39bf464c35f1..63242840b1af 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1235,19 +1235,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
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8173ae688814..d4c9d580d32f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1802,18 +1802,12 @@ static int f2fs_set_context(struct inode *inode, const void *ctx, size_t len,
 				ctx, len, fs_data, XATTR_CREATE);
 }
 
-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,
 	.empty_dir	= f2fs_empty_dir,
-	.max_namelen	= f2fs_max_namelen,
+	.max_namelen	= F2FS_NAME_LEN,
 };
 #endif
 
diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
index 616a688f5d8f..55c508fe8131 100644
--- a/fs/ubifs/crypto.c
+++ b/fs/ubifs/crypto.c
@@ -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,
 };
diff --git a/include/linux/fscrypt_supp.h b/include/linux/fscrypt_supp.h
index c9c2cc26bc62..5080cb1bec4c 100644
--- a/include/linux/fscrypt_supp.h
+++ b/include/linux/fscrypt_supp.h
@@ -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 {
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 09/14] fscrypt: make fscrypt_operations.max_namelen an integer
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 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(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index c4eb3a235ae4..39091fc31e98 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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,
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 39bf464c35f1..63242840b1af 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1235,19 +1235,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
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8173ae688814..d4c9d580d32f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1802,18 +1802,12 @@ static int f2fs_set_context(struct inode *inode, const void *ctx, size_t len,
 				ctx, len, fs_data, XATTR_CREATE);
 }
 
-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,
 	.empty_dir	= f2fs_empty_dir,
-	.max_namelen	= f2fs_max_namelen,
+	.max_namelen	= F2FS_NAME_LEN,
 };
 #endif
 
diff --git a/fs/ubifs/crypto.c b/fs/ubifs/crypto.c
index 616a688f5d8f..55c508fe8131 100644
--- a/fs/ubifs/crypto.c
+++ b/fs/ubifs/crypto.c
@@ -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,
 };
diff --git a/include/linux/fscrypt_supp.h b/include/linux/fscrypt_supp.h
index c9c2cc26bc62..5080cb1bec4c 100644
--- a/include/linux/fscrypt_supp.h
+++ b/include/linux/fscrypt_supp.h
@@ -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 {
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 10/14] fscrypt: remove unnecessary check for non-logon key type
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 1435a1c81010..4913a4d5204f 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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 */
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 10/14] fscrypt: remove unnecessary check for non-logon key type
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 1435a1c81010..4913a4d5204f 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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 */
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 11/14] fscrypt: remove internal key size constants
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h | 10 +---------
 fs/crypto/keyinfo.c         | 19 +++++++------------
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index bf0bbba783f8..a3697397e190 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -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
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 4913a4d5204f..0a134f985681 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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,15 +120,11 @@ 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_SPECK128_256_XTS] = { "xts(speck128)", 64 },
+	[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 },
+	[FS_ENCRYPTION_MODE_SPECK128_256_XTS] = { "xts(speck128)",	64 },
 	[FS_ENCRYPTION_MODE_SPECK128_256_CTS] = { "cts(cbc(speck128))", 32 },
 };
 
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 11/14] fscrypt: remove internal key size constants
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h | 10 +---------
 fs/crypto/keyinfo.c         | 19 +++++++------------
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index bf0bbba783f8..a3697397e190 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -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
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 4913a4d5204f..0a134f985681 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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,15 +120,11 @@ 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_SPECK128_256_XTS] = { "xts(speck128)", 64 },
+	[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 },
+	[FS_ENCRYPTION_MODE_SPECK128_256_XTS] = { "xts(speck128)",	64 },
 	[FS_ENCRYPTION_MODE_SPECK128_256_CTS] = { "cts(cbc(speck128))", 32 },
 };
 
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 12/14] fscrypt: use a common logging function
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 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(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index d22887f689ad..a40d7d73b633 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -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.
  */
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 39091fc31e98..d7a0f682ca12 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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;
 	}
 
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a3697397e190..37562394c5de 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -104,6 +104,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);
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index bec06490fb13..926e5df20ec3 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -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);
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 0a134f985681..477e2d1fa04e 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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;
 	}
@@ -134,9 +133,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;
 	}
 
@@ -175,8 +175,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);
@@ -311,8 +312,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;
@@ -329,8 +331,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;
 		}
 	}
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 12/14] fscrypt: use a common logging function
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 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(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index d22887f689ad..a40d7d73b633 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -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.
  */
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 39091fc31e98..d7a0f682ca12 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -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;
 	}
 
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a3697397e190..37562394c5de 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -104,6 +104,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);
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index bec06490fb13..926e5df20ec3 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -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);
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 0a134f985681..477e2d1fa04e 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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;
 	}
@@ -134,9 +133,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;
 	}
 
@@ -175,8 +175,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);
@@ -311,8 +312,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;
@@ -329,8 +331,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;
 		}
 	}
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 13/14] fscrypt: separate key lookup from key derivation
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 122 +++++++++++++++++++++++++++-----------------
 1 file changed, 74 insertions(+), 48 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 477e2d1fa04e..f00bf1437c3c 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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 {
@@ -295,20 +331,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);
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 13/14] fscrypt: separate key lookup from key derivation
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 122 +++++++++++++++++++++++++++-----------------
 1 file changed, 74 insertions(+), 48 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 477e2d1fa04e..f00bf1437c3c 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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 {
@@ -295,20 +331,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);
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 14/14] fscrypt: only derive the needed portion of the key
  2018-03-23 19:23 ` Eric Biggers
@ 2018-03-23 19:23   ` Eric Biggers
  -1 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: linux-ext4, linux-f2fs-devel, linux-mtd, Jaegeuk Kim, Eric Biggers

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index f00bf1437c3c..81cd7ea86c07 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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;
@@ -327,7 +324,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;
 
@@ -345,10 +342,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;
-- 
2.17.0.rc0.231.g781580f067-goog

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 14/14] fscrypt: only derive the needed portion of the key
@ 2018-03-23 19:23   ` Eric Biggers
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Biggers @ 2018-03-23 19:23 UTC (permalink / raw)
  To: linux-fscrypt, Theodore Y . Ts'o
  Cc: Jaegeuk Kim, linux-ext4, linux-mtd, Eric Biggers, linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

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 <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index f00bf1437c3c..81cd7ea86c07 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -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;
@@ -327,7 +324,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;
 
@@ -345,10 +342,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;
-- 
2.17.0.rc0.231.g781580f067-goog


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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2018-03-23 19:23 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-23 19:23 [PATCH 00/14] fscrypt: improved logging and other cleanups Eric Biggers
2018-03-23 19:23 ` Eric Biggers
2018-03-23 19:23 ` [PATCH 01/14] fscrypt: remove unnecessary NULL check when allocating skcipher Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 02/14] fscrypt: remove error messages for skcipher_request_alloc() failure Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 03/14] fscrypt: remove WARN_ON_ONCE() when decryption fails Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 04/14] fscrypt: remove stale comment from fscrypt_d_revalidate() Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 05/14] fscrypt: don't clear flags on crypto transform Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 06/14] fscrypt: don't special-case EOPNOTSUPP from fscrypt_get_encryption_info() Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 07/14] fscrypt: drop max_namelen check from fname_decrypt() Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 08/14] fscrypt: drop empty name " Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 09/14] fscrypt: make fscrypt_operations.max_namelen an integer Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 10/14] fscrypt: remove unnecessary check for non-logon key type Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 11/14] fscrypt: remove internal key size constants Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 12/14] fscrypt: use a common logging function Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 13/14] fscrypt: separate key lookup from key derivation Eric Biggers
2018-03-23 19:23   ` Eric Biggers
2018-03-23 19:23 ` [PATCH 14/14] fscrypt: only derive the needed portion of the key Eric Biggers
2018-03-23 19:23   ` Eric Biggers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.