linux-ext4 - Re: [2/6] fscrypt: avoid collisions when presenting long encrypted filenames

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170430061947.hawb6tsjg3tnxeue@thunk.org>
Date:   Sun, 30 Apr 2017 02:19:47 -0400
From:   Theodore Ts'o <tytso@....edu>
To:     Eric Biggers <ebiggers3@...il.com>
Cc:     linux-fscrypt@...r.kernel.org, Jaegeuk Kim <jaegeuk@...nel.org>,
        linux-f2fs-devel@...ts.sourceforge.net, linux-ext4@...r.kernel.org,
        linux-mtd@...ts.infradead.org,
        Gwendal Grignou <gwendal@...omium.org>, hashimoto@...omium.org,
        kinaba@...omium.org, Eric Biggers <ebiggers@...gle.com>,
        stable@...r.kernel.org
Subject: Re: [2/6] fscrypt: avoid collisions when presenting long encrypted
 filenames

On Mon, Apr 24, 2017 at 10:00:09AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@...gle.com>
> 
> 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@...omium.org>
> Cc: stable@...r.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@...gle.com>

Thanks, applied.

						- Ted