From 4d0ea628575e6e87e10e3d474060b7222f0dff58 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 18 Aug 2022 17:27:52 +0200 Subject: [PATCH] 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 {