feedc0de/wolfssl
1
0
Fork 0
forked from wolfSSL/wolfssl
Code Pull Requests Activity

Fix misc memory issues

Browse Source 
- Make `InternalTicket` memory alignment independent
This commit is contained in:
Juliusz Sosinowicz
2022-02-14 19:13:08 +01:00
parent b402102e58
commit 617eda9d44
4 changed files with 68 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
configure.ac
View File

@ -6829,7 +6829,7 @@ AS_CASE(["$CFLAGS $CPPFLAGS $AM_CFLAGS"],[*'OPENSSL_COMPATIBLE_DEFAULTS'*],
[ENABLED_OPENSSL_COMPATIBLE_DEFAULTS=yes]) [ENABLED_OPENSSL_COMPATIBLE_DEFAULTS=yes])
if test "x$ENABLED_OPENSSL_COMPATIBLE_DEFAULTS" = "xyes" if test "x$ENABLED_OPENSSL_COMPATIBLE_DEFAULTS" = "xyes"
then then
AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_TRUST_PEER_CERT -DWOLFSSL_TLS13_MIDDLEBOX_COMPAT" AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_TRUST_PEER_CERT"
AM_CFLAGS="$AM_CFLAGS -DNO_SESSION_CACHE_REF" AM_CFLAGS="$AM_CFLAGS -DNO_SESSION_CACHE_REF"
ENABLED_TRUSTED_PEER_CERT=yes ENABLED_TRUSTED_PEER_CERT=yes
fi fi

86
src/internal.c
View File

@ -20206,12 +20206,8 @@ const char* wolfSSL_ERR_reason_error_string(unsigned long e)
case SIDE_ERROR : case SIDE_ERROR :
return "wrong client/server type"; return "wrong client/server type";
case NO_PEER_CERT : case NO_PEER_CERT : /* OpenSSL compatibility expects this exact text */
#ifndef OPENSSL_EXTRA
return "peer didn't send cert";
#else
return "peer did not return a certificate"; return "peer did not return a certificate";
#endif
case UNKNOWN_HANDSHAKE_TYPE : case UNKNOWN_HANDSHAKE_TYPE :
return "weird handshake type"; return "weird handshake type";
@ -20525,12 +20521,8 @@ const char* wolfSSL_ERR_reason_error_string(unsigned long e)
case MISSING_HANDSHAKE_DATA: case MISSING_HANDSHAKE_DATA:
return "The handshake message is missing required data"; return "The handshake message is missing required data";
case BAD_BINDER: case BAD_BINDER: /* OpenSSL compatibility expects this exact text */
#ifndef OPENSSL_EXTRA
return "Binder value does not match value server calculated";
#else
return "binder does not verify"; return "binder does not verify";
#endif
case EXT_NOT_ALLOWED: case EXT_NOT_ALLOWED:
return "Extension type not allowed in handshake message type"; return "Extension type not allowed in handshake message type";
@ -29955,15 +29947,13 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
#endif #endif
#endif #endif
ret = MatchSuite(ssl, &clSuites);
#ifdef OPENSSL_EXTRA #ifdef OPENSSL_EXTRA
/* Give user last chance to provide a cert for cipher selection */ /* Give user last chance to provide a cert for cipher selection */
if (ret == 0)
ret = CertSetupCbWrapper(ssl);
/* Call again in case user changes affect cipher selection */
if (ret == 0 && ssl->ctx->certSetupCb != NULL) if (ret == 0 && ssl->ctx->certSetupCb != NULL)
ret = MatchSuite(ssl, &clSuites); ret = CertSetupCbWrapper(ssl);
#endif #endif
if (ret == 0)
ret = MatchSuite(ssl, &clSuites);
#ifdef WOLFSSL_EXTRA_ALERTS #ifdef WOLFSSL_EXTRA_ALERTS
if (ret == BUFFER_ERROR) if (ret == BUFFER_ERROR)
@ -30504,19 +30494,21 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
WOLFSSL_TICKET_IV_SZ + WOLFSSL_TICKET_MAC_SZ + LENGTH_SZ) WOLFSSL_TICKET_IV_SZ + WOLFSSL_TICKET_MAC_SZ + LENGTH_SZ)
#define WOLFSSL_TICKET_ENC_SZ (SESSION_TICKET_LEN - WOLFSSL_TICKET_FIXED_SZ) #define WOLFSSL_TICKET_ENC_SZ (SESSION_TICKET_LEN - WOLFSSL_TICKET_FIXED_SZ)
/* our ticket format */ /* Our ticket format. All members need to be a byte or array of byte to
* avoid alignment issues */
typedef struct InternalTicket { typedef struct InternalTicket {
ProtocolVersion pv; /* version when ticket created */ ProtocolVersion pv; /* version when ticket created */
byte suite[SUITE_LEN]; /* cipher suite when created */ byte suite[SUITE_LEN]; /* cipher suite when created */
byte msecret[SECRET_LEN]; /* master secret */ byte msecret[SECRET_LEN]; /* master secret */
word32 timestamp; /* born on */ byte timestamp[TIMESTAMP_LEN]; /* born on */
word16 haveEMS; /* have extended master secret */ byte haveEMS; /* have extended master secret */
#ifdef WOLFSSL_TLS13 #ifdef WOLFSSL_TLS13
word32 ageAdd; /* Obfuscation of age */ byte ageAdd[AGEADD_LEN]; /* Obfuscation of age */
word16 namedGroup; /* Named group used */ byte namedGroup[NAMEDGREOUP_LEN]; /* Named group used */
TicketNonce ticketNonce; /* Ticket nonce */ TicketNonce ticketNonce; /* Ticket nonce */
#ifdef WOLFSSL_EARLY_DATA #ifdef WOLFSSL_EARLY_DATA
word32 maxEarlyDataSz; /* Max size of early data */ byte maxEarlyDataSz[MAXEARLYDATASZ_LEN]; /* Max size of
* early data */
#endif #endif
#endif #endif
#ifdef WOLFSSL_TICKET_HAVE_ID #ifdef WOLFSSL_TICKET_HAVE_ID
@ -30581,7 +30573,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
it.suite[1] = ssl->options.cipherSuite; it.suite[1] = ssl->options.cipherSuite;
#ifdef WOLFSSL_EARLY_DATA #ifdef WOLFSSL_EARLY_DATA
it.maxEarlyDataSz = ssl->options.maxEarlyDataSz; c32toa(ssl->options.maxEarlyDataSz, it.maxEarlyDataSz);
#endif #endif
if (!ssl->options.tls1_3) { if (!ssl->options.tls1_3) {
@ -30598,9 +30590,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
sizeof(it.ageAdd)); sizeof(it.ageAdd));
if (ret != 0) if (ret != 0)
return BAD_TICKET_ENCRYPT; return BAD_TICKET_ENCRYPT;
ssl->session->ticketAdd = it.ageAdd; ato32(it.ageAdd, &ssl->session->ticketAdd);
it.namedGroup = ssl->session->namedGroup; c16toa(ssl->session->namedGroup, it.namedGroup);
it.timestamp = TimeNowInMilliseconds(); c32toa(TimeNowInMilliseconds(), it.timestamp);
/* Resumption master secret. */ /* Resumption master secret. */
XMEMCPY(it.msecret, ssl->session->masterSecret, SECRET_LEN); XMEMCPY(it.msecret, ssl->session->masterSecret, SECRET_LEN);
XMEMCPY(&it.ticketNonce, &ssl->session->ticketNonce, XMEMCPY(&it.ticketNonce, &ssl->session->ticketNonce,
@ -30721,7 +30713,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
int DoClientTicket(WOLFSSL* ssl, const byte* input, word32 len) int DoClientTicket(WOLFSSL* ssl, const byte* input, word32 len)
{ {
ExternalTicket* et; ExternalTicket* et;
InternalTicket it; InternalTicket* it;
int ret; int ret;
int outLen; int outLen;
word16 inLen; word16 inLen;
@ -30767,19 +30759,17 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
return BAD_TICKET_KEY_CB_SZ; return BAD_TICKET_KEY_CB_SZ;
} }
/* copy the decrypted ticket to avoid alignment issues */ it = (InternalTicket*)et->enc_ticket;
XMEMCPY(&it, et->enc_ticket, sizeof(InternalTicket));
ForceZero(et->enc_ticket, sizeof(it));
/* get master secret */ /* get master secret */
if (ret == WOLFSSL_TICKET_RET_OK || ret == WOLFSSL_TICKET_RET_CREATE) { if (ret == WOLFSSL_TICKET_RET_OK || ret == WOLFSSL_TICKET_RET_CREATE) {
if (ssl->version.minor < it.pv.minor) { if (ssl->version.minor < it->pv.minor) {
ForceZero(&it, sizeof(it)); ForceZero(&it, sizeof(it));
WOLFSSL_MSG("Ticket has greater version"); WOLFSSL_MSG("Ticket has greater version");
return VERSION_ERROR; return VERSION_ERROR;
} }
else if (ssl->version.minor > it.pv.minor) { else if (ssl->version.minor > it->pv.minor) {
if (IsAtLeastTLSv1_3(it.pv) != IsAtLeastTLSv1_3(ssl->version)) { if (IsAtLeastTLSv1_3(it->pv) != IsAtLeastTLSv1_3(ssl->version)) {
ForceZero(&it, sizeof(it)); ForceZero(&it, sizeof(it));
WOLFSSL_MSG("Tickets cannot be shared between " WOLFSSL_MSG("Tickets cannot be shared between "
"TLS 1.3 and TLS 1.2 and lower"); "TLS 1.3 and TLS 1.2 and lower");
@ -30794,17 +30784,17 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
WOLFSSL_MSG("Downgrading protocol due to ticket"); WOLFSSL_MSG("Downgrading protocol due to ticket");
if (it.pv.minor < ssl->options.minDowngrade) { if (it->pv.minor < ssl->options.minDowngrade) {
ForceZero(&it, sizeof(it)); ForceZero(&it, sizeof(it));
return VERSION_ERROR; return VERSION_ERROR;
} }
ssl->version.minor = it.pv.minor; ssl->version.minor = it->pv.minor;
} }
#ifdef WOLFSSL_TICKET_HAVE_ID #ifdef WOLFSSL_TICKET_HAVE_ID
{ {
ssl->session->haveAltSessionID = 1; ssl->session->haveAltSessionID = 1;
XMEMCPY(ssl->session->altSessionID, it.id, ID_LEN); XMEMCPY(ssl->session->altSessionID, it->id, ID_LEN);
if (wolfSSL_GetSession(ssl, NULL, 1) != NULL) { if (wolfSSL_GetSession(ssl, NULL, 1) != NULL) {
WOLFSSL_MSG("Found session matching the session id" WOLFSSL_MSG("Found session matching the session id"
" found in the ticket"); " found in the ticket");
@ -30817,31 +30807,31 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
#endif #endif
if (!IsAtLeastTLSv1_3(ssl->version)) { if (!IsAtLeastTLSv1_3(ssl->version)) {
XMEMCPY(ssl->arrays->masterSecret, it.msecret, SECRET_LEN); XMEMCPY(ssl->arrays->masterSecret, it->msecret, SECRET_LEN);
/* Copy the haveExtendedMasterSecret property from the ticket to /* Copy the haveExtendedMasterSecret property from the ticket to
* the saved session, so the property may be checked later. */ * the saved session, so the property may be checked later. */
ssl->session->haveEMS = it.haveEMS; ssl->session->haveEMS = it->haveEMS;
ato32((const byte*)&it.timestamp, &ssl->session->bornOn); ato32((const byte*)&it->timestamp, &ssl->session->bornOn);
#ifndef NO_RESUME_SUITE_CHECK #ifndef NO_RESUME_SUITE_CHECK
ssl->session->cipherSuite0 = it.suite[0]; ssl->session->cipherSuite0 = it->suite[0];
ssl->session->cipherSuite = it.suite[1]; ssl->session->cipherSuite = it->suite[1];
#endif #endif
} }
else { else {
#ifdef WOLFSSL_TLS13 #ifdef WOLFSSL_TLS13
/* Restore information to renegotiate. */ /* Restore information to renegotiate. */
ssl->session->ticketSeen = it.timestamp; ato32(it->timestamp, &ssl->session->ticketSeen);
ssl->session->ticketAdd = it.ageAdd; ato32(it->ageAdd, &ssl->session->ticketAdd);
ssl->session->cipherSuite0 = it.suite[0]; ssl->session->cipherSuite0 = it->suite[0];
ssl->session->cipherSuite = it.suite[1]; ssl->session->cipherSuite = it->suite[1];
#ifdef WOLFSSL_EARLY_DATA #ifdef WOLFSSL_EARLY_DATA
ssl->session->maxEarlyDataSz = it.maxEarlyDataSz; ato32(it->maxEarlyDataSz, &ssl->session->maxEarlyDataSz);
#endif #endif
/* Resumption master secret. */ /* Resumption master secret. */
XMEMCPY(ssl->session->masterSecret, it.msecret, SECRET_LEN); XMEMCPY(ssl->session->masterSecret, it->msecret, SECRET_LEN);
XMEMCPY(&ssl->session->ticketNonce, &it.ticketNonce, XMEMCPY(&ssl->session->ticketNonce, &it->ticketNonce,
sizeof(TicketNonce)); sizeof(TicketNonce));
ssl->session->namedGroup = it.namedGroup; ato16(it->namedGroup, &ssl->session->namedGroup);
#endif #endif
} }
} }

27
src/ssl.c
View File

@ -15980,6 +15980,8 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
WOLFSSL_X509* peer = NULL; WOLFSSL_X509* peer = NULL;
#endif #endif
#ifdef HAVE_SESSION_TICKET #ifdef HAVE_SESSION_TICKET
byte* cacheTicBuff = NULL;
byte ticBuffUsed = 0;
byte* ticBuff = NULL; byte* ticBuff = NULL;
int ticLen = 0; int ticLen = 0;
#endif #endif
@ -16004,7 +16006,7 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
ticLen = addSession->ticketLen; ticLen = addSession->ticketLen;
/* Alloc Memory here to avoid syscalls during lock */ /* Alloc Memory here to avoid syscalls during lock */
if (ticLen > SESSION_TICKET_LEN) { if (ticLen > SESSION_TICKET_LEN) {
ticBuff = (byte*)XMALLOC(ticLen, addSession->heap, ticBuff = (byte*)XMALLOC(ticLen, NULL,
DYNAMIC_TYPE_SESSION_TICK); DYNAMIC_TYPE_SESSION_TICK);
if (ticBuff == NULL) { if (ticBuff == NULL) {
return MEMORY_E; return MEMORY_E;
@ -16016,7 +16018,7 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
if (ret != 0) { if (ret != 0) {
WOLFSSL_MSG("Hash session failed"); WOLFSSL_MSG("Hash session failed");
#ifdef HAVE_SESSION_TICKET #ifdef HAVE_SESSION_TICKET
XFREE(ticBuff, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); XFREE(ticBuff, NULL, DYNAMIC_TYPE_SESSION_TICK);
#endif #endif
return ret; return ret;
} }
@ -16024,7 +16026,7 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
sessRow = &SessionCache[row]; sessRow = &SessionCache[row];
if (SESSION_ROW_LOCK(sessRow) != 0) { if (SESSION_ROW_LOCK(sessRow) != 0) {
#ifdef HAVE_SESSION_TICKET #ifdef HAVE_SESSION_TICKET
XFREE(ticBuff, addSession->heap, DYNAMIC_TYPE_SESSION_TICK); XFREE(ticBuff, NULL, DYNAMIC_TYPE_SESSION_TICK);
#endif #endif
WOLFSSL_MSG("Session row lock failed"); WOLFSSL_MSG("Session row lock failed");
return BAD_MUTEX_E; return BAD_MUTEX_E;
@ -16058,7 +16060,12 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
cacheSession->peer = NULL; cacheSession->peer = NULL;
#endif #endif
#ifdef HAVE_SESSION_TICKET #ifdef HAVE_SESSION_TICKET
if (ticBuff != NULL) { /* If we can re-use the existing buffer in cacheSession then we won't touch
* ticBuff at all making it a very cheap malloc/free. The page on a modern
* OS will most likely not even be allocated to the process. */
if (ticBuff != NULL && cacheSession->ticketLenAlloc < ticLen) {
cacheTicBuff = cacheSession->ticket;
ticBuffUsed = 1;
cacheSession->ticket = ticBuff; cacheSession->ticket = ticBuff;
cacheSession->ticketLenAlloc = ticLen; cacheSession->ticketLenAlloc = ticLen;
} }
@ -16073,6 +16080,7 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
sizeof(x509_buffer) * cacheSession->chain.count); sizeof(x509_buffer) * cacheSession->chain.count);
} }
#endif /* SESSION_CERTS */ #endif /* SESSION_CERTS */
cacheSession->heap = NULL;
/* Copy data into the cache object */ /* Copy data into the cache object */
ret = wolfSSL_DupSession(addSession, cacheSession, 1) == WOLFSSL_FAILURE; ret = wolfSSL_DupSession(addSession, cacheSession, 1) == WOLFSSL_FAILURE;
@ -16089,7 +16097,7 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
} }
} }
#ifdef HAVE_SESSION_TICKET #ifdef HAVE_SESSION_TICKET
else if (ticBuff != NULL) { else if (ticBuffUsed) {
/* Error occured. Need to clean up the ticket buffer. */ /* Error occured. Need to clean up the ticket buffer. */
cacheSession->ticket = cacheSession->_staticTicket; cacheSession->ticket = cacheSession->_staticTicket;
cacheSession->ticketLenAlloc = 0; cacheSession->ticketLenAlloc = 0;
@ -16109,6 +16117,13 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz,
} }
#endif #endif
#ifdef HAVE_SESSION_TICKET
if (ticBuff != NULL && !ticBuffUsed)
XFREE(ticBuff, NULL, DYNAMIC_TYPE_SESSION_TICK);
if (cacheTicBuff != NULL)
XFREE(cacheTicBuff, NULL, DYNAMIC_TYPE_SESSION_TICK);
#endif
#if defined(SESSION_CERTS) && defined(OPENSSL_EXTRA) #if defined(SESSION_CERTS) && defined(OPENSSL_EXTRA)
if (peer != NULL) { if (peer != NULL) {
wolfSSL_X509_free(peer); wolfSSL_X509_free(peer);
@ -16203,7 +16218,7 @@ void AddSession(WOLFSSL* ssl)
WC_RNG* rng = NULL; WC_RNG* rng = NULL;
if (ssl->rng != NULL) if (ssl->rng != NULL)
rng = ssl->rng; rng = ssl->rng;
#ifdef HAVE_GLOBAL_RNG #if defined(HAVE_GLOBAL_RNG) && defined(OPENSSL_EXTRA)
else if (initGlobalRNG == 1 || wolfSSL_RAND_Init() == WOLFSSL_SUCCESS) { else if (initGlobalRNG == 1 || wolfSSL_RAND_Init() == WOLFSSL_SUCCESS) {
rng = &globalRNG; rng = &globalRNG;
} }

8
wolfssl/internal.h
View File

@ -1185,6 +1185,14 @@ enum Misc {
HELLO_EXT_EXTMS = 0x0017, /* ID for the extended master secret ext */ HELLO_EXT_EXTMS = 0x0017, /* ID for the extended master secret ext */
SECRET_LEN = WOLFSSL_MAX_MASTER_KEY_LENGTH, SECRET_LEN = WOLFSSL_MAX_MASTER_KEY_LENGTH,
/* pre RSA and all master */ /* pre RSA and all master */
TIMESTAMP_LEN = 4, /* timestamp size in ticket */
#ifdef WOLFSSL_TLS13
AGEADD_LEN = 4, /* ageAdd size in ticket */
NAMEDGREOUP_LEN = 2, /* namedGroup size in ticket */
#ifdef WOLFSSL_EARLY_DATA
MAXEARLYDATASZ_LEN = 4, /* maxEarlyDataSz size in ticket */
#endif
#endif
#ifdef HAVE_PQC #ifdef HAVE_PQC
ENCRYPT_LEN = 1500, /* allow 1500 bit static buffer for falcon */ ENCRYPT_LEN = 1500, /* allow 1500 bit static buffer for falcon */
#else #else