2022149 - NSS: Wrong Elliptic Curve Used for ECDH in SecP384r1MLKEM1024 Hybrid Key Exchange (tls13con.c)

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[YUE LI]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/145.0.0.0 Safari/537.36

Steps to reproduce:

During a systematic audit of hybrid post-quantum cryptography
implementations, we reviewed the TLS 1.3 hybrid key exchange
code in NSS (lib/ssl/tls13con.c).

In the tls13_HandleKeyShare function (line 970), the switch
statement at line 1022 handles the SecP384r1MLKEM1024 hybrid
group (0x11ED). This case correctly sets ec_len to
SECP384_PUBLIC_KEY_BYTES (97 bytes) but incorrectly assigns
ecGroup to P-256 instead of P-384(line 1028):

case ssl_grp_kem_secp384r1mlkem1024:
    ec_len = SECP384_PUBLIC_KEY_BYTES;
    ec_data = entry->key_exchange.len < ec_len
                  ? NULL
                  : entry->key_exchange.data;
    ecGroup = ssl_LookupNamedGroup(ssl_grp_ec_secp256r1);
    break;
    // should be ssl_grp_ec_secp384r1 (line 1028)

The bug was introduced in commit cd2ce1a240 (2025-10-28) by
Robert Relyea, "Bug 1983313 support secp384r1mlkem1024",
Phabricator D265043.

To reproduce: configure an NSS client or server to negotiate
SecP384r1MLKEM1024 (NamedGroup 0x11ED) in a TLS 1.3
handshake. Both sides share the same tls13_HandleKeyShare
function, so both client and server are affected.

Source: https://github.com/mozilla-firefox/firefox/blob/021cb4edd621e1f5d60ed62409587d6cf1d02845/security/nss/lib/ssl/tls13con.c#L1028

Actual results:

ssl_ImportECDHKeyShare receives a 97-byte P-384 public key
point but sets P-256 curve OID via ssl_NamedGroup2ECParams.
The function does not validate point length vs curve, so it
returns SECSuccess with inconsistent data: P-256 DER
parameters + 97 bytes of P-384 point data.

PK11_PubDeriveWithKDF (line 1067) then attempts ECDH using
the local P-384 private key against a peerKey containing
P-256 curve parameters with 97 bytes of P-384 data.

Most likely outcome: PKCS#11 softtoken detects the curve
mismatch, handshake fails with SSL_ERROR_KEY_EXCHANGE_FAILURE.
SecP384r1MLKEM1024 is completely non-functional.

Worst-case outcome: if softtoken does not validate that the
point lies on the specified curve, ECDH is computed with
mismatched parameters, potentially producing a weak shared
secret or leaking private key information through
invalid-curve arithmetic.

The other three hybrid groups are NOT affected:
xyber768d00: ecGroup = curve25519 correct
mlkem768x25519: ecGroup = curve25519 correct
secp256r1mlkem768: ecGroup = secp256r1 correct

Expected results:

The secp384r1mlkem1024 case should use ssl_grp_ec_secp384r1:

case ssl_grp_kem_secp384r1mlkem1024:
    ec_len = SECP384_PUBLIC_KEY_BYTES;
    ec_data = ...;

•   ecGroup = ssl_LookupNamedGroup(ssl_grp_ec_secp256r1);
  

•   ecGroup = ssl_LookupNamedGroup(ssl_grp_ec_secp384r1);
    break;
  

With this fix, ECDH would be computed on the correct P-384
curve, and the SecP384r1MLKEM1024 Level V hybrid handshake
would function as intended.

Additionally, ssl_ImportECDHKeyShare should validate that the
point length matches the curve as defense-in-depth.

CWE: CWE-687 (Function Call With Incorrectly Specified
Arguments)

This issue was discovered during a joint PKU-SJTU research
project auditing hybrid PQC implementations across
open-source projects. We have not publicly disclosed this
issue.

Comment 1

[John Schanck [:jschanck]]

Thanks for this report! The negative outcomes you suggest do not actually occur because the peer's public value is passed to the PKCS#11 C_Derive function in a CK_ECDH1_DERIVE_PARAMS structure:

typedef struct CK_ECDH1_DERIVE_PARAMS {
   CK_EC_KDF_TYPE  kdf;
   CK_ULONG       ulSharedDataLen;
   CK_BYTE_PTR     pSharedData;
   CK_ULONG       ulPublicDataLen;
   CK_BYTE_PTR     pPublicData;
}  CK_ECDH1_DERIVE_PARAMS;

Notably, the peer's curve parameters are not passed. The token performs point validation based on the parameters of the private key which is being used for the other DH input. The incorrectly set peerKey->u.ec.DEREncodedParams is completely unused in this code path.

Comment 2

[John Schanck [:jschanck]]

Attachment: Bug 2022149 - set correct value of unused curve parameters in tls13_HandleKeyShare. r=#nss-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/nss/rev/5d633899b065
set correct value of unused curve parameters in tls13_HandleKeyShare. r=nss-reviewers,keeler