Re: [PATCH v6 6/8] fscrypt: move all the shared mode key setup deeper

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Aug 08, 2023 at 01:08:06PM -0400, Sweet Tea Dorminy wrote:
> Currently, fscrypt_setup_v2_file_key() has a set of ifs which encode
> various information about how to set up a new mode key if necessary for
> a shared-key policy (DIRECT or IV_INO_LBLK_*). This is somewhat awkward
> -- this information is only needed at the point that we need to setup a
> new key, which is not the common case; the setup details are recorded as
> function parameters relatively far from where they're actually used; and
> at the point we use the parameters, we can derive the information
> equally well.
> 
> So this moves mode and policy checking as deep into the callstack as
> possible. mk_prepared_key_for_mode_policy() deals with the array lookup
> within a master key. And fill_hkdf_info_for mode_key() deals with
> filling in the hkdf info as necessary for a particular policy. These
> seem a little clearer in broad strokes, emphasizing the similarities
> between the policies, but it does spread out the information on how the
> key is derived for a particular policy more.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/keysetup.c | 131 +++++++++++++++++++++++++++++--------------
>  1 file changed, 88 insertions(+), 43 deletions(-)

Looking at this again, I think this patch makes things worse, not better.  The
diffstat is significantly positive, and it splits the handling of each policy
type into several more places, which makes it harder to understand how each one
works.  Also problematic is how the new code makes it look like HKDF context 0
is potentially used, which could confuse people trying to review the crypto.

Do any of your later patches depend on this patch?

- Eric