Received-SPF: pass (google.com: domain of linux-kernel+bounces-55836-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 6 Feb 2024 21:09:18 -0500
From: Kent Overstreet <kent.overstreet@linux.dev>
To: Eric Biggers <ebiggers@kernel.org>
Cc: brauner@kernel.org, linux-fsdevel@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/7] filesystem visibililty ioctls
Message-ID: <uaap4vjc7bsbavyetc2nsxvdaepw5cem35pfqvu2uttmaczdpk@grzgraa473zv>
References: <20240206201858.952303-1-kent.overstreet@linux.dev>
 <20240207014729.GC35324@sol.localdomain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240207014729.GC35324@sol.localdomain>

On Tue, Feb 06, 2024 at 05:47:29PM -0800, Eric Biggers wrote:
> On Tue, Feb 06, 2024 at 03:18:48PM -0500, Kent Overstreet wrote:
> > 
> > Darrick also noticed that fscrypt (!) is using sb->s_uuid, which looks
> > busted - they want to be using the "this can never change" UUID, but
> > that is not an item for this patchset.
> > 
> 
> fscrypt only uses sb->s_uuid if FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64 or
> FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32 is being used in the encryption policy.
> These flags are only supported by ext4 and f2fs, and they are only useful when
> the file contents encryption is being done with inline encryption hardware that
> only allows 64-bits or less of the initialization vector to be specified and
> that has poor performance when switching keys.  This hardware is currently only
> known to be present on mobile and embedded systems that use eMMC or UFS storage.
> 
> Note that these settings assume the inode numbers are stable as well as the
> UUID.  So, when they are in use, filesystem shrinking is prohibited as well as
> changing the filesystem UUID.  (In ext4, both operations are forbidden using the
> stable_inodes feature flag.  f2fs doesn't support either operation regardless.)
> 
> These restrictions are unfortunate, but so far they haven't been a problem for
> the only known use case for these non-default settings.
> 
> In the case of s_uuid, for both ext4 and f2fs it's true that we could have used
> s_encrypt_pw_salt instead, or added a new general-purpose internal UUID field
> and used that.  Maybe we even should have, considering the precedent of ext4's
> metadata_csum migrating away from using the UUID to its own internal seed.  I do
> worry that relying on an internal UUID for these settings would make it easier
> for people to create insecure setups where they're using the same fscrypt key on
> multiple filesystems with the same internal UUID.  With the external UUID, such
> misconfigurations are obvious and will be noticed and fixed.  With the internal
> UUID, such vulnerabilities would not be noticed, as things will "just work".
> Which is better?  It's not entirely clear to me.  We do encourage the use of
> different fscrypt keys on different filesystems anyway, but this isn't required.

*nod* nonce reuse is a thorny issue - that makes using the changeable,
external UUID a bit more of a feature than a bug.

> Of course, even if the usability improvement outweighs that concern, switching
> these already-existing encryption settings over to use an internal UUID can't be
> done trivially; it would have to be controlled by a new filesystem feature flag.
> We probably shouldn't bother unless/until there's a clear use case for it.
> 
> If anyone does have any new use case for these weird and non-default encryption
> settings (and I hope you don't!), I'd be interested to hear...

I just wanted to make sure I wasn't breaking fscrypt by baking-in that
s_uuid is the user facing one - thanks for the info.

Can we get this documented in a code comment somewhere visible? It was
definitely a "wtf" moment when Darrick and I saw it, I want to make sure
people know what's going on later.