From 68f71d0d968e4a26d7a2af63a2c5a59fa22bb3b7 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 17 Aug 2022 18:19:28 +0200 Subject: [PATCH 1/4] Remove WOLFSSL_SESSION_TYPE_REF buffers from WOLFSSL_SESSION --- src/internal.c | 4 ++-- src/ssl.c | 46 +++++++++++++++++----------------------------- wolfssl/internal.h | 23 ++++------------------- 3 files changed, 23 insertions(+), 50 deletions(-) diff --git a/src/internal.c b/src/internal.c index 1add79966..f620fb22a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -29497,11 +29497,11 @@ int SetTicket(WOLFSSL* ssl, const byte* ticket, word32 length) /* Free old dynamic ticket if we already had one */ if (ssl->session->ticketLenAlloc > 0) { XFREE(ssl->session->ticket, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); - ssl->session->ticket = ssl->session->_staticTicket; + ssl->session->ticket = ssl->session->staticTicket; ssl->session->ticketLenAlloc = 0; } - if (length > sizeof(ssl->session->_staticTicket)) { + if (length > sizeof(ssl->session->staticTicket)) { byte* sessionTicket = (byte*)XMALLOC(length, ssl->heap, DYNAMIC_TYPE_SESSION_TICK); if (sessionTicket == NULL) diff --git a/src/ssl.c b/src/ssl.c index dc629deed..870383023 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -3418,7 +3418,7 @@ WOLFSSL_API int wolfSSL_set_SessionTicket(WOLFSSL* ssl, const byte* buf, XFREE(ssl->session->ticket, ssl->session->heap, DYNAMIC_TYPE_SESSION_TICK); ssl->session->ticketLenAlloc = 0; - ssl->session->ticket = ssl->session->_staticTicket; + ssl->session->ticket = ssl->session->staticTicket; } } else { /* Ticket requires dynamic ticket storage */ @@ -3430,7 +3430,7 @@ WOLFSSL_API int wolfSSL_set_SessionTicket(WOLFSSL* ssl, const byte* buf, ssl->session->ticket = (byte*)XMALLOC(bufSz, ssl->session->heap, DYNAMIC_TYPE_SESSION_TICK); if(ssl->session->ticket == NULL) { - ssl->session->ticket = ssl->session->_staticTicket; + ssl->session->ticket = ssl->session->staticTicket; ssl->session->ticketLenAlloc = 0; return MEMORY_ERROR; } @@ -13599,7 +13599,7 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) WOLFSSL_MSG("Session cache row lock failure"); #ifdef HAVE_SESSION_TICKET if (tmpBufSet) { - output->ticket = output->_staticTicket; + output->ticket = output->staticTicket; output->ticketLenAlloc = 0; } #ifdef WOLFSSL_SMALL_STACK @@ -13672,18 +13672,18 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) DYNAMIC_TYPE_SESSION_TICK); if (output->ticket == NULL) { error = WOLFSSL_FAILURE; - output->ticket = output->_staticTicket; + output->ticket = output->staticTicket; output->ticketLenAlloc = 0; output->ticketLen = 0; } } else { - output->ticket = output->_staticTicket; + output->ticket = output->staticTicket; output->ticketLenAlloc = 0; } } else { - output->ticket = output->_staticTicket; + output->ticket = output->staticTicket; output->ticketLenAlloc = 0; output->ticketLen = 0; } @@ -14101,7 +14101,9 @@ int AddSessionToCache(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* addSession, * 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; + /* Save pointer only if separately allocated */ + if (cacheSession->ticket != cacheSession->staticTicket) + cacheTicBuff = cacheSession->ticket; ticBuffUsed = 1; cacheSession->ticket = ticBuff; cacheSession->ticketLenAlloc = (word16) ticLen; @@ -14143,7 +14145,7 @@ int AddSessionToCache(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* addSession, #ifdef HAVE_SESSION_TICKET else if (ticBuffUsed) { /* Error occured. Need to clean up the ticket buffer. */ - cacheSession->ticket = cacheSession->_staticTicket; + cacheSession->ticket = cacheSession->staticTicket; cacheSession->ticketLenAlloc = 0; cacheSession->ticketLen = 0; } @@ -19902,19 +19904,12 @@ WOLFSSL_SESSION* wolfSSL_NewSession(void* heap) #endif ret->type = WOLFSSL_SESSION_TYPE_HEAP; ret->heap = heap; - ret->masterSecret = ret->_masterSecret; #ifdef WOLFSSL_CHECK_MEM_ZERO wc_MemZero_Add("SESSION master secret", ret->masterSecret, SECRET_LEN); wc_MemZero_Add("SESSION id", ret->sessionID, ID_LEN); #endif - #ifndef NO_CLIENT_CACHE - ret->serverID = ret->_serverID; - #endif - #ifdef OPENSSL_EXTRA - ret->sessionCtx = ret->_sessionCtx; - #endif #ifdef HAVE_SESSION_TICKET - ret->ticket = ret->_staticTicket; + ret->ticket = ret->staticTicket; #endif #ifdef HAVE_STUNNEL /* stunnel has this funny mechanism of storing the "is_authenticated" @@ -20001,7 +19996,7 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, } #ifdef HAVE_SESSION_TICKET - if (output->ticket != output->_staticTicket) { + if (output->ticket != output->staticTicket) { ticBuff = output->ticket; ticLenAlloc = output->ticketLenAlloc; } @@ -20022,8 +20017,8 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, sizeof(WOLFSSL_SESSION) - copyOffset); /* Set sane values for copy */ - if (output->type != WOLFSSL_SESSION_TYPE_CACHE) #ifndef NO_SESSION_CACHE + if (output->type != WOLFSSL_SESSION_TYPE_CACHE) output->cacheRow = INVALID_SESSION_ROW; #endif #if defined(SESSION_CERTS) && defined(OPENSSL_EXTRA) @@ -20038,13 +20033,6 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, else /* output->peer is not that important to copy */ output->peer = NULL; -#endif - output->masterSecret = output->_masterSecret; -#ifndef NO_CLIENT_CACHE - output->serverID = output->_serverID; -#endif -#ifdef OPENSSL_EXTRA - output->sessionCtx = output->_sessionCtx; #endif #ifdef HAVE_SESSION_TICKET if (input->ticketLen > SESSION_TICKET_LEN) { @@ -20090,7 +20078,7 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, * the static buffer. */ if (ticBuff != NULL) { if (ticLenAlloc >= input->ticketLen) { - output->ticket = output->_staticTicket; + output->ticket = output->staticTicket; output->ticketLenAlloc = 0; } else { @@ -20103,14 +20091,14 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, } } else { - output->ticket = output->_staticTicket; + output->ticket = output->staticTicket; output->ticketLenAlloc = 0; } } else { if (ticBuff != NULL) XFREE(ticBuff, output->heap, DYNAMIC_TYPE_SESSION_TICK); - output->ticket = output->_staticTicket; + output->ticket = output->staticTicket; output->ticketLenAlloc = 0; } if (input->ticketLenAlloc > 0 && ret == WOLFSSL_SUCCESS) { @@ -25763,7 +25751,7 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess, XFREE(s->ticket, NULL, DYNAMIC_TYPE_SESSION_TICK); } if (s->ticketLen <= SESSION_TICKET_LEN) - s->ticket = s->_staticTicket; + s->ticket = s->staticTicket; else { s->ticket = (byte*)XMALLOC(s->ticketLen, NULL, DYNAMIC_TYPE_SESSION_TICK); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 39744f996..6af978e48 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3585,7 +3585,7 @@ struct WOLFSSL_SESSION { * ID for TLS 1.3 */ byte sessionIDSz; - byte* masterSecret; /* stored secret */ + byte masterSecret[SECRET_LEN]; /* stored secret */ word16 haveEMS; /* ext master secret flag */ #if defined(SESSION_CERTS) && defined(OPENSSL_EXTRA) WOLFSSL_X509* peer; /* peer cert */ @@ -3601,11 +3601,11 @@ struct WOLFSSL_SESSION { #endif #ifndef NO_CLIENT_CACHE word16 idLen; /* serverID length */ - byte* serverID; /* for easier client lookup */ + byte serverID[SERVER_ID_LEN]; /* for easier client lookup */ #endif #ifdef OPENSSL_EXTRA byte sessionCtxSz; /* sessionCtx length */ - byte* sessionCtx; /* app specific context id */ + byte sessionCtx[ID_LEN]; /* app specific context id */ #endif /* OPENSSL_EXTRA */ #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) byte peerVerifyRet; /* cert verify error */ @@ -3624,6 +3624,7 @@ struct WOLFSSL_SESSION { #endif #endif #ifdef HAVE_SESSION_TICKET + byte staticTicket[SESSION_TICKET_LEN]; byte* ticket; word16 ticketLen; word16 ticketLenAlloc; /* is dynamic */ @@ -3638,22 +3639,6 @@ struct WOLFSSL_SESSION { #ifdef HAVE_EX_DATA WOLFSSL_CRYPTO_EX_DATA ex_data; #endif - - /* Below buffers are not allocated for the WOLFSSL_SESSION_TYPE_REF, instead - * the above pointers reference the session cache for backwards - * compatibility. For all other session types the above pointers reference - * these buffers directly. Keep these buffers at the end so that they don't - * get copied into the WOLFSSL_SESSION_TYPE_REF object. */ - byte _masterSecret[SECRET_LEN]; -#ifndef NO_CLIENT_CACHE - byte _serverID[SERVER_ID_LEN]; -#endif -#ifdef HAVE_SESSION_TICKET - byte _staticTicket[SESSION_TICKET_LEN]; -#endif -#ifdef OPENSSL_EXTRA - byte _sessionCtx[ID_LEN]; -#endif }; WOLFSSL_LOCAL int wolfSSL_RAND_Init(void); From 4d0ea628575e6e87e10e3d474060b7222f0dff58 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 18 Aug 2022 17:27:52 +0200 Subject: [PATCH 2/4] Refactor ticket size to not accidentally go over WOLFSSL_TICKET_ENC_SZ - Optimize memory usage. Write directly to ssl->session->ticket in CreateTicket() and use a hash to make sure the InternalTicket was encrypted. - DoClientTicket does not fatally error out anymore. Errors in the ticket result in the ticket being rejected instead. --- src/internal.c | 385 +++++++++++++++++++-------------------------- src/ssl.c | 35 +++-- wolfssl/internal.h | 72 +++++++-- 3 files changed, 246 insertions(+), 246 deletions(-) diff --git a/src/internal.c b/src/internal.c index f620fb22a..1335f6a62 100644 --- a/src/internal.c +++ b/src/internal.c @@ -33340,134 +33340,93 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #endif /* !WOLFSSL_NO_TLS12 */ #ifdef HAVE_SESSION_TICKET - -#define WOLFSSL_TICKET_FIXED_SZ (WOLFSSL_TICKET_NAME_SZ + \ - WOLFSSL_TICKET_IV_SZ + WOLFSSL_TICKET_MAC_SZ + OPAQUE32_LEN) - -#if defined(WOLFSSL_GENERAL_ALIGNMENT) && WOLFSSL_GENERAL_ALIGNMENT > 0 - /* round up to WOLFSSL_GENERAL_ALIGNMENT */ - #define WOLFSSL_TICKET_ENC_SZ \ - (((SESSION_TICKET_LEN - WOLFSSL_TICKET_FIXED_SZ) + \ - WOLFSSL_GENERAL_ALIGNMENT - 1) & ~(WOLFSSL_GENERAL_ALIGNMENT-1)) -#else - #define WOLFSSL_TICKET_ENC_SZ (SESSION_TICKET_LEN - WOLFSSL_TICKET_FIXED_SZ) -#endif - - /* Our ticket format. All members need to be a byte or array of byte to - * avoid alignment issues */ - typedef struct InternalTicket { - ProtocolVersion pv; /* version when ticket created */ - byte suite[SUITE_LEN]; /* cipher suite when created */ - byte msecret[SECRET_LEN]; /* master secret */ - byte timestamp[TIMESTAMP_LEN]; /* born on */ - byte haveEMS; /* have extended master secret */ -#ifdef WOLFSSL_TLS13 - byte ageAdd[AGEADD_LEN]; /* Obfuscation of age */ - byte namedGroup[NAMEDGROUP_LEN]; /* Named group used */ - TicketNonce ticketNonce; /* Ticket nonce */ - #ifdef WOLFSSL_EARLY_DATA - byte maxEarlyDataSz[MAXEARLYDATASZ_LEN]; /* Max size of - * early data */ - #endif -#endif -#ifdef WOLFSSL_TICKET_HAVE_ID - byte id[ID_LEN]; -#endif - } InternalTicket; - - static WC_INLINE int compare_InternalTickets( - InternalTicket *a, - InternalTicket *b) + /* Make a work from the front of random hash */ + static WC_INLINE word32 MakeWordFromHash(const byte* hashID) { - if ((a->pv.major == b->pv.major) && - (a->pv.minor == b->pv.minor) && - (XMEMCMP(a->suite,b->suite,sizeof a->suite) == 0) && - (XMEMCMP(a->msecret,b->msecret,sizeof a->msecret) == 0) && - (XMEMCMP(a->timestamp, b->timestamp, sizeof a->timestamp) == 0) && - (a->haveEMS == b->haveEMS) -#ifdef WOLFSSL_TLS13 - && - (XMEMCMP(a->ageAdd, b->ageAdd, sizeof a->ageAdd) == 0) && - (XMEMCMP(a->namedGroup, b->namedGroup, sizeof a->namedGroup) - == 0) && - (a->ticketNonce.len == b->ticketNonce.len) && - (XMEMCMP( - a->ticketNonce.data, - b->ticketNonce.data, - a->ticketNonce.len) == 0) -#ifdef WOLFSSL_EARLY_DATA - && (XMEMCMP( - a->maxEarlyDataSz, - b->maxEarlyDataSz, - sizeof a->maxEarlyDataSz) == 0) -#endif -#endif - ) - return 0; - else - return -1; + return ((word32)hashID[0] << 24) | ((word32)hashID[1] << 16) | + ((word32)hashID[2] << 8) | (word32)hashID[3]; } - /* RFC 5077 defines this for session tickets */ - /* fit within SESSION_TICKET_LEN */ - typedef struct ExternalTicket { - byte key_name[WOLFSSL_TICKET_NAME_SZ]; /* key context name - 16 */ - byte iv[WOLFSSL_TICKET_IV_SZ]; /* this ticket's iv - 16 */ - byte enc_len[OPAQUE32_LEN]; /* encrypted length - 4 */ - byte enc_ticket[WOLFSSL_TICKET_ENC_SZ]; /* encrypted internal ticket */ - byte mac[WOLFSSL_TICKET_MAC_SZ]; /* total mac - 32 */ - /* !! if add to structure, add to TICKET_FIXED_SZ !! */ - } ExternalTicket; + /* Check to make sure that the callback has actually encrypted the ticket */ + static word32 compute_InternalTicket_hash(InternalTicket *a) + { + byte digest[WC_MAX_DIGEST_SIZE]; + int error; + + #ifndef NO_MD5 + error = wc_Md5Hash((byte*)a, sizeof(*a), digest); + #elif !defined(NO_SHA) + error = wc_ShaHash((byte*)a, sizeof(*a), digest); + #elif !defined(NO_SHA256) + error = wc_Sha256Hash((byte*)a, sizeof(*a), digest); + #else + #error "We need a digest to hash the InternalTicket" + #endif + + return error == 0 ? MakeWordFromHash(digest) : 0; /* 0 on failure */ + } /* create a new session ticket, 0 on success */ int CreateTicket(WOLFSSL* ssl) { - InternalTicket it; - ExternalTicket* et = (ExternalTicket*)ssl->session->ticket; + InternalTicket* it; + ExternalTicket* et; int encLen; int ret; + word32 itHash = 0; byte zeros[WOLFSSL_TICKET_MAC_SZ]; /* biggest cmp size */ - XMEMSET(&it, 0, sizeof(it)); + WOLFSSL_ASSERT_SIZEOF_GE(ssl->session->staticTicket, *et); + WOLFSSL_ASSERT_SIZEOF_GE(et->enc_ticket, *it); + + if (ssl->session->ticket != ssl->session->staticTicket) { + /* Always use the static ticket buffer */ + XFREE(ssl->session->ticket, NULL, DYNAMIC_TYPE_SESSION_TICK); + ssl->session->ticket = ssl->session->staticTicket; + ssl->session->ticketLenAlloc = 0; + } + + et = (ExternalTicket*)ssl->session->ticket; + it = (InternalTicket*)et->enc_ticket; + + XMEMSET(et, 0, sizeof(*et)); /* build internal */ - it.pv.major = ssl->version.major; - it.pv.minor = ssl->version.minor; + it->pv.major = ssl->version.major; + it->pv.minor = ssl->version.minor; - it.suite[0] = ssl->options.cipherSuite0; - it.suite[1] = ssl->options.cipherSuite; + it->suite[0] = ssl->options.cipherSuite0; + it->suite[1] = ssl->options.cipherSuite; #ifdef WOLFSSL_EARLY_DATA - c32toa(ssl->options.maxEarlyDataSz, it.maxEarlyDataSz); + c32toa(ssl->options.maxEarlyDataSz, it->maxEarlyDataSz); #endif if (!ssl->options.tls1_3) { - XMEMCPY(it.msecret, ssl->arrays->masterSecret, SECRET_LEN); + XMEMCPY(it->msecret, ssl->arrays->masterSecret, SECRET_LEN); #ifndef NO_ASN_TIME - c32toa(LowResTimer(), (byte*)&it.timestamp); + c32toa(LowResTimer(), it->timestamp); #endif - it.haveEMS = (byte) ssl->options.haveEMS; + it->haveEMS = (byte) ssl->options.haveEMS; } else { #ifdef WOLFSSL_TLS13 /* Client adds to ticket age to obfuscate. */ - ret = wc_RNG_GenerateBlock(ssl->rng, (byte*)&it.ageAdd, - sizeof(it.ageAdd)); - if (ret != 0) - return BAD_TICKET_ENCRYPT; - ato32(it.ageAdd, &ssl->session->ticketAdd); - c16toa(ssl->session->namedGroup, it.namedGroup); - c32toa(TimeNowInMilliseconds(), it.timestamp); + ret = wc_RNG_GenerateBlock(ssl->rng, it->ageAdd, + sizeof(it->ageAdd)); + if (ret != 0) { + ret = BAD_TICKET_ENCRYPT; + goto error; + } + ato32(it->ageAdd, &ssl->session->ticketAdd); + c16toa(ssl->session->namedGroup, it->namedGroup); + c32toa(TimeNowInMilliseconds(), it->timestamp); /* Resumption master secret. */ - XMEMCPY(it.msecret, ssl->session->masterSecret, SECRET_LEN); - XMEMCPY(&it.ticketNonce, &ssl->session->ticketNonce, + XMEMCPY(it->msecret, ssl->session->masterSecret, SECRET_LEN); + XMEMCPY(&it->ticketNonce, &ssl->session->ticketNonce, sizeof(TicketNonce)); #endif } - #ifdef WOLFSSL_CHECK_MEM_ZERO - /* Ticket has sensitive data in it now. */ - wc_MemZero_Add("Create Ticket internal", &it, sizeof(InternalTicket)); - #endif #ifdef WOLFSSL_TICKET_HAVE_ID { @@ -33488,13 +33447,8 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, if (idSz == 0) { ret = wc_RNG_GenerateBlock(ssl->rng, ssl->session->altSessionID, ID_LEN); - if (ret != 0) { - ForceZero(&it, sizeof(it)); - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(&it, sizeof(InternalTicket)); - #endif - return ret; - } + if (ret != 0) + goto error; ssl->session->haveAltSessionID = 1; id = ssl->session->altSessionID; idSz = ID_LEN; @@ -33502,7 +33456,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* make sure idSz is not larger than ID_LEN */ if (idSz > ID_LEN) idSz = ID_LEN; - XMEMCPY(it.id, id, idSz); + XMEMCPY(it->id, id, idSz); } #endif @@ -33518,107 +33472,85 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, (ssl->options.mask & WOLFSSL_OP_NO_TICKET) != 0) #endif ) { - ForceZero(&it, sizeof(it)); - ret = WOLFSSL_TICKET_RET_FATAL; - WOLFSSL_ERROR_VERBOSE(ret); + /* Use BAD_TICKET_ENCRYPT to signal missing ticket callback */ + ret = BAD_TICKET_ENCRYPT; } else { - /* build external */ - XMEMCPY(et->enc_ticket, &it, sizeof(InternalTicket)); - + itHash = compute_InternalTicket_hash(it); ret = ssl->ctx->ticketEncCb(ssl, et->key_name, et->iv, et->mac, 1, et->enc_ticket, sizeof(InternalTicket), &encLen, ssl->ctx->ticketEncCtx); - if (ret != WOLFSSL_TICKET_RET_OK) { - #ifdef WOLFSSL_ASYNC_CRYPT - if (ret == WC_PENDING_E) { - return ret; - } - #endif - #ifdef WOLFSSL_CHECK_MEM_ZERO - /* Internal ticket data wasn't encrypted maybe. */ - wc_MemZero_Add("Create Ticket enc_ticket", et->enc_ticket, - sizeof(InternalTicket)); - #endif - ForceZero(&it, sizeof(it)); - ForceZero(et->enc_ticket, sizeof(it)); - } } - if (ret == WOLFSSL_TICKET_RET_OK) { - if (encLen < (int)sizeof(InternalTicket) || - encLen > WOLFSSL_TICKET_ENC_SZ) { - ForceZero(&it, sizeof(it)); - ForceZero(et->enc_ticket, sizeof(it)); - WOLFSSL_MSG("Bad user ticket encrypt size"); - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(&it, sizeof(InternalTicket)); - #endif - WOLFSSL_ERROR_VERBOSE(BAD_TICKET_KEY_CB_SZ); - return BAD_TICKET_KEY_CB_SZ; - } - - /* sanity checks on encrypt callback */ - - /* internal ticket can't be the same if encrypted */ - if (compare_InternalTickets((InternalTicket *)et->enc_ticket, &it) - == 0) - { - ForceZero(&it, sizeof(it)); - ForceZero(et->enc_ticket, sizeof(it)); - WOLFSSL_MSG("User ticket encrypt didn't encrypt"); - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(&it, sizeof(InternalTicket)); - #endif - WOLFSSL_ERROR_VERBOSE(BAD_TICKET_ENCRYPT); - return BAD_TICKET_ENCRYPT; - } - - ForceZero(&it, sizeof(it)); - XMEMSET(zeros, 0, sizeof(zeros)); - - /* name */ - if (XMEMCMP(et->key_name, zeros, WOLFSSL_TICKET_NAME_SZ) == 0) { - WOLFSSL_MSG("User ticket encrypt didn't set name"); - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(&it, sizeof(InternalTicket)); - #endif - WOLFSSL_ERROR_VERBOSE(BAD_TICKET_ENCRYPT); - return BAD_TICKET_ENCRYPT; - } - - /* iv */ - if (XMEMCMP(et->iv, zeros, WOLFSSL_TICKET_IV_SZ) == 0) { - WOLFSSL_MSG("User ticket encrypt didn't set iv"); - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(&it, sizeof(InternalTicket)); - #endif - WOLFSSL_ERROR_VERBOSE(BAD_TICKET_ENCRYPT); - return BAD_TICKET_ENCRYPT; - } - - /* mac */ - if (XMEMCMP(et->mac, zeros, WOLFSSL_TICKET_MAC_SZ) == 0) { - WOLFSSL_MSG("User ticket encrypt didn't set mac"); - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(&it, sizeof(InternalTicket)); - #endif - WOLFSSL_ERROR_VERBOSE(BAD_TICKET_ENCRYPT); - return BAD_TICKET_ENCRYPT; - } - - /* set size */ - c32toa((word32)encLen, et->enc_len); - ssl->session->ticketLen = (word16)(encLen + WOLFSSL_TICKET_FIXED_SZ); - if (encLen < WOLFSSL_TICKET_ENC_SZ) { - /* move mac up since whole enc buffer not used */ - XMEMMOVE(et->enc_ticket +encLen, et->mac,WOLFSSL_TICKET_MAC_SZ); + if (ret != WOLFSSL_TICKET_RET_OK) { +#ifdef WOLFSSL_ASYNC_CRYPT + if (ret == WC_PENDING_E) { + return ret; } +#endif + goto error; + } + if (encLen < (int)sizeof(InternalTicket) || + encLen > (int)WOLFSSL_TICKET_ENC_SZ) { + WOLFSSL_MSG("Bad user ticket encrypt size"); + ret = BAD_TICKET_KEY_CB_SZ; } - #ifdef WOLFSSL_CHECK_MEM_ZERO - wc_MemZero_Check(&it, sizeof(InternalTicket)); - #endif + /* sanity checks on encrypt callback */ + + /* internal ticket can't be the same if encrypted */ + if (itHash == compute_InternalTicket_hash(it)) + { + WOLFSSL_MSG("User ticket encrypt didn't encrypt"); + ret = BAD_TICKET_ENCRYPT; + goto error; + } + + XMEMSET(zeros, 0, sizeof(zeros)); + + /* name */ + if (XMEMCMP(et->key_name, zeros, WOLFSSL_TICKET_NAME_SZ) == 0) { + WOLFSSL_MSG("User ticket encrypt didn't set name"); + ret = BAD_TICKET_ENCRYPT; + goto error; + } + + /* iv */ + if (XMEMCMP(et->iv, zeros, WOLFSSL_TICKET_IV_SZ) == 0) { + WOLFSSL_MSG("User ticket encrypt didn't set iv"); + ret = BAD_TICKET_ENCRYPT; + goto error; + } + + /* mac */ + if (XMEMCMP(et->mac, zeros, WOLFSSL_TICKET_MAC_SZ) == 0) { + WOLFSSL_MSG("User ticket encrypt didn't set mac"); + ret = BAD_TICKET_ENCRYPT; + goto error; + } + + /* set size */ + c16toa((word16)encLen, et->enc_len); + if (encLen < (int)WOLFSSL_TICKET_ENC_SZ) { + /* move mac up since whole enc buffer not used */ + XMEMMOVE(et->enc_ticket + encLen, et->mac, + WOLFSSL_TICKET_MAC_SZ); + } + ssl->session->ticketLen = + (word16)(encLen + WOLFSSL_TICKET_FIXED_SZ); + return ret; + error: +#ifdef WOLFSSL_CHECK_MEM_ZERO + /* Ticket has sensitive data in it now. */ + wc_MemZero_Add("Create Ticket internal", it, sizeof(InternalTicket)); +#endif + ForceZero(it, sizeof(*it)); +#ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Check(it, sizeof(InternalTicket)); +#endif + WOLFSSL_ERROR_VERBOSE(ret); + return ret; + } @@ -33629,24 +33561,24 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, InternalTicket* it; int ret; int outLen; - word32 inLen; + word16 inLen; WOLFSSL_START(WC_FUNC_TICKET_DO); WOLFSSL_ENTER("DoClientTicket"); if (len > SESSION_TICKET_LEN || - len < (word32)(sizeof(InternalTicket) + WOLFSSL_TICKET_FIXED_SZ)) { + len < (word32)(sizeof(InternalTicket) + WOLFSSL_TICKET_FIXED_SZ)) { WOLFSSL_ERROR_VERBOSE(BAD_TICKET_MSG_SZ); - return BAD_TICKET_MSG_SZ; + return WOLFSSL_TICKET_RET_REJECT; } et = (ExternalTicket*)input; /* decrypt */ - ato32(et->enc_len, &inLen); - if (inLen > (word16)(len - WOLFSSL_TICKET_FIXED_SZ)) { + ato16(et->enc_len, &inLen); + if (inLen > WOLFSSL_TICKET_ENC_SZ) { WOLFSSL_ERROR_VERBOSE(BAD_TICKET_MSG_SZ); - return BAD_TICKET_MSG_SZ; + return WOLFSSL_TICKET_RET_REJECT; } outLen = (int)inLen; /* may be reduced by user padding */ @@ -33660,8 +33592,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, (ssl->options.mask & WOLFSSL_OP_NO_TICKET) != 0) #endif ) { - ret = WOLFSSL_TICKET_RET_FATAL; - WOLFSSL_ERROR_VERBOSE(ret); + /* Use BAD_TICKET_ENCRYPT to signal missing ticket callback */ + WOLFSSL_ERROR_VERBOSE(BAD_TICKET_ENCRYPT); + ret = WOLFSSL_TICKET_RET_REJECT; } else { ret = ssl->ctx->ticketEncCb(ssl, et->key_name, et->iv, @@ -33669,10 +33602,10 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, et->enc_ticket, inLen, &outLen, ssl->ctx->ticketEncCtx); } - if (ret == WOLFSSL_TICKET_RET_FATAL) - ret = WOLFSSL_TICKET_RET_REJECT; - if (ret < 0) - return ret; + if (ret != WOLFSSL_TICKET_RET_OK && ret != WOLFSSL_TICKET_RET_CREATE) { + WOLFSSL_ERROR_VERBOSE(BAD_TICKET_KEY_CB_SZ); + return WOLFSSL_TICKET_RET_REJECT; + } if (outLen > (int)inLen || outLen < (int)sizeof(InternalTicket)) { WOLFSSL_MSG("Bad user ticket decrypt len"); WOLFSSL_ERROR_VERBOSE(BAD_TICKET_KEY_CB_SZ); @@ -33688,33 +33621,30 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* get master secret */ if (ret == WOLFSSL_TICKET_RET_OK || ret == WOLFSSL_TICKET_RET_CREATE) { if (ssl->version.minor < it->pv.minor) { - ForceZero(it, sizeof(*it)); WOLFSSL_MSG("Ticket has greater version"); - WOLFSSL_ERROR_VERBOSE(VERSION_ERROR); - return VERSION_ERROR; + ret = VERSION_ERROR; + goto error; } else if (ssl->version.minor > it->pv.minor) { if (IsAtLeastTLSv1_3(it->pv) != IsAtLeastTLSv1_3(ssl->version)) { - ForceZero(it, sizeof(*it)); WOLFSSL_MSG("Tickets cannot be shared between " "TLS 1.3 and TLS 1.2 and lower"); - WOLFSSL_ERROR_VERBOSE(VERSION_ERROR); - return VERSION_ERROR; + ret = VERSION_ERROR; + goto error; } if (!ssl->options.downgrade) { - ForceZero(it, sizeof(*it)); WOLFSSL_MSG("Ticket has lesser version"); - WOLFSSL_ERROR_VERBOSE(VERSION_ERROR); - return VERSION_ERROR; + ret = VERSION_ERROR; + goto error; } WOLFSSL_MSG("Downgrading protocol due to ticket"); if (it->pv.minor < ssl->options.minDowngrade) { - ForceZero(it, sizeof(*it)); - WOLFSSL_ERROR_VERBOSE(VERSION_ERROR); - return VERSION_ERROR; + WOLFSSL_MSG("Ticket has lesser version than allowed"); + ret = VERSION_ERROR; + goto error; } ssl->version.minor = it->pv.minor; } @@ -33765,11 +33695,22 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } ForceZero(it, sizeof(*it)); +#ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Check(it, sizeof(InternalTicket)); +#endif WOLFSSL_LEAVE("DoClientTicket", ret); WOLFSSL_END(WC_FUNC_TICKET_DO); return ret; + +error: + ForceZero(it, sizeof(*it)); +#ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Check(it, sizeof(InternalTicket)); +#endif + WOLFSSL_ERROR_VERBOSE(ret); + return WOLFSSL_TICKET_RET_REJECT; } diff --git a/src/ssl.c b/src/ssl.c index 870383023..8a3c5111a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -33350,6 +33350,7 @@ static int wolfSSL_TicketKeyCb(WOLFSSL* ssl, int len = 0; int ret = WOLFSSL_TICKET_RET_FATAL; int res; + int totalSz = 0; (void)ctx; @@ -33386,27 +33387,30 @@ static int wolfSSL_TicketKeyCb(WOLFSSL* ssl, goto end; } + if (wolfSSL_HMAC_size(&hmacCtx) > WOLFSSL_TICKET_MAC_SZ) { + WOLFSSL_MSG("Ticket cipher MAC size error"); + goto end; + } + if (enc) { /* Encrypt in place. */ if (!wolfSSL_EVP_CipherUpdate(evpCtx, encTicket, &len, encTicket, encTicketLen)) goto end; - encTicketLen = len; - if (!wolfSSL_EVP_EncryptFinal(evpCtx, &encTicket[encTicketLen], &len)) + totalSz = len; + if (totalSz > *encLen) + goto end; + if (!wolfSSL_EVP_EncryptFinal(evpCtx, &encTicket[len], &len)) goto end; /* Total length of encrypted data. */ - encTicketLen += len; - *encLen = encTicketLen; + totalSz += len; + if (totalSz > *encLen) + goto end; /* HMAC the encrypted data into the parameter 'mac'. */ - if (!wolfSSL_HMAC_Update(&hmacCtx, encTicket, encTicketLen)) + if (!wolfSSL_HMAC_Update(&hmacCtx, encTicket, totalSz)) goto end; -#ifdef WOLFSSL_SHA512 - /* Check for SHA512, which would overrun the mac buffer */ - if (hmacCtx.hmac.macType == WC_SHA512) - goto end; -#endif if (!wolfSSL_HMAC_Final(&hmacCtx, mac, &mdSz)) goto end; } @@ -33424,12 +33428,17 @@ static int wolfSSL_TicketKeyCb(WOLFSSL* ssl, if (!wolfSSL_EVP_CipherUpdate(evpCtx, encTicket, &len, encTicket, encTicketLen)) goto end; - encTicketLen = len; - if (!wolfSSL_EVP_DecryptFinal(evpCtx, &encTicket[encTicketLen], &len)) + totalSz = len; + if (totalSz > encTicketLen) + goto end; + if (!wolfSSL_EVP_DecryptFinal(evpCtx, &encTicket[len], &len)) goto end; /* Total length of decrypted data. */ - *encLen = encTicketLen + len; + totalSz += len; + if (totalSz > encTicketLen) + goto end; } + *encLen = totalSz; if (res == TICKET_KEY_CB_RET_RENEW && !IsAtLeastTLSv1_3(ssl->version) && !enc) diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 6af978e48..b9bd5521c 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1676,10 +1676,6 @@ enum Misc { #define MAX_HANDSHAKE_SZ MAX_CERTIFICATE_SZ #endif -#ifndef SESSION_TICKET_LEN - #define SESSION_TICKET_LEN 256 -#endif - #ifndef PREALLOC_SESSION_TICKET_LEN #define PREALLOC_SESSION_TICKET_LEN 512 #endif @@ -2694,7 +2690,68 @@ WOLFSSL_LOCAL int TLSX_AddEmptyRenegotiationInfo(TLSX** extensions, void* heap); #endif /* HAVE_SECURE_RENEGOTIATION */ /** Session Ticket - RFC 5077 (session 3.2) */ +#if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) +/* Ticket nonce - for deriving PSK. + * Length allowed to be: 1..255. Only support 4 bytes. + * Defined here so that it can be included in InternalTicket. + */ +typedef struct TicketNonce { + byte len; + byte data[MAX_TICKET_NONCE_SZ]; +} TicketNonce; +#endif + #ifdef HAVE_SESSION_TICKET +/* Our ticket format. All members need to be a byte or array of byte to + * avoid alignment issues */ +typedef struct InternalTicket { + ProtocolVersion pv; /* version when ticket created */ + byte suite[SUITE_LEN]; /* cipher suite when created */ + byte msecret[SECRET_LEN]; /* master secret */ + byte timestamp[TIMESTAMP_LEN]; /* born on */ + byte haveEMS; /* have extended master secret */ +#ifdef WOLFSSL_TLS13 + byte ageAdd[AGEADD_LEN]; /* Obfuscation of age */ + byte namedGroup[NAMEDGROUP_LEN]; /* Named group used */ + TicketNonce ticketNonce; /* Ticket nonce */ +#ifdef WOLFSSL_EARLY_DATA + byte maxEarlyDataSz[MAXEARLYDATASZ_LEN]; /* Max size of + * early data */ +#endif +#endif +#ifdef WOLFSSL_TICKET_HAVE_ID + byte id[ID_LEN]; +#endif +} InternalTicket; + +#ifndef WOLFSSL_TICKET_EXTRA_PADDING_SZ +#define WOLFSSL_TICKET_EXTRA_PADDING_SZ 32 +#endif + +#if defined(WOLFSSL_GENERAL_ALIGNMENT) && WOLFSSL_GENERAL_ALIGNMENT > 0 + /* round up to WOLFSSL_GENERAL_ALIGNMENT */ + #define WOLFSSL_TICKET_ENC_SZ \ + (((sizeof(InternalTicket) + WOLFSSL_TICKET_EXTRA_PADDING_SZ) + \ + WOLFSSL_GENERAL_ALIGNMENT - 1) & ~(WOLFSSL_GENERAL_ALIGNMENT-1)) +#else + #define WOLFSSL_TICKET_ENC_SZ \ + (sizeof(InternalTicket) + WOLFSSL_TICKET_EXTRA_PADDING_SZ) +#endif + +/* RFC 5077 defines this for session tickets. All members need to be a byte or + * array of byte to avoid alignment issues */ +typedef struct ExternalTicket { + byte key_name[WOLFSSL_TICKET_NAME_SZ]; /* key context name - 16 */ + byte iv[WOLFSSL_TICKET_IV_SZ]; /* this ticket's iv - 16 */ + byte enc_len[OPAQUE16_LEN]; /* encrypted length - 2 */ + byte enc_ticket[WOLFSSL_TICKET_ENC_SZ]; + /* encrypted internal ticket */ + byte mac[WOLFSSL_TICKET_MAC_SZ]; /* total mac - 32 */ +} ExternalTicket; + +/* Cast to int to reduce amount of casts in code */ +#define SESSION_TICKET_LEN ((int)sizeof(ExternalTicket)) +#define WOLFSSL_TICKET_FIXED_SZ (SESSION_TICKET_LEN - WOLFSSL_TICKET_ENC_SZ) typedef struct SessionTicket { word32 lifetime; @@ -2779,13 +2836,6 @@ WOLFSSL_LOCAL int TLSX_KeyShare_DeriveSecret(WOLFSSL* ssl); #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) -/* Ticket nonce - for deriving PSK. - * Length allowed to be: 1..255. Only support 4 bytes. - */ -typedef struct TicketNonce { - byte len; - byte data[MAX_TICKET_NONCE_SZ]; -} TicketNonce; /* The PreSharedKey extension information - entry in a linked list. */ typedef struct PreSharedKey { From 06022e85a3535ec8f98f5f7efa5f611b1c56c60b Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 18 Aug 2022 19:43:40 +0200 Subject: [PATCH 3/4] Fix avoidSysCalls logic --- src/ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index 8a3c5111a..b9bbb336e 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -20040,7 +20040,7 @@ int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, if (ticBuff == NULL || ticLenAlloc < input->ticketLen) { /* allocate new one */ byte* tmp; - if (!avoidSysCalls) { + if (avoidSysCalls) { WOLFSSL_MSG("Failed to allocate memory for ticket when avoiding" " syscalls"); output->ticket = ticBuff; From e565d0d7deff8f6000ca009693fae4c03206694a Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 19 Aug 2022 11:31:00 +0200 Subject: [PATCH 4/4] Refactor and code review - Refactor object hashing into one function - Allow multiple WOLFSSL_ASSERT_SIZEOF_GE in one function --- src/internal.c | 43 ++++++++++------------------------ src/ssl.c | 50 ++++++++-------------------------------- wolfcrypt/src/misc.c | 34 +++++++++++++++++++++++++++ wolfssl/internal.h | 7 +++--- wolfssl/wolfcrypt/misc.h | 2 ++ 5 files changed, 61 insertions(+), 75 deletions(-) diff --git a/src/internal.c b/src/internal.c index 1335f6a62..af0131a05 100644 --- a/src/internal.c +++ b/src/internal.c @@ -33340,31 +33340,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #endif /* !WOLFSSL_NO_TLS12 */ #ifdef HAVE_SESSION_TICKET - /* Make a work from the front of random hash */ - static WC_INLINE word32 MakeWordFromHash(const byte* hashID) - { - return ((word32)hashID[0] << 24) | ((word32)hashID[1] << 16) | - ((word32)hashID[2] << 8) | (word32)hashID[3]; - } - - /* Check to make sure that the callback has actually encrypted the ticket */ - static word32 compute_InternalTicket_hash(InternalTicket *a) - { - byte digest[WC_MAX_DIGEST_SIZE]; - int error; - - #ifndef NO_MD5 - error = wc_Md5Hash((byte*)a, sizeof(*a), digest); - #elif !defined(NO_SHA) - error = wc_ShaHash((byte*)a, sizeof(*a), digest); - #elif !defined(NO_SHA256) - error = wc_Sha256Hash((byte*)a, sizeof(*a), digest); - #else - #error "We need a digest to hash the InternalTicket" - #endif - - return error == 0 ? MakeWordFromHash(digest) : 0; /* 0 on failure */ - } /* create a new session ticket, 0 on success */ int CreateTicket(WOLFSSL* ssl) @@ -33373,6 +33348,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ExternalTicket* et; int encLen; int ret; + int error; word32 itHash = 0; byte zeros[WOLFSSL_TICKET_MAC_SZ]; /* biggest cmp size */ @@ -33476,10 +33452,15 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ret = BAD_TICKET_ENCRYPT; } else { - itHash = compute_InternalTicket_hash(it); - ret = ssl->ctx->ticketEncCb(ssl, et->key_name, et->iv, et->mac, 1, - et->enc_ticket, sizeof(InternalTicket), - &encLen, ssl->ctx->ticketEncCtx); + itHash = HashObject((byte*)it, sizeof(*it), &error); + if (error == 0) { + ret = ssl->ctx->ticketEncCb(ssl, et->key_name, et->iv, et->mac, + 1, et->enc_ticket, sizeof(InternalTicket), &encLen, + ssl->ctx->ticketEncCtx); + } + else { + ret = WOLFSSL_TICKET_RET_FATAL; + } } if (ret != WOLFSSL_TICKET_RET_OK) { #ifdef WOLFSSL_ASYNC_CRYPT @@ -33498,9 +33479,9 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* sanity checks on encrypt callback */ /* internal ticket can't be the same if encrypted */ - if (itHash == compute_InternalTicket_hash(it)) + if (itHash == HashObject((byte*)it, sizeof(*it), &error) || error != 0) { - WOLFSSL_MSG("User ticket encrypt didn't encrypt"); + WOLFSSL_MSG("User ticket encrypt didn't encrypt or hash failed"); ret = BAD_TICKET_ENCRYPT; goto error; } diff --git a/src/ssl.c b/src/ssl.c index b9bbb336e..a2ffb0161 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -4731,19 +4731,6 @@ int wolfSSL_SetVersion(WOLFSSL* ssl, int version) } #endif /* !leanpsk */ - -#if !defined(NO_CERTS) || !defined(NO_SESSION_CACHE) - -/* Make a work from the front of random hash */ -static WC_INLINE word32 MakeWordFromHash(const byte* hashID) -{ - return ((word32)hashID[0] << 24) | ((word32)hashID[1] << 16) | - ((word32)hashID[2] << 8) | (word32)hashID[3]; -} - -#endif /* !NO_CERTS || !NO_SESSION_CACHE */ - - #ifndef NO_CERTS /* hash is the SHA digest of name, just use first 32 bits as hash */ @@ -13249,25 +13236,6 @@ int wolfSSL_Cleanup(void) #ifndef NO_SESSION_CACHE -/* some session IDs aren't random after all, let's make them random */ -static WC_INLINE word32 HashSession(const byte* sessionID, word32 len, int* error) -{ - byte digest[WC_MAX_DIGEST_SIZE]; - -#ifndef NO_MD5 - *error = wc_Md5Hash(sessionID, len, digest); -#elif !defined(NO_SHA) - *error = wc_ShaHash(sessionID, len, digest); -#elif !defined(NO_SHA256) - *error = wc_Sha256Hash(sessionID, len, digest); -#else - #error "We need a digest to hash the session IDs" -#endif - - return *error == 0 ? MakeWordFromHash(digest) : 0; /* 0 on failure */ -} - - WOLFSSL_ABI void wolfSSL_flush_sessions(WOLFSSL_CTX* ctx, long tm) { @@ -13396,7 +13364,7 @@ WOLFSSL_SESSION* wolfSSL_GetSessionClient(WOLFSSL* ssl, const byte* id, int len) } #endif - row = HashSession(id, len, &error) % CLIENT_SESSION_ROWS; + row = HashObject(id, len, &error) % CLIENT_SESSION_ROWS; if (error != 0) { WOLFSSL_MSG("Hash session failed"); return NULL; @@ -13559,7 +13527,7 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) } #endif - row = HashSession(id, ID_LEN, &error) % SESSION_ROWS; + row = HashObject(id, ID_LEN, &error) % SESSION_ROWS; if (error != 0) { WOLFSSL_MSG("Hash session failed"); return WOLFSSL_FAILURE; @@ -13851,11 +13819,11 @@ ClientSession* AddSessionToClientCache(int side, int row, int idx, byte* serverI WOLFSSL_MSG("Trying to add client cache entry"); if (idLen) { - clientRow = HashSession(serverID, + clientRow = HashObject(serverID, idLen, &error) % CLIENT_SESSION_ROWS; } else if (serverID != NULL) { - clientRow = HashSession(sessionID, + clientRow = HashObject(sessionID, ID_LEN, &error) % CLIENT_SESSION_ROWS; } else { @@ -13869,7 +13837,7 @@ ClientSession* AddSessionToClientCache(int side, int row, int idx, byte* serverI ClientCache[clientRow].Clients[clientIdx].serverIdx = (word16)idx; if (sessionID != NULL) { - sessionIDHash = HashSession(sessionID, ID_LEN, &error); + sessionIDHash = HashObject(sessionID, ID_LEN, &error); if (error == 0) { ClientCache[clientRow].Clients[clientIdx].sessionIDHash = sessionIDHash; @@ -13963,7 +13931,7 @@ WOLFSSL_SESSION* ClientSessionToSession(const WOLFSSL_SESSION* session) } if (error == 0) { /* Calculate the hash of the session ID */ - sessionIDHash = HashSession(cacheSession->sessionID, ID_LEN, + sessionIDHash = HashObject(cacheSession->sessionID, ID_LEN, &error); } if (error == 0) { @@ -14041,7 +14009,7 @@ int AddSessionToCache(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* addSession, } #endif /* Use the session object in the cache for external cache if required */ - row = (int)(HashSession(id, ID_LEN, &ret) % SESSION_ROWS); + row = (int)(HashObject(id, ID_LEN, &ret) % SESSION_ROWS); if (ret != 0) { WOLFSSL_MSG("Hash session failed"); #ifdef HAVE_SESSION_TICKET @@ -31226,7 +31194,7 @@ static void SESSION_ex_data_cache_update(WOLFSSL_SESSION* session, int idx, if (session->haveAltSessionID) id = session->altSessionID; - row = (int)(HashSession(id, ID_LEN, &error) % SESSION_ROWS); + row = (int)(HashObject(id, ID_LEN, &error) % SESSION_ROWS); if (error != 0) { WOLFSSL_MSG("Hash session failed"); return; @@ -33038,7 +33006,7 @@ int wolfSSL_SSL_CTX_remove_session(WOLFSSL_CTX *ctx, WOLFSSL_SESSION *s) if (s->haveAltSessionID) id = s->altSessionID; - row = (int)(HashSession(id, ID_LEN, &ret) % SESSION_ROWS); + row = (int)(HashObject(id, ID_LEN, &ret) % SESSION_ROWS); if (ret != 0) { WOLFSSL_MSG("Hash session failed"); return ret; diff --git a/wolfcrypt/src/misc.c b/wolfcrypt/src/misc.c index 2c8172247..57803ef71 100644 --- a/wolfcrypt/src/misc.c +++ b/wolfcrypt/src/misc.c @@ -802,6 +802,40 @@ WC_STATIC WC_INLINE byte w64LT(w64wrapper a, w64wrapper b) #endif /* WORD64_AVAILABLE && !WOLFSSL_W64_WRAPPER_TEST */ #endif /* WOLFSSL_W64_WRAPPER */ +#if defined(HAVE_SESSION_TICKET) || !defined(NO_CERTS) || \ + !defined(NO_SESSION_CACHE) +/* Make a word from the front of random hash */ +WC_STATIC WC_INLINE word32 MakeWordFromHash(const byte* hashID) +{ + return ((word32)hashID[0] << 24) | ((word32)hashID[1] << 16) | + ((word32)hashID[2] << 8) | (word32)hashID[3]; +} +#endif /* HAVE_SESSION_TICKET || !NO_CERTS || !NO_SESSION_CACHE */ + + +#if !defined(NO_SESSION_CACHE) || defined(HAVE_SESSION_TICKET) + +#include + +/* some session IDs aren't random after all, let's make them random */ +WC_STATIC WC_INLINE word32 HashObject(const byte* o, word32 len, int* error) +{ + byte digest[WC_MAX_DIGEST_SIZE]; + +#ifndef NO_MD5 + *error = wc_Md5Hash(o, len, digest); +#elif !defined(NO_SHA) + *error = wc_ShaHash(o, len, digest); +#elif !defined(NO_SHA256) + *error = wc_Sha256Hash(o, len, digest); +#else + #error "We need a digest to hash the session IDs" +#endif + + return *error == 0 ? MakeWordFromHash(digest) : 0; /* 0 on failure */ +} +#endif /* !NO_SESSION_CACHE || HAVE_SESSION_TICKET */ + #undef WC_STATIC #endif /* !WOLFSSL_MISC_INCLUDED && !NO_INLINE */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index b9bd5521c..969fdba6c 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1706,9 +1706,10 @@ enum Misc { #define MAX_ENCRYPT_SZ ENCRYPT_LEN -#define WOLFSSL_ASSERT_SIZEOF_GE(x, y) \ - typedef char _args_test[sizeof((x)) >= sizeof((y)) ? 1 : -1]; \ - (void)sizeof(_args_test) +#define WOLFSSL_ASSERT_SIZEOF_GE(x, y) do { \ + typedef char _args_test_[sizeof((x)) >= sizeof((y)) ? 1 : -1]; \ + (void)sizeof(_args_test_); \ +} while(0) /* states. Adding state before HANDSHAKE_DONE will break session importing */ enum states { diff --git a/wolfssl/wolfcrypt/misc.h b/wolfssl/wolfcrypt/misc.h index f732faeb8..8f8fc1764 100644 --- a/wolfssl/wolfcrypt/misc.h +++ b/wolfssl/wolfcrypt/misc.h @@ -127,6 +127,8 @@ WOLFSSL_LOCAL byte ctMaskSel(byte m, byte a, byte b); WOLFSSL_LOCAL int ctMaskSelInt(byte m, int a, int b); WOLFSSL_LOCAL byte ctSetLTE(int a, int b); WOLFSSL_LOCAL void ctMaskCopy(byte mask, byte* dst, byte* src, word16 size); +WOLFSSL_LOCAL word32 MakeWordFromHash(const byte* hashID); +WOLFSSL_LOCAL word32 HashObject(const byte* o, word32 len, int* error); #endif /* NO_INLINE */