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/scripts/build_ffi.py b/scripts/build_ffi.py index 7e588a6..2a36eb2 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); @@ -1092,10 +1092,11 @@ 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); + int wc_ecc_get_curve_size_from_id(int curve_id); int wc_ecc_shared_secret(ecc_key* private_key, ecc_key* public_key, @@ -1172,11 +1173,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/tests/test_ciphers.py b/tests/test_ciphers.py index c8b1e25..0f759ad 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) @@ -938,6 +969,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 @@ -969,6 +1029,83 @@ 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, 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) + 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 + 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 + 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 b3899b5..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 @@ -363,7 +366,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) @@ -527,16 +530,27 @@ 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 - if self._enc: + # _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: 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) @@ -560,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) @@ -896,7 +910,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) @@ -1099,6 +1118,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: @@ -1277,6 +1305,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: @@ -1692,6 +1732,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, @@ -1812,6 +1855,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, 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)