From 6c1c1b76bb8fa91180820247d2eee4ec613fe626 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 11:47:56 +0100 Subject: [PATCH 1/9] Fix AesSiv silently mangling associated data AesSiv._prepare_associated_data only checked for str and bytes when deciding whether the input was a single associated-data block, so bytearray and memoryview fell through to the "list of blocks" branch. Iterating those types yields integers, which t2b() then turned into ASCII decimal byte-strings (b'16', b'17', ...), producing many bogus blocks instead of one. The C SIV computation succeeded over wrong associated data, so encryption/decryption silently produced an incorrect tag rather than raising. This broke interoperability between callers passing the same content as different buffer types. Match the set of types accepted by t2b() by including bytearray and memoryview in the isinstance check. Add a parametrized test that reuses the OpenSSL KAT vectors with bytes, bytearray, and memoryview wrappers; a round-trip test would not have caught this since both sides mangle identically. F-1981 --- tests/test_ciphers.py | 29 +++++++++++++++++++++++++++++ wolfcrypt/ciphers.py | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/test_ciphers.py b/tests/test_ciphers.py index c8b1e25..4615b07 100644 --- a/tests/test_ciphers.py +++ b/tests/test_ciphers.py @@ -938,6 +938,35 @@ def test_aessiv_decrypt_kat_openssl(): assert plaintext == TEST_VECTOR_PLAINTEXT_OPENSSL +@pytest.mark.skipif(not _lib.AES_SIV_ENABLED, reason="AES-SIV not enabled") +@pytest.mark.parametrize("wrap", [bytes, bytearray, memoryview], + ids=["bytes", "bytearray", "memoryview"]) +def test_aessiv_associated_data_accepts_buffer_types(wrap): + """ + Single-block associated_data passed as bytes, bytearray, or memoryview + must all produce the same SIV/ciphertext as the OpenSSL KAT. A previous + bug treated bytearray/memoryview as a sequence of int blocks, producing + a different (incorrect) tag without raising. + """ + aessiv = AesSiv(TEST_VECTOR_KEY_OPENSSL) + associated_data = wrap(TEST_VECTOR_ASSOCIATED_DATA_OPENSSL) + siv, ciphertext = aessiv.encrypt( + associated_data, + TEST_VECTOR_NONCE_OPENSSL, + TEST_VECTOR_PLAINTEXT_OPENSSL + ) + assert siv == TEST_VECTOR_SIV_OPENSSL + assert ciphertext == TEST_VECTOR_CIPHERTEXT_OPENSSL + + plaintext = aessiv.decrypt( + wrap(TEST_VECTOR_ASSOCIATED_DATA_OPENSSL), + TEST_VECTOR_NONCE_OPENSSL, + TEST_VECTOR_SIV_OPENSSL, + TEST_VECTOR_CIPHERTEXT_OPENSSL + ) + assert plaintext == TEST_VECTOR_PLAINTEXT_OPENSSL + + if _lib.DES3_ENABLED: def test_des3_rejects_mode_ctr(): key = b"\x01\x23\x45\x67\x89\xab\xcd\xef" * 3 diff --git a/wolfcrypt/ciphers.py b/wolfcrypt/ciphers.py index b3899b5..af8a7aa 100644 --- a/wolfcrypt/ciphers.py +++ b/wolfcrypt/ciphers.py @@ -363,7 +363,7 @@ def _prepare_associated_data(associated_data): C function has been called, in order to make sure that the memory is not freed by the FFI garbage collector before the data is read. """ - if isinstance(associated_data, str) or isinstance(associated_data, bytes): + if isinstance(associated_data, (str, bytes, bytearray, memoryview)): # A single block is provided. # Make sure we have bytes. associated_data = t2b(associated_data) From ce931745700bdc36bcde098726390f674ad54ce6 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 11:58:36 +0100 Subject: [PATCH 2/9] Fix Random.__init__ AttributeError on builds without ML-KEM/ML-DSA Random.__init__ used _lib.INVALID_DEVID as a default parameter value, but INVALID_DEVID is only declared in the CFFI cdef inside `if features["ML_KEM"] or features["ML_DSA"]`. Since default parameters are evaluated at module import time, any wolfSSL build that lacks both ML-KEM and ML-DSA (e.g. via USE_LOCAL_WOLFSSL pointing at a custom build) raises AttributeError on `import wolfcrypt.random`, cascading into wolfcrypt.ciphers and effectively breaking the entire library. Hardcode the default to -2, matching the convention already used by _Hmac._init and AesGcmStream.__init__ (which call wc_HmacInit / wc_AesInit with -2 directly rather than referencing _lib.INVALID_DEVID). Add a build-no-pqc GitHub Actions job that builds wolfSSL with --disable-kyber --disable-dilithium, installs wolfcrypt-py against it via USE_LOCAL_WOLFSSL, and runs an import-smoke step plus the full pytest suite. The smoke step is the explicit regression guard: it imports wolfcrypt.random and instantiates Random(), which would fail at import time if the default ever again references a CFFI symbol that is only conditionally declared. F-2659 --- .github/workflows/python-app.yml | 52 ++++++++++++++++++++++++++++++++ wolfcrypt/random.py | 2 +- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 22bb2c4..4209e4a 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -38,3 +38,55 @@ jobs: - name: Test with tox run: | tox + + build-no-pqc: + # Regression coverage for issue #2659: INVALID_DEVID is only declared + # in the CFFI cdef when ML_KEM or ML_DSA is enabled, so a wolfSSL build + # without those features must still import wolfcrypt.random successfully. + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + with: + submodules: recursive + - name: Set up Python 3.10 + uses: actions/setup-python@v3 + with: + python-version: "3.10" + - name: Install build deps + run: | + sudo apt-get update + sudo apt-get install -y autoconf automake libtool + - name: Build wolfSSL without ML-KEM/ML-DSA + run: | + cd lib/wolfssl + ./autogen.sh + ./configure --enable-cryptonly --disable-shared \ + --disable-kyber --disable-dilithium \ + --enable-aes --enable-aesgcm --enable-aessiv \ + --enable-aesctr --enable-aesgcm-stream \ + --enable-des3 --enable-chacha --enable-poly1305 \ + --enable-sha --enable-sha384 --enable-sha512 --enable-sha3 \ + --enable-hkdf --enable-rsa --enable-rsapss --enable-ecc \ + --enable-ed25519 --enable-ed448 --enable-curve25519 \ + --enable-keygen --enable-pwdbased --enable-pkcs7 \ + --disable-md5 --disable-sha224 --disable-dh \ + --disable-oldtls --disable-oldnames --disable-extended-master \ + CFLAGS=-fPIC \ + --prefix=$GITHUB_WORKSPACE/wolfssl-install + make + make install + - name: Install wolfcrypt-py against the local wolfSSL + env: + USE_LOCAL_WOLFSSL: ${{ github.workspace }}/wolfssl-install + run: | + python -m pip install --upgrade pip + pip install -r requirements/test.txt + pip install -e . + - name: Import smoke (regression for INVALID_DEVID) + run: python -c "from wolfcrypt.random import Random; Random()" + - name: Run tests + env: + USE_LOCAL_WOLFSSL: ${{ github.workspace }}/wolfssl-install + run: pytest tests/ diff --git a/wolfcrypt/random.py b/wolfcrypt/random.py index c94e343..0cebe26 100644 --- a/wolfcrypt/random.py +++ b/wolfcrypt/random.py @@ -33,7 +33,7 @@ class Random: A Cryptographically Secure Pseudo Random Number Generator - CSPRNG """ - def __init__(self, nonce: __builtins__.bytes = b"", device_id: int = _lib.INVALID_DEVID) -> None: + def __init__(self, nonce: __builtins__.bytes = b"", device_id: int = -2) -> None: self.native_object: _lib.RNG | None = _ffi.new("WC_RNG *") ret = _lib.wc_InitRngNonce_ex(self.native_object, nonce, len(nonce), _ffi.NULL, device_id) From 5e01d07e7f1e1afe0b84dcd7a0736b25836937ed Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 12:03:48 +0100 Subject: [PATCH 3/9] Fix ChaCha._set_key wiping encrypt stream state on first decrypt ChaCha._set_key ignored its `direction` argument and unconditionally re-keyed both self._enc and self._dec whenever either was allocated. Because _Cipher.encrypt and _Cipher.decrypt only call _set_key the first time their respective context is allocated, doing `encrypt(...); decrypt(...); encrypt(...)` on the same instance silently rewound the encryption stream: the first decrypt() allocated self._dec and re-keyed self._enc back to counter 0, so the second encrypt() produced ciphertext as if starting from the beginning of the keystream rather than continuing it. set_iv() relies on the both-directions reset to change the IV, so that path must be preserved. Honor the direction argument: direction _ENCRYPTION / _DECRYPTION only touches the matching context, while direction 0 (the value set_iv passes) keeps the existing reset-both behavior. Add two regression tests: one that interleaves encrypt/decrypt/encrypt and compares against a baseline two-encrypt sequence to catch any future re-introduction of cross-direction state stomping, and one that locks in set_iv's reset-both semantics so the fix is not later narrowed in a way that breaks IV changes. F-3586 --- tests/test_ciphers.py | 58 +++++++++++++++++++++++++++++++++++++++++++ wolfcrypt/ciphers.py | 12 +++++++-- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/tests/test_ciphers.py b/tests/test_ciphers.py index 4615b07..33b5074 100644 --- a/tests/test_ciphers.py +++ b/tests/test_ciphers.py @@ -998,6 +998,64 @@ def test_chacha_invalid_key_length(): with pytest.raises(ValueError, match="key must be"): ChaCha(b"\x00" * 20) + 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. + """ + key = b"\x00" * 32 + nonce = b"\x00" * 12 + block1 = b"A" * 64 + block2 = b"B" * 64 + + baseline = ChaCha(key) + baseline.set_iv(nonce) + expected_ct1 = baseline.encrypt(block1) + expected_ct2 = baseline.encrypt(block2) + assert expected_ct1 != expected_ct2 + + chacha = ChaCha(key) + 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 + + def test_chacha_set_iv_resets_both_directions(): + """ + set_iv() is documented to reset the stream, and the existing + implementation relies on _set_key re-keying both contexts when + the IV changes. Lock that behavior in so a fix to the + interleave bug does not regress set_iv semantics. + """ + key = b"\x00" * 32 + nonce_a = b"\x00" * 12 + nonce_b = b"\x01" + b"\x00" * 11 + plaintext = b"Z" * 32 + + chacha = ChaCha(key) + chacha.set_iv(nonce_a) + ct_a1 = chacha.encrypt(plaintext) + # Allocate the decryption context too. + chacha.decrypt(b"\x00" * 16) + # Changing IV must reset both contexts: subsequent encrypt/decrypt + # under nonce_b must match a freshly-keyed instance under nonce_b. + chacha.set_iv(nonce_b) + ct_b = chacha.encrypt(plaintext) + pt_back = chacha.decrypt(ct_b) + assert pt_back == plaintext + + fresh = ChaCha(key) + fresh.set_iv(nonce_b) + assert fresh.encrypt(plaintext) == ct_b + if _lib.RSA_ENABLED: def test_encrypt_oaep_requires_hash_type(vectors): diff --git a/wolfcrypt/ciphers.py b/wolfcrypt/ciphers.py index af8a7aa..24382e8 100644 --- a/wolfcrypt/ciphers.py +++ b/wolfcrypt/ciphers.py @@ -530,13 +530,21 @@ def __init__(self, key="", size=32): # pylint: disable=unused-argument def _set_key(self, direction): if self._key is None: return -1 - if self._enc: + # 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) + if do_enc: ret = _lib.wc_Chacha_SetKey(self._enc, self._key, len(self._key)) if ret == 0: ret = _lib.wc_Chacha_SetIV(self._enc, self._IV_nonce, self._IV_counter) if ret != 0: return ret - if self._dec: + if do_dec: ret = _lib.wc_Chacha_SetKey(self._dec, self._key, len(self._key)) if ret == 0: ret = _lib.wc_Chacha_SetIV(self._dec, self._IV_nonce, self._IV_counter) From 09770093bd60a1c1ba73699c83506e1007034ae7 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 12:12:36 +0100 Subject: [PATCH 4/9] Fix const on `wc_RsaPSS_Verify` --- scripts/build_ffi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/build_ffi.py b/scripts/build_ffi.py index 7e588a6..813b5ef 100644 --- a/scripts/build_ffi.py +++ b/scripts/build_ffi.py @@ -1051,7 +1051,7 @@ def build_ffi(local_wolfssl, features): cdef += """ int wc_RsaPSS_Sign(const byte* in, word32 inLen, byte* out, word32 outLen, enum wc_HashType hash, int mgf, RsaKey* key, WC_RNG* rng); - int wc_RsaPSS_Verify(byte* in, word32 inLen, byte* out, word32 outLen, + int wc_RsaPSS_Verify(const byte* in, word32 inLen, byte* out, word32 outLen, enum wc_HashType hash, int mgf, RsaKey* key); int wc_RsaPSS_CheckPadding(const byte* in, word32 inSz, byte* sig, word32 sigSz, enum wc_HashType hashType); From e961e83656c5628ee47d16a7f0496a732dd83cf0 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 12:15:51 +0100 Subject: [PATCH 5/9] Avoid passing Python bytes to non-const wc_GetPkcs8TraditionalOffset wc_GetPkcs8TraditionalOffset is declared in the wolfSSL public header as taking byte* input (non-const), even though the current implementation only reads from the buffer via ToTraditionalInline_ex. RsaPrivate.__init__ was passing a Python bytes object directly into that non-const parameter on the PKCS#8 fallback path, which lets CFFI hand the C library a writable pointer into immutable storage. There is no observable mutation today, but the contract permits it, and any future wolfSSL change to actually write through the parameter would silently corrupt the caller's bytes object. Copy the key into a CFFI-owned byte array for that one call. The adjacent wc_RsaPrivateKeyDecode calls are unaffected since their cdef is already const byte*. PKCS#8 key load is a one-time cost at construction, so the extra allocation is negligible. Not changing the cdef to const byte* because the wolfSSL public header itself declares the parameter non-const; diverging from the header would cause compile warnings in the CFFI-generated glue code. If wolfSSL ever const-corrects the header upstream, this copy can be dropped. F-2664 --- wolfcrypt/ciphers.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/ciphers.py b/wolfcrypt/ciphers.py index 24382e8..d654fd8 100644 --- a/wolfcrypt/ciphers.py +++ b/wolfcrypt/ciphers.py @@ -904,7 +904,12 @@ def __init__(self, key=None, hash_type=None, rng=None): # pylint: disable=super self.native_object, len(key)) if ret < 0: idx[0] = 0 - ret = _lib.wc_GetPkcs8TraditionalOffset(key, idx, len(key)) + # wc_GetPkcs8TraditionalOffset takes byte* (non-const) per + # the wolfSSL public header, so route it through a CFFI- + # owned buffer rather than handing it a writable pointer + # into the Python bytes object. + key_buf = _ffi.new("byte[]", key) + ret = _lib.wc_GetPkcs8TraditionalOffset(key_buf, idx, len(key)) if ret < 0: raise WolfCryptApiError("Invalid key error", ret) From 000472761c97d1f6b9fed16002b4ffa9d16db63c Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 12:27:23 +0100 Subject: [PATCH 6/9] Fix Ed448 ctx cdef to match wolfSSL header signature scripts/build_ffi.py declared wc_ed448_sign_msg and wc_ed448_verify_msg with two cdef discrepancies vs the wolfSSL header (ed448.h): - ctx was typed `byte*` instead of `const byte*`. CFFI was therefore happy to forward a writable pointer into Python bytes storage. The wolfSSL functions are const-correct and do not mutate ctx, so this had no runtime impact, but it widened the surface CFFI was willing to permit. - ctx_len was typed `word32` instead of `byte`. RFC 8032 caps Ed448 context at 255 bytes, and wolfSSL's prototype reflects that with a 1-byte parameter. The mismatch happens to work on x86_64 SysV because the low byte is what the callee reads, but it is still a real ABI discrepancy and would not be guaranteed on other platforms or calling conventions. Update the cdef to const byte* / byte to match the header on both functions. Add a guard in Ed448Public.verify and Ed448Private.sign so callers passing a ctx longer than 255 bytes get a clear ValueError rather than relying on CFFI's narrowing behavior. ML-DSA's wc_dilithium_sign_ctx_msg / wc_dilithium_verify_ctx_msg cdefs were also reviewed and already match the wolfSSL header. F-3086 --- scripts/build_ffi.py | 8 ++++---- wolfcrypt/ciphers.py | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/scripts/build_ffi.py b/scripts/build_ffi.py index 813b5ef..a6ab7a5 100644 --- a/scripts/build_ffi.py +++ b/scripts/build_ffi.py @@ -1172,11 +1172,11 @@ def build_ffi(local_wolfssl, features): int wc_ed448_size(ed448_key* key); int wc_ed448_sig_size(ed448_key* key); int wc_ed448_sign_msg(const byte* in, word32 inlen, byte* out, - word32 *outlen, ed448_key* key, byte* ctx, - word32 ctx_len); + word32 *outlen, ed448_key* key, const byte* ctx, + byte ctx_len); int wc_ed448_verify_msg(const byte* sig, word32 siglen, const byte* msg, - word32 msglen, int* stat, ed448_key* key, byte *ctx, - word32 ctx_len); + word32 msglen, int* stat, ed448_key* key, const byte *ctx, + byte ctx_len); int wc_Ed448PrivateKeyDecode(const byte*, word32*, ed448_key*, word32); int wc_Ed448KeyToDer(ed448_key*, byte* output, word32 inLen); diff --git a/wolfcrypt/ciphers.py b/wolfcrypt/ciphers.py index d654fd8..e778454 100644 --- a/wolfcrypt/ciphers.py +++ b/wolfcrypt/ciphers.py @@ -1705,6 +1705,9 @@ def verify(self, signature, data, ctx=None): if ctx is not None: ctx_buf = t2b(ctx) ctx_buf_len = len(ctx_buf) + if ctx_buf_len > 255: + raise ValueError("Ed448 ctx must be at most 255 bytes, got %d" % + ctx_buf_len) ret = _lib.wc_ed448_verify_msg(signature, len(signature), data, len(data), status, @@ -1825,6 +1828,9 @@ def sign(self, plaintext, ctx=None): if ctx is not None: ctx_buf = t2b(ctx) ctx_buf_len = len(ctx_buf) + if ctx_buf_len > 255: + raise ValueError("Ed448 ctx must be at most 255 bytes, got %d" % + ctx_buf_len) ret = _lib.wc_ed448_sign_msg(plaintext, len(plaintext), signature, signature_size, From cd9cdeba5fc17c546a581d3b9ebae27a5d22f191 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 12:29:15 +0100 Subject: [PATCH 7/9] Fix wc_ecc_import_unsigned cdef to match wolfSSL header scripts/build_ffi.py declared qx, qy, and d as `byte*` while the wolfSSL header (ecc.h) declares all three as `const byte*`. wolfSSL is const-correct here and does not mutate the inputs, so there is no observable bug today, but the non-const cdef let CFFI hand a writable pointer into Python bytes storage on EccPublic.decode_key_raw and EccPrivate.decode_key_raw calls. Update the cdef to match the header. F-3087 --- scripts/build_ffi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/build_ffi.py b/scripts/build_ffi.py index a6ab7a5..c647233 100644 --- a/scripts/build_ffi.py +++ b/scripts/build_ffi.py @@ -1092,8 +1092,8 @@ def build_ffi(local_wolfssl, features): int wc_ecc_import_x963(const byte* in, word32 inLen, ecc_key* key); int wc_ecc_export_private_raw(ecc_key* key, byte* qx, word32* qxLen, byte* qy, word32* qyLen, byte* d, word32* dLen); - int wc_ecc_import_unsigned(ecc_key* key, byte* qx, byte* qy, - byte* d, int curve_id); + int wc_ecc_import_unsigned(ecc_key* key, const byte* qx, const byte* qy, + const byte* d, int curve_id); int wc_ecc_export_public_raw(ecc_key* key, byte* qx, word32* qxLen, byte* qy, word32* qyLen); From 57cca0e90de819b2428f34686fd4f192ead703a7 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 12:33:41 +0100 Subject: [PATCH 8/9] Validate raw element lengths in EccPublic/EccPrivate.decode_key_raw wc_ecc_import_unsigned takes no length parameters for qx/qy/d: it reads exactly curve_size bytes from each pointer based on curve_id, via mp_read_unsigned_bin in wc_ecc_import_raw_private. The Python decode_key_raw wrappers handed the user-supplied buffers straight through without any length check, so a shorter buffer caused the C library to read past the end of the Python buffer (OOB read of adjacent memory, potentially leaking it into the imported key or segfaulting). A longer buffer silently dropped the extra bytes. Add wc_ecc_get_curve_size_from_id to the CFFI cdef, then in both decode_key_raw methods t2b the inputs, look up the expected curve size, and raise ValueError if any of qx/qy/d does not match. Reject unknown curve_id values with a clear message rather than falling through to wolfSSL with a bogus size. Add test_ecc_decode_key_raw_rejects_wrong_length covering short qx, long qy, short d, unknown curve_id, and the happy path on both EccPublic and EccPrivate. F-3088 --- scripts/build_ffi.py | 1 + tests/test_ciphers.py | 31 +++++++++++++++++++++++++++++++ wolfcrypt/ciphers.py | 21 +++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/scripts/build_ffi.py b/scripts/build_ffi.py index c647233..2a36eb2 100644 --- a/scripts/build_ffi.py +++ b/scripts/build_ffi.py @@ -1096,6 +1096,7 @@ def build_ffi(local_wolfssl, features): const byte* d, int curve_id); int wc_ecc_export_public_raw(ecc_key* key, byte* qx, word32* qxLen, byte* qy, word32* qyLen); + int wc_ecc_get_curve_size_from_id(int curve_id); int wc_ecc_shared_secret(ecc_key* private_key, ecc_key* public_key, diff --git a/tests/test_ciphers.py b/tests/test_ciphers.py index 33b5074..fe2894b 100644 --- a/tests/test_ciphers.py +++ b/tests/test_ciphers.py @@ -614,6 +614,37 @@ def test_key_encoding(vectors): assert qy[0:32] == vectors[EccPublic].raw_key[32:64] + def test_ecc_decode_key_raw_rejects_wrong_length(vectors): + """ + wc_ecc_import_unsigned reads exactly curve_size bytes from each + of qx/qy/d with no length information from the caller. The + Python wrappers must validate the lengths up-front so a short + buffer cannot cause an out-of-bounds read in the C library. + """ + raw_priv = EccPrivate() + raw_pub = EccPublic() + key = vectors[EccPrivate].raw_key + qx_good, qy_good, d_good = key[0:32], key[32:64], key[64:96] + + # Short qx + with pytest.raises(ValueError, match="must each be 32 bytes"): + raw_pub.decode_key_raw(qx_good[:-1], qy_good) + with pytest.raises(ValueError, match="must each be 32 bytes"): + raw_priv.decode_key_raw(qx_good[:-1], qy_good, d_good) + # Long qy + with pytest.raises(ValueError, match="must each be 32 bytes"): + raw_pub.decode_key_raw(qx_good, qy_good + b"\x00") + with pytest.raises(ValueError, match="must each be 32 bytes"): + raw_priv.decode_key_raw(qx_good, qy_good + b"\x00", d_good) + # Short d + with pytest.raises(ValueError, match="must each be 32 bytes"): + raw_priv.decode_key_raw(qx_good, qy_good, d_good[:-1]) + # Unknown curve id + with pytest.raises(ValueError, match="Unknown ECC curve_id"): + raw_pub.decode_key_raw(qx_good, qy_good, curve_id=-99999) + # Happy path still works after validation + raw_pub.decode_key_raw(qx_good, qy_good) + raw_priv.decode_key_raw(qx_good, qy_good, d_good) diff --git a/wolfcrypt/ciphers.py b/wolfcrypt/ciphers.py index e778454..11ede32 100644 --- a/wolfcrypt/ciphers.py +++ b/wolfcrypt/ciphers.py @@ -1112,6 +1112,15 @@ def decode_key_raw(self, qx, qy, curve_id=ECC_SECP256R1): """ Decodes an ECC public key from its raw elements: (Qx,Qy) """ + qx = t2b(qx) + qy = t2b(qy) + curve_size = _lib.wc_ecc_get_curve_size_from_id(curve_id) + if curve_size <= 0: + raise ValueError("Unknown ECC curve_id %d" % curve_id) + if len(qx) != curve_size or len(qy) != curve_size: + raise ValueError( + "qx and qy must each be %d bytes for curve_id %d, got " + "qx=%d qy=%d" % (curve_size, curve_id, len(qx), len(qy))) ret = _lib.wc_ecc_import_unsigned(self.native_object, qx, qy, _ffi.NULL, curve_id) if ret != 0: @@ -1290,6 +1299,18 @@ def decode_key_raw(self, qx, qy, d, curve_id=ECC_SECP256R1): Decodes an ECC private key from its raw elements: public (Qx,Qy) and private(d) """ + qx = t2b(qx) + qy = t2b(qy) + d = t2b(d) + curve_size = _lib.wc_ecc_get_curve_size_from_id(curve_id) + if curve_size <= 0: + raise ValueError("Unknown ECC curve_id %d" % curve_id) + if (len(qx) != curve_size or len(qy) != curve_size + or len(d) != curve_size): + raise ValueError( + "qx, qy and d must each be %d bytes for curve_id %d, got " + "qx=%d qy=%d d=%d" % (curve_size, curve_id, + len(qx), len(qy), len(d))) ret = _lib.wc_ecc_import_unsigned(self.native_object, qx, qy, d, curve_id) if ret != 0: From 98215b60f60649639b32f19fd37f8d986d67502a Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 11 May 2026 13:05:23 +0100 Subject: [PATCH 9/9] 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 @@ def test_chacha_invalid_key_length(): 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 @@ def test_chacha_decrypt_does_not_reset_encrypt_stream(): 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 @@ def encrypt(self, associated_data, nonce, plaintext): 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 @@ def decrypt(self, associated_data, nonce, siv, ciphertext): 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 @@ def _prepare_associated_data(associated_data): """ 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 @@ def __init__(self, key="", size=32): # pylint: disable=unused-argument 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 @@ def set_iv(self, nonce, counter = 0): 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)