[RFC PATCH 00/14] ceph+fscrypt: together at last (contexts and filenames)

[RFC PATCH 00/14] ceph+fscrypt: together at last (contexts and filenames)

[Jeff Layton]

This is a (very rough and incomplete) draft patchset that I've been
working on to add fscrypt support to cephfs. The main use case is being
able to allow encryption at the edges, without having to trust your storage
provider with keys.

Implementing fscrypt on a network filesystem has some challenges that
you don't have to deal with on a local fs:

Ceph (and most other netfs') will need to pre-create a crypto context
when creating a new inode as we'll need to encrypt some things before we
have an inode. This patchset stores contexts in an xattr, but that's
probably not ideal for the final implementation [1].

Storing a binary crypttext filename on the MDS (or most network
fileservers) may be problematic. We'll probably end up having to base64
encode the names when storing them. I expect most network filesystems to
have similar issues. That may limit the effective NAME_MAX for some
filesystems [2].

For content encryption, Ceph (and probably at least CIFS/SMB) will need
to deal with writes not aligned on crypto blocks. These filesystems
sometimes write synchronously to the server instead of going through the
pagecache [3].

Symlink handling in fscrypt will also need to be refactored a bit, as we
won't have an inode before we'll need to encrypt its contents.

This draft is _very_ rough and not ready for merge. This only covers the
context handling and filename encryption. It's missing a lot of stuff
still, but what's there basically works.

I'm mostly posting this now to get some early feedback on the basic idea
and approach. In particular, I'd appreciate some feedback from the
fscrypt maintainers. Please let me know if any of the changes I'm
proposing there look problematic.

Thanks for looking!
-- Jeff

[1]: We'll likely add a dedicated field to the standard on-the-wire
inode representation and in the MDS, but that's a separate sub-project.

[2]: For ceph, it looks like it's not a huge problem as the MDS doesn't
enforce filename lengths at all. Still, we may extend the protocol to
handle that better.

[3]: For Ceph, I think we'll be able to do a CMPEXT op to do a
read/modify/write-if-nothing-changed cycle for this case.

Jeff Layton (14):
  fscrypt: drop unused inode argument from fscrypt_fname_alloc_buffer
  fscrypt: add fscrypt_new_context_from_parent
  fscrypt: don't balk when inode is already marked encrypted
  fscrypt: export fscrypt_d_revalidate
  lib: lift fscrypt base64 conversion into lib/
  ceph: add fscrypt ioctls
  ceph: crypto context handling for ceph
  ceph: add routine to create context prior to RPC
  ceph: set S_ENCRYPTED bit if new inode has encryption.ctx xattr
  ceph: make ceph_msdc_build_path use ref-walk
  ceph: add encrypted fname handling to ceph_mdsc_build_path
  ceph: make d_revalidate call fscrypt revalidator for encrypted
    dentries
  ceph: add support to readdir for encrypted filenames
  ceph: add fscrypt support to ceph_fill_trace

 fs/ceph/Makefile        |   1 +
 fs/ceph/crypto.c        | 171 ++++++++++++++++++++++++++++++++++++++++
 fs/ceph/crypto.h        |  81 +++++++++++++++++++
 fs/ceph/dir.c           |  97 ++++++++++++++++++++---
 fs/ceph/file.c          |   4 +
 fs/ceph/inode.c         |  62 +++++++++++++--
 fs/ceph/ioctl.c         |  26 ++++++
 fs/ceph/mds_client.c    |  74 ++++++++++++-----
 fs/ceph/super.c         |  37 +++++++++
 fs/ceph/super.h         |  11 ++-
 fs/ceph/xattr.c         |  32 ++++++++
 fs/crypto/Kconfig       |   1 +
 fs/crypto/fname.c       |  67 +---------------
 fs/crypto/hooks.c       |   2 +-
 fs/crypto/keysetup.c    |   2 +-
 fs/crypto/policy.c      |  42 +++++++---
 fs/ext4/dir.c           |   2 +-
 fs/ext4/namei.c         |   7 +-
 fs/f2fs/dir.c           |   2 +-
 fs/ubifs/dir.c          |   2 +-
 include/linux/base64.h  |  11 +++
 include/linux/fscrypt.h |  12 ++-
 lib/Kconfig             |   3 +
 lib/Makefile            |   1 +
 lib/base64.c            |  71 +++++++++++++++++
 25 files changed, 696 insertions(+), 125 deletions(-)
 create mode 100644 fs/ceph/crypto.c
 create mode 100644 fs/ceph/crypto.h
 create mode 100644 include/linux/base64.h
 create mode 100644 lib/base64.c

-- 
2.26.2

[RFC PATCH 01/14] fscrypt: drop unused inode argument from fscrypt_fname_alloc_buffer

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/crypto/fname.c       | 5 +----
 fs/crypto/hooks.c       | 2 +-
 fs/ext4/dir.c           | 2 +-
 fs/ext4/namei.c         | 7 +++----
 fs/f2fs/dir.c           | 2 +-
 fs/ubifs/dir.c          | 2 +-
 include/linux/fscrypt.h | 5 ++---
 7 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 011830f84d8d..47bcfddb278b 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -260,8 +260,6 @@ bool fscrypt_fname_encrypted_size(const struct inode *inode, u32 orig_len,
 
 /**
  * fscrypt_fname_alloc_buffer() - allocate a buffer for presented filenames
- * @inode: inode of the parent directory (for regular filenames)
- *	   or of the symlink (for symlink targets)
  * @max_encrypted_len: maximum length of encrypted filenames the buffer will be
  *		       used to present
  * @crypto_str: (output) buffer to allocate
@@ -271,8 +269,7 @@ bool fscrypt_fname_encrypted_size(const struct inode *inode, u32 orig_len,
  *
  * Return: 0 on success, -errno on failure
  */
-int fscrypt_fname_alloc_buffer(const struct inode *inode,
-			       u32 max_encrypted_len,
+int fscrypt_fname_alloc_buffer(u32 max_encrypted_len,
 			       struct fscrypt_str *crypto_str)
 {
 	const u32 max_encoded_len = BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX);
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 09fb8aa0f2e9..491b252843eb 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -319,7 +319,7 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 	if (cstr.len + sizeof(*sd) - 1 > max_size)
 		return ERR_PTR(-EUCLEAN);
 
-	err = fscrypt_fname_alloc_buffer(inode, cstr.len, &pstr);
+	err = fscrypt_fname_alloc_buffer(cstr.len, &pstr);
 	if (err)
 		return ERR_PTR(err);
 
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 1d82336b1cd4..efe77cffc322 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -148,7 +148,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
 	}
 
 	if (IS_ENCRYPTED(inode)) {
-		err = fscrypt_fname_alloc_buffer(inode, EXT4_NAME_LEN, &fstr);
+		err = fscrypt_fname_alloc_buffer(EXT4_NAME_LEN, &fstr);
 		if (err < 0)
 			return err;
 	}
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 56738b538ddf..f41c8bfe8b5a 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -663,8 +663,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 
 					/* Directory is encrypted */
 					res = fscrypt_fname_alloc_buffer(
-						dir, len,
-						&fname_crypto_str);
+						len, &fname_crypto_str);
 					if (res)
 						printk(KERN_WARNING "Error "
 							"allocating crypto "
@@ -1016,8 +1015,8 @@ static int htree_dirblock_to_tree(struct file *dir_file,
 			brelse(bh);
 			return err;
 		}
-		err = fscrypt_fname_alloc_buffer(dir, EXT4_NAME_LEN,
-						     &fname_crypto_str);
+		err = fscrypt_fname_alloc_buffer(EXT4_NAME_LEN,
+						 &fname_crypto_str);
 		if (err < 0) {
 			brelse(bh);
 			return err;
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 069f498af1e3..b2530b9507bd 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -1032,7 +1032,7 @@ static int f2fs_readdir(struct file *file, struct dir_context *ctx)
 		if (err)
 			goto out;
 
-		err = fscrypt_fname_alloc_buffer(inode, F2FS_NAME_LEN, &fstr);
+		err = fscrypt_fname_alloc_buffer(F2FS_NAME_LEN, &fstr);
 		if (err < 0)
 			goto out;
 	}
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 9d042942d8b2..a9c1f5a9c9bd 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -515,7 +515,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 		if (err)
 			return err;
 
-		err = fscrypt_fname_alloc_buffer(dir, UBIFS_MAX_NLEN, &fstr);
+		err = fscrypt_fname_alloc_buffer(UBIFS_MAX_NLEN, &fstr);
 		if (err)
 			return err;
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 991ff8575d0e..eaf16eb55788 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -197,7 +197,7 @@ static inline void fscrypt_free_filename(struct fscrypt_name *fname)
 	kfree(fname->crypto_buf.name);
 }
 
-int fscrypt_fname_alloc_buffer(const struct inode *inode, u32 max_encrypted_len,
+int fscrypt_fname_alloc_buffer(u32 max_encrypted_len,
 			       struct fscrypt_str *crypto_str);
 void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str);
 int fscrypt_fname_disk_to_usr(const struct inode *inode,
@@ -428,8 +428,7 @@ static inline void fscrypt_free_filename(struct fscrypt_name *fname)
 	return;
 }
 
-static inline int fscrypt_fname_alloc_buffer(const struct inode *inode,
-					     u32 max_encrypted_len,
+static inline int fscrypt_fname_alloc_buffer(u32 max_encrypted_len,
 					     struct fscrypt_str *crypto_str)
 {
 	return -EOPNOTSUPP;
-- 
2.26.2

[RFC PATCH 02/14] fscrypt: add fscrypt_new_context_from_parent

[Jeff Layton]

Network filesystems usually don't know what inode number they'll have
until after the reply to a create RPC comes in. But we do need a
(blank) crypto context before that point. Break out the part of
fscrypt_inherit_context that creates a new context without setting it
into the new inode just yet.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/crypto/policy.c      | 42 ++++++++++++++++++++++++++++++-----------
 include/linux/fscrypt.h |  6 ++++++
 2 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 2d73fd39ad96..13e0a50157d5 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -621,19 +621,19 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 EXPORT_SYMBOL(fscrypt_has_permitted_context);
 
 /**
- * fscrypt_inherit_context() - Sets a child context from its parent
+ * fscrypt_new_context_from_parent() - generate a new fscrypt context from its parent
  * @parent: Parent inode from which the context is inherited.
- * @child:  Child inode that inherits the context from @parent.
- * @fs_data:  private data given by FS.
- * @preload:  preload child i_crypt_info if true
+ * @ctx: destination for new encryption context
  *
- * Return: 0 on success, -errno on failure
+ * Generate a new encryption context for an inode from its parent, suitable
+ * for later installation into a child dentry's inode. Returns positive
+ * length of the new context on success, or -errno on failure.
+ *
+ * Note that the caller must ensure that it provides sufficient buffer space
+ * to write out the context (at least FSCRYPT_SET_CONTEXT_MAX_SIZE bytes).
  */
-int fscrypt_inherit_context(struct inode *parent, struct inode *child,
-						void *fs_data, bool preload)
+int fscrypt_new_context_from_parent(struct inode *parent, void *ctx)
 {
-	union fscrypt_context ctx;
-	int ctxsize;
 	struct fscrypt_info *ci;
 	int res;
 
@@ -645,10 +645,30 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 	if (ci == NULL)
 		return -ENOKEY;
 
-	ctxsize = fscrypt_new_context_from_policy(&ctx, &ci->ci_policy);
+	return fscrypt_new_context_from_policy(ctx, &ci->ci_policy);
+}
+EXPORT_SYMBOL(fscrypt_new_context_from_parent);
+
+/**
+ * fscrypt_inherit_context() - Sets a child context from its parent
+ * @parent: Parent inode from which the context is inherited.
+ * @child:  Child inode that inherits the context from @parent.
+ * @fs_data:  private data given by FS.
+ * @preload:  preload child i_crypt_info if true
+ *
+ * Return: 0 on success, -errno on failure
+ */
+int fscrypt_inherit_context(struct inode *parent, struct inode *child,
+						void *fs_data, bool preload)
+{
+	union fscrypt_context ctx;
+	int res;
 
 	BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
-	res = parent->i_sb->s_cop->set_context(child, &ctx, ctxsize, fs_data);
+	res = fscrypt_new_context_from_parent(parent, &ctx);
+	if (res < 0)
+		return res;
+	res = parent->i_sb->s_cop->set_context(child, &ctx, res, fs_data);
 	if (res)
 		return res;
 	return preload ? fscrypt_get_encryption_info(child): 0;
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index eaf16eb55788..c5fe6f334a1d 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -156,6 +156,7 @@ int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg);
 int fscrypt_ioctl_get_policy_ex(struct file *filp, void __user *arg);
 int fscrypt_ioctl_get_nonce(struct file *filp, void __user *arg);
 int fscrypt_has_permitted_context(struct inode *parent, struct inode *child);
+int fscrypt_new_context_from_parent(struct inode *parent, void *);
 int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 			    void *fs_data, bool preload);
 
@@ -340,6 +341,11 @@ static inline int fscrypt_has_permitted_context(struct inode *parent,
 	return 0;
 }
 
+static inline int fscrypt_new_context_from_parent(struct inode *parent, void *ctx);
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int fscrypt_inherit_context(struct inode *parent,
 					  struct inode *child,
 					  void *fs_data, bool preload)
-- 
2.26.2

[RFC PATCH 03/14] fscrypt: don't balk when inode is already marked encrypted

[Jeff Layton]

Cephfs (currently) sets this flag early and only fetches the context
later. Eventually we may not need this, but for now it prevents this
warning from popping.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/crypto/keysetup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index fea6226afc2b..587335dc7cb9 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -474,7 +474,7 @@ int fscrypt_get_encryption_info(struct inode *inode)
 		const union fscrypt_context *dummy_ctx =
 			fscrypt_get_dummy_context(inode->i_sb);
 
-		if (IS_ENCRYPTED(inode) || !dummy_ctx) {
+		if (!dummy_ctx) {
 			fscrypt_warn(inode,
 				     "Error %d getting encryption context",
 				     res);
-- 
2.26.2

[RFC PATCH 04/14] fscrypt: export fscrypt_d_revalidate

[Jeff Layton]

ceph already has its own d_revalidate op so we can't rely on fscrypt
using that directly. Export this symbol so filesystems can call it
from their own d_revalidate op.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/crypto/fname.c       | 3 ++-
 include/linux/fscrypt.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 47bcfddb278b..9440a44e24ac 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -538,7 +538,7 @@ EXPORT_SYMBOL_GPL(fscrypt_fname_siphash);
  * Validate dentries in encrypted directories to make sure we aren't potentially
  * caching stale dentries after a key has been added.
  */
-static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
+int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 {
 	struct dentry *dir;
 	int err;
@@ -577,6 +577,7 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 
 	return valid;
 }
+EXPORT_SYMBOL_GPL(fscrypt_d_revalidate);
 
 const struct dentry_operations fscrypt_d_ops = {
 	.d_revalidate = fscrypt_d_revalidate,
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index c5fe6f334a1d..41300eac45a4 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -208,6 +208,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
 bool fscrypt_match_name(const struct fscrypt_name *fname,
 			const u8 *de_name, u32 de_name_len);
 u64 fscrypt_fname_siphash(const struct inode *dir, const struct qstr *name);
+int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags);
 
 /* bio.c */
 void fscrypt_decrypt_bio(struct bio *bio);
-- 
2.26.2

[RFC PATCH 05/14] lib: lift fscrypt base64 conversion into lib/

[Jeff Layton]

Once we allow encrypted filenames we'll end up with names that may have
illegal characters in them (embedded '\0' or '/'), or characters that
aren't printable.

It'll be safer to use strings that are printable. It turns out that the
MDS doesn't really care about the length of filenames, so we can just
base64 encode and decode filenames before writing and reading them.

Lift the base64 implementation that's in fscrypt into lib/. Make fscrypt
select it when it's enabled.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/crypto/Kconfig      |  1 +
 fs/crypto/fname.c      | 59 +----------------------------------
 include/linux/base64.h | 11 +++++++
 lib/Kconfig            |  3 ++
 lib/Makefile           |  1 +
 lib/base64.c           | 71 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 88 insertions(+), 58 deletions(-)
 create mode 100644 include/linux/base64.h
 create mode 100644 lib/base64.c

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index a5f5c30368a2..49219017f7e9 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -6,6 +6,7 @@ config FS_ENCRYPTION
 	select CRYPTO_SKCIPHER
 	select CRYPTO_LIB_SHA256
 	select KEYS
+	select BASE64
 	help
 	  Enable encryption of files and directories.  This
 	  feature is similar to ecryptfs, but it is more memory
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 9440a44e24ac..6a6bbe8f8db7 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -11,6 +11,7 @@
  * This has not yet undergone a rigorous security audit.
  */
 
+#include <linux/base64.h>
 #include <linux/namei.h>
 #include <linux/scatterlist.h>
 #include <crypto/hash.h>
@@ -184,64 +185,6 @@ static int fname_decrypt(const struct inode *inode,
 	return 0;
 }
 
-static const char lookup_table[65] =
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
-
-#define BASE64_CHARS(nbytes)	DIV_ROUND_UP((nbytes) * 4, 3)
-
-/**
- * base64_encode() - base64-encode some bytes
- * @src: the bytes to encode
- * @len: number of bytes to encode
- * @dst: (output) the base64-encoded string.  Not NUL-terminated.
- *
- * Encodes the input string using characters from the set [A-Za-z0-9+,].
- * The encoded string is roughly 4/3 times the size of the input string.
- *
- * Return: length of the encoded string
- */
-static int base64_encode(const u8 *src, int len, char *dst)
-{
-	int i, bits = 0, ac = 0;
-	char *cp = dst;
-
-	for (i = 0; i < len; i++) {
-		ac += src[i] << bits;
-		bits += 8;
-		do {
-			*cp++ = lookup_table[ac & 0x3f];
-			ac >>= 6;
-			bits -= 6;
-		} while (bits >= 6);
-	}
-	if (bits)
-		*cp++ = lookup_table[ac & 0x3f];
-	return cp - dst;
-}
-
-static int base64_decode(const char *src, int len, u8 *dst)
-{
-	int i, bits = 0, ac = 0;
-	const char *p;
-	u8 *cp = dst;
-
-	for (i = 0; i < len; i++) {
-		p = strchr(lookup_table, src[i]);
-		if (p == NULL || src[i] == 0)
-			return -2;
-		ac += (p - lookup_table) << bits;
-		bits += 6;
-		if (bits >= 8) {
-			*cp++ = ac & 0xff;
-			ac >>= 8;
-			bits -= 8;
-		}
-	}
-	if (ac)
-		return -1;
-	return cp - dst;
-}
-
 bool fscrypt_fname_encrypted_size(const struct inode *inode, u32 orig_len,
 				  u32 max_len, u32 *encrypted_len_ret)
 {
diff --git a/include/linux/base64.h b/include/linux/base64.h
new file mode 100644
index 000000000000..bde7a936ed21
--- /dev/null
+++ b/include/linux/base64.h
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#ifndef _LIB_BASE64_H
+#define _LIB_BASE64_H
+#include <linux/types.h>
+
+#define BASE64_CHARS(nbytes)	DIV_ROUND_UP((nbytes) * 4, 3)
+
+int base64_encode(const u8 *src, int len, char *dst);
+int base64_decode(const char *src, int len, u8 *dst);
+#endif /* _LIB_BASE64_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index b4b98a03ff98..507e851d220b 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -684,3 +684,6 @@ config GENERIC_LIB_UCMPDI2
 config PLDMFW
 	bool
 	default n
+
+config BASE64
+	tristate
diff --git a/lib/Makefile b/lib/Makefile
index e290fc5707ea..4d76755e609c 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -166,6 +166,7 @@ obj-$(CONFIG_LIBCRC32C)	+= libcrc32c.o
 obj-$(CONFIG_CRC8)	+= crc8.o
 obj-$(CONFIG_XXHASH)	+= xxhash.o
 obj-$(CONFIG_GENERIC_ALLOCATOR) += genalloc.o
+obj-$(CONFIG_BASE64) += base64.o
 
 obj-$(CONFIG_842_COMPRESS) += 842/
 obj-$(CONFIG_842_DECOMPRESS) += 842/
diff --git a/lib/base64.c b/lib/base64.c
new file mode 100644
index 000000000000..5f1480da6559
--- /dev/null
+++ b/lib/base64.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * base64 encode/decode functions
+ *
+ * Originally lifted from fs/crypto/fname.c
+ *
+ * Copyright (C) 2015, Jaegeuk Kim
+ * Copyright (C) 2015, Eric Biggers
+ */
+
+#include <linux/types.h>
+#include <linux/export.h>
+#include <linux/string.h>
+
+static const char lookup_table[65] =
+	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
+
+/**
+ * base64_encode() - base64-encode some bytes
+ * @src: the bytes to encode
+ * @len: number of bytes to encode
+ * @dst: (output) the base64-encoded string.  Not NUL-terminated.
+ *
+ * Encodes the input string using characters from the set [A-Za-z0-9+,].
+ * The encoded string is roughly 4/3 times the size of the input string.
+ *
+ * Return: length of the encoded string
+ */
+int base64_encode(const u8 *src, int len, char *dst)
+{
+	int i, bits = 0, ac = 0;
+	char *cp = dst;
+
+	for (i = 0; i < len; i++) {
+		ac += src[i] << bits;
+		bits += 8;
+		do {
+			*cp++ = lookup_table[ac & 0x3f];
+			ac >>= 6;
+			bits -= 6;
+		} while (bits >= 6);
+	}
+	if (bits)
+		*cp++ = lookup_table[ac & 0x3f];
+	return cp - dst;
+}
+EXPORT_SYMBOL(base64_encode);
+
+int base64_decode(const char *src, int len, u8 *dst)
+{
+	int i, bits = 0, ac = 0;
+	const char *p;
+	u8 *cp = dst;
+
+	for (i = 0; i < len; i++) {
+		p = strchr(lookup_table, src[i]);
+		if (p == NULL || src[i] == 0)
+			return -2;
+		ac += (p - lookup_table) << bits;
+		bits += 6;
+		if (bits >= 8) {
+			*cp++ = ac & 0xff;
+			ac >>= 8;
+			bits -= 8;
+		}
+	}
+	if (ac)
+		return -1;
+	return cp - dst;
+}
+EXPORT_SYMBOL(base64_decode);
-- 
2.26.2

Re: [RFC PATCH 05/14] lib: lift fscrypt base64 conversion into lib/

[Eric Biggers]

On Fri, Aug 21, 2020 at 02:28:04PM -0400, Jeff Layton wrote:
> Once we allow encrypted filenames we'll end up with names that may have
> illegal characters in them (embedded '\0' or '/'), or characters that
> aren't printable.
> 
> It'll be safer to use strings that are printable. It turns out that the
> MDS doesn't really care about the length of filenames, so we can just
> base64 encode and decode filenames before writing and reading them.
> 
> Lift the base64 implementation that's in fscrypt into lib/. Make fscrypt
> select it when it's enabled.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/crypto/Kconfig      |  1 +
>  fs/crypto/fname.c      | 59 +----------------------------------
>  include/linux/base64.h | 11 +++++++
>  lib/Kconfig            |  3 ++
>  lib/Makefile           |  1 +
>  lib/base64.c           | 71 ++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 88 insertions(+), 58 deletions(-)
>  create mode 100644 include/linux/base64.h
>  create mode 100644 lib/base64.c

You need to be careful here because there are many subtly different variants of
base64.  The Wikipedia article is a good reference for this:
https://en.wikipedia.org/wiki/Base64

For example, most versions of base64 use [A-Za-z0-9+/].  However that's *not*
what fs/crypto/fname.c uses, since it needs the encoded strings to be valid
filenames, and '/' isn't a valid character in filenames.  Therefore,
fs/crypto/fname.c uses ',' instead of '/'.

It happens that's probably what ceph needs too.  However, other kernel
developers who come across a very generic-sounding "lib/base64.c" might expect
it to implement a more common version of base64.

Also, some versions of base64 pad the encoded string with "=" whereas others
don't.  The fs/crypto/fname.c implementation doesn't use padding.

So if you're going to make a generic base64 library, you at least need to be
very clear about exactly what version of base64 is meant.

(FWIW, the existing use of base64 in fs/crypto/fname.c isn't part of a stable
API.  So it can still be changed to something else, as long as the encoding
doesn't use the '/' or '\0' characters.)

- Eric

Re: [RFC PATCH 05/14] lib: lift fscrypt base64 conversion into lib/

[Jeff Layton]

On Fri, 2020-08-21 at 17:38 -0700, Eric Biggers wrote:
> On Fri, Aug 21, 2020 at 02:28:04PM -0400, Jeff Layton wrote:
> > Once we allow encrypted filenames we'll end up with names that may have
> > illegal characters in them (embedded '\0' or '/'), or characters that
> > aren't printable.
> > 
> > It'll be safer to use strings that are printable. It turns out that the
> > MDS doesn't really care about the length of filenames, so we can just
> > base64 encode and decode filenames before writing and reading them.
> > 
> > Lift the base64 implementation that's in fscrypt into lib/. Make fscrypt
> > select it when it's enabled.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/crypto/Kconfig      |  1 +
> >  fs/crypto/fname.c      | 59 +----------------------------------
> >  include/linux/base64.h | 11 +++++++
> >  lib/Kconfig            |  3 ++
> >  lib/Makefile           |  1 +
> >  lib/base64.c           | 71 ++++++++++++++++++++++++++++++++++++++++++
> >  6 files changed, 88 insertions(+), 58 deletions(-)
> >  create mode 100644 include/linux/base64.h
> >  create mode 100644 lib/base64.c
> 
> You need to be careful here because there are many subtly different variants of
> base64.  The Wikipedia article is a good reference for this:
> https://en.wikipedia.org/wiki/Base64
> 
> For example, most versions of base64 use [A-Za-z0-9+/].  However that's *not*
> what fs/crypto/fname.c uses, since it needs the encoded strings to be valid
> filenames, and '/' isn't a valid character in filenames.  Therefore,
> fs/crypto/fname.c uses ',' instead of '/'.
> 
> It happens that's probably what ceph needs too.  However, other kernel
> developers who come across a very generic-sounding "lib/base64.c" might expect
> it to implement a more common version of base64.
> 
> Also, some versions of base64 pad the encoded string with "=" whereas others
> don't.  The fs/crypto/fname.c implementation doesn't use padding.
> 
> So if you're going to make a generic base64 library, you at least need to be
> very clear about exactly what version of base64 is meant.
> 
> (FWIW, the existing use of base64 in fs/crypto/fname.c isn't part of a stable
> API.  So it can still be changed to something else, as long as the encoding
> doesn't use the '/' or '\0' characters.)
> 
> - Eric

Ok, thanks -- that makes sense. We may need to rename this to something
less generic then. I'll plan to do that for the next set.

It may also make more sense to just create fscrypt functions that can
deal with base64-encoded strings instead of binary crypttext.
-- 
Jeff Layton <jlayton@kernel.org>

[RFC PATCH 06/14] ceph: add fscrypt ioctls

[Jeff Layton]

Boilerplate ioctls for controlling encryption.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/ioctl.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
index 6e061bf62ad4..4400b170eca9 100644
--- a/fs/ceph/ioctl.c
+++ b/fs/ceph/ioctl.c
@@ -6,6 +6,7 @@
 #include "mds_client.h"
 #include "ioctl.h"
 #include <linux/ceph/striper.h>
+#include <linux/fscrypt.h>
 
 /*
  * ioctls
@@ -289,6 +290,31 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	case CEPH_IOC_SYNCIO:
 		return ceph_ioctl_syncio(file);
+#ifdef CONFIG_FS_ENCRYPTION
+	case FS_IOC_SET_ENCRYPTION_POLICY:
+		return fscrypt_ioctl_set_policy(file, (const void __user *)arg);
+
+	case FS_IOC_GET_ENCRYPTION_POLICY:
+		return fscrypt_ioctl_get_policy(file, (void __user *)arg);
+
+	case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+		return fscrypt_ioctl_get_policy_ex(file, (void __user *)arg);
+
+	case FS_IOC_ADD_ENCRYPTION_KEY:
+		return fscrypt_ioctl_add_key(file, (void __user *)arg);
+
+	case FS_IOC_REMOVE_ENCRYPTION_KEY:
+		return fscrypt_ioctl_remove_key(file, (void __user *)arg);
+
+	case FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS:
+		return fscrypt_ioctl_remove_key_all_users(file, (void __user *)arg);
+
+	case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
+		return fscrypt_ioctl_get_key_status(file, (void __user *)arg);
+
+	case FS_IOC_GET_ENCRYPTION_NONCE:
+		return fscrypt_ioctl_get_nonce(file, (void __user *)arg);
+#endif /* CONFIG_FS_ENCRYPTION */
 	}
 
 	return -ENOTTY;
-- 
2.26.2

Re: [RFC PATCH 06/14] ceph: add fscrypt ioctls

[Eric Biggers]

On Fri, Aug 21, 2020 at 02:28:05PM -0400, Jeff Layton wrote:
> Boilerplate ioctls for controlling encryption.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/ioctl.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> index 6e061bf62ad4..4400b170eca9 100644
> --- a/fs/ceph/ioctl.c
> +++ b/fs/ceph/ioctl.c
> @@ -6,6 +6,7 @@
>  #include "mds_client.h"
>  #include "ioctl.h"
>  #include <linux/ceph/striper.h>
> +#include <linux/fscrypt.h>
>  
>  /*
>   * ioctls
> @@ -289,6 +290,31 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  
>  	case CEPH_IOC_SYNCIO:
>  		return ceph_ioctl_syncio(file);
> +#ifdef CONFIG_FS_ENCRYPTION
> +	case FS_IOC_SET_ENCRYPTION_POLICY:
> +		return fscrypt_ioctl_set_policy(file, (const void __user *)arg);
> +
> +	case FS_IOC_GET_ENCRYPTION_POLICY:
> +		return fscrypt_ioctl_get_policy(file, (void __user *)arg);
> +
> +	case FS_IOC_GET_ENCRYPTION_POLICY_EX:
> +		return fscrypt_ioctl_get_policy_ex(file, (void __user *)arg);
> +
> +	case FS_IOC_ADD_ENCRYPTION_KEY:
> +		return fscrypt_ioctl_add_key(file, (void __user *)arg);
> +
> +	case FS_IOC_REMOVE_ENCRYPTION_KEY:
> +		return fscrypt_ioctl_remove_key(file, (void __user *)arg);
> +
> +	case FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS:
> +		return fscrypt_ioctl_remove_key_all_users(file, (void __user *)arg);
> +
> +	case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
> +		return fscrypt_ioctl_get_key_status(file, (void __user *)arg);
> +
> +	case FS_IOC_GET_ENCRYPTION_NONCE:
> +		return fscrypt_ioctl_get_nonce(file, (void __user *)arg);
> +#endif /* CONFIG_FS_ENCRYPTION */
>  	}

The '#ifdef CONFIG_FS_ENCRYPTION' isn't needed here, since all the
fscrypt_ioctl_*() functions are stubbed out when !CONFIG_FS_ENCRYPTION.

- Eric

Re: [RFC PATCH 06/14] ceph: add fscrypt ioctls

[Jeff Layton]

On Fri, 2020-08-21 at 17:39 -0700, Eric Biggers wrote:
> On Fri, Aug 21, 2020 at 02:28:05PM -0400, Jeff Layton wrote:
> > Boilerplate ioctls for controlling encryption.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/ioctl.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> > index 6e061bf62ad4..4400b170eca9 100644
> > --- a/fs/ceph/ioctl.c
> > +++ b/fs/ceph/ioctl.c
> > @@ -6,6 +6,7 @@
> >  #include "mds_client.h"
> >  #include "ioctl.h"
> >  #include <linux/ceph/striper.h>
> > +#include <linux/fscrypt.h>
> >  
> >  /*
> >   * ioctls
> > @@ -289,6 +290,31 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >  
> >  	case CEPH_IOC_SYNCIO:
> >  		return ceph_ioctl_syncio(file);
> > +#ifdef CONFIG_FS_ENCRYPTION
> > +	case FS_IOC_SET_ENCRYPTION_POLICY:
> > +		return fscrypt_ioctl_set_policy(file, (const void __user *)arg);
> > +
> > +	case FS_IOC_GET_ENCRYPTION_POLICY:
> > +		return fscrypt_ioctl_get_policy(file, (void __user *)arg);
> > +
> > +	case FS_IOC_GET_ENCRYPTION_POLICY_EX:
> > +		return fscrypt_ioctl_get_policy_ex(file, (void __user *)arg);
> > +
> > +	case FS_IOC_ADD_ENCRYPTION_KEY:
> > +		return fscrypt_ioctl_add_key(file, (void __user *)arg);
> > +
> > +	case FS_IOC_REMOVE_ENCRYPTION_KEY:
> > +		return fscrypt_ioctl_remove_key(file, (void __user *)arg);
> > +
> > +	case FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS:
> > +		return fscrypt_ioctl_remove_key_all_users(file, (void __user *)arg);
> > +
> > +	case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
> > +		return fscrypt_ioctl_get_key_status(file, (void __user *)arg);
> > +
> > +	case FS_IOC_GET_ENCRYPTION_NONCE:
> > +		return fscrypt_ioctl_get_nonce(file, (void __user *)arg);
> > +#endif /* CONFIG_FS_ENCRYPTION */
> >  	}
> 
> The '#ifdef CONFIG_FS_ENCRYPTION' isn't needed here, since all the
> fscrypt_ioctl_*() functions are stubbed out when !CONFIG_FS_ENCRYPTION.
> 
> - Eric


Ahh, thanks. Will fix.

-- 
Jeff Layton <jlayton@kernel.org>

[RFC PATCH 07/14] ceph: crypto context handling for ceph

[Jeff Layton]

Eventually, we'll probably want to store the crypto context in a
dedicated field in the inode, but for now it's stored in a (plain)
xattr.

Also add support for "dummy" encryption (useful for testing with
automated test harnesses like xfstests).

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/Makefile |  1 +
 fs/ceph/crypto.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ceph/crypto.h | 26 ++++++++++++++++++
 fs/ceph/inode.c  |  3 +++
 fs/ceph/super.c  | 37 ++++++++++++++++++++++++++
 fs/ceph/super.h  |  6 ++++-
 6 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 fs/ceph/crypto.c
 create mode 100644 fs/ceph/crypto.h

diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
index 50c635dc7f71..1f77ca04c426 100644
--- a/fs/ceph/Makefile
+++ b/fs/ceph/Makefile
@@ -12,3 +12,4 @@ ceph-y := super.o inode.o dir.o file.o locks.o addr.o ioctl.o \
 
 ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
 ceph-$(CONFIG_CEPH_FS_POSIX_ACL) += acl.o
+ceph-$(CONFIG_FS_ENCRYPTION) += crypto.o
diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
new file mode 100644
index 000000000000..22a09d422b72
--- /dev/null
+++ b/fs/ceph/crypto.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/ceph/ceph_debug.h>
+#include <linux/xattr.h>
+#include <linux/fscrypt.h>
+
+#include "super.h"
+#include "crypto.h"
+
+static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
+{
+	int ret = __ceph_getxattr(inode, CEPH_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len);
+
+	if (ret > 0)
+		inode_set_flags(inode, S_ENCRYPTED, S_ENCRYPTED);
+	return ret;
+}
+
+static int ceph_crypt_set_context(struct inode *inode, const void *ctx, size_t len, void *fs_data)
+{
+	int ret;
+
+	WARN_ON_ONCE(fs_data);
+	ret = __ceph_setxattr(inode, CEPH_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len, XATTR_CREATE);
+	if (ret == 0)
+		inode_set_flags(inode, S_ENCRYPTED, S_ENCRYPTED);
+	return ret;
+}
+
+static bool ceph_crypt_empty_dir(struct inode *inode)
+{
+	struct ceph_inode_info *ci = ceph_inode(inode);
+
+	return ci->i_rsubdirs + ci->i_rfiles == 1;
+}
+
+static const union fscrypt_context *
+ceph_get_dummy_context(struct super_block *sb)
+{
+	return ceph_sb_to_client(sb)->dummy_enc_ctx.ctx;
+}
+
+static struct fscrypt_operations ceph_fscrypt_ops = {
+	.key_prefix		= "ceph:",
+	.get_context		= ceph_crypt_get_context,
+	.set_context		= ceph_crypt_set_context,
+	.get_dummy_context	= ceph_get_dummy_context,
+	.empty_dir		= ceph_crypt_empty_dir,
+	.max_namelen		= NAME_MAX,
+};
+
+int ceph_fscrypt_set_ops(struct super_block *sb)
+{
+	struct ceph_fs_client *fsc = sb->s_fs_info;
+
+	fscrypt_set_ops(sb, &ceph_fscrypt_ops);
+
+	if (ceph_test_mount_opt(fsc, TEST_DUMMY_ENC)) {
+		substring_t arg = { };
+
+		/* Ewwwwwwww */
+		if (fsc->mount_options->test_dummy_encryption) {
+			arg.from = fsc->mount_options->test_dummy_encryption;
+			arg.to = arg.from + strlen(arg.from) - 1;
+		}
+
+		return fscrypt_set_test_dummy_encryption(sb, &arg, &fsc->dummy_enc_ctx);
+	}
+	return 0;
+}
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
new file mode 100644
index 000000000000..309925b345fc
--- /dev/null
+++ b/fs/ceph/crypto.h
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ceph fscrypt functionality
+ */
+
+#ifndef _CEPH_CRYPTO_H
+#define _CEPH_CRYPTO_H
+
+#ifdef CONFIG_FS_ENCRYPTION
+
+#define DUMMY_ENCRYPTION_ENABLED(fsc) ((fsc)->dummy_enc_ctx.ctx != NULL)
+
+int ceph_fscrypt_set_ops(struct super_block *sb);
+
+#else /* CONFIG_FS_ENCRYPTION */
+
+#define DUMMY_ENCRYPTION_ENABLED(fsc) (0)
+
+static inline int ceph_fscrypt_set_ops(struct super_block *sb)
+{
+	return 0;
+}
+
+#endif /* CONFIG_FS_ENCRYPTION */
+
+#endif
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 156b98bda6aa..877bdda40db8 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -532,6 +532,8 @@ void ceph_free_inode(struct inode *inode)
 	struct ceph_inode_info *ci = ceph_inode(inode);
 
 	kfree(ci->i_symlink);
+	// FIXME: only call this once we have symlink support
+	// fscrypt_free_inode(inode);
 	kmem_cache_free(ceph_inode_cachep, ci);
 }
 
@@ -543,6 +545,7 @@ void ceph_evict_inode(struct inode *inode)
 
 	dout("evict_inode %p ino %llx.%llx\n", inode, ceph_vinop(inode));
 
+	fscrypt_put_encryption_info(inode);
 	truncate_inode_pages_final(&inode->i_data);
 	clear_inode(inode);
 
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 7ec0e6d03d10..95f5a7cf60f2 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -20,6 +20,7 @@
 #include "super.h"
 #include "mds_client.h"
 #include "cache.h"
+#include "crypto.h"
 
 #include <linux/ceph/ceph_features.h>
 #include <linux/ceph/decode.h>
@@ -44,6 +45,7 @@ static void ceph_put_super(struct super_block *s)
 	struct ceph_fs_client *fsc = ceph_sb_to_client(s);
 
 	dout("put_super\n");
+	fscrypt_free_dummy_context(&fsc->dummy_enc_ctx);
 	ceph_mdsc_close_sessions(fsc->mdsc);
 }
 
@@ -159,6 +161,7 @@ enum {
 	Opt_quotadf,
 	Opt_copyfrom,
 	Opt_wsync,
+	Opt_test_dummy_encryption,
 };
 
 enum ceph_recover_session_mode {
@@ -197,6 +200,8 @@ static const struct fs_parameter_spec ceph_mount_parameters[] = {
 	fsparam_u32	("rsize",			Opt_rsize),
 	fsparam_string	("snapdirname",			Opt_snapdirname),
 	fsparam_string	("source",			Opt_source),
+	fsparam_flag_no ("test_dummy_encryption",	Opt_test_dummy_encryption),
+	fsparam_string	("test_dummy_encryption",	Opt_test_dummy_encryption),
 	fsparam_u32	("wsize",			Opt_wsize),
 	fsparam_flag_no	("wsync",			Opt_wsync),
 	{}
@@ -455,6 +460,21 @@ static int ceph_parse_mount_param(struct fs_context *fc,
 		else
 			fsopt->flags |= CEPH_MOUNT_OPT_ASYNC_DIROPS;
 		break;
+	case Opt_test_dummy_encryption:
+		kfree(fsopt->test_dummy_encryption);
+		fsopt->test_dummy_encryption = NULL;
+		if (!result.negated) {
+#ifdef CONFIG_FS_ENCRYPTION
+			fsopt->test_dummy_encryption = param->string;
+			param->string = NULL;
+			fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
+#else
+			return warnfc(fc, "FS encryption not supported: test_dummy_encryption mount option ignored");
+#endif
+		} else {
+			fsopt->flags &= ~CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
+		}
+		break;
 	default:
 		BUG();
 	}
@@ -474,6 +494,7 @@ static void destroy_mount_options(struct ceph_mount_options *args)
 	kfree(args->mds_namespace);
 	kfree(args->server_path);
 	kfree(args->fscache_uniq);
+	kfree(args->test_dummy_encryption);
 	kfree(args);
 }
 
@@ -581,6 +602,8 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
 	if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)
 		seq_puts(m, ",nowsync");
 
+	fscrypt_show_test_dummy_encryption(m, ',', root->d_sb);
+
 	if (fsopt->wsize != CEPH_MAX_WRITE_SIZE)
 		seq_printf(m, ",wsize=%u", fsopt->wsize);
 	if (fsopt->rsize != CEPH_MAX_READ_SIZE)
@@ -984,7 +1007,12 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_min = 0;
 	s->s_time_max = U32_MAX;
 
+	ret = ceph_fscrypt_set_ops(s);
+	if (ret)
+		goto out;
+
 	ret = set_anon_super_fc(s, fc);
+out:
 	if (ret != 0)
 		fsc->sb = NULL;
 	return ret;
@@ -1140,6 +1168,15 @@ static int ceph_reconfigure_fc(struct fs_context *fc)
 	else
 		ceph_clear_mount_opt(fsc, ASYNC_DIROPS);
 
+	/* Don't allow test_dummy_encryption to change on remount */
+	if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
+		if (!ceph_test_mount_opt(fsc, TEST_DUMMY_ENC))
+			return -EEXIST;
+	} else {
+		if (ceph_test_mount_opt(fsc, TEST_DUMMY_ENC))
+			return -EEXIST;
+	}
+
 	sync_filesystem(fc->root->d_sb);
 	return 0;
 }
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index b3aa2395b66e..6327b5739286 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -44,6 +44,7 @@
 #define CEPH_MOUNT_OPT_NOQUOTADF       (1<<13) /* no root dir quota in statfs */
 #define CEPH_MOUNT_OPT_NOCOPYFROM      (1<<14) /* don't use RADOS 'copy-from' op */
 #define CEPH_MOUNT_OPT_ASYNC_DIROPS    (1<<15) /* allow async directory ops */
+#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC  (1<<16) /* enable dummy encryption (for testing) */
 
 #define CEPH_MOUNT_OPT_DEFAULT			\
 	(CEPH_MOUNT_OPT_DCACHE |		\
@@ -96,6 +97,7 @@ struct ceph_mount_options {
 	char *mds_namespace;  /* default NULL */
 	char *server_path;    /* default NULL (means "/") */
 	char *fscache_uniq;   /* default NULL */
+	char *test_dummy_encryption;	/* default NULL */
 };
 
 struct ceph_fs_client {
@@ -135,9 +137,11 @@ struct ceph_fs_client {
 #ifdef CONFIG_CEPH_FSCACHE
 	struct fscache_cookie *fscache;
 #endif
+#ifdef CONFIG_FS_ENCRYPTION
+	struct fscrypt_dummy_context dummy_enc_ctx;
+#endif
 };
 
-
 /*
  * File i/o capability.  This tracks shared state with the metadata
  * server that allows us to cache or writeback attributes or to read
-- 
2.26.2

[RFC PATCH 08/14] ceph: add routine to create context prior to RPC

[Jeff Layton]

Allow the client to create a new encryption context prior to creating a
new inode, and ensure that we set it in the ceph_acl_sec_ctx (alongside
acls, selinux contexts, etc.).

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/crypto.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ceph/crypto.h |  8 +++++++
 fs/ceph/dir.c    | 15 +++++++++++++
 fs/ceph/file.c   |  4 ++++
 fs/ceph/super.h  |  4 ++++
 5 files changed, 86 insertions(+)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 22a09d422b72..0e57497c2e55 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -67,3 +67,58 @@ int ceph_fscrypt_set_ops(struct super_block *sb)
 	}
 	return 0;
 }
+
+int ceph_fscrypt_new_context(struct inode *parent, struct ceph_acl_sec_ctx *as)
+{
+	int ret, ctxsize;
+	size_t name_len;
+	char *name;
+	struct ceph_pagelist *pagelist = as->pagelist;
+
+	/* Do nothing if subtree isn't encrypted */
+	if (!IS_ENCRYPTED(parent))
+		return 0;
+
+	ctxsize = fscrypt_new_context_from_parent(parent, as->fscrypt);
+	if (ctxsize <= 0)
+		return ctxsize;
+
+	/* marshal it in page array */
+	if (!pagelist) {
+		pagelist = ceph_pagelist_alloc(GFP_KERNEL);
+		if (!pagelist)
+			return -ENOMEM;
+		ret = ceph_pagelist_reserve(pagelist, PAGE_SIZE);
+		if (ret)
+			goto out;
+		ceph_pagelist_encode_32(pagelist, 1);
+	}
+
+	name = CEPH_XATTR_NAME_ENCRYPTION_CONTEXT;
+	name_len = strlen(name);
+	ret = ceph_pagelist_reserve(pagelist, 4 * 2 + name_len + ctxsize);
+	if (ret)
+		goto out;
+
+	if (as->pagelist) {
+		BUG_ON(pagelist->length <= sizeof(__le32));
+		if (list_is_singular(&pagelist->head)) {
+			le32_add_cpu((__le32*)pagelist->mapped_tail, 1);
+		} else {
+			struct page *page = list_first_entry(&pagelist->head,
+							     struct page, lru);
+			void *addr = kmap_atomic(page);
+			le32_add_cpu((__le32*)addr, 1);
+			kunmap_atomic(addr);
+		}
+	}
+
+	ceph_pagelist_encode_32(pagelist, name_len);
+	ceph_pagelist_append(pagelist, name, name_len);
+	ceph_pagelist_encode_32(pagelist, ctxsize);
+	ceph_pagelist_append(pagelist, as->fscrypt, ctxsize);
+out:
+	if (pagelist && !as->pagelist)
+		ceph_pagelist_release(pagelist);
+	return ret;
+}
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index 309925b345fc..e7d00831ef36 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -8,9 +8,12 @@
 
 #ifdef CONFIG_FS_ENCRYPTION
 
+#define CEPH_XATTR_NAME_ENCRYPTION_CONTEXT	"encryption.ctx"
+
 #define DUMMY_ENCRYPTION_ENABLED(fsc) ((fsc)->dummy_enc_ctx.ctx != NULL)
 
 int ceph_fscrypt_set_ops(struct super_block *sb);
+int ceph_fscrypt_new_context(struct inode *parent, struct ceph_acl_sec_ctx *as);
 
 #else /* CONFIG_FS_ENCRYPTION */
 
@@ -21,6 +24,11 @@ static inline int ceph_fscrypt_set_ops(struct super_block *sb)
 	return 0;
 }
 
+static int ceph_fscrypt_new_context(struct inode *parent, struct ceph_acl_sec_ctx *as)
+{
+	return 0;
+}
+
 #endif /* CONFIG_FS_ENCRYPTION */
 
 #endif
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 34f669220a8b..2e7f2bfa2c12 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -9,6 +9,7 @@
 
 #include "super.h"
 #include "mds_client.h"
+#include "crypto.h"
 
 /*
  * Directory operations: readdir, lookup, create, link, unlink,
@@ -848,6 +849,12 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 	if (err < 0)
 		goto out;
 
+	if (S_ISREG(mode)) {
+		err = ceph_fscrypt_new_context(dir, &as_ctx);
+		if (err < 0)
+			goto out;
+	}
+
 	dout("mknod in dir %p dentry %p mode 0%ho rdev %d\n",
 	     dir, dentry, mode, rdev);
 	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_MKNOD, USE_AUTH_MDS);
@@ -907,6 +914,10 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	if (err < 0)
 		goto out;
 
+	err = ceph_fscrypt_new_context(dir, &as_ctx);
+	if (err < 0)
+		goto out;
+
 	dout("symlink in dir %p dentry %p to '%s'\n", dir, dentry, dest);
 	req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_SYMLINK, USE_AUTH_MDS);
 	if (IS_ERR(req)) {
@@ -975,6 +986,10 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	if (err < 0)
 		goto out;
 
+	err = ceph_fscrypt_new_context(dir, &as_ctx);
+	if (err < 0)
+		goto out;
+
 	req = ceph_mdsc_create_request(mdsc, op, USE_AUTH_MDS);
 	if (IS_ERR(req)) {
 		err = PTR_ERR(req);
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index cf3f66d75162..94558e04f34a 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -19,6 +19,7 @@
 #include "cache.h"
 #include "io.h"
 #include "metric.h"
+#include "crypto.h"
 
 static __le32 ceph_flags_sys2wire(u32 flags)
 {
@@ -686,6 +687,9 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
 		if (err < 0)
 			goto out_ctx;
+		err = ceph_fscrypt_new_context(dir, &as_ctx);
+		if (err < 0)
+			goto out_ctx;
 	} else if (!d_in_lookup(dentry)) {
 		/* If it's not being looked up, it's negative */
 		return -ENOENT;
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 6327b5739286..a481cacf775a 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -17,6 +17,7 @@
 #include <linux/posix_acl.h>
 #include <linux/refcount.h>
 #include <linux/security.h>
+#include <linux/fscrypt.h>
 
 #include <linux/ceph/libceph.h>
 
@@ -991,6 +992,9 @@ struct ceph_acl_sec_ctx {
 #ifdef CONFIG_CEPH_FS_SECURITY_LABEL
 	void *sec_ctx;
 	u32 sec_ctxlen;
+#endif
+#ifdef CONFIG_FS_ENCRYPTION
+	u8	fscrypt[FSCRYPT_SET_CONTEXT_MAX_SIZE];
 #endif
 	struct ceph_pagelist *pagelist;
 };
-- 
2.26.2

[RFC PATCH 09/14] ceph: set S_ENCRYPTED bit if new inode has encryption.ctx xattr

[Jeff Layton]

This hack fixes a chicken-and-egg problem when fetching inodes from the
server. Once we move to a dedicated field in the inode, then this should
be able to go away.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/inode.c |  5 +++++
 fs/ceph/super.h |  1 +
 fs/ceph/xattr.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 877bdda40db8..fe231d7bd892 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -18,6 +18,7 @@
 #include "super.h"
 #include "mds_client.h"
 #include "cache.h"
+#include "crypto.h"
 #include <linux/ceph/decode.h>
 
 /*
@@ -908,6 +909,10 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
 		ceph_forget_all_cached_acls(inode);
 		ceph_security_invalidate_secctx(inode);
 		xattr_blob = NULL;
+		if ((inode->i_state & I_NEW) &&
+		    (DUMMY_ENCRYPTION_ENABLED(mdsc->fsc) ||
+		     ceph_inode_has_xattr(ci, CEPH_XATTR_NAME_ENCRYPTION_CONTEXT)))
+			inode_set_flags(inode, S_ENCRYPTED, S_ENCRYPTED);
 	}
 
 	/* finally update i_version */
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index a481cacf775a..ea111c0eadba 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -983,6 +983,7 @@ extern ssize_t ceph_listxattr(struct dentry *, char *, size_t);
 extern struct ceph_buffer *__ceph_build_xattrs_blob(struct ceph_inode_info *ci);
 extern void __ceph_destroy_xattrs(struct ceph_inode_info *ci);
 extern const struct xattr_handler *ceph_xattr_handlers[];
+bool ceph_inode_has_xattr(struct ceph_inode_info *ci, char *name);
 
 struct ceph_acl_sec_ctx {
 #ifdef CONFIG_CEPH_FS_POSIX_ACL
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 3a733ac33d9b..9dcb060cba9a 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -1283,6 +1283,38 @@ void ceph_release_acl_sec_ctx(struct ceph_acl_sec_ctx *as_ctx)
 		ceph_pagelist_release(as_ctx->pagelist);
 }
 
+/* Return true if inode's xattr blob has an xattr named "name" */
+bool ceph_inode_has_xattr(struct ceph_inode_info *ci, char *name)
+{
+	void *p, *end;
+	u32 numattr;
+	size_t namelen;
+
+	lockdep_assert_held(&ci->i_ceph_lock);
+
+	if (!ci->i_xattrs.blob || ci->i_xattrs.blob->vec.iov_len <= 4)
+		return false;
+
+	namelen = strlen(name);
+	p = ci->i_xattrs.blob->vec.iov_base;
+	end = p + ci->i_xattrs.blob->vec.iov_len;
+	ceph_decode_32_safe(&p, end, numattr, bad);
+
+	while (numattr--) {
+		u32 len;
+
+		ceph_decode_32_safe(&p, end, len, bad);
+		ceph_decode_need(&p, end, len, bad);
+		if (len == namelen && !memcmp(p, name, len))
+			return true;
+		p += len;
+		ceph_decode_32_safe(&p, end, len, bad);
+		ceph_decode_skip_n(&p, end, len, bad);
+	}
+bad:
+	return false;
+}
+
 /*
  * List of handlers for synthetic system.* attributes. Other
  * attributes are handled directly.
-- 
2.26.2

[RFC PATCH 10/14] ceph: make ceph_msdc_build_path use ref-walk

[Jeff Layton]

Encryption potentially requires allocation, at which point we'll need to
be in a non-atomic context. Convert ceph_msdc_build_path to take dentry
spinlocks and references instead of using rcu_read_lock to walk the
path.

This is slightly less efficient, and we may want to eventually allow
using RCU when the leaf dentry isn't encrypted.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/mds_client.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 76d8d9495d1d..c4bfa383e8c4 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2326,7 +2326,8 @@ static inline  u64 __get_oldest_tid(struct ceph_mds_client *mdsc)
 char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
 			   int stop_on_nosnap)
 {
-	struct dentry *temp;
+	struct dentry *cur;
+	struct inode *inode;
 	char *path;
 	int pos;
 	unsigned seq;
@@ -2343,34 +2344,35 @@ char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
 	path[pos] = '\0';
 
 	seq = read_seqbegin(&rename_lock);
-	rcu_read_lock();
-	temp = dentry;
+	cur = dget(dentry);
 	for (;;) {
-		struct inode *inode;
+		struct dentry *temp;
 
-		spin_lock(&temp->d_lock);
-		inode = d_inode(temp);
+		spin_lock(&cur->d_lock);
+		inode = d_inode(cur);
 		if (inode && ceph_snap(inode) == CEPH_SNAPDIR) {
 			dout("build_path path+%d: %p SNAPDIR\n",
-			     pos, temp);
-		} else if (stop_on_nosnap && inode && dentry != temp &&
+			     pos, cur);
+		} else if (stop_on_nosnap && inode && dentry != cur &&
 			   ceph_snap(inode) == CEPH_NOSNAP) {
-			spin_unlock(&temp->d_lock);
+			spin_unlock(&cur->d_lock);
 			pos++; /* get rid of any prepended '/' */
 			break;
 		} else {
-			pos -= temp->d_name.len;
+			pos -= cur->d_name.len;
 			if (pos < 0) {
-				spin_unlock(&temp->d_lock);
+				spin_unlock(&cur->d_lock);
 				break;
 			}
-			memcpy(path + pos, temp->d_name.name, temp->d_name.len);
+			memcpy(path + pos, cur->d_name.name, cur->d_name.len);
 		}
+		temp = cur;
+		cur = dget(temp->d_parent);
 		spin_unlock(&temp->d_lock);
-		temp = READ_ONCE(temp->d_parent);
+		dput(temp);
 
 		/* Are we at the root? */
-		if (IS_ROOT(temp))
+		if (IS_ROOT(cur))
 			break;
 
 		/* Are we out of buffer? */
@@ -2379,8 +2381,9 @@ char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
 
 		path[pos] = '/';
 	}
-	base = ceph_ino(d_inode(temp));
-	rcu_read_unlock();
+	inode = d_inode(cur);
+	base = inode ? ceph_ino(inode) : 0;
+	dput(cur);
 
 	if (read_seqretry(&rename_lock, seq))
 		goto retry;
-- 
2.26.2

[RFC PATCH 11/14] ceph: add encrypted fname handling to ceph_mdsc_build_path

[Jeff Layton]

Allow ceph_mdsc_build_path to encrypt and base64 encode the filename
when the parent is encrypted and we're sending the path to the MDS.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/mds_client.c | 51 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index c4bfa383e8c4..08e173b821b5 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -11,6 +11,7 @@
 #include <linux/ratelimit.h>
 #include <linux/bits.h>
 #include <linux/ktime.h>
+#include <linux/base64.h>
 
 #include "super.h"
 #include "mds_client.h"
@@ -2323,8 +2324,7 @@ static inline  u64 __get_oldest_tid(struct ceph_mds_client *mdsc)
  * Encode hidden .snap dirs as a double /, i.e.
  *   foo/.snap/bar -> foo//bar
  */
-char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
-			   int stop_on_nosnap)
+char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase, int for_wire)
 {
 	struct dentry *cur;
 	struct inode *inode;
@@ -2346,30 +2346,59 @@ char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase,
 	seq = read_seqbegin(&rename_lock);
 	cur = dget(dentry);
 	for (;;) {
-		struct dentry *temp;
+		struct dentry *parent;
 
 		spin_lock(&cur->d_lock);
 		inode = d_inode(cur);
+		parent = cur->d_parent;
 		if (inode && ceph_snap(inode) == CEPH_SNAPDIR) {
 			dout("build_path path+%d: %p SNAPDIR\n",
 			     pos, cur);
-		} else if (stop_on_nosnap && inode && dentry != cur &&
-			   ceph_snap(inode) == CEPH_NOSNAP) {
+			dget(parent);
+			spin_unlock(&cur->d_lock);
+		} else if (for_wire && inode && dentry != cur && ceph_snap(inode) == CEPH_NOSNAP) {
 			spin_unlock(&cur->d_lock);
 			pos++; /* get rid of any prepended '/' */
 			break;
-		} else {
+		} else if (!for_wire || !IS_ENCRYPTED(d_inode(parent))) {
 			pos -= cur->d_name.len;
 			if (pos < 0) {
 				spin_unlock(&cur->d_lock);
 				break;
 			}
 			memcpy(path + pos, cur->d_name.name, cur->d_name.len);
+			dget(parent);
+			spin_unlock(&cur->d_lock);
+		} else {
+			int err;
+			struct fscrypt_name fname = { };
+			int len;
+			char buf[BASE64_CHARS(NAME_MAX)];
+
+			dget(parent);
+			spin_unlock(&cur->d_lock);
+
+			err = fscrypt_setup_filename(d_inode(parent), &cur->d_name, 1, &fname);
+			if (err) {
+				dput(parent);
+				dput(cur);
+				return ERR_PTR(err);
+			}
+
+			/* base64 encode the encrypted name */
+			len = base64_encode(fname.disk_name.name, fname.disk_name.len, buf);
+			pos -= len;
+			if (pos < 0) {
+				dput(parent);
+				fscrypt_free_filename(&fname);
+				break;
+			}
+			memcpy(path + pos, buf, len);
+			dout("non-ciphertext name = %.*s\n", len, buf);
+			fscrypt_free_filename(&fname);
 		}
-		temp = cur;
-		cur = dget(temp->d_parent);
-		spin_unlock(&temp->d_lock);
-		dput(temp);
+		dput(cur);
+		cur = parent;
 
 		/* Are we at the root? */
 		if (IS_ROOT(cur))
@@ -2414,7 +2443,7 @@ static int build_dentry_path(struct dentry *dentry, struct inode *dir,
 	rcu_read_lock();
 	if (!dir)
 		dir = d_inode_rcu(dentry->d_parent);
-	if (dir && parent_locked && ceph_snap(dir) == CEPH_NOSNAP) {
+	if (dir && parent_locked && ceph_snap(dir) == CEPH_NOSNAP && !IS_ENCRYPTED(dir)) {
 		*pino = ceph_ino(dir);
 		rcu_read_unlock();
 		*ppath = dentry->d_name.name;
-- 
2.26.2

[RFC PATCH 12/14] ceph: make d_revalidate call fscrypt revalidator for encrypted dentries

[Jeff Layton]

If we have an encrypted dentry, then we need to test whether a new key
might have been established or removed. Do that before we test anything
else about the dentry.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/dir.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 2e7f2bfa2c12..d2104c115868 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1701,6 +1701,12 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
 	dout("d_revalidate %p '%pd' inode %p offset 0x%llx\n", dentry,
 	     dentry, inode, ceph_dentry(dentry)->offset);
 
+	if (IS_ENCRYPTED(dir)) {
+		valid = fscrypt_d_revalidate(dentry, flags);
+		if (valid <= 0)
+			return valid;
+	}
+
 	mdsc = ceph_sb_to_client(dir->i_sb)->mdsc;
 
 	/* always trust cached snapped dentries, snapdir dentry */
-- 
2.26.2

[RFC PATCH 13/14] ceph: add support to readdir for encrypted filenames

[Jeff Layton]

Add helper functions for buffer management and for decrypting filenames
returned by the MDS. Wire those into the readdir codepaths.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/crypto.c | 47 ++++++++++++++++++++++++++++++
 fs/ceph/crypto.h | 49 ++++++++++++++++++++++++++++++-
 fs/ceph/dir.c    | 76 ++++++++++++++++++++++++++++++++++++++++--------
 fs/ceph/inode.c  | 30 +++++++++++++++++--
 4 files changed, 186 insertions(+), 16 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 0e57497c2e55..521163ef3653 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -2,6 +2,7 @@
 #include <linux/ceph/ceph_debug.h>
 #include <linux/xattr.h>
 #include <linux/fscrypt.h>
+#include <linux/base64.h>
 
 #include "super.h"
 #include "crypto.h"
@@ -122,3 +123,49 @@ int ceph_fscrypt_new_context(struct inode *parent, struct ceph_acl_sec_ctx *as)
 		ceph_pagelist_release(pagelist);
 	return ret;
 }
+
+int ceph_fname_to_usr(struct inode *parent, char *name, u32 len,
+			struct fscrypt_str *tname, struct fscrypt_str *oname)
+{
+	int ret, declen;
+	u32 save_len;
+	struct fscrypt_str myname = FSTR_INIT(NULL, 0);
+
+	if (!IS_ENCRYPTED(parent)) {
+		oname->name = name;
+		oname->len = len;
+		return 0;
+	}
+
+	ret = fscrypt_get_encryption_info(parent);
+	if (ret)
+		return ret;
+
+	if (tname) {
+		save_len = tname->len;
+	} else {
+		int err;
+
+		save_len = 0;
+		err = fscrypt_fname_alloc_buffer(NAME_MAX, &myname);
+		if (err)
+			return err;
+		tname = &myname;
+	}
+
+	declen = base64_decode(name, len, tname->name);
+	if (declen < 0 || declen > NAME_MAX) {
+		ret = -EIO;
+		goto out;
+	}
+
+	tname->len = declen;
+
+	ret = fscrypt_fname_disk_to_usr(parent, 0, 0, tname, oname);
+
+	if (save_len)
+		tname->len = save_len;
+out:
+	fscrypt_fname_free_buffer(&myname);
+	return ret;
+}
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index e7d00831ef36..3ca4f469619a 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -6,6 +6,8 @@
 #ifndef _CEPH_CRYPTO_H
 #define _CEPH_CRYPTO_H
 
+#include <linux/fscrypt.h>
+
 #ifdef CONFIG_FS_ENCRYPTION
 
 #define CEPH_XATTR_NAME_ENCRYPTION_CONTEXT	"encryption.ctx"
@@ -15,6 +17,29 @@
 int ceph_fscrypt_set_ops(struct super_block *sb);
 int ceph_fscrypt_new_context(struct inode *parent, struct ceph_acl_sec_ctx *as);
 
+static inline int ceph_fname_alloc_buffer(struct inode *parent, struct fscrypt_str *fname)
+{
+	if (!IS_ENCRYPTED(parent))
+		return 0;
+	return fscrypt_fname_alloc_buffer(NAME_MAX, fname);
+}
+
+static inline void ceph_fname_free_buffer(struct inode *parent, struct fscrypt_str *fname)
+{
+	if (IS_ENCRYPTED(parent))
+		fscrypt_fname_free_buffer(fname);
+}
+
+static inline int ceph_get_encryption_info(struct inode *inode)
+{
+	if (!IS_ENCRYPTED(inode))
+		return 0;
+	return fscrypt_get_encryption_info(inode);
+}
+
+int ceph_fname_to_usr(struct inode *parent, char *name, u32 len,
+			struct fscrypt_str *tname, struct fscrypt_str *oname);
+
 #else /* CONFIG_FS_ENCRYPTION */
 
 #define DUMMY_ENCRYPTION_ENABLED(fsc) (0)
@@ -24,8 +49,30 @@ static inline int ceph_fscrypt_set_ops(struct super_block *sb)
 	return 0;
 }
 
-static int ceph_fscrypt_new_context(struct inode *parent, struct ceph_acl_sec_ctx *as)
+static inline int ceph_fscrypt_new_context(struct inode *parent, struct ceph_acl_sec_ctx *as)
+{
+	return 0;
+}
+
+static inline int ceph_fname_alloc_buffer(struct inode *parent, struct fscrypt_str *fname)
+{
+	return 0;
+}
+
+static inline void ceph_fname_free_buffer(struct inode *parent, struct fscrypt_str *fname)
+{
+}
+
+static inline int ceph_get_encryption_info(struct inode *inode)
+{
+	return 0;
+}
+
+static inline int ceph_fname_to_usr(struct inode *inode, char *name, u32 len,
+			struct fscrypt_str *tname, struct fscrypt_str *oname)
 {
+	oname->name = dname;
+	oname->len = dlen;
 	return 0;
 }
 
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index d2104c115868..858285d1cdfe 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -6,6 +6,7 @@
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/xattr.h>
+#include <linux/base64.h>
 
 #include "super.h"
 #include "mds_client.h"
@@ -169,6 +170,27 @@ __dcache_find_get_entry(struct dentry *parent, u64 idx,
 	return dentry ? : ERR_PTR(-EAGAIN);
 }
 
+static bool fscrypt_key_status_change(struct dentry *dentry)
+{
+	struct inode *dir;
+	bool encrypted_name, have_key;
+
+	lockdep_assert_held(&dentry->d_lock);
+
+	dir = d_inode(dentry->d_parent);
+	if (!IS_ENCRYPTED(dir))
+		return false;
+
+	encrypted_name = dentry->d_flags & DCACHE_ENCRYPTED_NAME;
+	have_key = fscrypt_has_encryption_key(dir);
+
+	if (encrypted_name == have_key)
+		ceph_dir_clear_complete(dir);
+
+	dout("%s encrypted_name=%d have_key=%d\n", __func__, encrypted_name, have_key);
+	return encrypted_name == have_key;
+}
+
 /*
  * When possible, we try to satisfy a readdir by peeking at the
  * dcache.  We make this work by carefully ordering dentries on
@@ -239,11 +261,11 @@ static int __dcache_readdir(struct file *file,  struct dir_context *ctx,
 			goto out;
 		}
 
-		spin_lock(&dentry->d_lock);
 		di = ceph_dentry(dentry);
 		if (d_unhashed(dentry) ||
 		    d_really_is_negative(dentry) ||
-		    di->lease_shared_gen != shared_gen) {
+		    di->lease_shared_gen != shared_gen ||
+		    fscrypt_key_status_change(dentry)) {
 			spin_unlock(&dentry->d_lock);
 			dput(dentry);
 			err = -EAGAIN;
@@ -315,6 +337,8 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 	int err;
 	unsigned frag = -1;
 	struct ceph_mds_reply_info_parsed *rinfo;
+	struct fscrypt_str tname = FSTR_INIT(NULL, 0);
+	struct fscrypt_str oname = FSTR_INIT(NULL, 0);
 
 	dout("readdir %p file %p pos %llx\n", inode, file, ctx->pos);
 	if (dfi->file_info.flags & CEPH_F_ATEND)
@@ -342,6 +366,10 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		ctx->pos = 2;
 	}
 
+	err = ceph_get_encryption_info(inode);
+	if (err)
+		goto out;
+
 	spin_lock(&ci->i_ceph_lock);
 	/* request Fx cap. if have Fx, we don't need to release Fs cap
 	 * for later create/unlink. */
@@ -362,6 +390,14 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		spin_unlock(&ci->i_ceph_lock);
 	}
 
+	err = ceph_fname_alloc_buffer(inode, &tname);
+	if (err < 0)
+		goto out;
+
+	err = ceph_fname_alloc_buffer(inode, &oname);
+	if (err < 0)
+		goto out;
+
 	/* proceed with a normal readdir */
 more:
 	/* do we have the correct frag content buffered? */
@@ -389,12 +425,14 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		dout("readdir fetching %llx.%llx frag %x offset '%s'\n",
 		     ceph_vinop(inode), frag, dfi->last_name);
 		req = ceph_mdsc_create_request(mdsc, op, USE_AUTH_MDS);
-		if (IS_ERR(req))
-			return PTR_ERR(req);
+		if (IS_ERR(req)) {
+			err = PTR_ERR(req);
+			goto out;
+		}
 		err = ceph_alloc_readdir_reply_buffer(req, inode);
 		if (err) {
 			ceph_mdsc_put_request(req);
-			return err;
+			goto out;
 		}
 		/* hints to request -> mds selection code */
 		req->r_direct_mode = USE_AUTH_MDS;
@@ -407,7 +445,8 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 			req->r_path2 = kstrdup(dfi->last_name, GFP_KERNEL);
 			if (!req->r_path2) {
 				ceph_mdsc_put_request(req);
-				return -ENOMEM;
+				err = -ENOMEM;
+				goto out;
 			}
 		} else if (is_hash_order(ctx->pos)) {
 			req->r_args.readdir.offset_hash =
@@ -428,7 +467,7 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		err = ceph_mdsc_do_request(mdsc, NULL, req);
 		if (err < 0) {
 			ceph_mdsc_put_request(req);
-			return err;
+			goto out;
 		}
 		dout("readdir got and parsed readdir result=%d on "
 		     "frag %x, end=%d, complete=%d, hash_order=%d\n",
@@ -481,7 +520,7 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 			err = note_last_dentry(dfi, rde->name, rde->name_len,
 					       next_offset);
 			if (err)
-				return err;
+				goto out;
 		} else if (req->r_reply_info.dir_end) {
 			dfi->next_offset = 2;
 			/* keep last name */
@@ -509,8 +548,10 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 	}
 	for (; i < rinfo->dir_nr; i++) {
 		struct ceph_mds_reply_dir_entry *rde = rinfo->dir_entries + i;
+		u32 olen = oname.len;
 
 		BUG_ON(rde->offset < ctx->pos);
+		BUG_ON(!rde->inode.in);
 
 		ctx->pos = rde->offset;
 		dout("readdir (%d/%d) -> %llx '%.*s' %p\n",
@@ -519,12 +560,20 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 
 		BUG_ON(!rde->inode.in);
 
-		if (!dir_emit(ctx, rde->name, rde->name_len,
+		err = ceph_fname_to_usr(inode, rde->name, rde->name_len, &tname, &oname);
+		if (err)
+			goto out;
+
+		if (!dir_emit(ctx, oname.name, oname.len,
 			      ceph_present_ino(inode->i_sb, le64_to_cpu(rde->inode.in->ino)),
 			      le32_to_cpu(rde->inode.in->mode) >> 12)) {
 			dout("filldir stopping us...\n");
-			return 0;
+			err = 0;
+			goto out;
 		}
+
+		/* Reset the lengths to their original allocated vals */
+		oname.len = olen;
 		ctx->pos++;
 	}
 
@@ -579,9 +628,12 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 					dfi->dir_ordered_count);
 		spin_unlock(&ci->i_ceph_lock);
 	}
-
+	err = 0;
 	dout("readdir %p file %p done.\n", inode, file);
-	return 0;
+out:
+	ceph_fname_free_buffer(inode, &tname);
+	ceph_fname_free_buffer(inode, &oname);
+	return err;
 }
 
 static void reset_readdir(struct ceph_dir_file_info *dfi)
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index fe231d7bd892..007d01b7a2ba 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1593,7 +1593,8 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 			     struct ceph_mds_session *session)
 {
 	struct dentry *parent = req->r_dentry;
-	struct ceph_inode_info *ci = ceph_inode(d_inode(parent));
+	struct inode *inode = d_inode(parent);
+	struct ceph_inode_info *ci = ceph_inode(inode);
 	struct ceph_mds_reply_info_parsed *rinfo = &req->r_reply_info;
 	struct qstr dname;
 	struct dentry *dn;
@@ -1604,6 +1605,8 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 	u32 last_hash = 0;
 	u32 fpos_offset;
 	struct ceph_readdir_cache_control cache_ctl = {};
+	struct fscrypt_str tname = FSTR_INIT(NULL, 0);
+	struct fscrypt_str oname = FSTR_INIT(NULL, 0);
 
 	if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags))
 		return readdir_prepopulate_inodes_only(req, session);
@@ -1655,14 +1658,28 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 	cache_ctl.index = req->r_readdir_cache_idx;
 	fpos_offset = req->r_readdir_offset;
 
+	err = ceph_fname_alloc_buffer(inode, &tname);
+	if (err < 0)
+		goto out;
+
+	err = ceph_fname_alloc_buffer(inode, &oname);
+	if (err < 0)
+		goto out;
+
 	/* FIXME: release caps/leases if error occurs */
 	for (i = 0; i < rinfo->dir_nr; i++) {
 		struct ceph_mds_reply_dir_entry *rde = rinfo->dir_entries + i;
 		struct ceph_vino tvino;
+		u32 olen = oname.len;
 
-		dname.name = rde->name;
-		dname.len = rde->name_len;
+		err = ceph_fname_to_usr(inode, rde->name, rde->name_len, &tname, &oname);
+		if (err)
+			goto out;
+
+		dname.name = oname.name;
+		dname.len = oname.len;
 		dname.hash = full_name_hash(parent, dname.name, dname.len);
+		oname.len = olen;
 
 		tvino.ino = le64_to_cpu(rde->inode.in->ino);
 		tvino.snap = le64_to_cpu(rde->inode.in->snapid);
@@ -1693,6 +1710,11 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 				err = -ENOMEM;
 				goto out;
 			}
+			if (IS_ENCRYPTED(inode) && !fscrypt_has_encryption_key(inode)) {
+				spin_lock(&dn->d_lock);
+				dn->d_flags |= DCACHE_ENCRYPTED_NAME;
+				spin_unlock(&dn->d_lock);
+			}
 		} else if (d_really_is_positive(dn) &&
 			   (ceph_ino(d_inode(dn)) != tvino.ino ||
 			    ceph_snap(d_inode(dn)) != tvino.snap)) {
@@ -1783,6 +1805,8 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 		req->r_readdir_cache_idx = cache_ctl.index;
 	}
 	ceph_readdir_cache_release(&cache_ctl);
+	ceph_fname_free_buffer(inode, &tname);
+	ceph_fname_free_buffer(inode, &oname);
 	dout("readdir_prepopulate done\n");
 	return err;
 }
-- 
2.26.2

[RFC PATCH 14/14] ceph: add fscrypt support to ceph_fill_trace

[Jeff Layton]

When we get a dentry in a trace, decrypt the name so we can properly
instantiate the dentry.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/inode.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 007d01b7a2ba..945c25e7ed20 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1273,6 +1273,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 		    !test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) {
 			struct qstr dname;
 			struct dentry *dn, *parent;
+			struct fscrypt_str oname = FSTR_INIT(NULL, 0);
 
 			BUG_ON(!rinfo->head->is_target);
 			BUG_ON(req->r_dentry);
@@ -1280,8 +1281,20 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 			parent = d_find_any_alias(dir);
 			BUG_ON(!parent);
 
-			dname.name = rinfo->dname;
-			dname.len = rinfo->dname_len;
+			err = ceph_fname_alloc_buffer(dir, &oname);
+			if (err < 0) {
+				dput(parent);
+				goto done;
+			}
+
+			err = ceph_fname_to_usr(dir, rinfo->dname, rinfo->dname_len, NULL, &oname);
+			if (err < 0) {
+				dput(parent);
+				ceph_fname_free_buffer(dir, &oname);
+				goto done;
+			}
+			dname.name = oname.name;
+			dname.len = oname.len;
 			dname.hash = full_name_hash(parent, dname.name, dname.len);
 			tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
 			tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
@@ -1296,9 +1309,15 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 				     dname.len, dname.name, dn);
 				if (!dn) {
 					dput(parent);
+					ceph_fname_free_buffer(dir, &oname);
 					err = -ENOMEM;
 					goto done;
 				}
+				if (IS_ENCRYPTED(dir) && !fscrypt_has_encryption_key(dir)) {
+					spin_lock(&dn->d_lock);
+					dn->d_flags |= DCACHE_ENCRYPTED_NAME;
+					spin_unlock(&dn->d_lock);
+				}
 				err = 0;
 			} else if (d_really_is_positive(dn) &&
 				   (ceph_ino(d_inode(dn)) != tvino.ino ||
@@ -1310,6 +1329,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 				dput(dn);
 				goto retry_lookup;
 			}
+			ceph_fname_free_buffer(dir, &oname);
 
 			req->r_dentry = dn;
 			dput(parent);
-- 
2.26.2

Re: [RFC PATCH 00/14] ceph+fscrypt: together at last (contexts and filenames)

[Eric Biggers]

Hi Jeff,

On Fri, Aug 21, 2020 at 02:27:59PM -0400, Jeff Layton wrote:
> This is a (very rough and incomplete) draft patchset that I've been
> working on to add fscrypt support to cephfs. The main use case is being
> able to allow encryption at the edges, without having to trust your storage
> provider with keys.

This is very interesting work -- thanks for sending this out!

> Implementing fscrypt on a network filesystem has some challenges that
> you don't have to deal with on a local fs:
> 
> Ceph (and most other netfs') will need to pre-create a crypto context
> when creating a new inode as we'll need to encrypt some things before we
> have an inode. This patchset stores contexts in an xattr, but that's
> probably not ideal for the final implementation [1].

Coincidentally, I've currently working on solving a similar problem.  On ext4,
the inode number can't be assigned, and the encryption xattr can't be set, until
the jbd2 transaction which creates the inode.  Also, if the new inode is a
symlink, then fscrypt_encrypt_symlink() has to be called during the transaction.
Together, these imply that fscrypt_get_encryption_info() has to be called during
the transaction.

That's what we do, currently.  However, it's technically wrong and can deadlock,
since fscrypt_get_encryption_info() isn't GFP_NOFS-safe (and it can't be).

f2fs appears to have a similar problem, though I'm still investigating.

To fix this, I'm planning to add new functions:

   - fscrypt_prepare_new_inode() will set up the fscrypt_info for a new
     'struct inode' which hasn't necessarily had an inode number assigned yet.
     It won't set the encryption xattr yet.

   - fscrypt_set_context() will set the encryption xattr, using the fscrypt_info
     that fscrypt_prepare_new_inode() created earlier.  It will replace
     fscrypt_inherit_context().

I'm working on my patches at
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=wip-fscrypt.
They're not ready to send out yet, but I'll Cc you when I do.

It seems there's still something a bit different that you need: you want
fs/crypto/ to provide a buffer containing the encryption xattr (presumably
because ceph needs to package it up into a single network request that creates
the file?), instead of calling a function which then uses
fscrypt_operations::set_context().  I could pretty easily handle that by adding
a function that returns the xattr directly and would be an alternative to
fscrypt_set_context().

> Storing a binary crypttext filename on the MDS (or most network
> fileservers) may be problematic. We'll probably end up having to base64
> encode the names when storing them. I expect most network filesystems to
> have similar issues. That may limit the effective NAME_MAX for some
> filesystems [2].

I strongly recommend keeping support for the full NAME_MAX (255 bytes), if it's
at all possible.  eCryptfs limited filenames to 143 bytes, which has
historically caused lots of problems.  Try the following Google search to get a
sense of the large number of users that have run into this limitation:
https://www.google.com/search?q=ecryptfs+143+filename

> 
> For content encryption, Ceph (and probably at least CIFS/SMB) will need
> to deal with writes not aligned on crypto blocks. These filesystems
> sometimes write synchronously to the server instead of going through the
> pagecache [3].

I/O that isn't aligned to the "encryption data unit size" (which on the
filesystems that currently support fscrypt, is the same thing as the
"filesystem block size") isn't really possible unless it's buffered.  For
AES-XTS specifically, *in principle* it's possible to encrypt/decrypt an
individual 16-byte aligned region.  But Linux's crypto API doesn't currently
support sub-message crypto, and also fscrypt supports the AES-CBC and Adiantum
encryption modes which have stricter requirements.

So, I think that reads/writes to encrypted files will always need to go through
the page cache.

> 
> Symlink handling in fscrypt will also need to be refactored a bit, as we
> won't have an inode before we'll need to encrypt its contents.

Will there be an in-memory inode allocated yet (a 'struct inode'), just with no
inode number assigned yet?  If so, my work-in-progress patchset I mentioned
earlier should be sufficient to address this.  The order would be:

	1. fscrypt_prepare_new_inode()
	2. fscrypt_encrypt_symlink()
	3. Assign inode number

Or does ceph not have a 'struct inode' at all until step (3)?

- Eric

Re: [RFC PATCH 00/14] ceph+fscrypt: together at last (contexts and filenames)

[Jeff Layton]

On Fri, 2020-08-21 at 17:23 -0700, Eric Biggers wrote:
> Hi Jeff,
> 
> On Fri, Aug 21, 2020 at 02:27:59PM -0400, Jeff Layton wrote:
> > This is a (very rough and incomplete) draft patchset that I've been
> > working on to add fscrypt support to cephfs. The main use case is being
> > able to allow encryption at the edges, without having to trust your storage
> > provider with keys.
> 
> This is very interesting work -- thanks for sending this out!
> 

Thanks. As I said, it's still very rough at this point, but I'm hoping
to have something ready for v5.11. I think we'll need to plumb some
support into the MDS too, so getting all of that lined up may take
longer.

> > Implementing fscrypt on a network filesystem has some challenges that
> > you don't have to deal with on a local fs:
> > 
> > Ceph (and most other netfs') will need to pre-create a crypto context
> > when creating a new inode as we'll need to encrypt some things before we
> > have an inode. This patchset stores contexts in an xattr, but that's
> > probably not ideal for the final implementation [1].
> 
> Coincidentally, I've currently working on solving a similar problem.  On ext4,
> the inode number can't be assigned, and the encryption xattr can't be set, until
> the jbd2 transaction which creates the inode.  Also, if the new inode is a
> symlink, then fscrypt_encrypt_symlink() has to be called during the transaction.
> Together, these imply that fscrypt_get_encryption_info() has to be called during
> the transaction.
> 

Yes, similar problem. I started looking at symlinks today, and got a
little ways into a patchset to refactor some fscrypt code to handle
them, but I don't think it's quite right yet. A more general solution
would be nice.

> That's what we do, currently.  However, it's technically wrong and can deadlock,
> since fscrypt_get_encryption_info() isn't GFP_NOFS-safe (and it can't be).
> 
> f2fs appears to have a similar problem, though I'm still investigating.
> 
> To fix this, I'm planning to add new functions:
> 
>    - fscrypt_prepare_new_inode() will set up the fscrypt_info for a new
>      'struct inode' which hasn't necessarily had an inode number assigned yet.
>      It won't set the encryption xattr yet.
> 

I more or less have that in 02/14, I think, but if you have something
else in mind, I'm happy to follow suit.

>    - fscrypt_set_context() will set the encryption xattr, using the fscrypt_info
>      that fscrypt_prepare_new_inode() created earlier.  It will replace
>      fscrypt_inherit_context().
> 

Ok. I don't think we'll need that for ceph, generally as we'll want to
send the context so it can be attached atomically during the create.

> I'm working on my patches at
> https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=wip-fscrypt.
> They're not ready to send out yet, but I'll Cc you when I do.
> 

Thanks. I'll check them out.

> It seems there's still something a bit different that you need: you want
> fs/crypto/ to provide a buffer containing the encryption xattr (presumably
> because ceph needs to package it up into a single network request that creates
> the file?), instead of calling a function which then uses
> fscrypt_operations::set_context().  I could pretty easily handle that by adding
> a function that returns the xattr directly and would be an alternative to
> fscrypt_set_context().
> 

Exactly. We want to send the context with the create call.

> > Storing a binary crypttext filename on the MDS (or most network
> > fileservers) may be problematic. We'll probably end up having to base64
> > encode the names when storing them. I expect most network filesystems to
> > have similar issues. That may limit the effective NAME_MAX for some
> > filesystems [2].
> 
> I strongly recommend keeping support for the full NAME_MAX (255 bytes), if it's
> at all possible.  eCryptfs limited filenames to 143 bytes, which has
> historically caused lots of problems.  Try the following Google search to get a
> sense of the large number of users that have run into this limitation:
> https://www.google.com/search?q=ecryptfs+143+filename
> 

Absolutely. I don't think it'll be a problem with ceph as we have more
flexibility to change the protocol and underlying server
implementation. 

It would be really cool to weave this into NFS and CIFS/SMB eventually
too, but that might be more difficult as Linux has less control over
servers in the field, and most of them will cap out at 255 chars. Maybe
we could extend those protocols too though, if there were desire.

> > For content encryption, Ceph (and probably at least CIFS/SMB) will need
> > to deal with writes not aligned on crypto blocks. These filesystems
> > sometimes write synchronously to the server instead of going through the
> > pagecache [3].
> 
> I/O that isn't aligned to the "encryption data unit size" (which on the
> filesystems that currently support fscrypt, is the same thing as the
> "filesystem block size") isn't really possible unless it's buffered.  For
> AES-XTS specifically, *in principle* it's possible to encrypt/decrypt an
> individual 16-byte aligned region.  But Linux's crypto API doesn't currently
> support sub-message crypto, and also fscrypt supports the AES-CBC and Adiantum
> encryption modes which have stricter requirements.
> 
> So, I think that reads/writes to encrypted files will always need to go through
> the page cache.
> 

The ceph OSD protocol (which is the data path) can do cmpxchg-like
operations on an object. So, theoretically we can do a
read/modify/write, and the write would only work if the content hadn't
changed since the read. We'd then just redo the whole thing if there was
a conflicting change in the interim.

It'd be slow and would suck, but I think it would give us a correct
result. The good news is that usually the clients are granted the
capability to buffer up writes, at which point we can use the pagecache
like anything else. This would only be to ensure correctness in
contended cases.

> > Symlink handling in fscrypt will also need to be refactored a bit, as we
> > won't have an inode before we'll need to encrypt its contents.
> 
> Will there be an in-memory inode allocated yet (a 'struct inode'), just with no
> inode number assigned yet?  If so, my work-in-progress patchset I mentioned
> earlier should be sufficient to address this.  The order would be:
> 
> 	1. fscrypt_prepare_new_inode()
> 	2. fscrypt_encrypt_symlink()
> 	3. Assign inode number
> 
> 
> Or does ceph not have a 'struct inode' at all until step (3)?

No, generally ceph doesn't create an inode until the reply comes in. I
think we'll need to be able to create a context and encrypt the symlink
before we issue the call to the server. I started hacking at the fscrypt
code for this today, but I didn't get very far.

FWIW, ceph is a bit of an odd netfs protocol in that there is a standard
"trace" that holds info about dentries and inodes that are created or
modified as a result of an operation. Most of the dentry/inode cache
manipulation is done at that point, which is done as part of the reply
processing.

Thanks for the comments so far!
--
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH 00/14] ceph+fscrypt: together at last (contexts and filenames)

[Eric Biggers]

On Fri, Aug 21, 2020 at 08:58:35PM -0400, Jeff Layton wrote:
> > > Ceph (and most other netfs') will need to pre-create a crypto context
> > > when creating a new inode as we'll need to encrypt some things before we
> > > have an inode. This patchset stores contexts in an xattr, but that's
> > > probably not ideal for the final implementation [1].
> > 
> > Coincidentally, I've currently working on solving a similar problem.  On ext4,
> > the inode number can't be assigned, and the encryption xattr can't be set, until
> > the jbd2 transaction which creates the inode.  Also, if the new inode is a
> > symlink, then fscrypt_encrypt_symlink() has to be called during the transaction.
> > Together, these imply that fscrypt_get_encryption_info() has to be called during
> > the transaction.
> > 
> 
> Yes, similar problem. I started looking at symlinks today, and got a
> little ways into a patchset to refactor some fscrypt code to handle
> them, but I don't think it's quite right yet. A more general solution
> would be nice.
> 
> > That's what we do, currently.  However, it's technically wrong and can deadlock,
> > since fscrypt_get_encryption_info() isn't GFP_NOFS-safe (and it can't be).
> > 
> > f2fs appears to have a similar problem, though I'm still investigating.
> > 
> > To fix this, I'm planning to add new functions:
> > 
> >    - fscrypt_prepare_new_inode() will set up the fscrypt_info for a new
> >      'struct inode' which hasn't necessarily had an inode number assigned yet.
> >      It won't set the encryption xattr yet.
> > 
> 
> I more or less have that in 02/14, I think, but if you have something
> else in mind, I'm happy to follow suit.
[...]
> > > Symlink handling in fscrypt will also need to be refactored a bit, as we
> > > won't have an inode before we'll need to encrypt its contents.
> > 
> > Will there be an in-memory inode allocated yet (a 'struct inode'), just with no
> > inode number assigned yet?  If so, my work-in-progress patchset I mentioned
> > earlier should be sufficient to address this.  The order would be:
> > 
> > 	1. fscrypt_prepare_new_inode()
> > 	2. fscrypt_encrypt_symlink()
> > 	3. Assign inode number
> > 
> > 
> > Or does ceph not have a 'struct inode' at all until step (3)?
> 
> No, generally ceph doesn't create an inode until the reply comes in. I
> think we'll need to be able to create a context and encrypt the symlink
> before we issue the call to the server. I started hacking at the fscrypt
> code for this today, but I didn't get very far.
> 
> FWIW, ceph is a bit of an odd netfs protocol in that there is a standard
> "trace" that holds info about dentries and inodes that are created or
> modified as a result of an operation. Most of the dentry/inode cache
> manipulation is done at that point, which is done as part of the reply
> processing.

Your patch "fscrypt: add fscrypt_new_context_from_parent" takes in a directory
and generates an fscrypt_context (a.k.a. an encryption xattr) for a new file
that will be created in that directory.

fscrypt_prepare_new_inode() from my work-in-progress patches would do a bit more
than that.  It would actually set up a "struct fscrypt_info" for a new inode.
That includes the encryption key and all information needed to build the
fscrypt_context.  So, afterwards it will be possible to call
fscrypt_encrypt_symlink() before the fscrypt_context is "saved to disk".
IIUC, that's part of what ceph will need.

The catch is that there will still have to be a 'struct inode' to associate the
'struct fscrypt_info' with.  It won't have to have ->i_ino set yet, but some
other fields (at least ->i_mode and ->i_sb) will have to be set, since lots of
code in fs/crypto/ uses those fields.

I think it would be possible to refactor things to make 'struct fscrypt_info'
more separate from 'struct inode', so that filesystems could create a
'struct fscrypt_info' that isn't associated with an inode yet, then encrypt a
symlink target using it (not caching it in ->i_link as we currently do).

However, it would require a lot of changes.

So I'm wondering if it would be easier to instead change ceph to create and
start initializing the 'struct inode' earlier.  It doesn't have to have an inode
number assigned or be added to the inode cache yet; it just needs to be
allocated in memory and some basic fields need to be initialized.  In theory
it's possible, right?  I'd expect that local filesystems aren't even that much
different, in principle; they start initializing a new 'struct inode' in memory
first, and only later do they *really* create the inode by allocating an inode
number and saving the changes to disk.

- Eric

Re: [RFC PATCH 00/14] ceph+fscrypt: together at last (contexts and filenames)

[Jeff Layton]

On Fri, 2020-08-21 at 19:34 -0700, Eric Biggers wrote:
> On Fri, Aug 21, 2020 at 08:58:35PM -0400, Jeff Layton wrote:
> > > > Ceph (and most other netfs') will need to pre-create a crypto context
> > > > when creating a new inode as we'll need to encrypt some things before we
> > > > have an inode. This patchset stores contexts in an xattr, but that's
> > > > probably not ideal for the final implementation [1].
> > > 
> > > Coincidentally, I've currently working on solving a similar problem.  On ext4,
> > > the inode number can't be assigned, and the encryption xattr can't be set, until
> > > the jbd2 transaction which creates the inode.  Also, if the new inode is a
> > > symlink, then fscrypt_encrypt_symlink() has to be called during the transaction.
> > > Together, these imply that fscrypt_get_encryption_info() has to be called during
> > > the transaction.
> > > 
> > 
> > Yes, similar problem. I started looking at symlinks today, and got a
> > little ways into a patchset to refactor some fscrypt code to handle
> > them, but I don't think it's quite right yet. A more general solution
> > would be nice.
> > 
> > > That's what we do, currently.  However, it's technically wrong and can deadlock,
> > > since fscrypt_get_encryption_info() isn't GFP_NOFS-safe (and it can't be).
> > > 
> > > f2fs appears to have a similar problem, though I'm still investigating.
> > > 
> > > To fix this, I'm planning to add new functions:
> > > 
> > >    - fscrypt_prepare_new_inode() will set up the fscrypt_info for a new
> > >      'struct inode' which hasn't necessarily had an inode number assigned yet.
> > >      It won't set the encryption xattr yet.
> > > 
> > 
> > I more or less have that in 02/14, I think, but if you have something
> > else in mind, I'm happy to follow suit.
> [...]
> > > > Symlink handling in fscrypt will also need to be refactored a bit, as we
> > > > won't have an inode before we'll need to encrypt its contents.
> > > 
> > > Will there be an in-memory inode allocated yet (a 'struct inode'), just with no
> > > inode number assigned yet?  If so, my work-in-progress patchset I mentioned
> > > earlier should be sufficient to address this.  The order would be:
> > > 
> > > 	1. fscrypt_prepare_new_inode()
> > > 	2. fscrypt_encrypt_symlink()
> > > 	3. Assign inode number
> > > 
> > > 
> > > Or does ceph not have a 'struct inode' at all until step (3)?
> > 
> > No, generally ceph doesn't create an inode until the reply comes in. I
> > think we'll need to be able to create a context and encrypt the symlink
> > before we issue the call to the server. I started hacking at the fscrypt
> > code for this today, but I didn't get very far.
> > 
> > FWIW, ceph is a bit of an odd netfs protocol in that there is a standard
> > "trace" that holds info about dentries and inodes that are created or
> > modified as a result of an operation. Most of the dentry/inode cache
> > manipulation is done at that point, which is done as part of the reply
> > processing.
> 
> Your patch "fscrypt: add fscrypt_new_context_from_parent" takes in a directory
> and generates an fscrypt_context (a.k.a. an encryption xattr) for a new file
> that will be created in that directory.
> 
> fscrypt_prepare_new_inode() from my work-in-progress patches would do a bit more
> than that.  It would actually set up a "struct fscrypt_info" for a new inode.
> That includes the encryption key and all information needed to build the
> fscrypt_context.  So, afterwards it will be possible to call
> fscrypt_encrypt_symlink() before the fscrypt_context is "saved to disk".
> IIUC, that's part of what ceph will need.
> 
> The catch is that there will still have to be a 'struct inode' to associate the
> 'struct fscrypt_info' with.  It won't have to have ->i_ino set yet, but some
> other fields (at least ->i_mode and ->i_sb) will have to be set, since lots of
> code in fs/crypto/ uses those fields.
> 
> I think it would be possible to refactor things to make 'struct fscrypt_info'
> more separate from 'struct inode', so that filesystems could create a
> 'struct fscrypt_info' that isn't associated with an inode yet, then encrypt a
> symlink target using it (not caching it in ->i_link as we currently do).
> 
> However, it would require a lot of changes.
> 
> So I'm wondering if it would be easier to instead change ceph to create and
> start initializing the 'struct inode' earlier.  It doesn't have to have an inode
> number assigned or be added to the inode cache yet; it just needs to be
> allocated in memory and some basic fields need to be initialized.  In theory
> it's possible, right?  I'd expect that local filesystems aren't even that much
> different, in principle; they start initializing a new 'struct inode' in memory
> first, and only later do they *really* create the inode by allocating an inode
> number and saving the changes to disk.
> 

It's probably possible. I think we'd just need to attach the nascent
inode to the MDS request tracking structure, and convert that from using
iget5_locked to inode_insert5.

Would we need to do this for all inode types or just symlinks?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH 00/14] ceph+fscrypt: together at last (contexts and filenames)

[Eric Biggers]

On Mon, Aug 24, 2020 at 08:03:35AM -0400, Jeff Layton wrote:
> On Fri, 2020-08-21 at 19:34 -0700, Eric Biggers wrote:
> > On Fri, Aug 21, 2020 at 08:58:35PM -0400, Jeff Layton wrote:
> > > > > Ceph (and most other netfs') will need to pre-create a crypto context
> > > > > when creating a new inode as we'll need to encrypt some things before we
> > > > > have an inode. This patchset stores contexts in an xattr, but that's
> > > > > probably not ideal for the final implementation [1].
> > > > 
> > > > Coincidentally, I've currently working on solving a similar problem.  On ext4,
> > > > the inode number can't be assigned, and the encryption xattr can't be set, until
> > > > the jbd2 transaction which creates the inode.  Also, if the new inode is a
> > > > symlink, then fscrypt_encrypt_symlink() has to be called during the transaction.
> > > > Together, these imply that fscrypt_get_encryption_info() has to be called during
> > > > the transaction.
> > > > 
> > > 
> > > Yes, similar problem. I started looking at symlinks today, and got a
> > > little ways into a patchset to refactor some fscrypt code to handle
> > > them, but I don't think it's quite right yet. A more general solution
> > > would be nice.
> > > 
> > > > That's what we do, currently.  However, it's technically wrong and can deadlock,
> > > > since fscrypt_get_encryption_info() isn't GFP_NOFS-safe (and it can't be).
> > > > 
> > > > f2fs appears to have a similar problem, though I'm still investigating.
> > > > 
> > > > To fix this, I'm planning to add new functions:
> > > > 
> > > >    - fscrypt_prepare_new_inode() will set up the fscrypt_info for a new
> > > >      'struct inode' which hasn't necessarily had an inode number assigned yet.
> > > >      It won't set the encryption xattr yet.
> > > > 
> > > 
> > > I more or less have that in 02/14, I think, but if you have something
> > > else in mind, I'm happy to follow suit.
> > [...]
> > > > > Symlink handling in fscrypt will also need to be refactored a bit, as we
> > > > > won't have an inode before we'll need to encrypt its contents.
> > > > 
> > > > Will there be an in-memory inode allocated yet (a 'struct inode'), just with no
> > > > inode number assigned yet?  If so, my work-in-progress patchset I mentioned
> > > > earlier should be sufficient to address this.  The order would be:
> > > > 
> > > > 	1. fscrypt_prepare_new_inode()
> > > > 	2. fscrypt_encrypt_symlink()
> > > > 	3. Assign inode number
> > > > 
> > > > 
> > > > Or does ceph not have a 'struct inode' at all until step (3)?
> > > 
> > > No, generally ceph doesn't create an inode until the reply comes in. I
> > > think we'll need to be able to create a context and encrypt the symlink
> > > before we issue the call to the server. I started hacking at the fscrypt
> > > code for this today, but I didn't get very far.
> > > 
> > > FWIW, ceph is a bit of an odd netfs protocol in that there is a standard
> > > "trace" that holds info about dentries and inodes that are created or
> > > modified as a result of an operation. Most of the dentry/inode cache
> > > manipulation is done at that point, which is done as part of the reply
> > > processing.
> > 
> > Your patch "fscrypt: add fscrypt_new_context_from_parent" takes in a directory
> > and generates an fscrypt_context (a.k.a. an encryption xattr) for a new file
> > that will be created in that directory.
> > 
> > fscrypt_prepare_new_inode() from my work-in-progress patches would do a bit more
> > than that.  It would actually set up a "struct fscrypt_info" for a new inode.
> > That includes the encryption key and all information needed to build the
> > fscrypt_context.  So, afterwards it will be possible to call
> > fscrypt_encrypt_symlink() before the fscrypt_context is "saved to disk".
> > IIUC, that's part of what ceph will need.
> > 
> > The catch is that there will still have to be a 'struct inode' to associate the
> > 'struct fscrypt_info' with.  It won't have to have ->i_ino set yet, but some
> > other fields (at least ->i_mode and ->i_sb) will have to be set, since lots of
> > code in fs/crypto/ uses those fields.
> > 
> > I think it would be possible to refactor things to make 'struct fscrypt_info'
> > more separate from 'struct inode', so that filesystems could create a
> > 'struct fscrypt_info' that isn't associated with an inode yet, then encrypt a
> > symlink target using it (not caching it in ->i_link as we currently do).
> > 
> > However, it would require a lot of changes.
> > 
> > So I'm wondering if it would be easier to instead change ceph to create and
> > start initializing the 'struct inode' earlier.  It doesn't have to have an inode
> > number assigned or be added to the inode cache yet; it just needs to be
> > allocated in memory and some basic fields need to be initialized.  In theory
> > it's possible, right?  I'd expect that local filesystems aren't even that much
> > different, in principle; they start initializing a new 'struct inode' in memory
> > first, and only later do they *really* create the inode by allocating an inode
> > number and saving the changes to disk.
> > 
> 
> It's probably possible. I think we'd just need to attach the nascent
> inode to the MDS request tracking structure, and convert that from using
> iget5_locked to inode_insert5.
> 
> Would we need to do this for all inode types or just symlinks?

It would be all inodes, since fscrypt_prepare_new_inode() will handle all types
of inodes.

- Eric