Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 29 additions & 4 deletions src/internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -8271,13 +8271,32 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
}
#endif

/* Lazily allocating and deriving ctx->suites mutates shared CTX state.
Comment thread
kareem-wolfssl marked this conversation as resolved.
* A CTX is commonly configured once and then shared by many threads
* that each call wolfSSL_new(), so guard the check and initialization
* with the CTX mutex. Without it, two threads can both observe
* ctx->suites == NULL and both allocate, leaking the Suites that loses
* the race. */
ret = wolfSSL_RefWithMutexLock(&ctx->ref);
if (ret != 0) {
WOLFSSL_MSG("Failed to lock CTX mutex for suites init");
return ret;
}
if (ctx->suites == NULL) {
/* suites */
ret = AllocateCtxSuites(ctx);
if (ret != 0)
return ret;
InitSSL_CTX_Suites(ctx);
if (ret == 0)
InitSSL_CTX_Suites(ctx);
}
if (wolfSSL_RefWithMutexUnlock(&ctx->ref) != 0) {
WOLFSSL_MSG("Failed to unlock CTX mutex after suites init");
/* A stuck lock would deadlock the up_ref in SetSSL_CTX() below, so
* surface it as an error. Keep any earlier suites-init error, as
* that is the more meaningful failure. */
if (ret == 0)
ret = BAD_MUTEX_E;
}
if (ret != 0)
return ret;
#ifdef OPENSSL_ALL
ssl->suitesStack = NULL;
#endif
Expand Down Expand Up @@ -8632,6 +8651,12 @@ int AllocKey(WOLFSSL* ssl, int type, void** pKey)
return MEMORY_E;
}

/* Zero the allocation before initializing. XMALLOC does not clear memory,
* and the key-specific init below may fail before it has zeroed/initialized
* the structure's members. Starting from all-zero makes any partial-init
* failure safe to free. */
XMEMSET(*pKey, 0, sz);

/* Initialize key */
switch (type) {
#ifndef NO_RSA
Expand Down
15 changes: 8 additions & 7 deletions src/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -8472,13 +8472,6 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
if (ssl->ctx == ctx)
return ssl->ctx;

if (ctx->suites == NULL) {
/* suites */
if (AllocateCtxSuites(ctx) != 0)
return NULL;
InitSSL_CTX_Suites(ctx);
}

wolfSSL_RefWithMutexInc(&ctx->ref, &ret);
#ifdef WOLFSSL_REFCNT_ERROR_RETURN
if (ret != 0) {
Expand All @@ -8489,6 +8482,14 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
#else
(void)ret;
#endif

if (ctx->suites == NULL) {
/* suites */
if (AllocateCtxSuites(ctx) != 0)
return NULL;
InitSSL_CTX_Suites(ctx);
}

if (ssl->ctx != NULL)
wolfSSL_CTX_free(ssl->ctx);
ssl->ctx = ctx;
Expand Down
13 changes: 11 additions & 2 deletions src/ssl_api_cert.c
Original file line number Diff line number Diff line change
Expand Up @@ -2721,8 +2721,13 @@ int wolfSSL_get0_chain_certs(WOLFSSL *ssl, WOLF_STACK_OF(WOLFSSL_X509) **sk)
InitDecodedCert(cert, (byte*)in, (word32)len, NULL);
if ((ret = ParseCertRelative(cert, CERT_TYPE, 0, NULL, NULL)) == 0) {
/* Check if x509 was not previously initialized by wolfSSL_X509_new() */
if (x509->dynamicMemory != TRUE)
InitX509(x509, 0, NULL);
if (x509->dynamicMemory != TRUE) {
/* A non-dynamic x509 (e.g. ssl->peerCert) may already hold
* decoded contents. InitX509 only zeroes the pointers, so free
* the existing allocations first to avoid orphaning them. */
FreeX509(x509);
InitX509(x509, 0, x509->heap);
}
ret = CopyDecodedToX509(x509, cert);
}
FreeDecodedCert(cert);
Expand Down Expand Up @@ -2755,6 +2760,10 @@ int wolfSSL_get0_chain_certs(WOLFSSL *ssl, WOLF_STACK_OF(WOLFSSL_X509) **sk)
ret = wolfSSL_X509_dup(&ssl->peerCert);
#ifdef SESSION_CERTS
else if (ssl->session->chain.count > 0) {
/* Re-decoding into ssl->peerCert would orphan any contents it
* already holds (e.g. the handshake's copy). Free them first. */
FreeX509(&ssl->peerCert);
InitX509(&ssl->peerCert, 0, ssl->heap);
if (DecodeToX509(&ssl->peerCert,
ssl->session->chain.certs[0].buffer,
ssl->session->chain.certs[0].length) == 0) {
Expand Down
Loading