On Mon, Apr 24, 2017 at 10:00:09AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
>
> When accessing an encrypted directory without the key, userspace must
> operate on filenames derived from the ciphertext names, which contain
> arbitrary bytes. Since we must support filenames as long as NAME_MAX,
> we can't always just base64-encode the ciphertext, since that may make
> it too long. Currently, this is solved by presenting long names in an
> abbreviated form containing any needed filesystem-specific hashes (e.g.
> to identify a directory block), then the last 16 bytes of ciphertext.
> This needs to be sufficient to identify the actual name on lookup.
>
> However, there is a bug. It seems to have been assumed that due to the
> use of a CBC (ciphertext block chaining)-based encryption mode, the last
> 16 bytes (i.e. the AES block size) of ciphertext would depend on the
> full plaintext, preventing collisions. However, we actually use CBC
> with ciphertext stealing (CTS), which handles the last two blocks
> specially, causing them to appear "flipped". Thus, it's actually the
> second-to-last block which depends on the full plaintext.
>
> This caused long filenames that differ only near the end of their
> plaintexts to, when observed without the key, point to the wrong inode
> and be undeletable. For example, with ext4:
>
> # echo pass | e4crypt add_key -p 16 edir/
> # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
> # find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
> 100000
> # sync
> # echo 3 > /proc/sys/vm/drop_caches
> # keyctl new_session
> # find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
> 2004
> # rm -rf edir/
> rm: cannot remove 'edir/_A7nNFi3rhkEQlJ6P,hdzluhODKOeWx5V': Structure needs cleaning
> ...
>
> To fix this, when presenting long encrypted filenames, encode the
> second-to-last block of ciphertext rather than the last 16 bytes.
>
> Although it would be nice to solve this without depending on a specific
> encryption mode, that would mean doing a cryptographic hash like SHA-256
> which would be much less efficient. This way is sufficient for now, and
> it's still compatible with encryption modes like HEH which are strong
> pseudorandom permutations. Also, changing the presented names is still
> allowed at any time because they are only provided to allow applications
> to do things like delete encrypted directories. They're not designed to
> be used to persistently identify files --- which would be hard to do
> anyway, given that they're encrypted after all.
>
> For ease of backports, this patch only makes the minimal fix to both
> ext4 and f2fs. It leaves ubifs as-is, since ubifs doesn't compare the
> ciphertext block yet. Follow-on patches will clean things up properly
> and make the filesystems use a shared helper function.
>
> Fixes: 5de0b4d0cd15 ("ext4 crypto: simplify and speed up filename encryption")
> Reported-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
Thanks, applied.
- Ted