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.
