Re: [PATCH v2 07/11] fscrypt: move all the shared mode key setup deeper

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Apr 10, 2023 at 03:40:00PM -0400, Sweet Tea Dorminy wrote:
> +static const u8 FSCRYPT_POLICY_FLAGS_KEY_MASK =
> +	(FSCRYPT_POLICY_FLAG_DIRECT_KEY
> +	 | FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64
> +	 | FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32);

A comment describing the meaning of the above constant would be helpful.

> +static size_t fill_hkdf_info(const struct fscrypt_info *ci, u8 *hkdf_info)

Maybe call this fill_hkdf_info_for_mode_key() to avoid ambiguity with other uses
of HKDF?  Also, maybe add an explicit array size to hkdf_info?  E.g.
hkdf_info[MAX_MODE_KEY_HKDF_INFO_SIZE]

> +static u8 hkdf_context_for_policy(const union fscrypt_policy *policy)
> +{
> +	switch (fscrypt_policy_flags(policy) & FSCRYPT_POLICY_FLAGS_KEY_MASK) {
> +		case FSCRYPT_POLICY_FLAG_DIRECT_KEY:
> +			return HKDF_CONTEXT_DIRECT_KEY;
> +		case FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64:
> +			return HKDF_CONTEXT_IV_INO_LBLK_64_KEY;
> +		case FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32:
> +			return HKDF_CONTEXT_IV_INO_LBLK_32_KEY;
> +		default:
> +			return 0;
> +	}
> +}

There's an extra level of indentation above.

Also, more importantly, since fill_hkdf_info() checks the policy flags anyway,
maybe just handle the HKDF context bytes directly in there?  E.g.:

	if (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY) {
		hkdf_info[0] = HKDF_CONTEXT_DIRECT_KEY;
		return 1;
	}
	if (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32)
		hkdf_info[0] = HKDF_CONTEXT_IV_INO_LBLK_32_KEY;
	else
		hkdf_info[0] = HKDF_CONTEXT_IV_INO_LBLK_64_KEY;
	memcpy(&hkdf_info[1], &sb->s_uuid, sizeof(sb->s_uuid));
	return 1 + sizeof(sb->s_uuid);

- Eric