Add more PQC hybrid key exchange algorithms

[Frauschi]

trafficstars

Hi,

This PR adds support for all remaining hybrid PQC + ECC hybrid key exchange groups to match OQS. Next to two new combinations with SECP curves, this mainly also adds support for combinations with X25519 and X448.

This also enables compatibility with the PQC key exchange support in Chromium browsers and Mozilla Firefox (hybrid Kyber768 and X25519; when WOLFSSL_ML_KEM is not defined).

In the process of extending support, some code and logic cleanup happened. Furthermore, two memory leaks within the hybrid code path have been fixed.

Looking forward to your feedback.

[wolfSSL-Bot]

Can one of the admins verify this patch?

[dgarske]

Okay to test. Contributor agreement on file. @anhu please review. Thanks

[dgarske]

Seems to be consistently failing with:

   842: test_multiple_crls_same_issuer                      :error = -361, certificate revoked
error = -313, received alert fatal error

ERROR - tests/api.c line 7417 failed with:
    expected: test_ssl_memio_do_handshake(&test_ctx, 10, ((void *)0)) == (1)
    result:   0 != 1

error = -361, certificate revoked
error = -313, received alert fatal error

ERROR - tests/api.c line 7417 failed with:
    expected: test_ssl_memio_do_handshake(&test_ctx, 10, ((void *)0)) == (1)
    result:   0 != 1

...

Server Random : 136CBD0B1E6A36EBA50790532D5963534E14BF12E3SSL_accept error -352, Bad ECC Peer Key
wolfSSL error: SSL_accept failed
C35D79C957A2C04C2DF2C2
Client message: hello wolfssl!
I hear you fa shizzle!
trying server command line[1541]: SuiteTest -v 4 -l TLS13-AES128-GCM-SHA256 -c ./certs/server-ecc.pem -k ./certs/ecc-key.pem -Y -2 -p 0 
trying client command line[1542]: SuiteTest -v 4 -l TLS13-AES128-GCM-SHA256 -A ./certs/ca-ecc-cert.pem -y -2 -p 34205 
FAIL scripts/unit.test (exit status: 1)

[Frauschi]

Updated the PR with a fix for the failing tests.

Please be sure to test against --enable-kyber

The tests now properly work with --enable-all --enable-asn=template, --enable-all --enable-asn=template --enable-experimental --enable-kyber and --enable-all --enable-asn=template --enable-experimental --with-liboqs

Furthermore, I updated the unit tests to also test the new changes. The only thing I had to test manually are the new hybrid curves with DTLS (when also enabled with --enable-dtls13 --enable-dtls-frag-ch), as the unit tests break before even reaching those tests (pretty surely not related to this PR, as it fails equally on current master). Using the example client and server with the same arguments as the unit tests, the new curves also work with DTLS. I opened an issue for the failing DTLS tests (see #7825).

[dgarske]

Hi @Frauschi we had some issues with our GitHub CI actions. If you rebase this to latest master it should resolve the errors you are seeing. Thank you

[Frauschi]

Rebased to current master. Thanks for the hint.

FYI, I'm on vacation for the next two weeks, so any upcoming work on this will probably be delayed until afterward.

[Frauschi]

Rebased to current master (since #7807 has been merged). I also think that the one NGINX test that has been failing wasn't related to this PR's changes? Can someone retest this please?

[douzzer]

retest this please.

[JacobBarthelmeh]

This says the branch has conflicts that must be resolved. Could you rebase on top of wolfssl/master please?

[Frauschi]

This says the branch has conflicts that must be resolved. Could you rebase on top of wolfssl/master please?

Rebased to current master.

I also updated the PR to reflect the latest Code Point changes in OQS and also incorporated the new Code Points currently set in draft-kwiatkowski-tls-ecdhe-mlkem.

The only thing that is not done yet is the proposed swap of classic and PQC key material within draft-kwiatkowski-tls-ecdhe-mlkem in the key share in case of X25519 hybrids. This is done to always have a FIPS approved algorithms first in case of hybrids (and X25519 is not FIPS approved). See here.

This swap still has to be implemented. However, that will require more thorough code changes. I think I can tackle that around next week. You can decide if this should also go into this PR, or if that should go in a separate one.

[Frauschi]

I updated the PR with two new commits to implement the reversed order of X25519/X448 based hybrids, following draft-kwiatkowski-tls-ecdhe-mlkem now.

I tested compatibility with both Firefox and Google Chrome nightly, as both have support for X25519MLKEM768. When compiling with Kyber Round 3 compatibility (WOLFSSL_KYBER_ORIGINALdefined), then we are still compatible to current versions using hybrids of X25519 and Kyber Round 3 (with the old order).

For testing the Round 3 compatibility, I used liboqs as the WolfSSL implementation is not working properly currently (see #8023).

[Frauschi]

With the fixes in #8027 applied, we now also have compatibility with current versions of Chrome and Firefox when compiled with the WolfSSL Kyber implementation and with WOLFSSL_KYBER_ORIGINAL defined. (I manually patched the preferredGroup array to make sure the PQC key share is selected during the handshake, as indicated in #8024).

[Frauschi]

I updated the PR, replacing the three commits with two new ones. The first is a concatenation of the old three ones with further additions. The second one is a proposed fix for #8024.

The first commit adds the new X25519 / X448 hybrids, both for the draft and final versions of ML-KEM (based on the recent addition in #8143). This enables PQC compatibility with all current browsers.

I also updated some of the code points to reflect the latest drafts of draft-connolly-tls-mlkem-key-agreement and draft-kwiatkowski-tls-ecdhe-mlkem.

The proposed fix in the second commit is a simple but pragmatic solution for #8024. If you don't see that as a proper solution or if you prefer to handle that in a separate PR, I can drop the commit.

Tagging @SparkiDev and @ColtonWilley in addition to @anhu to also take a look at the changes.

[Frauschi]

The failing GitHub CI tests should be fixed now.

Besides the spelling, the second commit regarding the group ranking caused problems in the tests related to the WOLFSSL_STATIC_EPHEMERAL option. The change results in a different selection of a key share group in case of a HelloRetryRequest message. For the tests, where static ephemeral keys are used with the WOLFSSL_STATIC_EPHEMERAL option , an additional check is necessary to make sure the correct key is used for the ECDH calculation.

[JacobBarthelmeh]

Results of some of the tests:

[check-source-text] [2 of 7] [wolfssl]

C++-style comments:

./src/tls.c:9160:    word32   ssSzEcc = ssl->arrays->preMasterSz; // We need the buffer size for

./src/tls.c:9161:                                                 // the logic to work downwards.

./src/tls.c:9830:    word32 ssSzEcc = ssl->arrays->preMasterSz; // We need the buffer size for

./src/tls.c:9831:                                               // the logic to work downwards.

overlong lines added:

src/tls.c:8434     #if (!defined(NO_DH) || defined(HAVE_FALCON) || defined(HAVE_DILITHIUM)) && \

    check-source-text fail_E

we use the /**/ comment style and have a check for lines longer than 80 characters.

[Frauschi]

Fixed the formatting issues (comment style and overlong lines. Sorry for that.

Not sure about the socat fails, as the two failing tests seem to be about IPv4 multicast?

[JacobBarthelmeh]

Fixed the formatting issues (comment style and overlong lines. Sorry for that.

Not sure about the socat fails, as the two failing tests seem to be about IPv4 multicast?

Thanks @Frauschi . Yeah the scoat failure is unrelated to your changes. Looks like multiple pull requests are having the issue.

[dgarske]

Retest this please. PRB-master-history lost.

[Frauschi]

Rebased to current master to fix the conflicts.

[douzzer]

@Frauschi note build failure in CI testing:

Testing configuration:
--with-liboqs --enable-experimental
[...]
src/tls.c: In function ‘TLSX_KeyShare_IsSupported’:
src/tls.c:10321:43: error: passing argument 3 of ‘findEccPqc’ makes pointer from integer without a cast [-Werror=int-conversion]
10321 |             findEccPqc(NULL, &namedGroup, namedGroup);
      |                                           ^~~~~~~~~~
      |                                           |
      |                                           int
src/tls.c:8193:49: note: expected ‘int *’ but argument is of type ‘int’
 8193 | static void findEccPqc(int *ecc, int *pqc, int *pqc_first, int group)
      |                                            ~~~~~^~~~~~~~~
src/tls.c:10321:13: error: too few arguments to function ‘findEccPqc’
10321 |             findEccPqc(NULL, &namedGroup, namedGroup);
      |             ^~~~~~~~~~
src/tls.c:8193:13: note: declared here
 8193 | static void findEccPqc(int *ecc, int *pqc, int *pqc_first, int group)
      |             ^~~~~~~~~~

[Frauschi]

@dgarske @douzzer Thanks for the error logs, should both be fixed now.

[dgarske]

Okay to test. Thank you @Frauschi

[Frauschi]

Rebased to current master and fixed two minor bugs. The code now also works in case ECC is disabled with only X25519 and X448 enabled.

[dgarske]

Hi @Frauschi , can you resolve the conflicts again? I'll work on getting this PR through.

[Frauschi]

Hi @dgarske, the PR is rebased now with the conficts resolved.

I also added a small improvement to the first commit to remove the unnecessary encoding and decoding of the ML-KEM private key on the client side. The KyberKey object is now stored similarly to the ECC keys via a void*. This also enables recent caching improvements (#8436) to be usable in the TLS handshake.

[dgarske]

Retest this please: PRB needed Approve to Run. Saying no test results found.

[devin-ai-integration[bot]]

Thank you for this comprehensive PR implementing hybrid PQC key exchange algorithms. Here's my review:

Key Improvements:

1. Memory Management

• Improved KyberKey object handling with proper cleanup using wc_KyberKey_Free
• Better memory management in hybrid key processing with separate ECC/PQC private key handling
• Optimized ML-KEM private key handling via void* pointer to reduce encoding/decoding overhead

2. Browser Compatibility

• Added X25519_KYBER_LEVEL3 support for Chrome/Firefox compatibility
• Implemented pqc_first flag for proper ordering of key material
• Support for both Round 3 and final ML-KEM implementations

3. Implementation Quality

• Clean separation of PQC and classic key material handling
• Proper memory cleanup in error paths
• Consistent error handling across different key exchange modes

4. New Hybrid Combinations

• Added X25519_KYBER_LEVEL1/3 and X448_KYBER_LEVEL3
• Added X25519_ML_KEM_512/768 and X448_ML_KEM_768
• Proper handling of reversed order for X25519/X448 hybrids

Suggestions:

1. Consider adding more test coverage for error paths in hybrid key processing
2. Document the browser compatibility requirements and configurations
3. Add performance benchmarks for different hybrid combinations

Overall, this is a well-implemented addition that improves the library's post-quantum cryptography support while maintaining compatibility with existing systems.

[dgarske]

Hi @Frauschi ,

Looks like this PR adds two new macros. Are these a typo? If not please add to the .wolfssl_known_macro_extras and document them.

[check-source-text] [2 of 7] [wolfssl]
    autogen.sh wolfssl...   real 0m6.032s  user 0m4.969s  sys 0m0.130s
    configure...   real 0m7.327s  user 0m6.701s  sys 0m1.332s
unrecognized macros used:
OQS_ENABLE_KEM_KYBER
OQS_ENABLE_KEM_ML_KEM
add well-formed but unknown macros to .wolfssl_known_macro_extras

Thanks, David Garske, wolfSSL

[devin-ai-integration[bot]]

Testing Results for X25519_ML_KEM_512 with TLS13-AES256-GCM-SHA384:

@author

1. Hybrid Key Exchange:

• Successfully tested X25519_ML_KEM_512 hybrid key exchange
• Verified both classic (X25519) and PQC (ML-KEM) components
• Confirmed proper negotiation in TLS 1.3 handshake

2. Key Material Ordering:

• Tested with and without pqc_first flag
• Both orderings worked correctly
• No issues with key material processing

3. Memory Management:

• No memory leaks detected in successful handshakes
• Proper cleanup verified in error cases
• Clean termination observed in all scenarios

4. Error Handling:

• Invalid curve configuration: Proper error returned ('invalid post-quantum KEM specified')
• Cipher suite mismatch: Correct alert handling (error -313)
• Memory properly freed in error conditions

All tests passed successfully. The implementation handles both successful and error cases appropriately, with proper memory management throughout.

[anhu]

Hi @Frauschi , Please note that we will soon be removing the liboqs integration to simplify our code base.