linux-kernel - Re: [PATCH] ext4: Fix no-key deletion for encrypt+casefold

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: <YK0z9US1ek615F8Z@sol.localdomain>
Date:   Tue, 25 May 2021 10:29:25 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Daniel Rosenberg <drosen@...gle.com>
Cc:     "Theodore Y . Ts'o" <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org,
        Gabriel Krisman Bertazi <krisman@...labora.com>,
        kernel-team@...roid.com
Subject: Re: [PATCH] ext4: Fix no-key deletion for encrypt+casefold

On Sat, May 22, 2021 at 12:41:32AM +0000, Daniel Rosenberg wrote:
> commit 471fbbea7ff7 ("ext4: handle casefolding with encryption") is
> missing a few checks for the encryption key which are needed to
> support deleting enrypted casefolded files when the key is not
> present.
> 
> Note from ebiggers:
> (These checks for the encryption key are still racy since they happen
> too late, but apparently they worked well enough...)
> 
> This bug made it impossible to delete encrypted+casefolded directories
> without the encryption key, due to errors like:
> 
>     W         : EXT4-fs warning (device vdc): __ext4fs_dirhash:270: inode #49202: comm Binder:378_4: Siphash requires key
> 
> Repro steps in kvm-xfstests test appliance:
>       mkfs.ext4 -F -E encoding=utf8 -O encrypt /dev/vdc
>       mount /vdc
>       mkdir /vdc/dir
>       chattr +F /vdc/dir
>       keyid=$(head -c 64 /dev/zero | xfs_io -c add_enckey /vdc | awk '{print $NF}')
>       xfs_io -c "set_encpolicy $keyid" /vdc/dir
>       for i in `seq 1 100`; do
>           mkdir /vdc/dir/$i
>       done
>       xfs_io -c "rm_enckey $keyid" /vdc
>       rm -rf /vdc/dir # fails with the bug

Looks fine, but can you please turn this reproducer into an xfstest?

I'm also wondering if you've done any investigation into fixing ext4 to handle
filenames properly like f2fs does, so that the above-mentioned race condition is
eliminated.  In particular, we should decide whether the user-supplied filename
is a no-key name, and whether it needs casefolding or not, just once -- rather
than separately for each directory entry compared in ext4_match().

- Eric