diff --git a/src/tls.c b/src/tls.c index 411a97271e..18a8024a24 100644 --- a/src/tls.c +++ b/src/tls.c @@ -5232,8 +5232,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) { @@ -5250,6 +5255,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 9cac424dca..0ec468a2a0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35141,6 +35141,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_TLSX_ECH_msg_type_validation), 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_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 49edaf29f5..bcb9d81407 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -1071,3 +1071,95 @@ int test_TLSX_ALPN_server_response_count(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_CLIENT) && !defined(NO_TLS) && \ + defined(HAVE_SUPPORTED_CURVES) && !defined(WOLFSSL_NO_TLS12) && \ + (!defined(NO_WOLFSSL_SERVER) || (defined(WOLFSSL_TLS13) && \ + !defined(WOLFSSL_NO_SERVER_GROUPS_EXT))) + /* 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; + /* 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 dc617e2d62..5bbd8ffc27 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_ALPN_server_response_count(void); +int test_TLSX_SupportedCurve_empty_or_unsupported(void); #endif /* TESTS_API_TEST_TLS_EMS_H */