Re: [RFC PATCH v2 12/18] ceph: set S_ENCRYPTED bit if new inode has encryption.ctx xattr

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Sep 09, 2020 at 11:53:35AM -0400, Jeff Layton wrote:
> > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > > index 651148194316..c1c1fe2547f9 100644
> > > --- a/fs/ceph/inode.c
> > > +++ b/fs/ceph/inode.c
> > > @@ -964,6 +964,10 @@ int ceph_fill_inode(struct inode *inode, struct page *locked_page,
> > >  		ceph_forget_all_cached_acls(inode);
> > >  		ceph_security_invalidate_secctx(inode);
> > >  		xattr_blob = NULL;
> > > +		if ((inode->i_state & I_NEW) &&
> > > +		    (DUMMY_ENCRYPTION_ENABLED(mdsc->fsc) ||
> > > +		     ceph_inode_has_xattr(ci, CEPH_XATTR_NAME_ENCRYPTION_CONTEXT)))
> > > +			inode_set_flags(inode, S_ENCRYPTED, S_ENCRYPTED);
> > 
> > The check for DUMMY_ENCRYPTION_ENABLED() here is wrong and should be removed.
> > When the filesystem is mounted with test_dummy_encryption, there may be
> > unencrypted inodes already on-disk.  Those shouldn't get S_ENCRYPTED set.
> > test_dummy_encryption does add some special handling for unencrypted
> > directories, but that doesn't require S_ENCRYPTED to be set on them.
> > 
> 
> I've been working through your comments. Symlinks work now, as long as I
> use the fscrypt utility instead of test_dummy_encryption.
> 
> When I remove that condition above, then test_dummy_encryption no longer
> works.  I think I must be missing some context in how
> test_dummy_encryption is supposed to be used.
> 
> Suppose I mount a ceph filesystem with -o test_dummy_encryption. The
> root inode of the fs is instantiated by going through here, but it's not
> marked with S_ENCRYPTED because the root has no context.
> 
> How will descendant dentries end up encrypted if this one is not marked
> as such?

See fscrypt_prepare_new_inode() (or in current mainline, the logic in
__ext4_new_inode() and f2fs_may_encrypt()).  Encryption gets inherited if:

	IS_ENCRYPTED(dir) || fscrypt_get_dummy_context(dir->i_sb) != NULL

instead of just:

	IS_ENCRYPTED(dir)

Note that this specifically refers to encryption being *inherited*.
If !IS_ENCRYPTED(dir), then the directory itself remains unencrypted ---
that means that the filenames are in the directory aren't encrypted, even if
they name encrypted files/directories/symlinks.

- Eric