From 11270fc46569869fa446914f23763140a1458e62 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] Check for EC_PF_UNCOMPRESSED in TLS 1.2 ClientHello Fixes F-4892 --- src/internal.c | 27 ++++++++++ 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 | 79 +++++++++++++++++++++++++++++- tests/api/test_tls_ext.h | 1 + wolfssl/internal.h | 9 +++- 8 files changed, 243 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 1d9c1273a3..16869c0bcc 100644 --- a/src/internal.c +++ b/src/internal.c @@ -38861,6 +38861,14 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) *inOutIdx = i; +#if defined(HAVE_TLS_EXTENSIONS) && defined(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 ((i - begin) < helloSz) { #ifdef HAVE_TLS_EXTENSIONS @@ -39073,6 +39081,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 18a8024a24..2fa6abd611 100644 --- a/src/tls.c +++ b/src/tls.c @@ -5667,6 +5667,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 0ec468a2a0..55f8f0f3d0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35142,6 +35142,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_TLSX_SRTP_msg_type_validation), TEST_DECL(test_TLSX_ALPN_server_response_count), 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 1bbe8f2e26..a0635675c0 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 cdda231bf5..79d6f18dc8 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); @@ -61,6 +63,9 @@ int test_wolfSSL_get_shared_ciphers(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 bcb9d81407..781042a4ce 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -1108,7 +1108,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); @@ -1163,3 +1163,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 5bbd8ffc27..00499de3b0 100644 --- a/tests/api/test_tls_ext.h +++ b/tests/api/test_tls_ext.h @@ -38,5 +38,6 @@ int test_TLSX_ECH_msg_type_validation(void); int test_TLSX_SRTP_msg_type_validation(void); int test_TLSX_ALPN_server_response_count(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 80dcb24c70..0480491e71 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 */