mbox series

[RFC,v2,00/18] ceph+fscrypt: context, filename and symlink support

Message ID 20200904160537.76663-1-jlayton@kernel.org (mailing list archive)
Headers show
Series ceph+fscrypt: context, filename and symlink support | expand

Message

Jeff Layton Sept. 4, 2020, 4:05 p.m. UTC
This is a second posting of the ceph+fscrypt integration work that I've
been experimenting with. The main change with this patch is that I've
based this on top of Eric's fscrypt-pending set. That necessitated a
change to allocate inodes much earlier than we have traditionally, prior
to sending an RPC instead of waiting on the reply.

Note that this just covers the crypto contexts and filenames. I've also
added a patch to encrypt symlink contents as well, but it doesn't seem to
be working correctly.

The file contents work is next, but I'm sort of waiting until some work
to the fscache infrastructure is settled. It would be nice if fscache
also stored encrypted file contents when we plumb this in.

Jeff Layton (18):
  vfs: export new_inode_pseudo
  fscrypt: drop unused inode argument from fscrypt_fname_alloc_buffer
  fscrypt: export fscrypt_d_revalidate
  fscrypt: add fscrypt_new_context_from_inode
  fscrypt: don't balk when inode is already marked encrypted
  fscrypt: move nokey_name conversion to separate function and export it
  lib: lift fscrypt base64 conversion into lib/
  ceph: add fscrypt ioctls
  ceph: crypto context handling for ceph
  ceph: preallocate inode for ops that may create one
  ceph: add routine to create context prior to RPC
  ceph: set S_ENCRYPTED bit if new inode has encryption.ctx xattr
  ceph: make ceph_msdc_build_path use ref-walk
  ceph: add encrypted fname handling to ceph_mdsc_build_path
  ceph: make d_revalidate call fscrypt revalidator for encrypted
    dentries
  ceph: add support to readdir for encrypted filenames
  ceph: add fscrypt support to ceph_fill_trace
  ceph: create symlinks with encrypted and base64-encoded targets

 fs/ceph/Makefile             |   1 +
 fs/ceph/crypto.c             | 179 +++++++++++++++++++++++++++++
 fs/ceph/crypto.h             |  83 ++++++++++++++
 fs/ceph/dir.c                | 162 ++++++++++++++++++++------
 fs/ceph/file.c               |  56 +++++----
 fs/ceph/inode.c              | 213 ++++++++++++++++++++++++++++++-----
 fs/ceph/ioctl.c              |  25 ++++
 fs/ceph/mds_client.c         |  75 ++++++++----
 fs/ceph/mds_client.h         |   1 +
 fs/ceph/super.c              |  37 ++++++
 fs/ceph/super.h              |  19 +++-
 fs/ceph/xattr.c              |  32 ++++++
 fs/crypto/Kconfig            |   1 +
 fs/crypto/fname.c            | 139 ++++++++---------------
 fs/crypto/hooks.c            |   2 +-
 fs/crypto/keysetup.c         |   2 +-
 fs/crypto/policy.c           |  20 ++++
 fs/ext4/dir.c                |   2 +-
 fs/ext4/namei.c              |   7 +-
 fs/f2fs/dir.c                |   2 +-
 fs/inode.c                   |   1 +
 fs/ubifs/dir.c               |   2 +-
 include/linux/base64_fname.h |  11 ++
 include/linux/fscrypt.h      |  10 +-
 lib/Kconfig                  |   3 +
 lib/Makefile                 |   1 +
 lib/base64_fname.c           |  71 ++++++++++++
 27 files changed, 943 insertions(+), 214 deletions(-)
 create mode 100644 fs/ceph/crypto.c
 create mode 100644 fs/ceph/crypto.h
 create mode 100644 include/linux/base64_fname.h
 create mode 100644 lib/base64_fname.c

Comments

Eric Biggers Sept. 8, 2020, 5:54 a.m. UTC | #1
On Fri, Sep 04, 2020 at 12:05:19PM -0400, Jeff Layton wrote:
> This is a second posting of the ceph+fscrypt integration work that I've
> been experimenting with. The main change with this patch is that I've
> based this on top of Eric's fscrypt-pending set. That necessitated a
> change to allocate inodes much earlier than we have traditionally, prior
> to sending an RPC instead of waiting on the reply.

FWIW, if possible you should create a git tag or branch for your patchset.
While just the mailed patches work fine for *me* for this particular patchset,
other people may not be able to figure out what the patchset applies to.
(In particular, it depends on another patchset:
https://lkml.kernel.org/r/20200824061712.195654-1-ebiggers@kernel.org)

> Note that this just covers the crypto contexts and filenames. I've also
> added a patch to encrypt symlink contents as well, but it doesn't seem to
> be working correctly.

What about symlink encryption isn't working correctly?

- Eric
Jeff Layton Sept. 8, 2020, 12:09 p.m. UTC | #2
On Mon, 2020-09-07 at 22:54 -0700, Eric Biggers wrote:
> On Fri, Sep 04, 2020 at 12:05:19PM -0400, Jeff Layton wrote:
> > This is a second posting of the ceph+fscrypt integration work that I've
> > been experimenting with. The main change with this patch is that I've
> > based this on top of Eric's fscrypt-pending set. That necessitated a
> > change to allocate inodes much earlier than we have traditionally, prior
> > to sending an RPC instead of waiting on the reply.
> 
> FWIW, if possible you should create a git tag or branch for your patchset.
> While just the mailed patches work fine for *me* for this particular patchset,
> other people may not be able to figure out what the patchset applies to.
> (In particular, it depends on another patchset:
> https://lkml.kernel.org/r/20200824061712.195654-1-ebiggers@kernel.org)
> 

I've tagged this out as 'ceph-fscrypt-rfc.2' in my kernel.org tree (the
first posting is ceph-fscrypt-rfc.1).

Note that this also is layered on top of David Howell's fscache rework,
and the work I've done to adapt cephfs to that.

> > Note that this just covers the crypto contexts and filenames. I've also
> > added a patch to encrypt symlink contents as well, but it doesn't seem to
> > be working correctly.
> 
> What about symlink encryption isn't working correctly?

What I was seeing is that after unmounting and mounting, the symlink
contents would be gibberish when read by readlink(). I confirmed that
the same crypttext that came out of fscrypt_encrypt_symlink() was being
fed into fscrypt_get_symlink(), but the result from that came back as
gibberish.

I need to do a bit more troubleshooting, but I now wonder if it's due to
the context handling being wrong when dummy encryption is enabled. I'll
have a look at that soon.

Thanks for the review so far!