diff --git a/src/internal.c b/src/internal.c index 9afda1e2b39..7202f8cb7af 100644 --- a/src/internal.c +++ b/src/internal.c @@ -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: @@ -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; + /* 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; @@ -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) @@ -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; @@ -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 */ @@ -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; } @@ -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) { @@ -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)) @@ -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 */ @@ -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; } @@ -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; } @@ -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, @@ -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; @@ -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 */ @@ -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) { @@ -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 @@ -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 */ @@ -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, @@ -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 */ diff --git a/src/keys.c b/src/keys.c index 45534d97e21..e955906b9a1 100644 --- a/src/keys.c +++ b/src/keys.c @@ -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 */ @@ -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; @@ -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); @@ -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 @@ -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 @@ -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 diff --git a/src/sniffer.c b/src/sniffer.c index 7e05022b777..4ef9a8e560a 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -2637,6 +2637,22 @@ static void FreeSetupKeysArgs(WOLFSSL* ssl, void* pArgs) } } +/* The sniffer never calls CleanPreMaster(), so both copies would otherwise + * stay resident in the session table for the life of the session. */ +static void CleanSnifferPreMaster(SnifferSession* session) +{ + if (session->sslServer != NULL && session->sslServer->arrays != NULL) { + ForceZero(session->sslServer->arrays->preMasterSecret, + MAX_PREMASTER_SZ); + session->sslServer->arrays->preMasterSz = 0; + } + if (session->sslClient != NULL && session->sslClient->arrays != NULL) { + ForceZero(session->sslClient->arrays->preMasterSecret, + MAX_PREMASTER_SZ); + session->sslClient->arrays->preMasterSz = 0; + } +} + /* Process Keys */ #if !defined(HAVE_ENCRYPT_THEN_MAC) && !defined(WOLFSSL_AEAD_ONLY) /* RFC 7366 only covers block ciphers and a peer must not negotiate it for an @@ -3043,7 +3059,7 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, /* Length is in bytes. Subtract 1 for the ECC key type. Divide * by two as the key is in (x,y) coordinates, where x and y are * the same size, the key size. Convert from bytes to bits. */ - session->sslServer->arrays->preMasterSz = ENCRYPT_LEN; + session->sslServer->arrays->preMasterSz = MAX_PREMASTER_SZ; } } #endif /* HAVE_ECC */ @@ -3135,7 +3151,7 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, if (ret == 0) { /* For Curve25519 length is always 32 */ session->keySz = CURVE25519_KEYSIZE; - session->sslServer->arrays->preMasterSz = ENCRYPT_LEN; + session->sslServer->arrays->preMasterSz = MAX_PREMASTER_SZ; } } #endif /* HAVE_CURVE25519 */ @@ -3217,7 +3233,7 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, if (ret == 0) { session->keySz = CURVE448_KEY_SIZE; - session->sslServer->arrays->preMasterSz = ENCRYPT_LEN; + session->sslServer->arrays->preMasterSz = MAX_PREMASTER_SZ; } } #endif /* HAVE_CURVE448 */ @@ -3429,6 +3445,9 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, ret += MakeMasterSecret(session->sslClient); ret += SetKeysSide(session->sslServer, ENCRYPT_AND_DECRYPT_SIDE); ret += SetKeysSide(session->sslClient, ENCRYPT_AND_DECRYPT_SIDE); + /* Last reader below TLS 1.3, so both copies can go. TLS 1.3 + * keeps the buffer until ProcessFinished() wipes it. */ + CleanSnifferPreMaster(session); #else /* No master secret is computed here, so installing cipher state * would be wrong. */ @@ -3439,6 +3458,7 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, break; #endif } + if (ret != 0) { SetError(BAD_DERIVE_STR, error, session, FATAL_ERROR_STATE); ret = WOLFSSL_FATAL_ERROR; break; @@ -3486,6 +3506,12 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, } #endif /* WOLFSSL_ASYNC_CRYPT */ + /* An error above may have left a secret in the arrays until the session + * is evicted. The async pending path returned already; it needs the + * buffer to resume. */ + if (ret < 0) + CleanSnifferPreMaster(session); + #ifdef WOLFSSL_SNIFFER_STATS if (ret < 0) INC_STAT(SnifferStats.sslKeyFails); @@ -4822,6 +4848,8 @@ static int ProcessFinished(const byte* input, int size, int* sslBytes, *sslBytes -= (int)inOutIdx; if (ret < 0) { + /* The handshake secret may already be here; wipe on the way out. */ + CleanSnifferPreMaster(session); SetError(BAD_FINISHED_MSG, error, session, FATAL_ERROR_STATE); return ret; } @@ -4862,6 +4890,8 @@ static int ProcessFinished(const byte* input, int size, int* sslBytes, ret += DeriveTls13Keys(session->sslServer, traffic_key, ENCRYPT_AND_DECRYPT_SIDE, 1); ret += DeriveTls13Keys(session->sslClient, traffic_key, ENCRYPT_AND_DECRYPT_SIDE, 1); #endif + /* The handshake secret is spent, derivation succeeded or not. */ + CleanSnifferPreMaster(session); if (ret != 0) { SetError(BAD_FINISHED_MSG, error, session, FATAL_ERROR_STATE); diff --git a/src/ssl.c b/src/ssl.c index 763757e21fc..1b4c17e32dd 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -6056,13 +6056,10 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, * An application that asked to keep the arrays still gets back * everything the API can hand it, so only the rest goes. */ if (ssl->arrays != NULL) { - if (ssl->arrays->preMasterSecret != NULL) { - ForceZero(ssl->arrays->preMasterSecret, ENCRYPT_LEN); - /* The key agreement routines take this as the size of the - * buffer they may write, so put back what a freshly - * allocated Arrays would carry. */ - ssl->arrays->preMasterSz = ENCRYPT_LEN; - } + /* The buffer lives as long as the arrays do. */ + ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ); + /* Key agreement reads this as the writable capacity. */ + ssl->arrays->preMasterSz = MAX_PREMASTER_SZ; #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) ForceZero(ssl->arrays->psk_key, MAX_PSK_KEY_LEN); ssl->arrays->psk_keySz = 0; diff --git a/src/ssl_api_dtls.c b/src/ssl_api_dtls.c index d2e7ed2583e..7c8387a7b12 100644 --- a/src/ssl_api_dtls.c +++ b/src/ssl_api_dtls.c @@ -760,9 +760,10 @@ static WC_INLINE word32 UpdateHighwaterMark(word32 cur, word32 first, * @param [in] serverRandom Server random data (RAN_LEN bytes). * @param [in] suite Cipher suite bytes (2). * @return WOLFSSL_SUCCESS on success. - * @return WOLFSSL_FATAL_ERROR on error, including invalid arguments and a - * failure to allocate the pre-master secret. The specific code is - * recorded in ssl->error and can be read with wolfSSL_get_error(). + * @return WOLFSSL_FATAL_ERROR on error, including invalid arguments, a + * secret longer than MAX_PREMASTER_SZ, and handshake arrays that + * have already been released. The specific code is recorded in + * ssl->error and can be read with wolfSSL_get_error(). */ int wolfSSL_set_secret(WOLFSSL* ssl, word16 epoch, const byte* preMasterSecret, word32 preMasterSz, @@ -774,7 +775,7 @@ int wolfSSL_set_secret(WOLFSSL* ssl, word16 epoch, WOLFSSL_ENTER("wolfSSL_set_secret"); if (ssl == NULL || preMasterSecret == NULL || - preMasterSz == 0 || preMasterSz > ENCRYPT_LEN || + preMasterSz == 0 || preMasterSz > MAX_PREMASTER_SZ || clientRandom == NULL || serverRandom == NULL || suite == NULL) { ret = BAD_FUNC_ARG; @@ -787,19 +788,10 @@ int wolfSSL_set_secret(WOLFSSL* ssl, word16 epoch, ret = BAD_FUNC_ARG; } - if (ret == 0 && 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) { - ret = MEMORY_E; - } - } - if (ret == 0) { XMEMCPY(ssl->arrays->preMasterSecret, preMasterSecret, preMasterSz); XMEMSET(ssl->arrays->preMasterSecret + preMasterSz, 0, - ENCRYPT_LEN - preMasterSz); + MAX_PREMASTER_SZ - preMasterSz); ssl->arrays->preMasterSz = preMasterSz; XMEMCPY(ssl->arrays->clientRandom, clientRandom, RAN_LEN); XMEMCPY(ssl->arrays->serverRandom, serverRandom, RAN_LEN); @@ -812,6 +804,14 @@ int wolfSSL_set_secret(WOLFSSL* ssl, word16 epoch, if (ret == 0) ret = MakeTlsMasterSecret(ssl); + /* MakeTlsMasterSecret() does not clean it and the buffer now lives as + * long as the object. This is the multicast entry point, so what was + * copied in is a long-lived group secret. */ + if (ssl != NULL && ssl->arrays != NULL) { + ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ); + ssl->arrays->preMasterSz = 0; + } + if (ret == 0) { ssl->keys.encryptionOn = 1; ret = SetKeysSide(ssl, ENCRYPT_AND_DECRYPT_SIDE); diff --git a/src/ssl_api_pk.c b/src/ssl_api_pk.c index d5ac839f561..6499eea64a0 100644 --- a/src/ssl_api_pk.c +++ b/src/ssl_api_pk.c @@ -1835,7 +1835,10 @@ int wolfSSL_SetEnableDhKeyTest(WOLFSSL* ssl, int enable) * * @param [in] ctx SSL/TLS context object. * @param [in] keySz_bits Minimum DH key size in bits. No more than 16000 and - * a multiple of 8. + * a multiple of 8. A floor above what the build's + * math supports is accepted: it simply refuses every + * DH key a peer can offer, which is what a caller + * tightening its policy asked for. * @return WOLFSSL_SUCCESS on success. * @return BAD_FUNC_ARG when ctx is NULL or keySz_bits is invalid. * @return CRYPTO_POLICY_FORBIDDEN when below the active crypto-policy minimum. @@ -1861,7 +1864,10 @@ int wolfSSL_CTX_SetMinDhKey_Sz(WOLFSSL_CTX* ctx, word16 keySz_bits) * * @param [in] ssl SSL/TLS object. * @param [in] keySz_bits Minimum DH key size in bits. No more than 16000 and - * a multiple of 8. + * a multiple of 8. A floor above what the build's + * math supports is accepted: it simply refuses every + * DH key a peer can offer, which is what a caller + * tightening its policy asked for. * @return WOLFSSL_SUCCESS on success. * @return BAD_FUNC_ARG when ssl is NULL or keySz_bits is invalid. * @return CRYPTO_POLICY_FORBIDDEN when below the active crypto-policy minimum. @@ -1887,7 +1893,8 @@ int wolfSSL_SetMinDhKey_Sz(WOLFSSL* ssl, word16 keySz_bits) * * @param [in] ctx SSL/TLS context object. * @param [in] keySz_bits Maximum DH key size in bits. No more than 16000 and - * a multiple of 8. + * a multiple of 8, and no more than what the build's + * math supports. * @return WOLFSSL_SUCCESS on success. * @return BAD_FUNC_ARG when ctx is NULL or keySz_bits is invalid. */ @@ -1904,6 +1911,15 @@ int wolfSSL_CTX_SetMaxDhKey_Sz(WOLFSSL_CTX* ctx, word16 keySz_bits) } #endif /* WOLFSSL_SYS_CRYPTO_POLICY */ + /* The pre-master secret buffer is sized for the largest prime the build's + * math supports, so a ceiling above that cannot be honoured. Checked + * after the crypto policy so a request that breaks both is still reported + * as the policy violation it is. */ + if ((keySz_bits / 8) > MAX_DHKEY_SZ) { + WOLFSSL_MSG("Requested maximum DH key size exceeds MAX_DHKEY_SZ"); + return BAD_FUNC_ARG; + } + ctx->maxDhKeySz = keySz_bits / 8; return WOLFSSL_SUCCESS; } @@ -1912,7 +1928,8 @@ int wolfSSL_CTX_SetMaxDhKey_Sz(WOLFSSL_CTX* ctx, word16 keySz_bits) * * @param [in] ssl SSL/TLS object. * @param [in] keySz_bits Maximum DH key size in bits. No more than 16000 and - * a multiple of 8. + * a multiple of 8, and no more than what the build's + * math supports. * @return WOLFSSL_SUCCESS on success. * @return BAD_FUNC_ARG when ssl is NULL or keySz_bits is invalid. */ @@ -1929,6 +1946,15 @@ int wolfSSL_SetMaxDhKey_Sz(WOLFSSL* ssl, word16 keySz_bits) } #endif /* WOLFSSL_SYS_CRYPTO_POLICY */ + /* The pre-master secret buffer is sized for the largest prime the build's + * math supports, so a ceiling above that cannot be honoured. Checked + * after the crypto policy so a request that breaks both is still reported + * as the policy violation it is. */ + if ((keySz_bits / 8) > MAX_DHKEY_SZ) { + WOLFSSL_MSG("Requested maximum DH key size exceeds MAX_DHKEY_SZ"); + return BAD_FUNC_ARG; + } + ssl->options.maxDhKeySz = keySz_bits / 8; return WOLFSSL_SUCCESS; } diff --git a/src/tls.c b/src/tls.c index c3e10bdfb29..5d5a3bef6dc 100644 --- a/src/tls.c +++ b/src/tls.c @@ -142,6 +142,10 @@ #endif #ifdef WOLFSSL_HAVE_MLKEM #include + #ifdef WOLFSSL_TLS13 + /* internal.h sizes the buffer without this header in scope. */ + wc_static_assert(PMS_MLKEM_SZ == WC_ML_KEM_SS_SZ); + #endif #endif #if defined(WOLFSSL_RENESAS_TSIP_TLS) @@ -9736,6 +9740,15 @@ static int TLSX_KeyShare_ProcessDh(WOLFSSL* ssl, KeyShareEntry* keyShareEntry) return PEER_KEY_ERROR; } + /* DhAgree() writes the whole prime and takes no capacity, so refuse here + * rather than truncate later. Not reachable today: MAX_PREMASTER_SZ + * covers every group compiled in. */ + if (pSz > MAX_PREMASTER_SZ) { + WOLFSSL_MSG("FFDHE group larger than the pre-master secret buffer"); + WOLFSSL_ERROR_VERBOSE(PEER_KEY_ERROR); + return PEER_KEY_ERROR; + } + /* if DhKey is not setup, do it now */ if (keyShareEntry->key == NULL) { keyShareEntry->key = (DhKey*)XMALLOC(sizeof(DhKey), ssl->heap, @@ -10674,7 +10687,7 @@ static int TLSX_KeyShare_ProcessPqcHybridClient(WOLFSSL* ssl, } if (ret == 0) { - if ((ssl->arrays->preMasterSz + ssSzPqc) > ENCRYPT_LEN) { + if ((ssl->arrays->preMasterSz + ssSzPqc) > MAX_PREMASTER_SZ) { WOLFSSL_MSG("shared secret is too long."); ret = LENGTH_ERROR; } @@ -10706,7 +10719,7 @@ static int TLSX_KeyShare_ProcessPqcHybridClient(WOLFSSL* ssl, * intermediate keys in the error case. Do not use preMasterSz * here as it may already been set to the ECC shared secret size, * which would be too small due to the PQC offset case. */ - ForceZero(ssl->arrays->preMasterSecret, ENCRYPT_LEN); + ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ); /* Prevent FreeAll from freeing pointers owned by keyShareEntry. */ if (ecc_kse != NULL) @@ -10743,7 +10756,7 @@ static int TLSX_KeyShare_Process(WOLFSSL* ssl, KeyShareEntry* keyShareEntry) #endif /* reset the pre master secret size */ if (ssl->arrays->preMasterSz == 0) - ssl->arrays->preMasterSz = ENCRYPT_LEN; + ssl->arrays->preMasterSz = MAX_PREMASTER_SZ; /* Use Key Share Data from server. */ if (WOLFSSL_NAMED_GROUP_IS_FFDHE(keyShareEntry->group)) @@ -11440,7 +11453,7 @@ int TLSX_KeyShare_HandlePqcHybridKeyServer(WOLFSSL* ssl, #endif } - if (ret == 0 && ssl->arrays->preMasterSz + ssSzPqc > ENCRYPT_LEN) { + if (ret == 0 && ssl->arrays->preMasterSz + ssSzPqc > MAX_PREMASTER_SZ) { WOLFSSL_MSG("shared secret is too long."); ret = LENGTH_ERROR; } @@ -11504,7 +11517,7 @@ int TLSX_KeyShare_HandlePqcHybridKeyServer(WOLFSSL* ssl, * intermediate keys in the error case. Do not use preMasterSz * here as it may already been set to the ECC shared secret size, * which would be too small due to the PQC offset case. */ - ForceZero(ssl->arrays->preMasterSecret, ENCRYPT_LEN); + ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ); } TLSX_KeyShare_FreeAll(ecc_kse, ssl->heap); diff --git a/src/tls13.c b/src/tls13.c index d000adb23ed..2fff0b8c431 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -14032,7 +14032,7 @@ static int SendTls13Finished(WOLFSSL* ssl) ssl->kdfDeriveStep = TLS13_SEND_KDF_FIN_MASTER_SECRET; } /* Last use of preMasterSecret - zeroize as soon as possible. */ - ForceZero(ssl->arrays->preMasterSecret, ENCRYPT_LEN); + ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ); #ifdef WOLFSSL_EARLY_DATA #ifdef WOLFSSL_DTLS13 @@ -15903,7 +15903,7 @@ int DoTls13MsgDerives(WOLFSSL* ssl, byte type) } /* Zeroized only after the derive completed: a pend retry * still reads preMasterSecret. */ - ForceZero(ssl->arrays->preMasterSecret, ENCRYPT_LEN); + ForceZero(ssl->arrays->preMasterSecret, MAX_PREMASTER_SZ); ssl->kdfMsgStep = TLS13_MSG_KDF_FIN_MASTER_SECRET; } #ifdef WOLFSSL_EARLY_DATA diff --git a/tests/api.c b/tests/api.c index f0f30b70181..3962213a4a7 100644 --- a/tests/api.c +++ b/tests/api.c @@ -11637,13 +11637,11 @@ static int test_wolfSSL_clear_zeroizes_secrets(void) if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL)) { ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->masterSecret, SECRET_LEN), 0); - /* The pre-master secret is not part of that contract. */ - ExpectNotNull(ssl_s->arrays->preMasterSecret); } - if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL) && - (ssl_s->arrays->preMasterSecret != NULL)) { + if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL)) { + /* The pre-master secret is not part of that contract. */ ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->preMasterSecret, - ENCRYPT_LEN), 1); + MAX_PREMASTER_SZ), 1); } if (EXPECT_SUCCESS() && (ssl_s != NULL) && (ssl_s->arrays != NULL)) { /* The key schedule secret is not part of the contract either. */ @@ -11683,9 +11681,8 @@ static int test_wolfSSL_clear_zeroizes_secrets(void) ExpectIntEQ(test_clear_all_zero(ssl_s->arrays->exporterSecret, WC_MAX_DIGEST_SIZE), 1); #endif - ExpectNotNull(ssl_s->arrays->preMasterSecret); /* The key agreement routines read this as the room they have. */ - ExpectIntEQ((int)ssl_s->arrays->preMasterSz, ENCRYPT_LEN); + ExpectIntEQ((int)ssl_s->arrays->preMasterSz, MAX_PREMASTER_SZ); } wolfSSL_free(ssl_c); diff --git a/tests/api/test_dtls.c b/tests/api/test_dtls.c index 19b9d70cdc3..949d5c8482c 100644 --- a/tests/api/test_dtls.c +++ b/tests/api/test_dtls.c @@ -3829,6 +3829,101 @@ static unsigned char version_3[] = { }; #endif /* defined(WOLFSSL_DTLS) && defined(WOLFSSL_SESSION_EXPORT) */ +/* A DH ceiling above what this build's math supports is clamped and a + * negotiated size above it is rejected, while the floor is carried over as it + * stands. The fields are found by value, not by offset, so the export layout + * is not encoded twice. + */ +int test_wolfSSL_dtls_import_dh_key_sz(void) +{ + EXPECT_DECLS; +#if defined(WOLFSSL_DTLS) && defined(WOLFSSL_SESSION_EXPORT) && \ + !defined(NO_DH) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(WOLFSSL_NO_TLS12) + WOLFSSL_CTX* ctx = NULL; + WOLFSSL* ssl = NULL; + byte blob[sizeof(version_3)]; + word16 minSz = 0; + word16 maxSz = 0; + word16 keySz = 0; + int at = -1; + int hits = 0; + unsigned int i; + + /* version_3 is shared and mutable, so fail loudly if it was poisoned. */ + ExpectIntEQ(version_3[0], 0xA5); + ExpectIntEQ(version_3[1], 0xA3); + + /* Import the blob as it stands to learn the three values it carries. */ + ExpectNotNull(ctx = wolfSSL_CTX_new(wolfDTLSv1_2_server_method())); + ExpectNotNull(ssl = wolfSSL_new(ctx)); + ExpectIntGE(wolfSSL_dtls_import(ssl, version_3, sizeof(version_3)), 0); + if (ssl != NULL) { + minSz = ssl->options.minDhKeySz; + maxSz = ssl->options.maxDhKeySz; + keySz = ssl->options.dhKeySz; + } + wolfSSL_free(ssl); + ssl = NULL; + + /* Refuse to guess if the triple is not unique. The ceiling may have been + * clamped on import, so match it either way. */ + for (i = 0; i + 6 <= sizeof(version_3); i++) { + word16 rawMax = (word16)((version_3[i + 2] << 8) | version_3[i + 3]); + + if (((version_3[i + 0] << 8) | version_3[i + 1]) != minSz) + continue; + if (((version_3[i + 4] << 8) | version_3[i + 5]) != keySz) + continue; + if ((rawMax != maxSz) && + !((rawMax > MAX_DHKEY_SZ) && (maxSz == MAX_DHKEY_SZ))) + continue; + at = (int)i; + hits++; + } + ExpectIntEQ(hits, 1); + if (at < 0) + goto done; + + /* A ceiling this build cannot honour is clamped, and the import stands. */ + XMEMCPY(blob, version_3, sizeof(blob)); + blob[at + 2] = (byte)((MAX_DHKEY_SZ + 1) >> 8); + blob[at + 3] = (byte)((MAX_DHKEY_SZ + 1) & 0xff); + ExpectNotNull(ssl = wolfSSL_new(ctx)); + ExpectIntGE(wolfSSL_dtls_import(ssl, blob, sizeof(blob)), 0); + if (ssl != NULL) + ExpectIntEQ(ssl->options.maxDhKeySz, MAX_DHKEY_SZ); + wolfSSL_free(ssl); + ssl = NULL; + + /* The floor bounds no buffer, so it stands even above the ceiling: it + * only refuses more keys, which is the policy the peer exported. */ + XMEMCPY(blob, version_3, sizeof(blob)); + blob[at + 0] = (byte)((MAX_DHKEY_SZ + 1) >> 8); + blob[at + 1] = (byte)((MAX_DHKEY_SZ + 1) & 0xff); + ExpectNotNull(ssl = wolfSSL_new(ctx)); + ExpectIntGE(wolfSSL_dtls_import(ssl, blob, sizeof(blob)), 0); + if (ssl != NULL) + ExpectIntEQ(ssl->options.minDhKeySz, MAX_DHKEY_SZ + 1); + wolfSSL_free(ssl); + ssl = NULL; + + /* The negotiated size this build cannot represent is rejected. */ + XMEMCPY(blob, version_3, sizeof(blob)); + blob[at + 4] = (byte)((MAX_DHKEY_SZ + 1) >> 8); + blob[at + 5] = (byte)((MAX_DHKEY_SZ + 1) & 0xff); + ExpectNotNull(ssl = wolfSSL_new(ctx)); + ExpectIntLT(wolfSSL_dtls_import(ssl, blob, sizeof(blob)), 0); + wolfSSL_free(ssl); + ssl = NULL; + +done: + wolfSSL_free(ssl); + wolfSSL_CTX_free(ctx); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_dtls_export(void) { EXPECT_DECLS; @@ -3904,6 +3999,8 @@ int test_wolfSSL_dtls_export(void) ExpectIntLT(wolfSSL_dtls_import(ssl, version_3, sizeof(version_3)), 0); version_3[2]--; version_3[1] = 0XA0; ExpectIntLT(wolfSSL_dtls_import(ssl, version_3, sizeof(version_3)), 0); + /* Shared with every later test, so put the version byte back. */ + version_3[1] = 0xA3; wolfSSL_free(ssl); wolfSSL_CTX_free(ctx); @@ -13410,12 +13507,54 @@ int test_wolfSSL_set_secret(void) ExpectIntEQ(wolfSSL_CTX_mcast_set_member_id(ctx, 0), WOLFSSL_SUCCESS); ExpectNotNull(ssl = wolfSSL_new(ctx)); - /* Invalid arguments take the error path and return WOLFSSL_FATAL_ERROR. */ + /* Invalid arguments return WOLFSSL_FATAL_ERROR, a NULL object included: + * the cleanup must not assume the argument check passed. */ + ExpectIntEQ(wolfSSL_set_secret(NULL, 23, preMasterSecret, + sizeof(preMasterSecret), clientRandom, serverRandom, suite), + WOLFSSL_FATAL_ERROR); ExpectIntEQ(wolfSSL_set_secret(ssl, 23, NULL, sizeof(preMasterSecret), clientRandom, serverRandom, suite), WOLFSSL_FATAL_ERROR); ExpectIntEQ(wolfSSL_set_secret(ssl, 23, preMasterSecret, 0, clientRandom, serverRandom, suite), WOLFSSL_FATAL_ERROR); + /* A secret larger than the buffer is refused rather than copied in: the + * check between an untrusted size and an overflow. */ + ExpectIntEQ(wolfSSL_set_secret(ssl, 23, preMasterSecret, + MAX_PREMASTER_SZ + 1, clientRandom, serverRandom, suite), + WOLFSSL_FATAL_ERROR); + + /* The secret is not left in the object. wolfSSL_set_secret() ends in + * FreeHandshakeResources(), which releases the arrays unless saveArrays + * is set, so keeping them is what makes the wipe observable. */ + wolfSSL_KeepArrays(ssl); + ExpectIntEQ(wolfSSL_set_secret(ssl, 23, preMasterSecret, + sizeof(preMasterSecret), clientRandom, serverRandom, suite), + WOLFSSL_SUCCESS); + ExpectNotNull(ssl == NULL ? NULL : ssl->arrays); + if ((ssl != NULL) && (ssl->arrays != NULL)) { + word32 i; + int nonZero = 0; + + for (i = 0; i < MAX_PREMASTER_SZ; i++) { + if (ssl->arrays->preMasterSecret[i] != 0) + nonZero = 1; + } + ExpectIntEQ(nonZero, 0); + ExpectIntEQ(ssl->arrays->preMasterSz, 0); + } + + /* Once the handshake arrays are gone the call reports a bad argument + * instead of dereferencing NULL. */ + if (ssl != NULL) { + Arrays* saved = ssl->arrays; + + ssl->arrays = NULL; + ExpectIntEQ(wolfSSL_set_secret(ssl, 23, preMasterSecret, + sizeof(preMasterSecret), clientRandom, serverRandom, suite), + WOLFSSL_FATAL_ERROR); + ssl->arrays = saved; + } + wolfSSL_free(ssl); wolfSSL_CTX_free(ctx); #endif diff --git a/tests/api/test_dtls.h b/tests/api/test_dtls.h index 6b2dab438ea..a8a37b577d1 100644 --- a/tests/api/test_dtls.h +++ b/tests/api/test_dtls.h @@ -175,6 +175,7 @@ int test_dtls13_hrr_cookie_state_guard(void); int test_dtls12_cookie_secret_generate_fail(void); int test_dtls13_hrr_cookie_secret_generate_fail(void); int test_wolfSSL_dtls_export(void); +int test_wolfSSL_dtls_import_dh_key_sz(void); int test_wolfSSL_dtls_export_peers(void); int test_wolfSSL_dtls_import_state_extra_window_words(void); int test_wolfSSL_DTLS_either_side(void); @@ -250,6 +251,7 @@ int test_dtls_no_cookie_ch_pause(WOLFSSL* ssl, void* ctx); TEST_DECL_GROUP("dtls", test_dtls_memio_wolfio_invalid_peer), \ TEST_DECL_GROUP("dtls", test_dtls_set_session_min_downgrade), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_export), \ + TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_import_dh_key_sz), \ TEST_DECL_GROUP("dtls", test_wolfSSL_dtls_export_peers), \ TEST_DECL_GROUP("dtls", \ test_wolfSSL_dtls_import_state_extra_window_words), \ diff --git a/tests/api/test_ssl_pk.c b/tests/api/test_ssl_pk.c index 3f74c93fa25..9d9f0731809 100644 --- a/tests/api/test_ssl_pk.c +++ b/tests/api/test_ssl_pk.c @@ -242,6 +242,11 @@ int test_wolfSSL_SetMinDhKey_Sz(void) return EXPECT_RESULT(); } +/* The largest ceiling every build accepts: 4096 bits where the math reaches + * it, otherwise what this build's math tops out at. */ +#define TEST_MAX_DHKEY_BITS \ + ((MAX_DHKEY_SZ >= (4096 / 8)) ? 4096 : (MAX_DHKEY_SZ * 8)) + int test_wolfSSL_CTX_SetMaxDhKey_Sz(void) { EXPECT_DECLS; @@ -258,7 +263,21 @@ int test_wolfSSL_CTX_SetMaxDhKey_Sz(void) ExpectIntEQ(wolfSSL_CTX_SetMaxDhKey_Sz(ctx, 1001), WC_NO_ERR_TRACE(BAD_FUNC_ARG)); - ExpectIntEQ(wolfSSL_CTX_SetMaxDhKey_Sz(ctx, 4096), WOLFSSL_SUCCESS); + /* Driven off MAX_DHKEY_SZ rather than a fixed 4096: a build whose math + * tops out below that refuses the larger value, by design. */ + ExpectIntEQ(wolfSSL_CTX_SetMaxDhKey_Sz(ctx, (word16)TEST_MAX_DHKEY_BITS), + WOLFSSL_SUCCESS); + /* The value asked for is the value stored, not a silently reduced one. */ + ExpectIntEQ(ctx->maxDhKeySz, TEST_MAX_DHKEY_BITS / 8); + + /* A ceiling above what the build's math supports cannot be honoured, so + * it is refused rather than accepted and quietly lowered. */ + if (MAX_DHKEY_SZ < (16000 / 8)) { + ExpectIntEQ(wolfSSL_CTX_SetMaxDhKey_Sz(ctx, + (word16)((MAX_DHKEY_SZ + 1) * 8)), WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + /* The refused call left the previous ceiling alone. */ + ExpectIntEQ(ctx->maxDhKeySz, TEST_MAX_DHKEY_BITS / 8); + } wolfSSL_CTX_free(ctx); #endif @@ -291,7 +310,21 @@ int test_wolfSSL_SetMaxDhKey_Sz(void) ExpectIntEQ(wolfSSL_SetMaxDhKey_Sz(ssl, 1001), WC_NO_ERR_TRACE(BAD_FUNC_ARG)); - ExpectIntEQ(wolfSSL_SetMaxDhKey_Sz(ssl, 4096), WOLFSSL_SUCCESS); + /* Driven off MAX_DHKEY_SZ rather than a fixed 4096: a build whose math + * tops out below that refuses the larger value, by design. */ + ExpectIntEQ(wolfSSL_SetMaxDhKey_Sz(ssl, (word16)TEST_MAX_DHKEY_BITS), + WOLFSSL_SUCCESS); + /* The value asked for is the value stored, not a silently reduced one. */ + ExpectIntEQ(ssl->options.maxDhKeySz, TEST_MAX_DHKEY_BITS / 8); + + /* A ceiling above what the build's math supports cannot be honoured, so + * it is refused rather than accepted and quietly lowered. */ + if (MAX_DHKEY_SZ < (16000 / 8)) { + ExpectIntEQ(wolfSSL_SetMaxDhKey_Sz(ssl, + (word16)((MAX_DHKEY_SZ + 1) * 8)), WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + /* The refused call left the previous ceiling alone. */ + ExpectIntEQ(ssl->options.maxDhKeySz, TEST_MAX_DHKEY_BITS / 8); + } wolfSSL_free(ssl); wolfSSL_CTX_free(ctx); diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index 694a0362554..1d2714c5289 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -4852,3 +4852,114 @@ int test_tls12_aesgcm_record_nonce_unique(void) #endif return EXPECT_RESULT(); } + +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(NO_PSK) && \ + !defined(NO_DH) && !defined(WOLFSSL_NO_TLS12) && \ + defined(BUILD_TLS_DHE_PSK_WITH_AES_128_GCM_SHA256) && \ + defined(HAVE_SUPPORTED_CURVES) && defined(HAVE_ECC) && \ + !defined(NO_ECC_SECP) && (MAX_DHKEY_SZ >= 256) && \ + (ECC_MIN_KEY_SZ <= 256) && \ + (!defined(NO_ECC256) || defined(HAVE_ALL_CURVES)) +/* The overflow is about the largest prime the build accepts, so use the + * parameters that match MAX_DHKEY_SZ rather than pinning 4096 bits. */ +#if MAX_DHKEY_SZ >= 512 + #define TEST_DHE_PSK_MAX_DH_FILE "certs/dh4096.pem" + #define TEST_DHE_PSK_MAX_DH_BITS 4096 +#elif MAX_DHKEY_SZ >= 384 + #define TEST_DHE_PSK_MAX_DH_FILE "certs/dh3072.pem" + #define TEST_DHE_PSK_MAX_DH_BITS 3072 +#else + #define TEST_DHE_PSK_MAX_DH_FILE "certs/dh2048.pem" + #define TEST_DHE_PSK_MAX_DH_BITS 2048 +#endif + +static unsigned int test_dhe_psk_max_client_cb(WOLFSSL* ssl, const char* hint, + char* identity, unsigned int id_max, unsigned char* key, + unsigned int key_max) +{ + (void)ssl; + (void)hint; + + if ((id_max < sizeof("Client_identity")) || (key_max < MAX_PSK_KEY_LEN)) + return 0; + + XSTRLCPY(identity, "Client_identity", id_max); + XMEMSET(key, 0xA5, MAX_PSK_KEY_LEN); + + return MAX_PSK_KEY_LEN; +} + +static unsigned int test_dhe_psk_max_server_cb(WOLFSSL* ssl, + const char* identity, unsigned char* key, unsigned int key_max) +{ + (void)ssl; + (void)identity; + + if (key_max < MAX_PSK_KEY_LEN) + return 0; + + XMEMSET(key, 0xA5, MAX_PSK_KEY_LEN); + + return MAX_PSK_KEY_LEN; +} +#endif + +/* A DHE-PSK exchange fills the pre-master secret buffer with a two byte + * length, the DH shared secret, another two byte length and the PSK. With the + * largest DH parameters the build accepts and a maximum length PSK that is + * more than ENCRYPT_LEN, which the buffer used to be sized with, so the last + * two bytes of the key landed outside it. + * + * The client asks for a named group that is not FFDHE so that the server keeps + * the 4096 bit parameters it was given rather than negotiating a smaller + * group. + */ +int test_dhe_psk_max_premaster(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && !defined(NO_PSK) && \ + !defined(NO_DH) && !defined(WOLFSSL_NO_TLS12) && \ + defined(BUILD_TLS_DHE_PSK_WITH_AES_128_GCM_SHA256) && \ + defined(HAVE_SUPPORTED_CURVES) && defined(HAVE_ECC) && \ + !defined(NO_ECC_SECP) && (MAX_DHKEY_SZ >= 256) && \ + (ECC_MIN_KEY_SZ <= 256) && \ + (!defined(NO_ECC256) || defined(HAVE_ALL_CURVES)) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + struct test_memio_ctx test_ctx; + int groups[1] = { WOLFSSL_ECC_SECP256R1 }; + + /* Both lengths, the largest secret the peer can pick and the longest key + * have to fit. */ + ExpectTrue(MAX_PREMASTER_SZ >= + OPAQUE16_LEN + MAX_DHKEY_SZ + OPAQUE16_LEN + MAX_PSK_KEY_LEN); + /* The exchange this test drives has to fit as well. */ + ExpectTrue(MAX_PREMASTER_SZ >= OPAQUE16_LEN + + (TEST_DHE_PSK_MAX_DH_BITS / 8) + OPAQUE16_LEN + MAX_PSK_KEY_LEN); + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_c, "DHE-PSK-AES128-GCM-SHA256"), + WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_cipher_list(ssl_s, "DHE-PSK-AES128-GCM-SHA256"), + WOLFSSL_SUCCESS); + if (EXPECT_SUCCESS()) { + wolfSSL_set_psk_client_callback(ssl_c, test_dhe_psk_max_client_cb); + wolfSSL_set_psk_server_callback(ssl_s, test_dhe_psk_max_server_cb); + } + ExpectIntEQ(wolfSSL_SetTmpDH_file(ssl_s, TEST_DHE_PSK_MAX_DH_FILE, + WOLFSSL_FILETYPE_PEM), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_groups(ssl_c, groups, 1), WOLFSSL_SUCCESS); + + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* The peer picked the parameters that make the secret its largest. */ + ExpectIntEQ(wolfSSL_GetDhKey_Sz(ssl_c), TEST_DHE_PSK_MAX_DH_BITS); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 0c629ad536a..8737b87955e 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -48,6 +48,7 @@ int test_tls12_no_null_compression(void); int test_tls12_ec_point_formats_no_uncompressed(void); int test_tls12_ec_point_formats_no_uncompressed_non_ecc(void); int test_tls_fallback_scsv(void); +int test_dhe_psk_max_premaster(void); int test_dtls_fallback_scsv(void); int test_dtls_fallback_scsv_no_downgrade(void); int test_tls_fallback_scsv_no_downgrade(void); @@ -144,6 +145,7 @@ int test_tls_param_flags_crl_check(void); test_record_size_cache_invalidated_on_renegotiation), \ TEST_DECL_GROUP("tls", test_wolfSSL_get_shared_ciphers), \ TEST_DECL_GROUP("tls", test_tls12_aesgcm_record_nonce_unique), \ - TEST_DECL_GROUP("tls", test_tls_param_flags_crl_check) + TEST_DECL_GROUP("tls", test_tls_param_flags_crl_check), \ + TEST_DECL_GROUP("tls", test_dhe_psk_max_premaster) #endif /* TESTS_API_TEST_TLS_H */ diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 17ded2802ee..94a32d22ba7 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -10169,13 +10169,9 @@ int test_tls13_fragmented_session_ticket(void) * configurations that already release the arrays (e.g. no HAVE_SESSION_TICKET) * they are NULL at this point and the free is skipped. */ if (EXPECT_SUCCESS() && ssl_c->arrays != NULL) { - /* Zero before freeing so WOLFSSL_CHECK_MEM_ZERO builds don't abort. */ - if (ssl_c->arrays->preMasterSecret != NULL) { - ForceZero(ssl_c->arrays->preMasterSecret, ENCRYPT_LEN); - XFREE(ssl_c->arrays->preMasterSecret, ssl_c->heap, - DYNAMIC_TYPE_SECRET); - ssl_c->arrays->preMasterSecret = NULL; - } + /* The pre-master secret is carried at the end of the same allocation, + * so it goes with it. Zero before freeing so WOLFSSL_CHECK_MEM_ZERO + * builds don't abort. */ ForceZero(ssl_c->arrays, sizeof(Arrays)); XFREE(ssl_c->arrays, ssl_c->heap, DYNAMIC_TYPE_ARRAYS); ssl_c->arrays = NULL; @@ -13870,7 +13866,7 @@ int test_tls13_hs_secret_zeroized_psk_ke(void) WOLFSSL* ssl_s = NULL; struct test_memio_ctx test_ctx; - byte zeros[ENCRYPT_LEN]; + byte zeros[MAX_PREMASTER_SZ]; XMEMSET(&test_ctx, 0, sizeof(test_ctx)); XMEMSET(zeros, 0, sizeof(zeros)); @@ -13889,16 +13885,18 @@ int test_tls13_hs_secret_zeroized_psk_ke(void) wolfSSL_KeepArrays(ssl_c); wolfSSL_KeepArrays(ssl_s); if (EXPECT_SUCCESS()) { - XMEMSET(ssl_c->arrays->preMasterSecret, 0xAA, ENCRYPT_LEN); - XMEMSET(ssl_s->arrays->preMasterSecret, 0xAA, ENCRYPT_LEN); + XMEMSET(ssl_c->arrays->preMasterSecret, 0xAA, MAX_PREMASTER_SZ); + XMEMSET(ssl_s->arrays->preMasterSecret, 0xAA, MAX_PREMASTER_SZ); } ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); ExpectIntEQ(ssl_c->options.noPskDheKe, 1); ExpectIntEQ(ssl_c->arrays->preMasterSz, 0); - ExpectIntEQ(XMEMCMP(ssl_c->arrays->preMasterSecret, zeros, ENCRYPT_LEN), 0); - ExpectIntEQ(XMEMCMP(ssl_s->arrays->preMasterSecret, zeros, ENCRYPT_LEN), 0); + ExpectIntEQ(XMEMCMP(ssl_c->arrays->preMasterSecret, zeros, + MAX_PREMASTER_SZ), 0); + ExpectIntEQ(XMEMCMP(ssl_s->arrays->preMasterSecret, zeros, + MAX_PREMASTER_SZ), 0); wolfSSL_free(ssl_c); wolfSSL_free(ssl_s); @@ -14144,7 +14142,7 @@ int test_tls13_hs_secret_zeroized_sha384(void) WOLFSSL* ssl_s = NULL; struct test_memio_ctx test_ctx; - byte zeros[ENCRYPT_LEN]; + byte zeros[MAX_PREMASTER_SZ]; XMEMSET(&test_ctx, 0, sizeof(test_ctx)); XMEMSET(zeros, 0, sizeof(zeros)); @@ -14164,15 +14162,17 @@ int test_tls13_hs_secret_zeroized_sha384(void) wolfSSL_KeepArrays(ssl_c); wolfSSL_KeepArrays(ssl_s); if (EXPECT_SUCCESS()) { - XMEMSET(ssl_c->arrays->preMasterSecret, 0xAA, ENCRYPT_LEN); - XMEMSET(ssl_s->arrays->preMasterSecret, 0xAA, ENCRYPT_LEN); + XMEMSET(ssl_c->arrays->preMasterSecret, 0xAA, MAX_PREMASTER_SZ); + XMEMSET(ssl_s->arrays->preMasterSecret, 0xAA, MAX_PREMASTER_SZ); } ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); ExpectIntEQ(ssl_c->specs.hash_size, WC_SHA384_DIGEST_SIZE); - ExpectIntEQ(XMEMCMP(ssl_c->arrays->preMasterSecret, zeros, ENCRYPT_LEN), 0); - ExpectIntEQ(XMEMCMP(ssl_s->arrays->preMasterSecret, zeros, ENCRYPT_LEN), 0); + ExpectIntEQ(XMEMCMP(ssl_c->arrays->preMasterSecret, zeros, + MAX_PREMASTER_SZ), 0); + ExpectIntEQ(XMEMCMP(ssl_s->arrays->preMasterSecret, zeros, + MAX_PREMASTER_SZ), 0); wolfSSL_free(ssl_c); wolfSSL_free(ssl_s); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index f68691a1714..be797aa65ce 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1895,6 +1895,108 @@ enum Misc { }; +/* Size of the pre-master secret buffer. Not ENCRYPT_LEN, which also has to + * hold an RSA signature and so follows the biggest RSA key the build supports. + * Each term below is the largest secret one key exchange can produce, or 0 + * when it is not compiled in. */ +#define WOLFSSL_PMS_MAX2(a, b) (((a) > (b)) ? (a) : (b)) + +/* A hybrid key share appends a 32 byte ML-KEM secret to the ECDH one. A + * literal because wc_mlkem.h is out of scope here; an assert in src/tls.c + * pins it to WC_ML_KEM_SS_SZ. */ +#if defined(WOLFSSL_HAVE_MLKEM) && defined(WOLFSSL_TLS13) + #define PMS_MLKEM_SZ 32 +#else + #define PMS_MLKEM_SZ 0 +#endif + +/* TLS 1.2 takes the parameters from the peer, up to MAX_DHKEY_SZ. TLS 1.3 + * takes the group from the peer's key share and never checks it against + * maxDhKeySz, so the largest group compiled in has to fit as well. */ +#if defined(NO_DH) + #define PMS_DH_SZ 0 +#elif defined(HAVE_FFDHE_8192) + #define PMS_DH_SZ WOLFSSL_PMS_MAX2(MAX_DHKEY_SZ, 8192 / 8) +#elif defined(HAVE_FFDHE_6144) + #define PMS_DH_SZ WOLFSSL_PMS_MAX2(MAX_DHKEY_SZ, 6144 / 8) +#elif defined(HAVE_FFDHE_4096) + #define PMS_DH_SZ WOLFSSL_PMS_MAX2(MAX_DHKEY_SZ, 4096 / 8) +#elif defined(HAVE_FFDHE_3072) + #define PMS_DH_SZ WOLFSSL_PMS_MAX2(MAX_DHKEY_SZ, 3072 / 8) +#elif defined(HAVE_FFDHE_2048) + #define PMS_DH_SZ WOLFSSL_PMS_MAX2(MAX_DHKEY_SZ, 2048 / 8) +#else + #define PMS_DH_SZ MAX_DHKEY_SZ +#endif + +/* Largest ECDH secret, taken per curve rather than from the widest one: + * MAX_ECC_BYTES follows the configured ECC key sizes, and a build can enable + * Curve448 alongside a smaller ECC. */ +#ifdef HAVE_ECC + #define PMS_ECC_SZ MAX_ECC_BYTES +#else + #define PMS_ECC_SZ 0 +#endif +#ifdef HAVE_CURVE25519 + #define PMS_X25519_SZ CURVE25519_KEYSIZE +#else + #define PMS_X25519_SZ 0 +#endif +#ifdef HAVE_CURVE448 + #define PMS_X448_SZ CURVE448_KEY_SIZE +#else + #define PMS_X448_SZ 0 +#endif +#define PMS_ECDH_SZ (WOLFSSL_PMS_MAX2(PMS_ECC_SZ, \ + WOLFSSL_PMS_MAX2(PMS_X25519_SZ, PMS_X448_SZ)) + \ + PMS_MLKEM_SZ) + +/* A PSK only exchange pads with a zeroed copy of the key, and every PSK suite + * frames the two secrets with a length each. */ +#ifndef NO_PSK + #define PMS_PSK_SZ MAX_PSK_KEY_LEN + #define PMS_PSK_EXTRA (MAX_PSK_KEY_LEN + OPAQUE16_LEN * 2) +#else + #define PMS_PSK_SZ 0 + #define PMS_PSK_EXTRA 0 +#endif + +/* wolfSSL_set_secret() has always accepted up to ENCRYPT_LEN bytes. */ +#ifdef WOLFSSL_MULTICAST + #define PMS_MCAST_SZ ENCRYPT_LEN +#else + #define PMS_MCAST_SZ 0 +#endif + +/* A GenPreMasterCb writes the secret itself. ENCRYPT_LEN stays the default so + * a callback written against the old contract cannot be overrun; an + * integration that knows better sets this smaller. */ +#ifdef HAVE_PK_CALLBACKS + #ifndef WOLFSSL_MAX_GEN_PREMASTER_SZ + #define WOLFSSL_MAX_GEN_PREMASTER_SZ ENCRYPT_LEN + #endif + #define PMS_GEN_CB_SZ WOLFSSL_MAX_GEN_PREMASTER_SZ +#else + #define PMS_GEN_CB_SZ 0 +#endif + +/* SECRET_LEN is the floor: the RSA key exchange works with a master secret + * sized buffer, and TLS 1.3 reuses this buffer for the handshake secret, + * which is one MAC hash and never longer than SHA-384. */ +#define PMS_KX_SZ \ + WOLFSSL_PMS_MAX2(WOLFSSL_PMS_MAX2(SECRET_LEN, PMS_DH_SZ), \ + WOLFSSL_PMS_MAX2(PMS_ECDH_SZ, PMS_PSK_SZ)) +#define MAX_PREMASTER_SZ \ + WOLFSSL_PMS_MAX2(WOLFSSL_PMS_MAX2(PMS_MCAST_SZ, PMS_GEN_CB_SZ), \ + PMS_KX_SZ + PMS_PSK_EXTRA) + +/* The PSK layout - length, secret, length, key - is what decides the size, + * and getting it wrong is what overran the buffer before. */ +#if !defined(NO_PSK) && !defined(NO_DH) +wc_static_assert(MAX_PREMASTER_SZ >= + OPAQUE16_LEN + MAX_DHKEY_SZ + OPAQUE16_LEN + MAX_PSK_KEY_LEN); +#endif + /* Size of the data to authenticate */ #if defined(WOLFSSL_DTLS) && defined(WOLFSSL_DTLS_CID) #define AEAD_AUTH_DATA_SZ WOLFSSL_TLS_AEAD_CID_AAD_SZ @@ -5917,7 +6019,9 @@ struct Options { }; typedef struct Arrays { - byte* preMasterSecret; + /* In the struct, not a second allocation, so nothing tracks or frees + * it separately. */ + byte preMasterSecret[MAX_PREMASTER_SZ]; word32 preMasterSz; /* differs for DH, actual size */ #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) word32 psk_keySz; /* actual size */