Free DH buffers after negotiation (server only)

pull/11115/head
Paul Adelsbach 2026-08-24 12:45:10 -07:00
parent bf4b4aa878
commit f6a6030d6c
2 changed files with 31 additions and 19 deletions

View File

@ -7630,7 +7630,8 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
* The parameters are plain buffers with no reference count on them, so a
* session must not point at the context's: the context is free to replace
* them at any time. They are small and only present when the application
* asked for them, so a copy per session is the cheap way to keep them safe.
* asked for them, so a copy is the cheap way to keep them safe. It is given
* back at the end of the handshake and taken again when next needed.
*
* @param [in, out] ssl SSL object. Any parameters it owns are let go of.
* @param [in] ctx SSL context object.
@ -10129,10 +10130,9 @@ void FreeHandshakeResources(WOLFSSL* ssl)
ssl->buffers.serverDH_Priv.buffer = NULL;
XFREE(ssl->buffers.serverDH_Pub.buffer, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY);
ssl->buffers.serverDH_Pub.buffer = NULL;
/* A server keeps the parameters (p,g): a renegotiation or a reused object
* needs them again, and they are freed with the object. A client reads new
* ones from every ServerKeyExchange, so it lets them go here. */
if ((ssl->options.side == WOLFSSL_CLIENT_END) && ssl->buffers.weOwnDH) {
/* Release the parameters (p,g) back here rather than hold them for
* the life of the connection. */
if (ssl->buffers.weOwnDH) {
XFREE(ssl->buffers.serverDH_G.buffer, ssl->heap,
DYNAMIC_TYPE_PUBLIC_KEY);
ssl->buffers.serverDH_G.buffer = NULL;
@ -39030,6 +39030,14 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
#endif
{
/* Allocate DH key buffers and generate key */
if (ssl->buffers.serverDH_P.buffer == NULL ||
ssl->buffers.serverDH_G.buffer == NULL) {
/* Buffers freed at the end of the last handshake,
* create a new copy from the context. */
if (CopySSL_CTX_DhParams(ssl, ssl->ctx) != 0) {
ERROR_OUT(MEMORY_E, exit_sske);
}
}
if (ssl->buffers.serverDH_P.buffer == NULL ||
ssl->buffers.serverDH_G.buffer == NULL) {
ERROR_OUT(NO_DH_PARAMS, exit_sske);

View File

@ -1072,12 +1072,12 @@ int test_wolfSSL_set_connect_state_dh(void)
return EXPECT_RESULT();
}
/* DH parameters taken from the context are the object's own copy, and they
* last as long as the object does.
/* DH parameters taken from the context are the object's own copy, given back
* at the end of the handshake and taken again when next needed.
*
* The copy is what lets the context replace its parameters while sessions are
* running. Since the session then holds the only copy it will ever have, the
* end of a handshake must not take it away: a reused object needs it again.
* running. Holding it only for the handshake keeps the cost off connections
* that are established and idle.
*
* @return TEST_SUCCESS on success.
*/
@ -1097,8 +1097,6 @@ int test_wolfSSL_dh_ctx_params_reuse(void)
WOLFSSL_CTX* ctx_s = NULL;
WOLFSSL* ssl_c = NULL;
WOLFSSL* ssl_s = NULL;
const byte* startP = NULL;
const byte* startG = NULL;
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
test_ctx.c_ciphers = test_ctx.s_ciphers = "DHE-RSA-AES128-GCM-SHA256";
@ -1143,8 +1141,6 @@ int test_wolfSSL_dh_ctx_params_reuse(void)
ExpectBufEQ(ssl_s->buffers.serverDH_G.buffer, ctx_s->serverDH_G.buffer,
ctx_s->serverDH_G.length);
ExpectIntEQ(ssl_s->buffers.weOwnDH, 1);
startP = ssl_s->buffers.serverDH_P.buffer;
startG = ssl_s->buffers.serverDH_G.buffer;
}
/* Offering no FFDHE group keeps the server on the parameters it was
@ -1158,10 +1154,14 @@ int test_wolfSSL_dh_ctx_params_reuse(void)
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
/* The handshake used them and the cleanup after it left them alone. */
/* The handshake ran on them and then gave them back. A build that may
* renegotiate holds on to every handshake resource, parameters included. */
if (ssl_s != NULL) {
ExpectPtrEq(ssl_s->buffers.serverDH_P.buffer, startP);
ExpectPtrEq(ssl_s->buffers.serverDH_G.buffer, startG);
ExpectIntEQ(ssl_s->specs.kea, diffie_hellman_kea);
#ifndef HAVE_SECURE_RENEGOTIATION
ExpectNull(ssl_s->buffers.serverDH_P.buffer);
ExpectNull(ssl_s->buffers.serverDH_G.buffer);
#endif
}
/* So the object can be handed a second connection. */
@ -1178,11 +1178,15 @@ int test_wolfSSL_dh_ctx_params_reuse(void)
test_memio_clear_buffer(&test_ctx, 0);
test_memio_clear_buffer(&test_ctx, 1);
/* The second handshake ran on the parameters the first one left behind. */
/* The second handshake took a fresh copy from the context. Without it
* there would be no parameters left to send and it would not complete. */
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
if (ssl_s != NULL) {
ExpectPtrEq(ssl_s->buffers.serverDH_P.buffer, startP);
ExpectPtrEq(ssl_s->buffers.serverDH_G.buffer, startG);
ExpectIntEQ(ssl_s->specs.kea, diffie_hellman_kea);
#ifndef HAVE_SECURE_RENEGOTIATION
ExpectNull(ssl_s->buffers.serverDH_P.buffer);
ExpectNull(ssl_s->buffers.serverDH_G.buffer);
#endif
}
/* And the context still has its own to hand out. */