Small refactoring of named group handling for readability and de-duplicating of code.

- add wolfSSL_CTX_curve_is_disabled() and wolfSSL_curve_is_disabled() to have common checks on wether a curve has been disabled by user - add macros returning 0 for above function when OPENSSL_EXTRA is not defined, enabling use without #fidef check - add macros for checking if named groups are in a certain range WOLFSSL_NAMED_GROUP_IS_FFHDE() WOLFSSL_NAMED_GROUP_IS_PQC() Fixed QuicTransportParam_free() use without case when compiling with c++.
2025-07-30 02:37:28 +02:00 · 2022-08-17 11:46:48 +02:00
parent 3f7d56d957
commit d6548b6b88
5 changed files with 102 additions and 40 deletions
--- a/src/ssl.c
+++ b/src/ssl.c
@ -33904,6 +33904,22 @@ void wolfSSL_get0_next_proto_negotiated(const WOLFSSL *s, const unsigned char **

 #endif /* WOLFSSL_NGINX  / WOLFSSL_HAPROXY */

+#ifdef OPENSSL_EXTRA
+int wolfSSL_CTX_curve_is_disabled(WOLFSSL_CTX* ctx, word16 named_curve)
+{
+    return (named_curve <= WOLFSSL_ECC_MAX &&
+            ctx->disabledCurves &&
+            ctx->disabledCurves & (1 << named_curve));
+}
+
+int wolfSSL_curve_is_disabled(WOLFSSL* ssl, word16 named_curve)
+{
+    /* FIXME: see wolfSSL_set1_curves_list() below on why
+     * this dependency on ssl->ctx alone is insufficient. */
+    return wolfSSL_CTX_curve_is_disabled(ssl->ctx, named_curve);
+}
+#endif
+
 #if defined(OPENSSL_EXTRA) && (defined(HAVE_ECC) || \
    defined(HAVE_CURVE25519) || defined(HAVE_CURVE448))
 int wolfSSL_CTX_set1_curves_list(WOLFSSL_CTX* ctx, const char* names)
@ -34011,6 +34027,8 @@ int wolfSSL_set1_curves_list(WOLFSSL* ssl, const char* names)
    if (ssl == NULL) {
        return WOLFSSL_FAILURE;
    }
+    /* FIXME: this manipulates the context from a WOLFSSL* and
+     * will lead to surprises for some. */
    return wolfSSL_CTX_set1_curves_list(ssl->ctx, names);
 }
 #endif /* OPENSSL_EXTRA && (HAVE_ECC || HAVE_CURVE25519 || HAVE_CURVE448) */
--- a/src/tls.c
+++ b/src/tls.c
@ -4183,8 +4183,7 @@ static int tlsx_ffdhe_find_group(WOLFSSL* ssl, SupportedCurve* clientGroup,
    const DhParams* params = NULL;

    for (; serverGroup != NULL; serverGroup = serverGroup->next) {
-        if (serverGroup->name < MIN_FFHDE_GROUP ||
-            serverGroup->name > MAX_FFHDE_GROUP)
+        if (!WOLFSSL_NAMED_GROUP_IS_FFHDE(serverGroup->name))
            continue;

        for (group = clientGroup; group != NULL; group = group->next) {
@ -4261,8 +4260,7 @@ static int tlsx_ffdhe_find_group(WOLFSSL* ssl, SupportedCurve* clientGroup,
    word32 p_len;

    for (; serverGroup != NULL; serverGroup = serverGroup->next) {
-        if (serverGroup->name < MIN_FFHDE_GROUP ||
-            serverGroup->name > MAX_FFHDE_GROUP)
+        if (!WOLFSSL_NAMED_GROUP_IS_FFHDE(serverGroup->name))
            continue;

        for (group = clientGroup; group != NULL; group = group->next) {
@ -4367,7 +4365,7 @@ int TLSX_SupportedFFDHE_Set(WOLFSSL* ssl)
        return 0;
    clientGroup = (SupportedCurve*)extension->data;
    for (group = clientGroup; group != NULL; group = group->next) {
-        if (group->name >= MIN_FFHDE_GROUP && group->name <= MAX_FFHDE_GROUP) {
+        if (WOLFSSL_NAMED_GROUP_IS_FFHDE(group->name)) {
            found = 1;
            break;
        }
@ -4495,11 +4493,10 @@ int TLSX_ValidateSupportedCurves(WOLFSSL* ssl, byte first, byte second) {
         curve = curve->next) {

    #ifdef OPENSSL_EXTRA
-        /* skip if name is not in supported ECC range */
-        if (curve->name > WOLFSSL_ECC_X448)
-            continue;
-        /* skip if curve is disabled by user */
-        if (ssl->ctx->disabledCurves & (1 << curve->name))
+        /* skip if name is not in supported ECC range
+         * or disabled by user */
+        if (curve->name > WOLFSSL_ECC_MAX ||
+            wolfSSL_curve_is_disabled(ssl, curve->name))
            continue;
    #endif

@ -7356,14 +7353,14 @@ static int TLSX_KeyShare_GenKey(WOLFSSL *ssl, KeyShareEntry *kse)
 {
    int ret;
    /* Named FFDHE groups have a bit set to identify them. */
-    if (kse->group >= MIN_FFHDE_GROUP && kse->group <= MAX_FFHDE_GROUP)
+    if (WOLFSSL_NAMED_GROUP_IS_FFHDE(kse->group))
        ret = TLSX_KeyShare_GenDhKey(ssl, kse);
    else if (kse->group == WOLFSSL_ECC_X25519)
        ret = TLSX_KeyShare_GenX25519Key(ssl, kse);
    else if (kse->group == WOLFSSL_ECC_X448)
        ret = TLSX_KeyShare_GenX448Key(ssl, kse);
 #ifdef HAVE_PQC
-    else if (kse->group >= WOLFSSL_PQC_MIN && kse->group <= WOLFSSL_PQC_MAX)
+    else if (WOLFSSL_NAMED_GROUP_IS_PQC(kse->group))
        ret = TLSX_KeyShare_GenPqcKey(ssl, kse);
 #endif
    else
@ -7385,8 +7382,7 @@ static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap)

    while ((current = list) != NULL) {
        list = current->next;
-        if (current->group >= MIN_FFHDE_GROUP &&
-            current->group <= MAX_FFHDE_GROUP) {
+        if (WOLFSSL_NAMED_GROUP_IS_FFHDE(current->group)) {
 #ifndef NO_DH
            wc_FreeDhKey((DhKey*)current->key);
 #endif
@ -7402,8 +7398,7 @@ static void TLSX_KeyShare_FreeAll(KeyShareEntry* list, void* heap)
 #endif
        }
 #ifdef HAVE_PQC
-        else if (current->group >= WOLFSSL_PQC_MIN &&
-                 current->group <= WOLFSSL_PQC_MAX &&
+        else if (WOLFSSL_NAMED_GROUP_IS_PQC(current->group) &&
                 current->key != NULL) {
            ForceZero((byte*)current->key, current->keyLen);
        }
@ -8251,16 +8246,14 @@ static int TLSX_KeyShare_Process(WOLFSSL* ssl, KeyShareEntry* keyShareEntry)
        ssl->arrays->preMasterSz = ENCRYPT_LEN;

    /* Use Key Share Data from server. */
-    if (keyShareEntry->group >= MIN_FFHDE_GROUP &&
-        keyShareEntry->group <= MAX_FFHDE_GROUP)
+    if (WOLFSSL_NAMED_GROUP_IS_FFHDE(keyShareEntry->group))
        ret = TLSX_KeyShare_ProcessDh(ssl, keyShareEntry);
    else if (keyShareEntry->group == WOLFSSL_ECC_X25519)
        ret = TLSX_KeyShare_ProcessX25519(ssl, keyShareEntry);
    else if (keyShareEntry->group == WOLFSSL_ECC_X448)
        ret = TLSX_KeyShare_ProcessX448(ssl, keyShareEntry);
 #ifdef HAVE_PQC
-    else if (keyShareEntry->group >= WOLFSSL_PQC_MIN &&
-             keyShareEntry->group <= WOLFSSL_PQC_MAX)
+    else if (WOLFSSL_NAMED_GROUP_IS_PQC(keyShareEntry->group))
        ret = TLSX_KeyShare_ProcessPqc(ssl, keyShareEntry);
 #endif
    else
@ -8311,8 +8304,7 @@ static int TLSX_KeyShareEntry_Parse(WOLFSSL* ssl, const byte* input,
        return BUFFER_ERROR;

 #ifdef HAVE_PQC
-    if (group >= WOLFSSL_PQC_MIN &&
-        group <= WOLFSSL_PQC_MAX &&
+    if (WOLFSSL_NAMED_GROUP_IS_PQC(group) &&
        ssl->options.side == WOLFSSL_SERVER_END) {
        /* For KEMs, the public key is not stored. Casting away const because
         * we know for KEMs, it will be read-only.*/
@ -8526,7 +8518,7 @@ static int TLSX_KeyShare_Parse(WOLFSSL* ssl, const byte* input, word16 length,

 #ifdef HAVE_PQC
        /* For post-quantum groups, do this in TLSX_PopulateExtensions(). */
-        if (group < WOLFSSL_PQC_MIN || group > WOLFSSL_PQC_MAX)
+        if (!WOLFSSL_NAMED_GROUP_IS_PQC(group))
 #endif
            ret = TLSX_KeyShare_Use(ssl, group, 0, NULL, NULL);
    }
@ -8907,8 +8899,7 @@ int TLSX_KeyShare_Use(WOLFSSL* ssl, word16 group, word16 len, byte* data,


 #ifdef HAVE_PQC
-    if (group >= WOLFSSL_PQC_MIN &&
-        group <= WOLFSSL_PQC_MAX &&
+    if (WOLFSSL_NAMED_GROUP_IS_PQC(group) &&
        ssl->options.side == WOLFSSL_SERVER_END) {
        ret = server_generate_pqc_ciphertext(ssl, keyShareEntry, data,
                                             len);
@ -9261,6 +9252,8 @@ static int TLSX_KeyShare_SetSupported(WOLFSSL* ssl)
    for (; curve != NULL; curve = curve->next) {
        if (!TLSX_KeyShare_IsSupported(curve->name))
            continue;
+        if (wolfSSL_curve_is_disabled(ssl, curve->name))
+            continue;

        rank = TLSX_KeyShare_GroupRank(ssl, curve->name);
        if (rank == -1)
@ -9355,21 +9348,16 @@ int TLSX_KeyShare_Establish(WOLFSSL *ssl, int* doHelloRetry)
        if (!TLSX_SupportedGroups_Find(ssl, clientKSE->group))
            continue;

-        if (clientKSE->group < MIN_FFHDE_GROUP ||
-            clientKSE->group > MAX_FFHDE_GROUP) {
+        if (!WOLFSSL_NAMED_GROUP_IS_FFHDE(clientKSE->group)) {
            /* Check max value supported. */
            if (clientKSE->group > WOLFSSL_ECC_MAX) {
 #ifdef HAVE_PQC
-                if (clientKSE->group < WOLFSSL_PQC_MIN ||
-                    clientKSE->group > WOLFSSL_PQC_MAX )
+                if (!WOLFSSL_NAMED_GROUP_IS_PQC(clientKSE->group))
 #endif
                    continue;
            }
-        #ifdef OPENSSL_EXTRA
-            /* Check if server supports group. */
-            if (ssl->ctx->disabledCurves & ((word32)1 << clientKSE->group))
+            if (wolfSSL_curve_is_disabled(ssl, clientKSE->group))
                continue;
-        #endif
        }
        if (!TLSX_KeyShare_IsSupported(clientKSE->group))
            continue;
@ -9402,8 +9390,7 @@ int TLSX_KeyShare_Establish(WOLFSSL *ssl, int* doHelloRetry)

    if (clientKSE->key == NULL) {
 #ifdef HAVE_PQC
-        if (clientKSE->group >= WOLFSSL_PQC_MIN &&
-            clientKSE->group <= WOLFSSL_PQC_MAX ) {
+        if (WOLFSSL_NAMED_GROUP_IS_PQC(clientKSE->group)) {
            /* Going to need the public key (AKA ciphertext). */
            serverKSE->pubKey = clientKSE->pubKey;
            clientKSE->pubKey = NULL;
@ -10446,7 +10433,7 @@ int TLSX_QuicTP_Use(WOLFSSL* ssl, TLSX_Type ext_type, int is_response)
        }
    }
    if (extension->data) {
-        QuicTransportParam_free(extension->data, ssl->heap);
+        QuicTransportParam_free((QuicTransportParam*)extension->data, ssl->heap);
        extension->data = NULL;
    }
    extension->resp = is_response;
@ -11476,7 +11463,7 @@ int TLSX_PopulateExtensions(WOLFSSL* ssl, byte isServer)
            if (namedGroup > 0) {
 #ifdef HAVE_PQC
                /* For KEMs, the key share has already been generated. */
-                if (namedGroup < WOLFSSL_PQC_MIN || namedGroup > WOLFSSL_PQC_MAX)
+                if (!WOLFSSL_NAMED_GROUP_IS_PQC(namedGroup))
 #endif
                    ret = TLSX_KeyShare_Use(ssl, namedGroup, 0, NULL, NULL);
                if (ret != 0)
--- a/src/tls13.c
+++ b/src/tls13.c
@ -10606,8 +10606,7 @@ int wolfSSL_UseKeyShare(WOLFSSL* ssl, word16 group)
 #endif

 #ifdef HAVE_PQC
-    if (group >= WOLFSSL_PQC_MIN &&
-        group <= WOLFSSL_PQC_MAX) {
+    if (WOLFSSL_NAMED_GROUP_IS_PQC(group)) {

        if (ssl->ctx != NULL && ssl->ctx->method != NULL &&
            ssl->ctx->method->version.minor != TLSv1_3_MINOR) {
--- a/tests/quic.c
+++ b/tests/quic.c
@ -218,7 +218,7 @@ static void dump_buffer(const char *name, const byte *p, size_t len, int indent)
    while((p != NULL) && (i < len)) {
        if((i % 0x10) == 0) {
            printf("\n");
-            printf("%*s  %04X - ", indent, " ", (int)(i / 0x10));
+            printf("%*s  %04X - ", indent, " ", (int)i);
        }
        else if((i % 0x08) == 0) {
            printf("  ");
@ -1142,6 +1142,46 @@ static int test_quic_server_hello(int verbose) {

 #ifdef HAVE_SESSION_TICKET

+static int test_quic_key_share(int verbose) {
+    WOLFSSL_CTX *ctx_c, *ctx_s;
+    int ret = 0;
+    QuicTestContext tclient, tserver;
+    QuicConversation conv;
+
+    AssertNotNull(ctx_c = wolfSSL_CTX_new(wolfTLSv1_3_client_method()));
+    AssertNotNull(ctx_s = wolfSSL_CTX_new(wolfTLSv1_3_server_method()));
+    AssertTrue(wolfSSL_CTX_use_certificate_file(ctx_s, svrCertFile, WOLFSSL_FILETYPE_PEM));
+    AssertTrue(wolfSSL_CTX_use_PrivateKey_file(ctx_s, svrKeyFile, WOLFSSL_FILETYPE_PEM));
+
+    /* setup & handshake defaults */
+    QuicTestContext_init(&tclient, ctx_c, "client", verbose);
+    QuicTestContext_init(&tserver, ctx_s, "server", verbose);
+    QuicConversation_init(&conv, &tclient, &tserver);
+    QuicConversation_do(&conv);
+    AssertStrEQ(conv.rec_log,
+        "ClientHello:ServerHello:EncryptedExtension:Certificate:CertificateVerify:Finished:Finished:SessionTicket");
+    QuicTestContext_free(&tclient);
+    QuicTestContext_free(&tserver);
+
+    /* setup & handshake, restricted groups, will trigger a
+     * HelloRetryRequest(ServerHello) and a new ClientHello */
+    QuicTestContext_init(&tclient, ctx_c, "client", verbose);
+    QuicTestContext_init(&tserver, ctx_s, "server", verbose);
+    AssertTrue(wolfSSL_set1_curves_list(tclient.ssl, "X25519:P-256")
+               == WOLFSSL_SUCCESS);
+    AssertTrue(wolfSSL_set1_curves_list(tserver.ssl, "X25519")
+               == WOLFSSL_SUCCESS);
+    QuicConversation_init(&conv, &tclient, &tserver);
+    QuicConversation_do(&conv);
+    AssertStrEQ(conv.rec_log,
+        "ClientHello:ServerHello:ClientHello:ServerHello:EncryptedExtension:Certificate:CertificateVerify:Finished:Finished:SessionTicket");
+    QuicTestContext_free(&tclient);
+    QuicTestContext_free(&tserver);
+
+    printf("    test_quic_key_share: %s\n", (ret == 0)? passed : failed);
+    return ret;
+}
+
 static int test_quic_resumption(int verbose) {
    WOLFSSL_CTX *ctx_c, *ctx_s;
    WOLFSSL_SESSION *session;
@ -1379,6 +1419,7 @@ int QuicTest(void)
    if ((ret = test_quic_client_hello(verbose)) != 0) goto leave;
    if ((ret = test_quic_server_hello(verbose)) != 0) goto leave;
 #ifdef HAVE_SESSION_TICKET
+    if ((ret = test_quic_key_share(verbose)) != 0) goto leave;
    if ((ret = test_quic_resumption(verbose)) != 0) goto leave;
 #ifdef WOLFSSL_EARLY_DATA
    if ((ret = test_quic_early_data(verbose)) != 0) goto leave;
--- a/wolfssl/internal.h
+++ b/wolfssl/internal.h
@ -1570,6 +1570,15 @@ enum Misc {
    READ_PROTO         = 0     /* reading a protocol message */
 };

+#define WOLFSSL_NAMED_GROUP_IS_FFHDE(group) \
+    (MIN_FFHDE_GROUP <= (group) && (group) <= MAX_FFHDE_GROUP)
+#ifdef HAVE_PQC
+#define WOLFSSL_NAMED_GROUP_IS_PQC(group) \
+    (WOLFSSL_PQC_MIN <= (group) && (group) <= WOLFSSL_PQC_MAX)
+#else
+#define WOLFSSL_NAMED_GROUP_IS_PQC(group)    ((void)(group), 0)
+#endif /* HAVE_PQC */
+
 /* minimum Downgrade Minor version */
 #ifndef WOLFSSL_MIN_DOWNGRADE
    #ifndef NO_OLD_TLS
@ -5241,6 +5250,14 @@ WOLFSSL_LOCAL int SetECKeyInternal(WOLFSSL_EC_KEY* eckey);
 WOLFSSL_LOCAL int SetECKeyExternal(WOLFSSL_EC_KEY* eckey);
 #endif

+#if defined(OPENSSL_EXTRA)
+WOLFSSL_LOCAL int wolfSSL_CTX_curve_is_disabled(WOLFSSL_CTX* ctx, word16 named_curve);
+WOLFSSL_LOCAL int wolfSSL_curve_is_disabled(WOLFSSL* ssl, word16 named_curve);
+#else
+#define wolfSSL_CTX_curve_is_disabled(ctx, c)   ((void)(ctx), (void)(c), 0)
+#define wolfSSL_curve_is_disabled(ssl, c)   ((void)(ssl), (void)(c), 0)
+#endif
+
 WOLFSSL_LOCAL WC_RNG* WOLFSSL_RSA_GetRNG(WOLFSSL_RSA *rsa, WC_RNG **tmpRNG,
                                         int *initTmpRng);