Message-ID: <20180105220149.GD44929@gmail.com>
Date:   Fri, 5 Jan 2018 14:01:49 -0800
From:   Eric Biggers <ebiggers3@...il.com>
To:     Jaegeuk Kim <jaegeuk@...nel.org>
Cc:     linux-fscrypt@...r.kernel.org, "Theodore Y . Ts'o" <tytso@....edu>,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-mtd@...ts.infradead.org, linux-fsdevel@...r.kernel.org,
        Eric Biggers <ebiggers@...gle.com>
Subject: Re: [PATCH v2 05/11] fscrypt: split fscrypt_dummy_context_enabled()
 into supp/notsupp versions

On Fri, Jan 05, 2018 at 01:36:08PM -0800, Jaegeuk Kim wrote:
> On 01/05, Eric Biggers wrote:
> > On Fri, Jan 05, 2018 at 12:40:09PM -0800, Jaegeuk Kim wrote:
> > > On 01/05, Eric Biggers wrote:
> > > > From: Eric Biggers <ebiggers@...gle.com>
> > > > 
> > > > fscrypt_dummy_context_enabled() accesses ->s_cop, which now is only set
> > > > when the filesystem is built with encryption support.  This didn't
> > > > actually matter because no filesystems called it.  However, it will
> > > > start being used soon, so fix it by moving it from fscrypt.h to
> > > > fscrypt_supp.h and stubbing it out in fscrypt_notsupp.h.
> > > 
> > > Ted, do we have a chance to get rid of this dummy_context? If there exists
> > > backward compatibility issue, please never mind tho.
> > > 
> > 
> > It's used to implement the test_dummy_encryption mount option for ext4, which is
> > used by the 'ext4/encrypt' config for gce-xfstests.  Its purpose is to cause all
> > new files (directories, regular files, and symlinks) to be automatically
> > encrypted with a default key, so that the encrypted I/O paths are tested more
> > thoroughly than by just running the 'encrypt' group tests.
> > 
> > There are no backward compatibility concerns with changing or removing the
> > test_dummy_encryption mount option; we just don't have a better solution yet.
> > 
> > Ideally, instead of using test_dummy_encryption we would encrypt the root
> > directory of the filesystem immediately after it is formatted.  However, that
> > doesn't work for ext4 because the lost+found directory has to be located in the
> > root directory, and must be unencrypted, and the lost+found directory entry must
> > be unencrypted.
> > 
> > So I think getting rid of test_dummy_encryption depends on a solution to the
> > lost+found problem.
> 
> Thank you for the explanation. Actually, I've got used to encrypt the root dir
> in f2fs for such the testing purpose, since it's indeed empty. If it's the only
> matter of lost_found in ext4, how about setting the encryption bit and dummy
> context in root inode by mke2fs or tune2fs?
> 

You can't mark a nonempty directory as encrypted.  First, you can't have an
unencrypted file *name* in an encrypted directory, because then all the
operations on that filename won't work, because the filesystem will think it is
encrypted.  Second, you cannot have an unencrypted file in an encrypted
directory because that violates the constraint that all files in an encrypted
directory are encrypted -- and by design that is verified on ->lookup().

So to get it to actually work in ext4 we'd need to add extra special-case logic
around the lost+found directory -- probably allowing both the directory entry
and directory itself to be unencrypted, despite existing in an encrypted
directory.  And I think that solution would be more complicated than the
test_dummy_encryption mount option we have now.

It may be worth reconsidering if people start asking for the ability to encrypt
the root directory for other reasons, though.  (Although, that use case is
largely redundant with dm-crypt.)

Eric

Powered by blists - more mailing lists