From 4976ebbea1849f0bb722258b0d15ef0fe94104dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 8 Jun 2026 07:50:03 +0200 Subject: [PATCH 1/7] Ensure a key is set for ARC4 operations Fixes F-5378 --- tests/api/test_arc4.c | 37 ++++++++++++++++++++++++++++++++++++ tests/api/test_arc4.h | 2 ++ tests/api/test_ossl_cipher.c | 4 +++- wolfcrypt/src/arc4.c | 8 ++++++++ wolfssl/openssl/rc4.h | 4 ++-- wolfssl/wolfcrypt/arc4.h | 1 + 6 files changed, 53 insertions(+), 3 deletions(-) diff --git a/tests/api/test_arc4.c b/tests/api/test_arc4.c index 11bd20d671f..3fe4d57eaed 100644 --- a/tests/api/test_arc4.c +++ b/tests/api/test_arc4.c @@ -104,6 +104,43 @@ int test_wc_Arc4Process(void) } /* END test_wc_Arc4Process */ +/* + * Regression test for issue 5378: wc_Arc4Process must refuse to run unless a + * key has been configured with wc_Arc4SetKey(), both before SetKey and after + * Free. Otherwise the keystream is all-zero and ARC4 silently copies the + * plaintext into the ciphertext. + */ +int test_wc_Arc4Process_no_key(void) +{ + EXPECT_DECLS; +#ifndef NO_RC4 + Arc4 arc4; + const char* key = "\x01\x23\x45\x67\x89\xab\xcd\xef"; + int keyLen = 8; + const char* input = "\x01\x23\x45\x67\x89\xab\xcd\xef"; + byte cipher[8]; + + XMEMSET(&arc4, 0, sizeof(arc4)); + XMEMSET(cipher, 0, sizeof(cipher)); + + /* Processing without a key after init must fail, not silently copy. */ + ExpectIntEQ(wc_Arc4Init(&arc4, NULL, INVALID_DEVID), 0); + ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen), + WC_NO_ERR_TRACE(MISSING_KEY)); + + /* After a key is set, processing succeeds. */ + ExpectIntEQ(wc_Arc4SetKey(&arc4, (byte*)key, (word32)keyLen), 0); + ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen), 0); + + /* After free, the keyed state is cleared and processing must fail again. */ + wc_Arc4Free(&arc4); + ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen), + WC_NO_ERR_TRACE(MISSING_KEY)); +#endif + return EXPECT_RESULT(); + +} /* END test_wc_Arc4Process_no_key */ + #include diff --git a/tests/api/test_arc4.h b/tests/api/test_arc4.h index f79104d9c6e..28075e3bc9b 100644 --- a/tests/api/test_arc4.h +++ b/tests/api/test_arc4.h @@ -26,11 +26,13 @@ int test_wc_Arc4SetKey(void); int test_wc_Arc4Process(void); +int test_wc_Arc4Process_no_key(void); int test_wc_Arc4_MonteCarlo(void); #define TEST_ARC4_DECLS \ TEST_DECL_GROUP("arc4", test_wc_Arc4SetKey), \ TEST_DECL_GROUP("arc4", test_wc_Arc4Process), \ + TEST_DECL_GROUP("arc4", test_wc_Arc4Process_no_key), \ TEST_DECL_GROUP("arc4", test_wc_Arc4_MonteCarlo) #endif /* WOLFCRYPT_TEST_ARC4_H */ diff --git a/tests/api/test_ossl_cipher.c b/tests/api/test_ossl_cipher.c index f40b097546c..c618d45c4d7 100644 --- a/tests/api/test_ossl_cipher.c +++ b/tests/api/test_ossl_cipher.c @@ -965,7 +965,9 @@ int test_wolfSSL_RC4(void) wolfSSL_RC4(NULL, 0, data, enc); ExpectIntEQ(1, 1); - for (i = 0; EXPECT_SUCCESS() && (i <= sizeof(key)); i++) { + /* Start at a key length of 1: a zero-length key is invalid and leaves the + * Arc4 object without a key set, so encrypt/decrypt is a no-op. */ + for (i = 1; EXPECT_SUCCESS() && (i <= sizeof(key)); i++) { for (j = 0; EXPECT_SUCCESS() && (j <= sizeof(data)); j++) { XMEMSET(enc, 0, sizeof(enc)); XMEMSET(dec, 0, sizeof(dec)); diff --git a/wolfcrypt/src/arc4.c b/wolfcrypt/src/arc4.c index 6d83ed2569d..dc370143358 100644 --- a/wolfcrypt/src/arc4.c +++ b/wolfcrypt/src/arc4.c @@ -67,6 +67,8 @@ int wc_Arc4SetKey(Arc4* arc4, const byte* key, word32 length) keyIndex = 0; } + arc4->keySet = 1; + return ret; } @@ -102,6 +104,10 @@ int wc_Arc4Process(Arc4* arc4, byte* out, const byte* in, word32 length) } #endif + if (!arc4->keySet) { + return MISSING_KEY; + } + x = arc4->x; y = arc4->y; @@ -123,6 +129,7 @@ int wc_Arc4Init(Arc4* arc4, void* heap, int devId) return BAD_FUNC_ARG; arc4->heap = heap; + arc4->keySet = 0; #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ARC4) ret = wolfAsync_DevCtxInit(&arc4->asyncDev, WOLFSSL_ASYNC_MARKER_ARC4, @@ -148,6 +155,7 @@ void wc_Arc4Free(Arc4* arc4) ForceZero(arc4->state, sizeof(arc4->state)); arc4->x = 0; arc4->y = 0; + arc4->keySet = 0; } #endif /* NO_RC4 */ diff --git a/wolfssl/openssl/rc4.h b/wolfssl/openssl/rc4.h index c53c47b6b60..53168c6f816 100644 --- a/wolfssl/openssl/rc4.h +++ b/wolfssl/openssl/rc4.h @@ -40,9 +40,9 @@ typedef struct WOLFSSL_RC4_KEY { /* big enough for Arc4 from wolfssl/wolfcrypt/arc4.h */ #ifdef WC_NO_PTR_INT_CAST - void* holder[(288 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; + void* holder[(296 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; #else - void* holder[(272 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; + void* holder[(280 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; #endif } WOLFSSL_RC4_KEY; diff --git a/wolfssl/wolfcrypt/arc4.h b/wolfssl/wolfcrypt/arc4.h index 041ee712132..059014d544c 100644 --- a/wolfssl/wolfcrypt/arc4.h +++ b/wolfssl/wolfcrypt/arc4.h @@ -51,6 +51,7 @@ typedef struct Arc4 { WC_ASYNC_DEV asyncDev; #endif void* heap; + WC_BITFIELD keySet:1; /* set to 1 once a key has been configured */ } Arc4; WOLFSSL_API int wc_Arc4Process(Arc4* arc4, byte* out, const byte* in, From f6b7624a91f129e336de207d1d14cfb215ce1c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 8 Jun 2026 08:04:40 +0200 Subject: [PATCH 2/7] Ensure a key is set for DES3 operations Fixes F-5379 --- tests/api/test_des3.c | 61 ++++++++++++++++++++++++++++++++++++++++ tests/api/test_des3.h | 12 ++++---- wolfcrypt/src/des3.c | 11 ++++++++ wolfssl/wolfcrypt/des3.h | 1 + 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/tests/api/test_des3.c b/tests/api/test_des3.c index c52badb22f3..ca8e0ba47c1 100644 --- a/tests/api/test_des3.c +++ b/tests/api/test_des3.c @@ -181,6 +181,67 @@ int test_wc_Des3_CbcEncryptDecrypt(void) } /* END wc_Des3_CbcEncrypt */ +/* + * Regression test for issue 5379: wc_Des3_CbcEncrypt/Decrypt must refuse to + * run unless a key has been configured with wc_Des3_SetKey(), both before + * SetKey and after Free. Otherwise the operation would silently run with + * uninitialized or zeroed key material and return success. + * + * FIPS builds use the FIPS-certified DES3 implementation which does not track + * key state, so skip the test for FIPS. + */ +int test_wc_Des3_CbcEncryptDecrypt_no_key(void) +{ + EXPECT_DECLS; +#if !defined(NO_DES3) && !defined(HAVE_FIPS) + Des3 des; + byte cipher[24]; + byte plain[24]; + const byte key[] = { + 0x01,0x23,0x45,0x67,0x89,0xab,0xcd,0xef, + 0xfe,0xde,0xba,0x98,0x76,0x54,0x32,0x10, + 0x89,0xab,0xcd,0xef,0x01,0x23,0x45,0x67 + }; + const byte iv[] = { + 0x12,0x34,0x56,0x78,0x90,0xab,0xcd,0xef, + 0x01,0x01,0x01,0x01,0x01,0x01,0x01,0x01, + 0x11,0x21,0x31,0x41,0x51,0x61,0x71,0x81 + }; + const byte vector[] = { /* "Now is the time for all " w/o trailing 0 */ + 0x4e,0x6f,0x77,0x20,0x69,0x73,0x20,0x74, + 0x68,0x65,0x20,0x74,0x69,0x6d,0x65,0x20, + 0x66,0x6f,0x72,0x20,0x61,0x6c,0x6c,0x20 + }; + + XMEMSET(&des, 0, sizeof(Des3)); + XMEMSET(cipher, 0, sizeof(cipher)); + XMEMSET(plain, 0, sizeof(plain)); + + /* Encrypt/decrypt without a key after init must fail, not silently run. */ + ExpectIntEQ(wc_Des3Init(&des, NULL, INVALID_DEVID), 0); + ExpectIntEQ(wc_Des3_CbcEncrypt(&des, cipher, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); + ExpectIntEQ(wc_Des3_CbcDecrypt(&des, plain, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); + + /* After a key is set, the operations succeed. */ + ExpectIntEQ(wc_Des3_SetKey(&des, key, iv, DES_ENCRYPTION), 0); + ExpectIntEQ(wc_Des3_CbcEncrypt(&des, cipher, vector, 24), 0); + ExpectIntEQ(wc_Des3_SetKey(&des, key, iv, DES_DECRYPTION), 0); + ExpectIntEQ(wc_Des3_CbcDecrypt(&des, plain, cipher, 24), 0); + ExpectIntEQ(XMEMCMP(plain, vector, 24), 0); + + /* After free, the keyed state is cleared and operations must fail again. */ + wc_Des3Free(&des); + ExpectIntEQ(wc_Des3_CbcEncrypt(&des, cipher, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); + ExpectIntEQ(wc_Des3_CbcDecrypt(&des, plain, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); +#endif + return EXPECT_RESULT(); + +} /* END test_wc_Des3_CbcEncryptDecrypt_no_key */ + /* * Unit test for wc_Des3_EcbEncrypt */ diff --git a/tests/api/test_des3.h b/tests/api/test_des3.h index 9f530f7e292..bd66779af87 100644 --- a/tests/api/test_des3.h +++ b/tests/api/test_des3.h @@ -27,14 +27,16 @@ int test_wc_Des3_SetIV(void); int test_wc_Des3_SetKey(void); int test_wc_Des3_CbcEncryptDecrypt(void); +int test_wc_Des3_CbcEncryptDecrypt_no_key(void); int test_wc_Des3_EcbEncrypt(void); int test_wc_Des3Cbc_MonteCarlo(void); -#define TEST_DES3_DECLS \ - TEST_DECL_GROUP("des3", test_wc_Des3_SetIV), \ - TEST_DECL_GROUP("des3", test_wc_Des3_SetKey), \ - TEST_DECL_GROUP("des3", test_wc_Des3_CbcEncryptDecrypt), \ - TEST_DECL_GROUP("des3", test_wc_Des3_EcbEncrypt), \ +#define TEST_DES3_DECLS \ + TEST_DECL_GROUP("des3", test_wc_Des3_SetIV), \ + TEST_DECL_GROUP("des3", test_wc_Des3_SetKey), \ + TEST_DECL_GROUP("des3", test_wc_Des3_CbcEncryptDecrypt), \ + TEST_DECL_GROUP("des3", test_wc_Des3_CbcEncryptDecrypt_no_key), \ + TEST_DECL_GROUP("des3", test_wc_Des3_EcbEncrypt), \ TEST_DECL_GROUP("des3", test_wc_Des3Cbc_MonteCarlo) #endif /* WOLFCRYPT_TEST_DES3_H */ diff --git a/wolfcrypt/src/des3.c b/wolfcrypt/src/des3.c index 12f884e0a5d..f1beae1b60c 100644 --- a/wolfcrypt/src/des3.c +++ b/wolfcrypt/src/des3.c @@ -1649,6 +1649,8 @@ if (ret != 0) return ret; + des->keySet = 1; + return wc_Des3_SetIV(des, iv); } @@ -1793,6 +1795,10 @@ return BAD_LENGTH_E; } + if (!des->keySet) { + return MISSING_KEY; + } + #ifdef WOLF_CRYPTO_CB if (des->devId != INVALID_DEVID) { int ret = wc_CryptoCb_Des3Encrypt(des, out, in, sz); @@ -1848,6 +1854,10 @@ return BAD_LENGTH_E; } + if (!des->keySet) { + return MISSING_KEY; + } + #ifdef WOLF_CRYPTO_CB if (des->devId != INVALID_DEVID) { int ret = wc_CryptoCb_Des3Decrypt(des, out, in, sz); @@ -1969,6 +1979,7 @@ int wc_Des3Init(Des3* des3, void* heap, int devId) return BAD_FUNC_ARG; des3->heap = heap; + des3->keySet = 0; #ifdef WOLF_CRYPTO_CB des3->devId = devId; diff --git a/wolfssl/wolfcrypt/des3.h b/wolfssl/wolfcrypt/des3.h index 4ff912fc8b3..117abeb97fb 100644 --- a/wolfssl/wolfcrypt/des3.h +++ b/wolfssl/wolfcrypt/des3.h @@ -113,6 +113,7 @@ struct Des3 { void* devCtx; #endif void* heap; + WC_BITFIELD keySet:1; /* set to 1 once a key has been configured */ }; #ifndef WC_DES3_TYPE_DEFINED From 2cc68d3a4f892f468ffd5bb7f716673f29afbbf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 8 Jun 2026 08:13:57 +0200 Subject: [PATCH 3/7] Fix minor CAVIUM issues Fixes F-4441 and F-4442 --- wolfcrypt/src/port/cavium/cavium_nitrox.c | 28 +++++++++++++++-------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/wolfcrypt/src/port/cavium/cavium_nitrox.c b/wolfcrypt/src/port/cavium/cavium_nitrox.c index 91defa24b86..c214e875777 100644 --- a/wolfcrypt/src/port/cavium/cavium_nitrox.c +++ b/wolfcrypt/src/port/cavium/cavium_nitrox.c @@ -230,7 +230,7 @@ int NitroxCheckRequests(WC_ASYNC_DEV* dev, word32 buf_size = sizeof(req_stat_buf->req); ret = CspGetAllResults(req_stat_buf->req, buf_size, &res_count, dev->nitrox.devId); - multi_req->count = res_count; + req_stat_buf->count = res_count; #endif return NitroxTranslateResponseCode(ret); @@ -324,6 +324,10 @@ int NitroxRsaPrivateDecrypt(const byte* in, word32 inLen, byte* out, word32* outLen, RsaKey* key) { int ret; + /* The Csp call writes a 16-bit length; use a dedicated Uint16 rather than + * casting the word32* outLen, which would place the bytes in the wrong half + * of the word on a big-endian host before the ntohs() below. */ + Uint16 localOutLen = 0; if (key == NULL || in == NULL || out == NULL || inLen != (word32)key->n.raw.len) { @@ -337,17 +341,17 @@ int NitroxRsaPrivateDecrypt(const byte* in, word32 inLen, byte* out, ret = CspPkcs1v15CrtDec(key->asyncDev.nitrox.devId, CAVIUM_REQ_MODE, CAVIUM_SSL_GRP, CAVIUM_DPORT, BT2, key->n.raw.len, key->q.raw.buf, key->dQ.raw.buf, key->p.raw.buf, key->dP.raw.buf, key->u.raw.buf, - (byte*)in, (Uint16*)outLen, out, &key->asyncDev.nitrox.reqId); + (byte*)in, &localOutLen, out, &key->asyncDev.nitrox.reqId); #else ret = CspPkcs1v15CrtDec(CAVIUM_REQ_MODE, BT2, key->n.raw.len, key->q.raw.buf, key->dQ.raw.buf, key->p.raw.buf, key->dP.raw.buf, - key->u.raw.buf, (byte*)in, &outLen, out, &key->asyncDev.nitrox.reqId, - key->asyncDev.nitrox.devId); + key->u.raw.buf, (byte*)in, &localOutLen, out, + &key->asyncDev.nitrox.reqId, key->asyncDev.nitrox.devId); #endif #ifdef WOLFSSL_NITROX_DEBUG printf("NitroxRsaPrivateDecrypt: ret %x, req %lx in %p (%d), out %p (%d)\n", - ret, key->asyncDev.nitrox.reqId, in, inLen, out, *outLen); + ret, key->asyncDev.nitrox.reqId, in, inLen, out, localOutLen); #endif ret = NitroxTranslateResponseCode(ret); @@ -355,7 +359,7 @@ int NitroxRsaPrivateDecrypt(const byte* in, word32 inLen, byte* out, return ret; } - *outLen = ntohs(*outLen); + *outLen = ntohs(localOutLen); return *outLen; } @@ -404,6 +408,10 @@ int NitroxRsaSSL_Verify(const byte* in, word32 inLen, byte* out, word32* outLen, RsaKey* key) { int ret; + /* The Csp call writes a 16-bit length; use a dedicated Uint16 rather than + * casting the word32* outLen, which would place the bytes in the wrong half + * of the word on a big-endian host before the ntohs() below. */ + Uint16 localOutLen = 0; if (key == NULL || in == NULL || out == NULL || inLen != (word32)key->n.raw.len) { @@ -416,17 +424,17 @@ int NitroxRsaSSL_Verify(const byte* in, word32 inLen, byte* out, #ifdef HAVE_CAVIUM_V ret = CspPkcs1v15Dec(key->asyncDev.nitrox.devId, CAVIUM_REQ_MODE, CAVIUM_SSL_GRP, CAVIUM_DPORT, BT1, key->n.raw.len, key->e.raw.len, - key->n.raw.buf, key->e.raw.buf, (byte*)in, (Uint16*)outLen, out, + key->n.raw.buf, key->e.raw.buf, (byte*)in, &localOutLen, out, &key->asyncDev.nitrox.reqId); #else ret = CspPkcs1v15Dec(CAVIUM_REQ_MODE, BT1, key->n.raw.len, key->e.raw.len, - key->n.raw.buf, key->e.raw.buf, (byte*)in, &outLen, out, + key->n.raw.buf, key->e.raw.buf, (byte*)in, &localOutLen, out, &key->asyncDev.nitrox.reqId, key->asyncDev.nitrox.devId); #endif #ifdef WOLFSSL_NITROX_DEBUG printf("NitroxRsaSSL_Verify: ret %x, req %lx in %p (%d), out %p (%d)\n", - ret, key->asyncDev.nitrox.reqId, in, inLen, out, *outLen); + ret, key->asyncDev.nitrox.reqId, in, inLen, out, localOutLen); #endif ret = NitroxTranslateResponseCode(ret); @@ -434,7 +442,7 @@ int NitroxRsaSSL_Verify(const byte* in, word32 inLen, byte* out, return ret; } - *outLen = ntohs(*outLen); + *outLen = ntohs(localOutLen); return *outLen; } From b7c9dc7c2e85eee9f9c5efcb6d24c695bb24362e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 8 Jun 2026 08:19:13 +0200 Subject: [PATCH 4/7] Minor fix in liboqs GetRandomData Fixes F-4443 --- wolfcrypt/src/port/liboqs/liboqs.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/port/liboqs/liboqs.c b/wolfcrypt/src/port/liboqs/liboqs.c index 826d1b3cd9a..220f704c9e2 100644 --- a/wolfcrypt/src/port/liboqs/liboqs.c +++ b/wolfcrypt/src/port/liboqs/liboqs.c @@ -49,7 +49,14 @@ static void wolfSSL_liboqsGetRandomData(uint8_t* buffer, size_t numOfBytes) while (numOfBytes > 0) { numOfBytes_word32 = (word32)numOfBytes; - numOfBytes -= numOfBytes_word32; + /* On platforms where size_t is wider than word32, the cast above can + * truncate. If numOfBytes does not fit into a word32 (including the + * case where it is an exact multiple of 2^32 and truncates to 0), + * generate the largest chunk that fits to guarantee forward progress + * and avoid an infinite loop. */ + if ((size_t)numOfBytes_word32 != numOfBytes) { + numOfBytes_word32 = 0xFFFFFFFFU; + } ret = wc_RNG_GenerateBlock(liboqsCurrentRNG, buffer, numOfBytes_word32); if (ret != 0) { @@ -62,6 +69,10 @@ static void wolfSSL_liboqsGetRandomData(uint8_t* buffer, size_t numOfBytes) ); abort(); } + /* Advance the buffer so subsequent iterations append rather than + * overwrite the previously generated bytes. */ + buffer += numOfBytes_word32; + numOfBytes -= numOfBytes_word32; } } From 9398abb368113fc0ae8d2f86dff2babb29041c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 8 Jun 2026 09:20:36 +0200 Subject: [PATCH 5/7] Add missing ForceZero() calls Fixes F-5437 and F-5438 --- wolfcrypt/src/asn.c | 35 +++++++++++++---------------------- wolfcrypt/src/wc_mldsa.c | 27 ++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 00be607506c..35c07d38b6d 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -31973,10 +31973,7 @@ static int eccToPKCS8(ecc_key* key, byte* output, word32* outLen, ret = wc_BuildEccKeyDer(key, tmpDer, &sz, includePublic, 0); if (ret < 0) { - #ifndef WOLFSSL_NO_MALLOC - XFREE(tmpDer, key->heap, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; + goto exit; } tmpDerSz = (word32)ret; @@ -31984,42 +31981,36 @@ static int eccToPKCS8(ecc_key* key, byte* output, word32* outLen, ret = wc_CreatePKCS8Key(NULL, &pkcs8Sz, tmpDer, tmpDerSz, algoID, curveOID, oidSz); if (ret != WC_NO_ERR_TRACE(LENGTH_ONLY_E)) { - #ifndef WOLFSSL_NO_MALLOC - XFREE(tmpDer, key->heap, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; + goto exit; } if (output == NULL) { - #ifndef WOLFSSL_NO_MALLOC - XFREE(tmpDer, key->heap, DYNAMIC_TYPE_TMP_BUFFER); - #endif *outLen = pkcs8Sz; - return WC_NO_ERR_TRACE(LENGTH_ONLY_E); - + ret = WC_NO_ERR_TRACE(LENGTH_ONLY_E); + goto exit; } else if (*outLen < pkcs8Sz) { - #ifndef WOLFSSL_NO_MALLOC - XFREE(tmpDer, key->heap, DYNAMIC_TYPE_TMP_BUFFER); - #endif WOLFSSL_MSG("Input buffer too small for ECC PKCS#8 key"); - return BUFFER_E; + ret = BUFFER_E; + goto exit; } ret = wc_CreatePKCS8Key(output, &pkcs8Sz, tmpDer, tmpDerSz, algoID, curveOID, oidSz); if (ret < 0) { - #ifndef WOLFSSL_NO_MALLOC - XFREE(tmpDer, key->heap, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; + goto exit; } + *outLen = (word32)ret; + +exit: + /* tmpDer holds a plaintext copy of the ECC private key - always zeroize + * it before releasing (or before the stack buffer goes out of scope). */ + ForceZero(tmpDer, ECC_BUFSIZE); #ifndef WOLFSSL_NO_MALLOC XFREE(tmpDer, key->heap, DYNAMIC_TYPE_TMP_BUFFER); #endif - *outLen = (word32)ret; return ret; } diff --git a/wolfcrypt/src/wc_mldsa.c b/wolfcrypt/src/wc_mldsa.c index 3e08dad88a6..73b7a9b1182 100644 --- a/wolfcrypt/src/wc_mldsa.c +++ b/wolfcrypt/src/wc_mldsa.c @@ -7927,6 +7927,12 @@ static int mldsa_make_key_from_seed(wc_MlDsaKey* key, const byte* seed) } #ifndef WC_MLDSA_CACHE_PRIV_VECTORS + /* Zeroize the private vectors s1, s2 and t before freeing. These occupy + * the front of the buffer; any matrix A that follows is public (expanded + * from the public seed) and need not be cleared. */ + if (s1 != NULL) { + ForceZero(s1, (word32)params->s1Sz + 2U * (word32)params->s2Sz); + } XFREE(s1, key->heap, DYNAMIC_TYPE_MLDSA); #endif return ret; @@ -7945,6 +7951,7 @@ static int mldsa_make_key_from_seed(wc_MlDsaKey* key, const byte* seed) unsigned int r; unsigned int s; byte kl[2]; + unsigned int allocSz = 0; #ifdef WOLFSSL_MLDSA_DYNAMIC_KEYS ret = mldsa_alloc_priv_buf(key); @@ -7959,8 +7966,6 @@ static int mldsa_make_key_from_seed(wc_MlDsaKey* key, const byte* seed) /* Allocate memory for large intermediates. */ if (ret == 0) { - unsigned int allocSz; - /* s1-l, s2-k, t-k, a-1 */ allocSz = (unsigned int)params->s1Sz + params->s2Sz + params->s2Sz + (unsigned int)MLDSA_REJ_NTT_POLY_H_SIZE + @@ -8158,6 +8163,15 @@ static int mldsa_make_key_from_seed(wc_MlDsaKey* key, const byte* seed) key->pubKeySet = 1; } + /* Zeroize the whole buffer before freeing. It holds the private vectors + * s1, s2 and t at the front; the rejection-sampling / matrix A region in + * the middle is public, but the trailing t64 accumulator (POLY64 builds) + * holds A o NTT(s1) - from which s1 is recoverable - so it must be + * cleared too. As the secret material is not contiguous, zeroize the + * entire allocation rather than a sub-range. */ + if (s1 != NULL) { + ForceZero(s1, allocSz); + } XFREE(s1, key->heap, DYNAMIC_TYPE_MLDSA); return ret; #endif @@ -8584,7 +8598,14 @@ static int mldsa_sign_with_seed_mu(wc_MlDsaKey* key, ForceZero(priv_rand_seed, sizeof(priv_rand_seed)); if (y != NULL) { - ForceZero(y, allocSz); + word32 zeroSz = allocSz; +#ifndef WC_MLDSA_CACHE_MATRIX_A + /* The public matrix A is appended at the end of the buffer and is + * expanded from the public seed - it need not be zeroized. The + * preceding vectors (y, w0, s1, s2, t0, ...) are secret dependent. */ + zeroSz -= (word32)params->aSz; +#endif + ForceZero(y, zeroSz); } XFREE(y, key->heap, DYNAMIC_TYPE_MLDSA); return ret; From 34febaf69c5d3e46da86d2056606664b18b0e626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 8 Jun 2026 11:07:38 +0200 Subject: [PATCH 6/7] Improve supported_groups handling Fixes F-4891 --- src/tls.c | 17 +++++++- tests/api.c | 1 + tests/api/test_tls_ext.c | 84 ++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls_ext.h | 1 + 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/tls.c b/src/tls.c index f9ca896ae32..944ff2416a7 100644 --- a/src/tls.c +++ b/src/tls.c @@ -5222,8 +5222,13 @@ int TLSX_SupportedCurve_Parse(const WOLFSSL* ssl, const byte* input, if (length != OPAQUE16_LEN + offset) return BUFFER_ERROR; offset = OPAQUE16_LEN; - if (offset == length) - return 0; + if (offset == length) { + /* An empty named group list is malformed (named_group_list<2..2^16-1>, + * RFC 8422 / RFC 8446). BUFFER_ERROR yields a decode_error alert (see + * TranslateErrorToAlert()). Accepting it would also make an explicit + * empty extension look absent and impose no group restriction. */ + return BUFFER_ERROR; + } extension = TLSX_Find(*extensions, TLSX_SUPPORTED_GROUPS); if (extension == NULL) { @@ -5240,6 +5245,14 @@ int TLSX_SupportedCurve_Parse(const WOLFSSL* ssl, const byte* input, break; ret = 0; } + /* All advertised groups are unsupported, so no node was added above. + * Record an empty node so suite selection still sees the restriction + * (e.g. ECC/ECDHE must not be chosen) instead of treating the + * extension as absent. */ + if (ret == 0 && isRequest && + TLSX_Find(*extensions, TLSX_SUPPORTED_GROUPS) == NULL) { + ret = TLSX_Push(extensions, TLSX_SUPPORTED_GROUPS, NULL, ssl->heap); + } } else { /* Find the intersection with what the user has set */ diff --git a/tests/api.c b/tests/api.c index 9676c470a32..679487d12d8 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35137,6 +35137,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_TLSX_SNI_GetSize_overflow), TEST_DECL(test_TLSX_ECH_msg_type_validation), TEST_DECL(test_TLSX_SRTP_msg_type_validation), + TEST_DECL(test_TLSX_SupportedCurve_empty_or_unsupported), TEST_DECL(test_wolfSSL_wolfSSL_UseSecureRenegotiation), TEST_DECL(test_wolfSSL_clear_secure_renegotiation), TEST_DECL(test_wolfSSL_SCR_Reconnect), diff --git a/tests/api/test_tls_ext.c b/tests/api/test_tls_ext.c index 294fa9c903c..c6c62d7036d 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -1033,3 +1033,87 @@ int test_TLSX_SRTP_msg_type_validation(void) #endif return EXPECT_RESULT(); } + +/* Regression test for the supported_groups (a.k.a. supported curves) parsing. + * + * A client that explicitly sends a supported_groups extension restricts the + * groups the server may use. An empty list, or a list that contains only + * groups the server does not support, must NOT be silently treated as if the + * extension was absent (which would impose no restriction and let the server + * pick an ECDHE suite/curve the client never advertised). + * + * - An empty named group list is malformed and must be rejected. + * - A list of only-unsupported groups must still leave a supported_groups + * node behind so suite selection sees the restriction. + */ +int test_TLSX_SupportedCurve_empty_or_unsupported(void) +{ + EXPECT_DECLS; +#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) && \ + defined(HAVE_SUPPORTED_CURVES) && !defined(WOLFSSL_NO_TLS12) + WOLFSSL_CTX* ctx = NULL; + WOLFSSL* ssl = NULL; + Suites* suites = NULL; + /* supported_groups (0x000a), ext len 0x0002, named_group_list len 0x0000 */ + const byte emptyList[] = { 0x00, 0x0a, 0x00, 0x02, 0x00, 0x00 }; + /* supported_groups (0x000a), ext len 0x0004, list len 0x0002, + * group 0xeeee (private-use value we do not support) */ + const byte unsupportedOnly[] = { 0x00, 0x0a, 0x00, 0x04, 0x00, 0x02, + 0xee, 0xee }; + + /* An empty named group list is malformed and must be rejected. */ + ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_2_server_method())); + ExpectNotNull(ssl = wolfSSL_new(ctx)); + if (ssl != NULL) + suites = (Suites*)WOLFSSL_SUITES(ssl); + ExpectIntEQ(TLSX_Parse(ssl, emptyList, (word16)sizeof(emptyList), + client_hello, suites), + WC_NO_ERR_TRACE(BUFFER_ERROR)); + wolfSSL_free(ssl); + ssl = NULL; + + /* A list with only unsupported groups must still record a supported_groups + * node so that ECC/ECDHE suite selection sees the (now empty) restriction + * instead of treating the extension as absent. */ + ExpectNotNull(ssl = wolfSSL_new(ctx)); + if (ssl != NULL) + suites = (Suites*)WOLFSSL_SUITES(ssl); + /* Precondition: server has not preconfigured supported groups. */ + ExpectNull(TLSX_Find(ssl->extensions, TLSX_SUPPORTED_GROUPS)); + ExpectIntEQ(TLSX_Parse(ssl, unsupportedOnly, (word16)sizeof(unsupportedOnly), + client_hello, suites), 0); + /* The fix records an (empty) supported_groups node. */ + ExpectNotNull(TLSX_Find(ssl->extensions, TLSX_SUPPORTED_GROUPS)); + wolfSSL_free(ssl); + ssl = NULL; + + wolfSSL_CTX_free(ctx); + +#if defined(WOLFSSL_TLS13) && !defined(NO_WOLFSSL_CLIENT) + /* An empty named group list is equally malformed in a TLS 1.3 + * EncryptedExtensions message (named_group_list<2..2^16-1>) and must be + * rejected with the same decode_error (BUFFER_ERROR), not silently + * accepted as the server advertising no groups. */ + { + WOLFSSL_CTX* ctx13 = NULL; + WOLFSSL* ssl13 = NULL; + const byte emptyListEE[] = { 0x00, 0x0a, 0x00, 0x02, 0x00, 0x00 }; + + ExpectNotNull(ctx13 = wolfSSL_CTX_new(wolfTLSv1_3_client_method())); + ExpectNotNull(ssl13 = wolfSSL_new(ctx13)); + /* Ensure the connection is treated as TLS 1.3 so EncryptedExtensions + * is a valid context for the extension. */ + if (ssl13 != NULL) { + ssl13->version.major = SSLv3_MAJOR; + ssl13->version.minor = TLSv1_3_MINOR; + } + ExpectIntEQ(TLSX_Parse(ssl13, emptyListEE, (word16)sizeof(emptyListEE), + encrypted_extensions, NULL), + WC_NO_ERR_TRACE(BUFFER_ERROR)); + wolfSSL_free(ssl13); + wolfSSL_CTX_free(ctx13); + } +#endif +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_tls_ext.h b/tests/api/test_tls_ext.h index e9d07d81ecd..29ad7b7c4cd 100644 --- a/tests/api/test_tls_ext.h +++ b/tests/api/test_tls_ext.h @@ -36,5 +36,6 @@ int test_TLSX_TCA_Find(void); int test_TLSX_SNI_GetSize_overflow(void); int test_TLSX_ECH_msg_type_validation(void); int test_TLSX_SRTP_msg_type_validation(void); +int test_TLSX_SupportedCurve_empty_or_unsupported(void); #endif /* TESTS_API_TEST_TLS_EMS_H */ From 9dad35a35312fa48de8dea6c76acb8bf5f2d9893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 8 Jun 2026 12:25:28 +0200 Subject: [PATCH 7/7] Check for EC_PF_UNCOMPRESSED in TLS 1.2 ClientHello Fixes F-4892 --- src/internal.c | 26 ++++++++++ src/tls.c | 20 ++++++++ tests/api.c | 1 + tests/api/test_tls.c | 103 +++++++++++++++++++++++++++++++++++++++ tests/api/test_tls.h | 5 ++ tests/api/test_tls_ext.c | 87 ++++++++++++++++++++++++++++++++- tests/api/test_tls_ext.h | 1 + wolfssl/internal.h | 9 +++- 8 files changed, 249 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 1d9c1273a32..90c73c724fc 100644 --- a/src/internal.c +++ b/src/internal.c @@ -38894,6 +38894,13 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) } #ifdef HAVE_TLS_EXTENSIONS + #ifdef HAVE_SUPPORTED_CURVES + /* Reset per-ClientHello extension state before (re)parsing so a + * stale value from an earlier handshake on this object (e.g. + * secure renegotiation, where Options is not zeroed) cannot + * trigger a spurious RFC 8422 abort below. */ + ssl->options.peerNoUncompPF = 0; + #endif /* tls extensions */ if ((ret = TLSX_Parse(ssl, input + i, totalExtSz, client_hello, ssl->clSuites))) @@ -39073,6 +39080,25 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) if (ret == 0) ret = MatchSuite(ssl, ssl->clSuites); +#if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_SUPPORTED_CURVES) + /* RFC 8422 Section 5.1.2: abort only when an ECC suite was actually + * negotiated and the client's ec_point_formats omitted the uncompressed + * (0) format (peerNoUncompPF, set in TLSX_PointFormat_Parse). Checked + * after MatchSuite so it keys off the chosen suite, not advertised + * groups. */ + if (ret == 0 && ssl->options.peerNoUncompPF && + (ssl->specs.kea == ecc_diffie_hellman_kea || + ssl->specs.kea == ecc_static_diffie_hellman_kea || + ssl->specs.kea == ecdhe_psk_kea)) { + WOLFSSL_MSG("Client ec_point_formats extension missing " + "uncompressed format for negotiated ECC suite"); + SendAlert(ssl, alert_fatal, illegal_parameter); + ret = INVALID_PARAMETER; + WOLFSSL_ERROR_VERBOSE(ret); + goto out; + } +#endif + #if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_ENCRYPT_THEN_MAC) && \ !defined(WOLFSSL_AEAD_ONLY) if (ret == 0 && ssl->options.encThenMac && diff --git a/src/tls.c b/src/tls.c index 944ff2416a7..f5c127b647f 100644 --- a/src/tls.c +++ b/src/tls.c @@ -5657,6 +5657,26 @@ static int TLSX_PointFormat_Parse(WOLFSSL* ssl, const byte* input, return BUFFER_ERROR; if (isRequest) { + #if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_SUPPORTED_CURVES) + /* RFC 8422 Section 5.1.2: a client that sends the ec_point_formats + * extension MUST include the uncompressed (0) format. Record whether + * it is missing so DoClientHello() can abort with an illegal_parameter + * alert if the client also advertised ECC named groups. The decision + * is deferred to after all extensions are parsed so it does not depend + * on the relative order of the supported_groups and ec_point_formats + * extensions in the ClientHello. */ + word16 i; + int found = 0; + + for (i = 0; i < input[0]; i++) { + if (input[ENUM_LEN + i] == WOLFSSL_EC_PF_UNCOMPRESSED) { + found = 1; + break; + } + } + ssl->options.peerNoUncompPF = (found == 0); + #endif + /* adding uncompressed point format to response */ ret = TLSX_UsePointFormat(&ssl->extensions, WOLFSSL_EC_PF_UNCOMPRESSED, ssl->heap); diff --git a/tests/api.c b/tests/api.c index 679487d12d8..927f41c1cc3 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35138,6 +35138,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_TLSX_ECH_msg_type_validation), TEST_DECL(test_TLSX_SRTP_msg_type_validation), TEST_DECL(test_TLSX_SupportedCurve_empty_or_unsupported), + TEST_DECL(test_TLSX_PointFormat_uncompressed_required), TEST_DECL(test_wolfSSL_wolfSSL_UseSecureRenegotiation), TEST_DECL(test_wolfSSL_clear_secure_renegotiation), TEST_DECL(test_wolfSSL_SCR_Reconnect), diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index aca0cc11569..088f287c5d1 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -885,6 +885,109 @@ int test_tls12_no_null_compression(void) return EXPECT_RESULT(); } +/* RFC 8422 Section 5.1.2: a client that sends an ec_point_formats extension + * omitting the uncompressed (0) format while negotiating an ECC suite must be + * rejected by the server with a fatal illegal_parameter alert. This drives a + * real handshake all the way through DoClientHello so the abort path (not just + * the parse-time detection) is exercised. + * + * Rather than hand-craft a ClientHello (which would pin the cipher suite, named + * group and exact byte offsets, making the test fragile as extension handling + * evolves), the client builds its own ClientHello and we only suppress the + * uncompressed point format: TLSX_PopulateExtensions() adds the default + * uncompressed format only when no ec_point_formats extension already exists, + * so pre-seeding the client with a compressed-only list makes it advertise + * exactly that. The curve is negotiated normally, so the test is independent of + * which named groups are enabled. */ +int test_tls12_ec_point_formats_no_uncompressed(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(WOLFSSL_NO_TLS12) \ + && defined(HAVE_ECC) && defined(HAVE_SUPPORTED_CURVES) \ + && defined(BUILD_TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) + /* Pin an ECDHE (ECC) suite so the server negotiates an ECC key exchange; + * gating on the BUILD_ macro skips the test in builds where the suite is + * unavailable (e.g. --disable-aescbc) instead of failing with + * MATCH_SUITE_ERROR. */ + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, "ECDHE-RSA-AES128-SHA"), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, "ECDHE-RSA-AES128-SHA"), + WOLFSSL_SUCCESS); + /* Make the client advertise only the compressed point format (1 == + * ansiX962_compressed_prime), i.e. omit the uncompressed (0) format. */ + ExpectIntEQ(TLSX_UsePointFormat(&ssl_c->extensions, 1, ssl_c->heap), + WOLFSSL_SUCCESS); + /* The server must reject the handshake with a fatal illegal_parameter + * alert (surfaced as INVALID_PARAMETER), not complete it. */ + ExpectIntNE(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(INVALID_PARAMETER)); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + +/* RFC 8422 Section 5.1.2 ties the missing-uncompressed-format abort to the + * server actually negotiating an ECC cipher suite. A client that omits the + * uncompressed point format but negotiates a NON-ECC suite (here DHE_RSA) must + * NOT be rejected - the handshake completes. This is the complement of + * test_tls12_ec_point_formats_no_uncompressed and guards against regressing + * back to an advertised-groups (parse-time) abort. + * + * As in that test the client builds a real ClientHello and we only suppress the + * uncompressed point format (see the comment there); the suite is pinned to a + * DHE (non-ECC) suite. */ +int test_tls12_ec_point_formats_no_uncompressed_non_ecc(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(WOLFSSL_NO_TLS12) \ + && defined(HAVE_SUPPORTED_CURVES) && !defined(NO_DH) && defined(HAVE_FFDHE) \ + && !defined(NO_RSA) && defined(BUILD_TLS_DHE_RSA_WITH_AES_128_CBC_SHA) + /* The negotiated suite must be non-ECC for the missing format to be + * irrelevant. RFC 9325 / WOLFSSL_HARDEN_TLS disables all TLS_DHE_* suites + * (NO_TLS_DH); gating on the BUILD_ macro skips the test there rather than + * failing with MATCH_SUITE_ERROR. */ + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, "DHE-RSA-AES128-SHA"), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, "DHE-RSA-AES128-SHA"), + WOLFSSL_SUCCESS); + /* Make the client advertise only the compressed point format (1 == + * ansiX962_compressed_prime), i.e. omit the uncompressed (0) format. */ + ExpectIntEQ(TLSX_UsePointFormat(&ssl_c->extensions, 1, ssl_c->heap), + WOLFSSL_SUCCESS); + /* The handshake must complete: the missing uncompressed format is + * irrelevant for a non-ECC (DHE) suite. */ + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* Sanity: the server really did observe a point-format list without the + * uncompressed format, yet proceeded. */ + ExpectIntEQ(ssl_s->options.peerNoUncompPF, 1); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + /* Test that set_curves_list correctly resolves ECC curve names that fall * through the kNistCurves table and reach the wc_ecc_get_curve_idx_from_name * fallback path. The kNistCurves lookup uses a case-sensitive XSTRNCMP, so diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 948a1e75908..f0ca46ee4e4 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -32,6 +32,8 @@ int test_tls_certreq_order(void); int test_tls12_certreq_odd_sigalgs(void); int test_tls12_bad_cv_sig_alg(void); int test_tls12_no_null_compression(void); +int test_tls12_ec_point_formats_no_uncompressed(void); +int test_tls12_ec_point_formats_no_uncompressed_non_ecc(void); int test_tls12_etm_failed_resumption(void); int test_tls_set_session_min_downgrade(void); int test_tls12_session_id_resumption_sni_mismatch(void); @@ -60,6 +62,9 @@ int test_record_size_cache_invalidated_on_renegotiation(void); TEST_DECL_GROUP("tls", test_tls12_certreq_odd_sigalgs), \ TEST_DECL_GROUP("tls", test_tls12_bad_cv_sig_alg), \ TEST_DECL_GROUP("tls", test_tls12_no_null_compression), \ + TEST_DECL_GROUP("tls", test_tls12_ec_point_formats_no_uncompressed), \ + TEST_DECL_GROUP("tls", \ + test_tls12_ec_point_formats_no_uncompressed_non_ecc), \ TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption), \ TEST_DECL_GROUP("tls", test_tls_set_session_min_downgrade), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \ diff --git a/tests/api/test_tls_ext.c b/tests/api/test_tls_ext.c index c6c62d7036d..fd7fb81fb0e 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -1049,8 +1049,14 @@ int test_TLSX_SRTP_msg_type_validation(void) int test_TLSX_SupportedCurve_empty_or_unsupported(void) { EXPECT_DECLS; -#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) && \ +#if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_TLS) && \ defined(HAVE_SUPPORTED_CURVES) && !defined(WOLFSSL_NO_TLS12) + /* This exercises the server's parsing of a received ClientHello: the + * relevant code path (TLSX_SupportedCurve_Parse) is selected by the + * message type passed to TLSX_Parse (client_hello => isRequest), not by + * the side of the WOLFSSL object. A client-side WOLFSSL is used purely as + * the parse vehicle because creating a server-side WOLFSSL would require a + * certificate to be loaded first (NO_PRIVATE_KEY otherwise). */ WOLFSSL_CTX* ctx = NULL; WOLFSSL* ssl = NULL; Suites* suites = NULL; @@ -1062,7 +1068,7 @@ int test_TLSX_SupportedCurve_empty_or_unsupported(void) 0xee, 0xee }; /* An empty named group list is malformed and must be rejected. */ - ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_2_server_method())); + ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_2_client_method())); ExpectNotNull(ssl = wolfSSL_new(ctx)); if (ssl != NULL) suites = (Suites*)WOLFSSL_SUITES(ssl); @@ -1117,3 +1123,80 @@ int test_TLSX_SupportedCurve_empty_or_unsupported(void) #endif return EXPECT_RESULT(); } + +/* RFC 8422 Section 5.1.2: a client that sends the ec_point_formats extension + * MUST include the uncompressed (0) point format. When the uncompressed format + * is omitted the server records this (ssl->options.peerNoUncompPF) during + * parsing so the handshake can be aborted with an illegal_parameter alert if + * the client also advertised ECC named groups. + * + * - A list that contains the uncompressed format must clear the flag. + * - A list that omits the uncompressed format must set the flag. + */ +int test_TLSX_PointFormat_uncompressed_required(void) +{ + EXPECT_DECLS; +#if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(NO_TLS) && defined(HAVE_SUPPORTED_CURVES) && \ + defined(HAVE_TLS_EXTENSIONS) && !defined(WOLFSSL_NO_TLS12) + /* This exercises the server's parsing of a received ClientHello: the + * relevant code path (TLSX_PointFormat_Parse) is selected by the message + * type passed to TLSX_Parse (client_hello => isRequest), not by the side + * of the WOLFSSL object. A client-side WOLFSSL is used purely as the parse + * vehicle because creating a server-side WOLFSSL would require a + * certificate to be loaded first (NO_PRIVATE_KEY otherwise). The server + * build is required because TLSX_PointFormat_Parse (the PF_PARSE dispatch + * macro) is compiled to a no-op when NO_WOLFSSL_SERVER is defined. */ + WOLFSSL_CTX* ctx = NULL; + WOLFSSL* ssl = NULL; + Suites* suites = NULL; + /* ec_point_formats (0x000b), ext len 0x0002, list len 0x01, + * format 0x00 (uncompressed) */ + const byte withUncomp[] = { 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00 }; + /* ec_point_formats (0x000b), ext len 0x0002, list len 0x01, + * format 0x01 (ansiX962_compressed_prime, uncompressed omitted) */ + const byte noUncomp[] = { 0x00, 0x0b, 0x00, 0x02, 0x01, 0x01 }; + /* As above but with two compressed formats and no uncompressed. */ + const byte noUncomp2[] = { 0x00, 0x0b, 0x00, 0x03, 0x02, 0x01, 0x02 }; + + ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_2_client_method())); + + /* A list containing the uncompressed format leaves the flag clear and + * still adds the uncompressed format to the response. */ + ExpectNotNull(ssl = wolfSSL_new(ctx)); + if (ssl != NULL) + suites = (Suites*)WOLFSSL_SUITES(ssl); + ExpectIntEQ(TLSX_Parse(ssl, withUncomp, (word16)sizeof(withUncomp), + client_hello, suites), 0); + if (ssl != NULL) + ExpectIntEQ(ssl->options.peerNoUncompPF, 0); + ExpectNotNull(TLSX_Find(ssl->extensions, TLSX_EC_POINT_FORMATS)); + wolfSSL_free(ssl); + ssl = NULL; + + /* A single-entry list that omits the uncompressed format sets the flag. */ + ExpectNotNull(ssl = wolfSSL_new(ctx)); + if (ssl != NULL) + suites = (Suites*)WOLFSSL_SUITES(ssl); + ExpectIntEQ(TLSX_Parse(ssl, noUncomp, (word16)sizeof(noUncomp), + client_hello, suites), 0); + if (ssl != NULL) + ExpectIntEQ(ssl->options.peerNoUncompPF, 1); + wolfSSL_free(ssl); + ssl = NULL; + + /* A multi-entry list that omits the uncompressed format sets the flag. */ + ExpectNotNull(ssl = wolfSSL_new(ctx)); + if (ssl != NULL) + suites = (Suites*)WOLFSSL_SUITES(ssl); + ExpectIntEQ(TLSX_Parse(ssl, noUncomp2, (word16)sizeof(noUncomp2), + client_hello, suites), 0); + if (ssl != NULL) + ExpectIntEQ(ssl->options.peerNoUncompPF, 1); + wolfSSL_free(ssl); + ssl = NULL; + + wolfSSL_CTX_free(ctx); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_tls_ext.h b/tests/api/test_tls_ext.h index 29ad7b7c4cd..56e6e010cbb 100644 --- a/tests/api/test_tls_ext.h +++ b/tests/api/test_tls_ext.h @@ -37,5 +37,6 @@ int test_TLSX_SNI_GetSize_overflow(void); int test_TLSX_ECH_msg_type_validation(void); int test_TLSX_SRTP_msg_type_validation(void); int test_TLSX_SupportedCurve_empty_or_unsupported(void); +int test_TLSX_PointFormat_uncompressed_required(void); #endif /* TESTS_API_TEST_TLS_EMS_H */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 80dcb24c701..0480491e719 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3466,7 +3466,12 @@ WOLFSSL_LOCAL int TLSX_SupportedCurve_Copy(TLSX* src, TLSX** dst, void* heap); WOLFSSL_LOCAL int TLSX_UseSupportedCurve(TLSX** extensions, word16 name, void* heap, int side); -WOLFSSL_LOCAL int TLSX_UsePointFormat(TLSX** extensions, byte point, +#ifdef WOLFSSL_API_PREFIX_MAP + #define TLSX_UsePointFormat wolfSSL_TLSX_UsePointFormat +#endif +/* WOLFSSL_TEST_VIS so the API tests can seed a client's ec_point_formats + * extension (the point-format negotiation has no public API). */ +WOLFSSL_TEST_VIS int TLSX_UsePointFormat(TLSX** extensions, byte point, void* heap); WOLFSSL_LOCAL int TLSX_IsGroupSupported(int namedGroup, int side); @@ -5197,6 +5202,8 @@ struct Options { #endif /* WOLFSSL_DTLS */ #if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_SUPPORTED_CURVES) word16 userCurves:1; /* indicates user called wolfSSL_UseSupportedCurve */ + word16 peerNoUncompPF:1; /* peer sent ec_point_formats without + * the uncompressed (0) format */ #endif word16 keepResources:1; /* Keep resources after handshake */ word16 useClientOrder:1; /* Use client's cipher order */