[RFC PATCH v3 00/16] ceph+fscrypt: context, filename and symlink support

[RFC PATCH v3 00/16] ceph+fscrypt: context, filename and symlink support

[Jeff Layton]

This is the third posting of the ceph+fscrypt integration work. This
just covers context handling, filename and symlink support.

The main changes since the last set are mainly to address Eric's review
comments. Hopefully this will be much closer to mergeable. Some highlights:

1/ rebase onto Eric's fscrypt-file-creation-v2 tag

2/ fscrypt_context_for_new_inode now takes a void * to hold the context

3/ make fscrypt_fname_disk_to_usr designate whether the returned name
   is a nokey name. This is necessary to close a potential race in
   readdir support

4/ fscrypt_base64_encode/decode remain in fs/crypto (not moved into lib/)

5/ test_dummy_encryption handling is moved into a separate patch, and
   several bugs fixed that resulted in context not being set up
   properly.

6/ symlink handling now works

Content encryption is the next step, but I want to get the fscache
rework done first. It would be nice if we were able to store encrypted
files in the cache, for instance.

This set has been tagged as "ceph-fscrypt-rfc.3" in my tree here:

    https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git

Note that this is still quite preliminary, but my goal is to get a set
merged for v5.11.

Jeff Layton (16):
  vfs: export new_inode_pseudo
  fscrypt: export fscrypt_base64_encode and fscrypt_base64_decode
  fscrypt: export fscrypt_d_revalidate
  fscrypt: add fscrypt_context_for_new_inode
  fscrypt: make fscrypt_fname_disk_to_usr return whether result is nokey
    name
  ceph: add fscrypt ioctls
  ceph: crypto context handling for ceph
  ceph: implement -o test_dummy_encryption mount option
  ceph: preallocate inode for ops that may create one
  ceph: add routine to create context prior to RPC
  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
  ceph: create symlinks with encrypted and base64-encoded targets

 fs/ceph/Makefile        |   1 +
 fs/ceph/crypto.c        | 156 ++++++++++++++++++++++++++++++
 fs/ceph/crypto.h        |  67 +++++++++++++
 fs/ceph/dir.c           | 141 ++++++++++++++++++++-------
 fs/ceph/file.c          |  56 +++++++----
 fs/ceph/inode.c         | 204 ++++++++++++++++++++++++++++++++++------
 fs/ceph/ioctl.c         |  25 +++++
 fs/ceph/mds_client.c    |  94 +++++++++++++-----
 fs/ceph/mds_client.h    |   1 +
 fs/ceph/super.c         |  73 +++++++++++++-
 fs/ceph/super.h         |  18 +++-
 fs/ceph/xattr.c         |  32 +++++++
 fs/crypto/fname.c       |  67 ++++++++++---
 fs/crypto/hooks.c       |   4 +-
 fs/crypto/policy.c      |  35 +++++--
 fs/ext4/dir.c           |   3 +-
 fs/ext4/namei.c         |   6 +-
 fs/f2fs/dir.c           |   3 +-
 fs/inode.c              |   1 +
 fs/ubifs/dir.c          |   4 +-
 include/linux/fscrypt.h |  10 +-
 21 files changed, 860 insertions(+), 141 deletions(-)
 create mode 100644 fs/ceph/crypto.c
 create mode 100644 fs/ceph/crypto.h

-- 
2.26.2

[RFC PATCH v3 01/16] vfs: export new_inode_pseudo

[Jeff Layton]

Ceph needs to be able to allocate inodes ahead of a create that might
involve a fscrypt-encrypted inode. new_inode() almost fits the bill,
but it puts the inode on the sb->s_inodes list and when we go to hash
it, that might be done again.

We could work around that by setting I_CREATING on the new inode, but
that causes ilookup5 to return -ESTALE if something tries to find it
before I_NEW is cleared. To work around all of this, just use
new_inode_pseudo which doesn't add it to the list.

Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/inode.c b/fs/inode.c
index 72c4c347afb7..61554c2477ab 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -935,6 +935,7 @@ struct inode *new_inode_pseudo(struct super_block *sb)
 	}
 	return inode;
 }
+EXPORT_SYMBOL(new_inode_pseudo);
 
 /**
  *	new_inode 	- obtain an inode
-- 
2.26.2

[RFC PATCH v3 02/16] fscrypt: export fscrypt_base64_encode and fscrypt_base64_decode

[Jeff Layton]

Ceph will need to base64-encode some encrypted inode names, so make
these routines, and FSCRYPT_BASE64_CHARS available to modules.

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

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index eb13408b50a7..a1cb6c2c50c4 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -187,10 +187,8 @@ static int fname_decrypt(const struct inode *inode,
 static const char lookup_table[65] =
 	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
 
-#define BASE64_CHARS(nbytes)	DIV_ROUND_UP((nbytes) * 4, 3)
-
 /**
- * base64_encode() - base64-encode some bytes
+ * fscrypt_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.
@@ -200,7 +198,7 @@ static const char lookup_table[65] =
  *
  * Return: length of the encoded string
  */
-static int base64_encode(const u8 *src, int len, char *dst)
+int fscrypt_base64_encode(const u8 *src, int len, char *dst)
 {
 	int i, bits = 0, ac = 0;
 	char *cp = dst;
@@ -218,8 +216,9 @@ static int base64_encode(const u8 *src, int len, char *dst)
 		*cp++ = lookup_table[ac & 0x3f];
 	return cp - dst;
 }
+EXPORT_SYMBOL(fscrypt_base64_encode);
 
-static int base64_decode(const char *src, int len, u8 *dst)
+int fscrypt_base64_decode(const char *src, int len, u8 *dst)
 {
 	int i, bits = 0, ac = 0;
 	const char *p;
@@ -241,6 +240,7 @@ static int base64_decode(const char *src, int len, u8 *dst)
 		return -1;
 	return cp - dst;
 }
+EXPORT_SYMBOL(fscrypt_base64_decode);
 
 bool fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
 				  u32 orig_len, u32 max_len,
@@ -272,7 +272,7 @@ bool fscrypt_fname_encrypted_size(const union fscrypt_policy *policy,
 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);
+	const u32 max_encoded_len = FSCRYPT_BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX);
 	u32 max_presented_len;
 
 	max_presented_len = max(max_encoded_len, max_encrypted_len);
@@ -300,6 +300,45 @@ void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str)
 }
 EXPORT_SYMBOL(fscrypt_fname_free_buffer);
 
+void fscrypt_encode_nokey_name(u32 hash, u32 minor_hash,
+			     const struct fscrypt_str *iname,
+			     struct fscrypt_str *oname)
+{
+	struct fscrypt_nokey_name nokey_name;
+	u32 size; /* size of the unencoded no-key name */
+
+	/*
+	 * Sanity check that struct fscrypt_nokey_name doesn't have padding
+	 * between fields and that its encoded size never exceeds NAME_MAX.
+	 */
+	BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, dirhash) !=
+		     offsetof(struct fscrypt_nokey_name, bytes));
+	BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) !=
+		     offsetof(struct fscrypt_nokey_name, sha256));
+	BUILD_BUG_ON(FSCRYPT_BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX);
+
+	if (hash) {
+		nokey_name.dirhash[0] = hash;
+		nokey_name.dirhash[1] = minor_hash;
+	} else {
+		nokey_name.dirhash[0] = 0;
+		nokey_name.dirhash[1] = 0;
+	}
+	if (iname->len <= sizeof(nokey_name.bytes)) {
+		memcpy(nokey_name.bytes, iname->name, iname->len);
+		size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]);
+	} else {
+		memcpy(nokey_name.bytes, iname->name, sizeof(nokey_name.bytes));
+		/* Compute strong hash of remaining part of name. */
+		fscrypt_do_sha256(&iname->name[sizeof(nokey_name.bytes)],
+				  iname->len - sizeof(nokey_name.bytes),
+				  nokey_name.sha256);
+		size = FSCRYPT_NOKEY_NAME_MAX;
+	}
+	oname->len = fscrypt_base64_encode((const u8 *)&nokey_name, size, oname->name);
+}
+EXPORT_SYMBOL(fscrypt_encode_nokey_name);
+
 /**
  * fscrypt_fname_disk_to_usr() - convert an encrypted filename to
  *				 user-presentable form
@@ -351,7 +390,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
 		     offsetof(struct fscrypt_nokey_name, bytes));
 	BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) !=
 		     offsetof(struct fscrypt_nokey_name, sha256));
-	BUILD_BUG_ON(BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX);
+	BUILD_BUG_ON(FSCRYPT_BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX);
 
 	if (hash) {
 		nokey_name.dirhash[0] = hash;
@@ -371,7 +410,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
 				  nokey_name.sha256);
 		size = FSCRYPT_NOKEY_NAME_MAX;
 	}
-	oname->len = base64_encode((const u8 *)&nokey_name, size, oname->name);
+	oname->len = fscrypt_base64_encode((const u8 *)&nokey_name, size, oname->name);
 	return 0;
 }
 EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
@@ -445,14 +484,14 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 	 * user-supplied name
 	 */
 
-	if (iname->len > BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX))
+	if (iname->len > FSCRYPT_BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX))
 		return -ENOENT;
 
 	fname->crypto_buf.name = kmalloc(FSCRYPT_NOKEY_NAME_MAX, GFP_KERNEL);
 	if (fname->crypto_buf.name == NULL)
 		return -ENOMEM;
 
-	ret = base64_decode(iname->name, iname->len, fname->crypto_buf.name);
+	ret = fscrypt_base64_decode(iname->name, iname->len, fname->crypto_buf.name);
 	if (ret < (int)offsetof(struct fscrypt_nokey_name, bytes[1]) ||
 	    (ret > offsetof(struct fscrypt_nokey_name, sha256) &&
 	     ret != FSCRYPT_NOKEY_NAME_MAX)) {
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index b3b0c5675c6b..95dddba3ed00 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -182,6 +182,10 @@ void fscrypt_free_inode(struct inode *inode);
 int fscrypt_drop_inode(struct inode *inode);
 
 /* fname.c */
+#define FSCRYPT_BASE64_CHARS(nbytes)	DIV_ROUND_UP((nbytes) * 4, 3)
+
+int fscrypt_base64_encode(const u8 *src, int len, char *dst);
+int fscrypt_base64_decode(const char *src, int len, u8 *dst);
 int fscrypt_setup_filename(struct inode *inode, const struct qstr *iname,
 			   int lookup, struct fscrypt_name *fname);
 
-- 
2.26.2

[RFC PATCH v3 03/16] 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 a1cb6c2c50c4..0d41eb4a5493 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -578,7 +578,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;
@@ -617,6 +617,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 95dddba3ed00..b547e1aabb00 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -204,6 +204,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 v3 04/16] fscrypt: add fscrypt_context_for_new_inode

[Jeff Layton]

CephFS will need to be able to generate a context for a new "prepared"
inode. Add a new routine for getting the context out of an in-core
inode.

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

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 97cf07543651..0888f950367b 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -655,6 +655,31 @@ const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir)
 	return fscrypt_get_dummy_policy(dir->i_sb);
 }
 
+/**
+ * fscrypt_context_for_new_inode() - create an encryption context for a new inode
+ * @ctx: where context should be written
+ * @inode: inode from which to fetch policy and nonce
+ *
+ * Given an in-core "prepared" (via fscrypt_prepare_new_inode) inode,
+ * generate a new context and write it to ctx. ctx _must_ be at least
+ * FSCRYPT_SET_CONTEXT_MAX_SIZE bytes.
+ *
+ * Returns size of the resulting context or a negative error code.
+ */
+int fscrypt_context_for_new_inode(void *ctx, struct inode *inode)
+{
+	struct fscrypt_info *ci = inode->i_crypt_info;
+
+	BUILD_BUG_ON(sizeof(union fscrypt_context) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
+
+	/* fscrypt_prepare_new_inode() should have set up the key already. */
+	if (WARN_ON_ONCE(!ci))
+		return -ENOKEY;
+
+	return fscrypt_new_context(ctx, &ci->ci_policy, ci->ci_nonce);
+}
+EXPORT_SYMBOL_GPL(fscrypt_context_for_new_inode);
+
 /**
  * fscrypt_set_context() - Set the fscrypt context of a new inode
  * @inode: a new inode
@@ -671,12 +696,9 @@ int fscrypt_set_context(struct inode *inode, void *fs_data)
 	union fscrypt_context ctx;
 	int ctxsize;
 
-	/* fscrypt_prepare_new_inode() should have set up the key already. */
-	if (WARN_ON_ONCE(!ci))
-		return -ENOKEY;
-
-	BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
-	ctxsize = fscrypt_new_context(&ctx, &ci->ci_policy, ci->ci_nonce);
+	ctxsize = fscrypt_context_for_new_inode(&ctx, inode);
+	if (ctxsize < 0)
+		return ctxsize;
 
 	/*
 	 * This may be the first time the inode number is available, so do any
@@ -689,7 +711,6 @@ int fscrypt_set_context(struct inode *inode, void *fs_data)
 
 		fscrypt_hash_inode_number(ci, mk);
 	}
-
 	return inode->i_sb->s_cop->set_context(inode, &ctx, ctxsize, fs_data);
 }
 EXPORT_SYMBOL_GPL(fscrypt_set_context);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index b547e1aabb00..a57d2a9869eb 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -148,6 +148,7 @@ 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_set_context(struct inode *inode, void *fs_data);
+int fscrypt_context_for_new_inode(void *ctx, struct inode *inode);
 
 struct fscrypt_dummy_policy {
 	const union fscrypt_policy *policy;
-- 
2.26.2

[RFC PATCH v3 05/16] fscrypt: make fscrypt_fname_disk_to_usr return whether result is nokey name

[Jeff Layton]

Ceph will sometimes need to know whether the resulting name from this
function is a nokey name, in order to set the dentry flags without
racy checks on the parent inode.

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

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 0d41eb4a5493..b97a81ccd838 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -353,6 +353,7 @@ EXPORT_SYMBOL(fscrypt_encode_nokey_name);
  * @oname: output buffer for the user-presentable filename.  The caller must
  *	   have allocated enough space for this, e.g. using
  *	   fscrypt_fname_alloc_buffer().
+ * @is_nokey: set to true if oname is a no-key name
  *
  * If the key is available, we'll decrypt the disk name.  Otherwise, we'll
  * encode it for presentation in fscrypt_nokey_name format.
@@ -363,7 +364,8 @@ EXPORT_SYMBOL(fscrypt_encode_nokey_name);
 int fscrypt_fname_disk_to_usr(const struct inode *inode,
 			      u32 hash, u32 minor_hash,
 			      const struct fscrypt_str *iname,
-			      struct fscrypt_str *oname)
+			      struct fscrypt_str *oname,
+			      bool *is_nokey)
 {
 	const struct qstr qname = FSTR_TO_QSTR(iname);
 	struct fscrypt_nokey_name nokey_name;
@@ -411,6 +413,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
 		size = FSCRYPT_NOKEY_NAME_MAX;
 	}
 	oname->len = fscrypt_base64_encode((const u8 *)&nokey_name, size, oname->name);
+	*is_nokey = true;
 	return 0;
 }
 EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 42f5ee9f592d..cdad06b4e521 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -310,7 +310,7 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 {
 	const struct fscrypt_symlink_data *sd;
 	struct fscrypt_str cstr, pstr;
-	bool has_key;
+	bool has_key, is_nokey = false;
 	int err;
 
 	/* This is for encrypted symlinks only */
@@ -352,7 +352,7 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 	if (err)
 		return ERR_PTR(err);
 
-	err = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr);
+	err = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr, &is_nokey);
 	if (err)
 		goto err_kfree;
 
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index efe77cffc322..ac6b04bebcf6 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -260,6 +260,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
 					    get_dtype(sb, de->file_type)))
 						goto done;
 				} else {
+					bool is_nokey = false;
 					int save_len = fstr.len;
 					struct fscrypt_str de_name =
 							FSTR_INIT(de->name,
@@ -267,7 +268,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
 
 					/* Directory is encrypted */
 					err = fscrypt_fname_disk_to_usr(inode,
-						0, 0, &de_name, &fstr);
+						0, 0, &de_name, &fstr, &is_nokey);
 					de_name = fstr;
 					fstr.len = save_len;
 					if (err)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 0d74615fcce3..3ba407833be5 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -658,6 +658,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 					       (unsigned) ((char *) de
 							   - base));
 				} else {
+					bool is_nokey = false;
 					struct fscrypt_str de_name =
 						FSTR_INIT(name, len);
 
@@ -671,7 +672,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 							"crypto\n");
 					res = fscrypt_fname_disk_to_usr(dir,
 						0, 0, &de_name,
-						&fname_crypto_str);
+						&fname_crypto_str, &is_nokey);
 					if (res) {
 						printk(KERN_WARNING "Error "
 							"converting filename "
@@ -1045,6 +1046,7 @@ static int htree_dirblock_to_tree(struct file *dir_file,
 				   hinfo->hash, hinfo->minor_hash, de,
 				   &tmp_str);
 		} else {
+			bool is_nokey = false;
 			int save_len = fname_crypto_str.len;
 			struct fscrypt_str de_name = FSTR_INIT(de->name,
 								de->name_len);
@@ -1052,7 +1054,7 @@ static int htree_dirblock_to_tree(struct file *dir_file,
 			/* Directory is encrypted */
 			err = fscrypt_fname_disk_to_usr(dir, hinfo->hash,
 					hinfo->minor_hash, &de_name,
-					&fname_crypto_str);
+					&fname_crypto_str, &is_nokey);
 			if (err) {
 				count = err;
 				goto errout;
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 414bc94fbd54..d235e40210cf 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -985,11 +985,12 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct f2fs_dentry_ptr *d,
 		}
 
 		if (IS_ENCRYPTED(d->inode)) {
+			bool is_nokey = false;
 			int save_len = fstr->len;
 
 			err = fscrypt_fname_disk_to_usr(d->inode,
 						(u32)le32_to_cpu(de->hash_code),
-						0, &de_name, fstr);
+						0, &de_name, fstr, &is_nokey);
 			if (err)
 				goto out;
 
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 155521e51ac5..da1a4bc861d5 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -584,12 +584,14 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 		fname_name(&nm) = dent->name;
 
 		if (encrypted) {
+			bool is_nokey = false;
+
 			fstr.len = fstr_real_len;
 
 			err = fscrypt_fname_disk_to_usr(dir, key_hash_flash(c,
 							&dent->key),
 							le32_to_cpu(dent->cookie),
-							&nm.disk_name, &fstr);
+							&nm.disk_name, &fstr, &is_nokey);
 			if (err)
 				goto out;
 		} else {
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index a57d2a9869eb..bae18aa4173a 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -201,7 +201,7 @@ void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str);
 int fscrypt_fname_disk_to_usr(const struct inode *inode,
 			      u32 hash, u32 minor_hash,
 			      const struct fscrypt_str *iname,
-			      struct fscrypt_str *oname);
+			      struct fscrypt_str *oname, bool *is_nokey);
 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);
@@ -442,7 +442,7 @@ static inline void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str)
 static inline int fscrypt_fname_disk_to_usr(const struct inode *inode,
 					    u32 hash, u32 minor_hash,
 					    const struct fscrypt_str *iname,
-					    struct fscrypt_str *oname)
+					    struct fscrypt_str *oname, bool *is_nokey)
 {
 	return -EOPNOTSUPP;
 }
-- 
2.26.2

[RFC PATCH v3 06/16] ceph: add fscrypt ioctls

[Jeff Layton]

Boilerplate ioctls for controlling encryption.

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

diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
index 6e061bf62ad4..381e44b2d60a 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,30 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 	case CEPH_IOC_SYNCIO:
 		return ceph_ioctl_syncio(file);
+
+	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);
 	}
 
 	return -ENOTTY;
-- 
2.26.2

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

[Jeff Layton]

Store the fscrypt context for an inode as an encryption.ctx xattr.
When we get a new inode in a trace, set the S_ENCRYPTED bit if
the xattr blob has an encryption.ctx xattr.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/Makefile |  1 +
 fs/ceph/crypto.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ceph/crypto.h | 24 ++++++++++++++++++++++++
 fs/ceph/inode.c  |  4 ++++
 fs/ceph/super.c  |  5 +++++
 fs/ceph/super.h  |  1 +
 fs/ceph/xattr.c  | 32 +++++++++++++++++++++++++++++++
 7 files changed, 116 insertions(+)
 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..74f07d44dbe9
--- /dev/null
+++ b/fs/ceph/crypto.c
@@ -0,0 +1,49 @@
+// 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)
+{
+	return __ceph_getxattr(inode, CEPH_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len);
+}
+
+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 = {
+	.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,
+};
+
+void ceph_fscrypt_set_ops(struct super_block *sb)
+{
+	fscrypt_set_ops(sb, &ceph_fscrypt_ops);
+}
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
new file mode 100644
index 000000000000..b5f38ee80553
--- /dev/null
+++ b/fs/ceph/crypto.h
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ceph fscrypt functionality
+ */
+
+#ifndef _CEPH_CRYPTO_H
+#define _CEPH_CRYPTO_H
+
+#ifdef CONFIG_FS_ENCRYPTION
+
+#define	CEPH_XATTR_NAME_ENCRYPTION_CONTEXT	"encryption.ctx"
+
+void ceph_fscrypt_set_ops(struct super_block *sb);
+
+#else /* CONFIG_FS_ENCRYPTION */
+
+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 526faf4778ce..daae18267fd8 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -549,6 +549,7 @@ void ceph_evict_inode(struct inode *inode)
 
 	percpu_counter_dec(&mdsc->metric.total_inodes);
 
+	fscrypt_put_encryption_info(inode);
 	truncate_inode_pages_final(&inode->i_data);
 	clear_inode(inode);
 
@@ -912,6 +913,9 @@ 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) &&
+		     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.c b/fs/ceph/super.c
index b3fc9bb61afc..055180218224 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>
@@ -984,6 +985,10 @@ 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);
 	if (ret != 0)
 		fsc->sb = NULL;
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 483a52d281cd..cc39cc36de77 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -985,6 +985,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 v3 08/16] ceph: implement -o test_dummy_encryption mount option

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/crypto.c |  7 ++---
 fs/ceph/super.c  | 74 +++++++++++++++++++++++++++++++++++++++++++-----
 fs/ceph/super.h  |  7 ++++-
 3 files changed, 76 insertions(+), 12 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 74f07d44dbe9..879d9a0d3751 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -29,16 +29,15 @@ static bool ceph_crypt_empty_dir(struct inode *inode)
 	return ci->i_rsubdirs + ci->i_rfiles == 1;
 }
 
-static const union fscrypt_context *
-ceph_get_dummy_context(struct super_block *sb)
+static const union fscrypt_policy *ceph_get_dummy_policy(struct super_block *sb)
 {
-	return ceph_sb_to_client(sb)->dummy_enc_ctx.ctx;
+	return ceph_sb_to_client(sb)->dummy_enc_policy.policy;
 }
 
 static struct fscrypt_operations ceph_fscrypt_ops = {
 	.get_context		= ceph_crypt_get_context,
 	.set_context		= ceph_crypt_set_context,
-	.get_dummy_context	= ceph_get_dummy_context,
+	.get_dummy_policy	= ceph_get_dummy_policy,
 	.empty_dir		= ceph_crypt_empty_dir,
 	.max_namelen		= NAME_MAX,
 };
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 055180218224..eefdea360c50 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -45,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_policy(&fsc->dummy_enc_policy);
 	ceph_mdsc_close_sessions(fsc->mdsc);
 }
 
@@ -160,6 +161,7 @@ enum {
 	Opt_quotadf,
 	Opt_copyfrom,
 	Opt_wsync,
+	Opt_test_dummy_encryption,
 };
 
 enum ceph_recover_session_mode {
@@ -198,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	("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),
 	{}
@@ -456,6 +460,16 @@ 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);
+#ifdef CONFIG_FS_ENCRYPTION
+		fsopt->test_dummy_encryption = param->string;
+		param->string = NULL;
+		fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
+#else
+		warnfc(fc, "FS encryption not supported: test_dummy_encryption mount option ignored");
+#endif
+		break;
 	default:
 		BUG();
 	}
@@ -475,6 +489,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);
 }
 
@@ -582,6 +597,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)
@@ -964,6 +981,43 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc,
 	return ERR_PTR(err);
 }
 
+#ifdef CONFIG_FS_ENCRYPTION
+static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
+						struct ceph_mount_options *fsopt)
+{
+	struct ceph_fs_client *fsc = sb->s_fs_info;
+
+	if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
+		substring_t arg = { };
+
+		/*
+		 * No changing encryption context on remount. Note that
+		 * fscrypt_set_test_dummy_encryption will validate the version
+		 * string passed in (if any).
+		 */
+		if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy)
+			return -EEXIST;
+
+		/* 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_policy);
+	} else {
+		if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && fsc->dummy_enc_policy.policy)
+			return -EEXIST;
+	}
+	return 0;
+}
+#else
+static inline int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
+						struct ceph_mount_options *fsopt)
+{
+	return 0;
+}
+#endif
+
 static int ceph_set_super(struct super_block *s, struct fs_context *fc)
 {
 	struct ceph_fs_client *fsc = s->s_fs_info;
@@ -985,12 +1039,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;
+	ceph_fscrypt_set_ops(s);
 
-	ret = set_anon_super_fc(s, fc);
-	if (ret != 0)
+	ret = ceph_set_test_dummy_encryption(s, fc, fsc->mount_options);
+	if (!ret)
+		ret = set_anon_super_fc(s, fc);
+	if (ret)
 		fsc->sb = NULL;
 	return ret;
 }
@@ -1136,16 +1190,22 @@ static void ceph_free_fc(struct fs_context *fc)
 
 static int ceph_reconfigure_fc(struct fs_context *fc)
 {
+	int err;
 	struct ceph_parse_opts_ctx *pctx = fc->fs_private;
 	struct ceph_mount_options *fsopt = pctx->opts;
-	struct ceph_fs_client *fsc = ceph_sb_to_client(fc->root->d_sb);
+	struct super_block *sb = fc->root->d_sb;
+	struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
 
 	if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)
 		ceph_set_mount_opt(fsc, ASYNC_DIROPS);
 	else
 		ceph_clear_mount_opt(fsc, ASYNC_DIROPS);
 
-	sync_filesystem(fc->root->d_sb);
+	err = ceph_set_test_dummy_encryption(sb, fc, fsopt);
+	if (err)
+		return err;
+
+	sync_filesystem(sb);
 	return 0;
 }
 
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index cc39cc36de77..11032b30a14f 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>
 
@@ -44,6 +45,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 +98,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 +138,11 @@ struct ceph_fs_client {
 #ifdef CONFIG_CEPH_FSCACHE
 	struct fscache_cookie *fscache;
 #endif
+#ifdef CONFIG_FS_ENCRYPTION
+	struct fscrypt_dummy_policy dummy_enc_policy;
+#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 v3 09/16] ceph: preallocate inode for ops that may create one

[Jeff Layton]

When creating a new inode, we need to determine the crypto context
before we can transmit the RPC. The fscrypt API has a routine for getting
a crypto context before a create occurs, but it requires an inode.

Change the ceph code to preallocate an inode in advance of a create of
any sort (open(), mknod(), symlink(), etc). Move the existing code that
generates the ACL and SELinux blobs into this routine since that's
mostly common across all the different codepaths.

In most cases, we just want to allow ceph_fill_trace to use that inode
after the reply comes in, so add a new field to the MDS request for it
(r_new_inode).

The async create codepath is a bit different though. In that case, we
want to hash the inode in advance of the RPC so that it can be used
before the reply comes in. If the call subsequently fails with
-EJUKEBOX, then just put the references and clean up the as_ctx. Note
that with this change, we now need to regenerate the as_ctx when this
occurs, but it's quite rare for it to happen.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/dir.c        | 49 ++++++++++++++++++--------------
 fs/ceph/file.c       | 56 +++++++++++++++++++++++--------------
 fs/ceph/inode.c      | 66 +++++++++++++++++++++++++++++++++++++++-----
 fs/ceph/mds_client.c |  1 +
 fs/ceph/mds_client.h |  1 +
 fs/ceph/super.h      |  5 +++-
 6 files changed, 130 insertions(+), 48 deletions(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index a4d48370b2b3..b1e04d7d4b68 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -839,13 +839,6 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 		goto out;
 	}
 
-	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
-	if (err < 0)
-		goto out;
-	err = ceph_security_init_secctx(dentry, mode, &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);
@@ -853,6 +846,14 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 		err = PTR_ERR(req);
 		goto out;
 	}
+
+	req->r_new_inode = ceph_new_inode(dir, dentry, &mode, &as_ctx);
+	if (IS_ERR(req->r_new_inode)) {
+		err = PTR_ERR(req->r_new_inode);
+		req->r_new_inode = NULL;
+		goto out_req;
+	}
+
 	req->r_dentry = dget(dentry);
 	req->r_num_caps = 2;
 	req->r_parent = dir;
@@ -868,6 +869,7 @@ static int ceph_mknod(struct inode *dir, struct dentry *dentry,
 	err = ceph_mdsc_do_request(mdsc, dir, req);
 	if (!err && !req->r_reply_info.head->is_dentry)
 		err = ceph_handle_notrace_create(dir, dentry);
+out_req:
 	ceph_mdsc_put_request(req);
 out:
 	if (!err)
@@ -890,6 +892,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(dir->i_sb);
 	struct ceph_mds_request *req;
 	struct ceph_acl_sec_ctx as_ctx = {};
+	umode_t mode = S_IFLNK | 0777;
 	int err;
 
 	if (ceph_snap(dir) != CEPH_NOSNAP)
@@ -900,21 +903,24 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 		goto out;
 	}
 
-	err = ceph_security_init_secctx(dentry, S_IFLNK | 0777, &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)) {
 		err = PTR_ERR(req);
 		goto out;
 	}
+
+	req->r_new_inode = ceph_new_inode(dir, dentry, &mode, &as_ctx);
+	if (IS_ERR(req->r_new_inode)) {
+		err = PTR_ERR(req->r_new_inode);
+		req->r_new_inode = NULL;
+		goto out_req;
+	}
+
 	req->r_path2 = kstrdup(dest, GFP_KERNEL);
 	if (!req->r_path2) {
 		err = -ENOMEM;
-		ceph_mdsc_put_request(req);
-		goto out;
+		goto out_req;
 	}
 	req->r_parent = dir;
 	set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags);
@@ -929,6 +935,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	err = ceph_mdsc_do_request(mdsc, dir, req);
 	if (!err && !req->r_reply_info.head->is_dentry)
 		err = ceph_handle_notrace_create(dir, dentry);
+out_req:
 	ceph_mdsc_put_request(req);
 out:
 	if (err)
@@ -963,13 +970,6 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 		goto out;
 	}
 
-	mode |= S_IFDIR;
-	err = ceph_pre_init_acls(dir, &mode, &as_ctx);
-	if (err < 0)
-		goto out;
-	err = ceph_security_init_secctx(dentry, mode, &as_ctx);
-	if (err < 0)
-		goto out;
 
 	req = ceph_mdsc_create_request(mdsc, op, USE_AUTH_MDS);
 	if (IS_ERR(req)) {
@@ -977,6 +977,14 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 		goto out;
 	}
 
+	mode |= S_IFDIR;
+	req->r_new_inode = ceph_new_inode(dir, dentry, &mode, &as_ctx);
+	if (IS_ERR(req->r_new_inode)) {
+		err = PTR_ERR(req->r_new_inode);
+		req->r_new_inode = NULL;
+		goto out_req;
+	}
+
 	req->r_dentry = dget(dentry);
 	req->r_num_caps = 2;
 	req->r_parent = dir;
@@ -993,6 +1001,7 @@ static int ceph_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	    !req->r_reply_info.head->is_target &&
 	    !req->r_reply_info.head->is_dentry)
 		err = ceph_handle_notrace_create(dir, dentry);
+out_req:
 	ceph_mdsc_put_request(req);
 out:
 	if (!err)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 762a280b7037..eee21c286d66 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -565,7 +565,8 @@ static void ceph_async_create_cb(struct ceph_mds_client *mdsc,
 	ceph_mdsc_release_dir_caps(req);
 }
 
-static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
+static int ceph_finish_async_create(struct inode *dir, struct inode *inode,
+				    struct dentry *dentry,
 				    struct file *file, umode_t mode,
 				    struct ceph_mds_request *req,
 				    struct ceph_acl_sec_ctx *as_ctx,
@@ -576,17 +577,12 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
 	struct ceph_mds_reply_inode in = { };
 	struct ceph_mds_reply_info_in iinfo = { .in = &in };
 	struct ceph_inode_info *ci = ceph_inode(dir);
-	struct inode *inode;
 	struct timespec64 now;
 	struct ceph_vino vino = { .ino = req->r_deleg_ino,
 				  .snap = CEPH_NOSNAP };
 
 	ktime_get_real_ts64(&now);
 
-	inode = ceph_get_inode(dentry->d_sb, vino);
-	if (IS_ERR(inode))
-		return PTR_ERR(inode);
-
 	iinfo.inline_version = CEPH_INLINE_NONE;
 	iinfo.change_attr = 1;
 	ceph_encode_timespec64(&iinfo.btime, &now);
@@ -622,8 +618,7 @@ static int ceph_finish_async_create(struct inode *dir, struct dentry *dentry,
 		ceph_dir_clear_complete(dir);
 		if (!d_unhashed(dentry))
 			d_drop(dentry);
-		if (inode->i_state & I_NEW)
-			discard_new_inode(inode);
+		discard_new_inode(inode);
 	} else {
 		struct dentry *dn;
 
@@ -663,6 +658,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mds_request *req;
+	struct inode *new_inode = NULL;
 	struct dentry *dn;
 	struct ceph_acl_sec_ctx as_ctx = {};
 	bool try_async = ceph_test_mount_opt(fsc, ASYNC_DIROPS);
@@ -675,21 +671,21 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 
 	if (dentry->d_name.len > NAME_MAX)
 		return -ENAMETOOLONG;
-
+retry:
 	if (flags & O_CREAT) {
 		if (ceph_quota_is_max_files_exceeded(dir))
 			return -EDQUOT;
-		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
-		if (err < 0)
-			return err;
-		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
-		if (err < 0)
+
+		new_inode = ceph_new_inode(dir, dentry, &mode, &as_ctx);
+		if (IS_ERR(new_inode)) {
+			err = PTR_ERR(new_inode);
 			goto out_ctx;
+		}
 	} else if (!d_in_lookup(dentry)) {
 		/* If it's not being looked up, it's negative */
 		return -ENOENT;
 	}
-retry:
+
 	/* do the open */
 	req = prepare_open_request(dir->i_sb, flags, mode);
 	if (IS_ERR(req)) {
@@ -713,21 +709,38 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 			req->r_pagelist = as_ctx.pagelist;
 			as_ctx.pagelist = NULL;
 		}
-		if (try_async &&
-		    (req->r_dir_caps =
-		      try_prep_async_create(dir, dentry, &lo,
-					    &req->r_deleg_ino))) {
+
+		if (try_async && (req->r_dir_caps =
+				  try_prep_async_create(dir, dentry, &lo, &req->r_deleg_ino))) {
+			struct ceph_vino vino = { .ino = req->r_deleg_ino,
+						  .snap = CEPH_NOSNAP };
+
 			set_bit(CEPH_MDS_R_ASYNC, &req->r_req_flags);
 			req->r_args.open.flags |= cpu_to_le32(CEPH_O_EXCL);
 			req->r_callback = ceph_async_create_cb;
+
+			/* Hash inode before RPC */
+			new_inode = ceph_get_inode(dir->i_sb, vino, new_inode);
+			if (IS_ERR(new_inode)) {
+				err = PTR_ERR(new_inode);
+				new_inode = NULL;
+				goto out_req;
+			}
+			WARN_ON_ONCE(!(new_inode->i_state & I_NEW));
+
 			err = ceph_mdsc_submit_request(mdsc, dir, req);
 			if (!err) {
-				err = ceph_finish_async_create(dir, dentry,
+				err = ceph_finish_async_create(dir, new_inode, dentry,
 							file, mode, req,
 							&as_ctx, &lo);
+				new_inode = NULL;
 			} else if (err == -EJUKEBOX) {
 				restore_deleg_ino(dir, req->r_deleg_ino);
 				ceph_mdsc_put_request(req);
+				discard_new_inode(new_inode);
+				ceph_release_acl_sec_ctx(&as_ctx);
+				memset(&as_ctx, 0, sizeof(as_ctx));
+				new_inode = NULL;
 				try_async = false;
 				goto retry;
 			}
@@ -736,6 +749,8 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	}
 
 	set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags);
+	req->r_new_inode = new_inode;
+	new_inode = NULL;
 	err = ceph_mdsc_do_request(mdsc,
 				   (flags & (O_CREAT|O_TRUNC)) ? dir : NULL,
 				   req);
@@ -773,6 +788,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	}
 out_req:
 	ceph_mdsc_put_request(req);
+	iput(new_inode);
 out_ctx:
 	ceph_release_acl_sec_ctx(&as_ctx);
 	dout("atomic_open result=%d\n", err);
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index daae18267fd8..f50e7873bf6e 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -52,15 +52,67 @@ static int ceph_set_ino_cb(struct inode *inode, void *data)
 	return 0;
 }
 
-struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino)
+/**
+ * ceph_new_inode - allocate a new inode in advance of an expected create
+ * @dir: parent directory for new inode
+ * @mode: mode of new inode
+ */
+struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
+			     umode_t *mode, struct ceph_acl_sec_ctx *as_ctx)
 {
+	int err;
 	struct inode *inode;
 
-	inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare,
-			     ceph_set_ino_cb, &vino);
+	inode = new_inode_pseudo(dir->i_sb);
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
+	if (!S_ISLNK(*mode)) {
+		err = ceph_pre_init_acls(dir, mode, as_ctx);
+		if (err < 0)
+			goto out_err;
+	}
+
+	err = ceph_security_init_secctx(dentry, *mode, as_ctx);
+	if (err < 0)
+		goto out_err;
+
+	inode->i_state = 0;
+	inode->i_mode = *mode;
+	return inode;
+out_err:
+	iput(inode);
+	return ERR_PTR(err);
+}
+
+/**
+ * ceph_get_inode - find or create/hash a new inode
+ * @sb: superblock to search and allocate in
+ * @vino: vino to search for
+ * @new: optional new inode to insert if one isn't found (may be NULL)
+ *
+ * Search for or insert a new inode into the hash for the given vino, and return a
+ * reference to it. If new is non-NULL, its reference is consumed.
+ */
+struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino, struct inode *new)
+{
+	struct inode *inode;
+
+	if (new) {
+		inode = inode_insert5(new, (unsigned long)vino.ino, ceph_ino_compare,
+					ceph_set_ino_cb, &vino);
+		if (inode != new)
+			iput(new);
+	} else {
+		inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare,
+				     ceph_set_ino_cb, &vino);
+	}
+
+	if (!inode) {
+		dout("No inode found for %llx.%llx\n", vino.ino, vino.snap);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	dout("get_inode on %llu=%llx.%llx got %p new %d\n", ceph_present_inode(inode),
 	     ceph_vinop(inode), inode, !!(inode->i_state & I_NEW));
 	return inode;
@@ -75,7 +127,7 @@ struct inode *ceph_get_snapdir(struct inode *parent)
 		.ino = ceph_ino(parent),
 		.snap = CEPH_SNAPDIR,
 	};
-	struct inode *inode = ceph_get_inode(parent->i_sb, vino);
+	struct inode *inode = ceph_get_inode(parent->i_sb, vino, NULL);
 	struct ceph_inode_info *ci = ceph_inode(inode);
 
 	BUG_ON(!S_ISDIR(parent->i_mode));
@@ -1322,7 +1374,7 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 		tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
 		tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
 
-		in = ceph_get_inode(sb, tvino);
+		in = ceph_get_inode(sb, tvino, xchg(&req->r_new_inode, NULL));
 		if (IS_ERR(in)) {
 			err = PTR_ERR(in);
 			goto done;
@@ -1516,7 +1568,7 @@ static int readdir_prepopulate_inodes_only(struct ceph_mds_request *req,
 		vino.ino = le64_to_cpu(rde->inode.in->ino);
 		vino.snap = le64_to_cpu(rde->inode.in->snapid);
 
-		in = ceph_get_inode(req->r_dentry->d_sb, vino);
+		in = ceph_get_inode(req->r_dentry->d_sb, vino, NULL);
 		if (IS_ERR(in)) {
 			err = PTR_ERR(in);
 			dout("new_inode badness got %d\n", err);
@@ -1720,7 +1772,7 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 		if (d_really_is_positive(dn)) {
 			in = d_inode(dn);
 		} else {
-			in = ceph_get_inode(parent->d_sb, tvino);
+			in = ceph_get_inode(parent->d_sb, tvino, NULL);
 			if (IS_ERR(in)) {
 				dout("new_inode badness\n");
 				d_drop(dn);
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 76d8d9495d1d..4107dc64cc8c 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -820,6 +820,7 @@ void ceph_mdsc_release_request(struct kref *kref)
 		ceph_async_iput(req->r_parent);
 	}
 	ceph_async_iput(req->r_target_inode);
+	ceph_async_iput(req->r_new_inode);
 	if (req->r_dentry)
 		dput(req->r_dentry);
 	if (req->r_old_dentry)
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 658800605bfb..63999f7db014 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -260,6 +260,7 @@ struct ceph_mds_request {
 
 	struct inode *r_parent;		    /* parent dir inode */
 	struct inode *r_target_inode;       /* resulting inode */
+	struct inode *r_new_inode;	    /* new inode (for creates) */
 
 #define CEPH_MDS_R_DIRECT_IS_HASH	(1) /* r_direct_hash is valid */
 #define CEPH_MDS_R_ABORTED		(2) /* call was aborted */
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 11032b30a14f..c05ed00035ec 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -936,6 +936,7 @@ static inline bool __ceph_have_pending_cap_snap(struct ceph_inode_info *ci)
 /* inode.c */
 struct ceph_mds_reply_info_in;
 struct ceph_mds_reply_dirfrag;
+struct ceph_acl_sec_ctx;
 
 extern const struct inode_operations ceph_file_iops;
 
@@ -943,8 +944,10 @@ extern struct inode *ceph_alloc_inode(struct super_block *sb);
 extern void ceph_evict_inode(struct inode *inode);
 extern void ceph_free_inode(struct inode *inode);
 
+struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
+			     umode_t *mode, struct ceph_acl_sec_ctx *as_ctx);
 extern struct inode *ceph_get_inode(struct super_block *sb,
-				    struct ceph_vino vino);
+				    struct ceph_vino vino, struct inode *new);
 extern struct inode *ceph_get_snapdir(struct inode *parent);
 extern int ceph_fill_file_size(struct inode *inode, int issued,
 			       u32 truncate_seq, u64 truncate_size, u64 size);
-- 
2.26.2

[RFC PATCH v3 10/16] ceph: add routine to create context prior to RPC

[Jeff Layton]

After pre-creating a new inode, do an fscrypt prepare on it, fetch a
new encryption context and then marshal that into the security context
to be sent along with the RPC.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/crypto.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ceph/crypto.h |  8 +++++++
 fs/ceph/inode.c  | 10 ++++++--
 fs/ceph/super.h  |  3 +++
 4 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 879d9a0d3751..f037a4939026 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -46,3 +46,64 @@ void ceph_fscrypt_set_ops(struct super_block *sb)
 {
 	fscrypt_set_ops(sb, &ceph_fscrypt_ops);
 }
+
+int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
+				 struct ceph_acl_sec_ctx *as)
+{
+	int ret, ctxsize;
+	size_t name_len;
+	char *name;
+	struct ceph_pagelist *pagelist = as->pagelist;
+	bool encrypted = false;
+
+	ret = fscrypt_prepare_new_inode(dir, inode, &encrypted);
+	if (ret)
+		return ret;
+	if (!encrypted)
+		return 0;
+
+	inode->i_flags |= S_ENCRYPTED;
+
+	ctxsize = fscrypt_context_for_new_inode(&as->fscrypt, inode);
+	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 b5f38ee80553..c1b6ec4b2961 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -11,6 +11,8 @@
 #define	CEPH_XATTR_NAME_ENCRYPTION_CONTEXT	"encryption.ctx"
 
 void ceph_fscrypt_set_ops(struct super_block *sb);
+int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
+				 struct ceph_acl_sec_ctx *as);
 
 #else /* CONFIG_FS_ENCRYPTION */
 
@@ -19,6 +21,12 @@ static inline int ceph_fscrypt_set_ops(struct super_block *sb)
 	return 0;
 }
 
+static inline int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
+						struct ceph_acl_sec_ctx *as)
+{
+	return 0;
+}
+
 #endif /* CONFIG_FS_ENCRYPTION */
 
 #endif
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index f50e7873bf6e..2fda08518312 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>
 
 /*
@@ -73,12 +74,17 @@ struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
 			goto out_err;
 	}
 
+	inode->i_state = 0;
+	inode->i_mode = *mode;
+
 	err = ceph_security_init_secctx(dentry, *mode, as_ctx);
 	if (err < 0)
 		goto out_err;
 
-	inode->i_state = 0;
-	inode->i_mode = *mode;
+	err = ceph_fscrypt_prepare_context(dir, inode, as_ctx);
+	if (err)
+		goto out_err;
+
 	return inode;
 out_err:
 	iput(inode);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index c05ed00035ec..5d5283552c03 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -1003,6 +1003,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 v3 11/16] 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 4107dc64cc8c..e3dc061252d4 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2327,7 +2327,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;
@@ -2344,34 +2345,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? */
@@ -2380,8 +2382,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 v3 12/16] 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 | 70 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 54 insertions(+), 16 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index e3dc061252d4..7eb504170981 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2314,18 +2314,27 @@ static inline  u64 __get_oldest_tid(struct ceph_mds_client *mdsc)
 	return mdsc->oldest_tid;
 }
 
-/*
- * Build a dentry's path.  Allocate on heap; caller must kfree.  Based
- * on build_path_from_dentry in fs/cifs/dir.c.
+/**
+ * ceph_mdsc_build_path - build a path string to a given dentry
+ * @dentry: dentry to which path should be built
+ * @plen: returned length of string
+ * @pbase: returned base inode number
+ * @for_wire: is this path going to be sent to the MDS?
+ *
+ * Build a string that represents the path to the dentry. This is mostly called
+ * for two different purposes:
+ *
+ * 1) we need to build a path string to send to the MDS (for_wire == true)
+ * 2) we need a path string for local presentation (e.g. debugfs) (for_wire == false)
  *
- * If @stop_on_nosnap, generate path relative to the first non-snapped
- * inode.
+ * The path is built in reverse, starting with the dentry. Walk back up toward
+ * the root, building the path until the first non-snapped inode is reached (for_wire)
+ * or the root inode is reached (!for_wire).
  *
  * 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;
@@ -2347,30 +2356,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[FSCRYPT_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 = fscrypt_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))
@@ -2415,7 +2453,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 v3 13/16] ceph: make d_revalidate call fscrypt revalidator for encrypted dentries

[Jeff Layton]

If we have a dentry which represents a no-key name, then we need to test
whether the parent directory's encryption key has since been added.  Do
that before we test anything else about the dentry.

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

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index b1e04d7d4b68..b6e5d751024d 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1689,6 +1689,10 @@ 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);
 
+	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 v3 14/16] 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 | 35 +++++++++++++++++++++++++++++
 fs/ceph/dir.c    | 58 +++++++++++++++++++++++++++++++++++++++---------
 fs/ceph/inode.c  | 31 +++++++++++++++++++++++---
 4 files changed, 157 insertions(+), 14 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index f037a4939026..e3038c88c7a0 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -107,3 +107,50 @@ int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
 		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,
+			bool *is_nokey)
+{
+	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 = fscrypt_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, is_nokey);
+
+	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 c1b6ec4b2961..df1a093848bb 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"
@@ -14,6 +16,22 @@ void ceph_fscrypt_set_ops(struct super_block *sb);
 int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
 				 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);
+}
+
+int ceph_fname_to_usr(struct inode *parent, char *name, u32 len, struct fscrypt_str *tname,
+			struct fscrypt_str *oname, bool *is_nokey);
+
 #else /* CONFIG_FS_ENCRYPTION */
 
 static inline int ceph_fscrypt_set_ops(struct super_block *sb)
@@ -27,6 +45,23 @@ static inline int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *
 	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_fname_to_usr(struct inode *inode, char *name, u32 len,
+			struct fscrypt_str *tname, struct fscrypt_str *oname, bool *is_nokey)
+{
+	oname->name = dname;
+	oname->len = dlen;
+	return 0;
+}
+
 #endif /* CONFIG_FS_ENCRYPTION */
 
 #endif
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index b6e5d751024d..e9fdb9c07320 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,
@@ -241,7 +242,9 @@ static int __dcache_readdir(struct file *file,  struct dir_context *ctx,
 		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 ||
+		    ((dentry->d_flags & DCACHE_ENCRYPTED_NAME) &&
+		     fscrypt_has_encryption_key(dir))) {
 			spin_unlock(&dentry->d_lock);
 			dput(dentry);
 			err = -EAGAIN;
@@ -313,6 +316,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)
@@ -340,6 +345,12 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		ctx->pos = 2;
 	}
 
+	if (IS_ENCRYPTED(inode)) {
+		err = fscrypt_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. */
@@ -360,6 +371,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? */
@@ -387,12 +406,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;
@@ -405,7 +426,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 =
@@ -426,7 +448,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",
@@ -479,7 +501,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 */
@@ -506,9 +528,12 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
 		}
 	}
 	for (; i < rinfo->dir_nr; i++) {
+		bool is_nokey = false;
 		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",
@@ -517,12 +542,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, &is_nokey);
+		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++;
 	}
 
@@ -577,9 +610,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 2fda08518312..c3a7a9f5603d 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1653,7 +1653,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;
@@ -1664,6 +1665,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);
@@ -1715,14 +1718,29 @@ 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++) {
+		bool is_nokey = false;
 		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, &is_nokey);
+		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);
@@ -1753,6 +1771,11 @@ int ceph_readdir_prepopulate(struct ceph_mds_request *req,
 				err = -ENOMEM;
 				goto out;
 			}
+			if (is_nokey) {
+				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)) {
@@ -1843,6 +1866,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 v3 15/16] 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 | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index c3a7a9f5603d..8e9fb1311bb8 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1331,8 +1331,10 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 		if (dir && req->r_op == CEPH_MDS_OP_LOOKUPNAME &&
 		    test_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags) &&
 		    !test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) {
+			bool is_nokey = false;
 			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);
@@ -1340,8 +1342,21 @@ 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, &is_nokey);
+			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);
@@ -1356,9 +1371,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_nokey) {
+					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 ||
@@ -1370,6 +1391,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

[RFC PATCH v3 16/16] ceph: create symlinks with encrypted and base64-encoded targets

[Jeff Layton]

When creating symlinks in encrypted directories, encrypt and
base64-encode the target with the new inode's key before sending to the
MDS.

When filling a symlinked inode, base64-decode it into a buffer that
we'll keep in ci->i_symlink. When get_link is called, decrypt the buffer
into a new one that will hang off i_link.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/dir.c   | 32 +++++++++++++++++++---
 fs/ceph/inode.c | 71 ++++++++++++++++++++++++++++++++++++++-----------
 fs/ceph/super.h |  2 ++
 3 files changed, 86 insertions(+), 19 deletions(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index e9fdb9c07320..65d82ab239b2 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -928,6 +928,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(dir->i_sb);
 	struct ceph_mds_request *req;
 	struct ceph_acl_sec_ctx as_ctx = {};
+	struct fscrypt_str osd_link = FSTR_INIT(NULL, 0);
 	umode_t mode = S_IFLNK | 0777;
 	int err;
 
@@ -953,11 +954,33 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 		goto out_req;
 	}
 
-	req->r_path2 = kstrdup(dest, GFP_KERNEL);
-	if (!req->r_path2) {
-		err = -ENOMEM;
-		goto out_req;
+	if (IS_ENCRYPTED(req->r_new_inode)) {
+		int len = strlen(dest);
+
+		err = fscrypt_prepare_symlink(dir, dest, len, PATH_MAX, &osd_link);
+		if (err)
+			goto out_req;
+
+		err = fscrypt_encrypt_symlink(req->r_new_inode, dest, len, &osd_link);
+		if (err)
+			goto out_req;
+
+		req->r_path2 = kmalloc(FSCRYPT_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
+		if (!req->r_path2) {
+			err = -ENOMEM;
+			goto out_req;
+		}
+
+		len = fscrypt_base64_encode(osd_link.name, osd_link.len, req->r_path2);
+		req->r_path2[len] = '\0';
+	} else {
+		req->r_path2 = kstrdup(dest, GFP_KERNEL);
+		if (!req->r_path2) {
+			err = -ENOMEM;
+			goto out_req;
+		}
 	}
+
 	req->r_parent = dir;
 	set_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags);
 	req->r_dentry = dget(dentry);
@@ -977,6 +1000,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	if (err)
 		d_drop(dentry);
 	ceph_release_acl_sec_ctx(&as_ctx);
+	fscrypt_fname_free_buffer(&osd_link);
 	return err;
 }
 
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 8e9fb1311bb8..4ac267cc9085 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -33,8 +33,6 @@
  * (typically because they are in the message handler path).
  */
 
-static const struct inode_operations ceph_symlink_iops;
-
 static void ceph_inode_work(struct work_struct *work);
 
 /*
@@ -593,6 +591,7 @@ void ceph_free_inode(struct inode *inode)
 	struct ceph_inode_info *ci = ceph_inode(inode);
 
 	kfree(ci->i_symlink);
+	fscrypt_free_inode(inode);
 	kmem_cache_free(ceph_inode_cachep, ci);
 }
 
@@ -996,26 +995,39 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
 		inode->i_fop = &ceph_file_fops;
 		break;
 	case S_IFLNK:
-		inode->i_op = &ceph_symlink_iops;
 		if (!ci->i_symlink) {
 			u32 symlen = iinfo->symlink_len;
 			char *sym;
 
 			spin_unlock(&ci->i_ceph_lock);
 
-			if (symlen != i_size_read(inode)) {
-				pr_err("%s %llx.%llx BAD symlink "
-					"size %lld\n", __func__,
-					ceph_vinop(inode),
-					i_size_read(inode));
+			if (IS_ENCRYPTED(inode)) {
+				/* Do base64 decode so that we get the right size (maybe?) */
+				err = -ENOMEM;
+				sym = kmalloc(symlen + 1, GFP_NOFS);
+				if (!sym)
+					goto out;
+
+				symlen = fscrypt_base64_decode(iinfo->symlink, symlen, sym);
+				/*
+				 * i_size as reported by the MDS may be wrong, due to base64
+				 * inflation and padding. Fix it up here.
+				 */
 				i_size_write(inode, symlen);
 				inode->i_blocks = calc_inode_blocks(symlen);
-			}
+			} else {
+				if (symlen != i_size_read(inode)) {
+					pr_err("%s %llx.%llx BAD symlink size %lld\n",
+						__func__, ceph_vinop(inode), i_size_read(inode));
+					i_size_write(inode, symlen);
+					inode->i_blocks = calc_inode_blocks(symlen);
+				}
 
-			err = -ENOMEM;
-			sym = kstrndup(iinfo->symlink, symlen, GFP_NOFS);
-			if (!sym)
-				goto out;
+				err = -ENOMEM;
+				sym = kstrndup(iinfo->symlink, symlen, GFP_NOFS);
+				if (!sym)
+					goto out;
+			}
 
 			spin_lock(&ci->i_ceph_lock);
 			if (!ci->i_symlink)
@@ -1023,7 +1035,18 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
 			else
 				kfree(sym); /* lost a race */
 		}
-		inode->i_link = ci->i_symlink;
+
+		if (IS_ENCRYPTED(inode)) {
+			/*
+			 * Encrypted symlinks need to be decrypted before we can
+			 * cache their targets in i_link. Leave it blank for now.
+			 */
+			inode->i_link = NULL;
+			inode->i_op = &ceph_encrypted_symlink_iops;
+		} else {
+			inode->i_link = ci->i_symlink;
+			inode->i_op = &ceph_symlink_iops;
+		}
 		break;
 	case S_IFDIR:
 		inode->i_op = &ceph_dir_iops;
@@ -2124,16 +2147,34 @@ static void ceph_inode_work(struct work_struct *work)
 	iput(inode);
 }
 
+static const char *ceph_encrypted_get_link(struct dentry *dentry, struct inode *inode,
+					   struct delayed_call *done)
+{
+	struct ceph_inode_info *ci = ceph_inode(inode);
+
+	if (!dentry)
+		return ERR_PTR(-ECHILD);
+
+	return fscrypt_get_symlink(inode, ci->i_symlink, ksize(ci->i_symlink), done);
+}
+
 /*
  * symlinks
  */
-static const struct inode_operations ceph_symlink_iops = {
+const struct inode_operations ceph_symlink_iops = {
 	.get_link = simple_get_link,
 	.setattr = ceph_setattr,
 	.getattr = ceph_getattr,
 	.listxattr = ceph_listxattr,
 };
 
+const struct inode_operations ceph_encrypted_symlink_iops = {
+	.get_link = ceph_encrypted_get_link,
+	.setattr = ceph_setattr,
+	.getattr = ceph_getattr,
+	.listxattr = ceph_listxattr,
+};
+
 int __ceph_setattr(struct inode *inode, struct iattr *attr)
 {
 	struct ceph_inode_info *ci = ceph_inode(inode);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 5d5283552c03..d58296bd8235 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -939,6 +939,8 @@ struct ceph_mds_reply_dirfrag;
 struct ceph_acl_sec_ctx;
 
 extern const struct inode_operations ceph_file_iops;
+extern const struct inode_operations ceph_symlink_iops;
+extern const struct inode_operations ceph_encrypted_symlink_iops;
 
 extern struct inode *ceph_alloc_inode(struct super_block *sb);
 extern void ceph_evict_inode(struct inode *inode);
-- 
2.26.2

Re: [RFC PATCH v3 01/16] vfs: export new_inode_pseudo

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:52PM -0400, Jeff Layton wrote:
> Ceph needs to be able to allocate inodes ahead of a create that might
> involve a fscrypt-encrypted inode. new_inode() almost fits the bill,
> but it puts the inode on the sb->s_inodes list and when we go to hash
> it, that might be done again.
> 
> We could work around that by setting I_CREATING on the new inode, but
> that causes ilookup5 to return -ESTALE if something tries to find it
> before I_NEW is cleared. To work around all of this, just use
> new_inode_pseudo which doesn't add it to the list.
> 
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

I'd still like to see an explanation for why ilookup5() shouldn't be fixed
instead.

- Eric

Re: [RFC PATCH v3 02/16] fscrypt: export fscrypt_base64_encode and fscrypt_base64_decode

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:53PM -0400, Jeff Layton wrote:
> Ceph will need to base64-encode some encrypted inode names, so make
> these routines, and FSCRYPT_BASE64_CHARS available to modules.

"inode names" => "filenames"

> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/crypto/fname.c       | 59 ++++++++++++++++++++++++++++++++++-------
>  include/linux/fscrypt.h |  4 +++
>  2 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index eb13408b50a7..a1cb6c2c50c4 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -187,10 +187,8 @@ static int fname_decrypt(const struct inode *inode,
>  static const char lookup_table[65] =
>  	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
>  
> -#define BASE64_CHARS(nbytes)	DIV_ROUND_UP((nbytes) * 4, 3)
> -
>  /**
> - * base64_encode() - base64-encode some bytes
> + * fscrypt_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.
> @@ -200,7 +198,7 @@ static const char lookup_table[65] =
>   *
>   * Return: length of the encoded string
>   */
> -static int base64_encode(const u8 *src, int len, char *dst)
> +int fscrypt_base64_encode(const u8 *src, int len, char *dst)
>  {
>  	int i, bits = 0, ac = 0;
>  	char *cp = dst;
> @@ -218,8 +216,9 @@ static int base64_encode(const u8 *src, int len, char *dst)
>  		*cp++ = lookup_table[ac & 0x3f];
>  	return cp - dst;
>  }
> +EXPORT_SYMBOL(fscrypt_base64_encode);
>  
> -static int base64_decode(const char *src, int len, u8 *dst)
> +int fscrypt_base64_decode(const char *src, int len, u8 *dst)

fscrypt_base64_decode() could use a kerneldoc comment, now that it will be
exported for filesystems to use.

> +void fscrypt_encode_nokey_name(u32 hash, u32 minor_hash,
> +			     const struct fscrypt_str *iname,
> +			     struct fscrypt_str *oname)
> +{
> +	struct fscrypt_nokey_name nokey_name;
> +	u32 size; /* size of the unencoded no-key name */
> +
> +	/*
> +	 * Sanity check that struct fscrypt_nokey_name doesn't have padding
> +	 * between fields and that its encoded size never exceeds NAME_MAX.
> +	 */
> +	BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, dirhash) !=
> +		     offsetof(struct fscrypt_nokey_name, bytes));
> +	BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) !=
> +		     offsetof(struct fscrypt_nokey_name, sha256));
> +	BUILD_BUG_ON(FSCRYPT_BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX);
> +
> +	if (hash) {
> +		nokey_name.dirhash[0] = hash;
> +		nokey_name.dirhash[1] = minor_hash;
> +	} else {
> +		nokey_name.dirhash[0] = 0;
> +		nokey_name.dirhash[1] = 0;
> +	}
> +	if (iname->len <= sizeof(nokey_name.bytes)) {
> +		memcpy(nokey_name.bytes, iname->name, iname->len);
> +		size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]);
> +	} else {
> +		memcpy(nokey_name.bytes, iname->name, sizeof(nokey_name.bytes));
> +		/* Compute strong hash of remaining part of name. */
> +		fscrypt_do_sha256(&iname->name[sizeof(nokey_name.bytes)],
> +				  iname->len - sizeof(nokey_name.bytes),
> +				  nokey_name.sha256);
> +		size = FSCRYPT_NOKEY_NAME_MAX;
> +	}
> +	oname->len = fscrypt_base64_encode((const u8 *)&nokey_name, size, oname->name);
> +}
> +EXPORT_SYMBOL(fscrypt_encode_nokey_name);

fscrypt_encode_nokey_name() still isn't actually used in this patchset.  Also,
the commit message doesn't mention it; it only mentions the base64 encoding and
decoding.

> +
>  /**
>   * fscrypt_fname_disk_to_usr() - convert an encrypted filename to
>   *				 user-presentable form
> @@ -351,7 +390,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
>  		     offsetof(struct fscrypt_nokey_name, bytes));
>  	BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) !=
>  		     offsetof(struct fscrypt_nokey_name, sha256));
> -	BUILD_BUG_ON(BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX);
> +	BUILD_BUG_ON(FSCRYPT_BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX);
>  
>  	if (hash) {
>  		nokey_name.dirhash[0] = hash;
> @@ -371,7 +410,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
>  				  nokey_name.sha256);
>  		size = FSCRYPT_NOKEY_NAME_MAX;
>  	}
> -	oname->len = base64_encode((const u8 *)&nokey_name, size, oname->name);
> +	oname->len = fscrypt_base64_encode((const u8 *)&nokey_name, size, oname->name);

Personally I still prefer keeping keeping lines at or below the traditional 80
column limit, at least in existing files, and not introducing random longer
lines...  I.e. pass --max-line-length=80 to checkpatch.

> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index b3b0c5675c6b..95dddba3ed00 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -182,6 +182,10 @@ void fscrypt_free_inode(struct inode *inode);
>  int fscrypt_drop_inode(struct inode *inode);
>  
>  /* fname.c */
> +#define FSCRYPT_BASE64_CHARS(nbytes)	DIV_ROUND_UP((nbytes) * 4, 3)
> +
> +int fscrypt_base64_encode(const u8 *src, int len, char *dst);
> +int fscrypt_base64_decode(const char *src, int len, u8 *dst);
>  int fscrypt_setup_filename(struct inode *inode, const struct qstr *iname,
>  			   int lookup, struct fscrypt_name *fname);

Aren't stubs for !CONFIG_FS_ENCRYPTION needed?

- Eric

Re: [RFC PATCH v3 03/16] fscrypt: export fscrypt_d_revalidate

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:54PM -0400, Jeff Layton wrote:
> 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.

IMO, a slightly clearer explanation would be:

	Since ceph already uses its own dentry_operations, it can't use
	fscrypt_d_ops.  Instead, export fscrypt_d_revalidate() so that
	ceph_d_revalidate() can call it.

Also, it turns out that ext4 and f2fs will need this too.  You could add
to the commit message:

	This change is also needed by ext4 and f2fs to add support for
	directories that are both encrypted and casefolded, since similarly the
	current "fscrypt_d_ops" approach is too inflexible for that.  See
	https://lore.kernel.org/r/20200307023611.204708-6-drosen@google.com and
	https://lore.kernel.org/r/20200307023611.204708-8-drosen@google.com.

FYI, I might take this patch for 5.10 to get it out of the way, since
now two patchsets are depending on it.

- Eric

Re: [RFC PATCH v3 04/16] fscrypt: add fscrypt_context_for_new_inode

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:55PM -0400, Jeff Layton wrote:
>  	/*
>  	 * This may be the first time the inode number is available, so do any
> @@ -689,7 +711,6 @@ int fscrypt_set_context(struct inode *inode, void *fs_data)
>  
>  		fscrypt_hash_inode_number(ci, mk);
>  	}
> -
>  	return inode->i_sb->s_cop->set_context(inode, &ctx, ctxsize, fs_data);

Unnecessary whitespace change.

> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index b547e1aabb00..a57d2a9869eb 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -148,6 +148,7 @@ 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_set_context(struct inode *inode, void *fs_data);
> +int fscrypt_context_for_new_inode(void *ctx, struct inode *inode);

Please keep declarations in the same order as the definitions.
So, fscrypt_context_for_new_inode() before fscrypt_set_context().

- Eric

Re: [RFC PATCH v3 05/16] fscrypt: make fscrypt_fname_disk_to_usr return whether result is nokey name

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:56PM -0400, Jeff Layton wrote:
> Ceph will sometimes need to know whether the resulting name from this
> function is a nokey name, in order to set the dentry flags without
> racy checks on the parent inode.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/crypto/fname.c       | 5 ++++-
>  fs/crypto/hooks.c       | 4 ++--
>  fs/ext4/dir.c           | 3 ++-
>  fs/ext4/namei.c         | 6 ++++--
>  fs/f2fs/dir.c           | 3 ++-
>  fs/ubifs/dir.c          | 4 +++-
>  include/linux/fscrypt.h | 4 ++--
>  7 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 0d41eb4a5493..b97a81ccd838 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -353,6 +353,7 @@ EXPORT_SYMBOL(fscrypt_encode_nokey_name);
>   * @oname: output buffer for the user-presentable filename.  The caller must
>   *	   have allocated enough space for this, e.g. using
>   *	   fscrypt_fname_alloc_buffer().
> + * @is_nokey: set to true if oname is a no-key name
>   *
>   * If the key is available, we'll decrypt the disk name.  Otherwise, we'll
>   * encode it for presentation in fscrypt_nokey_name format.
> @@ -363,7 +364,8 @@ EXPORT_SYMBOL(fscrypt_encode_nokey_name);
>  int fscrypt_fname_disk_to_usr(const struct inode *inode,
>  			      u32 hash, u32 minor_hash,
>  			      const struct fscrypt_str *iname,
> -			      struct fscrypt_str *oname)
> +			      struct fscrypt_str *oname,
> +			      bool *is_nokey)
>  {
>  	const struct qstr qname = FSTR_TO_QSTR(iname);
>  	struct fscrypt_nokey_name nokey_name;
> @@ -411,6 +413,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
>  		size = FSCRYPT_NOKEY_NAME_MAX;
>  	}
>  	oname->len = fscrypt_base64_encode((const u8 *)&nokey_name, size, oname->name);
> +	*is_nokey = true;
>  	return 0;
>  }

Can you do:

	if (is_nokey)
		*is_nokey = true;

... so that callers who don't care can just pass NULL?

Also, to make the kerneldoc clearer:

 * @is_nokey: (output) if non-NULL, set to true if the key wasn't available
 *
 * If the key is available, we'll decrypt the disk name.  Otherwise, we'll
 * encode it for presentation in fscrypt_nokey_name format and set
 * *is_nokey=true.  See struct fscrypt_nokey_name for details.

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

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:57PM -0400, Jeff Layton wrote:
> Boilerplate ioctls for controlling encryption.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/ioctl.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> index 6e061bf62ad4..381e44b2d60a 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,30 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  
>  	case CEPH_IOC_SYNCIO:
>  		return ceph_ioctl_syncio(file);
> +
> +	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);

Will you be implementing an encryption feature flag for ceph, similar to what
ext4 and f2fs have?  E.g., ext4 doesn't allow these ioctls unless the filesystem
was formatted with '-O encrypt' (or 'tune2fs -O encrypt' was run later).  There
would be various problems if we didn't do that; for example, old versions of
e2fsck would consider encrypted directories to be corrupted.

- Eric

Re: [RFC PATCH v3 07/16] ceph: crypto context handling for ceph

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:58PM -0400, Jeff Layton wrote:
> +static const union fscrypt_context *
> +ceph_get_dummy_context(struct super_block *sb)
> +{
> +	return ceph_sb_to_client(sb)->dummy_enc_ctx.ctx;
> +}

This hunk needs to go in the patch that adds test_dummy_encryption support.

> diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> new file mode 100644
> index 000000000000..b5f38ee80553
> --- /dev/null
> +++ b/fs/ceph/crypto.h
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0

checkpatch wants a /* comment */ here, not a // comment.

Can you run checkpatch on the whole patchset and fix the warnings?

> +/*
> + * Ceph fscrypt functionality
> + */
> +
> +#ifndef _CEPH_CRYPTO_H
> +#define _CEPH_CRYPTO_H
> +
> +#ifdef CONFIG_FS_ENCRYPTION
> +
> +#define	CEPH_XATTR_NAME_ENCRYPTION_CONTEXT	"encryption.ctx"
> +
> +void ceph_fscrypt_set_ops(struct super_block *sb);
> +
> +#else /* CONFIG_FS_ENCRYPTION */
> +
> +static inline int ceph_fscrypt_set_ops(struct super_block *sb)
> +{
> +	return 0;
> +}

The !CONFIG_FS_ENCRYPTION version of ceph_fscrypt_set_ops() needs to return
void.

> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 526faf4778ce..daae18267fd8 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -549,6 +549,7 @@ void ceph_evict_inode(struct inode *inode)
>  
>  	percpu_counter_dec(&mdsc->metric.total_inodes);
>  
> +	fscrypt_put_encryption_info(inode);
>  	truncate_inode_pages_final(&inode->i_data);
>  	clear_inode(inode);

Is it correct for fscrypt_put_encryption_info() to go before
truncate_inode_pages_final()?  The other filesystems call
fscrypt_put_encryption_info() later.  Note that all I/O needs to be done before
calling fscrypt_put_encryption_info().

> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index b3fc9bb61afc..055180218224 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>
> @@ -984,6 +985,10 @@ 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;
> +

This part doesn't compile when CONFIG_FS_ENCRYPTION=y.  It got fixed in a later
patch, but it should be fixed here.

> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index 483a52d281cd..cc39cc36de77 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -985,6 +985,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)

Use 'const char *' instead of 'char *'?


- Eric

Re: [RFC PATCH v3 08/16] ceph: implement -o test_dummy_encryption mount option

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:59PM -0400, Jeff Layton wrote:
> +	case Opt_test_dummy_encryption:
> +		kfree(fsopt->test_dummy_encryption);
> +#ifdef CONFIG_FS_ENCRYPTION
> +		fsopt->test_dummy_encryption = param->string;
> +		param->string = NULL;
> +		fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
> +#else
> +		warnfc(fc, "FS encryption not supported: test_dummy_encryption mount option ignored");
> +#endif

Seems that the kfree() should go in the CONFIG_FS_ENCRYPTION=y block.

Also, is there much reason to have the CEPH_MOUNT_OPT_TEST_DUMMY_ENC flag
instead of just checking fsopt->test_dummy_encryption != NULL?

> +#ifdef CONFIG_FS_ENCRYPTION
> +static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
> +						struct ceph_mount_options *fsopt)
> +{
> +	struct ceph_fs_client *fsc = sb->s_fs_info;
> +
> +	if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
> +		substring_t arg = { };
> +
> +		/*
> +		 * No changing encryption context on remount. Note that
> +		 * fscrypt_set_test_dummy_encryption will validate the version
> +		 * string passed in (if any).
> +		 */
> +		if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy)
> +			return -EEXIST;

Maybe show an error message here, with errorfc()?
See the message that ext4_set_test_dummy_encryption() shows.

> +
> +		/* Ewwwwwwww */
> +		if (fsc->mount_options->test_dummy_encryption) {
> +			arg.from = fsc->mount_options->test_dummy_encryption;
> +			arg.to = arg.from + strlen(arg.from) - 1;
> +		}

We should probably make fscrypt_set_test_dummy_encryption() take a
'const char *' to avoid having to create a substring_t here.

> +		return fscrypt_set_test_dummy_encryption(sb, &arg, &fsc->dummy_enc_policy);

Likewise, maybe show an error message if this fails.

> +	} else {
> +		if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && fsc->dummy_enc_policy.policy)
> +			return -EEXIST;

If remount on ceph behaves as "don't change options that aren't specified",
similar to ext4, then there's no need for this hunk here.

>  static int ceph_reconfigure_fc(struct fs_context *fc)
>  {
> +	int err;
>  	struct ceph_parse_opts_ctx *pctx = fc->fs_private;
>  	struct ceph_mount_options *fsopt = pctx->opts;
> -	struct ceph_fs_client *fsc = ceph_sb_to_client(fc->root->d_sb);
> +	struct super_block *sb = fc->root->d_sb;
> +	struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
>  
>  	if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)
>  		ceph_set_mount_opt(fsc, ASYNC_DIROPS);
>  	else
>  		ceph_clear_mount_opt(fsc, ASYNC_DIROPS);
>  
> -	sync_filesystem(fc->root->d_sb);
> +	err = ceph_set_test_dummy_encryption(sb, fc, fsopt);
> +	if (err)
> +		return err;
> +
> +	sync_filesystem(sb);
>  	return 0;
>  }

Seems that ceph_set_test_dummy_encryption() should go at the beginning, since
otherwise it can fail after something was already changed.

- Eric

Re: [RFC PATCH v3 09/16] ceph: preallocate inode for ops that may create one

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:17:00PM -0400, Jeff Layton wrote:
> @@ -663,6 +658,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
>  	struct ceph_mds_client *mdsc = fsc->mdsc;
>  	struct ceph_mds_request *req;
> +	struct inode *new_inode = NULL;
>  	struct dentry *dn;
>  	struct ceph_acl_sec_ctx as_ctx = {};
>  	bool try_async = ceph_test_mount_opt(fsc, ASYNC_DIROPS);
> @@ -675,21 +671,21 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  
>  	if (dentry->d_name.len > NAME_MAX)
>  		return -ENAMETOOLONG;
> -
> +retry:
>  	if (flags & O_CREAT) {
>  		if (ceph_quota_is_max_files_exceeded(dir))
>  			return -EDQUOT;
> -		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
> -		if (err < 0)
> -			return err;
> -		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> -		if (err < 0)
> +
> +		new_inode = ceph_new_inode(dir, dentry, &mode, &as_ctx);
> +		if (IS_ERR(new_inode)) {
> +			err = PTR_ERR(new_inode);
>  			goto out_ctx;
> +		}

Is the 'goto out_ctx;' correct here?  It looks like it should be
'return PTR_ERR(new_inode)'

> +/**
> + * ceph_new_inode - allocate a new inode in advance of an expected create
> + * @dir: parent directory for new inode
> + * @mode: mode of new inode
> + */
> +struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
> +			     umode_t *mode, struct ceph_acl_sec_ctx *as_ctx)

Some parameters aren't documented.

> +	int err;
>  	struct inode *inode;
>  
> -	inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare,
> -			     ceph_set_ino_cb, &vino);
> +	inode = new_inode_pseudo(dir->i_sb);
>  	if (!inode)
>  		return ERR_PTR(-ENOMEM);
>  
> +	if (!S_ISLNK(*mode)) {
> +		err = ceph_pre_init_acls(dir, mode, as_ctx);
> +		if (err < 0)
> +			goto out_err;
> +	}
> +
> +	err = ceph_security_init_secctx(dentry, *mode, as_ctx);
> +	if (err < 0)
> +		goto out_err;
> +
> +	inode->i_state = 0;
> +	inode->i_mode = *mode;
> +	return inode;
> +out_err:
> +	iput(inode);
> +	return ERR_PTR(err);
> +}

Should this be freeing anything from the ceph_acl_sec_ctx on error?

- Eric

Re: [RFC PATCH v3 10/16] ceph: add routine to create context prior to RPC

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:17:01PM -0400, Jeff Layton wrote:
> diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> index b5f38ee80553..c1b6ec4b2961 100644
> --- a/fs/ceph/crypto.h
> +++ b/fs/ceph/crypto.h
> @@ -11,6 +11,8 @@
>  #define	CEPH_XATTR_NAME_ENCRYPTION_CONTEXT	"encryption.ctx"
>  
>  void ceph_fscrypt_set_ops(struct super_block *sb);
> +int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
> +				 struct ceph_acl_sec_ctx *as);
>  
>  #else /* CONFIG_FS_ENCRYPTION */
>  
> @@ -19,6 +21,12 @@ static inline int ceph_fscrypt_set_ops(struct super_block *sb)
>  	return 0;
>  }
>  
> +static inline int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
> +						struct ceph_acl_sec_ctx *as)
> +{
> +	return 0;
> +}
> +
>  #endif /* CONFIG_FS_ENCRYPTION */

Seems there should at least be something that prevents you from creating a file
in an encrypted directory when !CONFIG_FS_ENCRYPTION.

The other filesystems use fscrypt_prepare_new_inode() for this; it returns
EOPNOTSUPP when IS_ENCRYPTED(dir).

- Eric

Re: [RFC PATCH v3 12/16] ceph: add encrypted fname handling to ceph_mdsc_build_path

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:17:03PM -0400, Jeff Layton wrote:
> +		} else {
> +			int err;
> +			struct fscrypt_name fname = { };
> +			int len;
> +			char buf[FSCRYPT_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);
> +			}

It's still not clear how no-key names are handled here (or if they are even
possible here).

> +
> +			/* base64 encode the encrypted name */
> +			len = fscrypt_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);

This says "non-ciphertext name", which suggest that it's a plaintext name.  But
actually it's a base64-encoded ciphertext name.

- Eric

Re: [RFC PATCH v3 14/16] ceph: add support to readdir for encrypted filenames

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:17:05PM -0400, Jeff Layton wrote:
> 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 | 35 +++++++++++++++++++++++++++++
>  fs/ceph/dir.c    | 58 +++++++++++++++++++++++++++++++++++++++---------
>  fs/ceph/inode.c  | 31 +++++++++++++++++++++++---
>  4 files changed, 157 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index f037a4939026..e3038c88c7a0 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -107,3 +107,50 @@ int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
>  		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,
> +			bool *is_nokey)
> +{
> +	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;

The 'err' variable isn't needed, since 'ret' can be used instead.

> +	}
> +
> +	declen = fscrypt_base64_decode(name, len, tname->name);
> +	if (declen < 0 || declen > NAME_MAX) {
> +		ret = -EIO;
> +		goto out;
> +	}

declen <= 0, to cover the empty name case.

Also, is there a point in checking for > NAME_MAX?

> +
> +	tname->len = declen;
> +
> +	ret = fscrypt_fname_disk_to_usr(parent, 0, 0, tname, oname, is_nokey);
> +
> +	if (save_len)
> +		tname->len = save_len;

This logic for temporarily overwriting the length is weird.
How about something like the following instead:

int ceph_fname_to_usr(struct inode *parent, char *name, u32 len,
		      struct fscrypt_str *tname, struct fscrypt_str *oname,
		      bool *is_nokey)
{
	int err, declen;
	struct fscrypt_str _tname = FSTR_INIT(NULL, 0);
	struct fscrypt_str iname;

	if (!IS_ENCRYPTED(parent)) {
		oname->name = name;
		oname->len = len;
		return 0;
	}

	err = fscrypt_get_encryption_info(parent);
	if (err)
		return err;

	if (!tname) {
		err = fscrypt_fname_alloc_buffer(NAME_MAX, &_tname);
		if (err)
			return err;
		tname = &_tname;
	}

	declen = fscrypt_base64_decode(name, len, tname->name);
	if (declen <= 0) {
		err = -EIO;
		goto out;
	}

	iname.name = tname->name;
	iname.len = declen;
	err = fscrypt_fname_disk_to_usr(parent, 0, 0, &iname, oname, is_nokey);
out:
	fscrypt_fname_free_buffer(&_tname);
	return err;
}

Re: [RFC PATCH v3 16/16] ceph: create symlinks with encrypted and base64-encoded targets

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:17:07PM -0400, Jeff Layton wrote:
> +	if (IS_ENCRYPTED(req->r_new_inode)) {
> +		int len = strlen(dest);
> +
> +		err = fscrypt_prepare_symlink(dir, dest, len, PATH_MAX, &osd_link);
> +		if (err)
> +			goto out_req;
> +
> +		err = fscrypt_encrypt_symlink(req->r_new_inode, dest, len, &osd_link);
> +		if (err)
> +			goto out_req;
> +
> +		req->r_path2 = kmalloc(FSCRYPT_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);

osd_link.len includes a null terminator.  It seems that's not what's wanted
here, and you should be subtracting 1 here.

(fscrypt_prepare_symlink() maybe should exclude the null terminator from the
length instead.  But for the other filesystems it was easier to include it...)

> @@ -996,26 +995,39 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
>  		inode->i_fop = &ceph_file_fops;
>  		break;
>  	case S_IFLNK:
> -		inode->i_op = &ceph_symlink_iops;
>  		if (!ci->i_symlink) {
>  			u32 symlen = iinfo->symlink_len;
>  			char *sym;
>  
>  			spin_unlock(&ci->i_ceph_lock);
>  
> -			if (symlen != i_size_read(inode)) {
> -				pr_err("%s %llx.%llx BAD symlink "
> -					"size %lld\n", __func__,
> -					ceph_vinop(inode),
> -					i_size_read(inode));
> +			if (IS_ENCRYPTED(inode)) {
> +				/* Do base64 decode so that we get the right size (maybe?) */
> +				err = -ENOMEM;
> +				sym = kmalloc(symlen + 1, GFP_NOFS);
> +				if (!sym)
> +					goto out;
> +
> +				symlen = fscrypt_base64_decode(iinfo->symlink, symlen, sym);
> +				/*
> +				 * i_size as reported by the MDS may be wrong, due to base64
> +				 * inflation and padding. Fix it up here.
> +				 */
>  				i_size_write(inode, symlen);

Note that fscrypt_base64_decode() can fail (return -1) if the input is not valid
base64.  That isn't being handled here.

> +static const char *ceph_encrypted_get_link(struct dentry *dentry, struct inode *inode,
> +					   struct delayed_call *done)
> +{
> +	struct ceph_inode_info *ci = ceph_inode(inode);
> +
> +	if (!dentry)
> +		return ERR_PTR(-ECHILD);
> +
> +	return fscrypt_get_symlink(inode, ci->i_symlink, ksize(ci->i_symlink), done);

Using ksize() seems wrong here, since that would allow fscrypt_get_symlink() to
read beyond the part of the buffer that is actually initialized.

> -static const struct inode_operations ceph_symlink_iops = {
> +const struct inode_operations ceph_symlink_iops = {
>  	.get_link = simple_get_link,
>  	.setattr = ceph_setattr,
>  	.getattr = ceph_getattr,
>  	.listxattr = ceph_listxattr,
>  };
>  
> +const struct inode_operations ceph_encrypted_symlink_iops = {
> +	.get_link = ceph_encrypted_get_link,
> +	.setattr = ceph_setattr,
> +	.getattr = ceph_getattr,
> +	.listxattr = ceph_listxattr,
> +};

These don't need to be made global, as they're only used in fs/ceph/inode.c.

- Eric

Re: [RFC PATCH v3 00/16] ceph+fscrypt: context, filename and symlink support

[Eric Biggers]

On Mon, Sep 14, 2020 at 03:16:51PM -0400, Jeff Layton wrote:
> This is the third posting of the ceph+fscrypt integration work. This
> just covers context handling, filename and symlink support.
> 
> The main changes since the last set are mainly to address Eric's review
> comments. Hopefully this will be much closer to mergeable. Some highlights:
> 
> 1/ rebase onto Eric's fscrypt-file-creation-v2 tag
> 
> 2/ fscrypt_context_for_new_inode now takes a void * to hold the context
> 
> 3/ make fscrypt_fname_disk_to_usr designate whether the returned name
>    is a nokey name. This is necessary to close a potential race in
>    readdir support
> 
> 4/ fscrypt_base64_encode/decode remain in fs/crypto (not moved into lib/)
> 
> 5/ test_dummy_encryption handling is moved into a separate patch, and
>    several bugs fixed that resulted in context not being set up
>    properly.
> 
> 6/ symlink handling now works
> 
> Content encryption is the next step, but I want to get the fscache
> rework done first. It would be nice if we were able to store encrypted
> files in the cache, for instance.
> 
> This set has been tagged as "ceph-fscrypt-rfc.3" in my tree here:
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git
> 
> Note that this is still quite preliminary, but my goal is to get a set
> merged for v5.11.

A few comments that didn't fit anywhere else:

I'm looking forward to contents encryption, as that's the most important part.

Is there any possibility that the fscrypt xfstests can be run on ceph?
See: https://www.kernel.org/doc/html/latest/filesystems/fscrypt.html#tests

In fs/ceph/Kconfig, CEPH_FS needs:

	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION

There are compile errors when !CONFIG_FS_ENCRYPTION.

- Eric

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

[Jeff Layton]

On Mon, 2020-09-14 at 17:45 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:16:57PM -0400, Jeff Layton wrote:
> > Boilerplate ioctls for controlling encryption.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/ioctl.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> > index 6e061bf62ad4..381e44b2d60a 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,30 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >  
> >  	case CEPH_IOC_SYNCIO:
> >  		return ceph_ioctl_syncio(file);
> > +
> > +	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);
> 
> Will you be implementing an encryption feature flag for ceph, similar to what
> ext4 and f2fs have?  E.g., ext4 doesn't allow these ioctls unless the filesystem
> was formatted with '-O encrypt' (or 'tune2fs -O encrypt' was run later).  There
> would be various problems if we didn't do that; for example, old versions of
> e2fsck would consider encrypted directories to be corrupted.
> 

Yes, we'll probably have something like that once the MDS support has
settled. We'll want to disallow encryption when dealing with MDS's that
don't support it, so I suspect we'll need to add a check for that in
these ioctl calls.

That feature bit hasn't been declared yet though, and this patchset is
still _really_ rough. I'll add a comment to that effect for now though.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 14/16] ceph: add support to readdir for encrypted filenames

[Jeff Layton]

On Mon, 2020-09-14 at 18:57 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:17:05PM -0400, Jeff Layton wrote:
> > 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 | 35 +++++++++++++++++++++++++++++
> >  fs/ceph/dir.c    | 58 +++++++++++++++++++++++++++++++++++++++---------
> >  fs/ceph/inode.c  | 31 +++++++++++++++++++++++---
> >  4 files changed, 157 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> > index f037a4939026..e3038c88c7a0 100644
> > --- a/fs/ceph/crypto.c
> > +++ b/fs/ceph/crypto.c
> > @@ -107,3 +107,50 @@ int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
> >  		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,
> > +			bool *is_nokey)
> > +{
> > +	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;
> 
> The 'err' variable isn't needed, since 'ret' can be used instead.
> 
> > +	}
> > +
> > +	declen = fscrypt_base64_decode(name, len, tname->name);
> > +	if (declen < 0 || declen > NAME_MAX) {
> > +		ret = -EIO;
> > +		goto out;
> > +	}
> 
> declen <= 0, to cover the empty name case.
> 
> Also, is there a point in checking for > NAME_MAX?
> 

IDK. We're getting these strings from the MDS and they could end up
being corrupt if there are bugs there (or if the MDS is compromised). 
Of course, if we get a name longer than NAME_MAX then we've overrun the
buffer.

Maybe we should add a maxlen parameter to fscrypt_base64_encode/decode ?
Or maybe I should just have fscrypt_fname_alloc_buffer allocate a buffer
the same size as "len"? It might be a little larger than necessary, but
that would be safer.

> > +
> > +	tname->len = declen;
> > +
> > +	ret = fscrypt_fname_disk_to_usr(parent, 0, 0, tname, oname, is_nokey);
> > +
> > +	if (save_len)
> > +		tname->len = save_len;
> 
> This logic for temporarily overwriting the length is weird.
> How about something like the following instead:
> 

Yeah, it is odd. I think I got spooked by the way that length in struct
fscrypt_str is handled.

Some functions treat it as representing the length of the allocated
buffer (e.g. fscrypt_fname_alloc_buffer), but others treat it as
representing the length of the string in ->name (e.g.
fscrypt_encode_nokey_name).

Your suggestion works around that though, so I'll probably adopt
something like it. Thanks!

> int ceph_fname_to_usr(struct inode *parent, char *name, u32 len,
> 		      struct fscrypt_str *tname, struct fscrypt_str *oname,
> 		      bool *is_nokey)
> {
> 	int err, declen;
> 	struct fscrypt_str _tname = FSTR_INIT(NULL, 0);
> 	struct fscrypt_str iname;
> 
> 	if (!IS_ENCRYPTED(parent)) {
> 		oname->name = name;
> 		oname->len = len;
> 		return 0;
> 	}
> 
> 	err = fscrypt_get_encryption_info(parent);
> 	if (err)
> 		return err;
> 
> 	if (!tname) {
> 		err = fscrypt_fname_alloc_buffer(NAME_MAX, &_tname);
> 		if (err)
> 			return err;
> 		tname = &_tname;
> 	}
> 
> 	declen = fscrypt_base64_decode(name, len, tname->name);
> 	if (declen <= 0) {
> 		err = -EIO;
> 		goto out;
> 	}
> 
> 	iname.name = tname->name;
> 	iname.len = declen;
> 	err = fscrypt_fname_disk_to_usr(parent, 0, 0, &iname, oname, is_nokey);
> out:
> 	fscrypt_fname_free_buffer(&_tname);
> 	return err;
> }

-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 00/16] ceph+fscrypt: context, filename and symlink support

[Jeff Layton]

On Mon, 2020-09-14 at 19:13 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:16:51PM -0400, Jeff Layton wrote:
> > This is the third posting of the ceph+fscrypt integration work. This
> > just covers context handling, filename and symlink support.
> > 
> > The main changes since the last set are mainly to address Eric's review
> > comments. Hopefully this will be much closer to mergeable. Some highlights:
> > 
> > 1/ rebase onto Eric's fscrypt-file-creation-v2 tag
> > 
> > 2/ fscrypt_context_for_new_inode now takes a void * to hold the context
> > 
> > 3/ make fscrypt_fname_disk_to_usr designate whether the returned name
> >    is a nokey name. This is necessary to close a potential race in
> >    readdir support
> > 
> > 4/ fscrypt_base64_encode/decode remain in fs/crypto (not moved into lib/)
> > 
> > 5/ test_dummy_encryption handling is moved into a separate patch, and
> >    several bugs fixed that resulted in context not being set up
> >    properly.
> > 
> > 6/ symlink handling now works
> > 
> > Content encryption is the next step, but I want to get the fscache
> > rework done first. It would be nice if we were able to store encrypted
> > files in the cache, for instance.
> > 
> > This set has been tagged as "ceph-fscrypt-rfc.3" in my tree here:
> > 
> >     https://git.kernel.org/pub/scm/linux/kernel/git/jlayton/linux.git
> > 
> > Note that this is still quite preliminary, but my goal is to get a set
> > merged for v5.11.
> 
> A few comments that didn't fit anywhere else:
> 
> I'm looking forward to contents encryption, as that's the most important part.
> 

Me too, but I've got a fairly substantial rework of the buffered
writeback code queued up to handle some fscache changes. We'll probably
need to teach fscache how to deal with encrypted data, so I haven't
really started on that part yet.

> Is there any possibility that the fscrypt xfstests can be run on ceph?
> See: https://www.kernel.org/doc/html/latest/filesystems/fscrypt.html#tests
> 

I've been testing with the xfstests "quick" group as a sanity test, but
it doesn't have the fscrypt tests. I'll try them out soon.

> In fs/ceph/Kconfig, CEPH_FS needs:
> 
> 	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
> 
> There are compile errors when !CONFIG_FS_ENCRYPTION.
> 

Thanks. I should have added the caveat that this is still _very_ rough
and not at all ready for merge. I'll definitely fix up
the !CONFIG_FS_ENCRYPTION case before I send the next set.

Thanks for the detailed review so far. I'm working through your comments
now and should address most of them in the next set.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 16/16] ceph: create symlinks with encrypted and base64-encoded targets

[Jeff Layton]

On Mon, 2020-09-14 at 19:07 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:17:07PM -0400, Jeff Layton wrote:
> > +	if (IS_ENCRYPTED(req->r_new_inode)) {
> > +		int len = strlen(dest);
> > +
> > +		err = fscrypt_prepare_symlink(dir, dest, len, PATH_MAX, &osd_link);
> > +		if (err)
> > +			goto out_req;
> > +
> > +		err = fscrypt_encrypt_symlink(req->r_new_inode, dest, len, &osd_link);
> > +		if (err)
> > +			goto out_req;
> > +
> > +		req->r_path2 = kmalloc(FSCRYPT_BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> 
> osd_link.len includes a null terminator.  It seems that's not what's wanted
> here, and you should be subtracting 1 here.
> 
> (fscrypt_prepare_symlink() maybe should exclude the null terminator from the
> length instead.  But for the other filesystems it was easier to include it...)
> 

Got it. Fixed.

> > @@ -996,26 +995,39 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
> >  		inode->i_fop = &ceph_file_fops;
> >  		break;
> >  	case S_IFLNK:
> > -		inode->i_op = &ceph_symlink_iops;
> >  		if (!ci->i_symlink) {
> >  			u32 symlen = iinfo->symlink_len;
> >  			char *sym;
> >  
> >  			spin_unlock(&ci->i_ceph_lock);
> >  
> > -			if (symlen != i_size_read(inode)) {
> > -				pr_err("%s %llx.%llx BAD symlink "
> > -					"size %lld\n", __func__,
> > -					ceph_vinop(inode),
> > -					i_size_read(inode));
> > +			if (IS_ENCRYPTED(inode)) {
> > +				/* Do base64 decode so that we get the right size (maybe?) */
> > +				err = -ENOMEM;
> > +				sym = kmalloc(symlen + 1, GFP_NOFS);
> > +				if (!sym)
> > +					goto out;
> > +
> > +				symlen = fscrypt_base64_decode(iinfo->symlink, symlen, sym);
> > +				/*
> > +				 * i_size as reported by the MDS may be wrong, due to base64
> > +				 * inflation and padding. Fix it up here.
> > +				 */
> >  				i_size_write(inode, symlen);
> 
> Note that fscrypt_base64_decode() can fail (return -1) if the input is not valid
> base64.  That isn't being handled here.
> 

Thanks, fixed. It'll return -EIO in that case now.

> > +static const char *ceph_encrypted_get_link(struct dentry *dentry, struct inode *inode,
> > +					   struct delayed_call *done)
> > +{
> > +	struct ceph_inode_info *ci = ceph_inode(inode);
> > +
> > +	if (!dentry)
> > +		return ERR_PTR(-ECHILD);
> > +
> > +	return fscrypt_get_symlink(inode, ci->i_symlink, ksize(ci->i_symlink), done);
> 
> Using ksize() seems wrong here, since that would allow fscrypt_get_symlink() to
> read beyond the part of the buffer that is actually initialized.
> 

Is that actually a problem? I did have an earlier patch that carried
around the length, but it didn't seem to be necessary.

ISTM that that might end up decrypting more data than is actually
needed, but eventually there will be a NULL terminator in the data and
the rest would be ignored.

If it is a problem, then we should probably change the comment header
over fscrypt_get_symlink. It currently says:

   * @max_size: size of @caddr buffer

...which is another reason why I figured using ksize there was OK.

> > -static const struct inode_operations ceph_symlink_iops = {
> > +const struct inode_operations ceph_symlink_iops = {
> >  	.get_link = simple_get_link,
> >  	.setattr = ceph_setattr,
> >  	.getattr = ceph_getattr,
> >  	.listxattr = ceph_listxattr,
> >  };
> >  
> > +const struct inode_operations ceph_encrypted_symlink_iops = {
> > +	.get_link = ceph_encrypted_get_link,
> > +	.setattr = ceph_setattr,
> > +	.getattr = ceph_getattr,
> > +	.listxattr = ceph_listxattr,
> > +};
> 
> These don't need to be made global, as they're only used in fs/ceph/inode.c.
> 

Thanks, fixed.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 14/16] ceph: add support to readdir for encrypted filenames

[Eric Biggers]

On Tue, Sep 15, 2020 at 09:27:49AM -0400, Jeff Layton wrote:
> > > +	}
> > > +
> > > +	declen = fscrypt_base64_decode(name, len, tname->name);
> > > +	if (declen < 0 || declen > NAME_MAX) {
> > > +		ret = -EIO;
> > > +		goto out;
> > > +	}
> > 
> > declen <= 0, to cover the empty name case.
> > 
> > Also, is there a point in checking for > NAME_MAX?
> > 
> 
> IDK. We're getting these strings from the MDS and they could end up
> being corrupt if there are bugs there (or if the MDS is compromised). 
> Of course, if we get a name longer than NAME_MAX then we've overrun the
> buffer.
> 
> Maybe we should add a maxlen parameter to fscrypt_base64_encode/decode ?
> Or maybe I should just have fscrypt_fname_alloc_buffer allocate a buffer
> the same size as "len"? It might be a little larger than necessary, but
> that would be safer.

How about checking that the base64-encoded filename is <= BASE64_CHARS(NAME_MAX)
in length?  Then decoding it can't give more than NAME_MAX bytes.

fscrypt_setup_filename() does a similar check when decoding a no-key name.

- Eric

Re: [RFC PATCH v3 16/16] ceph: create symlinks with encrypted and base64-encoded targets

[Eric Biggers]

On Tue, Sep 15, 2020 at 10:05:53AM -0400, Jeff Layton wrote:
> > > +static const char *ceph_encrypted_get_link(struct dentry *dentry, struct inode *inode,
> > > +					   struct delayed_call *done)
> > > +{
> > > +	struct ceph_inode_info *ci = ceph_inode(inode);
> > > +
> > > +	if (!dentry)
> > > +		return ERR_PTR(-ECHILD);
> > > +
> > > +	return fscrypt_get_symlink(inode, ci->i_symlink, ksize(ci->i_symlink), done);
> > 
> > Using ksize() seems wrong here, since that would allow fscrypt_get_symlink() to
> > read beyond the part of the buffer that is actually initialized.
> > 
> 
> Is that actually a problem? I did have an earlier patch that carried
> around the length, but it didn't seem to be necessary.
> 
> ISTM that that might end up decrypting more data than is actually
> needed, but eventually there will be a NULL terminator in the data and
> the rest would be ignored.
> 

Yes it's a problem.  The code that decrypts the symlink adds the null terminator
at the end.  So if the stated buffer size is wrong, then decrypted uninitialized
memory can be included into the symlink target that userspace then sees.

> If it is a problem, then we should probably change the comment header
> over fscrypt_get_symlink. It currently says:
> 
>    * @max_size: size of @caddr buffer
> 
> ...which is another reason why I figured using ksize there was OK.

ksize() is rarely used, as it should be.  (For one, it disables KASAN on the
buffer...)  I think that when people see "buffer size" they almost always think
the actual allocated size of the buffer, not ksize().  But we could change it to
say "allocated size" if that would make it clearer...

- Eric

Re: [RFC PATCH v3 16/16] ceph: create symlinks with encrypted and base64-encoded targets

[Jeff Layton]

On Tue, 2020-09-15 at 13:49 -0700, Eric Biggers wrote:
> On Tue, Sep 15, 2020 at 10:05:53AM -0400, Jeff Layton wrote:
> > > > +static const char *ceph_encrypted_get_link(struct dentry *dentry, struct inode *inode,
> > > > +					   struct delayed_call *done)
> > > > +{
> > > > +	struct ceph_inode_info *ci = ceph_inode(inode);
> > > > +
> > > > +	if (!dentry)
> > > > +		return ERR_PTR(-ECHILD);
> > > > +
> > > > +	return fscrypt_get_symlink(inode, ci->i_symlink, ksize(ci->i_symlink), done);
> > > 
> > > Using ksize() seems wrong here, since that would allow fscrypt_get_symlink() to
> > > read beyond the part of the buffer that is actually initialized.
> > > 
> > 
> > Is that actually a problem? I did have an earlier patch that carried
> > around the length, but it didn't seem to be necessary.
> > 
> > ISTM that that might end up decrypting more data than is actually
> > needed, but eventually there will be a NULL terminator in the data and
> > the rest would be ignored.
> > 
> 
> Yes it's a problem.  The code that decrypts the symlink adds the null terminator
> at the end.  So if the stated buffer size is wrong, then decrypted uninitialized
> memory can be included into the symlink target that userspace then sees.
> 
> > If it is a problem, then we should probably change the comment header
> > over fscrypt_get_symlink. It currently says:
> > 
> >    * @max_size: size of @caddr buffer
> > 
> > ...which is another reason why I figured using ksize there was OK.
> 
> ksize() is rarely used, as it should be.  (For one, it disables KASAN on the
> buffer...)  I think that when people see "buffer size" they almost always think
> the actual allocated size of the buffer, not ksize().  But we could change it to
> say "allocated size" if that would make it clearer...
> 

Ok, I'll rework it to carry around the length too. That should take care of the problem.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 14/16] ceph: add support to readdir for encrypted filenames

[Jeff Layton]

On Tue, 2020-09-15 at 13:40 -0700, Eric Biggers wrote:
> On Tue, Sep 15, 2020 at 09:27:49AM -0400, Jeff Layton wrote:
> > > > +	}
> > > > +
> > > > +	declen = fscrypt_base64_decode(name, len, tname->name);
> > > > +	if (declen < 0 || declen > NAME_MAX) {
> > > > +		ret = -EIO;
> > > > +		goto out;
> > > > +	}
> > > 
> > > declen <= 0, to cover the empty name case.
> > > 
> > > Also, is there a point in checking for > NAME_MAX?
> > > 
> > 
> > IDK. We're getting these strings from the MDS and they could end up
> > being corrupt if there are bugs there (or if the MDS is compromised). 
> > Of course, if we get a name longer than NAME_MAX then we've overrun the
> > buffer.
> > 
> > Maybe we should add a maxlen parameter to fscrypt_base64_encode/decode ?
> > Or maybe I should just have fscrypt_fname_alloc_buffer allocate a buffer
> > the same size as "len"? It might be a little larger than necessary, but
> > that would be safer.
> 
> How about checking that the base64-encoded filename is <= BASE64_CHARS(NAME_MAX)
> in length?  Then decoding it can't give more than NAME_MAX bytes.
> 
> fscrypt_setup_filename() does a similar check when decoding a no-key name.
> 

Good idea. I'll roll that in.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 10/16] ceph: add routine to create context prior to RPC

[Jeff Layton]

On Mon, 2020-09-14 at 18:37 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:17:01PM -0400, Jeff Layton wrote:
> > diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> > index b5f38ee80553..c1b6ec4b2961 100644
> > --- a/fs/ceph/crypto.h
> > +++ b/fs/ceph/crypto.h
> > @@ -11,6 +11,8 @@
> >  #define	CEPH_XATTR_NAME_ENCRYPTION_CONTEXT	"encryption.ctx"
> >  
> >  void ceph_fscrypt_set_ops(struct super_block *sb);
> > +int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
> > +				 struct ceph_acl_sec_ctx *as);
> >  
> >  #else /* CONFIG_FS_ENCRYPTION */
> >  
> > @@ -19,6 +21,12 @@ static inline int ceph_fscrypt_set_ops(struct super_block *sb)
> >  	return 0;
> >  }
> >  
> > +static inline int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
> > +						struct ceph_acl_sec_ctx *as)
> > +{
> > +	return 0;
> > +}
> > +
> >  #endif /* CONFIG_FS_ENCRYPTION */
> 
> Seems there should at least be something that prevents you from creating a file
> in an encrypted directory when !CONFIG_FS_ENCRYPTION.
> 
> The other filesystems use fscrypt_prepare_new_inode() for this; it returns
> EOPNOTSUPP when IS_ENCRYPTED(dir).
> 

Once we have the MDS support done, we should be able to make it reject
create requests from clients that don't support the new feature flag,
but denying it on the client is more efficient. Fixed in my tree.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 12/16] ceph: add encrypted fname handling to ceph_mdsc_build_path

[Jeff Layton]

On Mon, 2020-09-14 at 18:41 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:17:03PM -0400, Jeff Layton wrote:
> > +		} else {
> > +			int err;
> > +			struct fscrypt_name fname = { };
> > +			int len;
> > +			char buf[FSCRYPT_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);
> > +			}
> 
> It's still not clear how no-key names are handled here (or if they are even
> possible here).
> 

They're not really handled yet. We need support in the MDS for it, which
is being worked on by Xiubo (cc'ed):

    https://tracker.ceph.com/issues/47162

For now, working with names > ~149 characters can leave you with bad
dentries that the client may not be able to work with if you don't have
the key.

It sounds like we'll probably need to stabilize some version of the
nokey name so that we can allow the MDS to look them up. Would it be a
problem for us to use the current version of the nokey name format for
this, or would it be better to come up with some other distinct format
for this?

Using the current version of the nokey name is simple as we can just
pass it as-is to the MDS if someone is working in a directory w/o keys.

> > +
> > +			/* base64 encode the encrypted name */
> > +			len = fscrypt_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);
> 
> This says "non-ciphertext name", which suggest that it's a plaintext name.  But
> actually it's a base64-encoded ciphertext name.
> 

Thanks. I fixed the comment.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 09/16] ceph: preallocate inode for ops that may create one

[Jeff Layton]

On Mon, 2020-09-14 at 18:30 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:17:00PM -0400, Jeff Layton wrote:
> > @@ -663,6 +658,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> >  	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
> >  	struct ceph_mds_client *mdsc = fsc->mdsc;
> >  	struct ceph_mds_request *req;
> > +	struct inode *new_inode = NULL;
> >  	struct dentry *dn;
> >  	struct ceph_acl_sec_ctx as_ctx = {};
> >  	bool try_async = ceph_test_mount_opt(fsc, ASYNC_DIROPS);
> > @@ -675,21 +671,21 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> >  
> >  	if (dentry->d_name.len > NAME_MAX)
> >  		return -ENAMETOOLONG;
> > -
> > +retry:
> >  	if (flags & O_CREAT) {
> >  		if (ceph_quota_is_max_files_exceeded(dir))
> >  			return -EDQUOT;
> > -		err = ceph_pre_init_acls(dir, &mode, &as_ctx);
> > -		if (err < 0)
> > -			return err;
> > -		err = ceph_security_init_secctx(dentry, mode, &as_ctx);
> > -		if (err < 0)
> > +
> > +		new_inode = ceph_new_inode(dir, dentry, &mode, &as_ctx);
> > +		if (IS_ERR(new_inode)) {
> > +			err = PTR_ERR(new_inode);
> >  			goto out_ctx;
> > +		}
> 
> Is the 'goto out_ctx;' correct here?  It looks like it should be
> 'return PTR_ERR(new_inode)'
> 

Yes, it's correct...see below.

> > +/**
> > + * ceph_new_inode - allocate a new inode in advance of an expected create
> > + * @dir: parent directory for new inode
> > + * @mode: mode of new inode
> > + */
> > +struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
> > +			     umode_t *mode, struct ceph_acl_sec_ctx *as_ctx)
> 
> Some parameters aren't documented.
> 

Thanks, fixed.

> > +	int err;
> >  	struct inode *inode;
> >  
> > -	inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare,
> > -			     ceph_set_ino_cb, &vino);
> > +	inode = new_inode_pseudo(dir->i_sb);
> >  	if (!inode)
> >  		return ERR_PTR(-ENOMEM);
> >  
> > +	if (!S_ISLNK(*mode)) {
> > +		err = ceph_pre_init_acls(dir, mode, as_ctx);
> > +		if (err < 0)
> > +			goto out_err;
> > +	}
> > +
> > +	err = ceph_security_init_secctx(dentry, *mode, as_ctx);
> > +	if (err < 0)
> > +		goto out_err;
> > +
> > +	inode->i_state = 0;
> > +	inode->i_mode = *mode;
> > +	return inode;
> > +out_err:
> > +	iput(inode);
> > +	return ERR_PTR(err);
> > +}
> 
> Should this be freeing anything from the ceph_acl_sec_ctx on error?
> 

For now, I'm leaving that to the callers. It's arguably uglier to do it
that way but ceph_release_acl_sec_ctx needs to be called at the end of
the callers anyway, and it's not currently idempotent.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 08/16] ceph: implement -o test_dummy_encryption mount option

[Jeff Layton]

On Mon, 2020-09-14 at 18:23 -0700, Eric Biggers wrote:
> On Mon, Sep 14, 2020 at 03:16:59PM -0400, Jeff Layton wrote:
> > +	case Opt_test_dummy_encryption:
> > +		kfree(fsopt->test_dummy_encryption);
> > +#ifdef CONFIG_FS_ENCRYPTION
> > +		fsopt->test_dummy_encryption = param->string;
> > +		param->string = NULL;
> > +		fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
> > +#else
> > +		warnfc(fc, "FS encryption not supported: test_dummy_encryption mount option ignored");
> > +#endif
> 
> Seems that the kfree() should go in the CONFIG_FS_ENCRYPTION=y block.
> 
> Also, is there much reason to have the CEPH_MOUNT_OPT_TEST_DUMMY_ENC flag
> instead of just checking fsopt->test_dummy_encryption != NULL?
> 

Yes. That distinguishes between the case where someone has used
-o test_dummy_encryption and -o test_dummy_encryption=v2.

> > +#ifdef CONFIG_FS_ENCRYPTION
> > +static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
> > +						struct ceph_mount_options *fsopt)
> > +{
> > +	struct ceph_fs_client *fsc = sb->s_fs_info;
> > +
> > +	if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
> > +		substring_t arg = { };
> > +
> > +		/*
> > +		 * No changing encryption context on remount. Note that
> > +		 * fscrypt_set_test_dummy_encryption will validate the version
> > +		 * string passed in (if any).
> > +		 */
> > +		if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy)
> > +			return -EEXIST;
> 
> Maybe show an error message here, with errorfc()?
> See the message that ext4_set_test_dummy_encryption() shows.
> 

Good idea. I've rolled in error messages similar to the ones in ext4.

> > +
> > +		/* Ewwwwwwww */
> > +		if (fsc->mount_options->test_dummy_encryption) {
> > +			arg.from = fsc->mount_options->test_dummy_encryption;
> > +			arg.to = arg.from + strlen(arg.from) - 1;
> > +		}
> 
> We should probably make fscrypt_set_test_dummy_encryption() take a
> 'const char *' to avoid having to create a substring_t here.
> 

Yes, please. I didn't want to do that with most of the current fscrypt-
enabled fs using the old mount API. Some of them may need to copy the
argument so that it's properly terminated.

We could also just add a wrapper that turns the const char * into a
substring_t before calling fscrypt_set_test_dummy_encryption.

> > +		return fscrypt_set_test_dummy_encryption(sb, &arg, &fsc->dummy_enc_policy);
> 
> Likewise, maybe show an error message if this fails.
> 
> > +	} else {
> > +		if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && fsc->dummy_enc_policy.policy)
> > +			return -EEXIST;
> 
> If remount on ceph behaves as "don't change options that aren't specified",
> similar to ext4, then there's no need for this hunk here.
> 

Good point.

> >  static int ceph_reconfigure_fc(struct fs_context *fc)
> >  {
> > +	int err;
> >  	struct ceph_parse_opts_ctx *pctx = fc->fs_private;
> >  	struct ceph_mount_options *fsopt = pctx->opts;
> > -	struct ceph_fs_client *fsc = ceph_sb_to_client(fc->root->d_sb);
> > +	struct super_block *sb = fc->root->d_sb;
> > +	struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
> >  
> >  	if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)
> >  		ceph_set_mount_opt(fsc, ASYNC_DIROPS);
> >  	else
> >  		ceph_clear_mount_opt(fsc, ASYNC_DIROPS);
> >  
> > -	sync_filesystem(fc->root->d_sb);
> > +	err = ceph_set_test_dummy_encryption(sb, fc, fsopt);
> > +	if (err)
> > +		return err;
> > +
> > +	sync_filesystem(sb);
> >  	return 0;
> >  }
> 
> Seems that ceph_set_test_dummy_encryption() should go at the beginning, since
> otherwise it can fail after something was already changed.
> 

Good catch. Fixed.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH v3 12/16] ceph: add encrypted fname handling to ceph_mdsc_build_path

[Eric Biggers]

On Wed, Sep 16, 2020 at 08:30:01AM -0400, Jeff Layton wrote:
> 
> It sounds like we'll probably need to stabilize some version of the
> nokey name so that we can allow the MDS to look them up. Would it be a
> problem for us to use the current version of the nokey name format for
> this, or would it be better to come up with some other distinct format
> for this?
> 

You could use the current version, with the dirhash field changed from u32 to
__le32 so that it doesn't depend on CPU endianness.  But you should also
consider using just base64(SHA256(filename)).  The SHA256(filename) approach
wouldn't include a dirhash, and it would handle short filenames less
efficiently.  However, it would be simpler.  Would it be any easier for you?

I'm not sure which would be better from a fs/crypto/ perspective.  For *now*, it
would be easier if you just used the current 'struct fscrypt_nokey_name'.
However, anything you use would be set in stone, whereas as-is the format can be
changed at any time.  In fact, we changed it recently; see commit edc440e3d27f.

If we happen to change the nokey name in the future for local filesystems (say,
to use BLAKE2 instead of SHA256, or to support longer dirhashes), then it would
be easier if the stable format were just SHA256(filename).

It's not a huge deal though.  So if e.g. you like that the current format avoids
the cryptographic hash for the vast majority of filenames, and if you're fine
with the slightly increased complexity, you can just use it.

- Eric

Re: [RFC PATCH v3 12/16] ceph: add encrypted fname handling to ceph_mdsc_build_path

[Jeff Layton]

On Wed, 2020-09-16 at 10:36 -0700, Eric Biggers wrote:
> On Wed, Sep 16, 2020 at 08:30:01AM -0400, Jeff Layton wrote:
> > It sounds like we'll probably need to stabilize some version of the
> > nokey name so that we can allow the MDS to look them up. Would it be a
> > problem for us to use the current version of the nokey name format for
> > this, or would it be better to come up with some other distinct format
> > for this?
> > 
> 
> You could use the current version, with the dirhash field changed from u32 to
> __le32 so that it doesn't depend on CPU endianness.  But you should also
> consider using just base64(SHA256(filename)).  The SHA256(filename) approach
> wouldn't include a dirhash, and it would handle short filenames less
> efficiently.  However, it would be simpler.  Would it be any easier for you?
> 
> I'm not sure which would be better from a fs/crypto/ perspective.  For *now*, it
> would be easier if you just used the current 'struct fscrypt_nokey_name'.
> However, anything you use would be set in stone, whereas as-is the format can be
> changed at any time.  In fact, we changed it recently; see commit edc440e3d27f.
> 
> If we happen to change the nokey name in the future for local filesystems (say,
> to use BLAKE2 instead of SHA256, or to support longer dirhashes), then it would
> be easier if the stable format were just SHA256(filename).
> 
> It's not a huge deal though.  So if e.g. you like that the current format avoids
> the cryptographic hash for the vast majority of filenames, and if you're fine
> with the slightly increased complexity, you can just use it.
> 

The problem with using a different scheme from the presentation format
is this:

Suppose I don't have the key for a directory and do a readdir() in
there, and get back a nokey name with the hash at the end. A little
while later, the dentry gets evicted from the cache.

Userland then comes back and wants to do something with that dentry
(maybe an unlink or stat). Now I have to look it up. At that point, I
don't really have a way to resolve that on the client [1]. I have to ask
the server to do it. What do I ask it to look up?

Storing the stable format as a full SHA256 hash of the name is
problematic as I don't think we can convert the nokey name to it
directly (can we?).

If we store the current nokey format (or some variant of it that doesn't
include the dirhash fields) then we should be able to look up the
dentry, even when we don't have complete dir contents.
-- 
Jeff Layton <jlayton@kernel.org>

[1]: ok, technically we could do a readdir in the directory and try to
match the nokey name by deriving them from the full crypttext, but
that's potentially _very_ expensive if the dir is large.

Re: [RFC PATCH v3 12/16] ceph: add encrypted fname handling to ceph_mdsc_build_path

[Eric Biggers]

On Wed, Sep 16, 2020 at 02:04:23PM -0400, Jeff Layton wrote:
> On Wed, 2020-09-16 at 10:36 -0700, Eric Biggers wrote:
> > On Wed, Sep 16, 2020 at 08:30:01AM -0400, Jeff Layton wrote:
> > > It sounds like we'll probably need to stabilize some version of the
> > > nokey name so that we can allow the MDS to look them up. Would it be a
> > > problem for us to use the current version of the nokey name format for
> > > this, or would it be better to come up with some other distinct format
> > > for this?
> > > 
> > 
> > You could use the current version, with the dirhash field changed from u32 to
> > __le32 so that it doesn't depend on CPU endianness.  But you should also
> > consider using just base64(SHA256(filename)).  The SHA256(filename) approach
> > wouldn't include a dirhash, and it would handle short filenames less
> > efficiently.  However, it would be simpler.  Would it be any easier for you?
> > 
> > I'm not sure which would be better from a fs/crypto/ perspective.  For *now*, it
> > would be easier if you just used the current 'struct fscrypt_nokey_name'.
> > However, anything you use would be set in stone, whereas as-is the format can be
> > changed at any time.  In fact, we changed it recently; see commit edc440e3d27f.
> > 
> > If we happen to change the nokey name in the future for local filesystems (say,
> > to use BLAKE2 instead of SHA256, or to support longer dirhashes), then it would
> > be easier if the stable format were just SHA256(filename).
> > 
> > It's not a huge deal though.  So if e.g. you like that the current format avoids
> > the cryptographic hash for the vast majority of filenames, and if you're fine
> > with the slightly increased complexity, you can just use it.
> > 
> 
> The problem with using a different scheme from the presentation format
> is this:
> 
> Suppose I don't have the key for a directory and do a readdir() in
> there, and get back a nokey name with the hash at the end. A little
> while later, the dentry gets evicted from the cache.
> 
> Userland then comes back and wants to do something with that dentry
> (maybe an unlink or stat). Now I have to look it up. At that point, I
> don't really have a way to resolve that on the client [1]. I have to ask
> the server to do it. What do I ask it to look up?
> 
> Storing the stable format as a full SHA256 hash of the name is
> problematic as I don't think we can convert the nokey name to it
> directly (can we?).
> 
> If we store the current nokey format (or some variant of it that doesn't
> include the dirhash fields) then we should be able to look up the
> dentry, even when we don't have complete dir contents.
> -- 
> Jeff Layton <jlayton@kernel.org>
> 
> [1]: ok, technically we could do a readdir in the directory and try to
> match the nokey name by deriving them from the full crypttext, but
> that's potentially _very_ expensive if the dir is large.

You'd need to use the same format for storage and presentation.

My point is that other filesystems don't have that constraint, and it could
happen that we decide to change the presentation format for those *other*
filesystems in the future.  Say, if SHA-256 falls out of favor and people want
it replaced with a different cryptographic hash algorithm; or if a filesystem
with 128-bit dirhashes adds support for fscrypt; or if it turns out that a
different variant of base64 would be better.

The ceph format would then be a "legacy" format that we'd need to support.  That
would be somewhat easier if it was simply base64(SHA-256(filename)), vs.
something more complicated.  Again, not a huge deal though, and maybe you want
to avoid doing the hash for short filenames anyway.

- Eric

Re: [RFC PATCH v3 01/16] vfs: export new_inode_pseudo

[Al Viro]

On Mon, Sep 14, 2020 at 03:16:52PM -0400, Jeff Layton wrote:
> Ceph needs to be able to allocate inodes ahead of a create that might
> involve a fscrypt-encrypted inode. new_inode() almost fits the bill,
> but it puts the inode on the sb->s_inodes list and when we go to hash
> it, that might be done again.
> 
> We could work around that by setting I_CREATING on the new inode, but
> that causes ilookup5 to return -ESTALE if something tries to find it
> before I_NEW is cleared. To work around all of this, just use
> new_inode_pseudo which doesn't add it to the list.

Er...  Why would you _not_ want -ESTALE in that situation?

Re: [RFC PATCH v3 01/16] vfs: export new_inode_pseudo

[Jeff Layton]

On Wed, 2020-09-23 at 04:41 +0100, Al Viro wrote:
> On Mon, Sep 14, 2020 at 03:16:52PM -0400, Jeff Layton wrote:
> > Ceph needs to be able to allocate inodes ahead of a create that might
> > involve a fscrypt-encrypted inode. new_inode() almost fits the bill,
> > but it puts the inode on the sb->s_inodes list and when we go to hash
> > it, that might be done again.
> > 
> > We could work around that by setting I_CREATING on the new inode, but
> > that causes ilookup5 to return -ESTALE if something tries to find it
> > before I_NEW is cleared. To work around all of this, just use
> > new_inode_pseudo which doesn't add it to the list.
> 
> Er...  Why would you _not_ want -ESTALE in that situation?

I'm hashing the new inode just before sending the create request to the
MDS. When the reply comes in, the client then searches for that inode in
the hash. If I_NEW has been cleared in the meantime -- no problem. If
not, then we want the task handling the reply to wait until it is (and
not get back an -ESTALE).
-- 
Jeff Layton <jlayton@kernel.org>