From 98215b60f60649639b32f19fd37f8d986d67502a Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 13:05:23 +0100 Subject: [PATCH] Address Copilot concerns --- tests/test_ciphers.py | 33 ++++++++++++++++++++++++++------- wolfcrypt/ciphers.py | 36 +++++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/tests/test_ciphers.py b/tests/test_ciphers.py index fe2894b..0f759ad 100644 --- a/tests/test_ciphers.py +++ b/tests/test_ciphers.py @@ -1032,17 +1032,21 @@ if _lib.CHACHA_ENABLED: def test_chacha_decrypt_does_not_reset_encrypt_stream(): """ Interleaving decrypt() between two encrypt() calls on the same - ChaCha instance must not reset the encryption stream counter. - A previous bug in _set_key re-keyed both contexts whenever either - was allocated, so the first decrypt() (which lazily allocates the - decryption context) silently rewound the encryption stream to - counter 0, producing the wrong ciphertext on subsequent encrypts. + ChaCha instance must not reset the encryption stream counter, and + symmetrically interleaving encrypt() between two decrypt() calls + must not reset the decryption stream counter. A previous bug in + _set_key re-keyed both contexts whenever either was allocated, so + the first call to the other direction (which lazily allocates the + opposite context) silently rewound the existing stream to + counter 0, producing the wrong ciphertext/plaintext on subsequent + calls. """ key = b"\x00" * 32 nonce = b"\x00" * 12 block1 = b"A" * 64 block2 = b"B" * 64 + # --- encrypt -> decrypt -> encrypt: the lazy _dec must not wipe _enc. baseline = ChaCha(key) baseline.set_iv(nonce) expected_ct1 = baseline.encrypt(block1) @@ -1053,12 +1057,27 @@ if _lib.CHACHA_ENABLED: chacha.set_iv(nonce) ct1 = chacha.encrypt(block1) assert ct1 == expected_ct1 - # First decrypt() lazily allocates the decryption context and must - # not disturb the encryption stream state. chacha.decrypt(b"\x00" * 16) ct2 = chacha.encrypt(block2) assert ct2 == expected_ct2 + # --- decrypt -> encrypt -> decrypt: the lazy _enc must not wipe _dec. + # Pre-compute the two ciphertexts that would decrypt back to + # block1, block2 in stream order. + producer = ChaCha(key) + producer.set_iv(nonce) + ct_a = producer.encrypt(block1) + ct_b = producer.encrypt(block2) + + chacha = ChaCha(key) + chacha.set_iv(nonce) + pt1 = chacha.decrypt(ct_a) + assert pt1 == block1 + # encrypt() now lazily allocates _enc; it must not reset _dec. + chacha.encrypt(b"\x00" * 16) + pt2 = chacha.decrypt(ct_b) + assert pt2 == block2 + def test_chacha_set_iv_resets_both_directions(): """ set_iv() is documented to reset the stream, and the existing diff --git a/wolfcrypt/ciphers.py b/wolfcrypt/ciphers.py index 11ede32..c613793 100644 --- a/wolfcrypt/ciphers.py +++ b/wolfcrypt/ciphers.py @@ -299,8 +299,9 @@ if _lib.AES_SIV_ENABLED: Encrypt plaintext data using the nonce provided. The associated data is not encrypted but is included in the authentication tag. - Associated data may be provided as single str or bytes, or as a - list of str or bytes in case of multiple blocks. + Associated data may be provided as a single str, bytes, + bytearray, or memoryview, or as a list of any of those in case + of multiple blocks. Returns a tuple of the IV and ciphertext. """ @@ -325,8 +326,9 @@ if _lib.AES_SIV_ENABLED: Decrypt the ciphertext using the nonce and SIV provided. The integrity of the associated data is checked. - Associated data may be provided as single str or bytes, or as a - list of str or bytes in case of multiple blocks. + Associated data may be provided as a single str, bytes, + bytearray, or memoryview, or as a list of any of those in case + of multiple blocks. Returns the decrypted plaintext. """ @@ -354,8 +356,9 @@ if _lib.AES_SIV_ENABLED: """ Prepare associated data for sending to C library. - Associated data may be provided as single str or bytes, or as a - list of str or bytes in case of multiple blocks. + Associated data may be provided as a single str, bytes, + bytearray, or memoryview, or as a list of any of those in case + of multiple blocks. The result is a tuple of the list of cffi cdata pointers to AesSivAssoc structures, as well as the converted associated @@ -527,17 +530,20 @@ if _lib.CHACHA_ENABLED: self._IV_nonce = b"" self._IV_counter = 0 + # Sentinel for "rekey both contexts" used by set_iv. Must not + # collide with _ENCRYPTION (0) or _DECRYPTION (1). + _REKEY_BOTH = -1 + def _set_key(self, direction): if self._key is None: return -1 - # direction 0 (used by set_iv) re-keys whichever contexts are - # already allocated, since changing the IV must reset both - # encrypt and decrypt streams. Direction _ENCRYPTION / - # _DECRYPTION only touches the matching context so that lazy - # allocation from encrypt()/decrypt() does not wipe the other - # direction's stream state. - do_enc = self._enc and direction in (0, _ENCRYPTION) - do_dec = self._dec and direction in (0, _DECRYPTION) + # _REKEY_BOTH re-keys whichever contexts are already allocated, + # since changing the IV must reset both encrypt and decrypt + # streams. _ENCRYPTION / _DECRYPTION only touch the matching + # context so that lazy allocation from encrypt()/decrypt() does + # not wipe the other direction's stream state. + do_enc = self._enc and direction in (self._REKEY_BOTH, _ENCRYPTION) + do_dec = self._dec and direction in (self._REKEY_BOTH, _DECRYPTION) if do_enc: ret = _lib.wc_Chacha_SetKey(self._enc, self._key, len(self._key)) if ret == 0: @@ -568,7 +574,7 @@ if _lib.CHACHA_ENABLED: raise ValueError("nonce must be %d bytes, got %d" % (self._NONCE_SIZE, len(self._IV_nonce))) self._IV_counter = counter - ret = self._set_key(0) + ret = self._set_key(self._REKEY_BOTH) if ret < 0: raise WolfCryptApiError("ChaCha set_iv error", ret)