From: Jeff Layton <jlayton@kernel.org>
To: ceph-devel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org, linux-fscrypt@vger.kernel.org,
ebiggers@kernel.org
Subject: Re: [RFC PATCH v2 18/18] ceph: create symlinks with encrypted and base64-encoded targets
Date: Fri, 04 Sep 2020 12:11:45 -0400 [thread overview]
Message-ID: <baca6ac9ed6ae9ba0700595f7aaa6f4fe8cf058d.camel@kernel.org> (raw)
In-Reply-To: <20200904160537.76663-19-jlayton@kernel.org>
On Fri, 2020-09-04 at 12:05 -0400, Jeff Layton wrote:
> 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 | 82 +++++++++++++++++++++++++++++++++++++++----------
> fs/ceph/super.h | 3 ++
> 3 files changed, 96 insertions(+), 21 deletions(-)
>
FWIW, this patch is not working right, and I'm not sure why.
I've validated that I'm getting the same crypttext out
of fscrypt_encrypt_symlink that I'm feeding into fscrypt_get_symlink,
but the contents always look like gibberish in a readlink.
Did I make an obvious mistake somewhere?
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 0ba17c592fe1..7ff8921dd3a7 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -948,6 +948,7 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
> struct ceph_mds_client *mdsc = fsc->mdsc;
> 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;
>
> @@ -973,11 +974,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(BASE64_CHARS(osd_link.len) + 1, GFP_KERNEL);
> + if (!req->r_path2) {
> + err = -ENOMEM;
> + goto out_req;
> + }
> +
> + len = base64_encode_fname(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);
> @@ -997,6 +1020,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 e578e4cdcf30..dd39f886b03c 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -14,6 +14,7 @@
> #include <linux/random.h>
> #include <linux/sort.h>
> #include <linux/iversion.h>
> +#include <linux/base64_fname.h>
>
> #include "super.h"
> #include "mds_client.h"
> @@ -33,8 +34,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);
>
> /*
> @@ -503,6 +502,7 @@ struct inode *ceph_alloc_inode(struct super_block *sb)
> atomic64_set(&ci->i_complete_seq[0], 0);
> atomic64_set(&ci->i_complete_seq[1], 0);
> ci->i_symlink = NULL;
> + ci->i_symlink_len = 0;
>
> ci->i_max_bytes = 0;
> ci->i_max_files = 0;
> @@ -590,6 +590,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);
> }
>
> @@ -991,34 +992,60 @@ 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 = base64_decode_fname(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)
> + if (!ci->i_symlink) {
> ci->i_symlink = sym;
> - else
> + ci->i_symlink_len = symlen;
> + } else {
> kfree(sym); /* lost a race */
> + }
> + }
> +
> + 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;
> }
> - inode->i_link = ci->i_symlink;
> break;
> case S_IFDIR:
> inode->i_op = &ceph_dir_iops;
> @@ -2123,16 +2150,37 @@ 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 (inode->i_link)
> + return inode->i_link;
> +
> + if (!dentry)
> + return ERR_PTR(-ECHILD);
> +
> + return fscrypt_get_symlink(inode, ci->i_symlink, ci->i_symlink_len, 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 7c859824f64c..eea0ee17b747 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -344,6 +344,7 @@ struct ceph_inode_info {
> u64 i_max_bytes, i_max_files;
>
> s32 i_dir_pin;
> + u32 i_symlink_len;
>
> struct rb_root i_fragtree;
> int i_fragtree_nsplits;
> @@ -932,6 +933,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);
--
Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2020-09-04 16:11 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 16:05 [RFC PATCH v2 00/18] ceph+fscrypt: context, filename and symlink support Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 01/18] vfs: export new_inode_pseudo Jeff Layton
2020-09-08 3:38 ` Eric Biggers
2020-09-08 11:27 ` Jeff Layton
2020-09-08 22:31 ` Eric Biggers
2020-09-09 10:47 ` Jeff Layton
2020-09-09 16:12 ` Eric Biggers
2020-09-09 16:51 ` Jeff Layton
2020-09-09 18:49 ` Eric Biggers
2020-09-09 19:24 ` Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 02/18] fscrypt: drop unused inode argument from fscrypt_fname_alloc_buffer Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 03/18] fscrypt: export fscrypt_d_revalidate Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 04/18] fscrypt: add fscrypt_new_context_from_inode Jeff Layton
2020-09-08 3:48 ` Eric Biggers
2020-09-08 11:29 ` Jeff Layton
2020-09-08 12:29 ` Jeff Layton
2020-09-08 22:34 ` Eric Biggers
2020-09-04 16:05 ` [RFC PATCH v2 05/18] fscrypt: don't balk when inode is already marked encrypted Jeff Layton
2020-09-08 3:52 ` Eric Biggers
2020-09-08 12:54 ` Jeff Layton
2020-09-08 23:08 ` Eric Biggers
2020-09-04 16:05 ` [RFC PATCH v2 06/18] fscrypt: move nokey_name conversion to separate function and export it Jeff Layton
2020-09-08 3:55 ` Eric Biggers
2020-09-08 12:50 ` Jeff Layton
2020-09-08 22:53 ` Eric Biggers
2020-09-09 16:02 ` Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 07/18] lib: lift fscrypt base64 conversion into lib/ Jeff Layton
2020-09-08 3:59 ` Eric Biggers
2020-09-08 12:51 ` Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 08/18] ceph: add fscrypt ioctls Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 09/18] ceph: crypto context handling for ceph Jeff Layton
2020-09-08 4:29 ` Eric Biggers
2020-09-08 16:14 ` Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 10/18] ceph: preallocate inode for ops that may create one Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 11/18] ceph: add routine to create context prior to RPC Jeff Layton
2020-09-08 4:43 ` Eric Biggers
2020-09-04 16:05 ` [RFC PATCH v2 12/18] ceph: set S_ENCRYPTED bit if new inode has encryption.ctx xattr Jeff Layton
2020-09-08 4:57 ` Eric Biggers
2020-09-09 12:20 ` Jeff Layton
2020-09-09 15:53 ` Jeff Layton
2020-09-09 16:33 ` Eric Biggers
2020-09-09 17:19 ` Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 13/18] ceph: make ceph_msdc_build_path use ref-walk Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 14/18] ceph: add encrypted fname handling to ceph_mdsc_build_path Jeff Layton
2020-09-08 5:06 ` Eric Biggers
2020-09-09 12:24 ` Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 15/18] ceph: make d_revalidate call fscrypt revalidator for encrypted dentries Jeff Layton
2020-09-08 5:12 ` Eric Biggers
2020-09-09 12:26 ` Jeff Layton
2020-09-09 16:18 ` Eric Biggers
2020-09-04 16:05 ` [RFC PATCH v2 16/18] ceph: add support to readdir for encrypted filenames Jeff Layton
2020-09-08 5:34 ` Eric Biggers
2020-09-09 13:02 ` Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 17/18] ceph: add fscrypt support to ceph_fill_trace Jeff Layton
2020-09-04 16:05 ` [RFC PATCH v2 18/18] ceph: create symlinks with encrypted and base64-encoded targets Jeff Layton
2020-09-04 16:11 ` Jeff Layton [this message]
2020-09-08 5:43 ` Eric Biggers
2020-09-08 5:54 ` [RFC PATCH v2 00/18] ceph+fscrypt: context, filename and symlink support Eric Biggers
2020-09-08 12:09 ` Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=baca6ac9ed6ae9ba0700595f7aaa6f4fe8cf058d.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=ceph-devel@vger.kernel.org \
--cc=ebiggers@kernel.org \
--cc=linux-fscrypt@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.