lkml.org 
[lkml]   [2023]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] ceph: add support for encrypted snapshot names
From

On 9/6/23 21:47, Luís Henriques wrote:
> Since filenames in encrypted directories are encrypted and shown as
> a base64-encoded string when the directory is locked, make snapshot
> names show a similar behaviour.
>
> When creating a snapshot, .snap directories for every subdirectory will
> show the snapshot name in the "long format":
>
> # mkdir .snap/my-snap
> # ls my-dir/.snap/
> _my-snap_1099511627782
>
> Encrypted snapshots will need to be able to handle these by
> encrypting/decrypting only the snapshot part of the string ('my-snap').
>
> Also, since the MDS prevents snapshot names to be bigger than 240
> characters it is necessary to adapt CEPH_NOHASH_NAME_MAX to accommodate
> this extra limitation.
>
> [ idryomov: drop const on !CONFIG_FS_ENCRYPTION branch too ]
>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Reviewed-by: Xiubo Li <xiubli@redhat.com>
> Reviewed-by: Milind Changire <mchangir@redhat.com>
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> ---
> As reported by Dan Carpenter, the initial version of this patch wasn't
> dealing correctly with the ceph_get_inode() return value: it never returns
> NULL. Simply dropped the code that handled the case where a NULL was
> being returned.
>
> Xiubo, can you please pick this new revision, if you think it's correct?

Hi Luis,

Please generate a new patch to fix this, the fscrypt corresponding
commits have been merged already.

Thanks

- Xiubo

> And thanks, Dan!
>
> Cheers,
> --
> Luís
>
> fs/ceph/crypto.c | 195 +++++++++++++++++++++++++++++++++++++++--------
> fs/ceph/crypto.h | 12 +--
> fs/ceph/inode.c | 33 +++++++-
> 3 files changed, 199 insertions(+), 41 deletions(-)
>
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index 9494bf485f6a..7d0b9b5ccfc6 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -193,17 +193,99 @@ void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
> swap(req->r_fscrypt_auth, as->fscrypt_auth);
> }
>
> -int ceph_encode_encrypted_dname(const struct inode *parent,
> - struct qstr *d_name, char *buf)
> +/*
> + * User-created snapshots can't start with '_'. Snapshots that start with this
> + * character are special (hint: there aren't real snapshots) and use the
> + * following format:
> + *
> + * _<SNAPSHOT-NAME>_<INODE-NUMBER>
> + *
> + * where:
> + * - <SNAPSHOT-NAME> - the real snapshot name that may need to be decrypted,
> + * - <INODE-NUMBER> - the inode number (in decimal) for the actual snapshot
> + *
> + * This function parses these snapshot names and returns the inode
> + * <INODE-NUMBER>. 'name_len' will also bet set with the <SNAPSHOT-NAME>
> + * length.
> + */
> +static struct inode *parse_longname(const struct inode *parent,
> + const char *name, int *name_len)
> {
> + struct inode *dir = NULL;
> + struct ceph_vino vino = { .snap = CEPH_NOSNAP };
> + char *inode_number;
> + char *name_end;
> + int orig_len = *name_len;
> + int ret = -EIO;
> +
> + /* Skip initial '_' */
> + name++;
> + name_end = strrchr(name, '_');
> + if (!name_end) {
> + dout("Failed to parse long snapshot name: %s\n", name);
> + return ERR_PTR(-EIO);
> + }
> + *name_len = (name_end - name);
> + if (*name_len <= 0) {
> + pr_err("Failed to parse long snapshot name\n");
> + return ERR_PTR(-EIO);
> + }
> +
> + /* Get the inode number */
> + inode_number = kmemdup_nul(name_end + 1,
> + orig_len - *name_len - 2,
> + GFP_KERNEL);
> + if (!inode_number)
> + return ERR_PTR(-ENOMEM);
> + ret = kstrtou64(inode_number, 10, &vino.ino);
> + if (ret) {
> + dout("Failed to parse inode number: %s\n", name);
> + dir = ERR_PTR(ret);
> + goto out;
> + }
> +
> + /* And finally the inode */
> + dir = ceph_find_inode(parent->i_sb, vino);
> + if (!dir) {
> + /* This can happen if we're not mounting cephfs on the root */
> + dir = ceph_get_inode(parent->i_sb, vino, NULL);
> + }
> + if (IS_ERR(dir))
> + dout("Can't find inode %s (%s)\n", inode_number, name);
> +
> +out:
> + kfree(inode_number);
> + return dir;
> +}
> +
> +int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
> + char *buf)
> +{
> + struct inode *dir = parent;
> + struct qstr iname;
> u32 len;
> + int name_len;
> int elen;
> int ret;
> - u8 *cryptbuf;
> + u8 *cryptbuf = NULL;
> +
> + iname.name = d_name->name;
> + name_len = d_name->len;
> +
> + /* Handle the special case of snapshot names that start with '_' */
> + if ((ceph_snap(dir) == CEPH_SNAPDIR) && (name_len > 0) &&
> + (iname.name[0] == '_')) {
> + dir = parse_longname(parent, iname.name, &name_len);
> + if (IS_ERR(dir))
> + return PTR_ERR(dir);
> + iname.name++; /* skip initial '_' */
> + }
> + iname.len = name_len;
>
> - if (!fscrypt_has_encryption_key(parent)) {
> + if (!fscrypt_has_encryption_key(dir)) {
> memcpy(buf, d_name->name, d_name->len);
> - return d_name->len;
> + elen = d_name->len;
> + goto out;
> }
>
> /*
> @@ -212,19 +294,23 @@ int ceph_encode_encrypted_dname(const struct inode *parent,
> *
> * See: fscrypt_setup_filename
> */
> - if (!fscrypt_fname_encrypted_size(parent, d_name->len, NAME_MAX, &len))
> - return -ENAMETOOLONG;
> + if (!fscrypt_fname_encrypted_size(dir, iname.len, NAME_MAX, &len)) {
> + elen = -ENAMETOOLONG;
> + goto out;
> + }
>
> /* Allocate a buffer appropriate to hold the result */
> cryptbuf = kmalloc(len > CEPH_NOHASH_NAME_MAX ? NAME_MAX : len,
> GFP_KERNEL);
> - if (!cryptbuf)
> - return -ENOMEM;
> + if (!cryptbuf) {
> + elen = -ENOMEM;
> + goto out;
> + }
>
> - ret = fscrypt_fname_encrypt(parent, d_name, cryptbuf, len);
> + ret = fscrypt_fname_encrypt(dir, &iname, cryptbuf, len);
> if (ret) {
> - kfree(cryptbuf);
> - return ret;
> + elen = ret;
> + goto out;
> }
>
> /* hash the end if the name is long enough */
> @@ -243,13 +329,31 @@ int ceph_encode_encrypted_dname(const struct inode *parent,
>
> /* base64 encode the encrypted name */
> elen = ceph_base64_encode(cryptbuf, len, buf);
> - kfree(cryptbuf);
> dout("base64-encoded ciphertext name = %.*s\n", elen, buf);
> +
> + /* To understand the 240 limit, see CEPH_NOHASH_NAME_MAX comments */
> + WARN_ON(elen > 240);
> + if ((elen > 0) && (dir != parent)) {
> + char tmp_buf[NAME_MAX];
> +
> + elen = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
> + elen, buf, dir->i_ino);
> + memcpy(buf, tmp_buf, elen);
> + }
> +
> +out:
> + kfree(cryptbuf);
> + if (dir != parent) {
> + if ((dir->i_state & I_NEW))
> + discard_new_inode(dir);
> + else
> + iput(dir);
> + }
> return elen;
> }
>
> -int ceph_encode_encrypted_fname(const struct inode *parent,
> - struct dentry *dentry, char *buf)
> +int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
> + char *buf)
> {
> WARN_ON_ONCE(!fscrypt_has_encryption_key(parent));
>
> @@ -274,29 +378,42 @@ int ceph_encode_encrypted_fname(const struct inode *parent,
> int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> struct fscrypt_str *oname, bool *is_nokey)
> {
> - int ret;
> + struct inode *dir = fname->dir;
> struct fscrypt_str _tname = FSTR_INIT(NULL, 0);
> struct fscrypt_str iname;
> -
> - if (!IS_ENCRYPTED(fname->dir)) {
> - oname->name = fname->name;
> - oname->len = fname->name_len;
> - return 0;
> - }
> + char *name = fname->name;
> + int name_len = fname->name_len;
> + int ret;
>
> /* Sanity check that the resulting name will fit in the buffer */
> if (fname->name_len > NAME_MAX || fname->ctext_len > NAME_MAX)
> return -EIO;
>
> - ret = ceph_fscrypt_prepare_readdir(fname->dir);
> - if (ret < 0)
> - return ret;
> + /* Handle the special case of snapshot names that start with '_' */
> + if ((ceph_snap(dir) == CEPH_SNAPDIR) && (name_len > 0) &&
> + (name[0] == '_')) {
> + dir = parse_longname(dir, name, &name_len);
> + if (IS_ERR(dir))
> + return PTR_ERR(dir);
> + name++; /* skip initial '_' */
> + }
> +
> + if (!IS_ENCRYPTED(dir)) {
> + oname->name = fname->name;
> + oname->len = fname->name_len;
> + ret = 0;
> + goto out_inode;
> + }
> +
> + ret = ceph_fscrypt_prepare_readdir(dir);
> + if (ret)
> + goto out_inode;
>
> /*
> * Use the raw dentry name as sent by the MDS instead of
> * generating a nokey name via fscrypt.
> */
> - if (!fscrypt_has_encryption_key(fname->dir)) {
> + if (!fscrypt_has_encryption_key(dir)) {
> if (fname->no_copy)
> oname->name = fname->name;
> else
> @@ -304,7 +421,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> oname->len = fname->name_len;
> if (is_nokey)
> *is_nokey = true;
> - return 0;
> + ret = 0;
> + goto out_inode;
> }
>
> if (fname->ctext_len == 0) {
> @@ -313,12 +431,11 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> if (!tname) {
> ret = fscrypt_fname_alloc_buffer(NAME_MAX, &_tname);
> if (ret)
> - return ret;
> + goto out_inode;
> tname = &_tname;
> }
>
> - declen = ceph_base64_decode(fname->name, fname->name_len,
> - tname->name);
> + declen = ceph_base64_decode(name, name_len, tname->name);
> if (declen <= 0) {
> ret = -EIO;
> goto out;
> @@ -330,9 +447,25 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
> iname.len = fname->ctext_len;
> }
>
> - ret = fscrypt_fname_disk_to_usr(fname->dir, 0, 0, &iname, oname);
> + ret = fscrypt_fname_disk_to_usr(dir, 0, 0, &iname, oname);
> + if (!ret && (dir != fname->dir)) {
> + char tmp_buf[CEPH_BASE64_CHARS(NAME_MAX)];
> +
> + name_len = snprintf(tmp_buf, sizeof(tmp_buf), "_%.*s_%ld",
> + oname->len, oname->name, dir->i_ino);
> + memcpy(oname->name, tmp_buf, name_len);
> + oname->len = name_len;
> + }
> +
> out:
> fscrypt_fname_free_buffer(&_tname);
> +out_inode:
> + if ((dir != fname->dir) && !IS_ERR(dir)) {
> + if ((dir->i_state & I_NEW))
> + discard_new_inode(dir);
> + else
> + iput(dir);
> + }
> return ret;
> }
>
> diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> index 6afa6d888fe0..47e0c319fc68 100644
> --- a/fs/ceph/crypto.h
> +++ b/fs/ceph/crypto.h
> @@ -102,10 +102,10 @@ int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
> struct ceph_acl_sec_ctx *as);
> void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
> struct ceph_acl_sec_ctx *as);
> -int ceph_encode_encrypted_dname(const struct inode *parent,
> - struct qstr *d_name, char *buf);
> -int ceph_encode_encrypted_fname(const struct inode *parent,
> - struct dentry *dentry, char *buf);
> +int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name,
> + char *buf);
> +int ceph_encode_encrypted_fname(struct inode *parent, struct dentry *dentry,
> + char *buf);
>
> static inline int ceph_fname_alloc_buffer(struct inode *parent,
> struct fscrypt_str *fname)
> @@ -194,14 +194,14 @@ static inline void ceph_fscrypt_as_ctx_to_req(struct ceph_mds_request *req,
> {
> }
>
> -static inline int ceph_encode_encrypted_dname(const struct inode *parent,
> +static inline int ceph_encode_encrypted_dname(struct inode *parent,
> struct qstr *d_name, char *buf)
> {
> memcpy(buf, d_name->name, d_name->len);
> return d_name->len;
> }
>
> -static inline int ceph_encode_encrypted_fname(const struct inode *parent,
> +static inline int ceph_encode_encrypted_fname(struct inode *parent,
> struct dentry *dentry, char *buf)
> {
> return -EOPNOTSUPP;
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 054fd66609b7..83c420646f90 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -91,9 +91,15 @@ struct inode *ceph_new_inode(struct inode *dir, struct dentry *dentry,
> if (err < 0)
> goto out_err;
>
> - err = ceph_fscrypt_prepare_context(dir, inode, as_ctx);
> - if (err)
> - goto out_err;
> + /*
> + * We'll skip setting fscrypt context for snapshots, leaving that for
> + * the handle_reply().
> + */
> + if (ceph_snap(dir) != CEPH_SNAPDIR) {
> + err = ceph_fscrypt_prepare_context(dir, inode, as_ctx);
> + if (err)
> + goto out_err;
> + }
>
> return inode;
> out_err:
> @@ -159,6 +165,7 @@ struct inode *ceph_get_snapdir(struct inode *parent)
> };
> struct inode *inode = ceph_get_inode(parent->i_sb, vino, NULL);
> struct ceph_inode_info *ci = ceph_inode(inode);
> + int ret = -ENOTDIR;
>
> if (IS_ERR(inode))
> return inode;
> @@ -184,6 +191,24 @@ struct inode *ceph_get_snapdir(struct inode *parent)
> ci->i_rbytes = 0;
> ci->i_btime = ceph_inode(parent)->i_btime;
>
> +#ifdef CONFIG_FS_ENCRYPTION
> + /* if encrypted, just borrow fscrypt_auth from parent */
> + if (IS_ENCRYPTED(parent)) {
> + struct ceph_inode_info *pci = ceph_inode(parent);
> +
> + ci->fscrypt_auth = kmemdup(pci->fscrypt_auth,
> + pci->fscrypt_auth_len,
> + GFP_KERNEL);
> + if (ci->fscrypt_auth) {
> + inode->i_flags |= S_ENCRYPTED;
> + ci->fscrypt_auth_len = pci->fscrypt_auth_len;
> + } else {
> + dout("Failed to alloc snapdir fscrypt_auth\n");
> + ret = -ENOMEM;
> + goto err;
> + }
> + }
> +#endif
> if (inode->i_state & I_NEW) {
> inode->i_op = &ceph_snapdir_iops;
> inode->i_fop = &ceph_snapdir_fops;
> @@ -197,7 +222,7 @@ struct inode *ceph_get_snapdir(struct inode *parent)
> discard_new_inode(inode);
> else
> iput(inode);
> - return ERR_PTR(-ENOTDIR);
> + return ERR_PTR(ret);
> }
>
> const struct inode_operations ceph_file_iops = {
>

\
 
 \ /
  Last update: 2023-09-07 02:27    [W:1.608 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site