Re: [PATCH v2] lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt

[Eric Biggers <ebiggers@kernel.org>]

On Sun, Oct 04, 2026 at 10:04:13AM +0530, Mohamad Raizudeen wrote:
> The `__chacha20poly1305_decrypt` function does not zeroize the chacha
> state, unlike its encrypt counterpart. The regular
> `chacha20poly1305_decrypt` function handles this by manually calling
> chacha_zeroize_state(). However, `xchacha20poly1305_decrypt` returns
> the result directly without clearing the state, leaving the derived
> chacha20 subkey on the stack.
> 
> Fix this by moving the chacha_zeroize_state() call into
> __chacha20poly1305_decrypt() itself, so the helper cleans up after
> itself just like __chacha20poly1305_encrypt does. This ensures all
> callers are secure without needing manual cleanup.
> 
> Fixes: ed20078b7e333 ("crypto: chacha20poly1305 - import construction and selftest from Zinc")
> Cc: stable@vger.kernel.org
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>

Sorry, to nitpick this a bit more:

Can you reword this to clarify that this is an ABI robustness
improvement rather than a fix, since currently the single caller of
xchacha20poly1305_decrypt() in wg_cookie_message_consume() doesn't
require forward secrecy, as mentioned by Jason.  And maybe remove Fixes
and 'Cc stable'.  Otherwise this commit will unnecessarily trigger all
the stable backport and CVE spam.

> diff --git a/lib/crypto/chacha20poly1305.c b/lib/crypto/chacha20poly1305.c
> index ea42a28f4ff7..80904321458b 100644
> --- a/lib/crypto/chacha20poly1305.c
> +++ b/lib/crypto/chacha20poly1305.c
> @@ -137,8 +137,10 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
>  		__le64 lens[2];
>  	} b;
>  
> -	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
> +	if (unlikely(src_len < POLY1305_DIGEST_SIZE)) {
> +		chacha_zeroize_state(chacha_state);
>  		return false;
> +	}

How about we move this length check into the two callers before they
write anything to the state at all?  Then the state would not need to be
zeroized if the length check fails.  Note that
chacha20poly1305_decrypt_sg_inplace() already does it this way.

>  	memzero_explicit(&b, sizeof(b));
>  
> +	chacha_zeroize_state(chacha_state);
>  	return !ret;

Nit: Use the same order and whitespace as
chacha20poly1305_crypt_sg_inplace():

	chacha_zeroize_state(chacha_state);
	memzero_explicit(&b, sizeof(b));

	return !ret;

- Eric