[PATCH 0/4] fscrypt: logging improvements, and use ENOPKG

[PATCH 0/4] fscrypt: logging improvements, and use ENOPKG

[Eric Biggers]

Patches 1-3 make some small improvements to warning and error messages
in fs/crypto/, including logging all types of unsupported encryption
contexts rather than just ones where the encryption modes are invalid.

Patch 4 changes the error code for "missing crypto API support" from
ENOENT to ENOPKG, to avoid an ambiguity.  This is a logically separate
change, but it's in this series to avoid conflicts.

Eric Biggers (4):
  fscrypt: make fscrypt_msg() take inode instead of super_block
  fscrypt: improve warning messages for unsupported encryption contexts
  fscrypt: improve warnings for missing crypto API support
  fscrypt: use ENOPKG when crypto API support missing

 fs/crypto/crypto.c          | 13 ++++----
 fs/crypto/fname.c           |  8 ++---
 fs/crypto/fscrypt_private.h | 10 +++---
 fs/crypto/hooks.c           |  6 ++--
 fs/crypto/keyinfo.c         | 61 +++++++++++++++++++++++++------------
 5 files changed, 57 insertions(+), 41 deletions(-)

-- 
2.22.0.657.g960e92d24f-goog

[PATCH 1/4] fscrypt: make fscrypt_msg() take inode instead of super_block

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Most of the warning and error messages in fs/crypto/ are for situations
related to a specific inode, not merely to a super_block.  So to make
things easier, make fscrypt_msg() take an inode rather than a
super_block, and make it print the inode number.

Note: This is the same approach I'm taking for fsverity_msg().

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/crypto.c          | 13 ++++++-------
 fs/crypto/fname.c           |  8 ++------
 fs/crypto/fscrypt_private.h | 10 +++++-----
 fs/crypto/hooks.c           |  6 +++---
 fs/crypto/keyinfo.c         | 26 ++++++++++++--------------
 5 files changed, 28 insertions(+), 35 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index d52c788b723d01..3e4624cfe4b54d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -188,10 +188,8 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
 		res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
 	skcipher_request_free(req);
 	if (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);
+		fscrypt_err(inode, "%scryption failed for block %llu: %d",
+			    (rw == FS_DECRYPT ? "De" : "En"), lblk_num, res);
 		return res;
 	}
 	return 0;
@@ -453,7 +451,7 @@ int fscrypt_initialize(unsigned int cop_flags)
 	return res;
 }
 
-void fscrypt_msg(struct super_block *sb, const char *level,
+void fscrypt_msg(const struct inode *inode, const char *level,
 		 const char *fmt, ...)
 {
 	static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
@@ -467,8 +465,9 @@ void fscrypt_msg(struct super_block *sb, const char *level,
 	va_start(args, fmt);
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	if (sb)
-		printk("%sfscrypt (%s): %pV\n", level, sb->s_id, &vaf);
+	if (inode)
+		printk("%sfscrypt (%s, inode %lu): %pV\n",
+		       level, inode->i_sb->s_id, inode->i_ino, &vaf);
 	else
 		printk("%sfscrypt: %pV\n", level, &vaf);
 	va_end(args);
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 6f9daba991684e..5cab3bb2d1fc00 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -71,9 +71,7 @@ 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) {
-		fscrypt_err(inode->i_sb,
-			    "Filename encryption failed for inode %lu: %d",
-			    inode->i_ino, res);
+		fscrypt_err(inode, "Filename encryption failed: %d", res);
 		return res;
 	}
 
@@ -117,9 +115,7 @@ static int fname_decrypt(struct inode *inode,
 	res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
 	skcipher_request_free(req);
 	if (res < 0) {
-		fscrypt_err(inode->i_sb,
-			    "Filename decryption failed for inode %lu: %d",
-			    inode->i_ino, res);
+		fscrypt_err(inode, "Filename decryption failed: %d", res);
 		return res;
 	}
 
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 224178294371a4..4d715708c6e1f7 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -125,12 +125,12 @@ extern struct page *fscrypt_alloc_bounce_page(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, ...);
+fscrypt_msg(const struct inode *inode, 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__)
+#define fscrypt_warn(inode, fmt, ...)		\
+	fscrypt_msg((inode), KERN_WARNING, fmt, ##__VA_ARGS__)
+#define fscrypt_err(inode, fmt, ...)		\
+	fscrypt_msg((inode), KERN_ERR, fmt, ##__VA_ARGS__)
 
 #define FSCRYPT_MAX_IV_SIZE	32
 
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index c1d6715d88e93c..bb3b7fcfdd48a3 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -39,9 +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)) {
-		fscrypt_warn(inode->i_sb,
-			     "inconsistent encryption contexts: %lu/%lu",
-			     d_inode(dir)->i_ino, inode->i_ino);
+		fscrypt_warn(inode,
+			     "Inconsistent encryption context (parent directory: %lu)",
+			     d_inode(dir)->i_ino);
 		err = -EPERM;
 	}
 	dput(dir);
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 9bcadc09e2aded..d0eb901a6d1a9f 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -166,10 +166,9 @@ static struct fscrypt_mode *
 select_encryption_mode(const struct fscrypt_info *ci, const struct inode *inode)
 {
 	if (!fscrypt_valid_enc_modes(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);
+		fscrypt_warn(inode,
+			     "Unsupported encryption modes (contents mode %d, filenames mode %d)",
+			     ci->ci_data_mode, ci->ci_filename_mode);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -206,14 +205,14 @@ static int find_and_derive_key(const struct inode *inode,
 
 	if (ctx->flags & FS_POLICY_FLAG_DIRECT_KEY) {
 		if (mode->ivsize < offsetofend(union fscrypt_iv, nonce)) {
-			fscrypt_warn(inode->i_sb,
-				     "direct key mode not allowed with %s",
+			fscrypt_warn(inode,
+				     "Direct key mode not allowed with %s",
 				     mode->friendly_name);
 			err = -EINVAL;
 		} else if (ctx->contents_encryption_mode !=
 			   ctx->filenames_encryption_mode) {
-			fscrypt_warn(inode->i_sb,
-				     "direct key mode not allowed with different contents and filenames modes");
+			fscrypt_warn(inode,
+				     "Direct key mode not allowed with different contents and filenames modes");
 			err = -EINVAL;
 		} else {
 			memcpy(derived_key, payload->raw, mode->keysize);
@@ -238,9 +237,8 @@ allocate_skcipher_for_mode(struct fscrypt_mode *mode, const u8 *raw_key,
 
 	tfm = crypto_alloc_skcipher(mode->cipher_str, 0, 0);
 	if (IS_ERR(tfm)) {
-		fscrypt_warn(inode->i_sb,
-			     "error allocating '%s' transform for inode %lu: %ld",
-			     mode->cipher_str, inode->i_ino, PTR_ERR(tfm));
+		fscrypt_warn(inode, "Error allocating '%s' transform: %ld",
+			     mode->cipher_str, PTR_ERR(tfm));
 		return tfm;
 	}
 	if (unlikely(!mode->logged_impl_name)) {
@@ -471,9 +469,9 @@ static int setup_crypto_transform(struct fscrypt_info *ci,
 
 		err = init_essiv_generator(ci, raw_key, mode->keysize);
 		if (err) {
-			fscrypt_warn(inode->i_sb,
-				     "error initializing ESSIV generator for inode %lu: %d",
-				     inode->i_ino, err);
+			fscrypt_warn(inode,
+				     "Error initializing ESSIV generator: %d",
+				     err);
 			return err;
 		}
 	}
-- 
2.22.0.657.g960e92d24f-goog

[PATCH 2/4] fscrypt: improve warning messages for unsupported encryption contexts

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

When fs/crypto/ encounters an inode with an invalid encryption context,
currently it prints a warning if the pair of encryption modes are
unrecognized, but it's silent if there are other problems such as
unsupported context size, format, or flags.  To help people debug such
situations, add more warning messages.

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

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index d0eb901a6d1a9f..e5ab18d98f32a3 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -510,8 +510,12 @@ int fscrypt_get_encryption_info(struct inode *inode)
 	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
 	if (res < 0) {
 		if (!fscrypt_dummy_context_enabled(inode) ||
-		    IS_ENCRYPTED(inode))
+		    IS_ENCRYPTED(inode)) {
+			fscrypt_warn(inode,
+				     "Error %d getting encryption context",
+				     res);
 			return res;
+		}
 		/* Fake up a context for an unencrypted directory */
 		memset(&ctx, 0, sizeof(ctx));
 		ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
@@ -519,14 +523,22 @@ int fscrypt_get_encryption_info(struct inode *inode)
 		ctx.filenames_encryption_mode = FS_ENCRYPTION_MODE_AES_256_CTS;
 		memset(ctx.master_key_descriptor, 0x42, FS_KEY_DESCRIPTOR_SIZE);
 	} else if (res != sizeof(ctx)) {
+		fscrypt_warn(inode,
+			     "Unknown encryption context size (%d bytes)", res);
 		return -EINVAL;
 	}
 
-	if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
+	if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1) {
+		fscrypt_warn(inode, "Unknown encryption context version (%d)",
+			     ctx.format);
 		return -EINVAL;
+	}
 
-	if (ctx.flags & ~FS_POLICY_FLAGS_VALID)
+	if (ctx.flags & ~FS_POLICY_FLAGS_VALID) {
+		fscrypt_warn(inode, "Unknown encryption context flags (0x%02x)",
+			     ctx.flags);
 		return -EINVAL;
+	}
 
 	crypt_info = kmem_cache_zalloc(fscrypt_info_cachep, GFP_NOFS);
 	if (!crypt_info)
-- 
2.22.0.657.g960e92d24f-goog

[PATCH 3/4] fscrypt: improve warnings for missing crypto API support

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Users of fscrypt with non-default algorithms will encounter an error
like the following if they fail to include the needed algorithms into
the crypto API when configuring the kernel (as per the documentation):

    Error allocating 'adiantum(xchacha12,aes)' transform: -2

This requires that the user figure out what the "-2" error means.
Make it more friendly by printing a warning like the following instead:

    Missing crypto API support for Adiantum (API name: "adiantum(xchacha12,aes)")

Also upgrade the log level for *other* errors to KERN_ERR.

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

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index e5ab18d98f32a3..b75678587c3a85 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -237,8 +237,13 @@ allocate_skcipher_for_mode(struct fscrypt_mode *mode, const u8 *raw_key,
 
 	tfm = crypto_alloc_skcipher(mode->cipher_str, 0, 0);
 	if (IS_ERR(tfm)) {
-		fscrypt_warn(inode, "Error allocating '%s' transform: %ld",
-			     mode->cipher_str, PTR_ERR(tfm));
+		if (PTR_ERR(tfm) == -ENOENT)
+			fscrypt_warn(inode,
+				     "Missing crypto API support for %s (API name: \"%s\")",
+				     mode->friendly_name, mode->cipher_str);
+		else
+			fscrypt_err(inode, "Error allocating '%s' transform: %ld",
+				    mode->cipher_str, PTR_ERR(tfm));
 		return tfm;
 	}
 	if (unlikely(!mode->logged_impl_name)) {
@@ -384,9 +389,13 @@ static int derive_essiv_salt(const u8 *key, int keysize, u8 *salt)
 
 		tfm = crypto_alloc_shash("sha256", 0, 0);
 		if (IS_ERR(tfm)) {
-			fscrypt_warn(NULL,
-				     "error allocating SHA-256 transform: %ld",
-				     PTR_ERR(tfm));
+			if (PTR_ERR(tfm) == -ENOENT)
+				fscrypt_warn(NULL,
+					     "Missing crypto API support for SHA-256");
+			else
+				fscrypt_err(NULL,
+					    "Error allocating SHA-256 transform: %ld",
+					    PTR_ERR(tfm));
 			return PTR_ERR(tfm);
 		}
 		prev_tfm = cmpxchg(&essiv_hash_tfm, NULL, tfm);
-- 
2.22.0.657.g960e92d24f-goog

[PATCH 4/4] fscrypt: use ENOPKG when crypto API support missing

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Return ENOPKG rather than ENOENT when trying to open a file that's
encrypted using algorithms not available in the kernel's crypto API.

This avoids an ambiguity, since ENOENT is also returned when the file
doesn't exist.

Note: this is the same approach I'm taking for fs-verity.

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

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index b75678587c3a85..2129943002335c 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -237,13 +237,14 @@ allocate_skcipher_for_mode(struct fscrypt_mode *mode, const u8 *raw_key,
 
 	tfm = crypto_alloc_skcipher(mode->cipher_str, 0, 0);
 	if (IS_ERR(tfm)) {
-		if (PTR_ERR(tfm) == -ENOENT)
+		if (PTR_ERR(tfm) == -ENOENT) {
 			fscrypt_warn(inode,
 				     "Missing crypto API support for %s (API name: \"%s\")",
 				     mode->friendly_name, mode->cipher_str);
-		else
-			fscrypt_err(inode, "Error allocating '%s' transform: %ld",
-				    mode->cipher_str, PTR_ERR(tfm));
+			return ERR_PTR(-ENOPKG);
+		}
+		fscrypt_err(inode, "Error allocating '%s' transform: %ld",
+			    mode->cipher_str, PTR_ERR(tfm));
 		return tfm;
 	}
 	if (unlikely(!mode->logged_impl_name)) {
@@ -389,13 +390,14 @@ static int derive_essiv_salt(const u8 *key, int keysize, u8 *salt)
 
 		tfm = crypto_alloc_shash("sha256", 0, 0);
 		if (IS_ERR(tfm)) {
-			if (PTR_ERR(tfm) == -ENOENT)
+			if (PTR_ERR(tfm) == -ENOENT) {
 				fscrypt_warn(NULL,
 					     "Missing crypto API support for SHA-256");
-			else
-				fscrypt_err(NULL,
-					    "Error allocating SHA-256 transform: %ld",
-					    PTR_ERR(tfm));
+				return -ENOPKG;
+			}
+			fscrypt_err(NULL,
+				    "Error allocating SHA-256 transform: %ld",
+				    PTR_ERR(tfm));
 			return PTR_ERR(tfm);
 		}
 		prev_tfm = cmpxchg(&essiv_hash_tfm, NULL, tfm);
-- 
2.22.0.657.g960e92d24f-goog

Re: [PATCH 0/4] fscrypt: logging improvements, and use ENOPKG

[Eric Biggers]

On Wed, Jul 24, 2019 at 12:54:18PM -0700, Eric Biggers wrote:
> Patches 1-3 make some small improvements to warning and error messages
> in fs/crypto/, including logging all types of unsupported encryption
> contexts rather than just ones where the encryption modes are invalid.
> 
> Patch 4 changes the error code for "missing crypto API support" from
> ENOENT to ENOPKG, to avoid an ambiguity.  This is a logically separate
> change, but it's in this series to avoid conflicts.
> 
> Eric Biggers (4):
>   fscrypt: make fscrypt_msg() take inode instead of super_block
>   fscrypt: improve warning messages for unsupported encryption contexts
>   fscrypt: improve warnings for missing crypto API support
>   fscrypt: use ENOPKG when crypto API support missing
> 
>  fs/crypto/crypto.c          | 13 ++++----
>  fs/crypto/fname.c           |  8 ++---
>  fs/crypto/fscrypt_private.h | 10 +++---
>  fs/crypto/hooks.c           |  6 ++--
>  fs/crypto/keyinfo.c         | 61 +++++++++++++++++++++++++------------
>  5 files changed, 57 insertions(+), 41 deletions(-)
> 
> -- 
> 2.22.0.657.g960e92d24f-goog
> 

All applied to fscrypt tree for 5.4.

- Eric