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. 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... - Eric
