Implement second step of RFC7919 in TLS 1.2 server

Before this commit, the logic for generating a temporary DH key for DHE
cipher suites is the following:
1) If dh_tmp_auto is set (see SSL_set_dh_auto), the SSL server
   automatically selects a set of DH parameters (P and G) appropriate
   for the security level of the cipher suite. The groups are taken from
   IKE (RFC 2409 and RFC 3526).
2) Otherwise, if the user provided a pre-generated set of DH parameters
   (SSL_set0_tmp_dh_pkey), those parameters are used.
3) Finally, if neither 1) or 2) are applicable, a callback function can
   be set using SSL_set_tmp_dh_callback, which will be invoked to
   generate the temporary DH parameters. From OpenSSL 3.0, this
   functionality is deprecated.
4) Using the parameters from step 1-3, an ephemeral DH key is
   generated. The parameters and the public key are sent to the client.

The logic above is updated by inserting an additional step, prior to
step 1:
0) If tls1_shared_group returns any shared known group between the
   server and the client, the DH parameters associated with this group
   are selected.

This is still compliant with RFC7919, as the server will already have
checked the Supported Groups extension during the ciphersuite selection
process (implemented in the previous commit).

Now, the tests need to be updated: By default, the TLS 1.2 server will
default to RFC7919 groups. To bypass this behavior, the supported groups
on the client side is set to "xorgroup", ensuring that the client does
not advertise any FFDHE group support and the server falls back to the
old logic.

An additional test is also added to ensure that the TLS 1.2 server does
select the right group if the client advertises any of the RFC7919
groups.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
MergeDate: Thu Feb  5 09:09:41 2026
(Merged from https://github.com/openssl/openssl/pull/24551)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index ee657d601568e4eeb0859bd80b6a837f9574000b..f105408ecd01821b3706073627a57a5d196ab054 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -4599,7 +4599,10 @@ void ssl_set_masks(SSL_CONNECTION *s)
 
     dh_tmp = (c->dh_tmp != NULL
         || c->dh_tmp_cb != NULL
-        || c->dh_tmp_auto);
+        || c->dh_tmp_auto
+        || tls1_shared_group(s, TLS1_GROUPS_RETURN_TMP_ID,
+               TLS1_GROUPS_FFDHE_GROUPS)
+            != 0);
 
     rsa_enc = pvalid[SSL_PKEY_RSA] & CERT_PKEY_VALID;
     rsa_sign = pvalid[SSL_PKEY_RSA] & CERT_PKEY_VALID;

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index b648e79d15fa231a8b89116b899d3ef6ec63b897..ea011c71a79a433517c93d611ed33fa337193a24 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2569,7 +2569,7 @@ CON_FUNC_RETURN tls_construct_server_key_exchange(SSL_CONNECTION *s,
     EVP_PKEY *pkdh = NULL;
     unsigned char *encodedPoint = NULL;
     size_t encodedlen = 0;
-    int curve_id = 0;
+    int group_id = 0;
     const SIGALG_LOOKUP *lu = s->s3.tmp.sigalg;
     int i;
     unsigned long type;
@@ -2603,49 +2603,64 @@ CON_FUNC_RETURN tls_construct_server_key_exchange(SSL_CONNECTION *s,
             CERT *cert = s->cert;
             EVP_PKEY *pkdhp = NULL;
 
-            if (s->cert->dh_tmp_auto) {
-                pkdh = ssl_get_auto_dh(s);
-                if (pkdh == NULL) {
+            /* Get NID of appropriate shared FFDHE group */
+            group_id = tls1_shared_group(s, TLS1_GROUPS_RETURN_TMP_ID,
+                TLS1_GROUPS_FFDHE_GROUPS);
+            if (group_id != 0) {
+                /* Cache the group used in the SSL_SESSION */
+                s->session->kex_group = group_id;
+
+                s->s3.tmp.pkey = ssl_generate_pkey_group(s, group_id);
+                if (s->s3.tmp.pkey == NULL) {
                     SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
                     goto err;
                 }
-                pkdhp = pkdh;
             } else {
-                pkdhp = cert->dh_tmp;
-            }
+
+                if (s->cert->dh_tmp_auto) {
+                    pkdh = ssl_get_auto_dh(s);
+                    if (pkdh == NULL) {
+                        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                        goto err;
+                    }
+                    pkdhp = pkdh;
+                } else {
+                    pkdhp = cert->dh_tmp;
+                }
 #if !defined(OPENSSL_NO_DEPRECATED_3_0)
-            if ((pkdhp == NULL) && (s->cert->dh_tmp_cb != NULL)) {
-                pkdh = ssl_dh_to_pkey(s->cert->dh_tmp_cb(SSL_CONNECTION_GET_USER_SSL(s),
-                    0, 1024));
-                if (pkdh == NULL) {
+                if ((pkdhp == NULL) && (s->cert->dh_tmp_cb != NULL)) {
+                    pkdh = ssl_dh_to_pkey(s->cert->dh_tmp_cb(SSL_CONNECTION_GET_USER_SSL(s),
+                        0, 1024));
+                    if (pkdh == NULL) {
+                        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                        goto err;
+                    }
+                    pkdhp = pkdh;
+                }
+#endif
+                if (pkdhp == NULL) {
+                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_MISSING_TMP_DH_KEY);
+                    goto err;
+                }
+                if (!ssl_security(s, SSL_SECOP_TMP_DH,
+                        EVP_PKEY_get_security_bits(pkdhp), 0, pkdhp)) {
+                    SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_R_DH_KEY_TOO_SMALL);
+                    goto err;
+                }
+                if (s->s3.tmp.pkey != NULL) {
                     SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
                     goto err;
                 }
-                pkdhp = pkdh;
-            }
-#endif
-            if (pkdhp == NULL) {
-                SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_MISSING_TMP_DH_KEY);
-                goto err;
-            }
-            if (!ssl_security(s, SSL_SECOP_TMP_DH,
-                    EVP_PKEY_get_security_bits(pkdhp), 0, pkdhp)) {
-                SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_R_DH_KEY_TOO_SMALL);
-                goto err;
-            }
-            if (s->s3.tmp.pkey != NULL) {
-                SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-                goto err;
-            }
 
-            s->s3.tmp.pkey = ssl_generate_pkey(s, pkdhp);
-            if (s->s3.tmp.pkey == NULL) {
-                SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-                goto err;
-            }
+                s->s3.tmp.pkey = ssl_generate_pkey(s, pkdhp);
+                if (s->s3.tmp.pkey == NULL) {
+                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                    goto err;
+                }
 
-            EVP_PKEY_free(pkdh);
-            pkdh = NULL;
+                EVP_PKEY_free(pkdh);
+                pkdh = NULL;
+            }
 
             /* These BIGNUMs need to be freed when we're finished */
             freer = 1;
@@ -2665,18 +2680,18 @@ CON_FUNC_RETURN tls_construct_server_key_exchange(SSL_CONNECTION *s,
                 goto err;
             }
 
-            /* Get NID of appropriate shared curve */
-            curve_id = tls1_shared_group(s, TLS1_GROUPS_RETURN_TMP_ID,
+            /* Get NID of appropriate shared ECDHE curve */
+            group_id = tls1_shared_group(s, TLS1_GROUPS_RETURN_TMP_ID,
                 TLS1_GROUPS_NON_FFDHE_GROUPS);
-            if (curve_id == 0) {
+            if (group_id == 0) {
                 SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE,
                     SSL_R_UNSUPPORTED_ELLIPTIC_CURVE);
                 goto err;
             }
             /* Cache the group used in the SSL_SESSION */
-            s->session->kex_group = curve_id;
+            s->session->kex_group = group_id;
             /* Generate a new key for this curve */
-            s->s3.tmp.pkey = ssl_generate_pkey_group(s, curve_id);
+            s->s3.tmp.pkey = ssl_generate_pkey_group(s, group_id);
             if (s->s3.tmp.pkey == NULL) {
                 /* SSLfatal() already called */
                 goto err;
@@ -2793,7 +2808,7 @@ CON_FUNC_RETURN tls_construct_server_key_exchange(SSL_CONNECTION *s,
          * point itself
          */
         if (!WPACKET_put_bytes_u8(pkt, NAMED_CURVE_TYPE)
-            || !WPACKET_put_bytes_u16(pkt, curve_id)
+            || !WPACKET_put_bytes_u16(pkt, group_id)
             || !WPACKET_sub_memcpy_u8(pkt, encodedPoint, encodedlen)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
             goto err;

diff --git a/test/sslapitest.c b/test/sslapitest.c
index 35c22126f6d0b1e070c30295da2cc3afa0289f9c..5e2ac8daecc7f509c3a78cb766d43686c427abf8 100644 (file)
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -11225,6 +11225,11 @@ static int test_set_tmp_dh(int idx)
         return 1;
 #endif
 
+    OSSL_PROVIDER *tlsprov = OSSL_PROVIDER_load(libctx, "tls-provider");
+
+    if (!TEST_ptr(tlsprov))
+        goto end;
+
     if (idx >= 5 && idx <= 8) {
         dhpkey = get_tmp_dh_params();
         if (!TEST_ptr(dhpkey))
@@ -11288,7 +11293,9 @@ static int test_set_tmp_dh(int idx)
 
     if (!TEST_true(SSL_set_min_proto_version(serverssl, TLS1_2_VERSION))
         || !TEST_true(SSL_set_max_proto_version(serverssl, TLS1_2_VERSION))
-        || !TEST_true(SSL_set_cipher_list(serverssl, "DHE-RSA-AES128-SHA")))
+        || !TEST_true(SSL_set_cipher_list(serverssl, "DHE-RSA-AES128-SHA"))
+        /* This is required so the server does not use RFC7919 groups */
+        || !TEST_true(SSL_set1_groups_list(clientssl, "xorgroup")))
         goto end;
 
     /*
@@ -11311,6 +11318,7 @@ end:
     SSL_CTX_free(sctx);
     SSL_CTX_free(cctx);
     EVP_PKEY_free(dhpkey);
+    OSSL_PROVIDER_unload(tlsprov);
 
     return testresult;
 }
@@ -11320,6 +11328,7 @@ end:
  */
 static int test_dh_auto(int idx)
 {
+    OSSL_PROVIDER *tlsprov = OSSL_PROVIDER_load(libctx, "tls-provider");
     SSL_CTX *cctx = SSL_CTX_new_ex(libctx, NULL, TLS_client_method());
     SSL_CTX *sctx = SSL_CTX_new_ex(libctx, NULL, TLS_server_method());
     SSL *clientssl = NULL, *serverssl = NULL;
@@ -11329,6 +11338,9 @@ static int test_dh_auto(int idx)
     size_t expdhsize = 0;
     const char *ciphersuite = "DHE-RSA-AES128-SHA";
 
+    if (!TEST_ptr(tlsprov))
+        goto end;
+
     if (!TEST_ptr(sctx) || !TEST_ptr(cctx))
         goto end;
 
@@ -11400,7 +11412,9 @@ static int test_dh_auto(int idx)
         || !TEST_true(SSL_set_min_proto_version(serverssl, TLS1_2_VERSION))
         || !TEST_true(SSL_set_max_proto_version(serverssl, TLS1_2_VERSION))
         || !TEST_true(SSL_set_cipher_list(serverssl, ciphersuite))
-        || !TEST_true(SSL_set_cipher_list(clientssl, ciphersuite)))
+        || !TEST_true(SSL_set_cipher_list(clientssl, ciphersuite))
+        /* This is required so the server does not use RFC7919 groups */
+        || !TEST_true(SSL_set1_groups_list(clientssl, "xorgroup")))
         goto end;
 
     /*
@@ -11428,6 +11442,7 @@ end:
     SSL_CTX_free(sctx);
     SSL_CTX_free(cctx);
     EVP_PKEY_free(tmpkey);
+    OSSL_PROVIDER_unload(tlsprov);
 
     return testresult;
 }
@@ -11563,7 +11578,97 @@ end:
     return testresult;
 }
 
+/*
+ * Test the server will use the supported FFDHE group advertised by the client.
+ */
+static int test_shared_ffdhe_group(int idx)
+{
+    SSL_CTX *cctx = SSL_CTX_new_ex(libctx, NULL, TLS_client_method());
+    SSL_CTX *sctx = SSL_CTX_new_ex(libctx, NULL, TLS_server_method());
+    SSL *clientssl = NULL, *serverssl = NULL;
+    int testresult = 0;
+    EVP_PKEY *tmpkey = NULL;
+    char *servergroups = "ffdhe2048:ffdhe3072:ffdhe4096:ffdhe6144:ffdhe8192";
+    char *clientgroup = NULL;
+    const char *ciphersuite = "DHE-RSA-AES128-SHA256";
+    char gname[10];
+    size_t gname_len;
+
+    if (!TEST_ptr(sctx) || !TEST_ptr(cctx))
+        goto end;
+
+    switch (idx) {
+    case 0:
+        clientgroup = "ffdhe2048";
+        break;
+    case 1:
+        clientgroup = "ffdhe3072";
+        break;
+    case 2:
+        clientgroup = "ffdhe4096";
+        break;
+    case 3:
+        clientgroup = "ffdhe6144";
+        break;
+    case 4:
+        clientgroup = "ffdhe8192";
+        break;
+    default:
+        TEST_error("Invalid text index");
+        goto end;
+    }
+
+    if (!TEST_true(create_ssl_ctx_pair(libctx, NULL,
+            NULL,
+            0,
+            0,
+            &sctx, &cctx, cert, privkey)))
+        goto end;
+
+    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+            NULL, NULL)))
+        goto end;
+
+    if (!TEST_true(SSL_set_dh_auto(serverssl, 1))
+        || !TEST_true(SSL_set_min_proto_version(serverssl, TLS1_2_VERSION))
+        || !TEST_true(SSL_set_max_proto_version(serverssl, TLS1_2_VERSION))
+        || !TEST_true(SSL_set_cipher_list(serverssl, ciphersuite))
+        || !TEST_true(SSL_set_cipher_list(clientssl, ciphersuite))
+        || !TEST_true(SSL_set1_groups_list(serverssl, servergroups))
+        || !TEST_true(SSL_set1_groups_list(clientssl, clientgroup)))
+        goto end;
+
+    /*
+     * Send the server's first flight. At this point the server has created the
+     * temporary DH key but hasn't finished using it yet. Once used it is
+     * removed, so we cannot test it.
+     */
+    if (!TEST_int_le(SSL_connect(clientssl), 0)
+        || !TEST_int_le(SSL_accept(serverssl), 0))
+        goto end;
+
+    if (!TEST_int_gt(SSL_get_tmp_key(serverssl, &tmpkey), 0))
+        goto end;
+    if (!TEST_true(EVP_PKEY_get_group_name(tmpkey, gname, sizeof(gname), &gname_len))
+        || !TEST_str_eq(gname, clientgroup))
+        goto end;
+
+    if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE)))
+        goto end;
+
+    testresult = 1;
+
+end:
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+    EVP_PKEY_free(tmpkey);
+
+    return testresult;
+}
 #endif /* OPENSSL_NO_TLS1_3 */
+
 #endif /* OPENSSL_NO_DH */
 #endif /* OPENSSL_NO_TLS1_2 */
 
@@ -14392,6 +14497,7 @@ int setup_tests(void)
     ADD_ALL_TESTS(test_dh_auto, 7);
 #ifndef OPENSSL_NO_TLS1_3
     ADD_ALL_TESTS(test_no_shared_ffdhe_group, 10);
+    ADD_ALL_TESTS(test_shared_ffdhe_group, 5);
 #endif
 #endif
 #endif

diff --git a/test/tls-provider.c b/test/tls-provider.c
index 0a9da8ae869a4768f8b1624fab14982d9ef1c0e4..5b4017543fd050ca435c4d9ff7b4b0d395b46aa7 100644 (file)
--- a/test/tls-provider.c
+++ b/test/tls-provider.c
@@ -214,7 +214,7 @@ struct tls_group_st {
 static struct tls_group_st xor_group = {
     0, /* group_id, set by randomize_tls_alg_id() */
     128, /* secbits */
-    TLS1_3_VERSION, /* mintls */
+    TLS1_2_VERSION, /* mintls */
     0, /* maxtls */
     -1, /* mindtls */
     -1, /* maxdtls */