Commit 2af8022aaf for openssl.org
commit 2af8022aaf799b42e59e2ea3332b41e56a9b9c73
Author: Joachim Vandersmissen <git@jvdsn.com>
Date: Mon Dec 15 18:29:21 2025 +1100
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 ee657d6015..f105408ecd 100644
--- 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 b648e79d15..ea011c71a7 100644
--- 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 35c22126f6..5e2ac8daec 100644
--- 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 0a9da8ae86..5b4017543f 100644
--- 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 */