Skip to contentSkip to conversation

lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt

PATCH Applied Mohamad Raizudeen 2 participants 2 messages Oct 5

Subsystems (2)
Crypto API Crypto library
lore
Share & tools
lore

Mohamad Raizudeen

author
#0 lore raw

PATCH v3 1/1 lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt

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.

Because this function is EXPORT_SYMBOL()ed, this is an ABI robustness improvement. To ensure consistent and safe cleanup for any future callers, move the chacha_zeroize_state() call into __chacha20poly1305_decrypt() itself, so the helper cleans up after its own state just like __chacha20poly1305_encrypt() does.

Additionally, moved the src_len check to the callers so the state is not initialized on invalid inputs, matching the logic in chacha20poly1305_crypt_sg_inplace().

Suggested-by Ard Biesheuvel <[email protected]>
Signed-off-by Mohamad Raizudeen <[email protected]>

---
Changes in v3:
- Remove Fixes and Cc: stable tags, as this is an ABI robustness improvement rather than a fix.
- Move the src_len check to the callers so the state is not initialized on invalid inputs.
- Fix whitespace and ordering of the chacha_zeroize_state() to match chacha20poly1305_crypt_sg_inplace().

Link to v2: https://lore.kernel.org/all/asMfBGbckJAhCa45@kernel/T/

 lib/crypto/chacha20poly1305.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
lib/crypto/chacha20poly1305.c +7 -4
index ea42a28f4ff7..c3995ac8acb7 100644
@@ -137,9 +137,6 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 		__le64 lens[2];
 	} b;
 
-	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
-		return false;
-
 	chacha20_crypt(chacha_state, b.block0, pad0, sizeof(b.block0));
 	poly1305_init(&poly1305_state, b.block0);
 

@@ -162,6 +159,7 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 	if (likely(!ret))
 		chacha20_crypt(chacha_state, dst, src, dst_len);
 
+	chacha_zeroize_state(chacha_state);
 	memzero_explicit(&b, sizeof(b));
 
 	return !ret;

@@ -177,6 +175,9 @@ bool chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 	__le64 iv[2];
 	bool ret;
 
+	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
+		return false;
+
 	chacha_load_key(k, key);
 
 	iv[0] = 0;

@@ -186,7 +187,6 @@ bool chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 	ret = __chacha20poly1305_decrypt(dst, src, src_len, ad, ad_len,
 					 &chacha_state);
 
-	chacha_zeroize_state(&chacha_state);
 	memzero_explicit(iv, sizeof(iv));
 	memzero_explicit(k, sizeof(k));
 	return ret;

@@ -200,6 +200,9 @@ bool xchacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 {
 	struct chacha_state chacha_state;
 
+	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
+		return false;
+
 	xchacha_init(&chacha_state, key, nonce);
 	return __chacha20poly1305_decrypt(dst, src, src_len, ad, ad_len,
 					  &chacha_state);
Signature · 1 line
2.53.0
maintainer Applied
#1 lore raw
Quoted 19 lines · Mohamad Raizudeen

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.

Because this function is EXPORT_SYMBOL()ed, this is an ABI robustness improvement. To ensure consistent and safe cleanup for any future callers, move the chacha_zeroize_state() call into __chacha20poly1305_decrypt() itself, so the helper cleans up after its own state just like __chacha20poly1305_encrypt() does.

Additionally, moved the src_len check to the callers so the state is not initialized on invalid inputs, matching the logic in chacha20poly1305_crypt_sg_inplace().

Suggested-by Ard Biesheuvel <[email protected]>
Signed-off-by Mohamad Raizudeen <[email protected]>
Quoted 8 lines

---
Changes in v3:
- Remove Fixes and Cc: stable tags, as this is an ABI robustness improvement rather than a fix.
- Move the src_len check to the callers so the state is not initialized on invalid inputs.
- Fix whitespace and ordering of the chacha_zeroize_state() to match chacha20poly1305_crypt_sg_inplace().

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-next

- Eric