From 0eb59d5c35219c62ae166ce893b47efc6e8d72e3 Mon Sep 17 00:00:00 2001 From: Nickolas Lapp Date: Fri, 11 Mar 2016 09:50:46 -0700 Subject: [PATCH] Fix rand num generation on MacOS, Improve organization with tic storage --- scripts/openssl.test | 4 +-- src/internal.c | 68 ++++++++++++++++++++++---------------- src/ssl.c | 79 +++++++++++++++++++++++++++----------------- src/tls.c | 1 - wolfssl/internal.h | 5 +-- 5 files changed, 92 insertions(+), 65 deletions(-) diff --git a/scripts/openssl.test b/scripts/openssl.test index c457c1281..f93f95dda 100755 --- a/scripts/openssl.test +++ b/scripts/openssl.test @@ -4,8 +4,8 @@ # need a unique port since may run the same time as testsuite generate_port() { - openssl_port=`tr -cd 0-9 sessionSecretCb = NULL; ssl->sessionSecretCtx = NULL; #endif + +#ifdef HAVE_SESSION_TICKET + ssl->session.ticket = ssl->session.staticTicket; + ssl->session.isDynamic = 0; + ssl->session.dynTicket = NULL; + ssl->session.ticketLen = 0; +#endif return 0; } @@ -2649,8 +2656,12 @@ void SSL_ResourceFree(WOLFSSL* ssl) FreeX509(&ssl->peerCert); #endif #ifdef HAVE_SESSION_TICKET - if (ssl->session.dynTicket) + if (ssl->session.dynTicket) { XFREE(ssl->session.dynTicket, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); + ssl->session.dynTicket = NULL; + ssl->session.isDynamic = 0; + ssl->session.ticket = ssl->session.staticTicket; + } #endif } @@ -11353,14 +11364,9 @@ static void PickHashSigAlgo(WOLFSSL* ssl, #ifdef HAVE_SESSION_TICKET if (ssl->options.resuming && ssl->session.ticketLen > 0) { SessionTicket* ticket; - byte* ticketData; - - ticketData = ssl->session.isDynamic ? - ssl->session.dynTicket : - ssl->session.ticket; ticket = TLSX_SessionTicket_Create(0, - ticketData, ssl->session.ticketLen); + ssl->session.ticket, ssl->session.ticketLen); if (ticket == NULL) return MEMORY_E; ret = TLSX_UseSessionTicket(&ssl->extensions, ticket); @@ -14294,15 +14300,30 @@ int DoSessionTicket(WOLFSSL* ssl, ato16(input + *inOutIdx, &length); *inOutIdx += OPAQUE16_LEN; - if (length > sizeof(ssl->session.ticket)) { - ssl->session.isDynamic = 1; - - ssl->session.dynTicket = (byte*)XMALLOC( - length, ssl->heap, - DYNAMIC_TYPE_SESSION_TICK); - if (ssl->session.dynTicket == NULL) { - return MEMORY_E; + if (length > sizeof(ssl->session.staticTicket)) { + /* Free old dynamic ticket if we already had one */ + if (ssl->session.dynTicket) { + XFREE(ssl->session.dynTicket, ssl->heap, + DYNAMIC_TYPE_SESSION_TICK); } + + ssl->session.dynTicket = + (byte*)XMALLOC(length, ssl->heap, + DYNAMIC_TYPE_SESSION_TICK); + + if (ssl->session.dynTicket == NULL) + return MEMORY_E; + + ssl->session.isDynamic = 1; + ssl->session.ticket = ssl->session.dynTicket; + } else { + if(ssl->session.dynTicket) { + XFREE(ssl->session.dynTicket, ssl->heap, + DYNAMIC_TYPE_SESSION_TICK); + ssl->session.dynTicket = NULL; + } + ssl->session.isDynamic = 0; + ssl->session.ticket = ssl->session.staticTicket; } if ((*inOutIdx - begin) + length > size) @@ -14311,11 +14332,7 @@ int DoSessionTicket(WOLFSSL* ssl, /* If the received ticket including its length is greater than * a length value, the save it. Otherwise, don't save it. */ if (length > 0) { - if (ssl->session.isDynamic) - XMEMCPY(ssl->session.dynTicket, input + *inOutIdx, length); - else - XMEMCPY(ssl->session.ticket, input + *inOutIdx, length); - + XMEMCPY(ssl->session.ticket, input + *inOutIdx, length); *inOutIdx += length; ssl->session.ticketLen = length; ssl->timeout = lifetime; @@ -14326,12 +14343,7 @@ int DoSessionTicket(WOLFSSL* ssl, } /* Create a fake sessionID based on the ticket, this will * supercede the existing session cache info. */ - ssl->options.haveSessionId = 1; - - if (ssl->session.isDynamic) - XMEMCPY(ssl->arrays->sessionID, - ssl->session.dynTicket + length - ID_LEN, ID_LEN); - else + ssl->options.haveSessionId = 1; XMEMCPY(ssl->arrays->sessionID, ssl->session.ticket + length - ID_LEN, ID_LEN); #ifndef NO_SESSION_CACHE @@ -16644,9 +16656,7 @@ int DoSessionTicket(WOLFSSL* ssl, static int CreateTicket(WOLFSSL* ssl) { InternalTicket it; - ExternalTicket* et = ssl->session.isDynamic ? - (ExternalTicket*)ssl->session.dynTicket : - (ExternalTicket*)ssl->session.ticket; + ExternalTicket* et = (ExternalTicket*)ssl->session.ticket; int encLen; int ret; byte zeros[WOLFSSL_TICKET_MAC_SZ]; /* biggest cmp size */ diff --git a/src/ssl.c b/src/ssl.c index 8f6ade691..986dae3ab 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -1251,10 +1251,7 @@ WOLFSSL_API int wolfSSL_get_SessionTicket(WOLFSSL* ssl, return BAD_FUNC_ARG; if (ssl->session.ticketLen <= *bufSz) { - if (ssl->session.isDynamic) - XMEMCPY(buf, ssl->session.dynTicket, ssl->session.ticketLen); - else - XMEMCPY(buf, ssl->session.ticket, ssl->session.ticketLen); + XMEMCPY(buf, ssl->session.ticket, ssl->session.ticketLen); *bufSz = ssl->session.ticketLen; } else @@ -1268,15 +1265,19 @@ WOLFSSL_API int wolfSSL_set_SessionTicket(WOLFSSL* ssl, byte* buf, word32 bufSz) if (ssl == NULL || (buf == NULL && bufSz > 0) || bufSz > SESSION_TICKET_LEN) return BAD_FUNC_ARG; - if (bufSz > 0) - XMEMCPY(ssl->session.ticket, buf, bufSz); + ssl->session.ticket = ssl->session.staticTicket; ssl->session.ticketLen = (word16)bufSz; - /* session ticket should only be size of static buffer. Delete dynamic buffer*/ - if (ssl->session.isDynamic) { - XFREE(ssl->session.dynTicket, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); - ssl->session.isDynamic = 0; + if (bufSz > 0) { + XMEMCPY(ssl->session.ticket, buf, bufSz); } + /* session ticket should only be size of static buffer. Delete dynamic buffer*/ + if (ssl->session.dynTicket) { + XFREE(ssl->session.dynTicket, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); + ssl->session.dynTicket = NULL; + } + ssl->session.isDynamic = 0; + return SSL_SUCCESS; } @@ -7039,6 +7040,9 @@ int AddSession(WOLFSSL* ssl) { word32 row, idx; int error = 0; +#ifdef HAVE_SESSION_TICKET + byte* tmpBuff = NULL; +#endif if (ssl->options.sessionCacheOff) return 0; @@ -7057,8 +7061,22 @@ int AddSession(WOLFSSL* ssl) return error; } - if (LockMutex(&session_mutex) != 0) +#ifdef HAVE_SESSION_TICKET + /* Alloc Memory here so if Malloc fails can exit outside of lock */ + if(ssl->session.ticketLen > SESSION_TICKET_LEN) { + tmpBuff = XMALLOC(ssl->session.ticketLen, ssl->heap, + DYNAMIC_TYPE_SESSION_TICK); + if(!tmpBuff) + return MEMORY_E; + } +#endif + + if (LockMutex(&session_mutex) != 0) { +#ifdef HAVE_SESSION_TICKET + XFREE(tmpBuff, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); +#endif return BAD_MUTEX_E; + } idx = SessionCache[row].nextIdx++; #ifdef SESSION_INDEX @@ -7075,29 +7093,28 @@ int AddSession(WOLFSSL* ssl) SessionCache[row].Sessions[idx].bornOn = LowResTimer(); #ifdef HAVE_SESSION_TICKET - if (ssl->session.isDynamic) { - if (!SessionCache[row].Sessions[idx].dynTicket) { - SessionCache[row].Sessions[idx].dynTicket = XMALLOC( - ssl->session.ticketLen, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); - if (!SessionCache[row].Sessions[idx].dynTicket) - return MEMORY_E; - } else if (SessionCache[row].Sessions[idx].ticketLen < ssl->session.ticketLen) { - XFREE(SessionCache[row].Sessions[idx].dynTicket, - ssl->heap, DYNAMIC_TYPE_SESS_TICK); - SessionCache[row].Sessions[idx].dynTicket = XMALLOC( - ssl->session.ticketLen, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); - if (!SessionCache[row].Sessions[idx].dynTicket) - return MEMORY_E; - } - XMEMCPY(SessionCache[row].Sessions[idx].dynTicket, - ssl->session.dynTicket, ssl->session.ticketLen); + /* Cleanup cache row's old Dynamic buff if exists */ + if(SessionCache[row].Sessions[idx].dynTicket) { + XFREE(SessionCache[row].Sessions[idx].dynTicket, + ssl->heap, DYNAMIC_TYPE_SESS_TICK); + } + + /* If too large to store in static buffer, use dyn buffer */ + if (ssl->session.ticketLen > SESSION_TICKET_LEN) { + SessionCache[row].Sessions[idx].dynTicket = tmpBuff; SessionCache[row].Sessions[idx].isDynamic = 1; + SessionCache[row].Sessions[idx].ticket = + SessionCache[row].Sessions[idx].dynTicket; + } else { + SessionCache[row].Sessions[idx].dynTicket = NULL; + SessionCache[row].Sessions[idx].isDynamic = 0; + SessionCache[row].Sessions[idx].ticket = + SessionCache[row].Sessions[idx].staticTicket; } - else { - XMEMCPY(SessionCache[row].Sessions[idx].ticket, - ssl->session.ticket, ssl->session.ticketLen); - } + SessionCache[row].Sessions[idx].ticketLen = ssl->session.ticketLen; + XMEMCPY(SessionCache[row].Sessions[idx].ticket, + ssl->session.ticket, ssl->session.ticketLen); #endif #ifdef SESSION_CERTS diff --git a/src/tls.c b/src/tls.c index 482c8f58d..b274e0932 100644 --- a/src/tls.c +++ b/src/tls.c @@ -3866,7 +3866,6 @@ void TLSX_FreeAll(TLSX* list) break; case TLSX_SESSION_TICKET: - /* Nothing to do. */ STK_FREE(extension->data); break; diff --git a/wolfssl/internal.h b/wolfssl/internal.h index ca473bede..60651e19d 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2181,9 +2181,10 @@ struct WOLFSSL_SESSION { #endif #ifdef HAVE_SESSION_TICKET word16 ticketLen; - byte *dynTicket; + byte* dynTicket; byte isDynamic; - byte ticket[SESSION_TICKET_LEN]; + byte staticTicket[SESSION_TICKET_LEN]; + byte* ticket; #endif #ifdef HAVE_STUNNEL void* ex_data[MAX_EX_DATA];