Skip to content

Size the pre-master secret buffer by the key exchanges, and fix a DHE-PSK overflow #11345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Frauschi wants to merge 4 commits into
base: master
Choose a base branch
from
+625 −157
Open

Size the pre-master secret buffer by the key exchanges, and fix a DHE-PSK overflow #11345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ee9927
Carry the pre-master secret with the handshake arrays
Frauschi Aug 21, 2026
044bbfc
Size the pre-master secret buffer by the key exchanges
Frauschi Aug 22, 2026
a92fa28
Test the pre-master secret buffer against a DHE-PSK exchange
Frauschi Aug 22, 2026
38b5c37
Drop the sanitizer claim from the DHE-PSK test comments
Frauschi Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 92 additions & 62 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,6 +1571,15 @@ static int ImportOptions(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,
{
int idx = 0;
Options* options = &ssl->options;
byte sendVerify;
byte verifyPeer;
byte verifyNone;
byte downgrade;
#ifndef NO_DH
word16 minDhKeySz;
word16 maxDhKeySz;
word16 dhKeySz;
#endif

switch (ver) {
case WOLFSSL_EXPORT_VERSION:
Expand Down Expand Up @@ -1613,19 +1622,40 @@ static int ImportOptions(WOLFSSL* ssl, const byte* exp, word32 len, byte ver,


/* these options are kept and sent to indicate verify status and strength
* of handshake */
options->sendVerify = exp[idx++];
options->verifyPeer = exp[idx++];
options->verifyNone = exp[idx++];
options->downgrade = exp[idx++];
* of handshake. Decoded into locals and written only once the checks
* below pass, so a blob they reject leaves ssl->options untouched. Later
* failure exits do not give that guarantee. */
sendVerify = exp[idx++];
verifyPeer = exp[idx++];
verifyNone = exp[idx++];
downgrade = exp[idx++];
#ifndef NO_DH
ato16(exp + idx, &(options->minDhKeySz)); idx += OPAQUE16_LEN;
ato16(exp + idx, &(options->maxDhKeySz)); idx += OPAQUE16_LEN;
ato16(exp + idx, &(options->dhKeySz)); idx += OPAQUE16_LEN;
ato16(exp + idx, &minDhKeySz); idx += OPAQUE16_LEN;
ato16(exp + idx, &maxDhKeySz); idx += OPAQUE16_LEN;
ato16(exp + idx, &dhKeySz); idx += OPAQUE16_LEN;
Comment thread
Frauschi marked this conversation as resolved.
/* The blob is not trusted to respect what the pre-master secret buffer
* was sized for. */
if (maxDhKeySz > MAX_DHKEY_SZ)
maxDhKeySz = MAX_DHKEY_SZ;
/* A negotiated size this build cannot represent means the blob is not a
* session it can resume. Reject rather than clamp. */
if (dhKeySz > MAX_DHKEY_SZ) {
WOLFSSL_MSG("Exported DH key size exceeds this build's max");
return BAD_FUNC_ARG;
}
#else
idx += OPAQUE16_LEN;
idx += OPAQUE16_LEN;
idx += OPAQUE16_LEN;
#endif
options->sendVerify = sendVerify;
options->verifyPeer = verifyPeer;
options->verifyNone = verifyNone;
options->downgrade = downgrade;
#ifndef NO_DH
options->minDhKeySz = minDhKeySz;
options->maxDhKeySz = maxDhKeySz;
options->dhKeySz = dhKeySz;
#endif
#ifndef NO_RSA
ato16(exp + idx, (word16*)&(options->minRsaKeySz)); idx += OPAQUE16_LEN;
Expand Down Expand Up @@ -8583,6 +8613,14 @@ int InitHandshakeHashesAndCopy(WOLFSSL* ssl, HS_Hashes* source,
return ret;
}

#if defined(WOLFSSL_ASYNC_CRYPT) && defined(HAVE_INTEL_QA)
/* QAT writes preMasterSecret in place only when it heads a NUMA allocation. */
wc_static_assert(WC_OFFSETOF(Arrays, preMasterSecret) == 0);
#define ARRAYS_MEM_TYPE DYNAMIC_TYPE_SECRET
#else
#define ARRAYS_MEM_TYPE DYNAMIC_TYPE_ARRAYS
#endif

/* called if user attempts to reuse WOLFSSL object for a new session.
* For example wolfSSL_clear() is called then wolfSSL_connect or accept */
int ReinitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
Expand All @@ -8593,8 +8631,10 @@ int ReinitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)

/* arrays */
if (!writeDup && ssl->arrays == NULL) {
/* The pre-master secret is a member of the struct, so the two are
* always created and released together. */
ssl->arrays = (Arrays*)XMALLOC(sizeof(Arrays), ssl->heap,
DYNAMIC_TYPE_ARRAYS);
ARRAYS_MEM_TYPE);
if (ssl->arrays == NULL) {
WOLFSSL_MSG("Arrays Memory error");
return MEMORY_E;
Expand All @@ -8603,19 +8643,7 @@ int ReinitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
wc_MemZero_Add("SSL Arrays", ssl->arrays, sizeof(*ssl->arrays));
#endif
XMEMSET(ssl->arrays, 0, sizeof(Arrays));
#if defined(WOLFSSL_TLS13) || defined(WOLFSSL_SNIFFER)
ssl->arrays->preMasterSz = ENCRYPT_LEN;
ssl->arrays->preMasterSecret = (byte*)XMALLOC(ENCRYPT_LEN, ssl->heap,
DYNAMIC_TYPE_SECRET);
if (ssl->arrays->preMasterSecret == NULL) {
WOLFSSL_MSG("preMasterSecret Memory error");
return MEMORY_E;
}
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Add("SSL Arrays", ssl->arrays->preMasterSecret, ENCRYPT_LEN);
#endif
XMEMSET(ssl->arrays->preMasterSecret, 0, ENCRYPT_LEN);
#endif
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ;
}

/* RNG */
Expand Down Expand Up @@ -9344,14 +9372,10 @@ void FreeArrays(WOLFSSL* ssl, int keep)
XMEMCPY(ssl->session->sessionID, ssl->arrays->sessionID, ID_LEN);
ssl->session->sessionIDSz = ssl->arrays->sessionIDSz;
}
if (ssl->arrays->preMasterSecret) {
ForceZero(ssl->arrays->preMasterSecret, ENCRYPT_LEN);
XFREE(ssl->arrays->preMasterSecret, ssl->heap, DYNAMIC_TYPE_SECRET);
ssl->arrays->preMasterSecret = NULL;
}
ForceZero(ssl->arrays, sizeof(Arrays)); /* clear arrays struct */
/* Clears the arrays struct, pre-master secret included. */
ForceZero(ssl->arrays, sizeof(Arrays));
}
XFREE(ssl->arrays, ssl->heap, DYNAMIC_TYPE_ARRAYS);
XFREE(ssl->arrays, ssl->heap, ARRAYS_MEM_TYPE);
ssl->arrays = NULL;
}

Expand Down Expand Up @@ -37832,15 +37856,10 @@ int SendClientKeyExchange(WOLFSSL* ssl)
if (args->encSecret == NULL) {
ERROR_OUT(MEMORY_E, exit_scke);
}
if (ssl->arrays->preMasterSecret == NULL) {
ssl->arrays->preMasterSz = ENCRYPT_LEN;
ssl->arrays->preMasterSecret = (byte*)XMALLOC(ENCRYPT_LEN,
ssl->heap, DYNAMIC_TYPE_SECRET);
if (ssl->arrays->preMasterSecret == NULL) {
ERROR_OUT(MEMORY_E, exit_scke);
}
XMEMSET(ssl->arrays->preMasterSecret, 0, ENCRYPT_LEN);
}
/* The buffer lives with the arrays, so only its contents need
* to be readied here. */
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ;
XMEMSET(ssl->arrays->preMasterSecret, 0, MAX_PREMASTER_SZ);

switch(ssl->specs.kea)
{
Expand All @@ -37850,12 +37869,20 @@ int SendClientKeyExchange(WOLFSSL* ssl)
#ifdef HAVE_PK_CALLBACKS
if (ssl->ctx->GenPreMasterCb) {
void* ctx = wolfSSL_GetGenPreMasterCtx(ssl);
/* PMS_GEN_CB_SZ is the capacity the callback
* contract has always named. Handing it a larger one
* would change what a callback that echoes the
* capacity records, and so the master secret. */
ret = ssl->ctx->GenPreMasterCb(ssl,
ssl->arrays->preMasterSecret, ENCRYPT_LEN, ctx);
ssl->arrays->preMasterSecret, PMS_GEN_CB_SZ,
ctx);
if (ret != 0 &&
ret != WC_NO_ERR_TRACE(PROTOCOLCB_UNAVAILABLE)) {
goto exit_scke;
}
/* The callback owns preMasterSz; the Renesas ports
* echo back the capacity. Only the RNG path below
* records SECRET_LEN. */
}
if (!ssl->ctx->GenPreMasterCb ||
ret == WC_NO_ERR_TRACE(PROTOCOLCB_UNAVAILABLE))
Expand Down Expand Up @@ -37909,7 +37936,7 @@ int SendClientKeyExchange(WOLFSSL* ssl)
args->encSecret, &args->encSz);

/* set the max agree result size */
ssl->arrays->preMasterSz = ENCRYPT_LEN;
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ;
break;
}
#endif /* !NO_DH */
Expand Down Expand Up @@ -38032,7 +38059,8 @@ int SendClientKeyExchange(WOLFSSL* ssl)

/* Create shared ECC key leaving room at the beginning
* of buffer for size of shared key. */
ssl->arrays->preMasterSz = ENCRYPT_LEN - OPAQUE16_LEN;
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ -
OPAQUE16_LEN;
ret = EcExportHsKey(ssl, args->output, &args->length);
break;
}
Expand All @@ -38041,7 +38069,7 @@ int SendClientKeyExchange(WOLFSSL* ssl)
defined(HAVE_CURVE448)
case ecc_diffie_hellman_kea:
{
ssl->arrays->preMasterSz = ENCRYPT_LEN;
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ;
ret = EcExportHsKey(ssl, args->encSecret, &args->encSz);
break;
}
Expand Down Expand Up @@ -38105,6 +38133,10 @@ int SendClientKeyExchange(WOLFSSL* ssl)
#if !defined(NO_DH) && !defined(NO_PSK)
case dhe_psk_kea:
{
/* The secret goes after the length prefix. DhAgree()
* treats this as an out parameter only; the buffer sizing
* and the maxDhKeySz check are what bound the write. */
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ - OPAQUE16_LEN;
ret = DhAgree(ssl, ssl->buffers.serverDH_Key,
ssl->buffers.sig.buffer, ssl->buffers.sig.length,
ssl->buffers.serverDH_Pub.buffer,
Expand Down Expand Up @@ -38509,8 +38541,10 @@ int SendClientKeyExchange(WOLFSSL* ssl)
int secretSz = SECRET_LEN;
ret = ssl->keyLogCb(ssl, ssl->arrays->masterSecret, &secretSz,
NULL);
/* Leave through exit_scke, or the only wipe on this path
* is skipped. */
if (ret != 0 || secretSz != SECRET_LEN)
return SESSION_SECRET_CB_E;
ERROR_OUT(SESSION_SECRET_CB_E, exit_scke);
}
#endif /* OPENSSL_EXTRA && HAVE_SECRET_CALLBACK */
break;
Expand All @@ -38536,10 +38570,9 @@ int SendClientKeyExchange(WOLFSSL* ssl)
}
#endif

/* No further need for PMS */
if (ssl->arrays->preMasterSecret != NULL) {
ForceZero(ssl->arrays->preMasterSecret, ssl->arrays->preMasterSz);
}
/* No further need for PMS. Wipe all of it, not the recorded length: a
* GenPreMasterCb may have written more than was used. */
ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ);
ssl->arrays->preMasterSz = 0;

/* Final cleanup */
Expand Down Expand Up @@ -45623,7 +45656,7 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
return BUFFER_ERROR;

if (kea == ecdhe_psk_kea)
args->sigSz = ENCRYPT_LEN - OPAQUE16_LEN;
args->sigSz = MAX_PREMASTER_SZ - OPAQUE16_LEN;

#ifdef HAVE_CURVE25519
if (ssl->ecdhCurveOID == ECC_X25519_OID) {
Expand Down Expand Up @@ -45902,15 +45935,10 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
}
#endif

if (ssl->arrays->preMasterSecret == NULL) {
ssl->arrays->preMasterSz = ENCRYPT_LEN;
ssl->arrays->preMasterSecret = (byte*)XMALLOC(ENCRYPT_LEN,
ssl->heap, DYNAMIC_TYPE_SECRET);
if (ssl->arrays->preMasterSecret == NULL) {
ERROR_OUT(MEMORY_E, exit_dcke);
}
XMEMSET(ssl->arrays->preMasterSecret, 0, ENCRYPT_LEN);
}
/* The buffer lives with the arrays, so only its contents
* need to be readied here. */
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ;
XMEMSET(ssl->arrays->preMasterSecret, 0, MAX_PREMASTER_SZ);

switch (ssl->specs.kea) {
#ifndef NO_RSA
Expand Down Expand Up @@ -46139,7 +46167,7 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
ssl->buffers.serverDH_G.length);

/* set the max agree result size */
ssl->arrays->preMasterSz = ENCRYPT_LEN;
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ;
break;
}
#endif /* !NO_DH */
Expand Down Expand Up @@ -46342,6 +46370,10 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
#if !defined(NO_DH) && !defined(NO_PSK)
case dhe_psk_kea:
{
/* The secret goes after the length prefix, so that
* much less of the buffer is available for it. */
ssl->arrays->preMasterSz = MAX_PREMASTER_SZ -
OPAQUE16_LEN;
ret = DhAgree(ssl, ssl->buffers.serverDH_Key,
ssl->buffers.serverDH_Priv.buffer,
ssl->buffers.serverDH_Priv.length,
Expand Down Expand Up @@ -46638,10 +46670,8 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
#endif


/* Cleanup PMS */
if (ssl->arrays->preMasterSecret != NULL) {
ForceZero(ssl->arrays->preMasterSecret, ssl->arrays->preMasterSz);
}
/* Cleanup PMS. Wipe all of it, not the recorded length. */
ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ);
ssl->arrays->preMasterSz = 0;

/* Final cleanup */
Expand Down
46 changes: 15 additions & 31 deletions src/keys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3950,21 +3950,10 @@ int StoreKeys(WOLFSSL* ssl, const byte* keyData, int side)
#if !defined(NO_OLD_TLS) || defined(HAVE_EXTENDED_MASTER)
static void CleanPreMaster(WOLFSSL* ssl)
{
int sz = (int)(ssl->arrays->preMasterSz);

#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Add("CleanPreMaster preMasterSecret",
ssl->arrays->preMasterSecret, sz);
#endif

ForceZero(ssl->arrays->preMasterSecret, sz);

#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(ssl->arrays->preMasterSecret, sz);
#endif

XFREE(ssl->arrays->preMasterSecret, ssl->heap, DYNAMIC_TYPE_SECRET);
ssl->arrays->preMasterSecret = NULL;
/* Wipe all of it, not the recorded length, which a caller can shrink
* after a larger write. A wc_MemZero_Check() here would alias the whole
* "SSL Arrays" entry, which starts at the same address. */
ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ);
ssl->arrays->preMasterSz = 0;
}
#endif /* !NO_OLD_TLS || HAVE_EXTENDED_MASTER */
Expand Down Expand Up @@ -4111,16 +4100,12 @@ static int MakeSslMasterSecret(WOLFSSL* ssl)
wc_Sha* sha;
#else
byte shaOutput[WC_SHA_DIGEST_SIZE];
byte md5Input[ENCRYPT_LEN + WC_SHA_DIGEST_SIZE];
byte shaInput[PREFIX + ENCRYPT_LEN + 2 * RAN_LEN];
byte md5Input[MAX_PREMASTER_SZ + WC_SHA_DIGEST_SIZE];
byte shaInput[PREFIX + MAX_PREMASTER_SZ + 2 * RAN_LEN];
wc_Md5 md5[1];
wc_Sha sha[1];
#endif

if (ssl->arrays->preMasterSecret == NULL) {
return BAD_FUNC_ARG;
}

#ifdef SHOW_SECRETS
{
word32 j;
Expand All @@ -4134,9 +4119,9 @@ static int MakeSslMasterSecret(WOLFSSL* ssl)
#ifdef WOLFSSL_SMALL_STACK
shaOutput = (byte*)XMALLOC(WC_SHA_DIGEST_SIZE,
NULL, DYNAMIC_TYPE_TMP_BUFFER);
md5Input = (byte*)XMALLOC(ENCRYPT_LEN + WC_SHA_DIGEST_SIZE,
md5Input = (byte*)XMALLOC(MAX_PREMASTER_SZ + WC_SHA_DIGEST_SIZE,
NULL, DYNAMIC_TYPE_TMP_BUFFER);
shaInput = (byte*)XMALLOC(PREFIX + ENCRYPT_LEN + 2 * RAN_LEN,
shaInput = (byte*)XMALLOC(PREFIX + MAX_PREMASTER_SZ + 2 * RAN_LEN,
NULL, DYNAMIC_TYPE_TMP_BUFFER);
md5 = (wc_Md5*)XMALLOC(sizeof(wc_Md5), NULL, DYNAMIC_TYPE_TMP_BUFFER);
sha = (wc_Sha*)XMALLOC(sizeof(wc_Sha), NULL, DYNAMIC_TYPE_TMP_BUFFER);
Expand All @@ -4154,9 +4139,9 @@ static int MakeSslMasterSecret(WOLFSSL* ssl)
#endif
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Add("MakeSslMasterSecret md5Input", md5Input,
ENCRYPT_LEN + WC_SHA_DIGEST_SIZE);
MAX_PREMASTER_SZ + WC_SHA_DIGEST_SIZE);
wc_MemZero_Add("MakeSslMasterSecret shaInput", shaInput,
PREFIX + ENCRYPT_LEN + 2 * RAN_LEN);
PREFIX + MAX_PREMASTER_SZ + 2 * RAN_LEN);
wc_MemZero_Add("MakeSslMasterSecret shaOutput", shaOutput,
WC_SHA_DIGEST_SIZE);
#endif
Expand Down Expand Up @@ -4219,12 +4204,12 @@ static int MakeSslMasterSecret(WOLFSSL* ssl)
ret = DeriveKeys(ssl);
}

ForceZero(md5Input, ENCRYPT_LEN + WC_SHA_DIGEST_SIZE);
ForceZero(shaInput, PREFIX + ENCRYPT_LEN + 2 * RAN_LEN);
ForceZero(md5Input, MAX_PREMASTER_SZ + WC_SHA_DIGEST_SIZE);
ForceZero(shaInput, PREFIX + MAX_PREMASTER_SZ + 2 * RAN_LEN);
ForceZero(shaOutput, WC_SHA_DIGEST_SIZE);
#ifdef WOLFSSL_CHECK_MEM_ZERO
wc_MemZero_Check(md5Input, ENCRYPT_LEN + WC_SHA_DIGEST_SIZE);
wc_MemZero_Check(shaInput, PREFIX + ENCRYPT_LEN + 2 * RAN_LEN);
wc_MemZero_Check(md5Input, MAX_PREMASTER_SZ + WC_SHA_DIGEST_SIZE);
wc_MemZero_Check(shaInput, PREFIX + MAX_PREMASTER_SZ + 2 * RAN_LEN);
wc_MemZero_Check(shaOutput, WC_SHA_DIGEST_SIZE);
#endif

Expand All @@ -4251,8 +4236,7 @@ int MakeMasterSecret(WOLFSSL* ssl)
WOLFSSL_MSG("EMS required but not negotiated with peer");
SendAlert(ssl, alert_fatal, handshake_failure);
WOLFSSL_ERROR_VERBOSE(EXT_MASTER_SECRET_NEEDED_E);
if (ssl->arrays->preMasterSecret != NULL)
CleanPreMaster(ssl);
CleanPreMaster(ssl);
return EXT_MASTER_SECRET_NEEDED_E;
}
#endif
Expand Down
Loading
Loading