diff --git a/src/internal.c b/src/internal.c index 2aead00269..dda3dcb3d2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -35383,16 +35383,14 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, int NamedGroupIsPqc(int group) { switch (group) { - #ifndef WOLFSSL_NO_ML_KEM + /* FIPS 204 ML-KEM */ case WOLFSSL_ML_KEM_512: case WOLFSSL_ML_KEM_768: case WOLFSSL_ML_KEM_1024: - #endif - #ifdef WOLFSSL_MLKEM_KYBER + /* Kyber Round 3 */ case WOLFSSL_KYBER_LEVEL1: case WOLFSSL_KYBER_LEVEL3: case WOLFSSL_KYBER_LEVEL5: - #endif return 1; default: return 0; @@ -35402,17 +35400,12 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* Returns 1 when the given group is a PQC hybrid group, 0 otherwise. */ int NamedGroupIsPqcHybrid(int group) { - #if defined(WOLFSSL_PQC_HYBRIDS) || defined(WOLFSSL_EXTRA_PQC_HYBRIDS) || \ - defined(WOLFSSL_MLKEM_KYBER) - switch (group) { - #ifndef WOLFSSL_NO_ML_KEM - #ifdef WOLFSSL_PQC_HYBRIDS + /* Standardized hybrids */ case WOLFSSL_SECP256R1MLKEM768: case WOLFSSL_X25519MLKEM768: case WOLFSSL_SECP384R1MLKEM1024: - #endif /* WOLFSSL_PQC_HYBRIDS */ - #ifdef WOLFSSL_EXTRA_PQC_HYBRIDS + /* Additional experimental hybrids */ case WOLFSSL_SECP256R1MLKEM512: case WOLFSSL_SECP384R1MLKEM768: case WOLFSSL_SECP521R1MLKEM1024: @@ -35423,9 +35416,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, case WOLFSSL_P384_ML_KEM_768_OLD: case WOLFSSL_P521_ML_KEM_1024_OLD: #endif - #endif /* WOLFSSL_EXTRA_PQC_HYBRIDS */ - #endif - #ifdef WOLFSSL_MLKEM_KYBER + /* Kyber round 3 hybrids */ case WOLFSSL_P256_KYBER_LEVEL3: case WOLFSSL_X25519_KYBER_LEVEL3: case WOLFSSL_P256_KYBER_LEVEL1: @@ -35433,15 +35424,10 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, case WOLFSSL_P521_KYBER_LEVEL5: case WOLFSSL_X25519_KYBER_LEVEL1: case WOLFSSL_X448_KYBER_LEVEL3: - #endif return 1; default: return 0; } - #else - (void)group; - return 0; - #endif } #endif /* WOLFSSL_HAVE_MLKEM */ diff --git a/src/tls.c b/src/tls.c index ed8267469d..2217c6624e 100644 --- a/src/tls.c +++ b/src/tls.c @@ -9164,16 +9164,18 @@ static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap) wc_curve448_free((curve448_key*)current->key); #endif } -#ifdef WOLFSSL_HAVE_MLKEM else if (WOLFSSL_NAMED_GROUP_IS_PQC(current->group)) { +#ifdef WOLFSSL_HAVE_MLKEM wc_KyberKey_Free((KyberKey*)current->key); #ifndef WOLFSSL_TLSX_PQC_MLKEM_STORE_OBJ if (current->privKey != NULL) { ForceZero(current->privKey, current->privKeyLen); } #endif +#endif } else if (WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(current->group)) { +#ifdef WOLFSSL_HAVE_MLKEM int ecc_group = 0; findEccPqc(&ecc_group, NULL, NULL, current->group); @@ -9211,8 +9213,8 @@ static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap) wc_ecc_free((ecc_key*)current->key); #endif } - } #endif + } else { #ifdef HAVE_ECC #if defined(WC_ECC_NONBLOCK) && defined(WOLFSSL_ASYNC_CRYPT_SW) && \ @@ -10373,42 +10375,16 @@ static int TLSX_KeyShareEntry_Parse(const WOLFSSL* ssl, const byte* input, *seenGroupsCnt = i + 1; } -#if defined(WOLFSSL_HAVE_MLKEM) - if ((WOLFSSL_NAMED_GROUP_IS_PQC(group) - #if !defined(WOLFSSL_ASYNC_CRYPT) - || WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(group) - #endif - ) && ssl->options.side == WOLFSSL_SERVER_END) { - /* When handling a key share containing a KEM public key on the server - * end, we have to perform the encapsulation immediately in order to - * send the resulting ciphertext back to the client in the ServerHello - * message. As the public key is not stored and we do not modify it, we - * don't have to create a copy of it. - * In case of a hybrid key exchange, the ECDH part is also performed - * immediately (to not split the generation of the master secret). - * Hence, we also don't have to store this public key either. - * - * When WOLFSSL_ASYNC_CRYPT is enabled, this handling is not possible - * for the hybrid case, as the ECC part is performed asynchronously, - * requiring the key share data to be stored. - */ - ke = (byte *)&input[offset]; - } else -#endif - { - /* Store a copy in the key share object. */ - ke = (byte*)XMALLOC(keLen, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); - if (ke == NULL) - return MEMORY_E; - XMEMCPY(ke, &input[offset], keLen); - } + /* Store a copy in the key share object. */ + ke = (byte*)XMALLOC(keLen, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); + if (ke == NULL) + return MEMORY_E; + XMEMCPY(ke, &input[offset], keLen); /* Populate a key share object in the extension. */ ret = TLSX_KeyShare_Use(ssl, group, keLen, ke, kse, extensions); if (ret != 0) { - if (ke != &input[offset]) { - XFREE(ke, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); - } + XFREE(ke, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); return ret; } @@ -11135,46 +11111,8 @@ int TLSX_KeyShare_Use(const WOLFSSL* ssl, word16 group, word16 len, byte* data, return ret; } - -#if defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) - if (ssl->options.side == WOLFSSL_SERVER_END && - WOLFSSL_NAMED_GROUP_IS_PQC(group)) { - if (TLSX_IsGroupSupported(group)) { - ret = TLSX_KeyShare_HandlePqcKeyServer((WOLFSSL*)ssl, - keyShareEntry, - data, len, - ssl->arrays->preMasterSecret, - &ssl->arrays->preMasterSz); - if (ret != 0) - return ret; - } - else { - XFREE(keyShareEntry->ke, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); - keyShareEntry->ke = NULL; - keyShareEntry->keLen = 0; - } - } - else -#if !defined(WOLFSSL_ASYNC_CRYPT) - if (ssl->options.side == WOLFSSL_SERVER_END && - WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(group)) { - if (TLSX_IsGroupSupported(group)) { - ret = TLSX_KeyShare_HandlePqcHybridKeyServer((WOLFSSL*)ssl, - keyShareEntry, - data, len); - if (ret != 0) - return ret; - } - else { - XFREE(keyShareEntry->ke, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); - keyShareEntry->ke = NULL; - keyShareEntry->keLen = 0; - } - } - else -#endif -#endif if (data != NULL) { + /* Store the peer data in the key share object. */ XFREE(keyShareEntry->ke, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY); keyShareEntry->ke = data; keyShareEntry->keLen = len; @@ -11362,7 +11300,10 @@ static int TLSX_KeyShare_GroupRank(const WOLFSSL* ssl, int group) byte numGroups; if (ssl->numGroups == 0) { - return 0; + /* If the user didn't specify a group list with a preferred order, + * use the internal preferred group list. */ + groups = preferredGroup; + numGroups = PREFERRED_GROUP_SZ; } else { groups = ssl->group; @@ -11664,9 +11605,7 @@ int TLSX_KeyShare_Choose(const WOLFSSL *ssl, TLSX* extensions, /* Use server's preference order. */ for (clientKSE = list; clientKSE != NULL; clientKSE = clientKSE->next) { - if ((clientKSE->ke == NULL) && - (!WOLFSSL_NAMED_GROUP_IS_PQC(clientKSE->group)) && - (!WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(clientKSE->group))) + if (clientKSE->ke == NULL) continue; #ifdef WOLFSSL_SM2 @@ -11755,26 +11694,17 @@ int TLSX_KeyShare_Setup(WOLFSSL *ssl, KeyShareEntry* clientKSE) return ret; if (clientKSE->key == NULL) { -#ifdef WOLFSSL_HAVE_MLKEM - if (WOLFSSL_NAMED_GROUP_IS_PQC(clientKSE->group) - #if !defined(WOLFSSL_ASYNC_CRYPT) - || WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(clientKSE->group) - #endif - ) { - /* Going to need the public key (AKA ciphertext). */ - serverKSE->pubKey = clientKSE->pubKey; - clientKSE->pubKey = NULL; - serverKSE->pubKeyLen = clientKSE->pubKeyLen; - clientKSE->pubKeyLen = 0; +#if defined(WOLFSSL_HAVE_MLKEM) && !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) + if (WOLFSSL_NAMED_GROUP_IS_PQC(clientKSE->group)) { + ret = TLSX_KeyShare_HandlePqcKeyServer(ssl, serverKSE, + clientKSE->ke, clientKSE->keLen, + ssl->arrays->preMasterSecret, &ssl->arrays->preMasterSz); } - else - #if defined(WOLFSSL_ASYNC_CRYPT) - if (WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(clientKSE->group)) { + else if (WOLFSSL_NAMED_GROUP_IS_PQC_HYBRID(clientKSE->group)) { ret = TLSX_KeyShare_HandlePqcHybridKeyServer(ssl, serverKSE, clientKSE->ke, clientKSE->keLen); } else - #endif #endif { ret = TLSX_KeyShare_GenKey(ssl, serverKSE); diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 194ea6e432..82818abbf5 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -2667,6 +2667,101 @@ int test_tls13_pq_groups(void) return EXPECT_RESULT(); } +/* Regression test handling multiple PQC key shares in the ClientHello. + * + * Previously, the server eagerly ran KEM encapsulation on every PQC/hybrid + * key_share entry while parsing the ClientHello, clobbering + * ssl->arrays->preMasterSecret with whichever entry was parsed last. + * When the ClientHello offers both SecP384R1_MLKEM1024 and pure + * ML_KEM_1024, the resulting handshake either produces keys that the + * client cannot decrypt (hybrid chosen, pure-ML-KEM secret written) or + * trips a BUFFER_E inside the second encapsulation. Either ordering + * causes the handshake to fail. + * + * The test runs a memio TLS 1.3 handshake for both orderings and + * expects the handshake to complete successfully with the hybrid group + * selected (higher server preference rank). */ +#if defined(WOLFSSL_TLS13) && defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + defined(WOLFSSL_HAVE_MLKEM) && defined(WOLFSSL_PQC_HYBRIDS) && \ + !defined(WOLFSSL_TLS_NO_MLKEM_STANDALONE) && \ + !defined(WOLFSSL_NO_ML_KEM_1024) && \ + !defined(WOLFSSL_MLKEM_NO_ENCAPSULATE) && \ + !defined(WOLFSSL_MLKEM_NO_DECAPSULATE) && \ + !defined(WOLFSSL_MLKEM_NO_MAKE_KEY) && \ + defined(HAVE_ECC) && (defined(HAVE_ECC384) || defined(HAVE_ALL_CURVES)) && \ + ECC_MIN_KEY_SZ <= 384 && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) +#define TEST_TLS13_MULTI_PQC_KEY_SHARE_ENABLED + +/* Run one TLS 1.3 memio handshake where the client offers both + * WOLFSSL_SECP384R1MLKEM1024 and WOLFSSL_ML_KEM_1024 in the key_share + * extension, in the order dictated by `hybridFirst`. */ +static int test_tls13_multi_pqc_key_share_once(int hybridFirst) +{ + EXPECT_DECLS; + WOLFSSL_CTX *ctx_c = NULL; + WOLFSSL_CTX *ctx_s = NULL; + WOLFSSL *ssl_c = NULL; + WOLFSSL *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_3_client_method, wolfTLSv1_3_server_method), 0); + + wolfSSL_set_verify(ssl_c, WOLFSSL_VERIFY_NONE, NULL); + wolfSSL_set_verify(ssl_s, WOLFSSL_VERIFY_NONE, NULL); + + /* Force the client to include both PQC key shares in the ClientHello + * by calling UseKeyShare twice. The order of the UseKeyShare calls + * determines the order of the entries in the key_share extension. */ + if (hybridFirst) { + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_SECP384R1MLKEM1024), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ML_KEM_1024), + WOLFSSL_SUCCESS); + } + else { + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_ML_KEM_1024), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseKeyShare(ssl_c, WOLFSSL_SECP384R1MLKEM1024), + WOLFSSL_SUCCESS); + } + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* The server ranks SecP384R1_MLKEM1024 higher than ML_KEM_1024, so + * the hybrid group must be selected regardless of client ordering. */ + ExpectStrEQ(wolfSSL_get_curve_name(ssl_s), "SecP384r1MLKEM1024"); + ExpectStrEQ(wolfSSL_get_curve_name(ssl_c), "SecP384r1MLKEM1024"); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); + return EXPECT_RESULT(); +} +#endif /* TEST_TLS13_MULTI_PQC_KEY_SHARE_ENABLED */ + +int test_tls13_multi_pqc_key_share(void) +{ + EXPECT_DECLS; +#ifdef TEST_TLS13_MULTI_PQC_KEY_SHARE_ENABLED + /* Hybrid first, then pure ML-KEM: pre-fix the server selected the + * hybrid but had overwritten preMasterSecret with the pure-KEM + * result, producing 32-byte KE Secret instead of 80 and causing the + * client to fail to decrypt the server's first encrypted record. */ + ExpectIntEQ(test_tls13_multi_pqc_key_share_once(1), TEST_SUCCESS); + + /* Pure ML-KEM first, then hybrid: pre-fix the server tripped + * BUFFER_E inside the second encapsulation because preMasterSz was + * left at 32 from the first call, and the hybrid handler then + * overflowed the preMasterSecret buffer. */ + ExpectIntEQ(test_tls13_multi_pqc_key_share_once(0), TEST_SUCCESS); +#endif + return EXPECT_RESULT(); +} + #if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ defined(WOLFSSL_EARLY_DATA) && defined(HAVE_SESSION_TICKET) static int test_tls13_read_until_write_ok(WOLFSSL* ssl, void* buf, int bufLen) diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index c8b42fc56c..c6f5942e72 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -29,6 +29,7 @@ int test_tls13_cipher_suites(void); int test_tls13_bad_psk_binder(void); int test_tls13_rpk_handshake(void); int test_tls13_pq_groups(void); +int test_tls13_multi_pqc_key_share(void); int test_tls13_early_data(void); int test_tls13_same_ch(void); int test_tls13_hrr_different_cs(void); @@ -67,6 +68,7 @@ int test_tls13_cert_with_extern_psk_sh_confirms_resumption(void); TEST_DECL_GROUP("tls13", test_tls13_bad_psk_binder), \ TEST_DECL_GROUP("tls13", test_tls13_rpk_handshake), \ TEST_DECL_GROUP("tls13", test_tls13_pq_groups), \ + TEST_DECL_GROUP("tls13", test_tls13_multi_pqc_key_share), \ TEST_DECL_GROUP("tls13", test_tls13_early_data), \ TEST_DECL_GROUP("tls13", test_tls13_same_ch), \ TEST_DECL_GROUP("tls13", test_tls13_hrr_different_cs), \ diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 236515157b..abf2f0f5c8 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -4766,9 +4766,8 @@ enum { WOLFSSL_FFDHE_8192 = 260, WOLFSSL_FFDHE_END = 511, -#ifdef HAVE_PQC + /* Post Quantum KEM code points */ -#ifdef WOLFSSL_MLKEM_KYBER /* Old code points to keep compatibility with Kyber Round 3. * Taken from OQS's openssl provider, see: * https://github.com/open-quantum-safe/oqs-provider/blob/main/oqs-template/ @@ -4785,8 +4784,7 @@ enum { WOLFSSL_X448_KYBER_LEVEL3 = 12176, WOLFSSL_X25519_KYBER_LEVEL3 = 25497, WOLFSSL_P256_KYBER_LEVEL3 = 25498, -#endif /* WOLFSSL_MLKEM_KYBER */ -#ifndef WOLFSSL_NO_ML_KEM + /* Taken from draft-ietf-tls-mlkem, see: * https://datatracker.ietf.org/doc/draft-ietf-tls-mlkem/ */ @@ -4815,8 +4813,7 @@ enum { WOLFSSL_SECP521R1MLKEM1024 = 12109, WOLFSSL_X25519MLKEM512 = 12214, WOLFSSL_X448MLKEM768 = 12215, -#endif /* WOLFSSL_NO_ML_KEM */ -#endif /* HAVE_PQC */ + WOLF_ENUM_DUMMY_LAST_ELEMENT(SSL_H) };