Re: [PATCH] crypto: chacha20poly1305 - Fix missing state zeroization in xchacha decrypt

[Mohamad Raizudeen]

On Sat, Oct 03, 2026 at 10:22:00PM +0200, Jason A. Donenfeld wrote:
> On Sat, Oct 03, 2026 at 11:38:26AM +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.
> > 
> > This leaves the derived chacha20 subkey on the stack after the function
> > returns. Fix this by storing the return value, calling
> > chacha_zeroize_state() and then returning the result, matching the
> > logic in `chacha20poly1305_decrypt`.
> > 
> > Fixes: ed20078b7e333 ("crypto: chacha20poly1305 - import construction and selftest from Zinc")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@xxxxxxxxx>
> > ---
> >  lib/crypto/chacha20poly1305.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/crypto/chacha20poly1305.c b/lib/crypto/chacha20poly1305.c
> > index ea42a28f4ff7..03b14d272520 100644
> > --- a/lib/crypto/chacha20poly1305.c
> > +++ b/lib/crypto/chacha20poly1305.c
> > @@ -199,10 +199,13 @@ bool xchacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
> >  			       const u8 key[at_least CHACHA20POLY1305_KEY_SIZE])
> >  {
> >  	struct chacha_state chacha_state;
> > +	bool ret;
> >  
> >  	xchacha_init(&chacha_state, key, nonce);
> > -	return __chacha20poly1305_decrypt(dst, src, src_len, ad, ad_len,
> > +	ret = __chacha20poly1305_decrypt(dst, src, src_len, ad, ad_len,
> >  					  &chacha_state);
> > +	chacha_zeroize_state(&chacha_state);
> > +	return ret;
> >  }
> >  EXPORT_SYMBOL(xchacha20poly1305_decrypt);
> 
> In my memory, the reason it's like this is because for WireGuard,
> xchapoly is just being used to encrypt a cookie value, which has no
> forward secrecy concerns at all. So it doesn't matter if it's left
> around on the stack.
> 
> Are there other use cases, though, where you think it might matter?
> 
> Jason

No, none that exist today. The only in-tree callers are the wireguard
cookie code and the kunit test and I agree the cookie has no forward
secrecy concerns. My only thought is that the function is
EXPORT_SYMBOL()ed, so a future caller with a long term key would leave
the derived subkey on the stack and since chacha20poly1305_decrypt()
already wipes the state, having the xchacha variant do the same seemed
like safer default.

Also, Eric requested a v2 that moves the zeroization into the helper
function for consistency with the encrypt path, so I will be sending
that out shortly.

Thanks,
Mohamad Raizudeen