Re: [PATCH v5 00/18] btrfs: add fscrypt integration

From: Eric Biggers <ebiggers@kernel.org>
To: Neal Gompa <ngompa13@gmail.com>
Cc: Paul Crowley <paulcrowley@google.com>,
	Sweet Tea Dorminy <sweettea-kernel@dorminy.me>,
	"Theodore Y. Ts'o" <tytso@mit.edu>,
	Jaegeuk Kim <jaegeuk@kernel.org>, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	linux-fscrypt@vger.kernel.org, linux-btrfs@vger.kernel.org,
	kernel-team@meta.com
Subject: Re: [PATCH v5 00/18] btrfs: add fscrypt integration
Date: Wed, 16 Nov 2022 12:35:55 -0800	[thread overview]
Message-ID: <Y3VJq6YDqrO7w70F@sol.localdomain> (raw)
In-Reply-To: <CAEg-Je96DB34Dj9_-DtgEHaiLf+omXoZTnK71Wv2f6bcWjE0yQ@mail.gmail.com>

On Wed, Nov 16, 2022 at 03:08:26PM -0500, Neal Gompa wrote:
> On Thu, Nov 3, 2022 at 3:54 PM Paul Crowley <paulcrowley@google.com> wrote:
> >
> > Thank you for creating this! I'm told the design document [1] no
> > longer reflects the current proposal in these patches. If that's so I
> > think it's worth bringing the design document up to date so we can
> > review the cryptography. Thanks!
> >
> > [1] https://docs.google.com/document/d/1iNnrqyZqJ2I5nfWKt7cd1T9xwU0iHhjhk9ALQW3XuII/edit
> 
> So this might be my ignorance here, but when I look at the patch set,
> I don't really see any significant mathematics or cryptographic work
> going on here. This seems to be primarily just interacting with the
> fscrypt stuff that exists in the kernel already.
> 
> Could you please clarify what you mean here?

There absolutely is significant cryptographic work going on here.  There needs
to be a new way to choose keys and IVs for file contents blocks, as the existing
ways are not appropriate for btrfs.  That is the main difficulty we are having.
One idea is the one which this patchset is intended to implement.  Other ideas
that have been brought up involve deriving per-extent keys, using per-block IVs,
or using authenticated encryption (or some combination of these).  These ideas
would be better cryptographically then the one that this patchset actually
implements, so it needs to be properly documented why they've been ruled out.
(Or maybe they haven't really been ruled out -- I'm not sure they have.)

And as I've mentioned, if we do go with the current proposal, which results in
some IV reuse, it needs to be decided whether we should try to ameliorate that
by hashing part of the IV with a secret key, like IV_INO_LBLK_32 does.

Another area where new cryptographic design is needed is the encryption of the
fsverity metadata.  ext4 and f2fs get encryption of the fsverity metadata "for
free" since they store it past EOF, but btrfs doesn't.

Anyway, I tried to get Paul's feedback on this patchset, but he
(understandingly) didn't want to dig through random mailing list discussions,
which don't really have all the information anyway.

I think updating the design document to fully reflect the current proposal and
the detailed reasoning behind it would be super helpful to get everyone on the
right page and to make sure the right design is being chosen.

- Eric