linux-ext4 - Re: [PATCH 1/4] generic: add utilities for testing filesystem encryption

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161121210849.GI31101@dastard>
Date:   Tue, 22 Nov 2016 08:08:49 +1100
From:   Dave Chinner <david@...morbit.com>
To:     Eric Biggers <ebiggers@...gle.com>
Cc:     fstests@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-f2fs@...r.kernel.org, "Theodore Y . Ts'o" <tytso@....edu>,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        Richard Weinberger <richard@....at>,
        David Gstir <david@...ma-star.at>
Subject: Re: [PATCH 1/4] generic: add utilities for testing filesystem
 encryption

On Mon, Nov 21, 2016 at 10:40:50AM -0800, Eric Biggers wrote:
> Hi Dave, thanks for reviewing.
> 
> On Mon, Nov 21, 2016 at 08:33:13AM +1100, Dave Chinner wrote:
> > > +
> > > +. ./common/rc
> > 
> > Tests will already have included common/rc before this file, so we
> > do not source it here.
> ...
> > These go in the tests, not here.
> ...
> > _requires_real_encryption()
> > 
> > In each test.
> ...
> > And this should all be in a _requires_encryption() function.
> > 
> 
> I'll do all of these.  Of course the intent was to avoid duplicating code in
> each test, but I will use the more verbose style if that's preferred.  I assume
> you'd also prefer explicitly formatting and mounting the scratch device in each
> test even though _require_encryption would already have to do that?

Yes, because in future _require_encryption may change to no require
touching the scratch device. Also, checking for encryption may
create a filesystem with different configuration than the one we
want to test. So it's better to be consistent across all tests and
require the scratch_mkfs call in each test so we know the exact
state of the filesystem before the test starts....

> > Ok, can we get this added to xfs_io rather than a stand-alone
> > fstests helper? There are three clear commands here:
> > 
> > 	{"gen_key", gen_key},
> > 	{"rm_key", rm_key},
> > 	{"set_policy", set_policy},
> > 
> > So it should plug straight into the xfs_io command parsing
> > infrastructure without much change at all.
> 
> I see that xfs_io is part of xfsprogs, not xfstests.  Does it make sense to add
> filesystem encryption commands to xfs_io even though XFS doesn't support them
> yet?  Currently only ext4 and f2fs support filesystem encryption via this common
> API.  (ubifs support has been proposed too.)

Yes, because it is in the plan to support the generic encryption in
XFS, too. It'll just take us a little while to get to it, but it
won't hurt to support these operations ahead of that time...

> If we do go that route then it should be considered only adding
> "set_policy" and "get_policy" commands, and for "gen_key" and
> "rm_key" instead using shell wrappers around 'keyctl' instead.
> gen_key and rm_key don't touch the filesystem at all; they only
> work with the keyring.  It's possible to use 'keyctl padd' to add
> a fscrypt key, though it's not completely trivial because you'd
> have to use 'echo -e' to generate the C structure 'struct
> fscrypt_key' with mode = 0, raw = actual key in binary, size = 64.

Sounds fine to me.

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html