From b6b007de3c64b8076938f1c23235927480fcbcc0 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 5 May 2022 14:51:30 +0200 Subject: [PATCH 1/2] Call ctx->rem_sess_cb when a session is about to be invalid Allow the user to register a session remove callback with wolfSSL_CTX_sess_set_remove_cb() that will be called when the session is about to be free'd or evicted from cache. --- src/internal.c | 15 ++- src/ssl.c | 247 ++++++++++++++++++++++++++++++++++++++------- src/tls13.c | 6 +- tests/api.c | 165 ++++++++++++++++++++++++++++++ wolfssl/internal.h | 17 +++- wolfssl/ssl.h | 2 + 6 files changed, 397 insertions(+), 55 deletions(-) diff --git a/src/internal.c b/src/internal.c index 6458eac18..23c25d54d 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2385,7 +2385,7 @@ void SSL_CtxResourceFree(WOLFSSL_CTX* ctx) wolfSSL_sk_X509_NAME_pop_free(ctx->ca_names, NULL); ctx->ca_names = NULL; #endif - #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) + #ifdef OPENSSL_EXTRA if (ctx->x509Chain) { wolfSSL_sk_X509_pop_free(ctx->x509Chain, NULL); ctx->x509Chain = NULL; @@ -7229,7 +7229,7 @@ void SSL_ResourceFree(WOLFSSL* ssl) #endif if (ssl->session != NULL) - wolfSSL_FreeSession(ssl->session); + wolfSSL_FreeSession(ssl->ctx, ssl->session); #ifdef HAVE_WRITE_DUP if (ssl->dupWrite) { FreeWriteDup(ssl); @@ -7299,7 +7299,7 @@ void SSL_ResourceFree(WOLFSSL* ssl) #endif } #endif /* WOLFSSL_STATIC_MEMORY */ -#if defined(OPENSSL_ALL) || defined(WOLFSSL_QT) +#ifdef OPENSSL_EXTRA /* Enough to free stack structure since WOLFSSL_CIPHER * isn't allocated separately. */ wolfSSL_sk_CIPHER_free(ssl->supportedCiphers); @@ -7531,11 +7531,11 @@ void FreeHandshakeResources(WOLFSSL* ssl) /* heap argument is the heap hint used when creating SSL */ void FreeSSL(WOLFSSL* ssl, void* heap) { - if (ssl->ctx) { - FreeSSL_Ctx(ssl->ctx); /* will decrement and free underlying CTX if 0 */ - } + WOLFSSL_CTX* ctx = ssl->ctx; SSL_ResourceFree(ssl); XFREE(ssl, heap, DYNAMIC_TYPE_SSL); + if (ctx) + FreeSSL_Ctx(ctx); /* will decrement and free underlying CTX if 0 */ (void)heap; } @@ -29335,9 +29335,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, WOLFSSL_MSG("Session lookup for resume failed"); ssl->options.resuming = 0; } else { - #ifdef HAVE_EXT_CACHE - wolfSSL_SESSION_free(session); - #endif if (MatchSuite(ssl, &clSuites) < 0) { WOLFSSL_MSG("Unsupported cipher suite, OldClientHello"); return UNSUPPORTED_SUITE; diff --git a/src/ssl.c b/src/ssl.c index 5260c1155..41477653a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -9950,7 +9950,7 @@ WOLFSSL_SESSION* wolfSSL_get_session(WOLFSSL* ssl) id = ssl->session->altSessionID; idSz = ID_LEN; } - error = AddSessionToCache(ssl->session, id, idSz, + error = AddSessionToCache(ssl->ctx, ssl->session, id, idSz, NULL, ssl->session->side, #ifdef HAVE_SESSION_TICKET ssl->session->ticketLen > 0, @@ -10038,7 +10038,7 @@ int wolfSSL_SetServerID(WOLFSSL* ssl, const byte* id, int len, int newSession) if (session) { if (wolfSSL_SetSession(ssl, session) != WOLFSSL_SUCCESS) { #ifdef HAVE_EXT_CACHE - wolfSSL_FreeSession(session); + wolfSSL_FreeSession(ssl->ctx, session); #endif WOLFSSL_MSG("wolfSSL_SetSession failed"); session = NULL; @@ -10054,7 +10054,7 @@ int wolfSSL_SetServerID(WOLFSSL* ssl, const byte* id, int len, int newSession) } #ifdef HAVE_EXT_CACHE else { - wolfSSL_FreeSession(session); + wolfSSL_FreeSession(ssl->ctx, session); } #endif @@ -12964,9 +12964,12 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) if (sess != NULL) { WOLFSSL_MSG("Session found in external cache"); error = wolfSSL_DupSession(sess, output, 0); +#ifdef HAVE_EX_DATA + output->ownExData = 0; /* Session cache owns external data */ +#endif /* If copy not set then free immediately */ if (!copy) - wolfSSL_SESSION_free(sess); + wolfSSL_FreeSession(ssl->ctx, sess); /* We want to restore the bogus ID for TLS compatibility */ if (ssl->session->haveAltSessionID && output == ssl->session) { @@ -13072,6 +13075,9 @@ int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output) } #endif error = wolfSSL_DupSession(sess, output, 1); +#ifdef HAVE_EX_DATA + output->ownExData = 0; /* Session cache owns external data */ +#endif } else { error = WOLFSSL_FAILURE; @@ -13402,9 +13408,9 @@ WOLFSSL_SESSION* ClientSessionToSession(const WOLFSSL_SESSION* session) #endif } -int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz, - int* sessionIndex, int side, word16 useTicket, - ClientSession** clientCacheEntry) +int AddSessionToCache(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* addSession, + const byte* id, byte idSz, int* sessionIndex, int side, + word16 useTicket, ClientSession** clientCacheEntry) { WOLFSSL_SESSION* cacheSession = NULL; SessionRow* sessRow = NULL; @@ -13423,6 +13429,7 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz, int i; int overwrite = 0; + (void)ctx; (void)sessionIndex; (void)useTicket; (void)clientCacheEntry; @@ -13483,6 +13490,16 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz, *sessionIndex = (row << SESSIDX_ROW_SHIFT) | idx; #endif cacheSession = &sessRow->Sessions[idx]; + +#ifdef HAVE_EX_DATA + if (cacheSession->rem_sess_cb && cacheSession->ownExData) { + cacheSession->rem_sess_cb(NULL, cacheSession); + /* Make sure not to call remove functions again */ + cacheSession->ownExData = 0; + cacheSession->rem_sess_cb = NULL; + } +#endif + cacheSession->type = WOLFSSL_SESSION_TYPE_CACHE; cacheSession->cacheRow = row; @@ -13528,6 +13545,13 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz, XMEMCPY(cacheSession->sessionID, id, ID_LEN); cacheSession->sessionIDSz = ID_LEN; } +#ifdef HAVE_EX_DATA + if (ctx->rem_sess_cb != NULL) { + addSession->ownExData = 0; + cacheSession->ownExData = 1; + cacheSession->rem_sess_cb = ctx->rem_sess_cb; + } +#endif } #ifdef HAVE_SESSION_TICKET else if (ticBuffUsed) { @@ -13542,10 +13566,10 @@ int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, byte idSz, cacheSession = NULL; /* Can't access after unlocked */ #ifndef NO_CLIENT_CACHE - if (ret == 0) { + if (ret == 0 && clientCacheEntry != NULL) { ClientSession* clientCache = AddSessionToClientCache(side, row, idx, addSession->serverID, addSession->idLen, id, useTicket); - if (clientCache != NULL && clientCacheEntry != NULL) + if (clientCache != NULL) *clientCacheEntry = clientCache; } #endif @@ -13665,12 +13689,15 @@ void AddSession(WOLFSSL* ssl) } /* Setup done */ + if (ssl->options.side == WOLFSSL_SERVER_END /* No point in adding a + * client session */ #ifdef HAVE_EXT_CACHE - if (!ssl->options.internalCacheOff) + && !ssl->options.internalCacheOff #endif + ) { /* Try to add the session to cache. Its ok if we don't succeed. */ - (void)AddSessionToCache(session, id, idSz, + (void)AddSessionToCache(ssl->ctx, session, id, idSz, #ifdef SESSION_INDEX &ssl->sessionIndex, #else @@ -13691,7 +13718,7 @@ void AddSession(WOLFSSL* ssl) wolfSSL_SESSION_up_ref(session); cbRet = ssl->ctx->new_sess_cb(ssl, session); if (cbRet == 0) - wolfSSL_FreeSession(session); + wolfSSL_FreeSession(ssl->ctx, session); } #endif @@ -19695,6 +19722,9 @@ WOLFSSL_SESSION* wolfSSL_NewSession(void* heap) XFREE(ret, NULL, DYNAMIC_TYPE_OPENSSL); return NULL; } +#endif +#ifdef HAVE_EX_DATA + ret->ownExData = 1; #endif } return ret; @@ -19912,7 +19942,7 @@ WOLFSSL_SESSION* wolfSSL_SESSION_dup(WOLFSSL_SESSION* session) copy = wolfSSL_NewSession(session->heap); if (copy != NULL && wolfSSL_DupSession(session, copy, 0) != WOLFSSL_SUCCESS) { - wolfSSL_FreeSession(copy); + wolfSSL_FreeSession(NULL, copy); copy = NULL; } return copy; @@ -19923,12 +19953,14 @@ WOLFSSL_SESSION* wolfSSL_SESSION_dup(WOLFSSL_SESSION* session) #endif /* HAVE_EXT_CACHE */ } -void wolfSSL_FreeSession(WOLFSSL_SESSION* session) +void wolfSSL_FreeSession(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* session) { session = ClientSessionToSession(session); if (session == NULL) return; + (void)ctx; + /* refCount will always be 1 or more if created externally. * Internal cache sessions don't initialize a refMutex. */ if (session->refCount > 0) { @@ -19951,6 +19983,18 @@ void wolfSSL_FreeSession(WOLFSSL_SESSION* session) #endif } +#if defined(HAVE_EXT_CACHE) || defined(HAVE_EX_DATA) + if (ctx != NULL && ctx->rem_sess_cb +#ifdef HAVE_EX_DATA + && session->ownExData /* This will be true if we are not using the + * internal cache so it will get called for + * externally cached sessions as well. */ +#endif + ) { + ctx->rem_sess_cb(ctx, session); + } +#endif + #ifdef HAVE_EX_DATA_CLEANUP_HOOKS wolfSSL_CRYPTO_cleanup_ex_data(&session->ex_data); #endif @@ -19980,7 +20024,7 @@ void wolfSSL_FreeSession(WOLFSSL_SESSION* session) void wolfSSL_SESSION_free(WOLFSSL_SESSION* session) { session = ClientSessionToSession(session); - wolfSSL_FreeSession(session); + wolfSSL_FreeSession(NULL, session); } #ifndef NO_SESSION_CACHE @@ -20006,7 +20050,7 @@ int wolfSSL_CTX_add_session(WOLFSSL_CTX* ctx, WOLFSSL_SESSION* session) idSz = ID_LEN; } - error = AddSessionToCache(session, id, idSz, + error = AddSessionToCache(ctx, session, id, idSz, NULL, session->side, #ifdef HAVE_SESSION_TICKET session->ticketLen > 0, @@ -20299,30 +20343,16 @@ const WOLFSSL_CIPHER* wolfSSL_get_cipher_by_value(word16 value) } -#if defined(OPENSSL_ALL) +#if defined(OPENSSL_EXTRA) /* Free the structure for WOLFSSL_CIPHER stack * * sk stack to free nodes in */ void wolfSSL_sk_CIPHER_free(WOLF_STACK_OF(WOLFSSL_CIPHER)* sk) { - WOLFSSL_STACK* node; - WOLFSSL_STACK* tmp; WOLFSSL_ENTER("wolfSSL_sk_CIPHER_free"); - if (sk == NULL) - return; - - /* parse through stack freeing each node */ - node = sk->next; - while (node) { - tmp = node; - node = node->next; - XFREE(tmp, NULL, DYNAMIC_TYPE_OPENSSL); - } - - /* free head of stack */ - XFREE(sk, NULL, DYNAMIC_TYPE_ASN1); + wolfSSL_sk_free(sk); } #endif /* OPENSSL_ALL */ @@ -25114,7 +25144,7 @@ void wolfSSL_CTX_sess_set_remove_cb(WOLFSSL_CTX* ctx, void (*f)(WOLFSSL_CTX*, if (ctx == NULL) return; -#ifdef HAVE_EXT_CACHE +#if defined(HAVE_EXT_CACHE) || defined(HAVE_EX_DATA) ctx->rem_sess_cb = f; #else (void)f; @@ -39379,8 +39409,53 @@ int wolfSSL_SESSION_set_ex_data(WOLFSSL_SESSION* session, int idx, void* data) WOLFSSL_ENTER("wolfSSL_SESSION_set_ex_data"); #ifdef HAVE_EX_DATA session = ClientSessionToSession(session); - if (session != NULL) - ret = wolfSSL_CRYPTO_set_ex_data(&session->ex_data, idx, data); + if (session != NULL) { +#ifndef NO_SESSION_CACHE + if (!session->ownExData) { + /* Need to update in cache */ + int row; + int i; + SessionRow* sessRow = NULL; + const byte* id; + byte foundCache = 0; + + id = session->sessionID; + if (session->haveAltSessionID) + id = session->altSessionID; + + row = (int)(HashSession(id, ID_LEN, &ret) % SESSION_ROWS); + if (ret != 0) { + WOLFSSL_MSG("Hash session failed"); + return WOLFSSL_FAILURE; + } + + sessRow = &SessionCache[row]; + if (SESSION_ROW_LOCK(sessRow) != 0) { + WOLFSSL_MSG("Session row lock failed"); + return WOLFSSL_FAILURE; + } + + for (i = 0; i < SESSIONS_PER_ROW && i < sessRow->totalCount; i++) { + if (XMEMCMP(id, sessRow->Sessions[i].sessionID, ID_LEN) == 0) { + ret = wolfSSL_CRYPTO_set_ex_data( + &sessRow->Sessions[i].ex_data, idx, data); + foundCache = 1; + break; + } + } + SESSION_ROW_UNLOCK(sessRow); + /* If we don't have a session in cache then clear the cache and + * own it */ + if (!foundCache) { + XMEMSET(&session->ex_data, 0, sizeof(WOLFSSL_CRYPTO_EX_DATA)); + session->ownExData = 1; + ret = wolfSSL_CRYPTO_set_ex_data(&session->ex_data, idx, data); + } + } + else +#endif + ret = wolfSSL_CRYPTO_set_ex_data(&session->ex_data, idx, data); + } #else (void)session; (void)idx; @@ -39412,7 +39487,54 @@ void* wolfSSL_SESSION_get_ex_data(const WOLFSSL_SESSION* session, int idx) #ifdef HAVE_EX_DATA session = ClientSessionToSession(session); if (session != NULL) { - return wolfSSL_CRYPTO_get_ex_data(&session->ex_data, idx); +#ifndef NO_SESSION_CACHE + if (!session->ownExData) { + /* Need to retrieve the data from the session cache */ + int row; + int i; + int error = 0; + SessionRow* sessRow = NULL; + const byte* id; + void* ret = NULL; + byte foundCache = 0; + + id = session->sessionID; + if (session->haveAltSessionID) + id = session->altSessionID; + + row = (int)(HashSession(id, ID_LEN, &error) % SESSION_ROWS); + if (error != 0) { + WOLFSSL_MSG("Hash session failed"); + return NULL; + } + + sessRow = &SessionCache[row]; + if (SESSION_ROW_LOCK(sessRow) != 0) { + WOLFSSL_MSG("Session row lock failed"); + return NULL; + } + + for (i = 0; i < SESSIONS_PER_ROW && i < sessRow->totalCount; i++) { + if (XMEMCMP(id, sessRow->Sessions[i].sessionID, ID_LEN) == 0) { + ret = wolfSSL_CRYPTO_get_ex_data( + &sessRow->Sessions[i].ex_data, idx); + foundCache = 1; + break; + } + } + SESSION_ROW_UNLOCK(sessRow); + /* If we don't have a session in cache then clear the cache and + * own it */ + if (!foundCache) { + WOLFSSL_SESSION* s = (WOLFSSL_SESSION*)session; + XMEMSET(&s->ex_data, 0, sizeof(WOLFSSL_CRYPTO_EX_DATA)); + s->ownExData = 1; + } + return ret; + } + else +#endif + return wolfSSL_CRYPTO_get_ex_data(&session->ex_data, idx); } #else (void)session; @@ -41177,11 +41299,60 @@ int wolfSSL_SSL_CTX_remove_session(WOLFSSL_CTX *ctx, WOLFSSL_SESSION *s) { /* Don't remove session just timeout session. */ s->timeout = 0; +#ifndef NO_SESSION_CACHE + /* Clear the timeout in the cache */ + { + int row; + int i; + SessionRow* sessRow = NULL; + WOLFSSL_SESSION *cacheSession; + const byte* id; + int ret = 0; + + id = s->sessionID; + if (s->haveAltSessionID) + id = s->altSessionID; + + row = (int)(HashSession(id, ID_LEN, &ret) % SESSION_ROWS); + if (ret != 0) { + WOLFSSL_MSG("Hash session failed"); + return ret; + } + + sessRow = &SessionCache[row]; + if (SESSION_ROW_LOCK(sessRow) != 0) { + WOLFSSL_MSG("Session row lock failed"); + return BAD_MUTEX_E; + } + + for (i = 0; i < SESSIONS_PER_ROW && i < sessRow->totalCount; i++) { + cacheSession = &sessRow->Sessions[i]; + if (XMEMCMP(id, cacheSession->sessionID, ID_LEN) == 0) { + if (ctx->method->side != cacheSession->side) + continue; + cacheSession->timeout = 0; +#ifdef HAVE_EX_DATA + if (cacheSession->ownExData) { + /* Most recent version of ex data is in cache. Copy it + * over so the user can free it. */ + XMEMCPY(&s->ex_data, &cacheSession->ex_data, + sizeof(WOLFSSL_CRYPTO_EX_DATA)); + } + cacheSession->ownExData = 0; /* We clear below */ + s->ownExData = 1; +#endif + break; + } + } + SESSION_ROW_UNLOCK(sessRow); + } +#endif } -#ifdef HAVE_EXT_CACHE - if (ctx->rem_sess_cb != NULL) +#if defined(HAVE_EXT_CACHE) || defined(HAVE_EX_DATA) + if (ctx->rem_sess_cb != NULL) { ctx->rem_sess_cb(ctx, s); + } #endif return 0; diff --git a/src/tls13.c b/src/tls13.c index 8720845a9..5da044305 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -2829,13 +2829,13 @@ static int SetupPskKey(WOLFSSL* ssl, PreSharedKey* psk, int clientHello) /* OpenSSL compatible callback that gets cached session. */ if (ssl->options.session_psk_cb(ssl, handshake_md, &id, &idlen, &psksession) == 0) { - wolfSSL_SESSION_free(psksession); + wolfSSL_FreeSession(ssl->ctx, psksession); WOLFSSL_MSG("psk session callback failed"); return PSK_KEY_ERROR; } if (psksession != NULL) { if (idlen > MAX_PSK_KEY_LEN) { - wolfSSL_SESSION_free(psksession); + wolfSSL_FreeSession(ssl->ctx, psksession); WOLFSSL_MSG("psk key length is too long"); return PSK_KEY_ERROR; } @@ -2845,7 +2845,7 @@ static int SetupPskKey(WOLFSSL* ssl, PreSharedKey* psk, int clientHello) suite[0] = psksession->cipherSuite0; suite[1] = psksession->cipherSuite; /* Not needed anymore. */ - wolfSSL_SESSION_free(psksession); + wolfSSL_FreeSession(ssl->ctx, psksession); /* Leave pointer not NULL to indicate success with callback. */ } } diff --git a/tests/api.c b/tests/api.c index 7edb136f5..d1947aaab 100644 --- a/tests/api.c +++ b/tests/api.c @@ -39980,6 +39980,170 @@ static void test_wolfSSL_SESSION(void) #endif } +#if defined(OPENSSL_EXTRA) && defined(HAVE_IO_TESTS_DEPENDENCIES) && \ + defined(HAVE_EX_DATA) +static int clientSessRemCountMalloc = 0; +static int serverSessRemCountMalloc = 0; +static int clientSessRemCountFree = 0; +static int serverSessRemCountFree = 0; +static WOLFSSL_CTX* serverSessCtx = NULL; +static WOLFSSL_SESSION* serverSess = NULL; +#ifndef NO_SESSION_CACHE_REF +static WOLFSSL_CTX* clientSessCtx = NULL; +static WOLFSSL_SESSION* clientSess = NULL; +#endif +static int serverSessRemIdx = 3; + +static void SessRemCtxCb(WOLFSSL_CTX *ctx, WOLFSSL_SESSION *sess) +{ + int* mallocedData = SSL_SESSION_get_ex_data(sess, serverSessRemIdx); + (void)ctx; + AssertNotNull(mallocedData); + if (!*mallocedData) + clientSessRemCountFree++; + else + serverSessRemCountFree++; + XFREE(mallocedData, NULL, DYNAMIC_TYPE_SESSION); + SSL_SESSION_set_ex_data(sess, serverSessRemIdx, NULL); +} + +static void SessRemCtxSetupCb(WOLFSSL_CTX* ctx) +{ + SSL_CTX_sess_set_remove_cb(ctx, SessRemCtxCb); +#if defined(WOLFSSL_TLS13) && !defined(HAVE_SESSION_TICKET) && \ + !defined(NO_SESSION_CACHE_REF) + /* Allow downgrade, set min version, and disable TLS 1.3. + * Do this because without NO_SESSION_CACHE_REF we will want to return a + * reference to the session cache. But with WOLFSSL_TLS13 and without + * HAVE_SESSION_TICKET we won't have a session ID to be able to place the + * session in the cache. In this case we need to downgrade to previous + * versions to just use the legacy session ID field. */ + AssertIntEQ(SSL_CTX_set_min_proto_version(ctx, SSL3_VERSION), SSL_SUCCESS); + AssertIntEQ(SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION), SSL_SUCCESS); +#endif +} + +static void SessRemSslSetupCb(WOLFSSL* ssl) +{ + int* mallocedData = (int*)XMALLOC(sizeof(int), NULL, DYNAMIC_TYPE_SESSION); + AssertNotNull(mallocedData); + *mallocedData = SSL_is_server(ssl); + if (!*mallocedData) { + clientSessRemCountMalloc++; +#ifndef NO_SESSION_CACHE_REF + AssertNotNull(clientSess = SSL_get1_session(ssl)); + AssertIntEQ(SSL_CTX_up_ref(clientSessCtx = SSL_get_SSL_CTX(ssl)), + SSL_SUCCESS); +#endif + } + else { + serverSessRemCountMalloc++; + AssertNotNull(serverSess = SSL_get1_session(ssl)); + AssertIntEQ(SSL_CTX_up_ref(serverSessCtx = SSL_get_SSL_CTX(ssl)), + SSL_SUCCESS); + } + AssertIntEQ(SSL_SESSION_set_ex_data(SSL_get_session(ssl), serverSessRemIdx, + mallocedData), SSL_SUCCESS); +} +#endif + +static void test_wolfSSL_CTX_sess_set_remove_cb(void) +{ +#if defined(OPENSSL_EXTRA) && defined(HAVE_IO_TESTS_DEPENDENCIES) && \ + defined(HAVE_EX_DATA) + /* Check that the remove callback gets called for external data in a + * session object */ + callback_functions func_cb; + tcp_ready ready; + func_args client_args; + func_args server_args; + THREAD_TYPE serverThread; + + printf(testingFmt, "wolfSSL_CTX_sess_set_remove_cb()"); + + XMEMSET(&client_args, 0, sizeof(func_args)); + XMEMSET(&server_args, 0, sizeof(func_args)); + XMEMSET(&func_cb, 0, sizeof(callback_functions)); + + +#ifdef WOLFSSL_TIRTOS + fdOpenSession(Task_self()); +#endif + + StartTCP(); + InitTcpReady(&ready); + +#if defined(USE_WINDOWS_API) + /* use RNG to get random port if using windows */ + ready.port = GetRandomPort(); +#endif + + server_args.signal = &ready; + client_args.signal = &ready; + client_args.callbacks = &func_cb; + server_args.callbacks = &func_cb; + func_cb.ctx_ready = SessRemCtxSetupCb; + func_cb.on_result = SessRemSslSetupCb; + + start_thread(test_server_nofail, &server_args, &serverThread); + wait_tcp_ready(&server_args); + test_client_nofail(&client_args, NULL); + join_thread(serverThread); + + AssertTrue(client_args.return_code); + AssertTrue(server_args.return_code); + + FreeTcpReady(&ready); + +#ifdef WOLFSSL_TIRTOS + fdOpenSession(Task_self()); +#endif + + /* Both should have been allocated */ + AssertIntEQ(clientSessRemCountMalloc, 1); + AssertIntEQ(serverSessRemCountMalloc, 1); +#ifdef NO_SESSION_CACHE_REF + /* Client session should not be added to cache so this should be free'd when + * the SSL object was being free'd */ + AssertIntEQ(clientSessRemCountFree, 1); +#else + /* Client session is in cache due to requiring a persistent reference */ + AssertIntEQ(clientSessRemCountFree, 0); + /* Force a cache lookup */ + AssertNotNull(SSL_SESSION_get_ex_data(clientSess, serverSessRemIdx)); + /* Force a cache update */ + AssertNotNull(SSL_SESSION_set_ex_data(clientSess, serverSessRemIdx - 1, 0)); + /* This should set the timeout to 0 and call the remove callback from within + * the session cache. */ + AssertIntEQ(SSL_CTX_remove_session(clientSessCtx, clientSess), 0); + AssertNull(SSL_SESSION_get_ex_data(clientSess, serverSessRemIdx)); + AssertIntEQ(clientSessRemCountFree, 1); +#endif + /* Server session is in the cache so ex_data isn't free'd with the SSL + * object */ + AssertIntEQ(serverSessRemCountFree, 0); + /* Force a cache lookup */ + AssertNotNull(SSL_SESSION_get_ex_data(serverSess, serverSessRemIdx)); + /* Force a cache update */ + AssertNotNull(SSL_SESSION_set_ex_data(serverSess, serverSessRemIdx - 1, 0)); + /* This should set the timeout to 0 and call the remove callback from within + * the session cache. */ + AssertIntEQ(SSL_CTX_remove_session(serverSessCtx, serverSess), 0); + AssertNull(SSL_SESSION_get_ex_data(serverSess, serverSessRemIdx)); + AssertIntEQ(serverSessRemCountFree, 1); + + /* Need to free the references that we kept */ + SSL_CTX_free(serverSessCtx); + SSL_SESSION_free(serverSess); +#ifndef NO_SESSION_CACHE_REF + SSL_CTX_free(clientSessCtx); + SSL_SESSION_free(clientSess); +#endif + + printf(resultFmt, passed); +#endif +} + static void test_wolfSSL_ticket_keys(void) { #if defined(HAVE_SESSION_TICKET) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && \ @@ -54115,6 +54279,7 @@ void ApiTest(void) #endif test_wolfSSL_cert_cb(); test_wolfSSL_SESSION(); + test_wolfSSL_CTX_sess_set_remove_cb(); test_wolfSSL_ticket_keys(); test_wolfSSL_DES_ecb_encrypt(); test_wolfSSL_sk_GENERAL_NAME(); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 5cca46ae1..21966bb34 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3079,7 +3079,9 @@ struct WOLFSSL_CTX { #ifdef HAVE_EXT_CACHE WOLFSSL_SESSION*(*get_sess_cb)(WOLFSSL*, const unsigned char*, int, int*); int (*new_sess_cb)(WOLFSSL*, WOLFSSL_SESSION*); - void (*rem_sess_cb)(WOLFSSL_CTX*, WOLFSSL_SESSION*); +#endif +#if defined(HAVE_EXT_CACHE) || defined(HAVE_EX_DATA) + Rem_Sess_Cb rem_sess_cb; #endif #if defined(OPENSSL_EXTRA) && defined(WOLFCRYPT_HAVE_SRP) && !defined(NO_SHA256) Srp* srp; /* TLS Secure Remote Password Protocol*/ @@ -3351,6 +3353,10 @@ struct WOLFSSL_SESSION { #endif byte altSessionID[ID_LEN]; byte haveAltSessionID:1; +#ifdef HAVE_EX_DATA + byte ownExData:1; + Rem_Sess_Cb rem_sess_cb; +#endif void* heap; /* WARNING The above fields (up to and including the heap) are not copied * in wolfSSL_DupSession. Place new fields after the heap @@ -3443,9 +3449,9 @@ WOLFSSL_LOCAL WOLFSSL_SESSION* wolfSSL_NewSession(void* heap); WOLFSSL_LOCAL WOLFSSL_SESSION* wolfSSL_GetSession( WOLFSSL* ssl, byte* masterSecret, byte restoreSessionCerts); WOLFSSL_LOCAL void AddSession(WOLFSSL* ssl); -WOLFSSL_LOCAL int AddSessionToCache(WOLFSSL_SESSION* addSession, const byte* id, - byte idSz, int* sessionIndex, int side, word16 useTicket, - ClientSession** clientCacheEntry); +WOLFSSL_LOCAL int AddSessionToCache(WOLFSSL_CTX* ssl, + WOLFSSL_SESSION* addSession, const byte* id, byte idSz, int* sessionIndex, + int side, word16 useTicket, ClientSession** clientCacheEntry); #ifndef NO_CLIENT_CACHE WOLFSSL_LOCAL ClientSession* AddSessionToClientCache(int side, int row, int idx, byte* serverID, word16 idLen, const byte* sessionID, @@ -3455,7 +3461,8 @@ WOLFSSL_LOCAL WOLFSSL_SESSION* ClientSessionToSession(const WOLFSSL_SESSION* session); WOLFSSL_LOCAL int wolfSSL_GetSessionFromCache(WOLFSSL* ssl, WOLFSSL_SESSION* output); WOLFSSL_LOCAL int wolfSSL_SetSession(WOLFSSL* ssl, WOLFSSL_SESSION* session); -WOLFSSL_LOCAL void wolfSSL_FreeSession(WOLFSSL_SESSION* session); +WOLFSSL_LOCAL void wolfSSL_FreeSession(WOLFSSL_CTX* ctx, + WOLFSSL_SESSION* session); WOLFSSL_LOCAL int wolfSSL_DupSession(const WOLFSSL_SESSION* input, WOLFSSL_SESSION* output, int avoidSysCalls); diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index fe9563892..2a4c8d316 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -4632,6 +4632,8 @@ WOLFSSL_API int wolfSSL_CTX_AsyncPoll(WOLFSSL_CTX* ctx, WOLF_EVENT** events, int typedef void (*SSL_Msg_Cb)(int write_p, int version, int content_type, const void *buf, size_t len, WOLFSSL *ssl, void *arg); +typedef void (*Rem_Sess_Cb)(WOLFSSL_CTX*, WOLFSSL_SESSION*); + #if defined(HAVE_SECRET_CALLBACK) typedef void (*wolfSSL_CTX_keylog_cb_func) (const WOLFSSL* ssl, const char* line); From 7f8f0dcffeecefa077c4315501833eb4183efea2 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 13 May 2022 12:54:50 +0200 Subject: [PATCH 2/2] Refactor cache ex_data update/retrieve into one function - Add explicit pointer cast --- src/ssl.c | 154 ++++++++++++++++++++++++-------------------------- tests/api.c | 2 +- wolfssl/ssl.h | 4 +- 3 files changed, 76 insertions(+), 84 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 41477653a..969d26988 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -39403,6 +39403,69 @@ void wolfSSL_print_all_errors_fp(XFILE fp) #if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) || \ defined(HAVE_EX_DATA) + +#if defined(HAVE_EX_DATA) && !defined(NO_SESSION_CACHE) +static void SESSION_ex_data_cache_update(WOLFSSL_SESSION* session, int idx, + void* data, byte get, void** getRet, int* setRet) +{ + int row; + int i; + int error = 0; + SessionRow* sessRow = NULL; + const byte* id; + byte foundCache = 0; + + if (getRet != NULL) + *getRet = NULL; + if (setRet != NULL) + *setRet = WOLFSSL_FAILURE; + + id = session->sessionID; + if (session->haveAltSessionID) + id = session->altSessionID; + + row = (int)(HashSession(id, ID_LEN, &error) % SESSION_ROWS); + if (error != 0) { + WOLFSSL_MSG("Hash session failed"); + return; + } + + sessRow = &SessionCache[row]; + if (SESSION_ROW_LOCK(sessRow) != 0) { + WOLFSSL_MSG("Session row lock failed"); + return; + } + + for (i = 0; i < SESSIONS_PER_ROW && i < sessRow->totalCount; i++) { + if (XMEMCMP(id, sessRow->Sessions[i].sessionID, ID_LEN) == 0 + && session->side == sessRow->Sessions[i].side) { + if (get) { + *getRet = wolfSSL_CRYPTO_get_ex_data( + &sessRow->Sessions[i].ex_data, idx); + } + else { + *setRet = wolfSSL_CRYPTO_set_ex_data( + &sessRow->Sessions[i].ex_data, idx, data); + } + foundCache = 1; + break; + } + } + SESSION_ROW_UNLOCK(sessRow); + /* If we don't have a session in cache then clear the ex_data and + * own it */ + if (!foundCache) { + XMEMSET(&session->ex_data, 0, sizeof(WOLFSSL_CRYPTO_EX_DATA)); + session->ownExData = 1; + if (!get) { + *setRet = wolfSSL_CRYPTO_set_ex_data(&session->ex_data, idx, + data); + } + } + +} +#endif + int wolfSSL_SESSION_set_ex_data(WOLFSSL_SESSION* session, int idx, void* data) { int ret = WOLFSSL_FAILURE; @@ -39413,48 +39476,13 @@ int wolfSSL_SESSION_set_ex_data(WOLFSSL_SESSION* session, int idx, void* data) #ifndef NO_SESSION_CACHE if (!session->ownExData) { /* Need to update in cache */ - int row; - int i; - SessionRow* sessRow = NULL; - const byte* id; - byte foundCache = 0; - - id = session->sessionID; - if (session->haveAltSessionID) - id = session->altSessionID; - - row = (int)(HashSession(id, ID_LEN, &ret) % SESSION_ROWS); - if (ret != 0) { - WOLFSSL_MSG("Hash session failed"); - return WOLFSSL_FAILURE; - } - - sessRow = &SessionCache[row]; - if (SESSION_ROW_LOCK(sessRow) != 0) { - WOLFSSL_MSG("Session row lock failed"); - return WOLFSSL_FAILURE; - } - - for (i = 0; i < SESSIONS_PER_ROW && i < sessRow->totalCount; i++) { - if (XMEMCMP(id, sessRow->Sessions[i].sessionID, ID_LEN) == 0) { - ret = wolfSSL_CRYPTO_set_ex_data( - &sessRow->Sessions[i].ex_data, idx, data); - foundCache = 1; - break; - } - } - SESSION_ROW_UNLOCK(sessRow); - /* If we don't have a session in cache then clear the cache and - * own it */ - if (!foundCache) { - XMEMSET(&session->ex_data, 0, sizeof(WOLFSSL_CRYPTO_EX_DATA)); - session->ownExData = 1; - ret = wolfSSL_CRYPTO_set_ex_data(&session->ex_data, idx, data); - } + SESSION_ex_data_cache_update(session, idx, data, 0, NULL, &ret); } else #endif + { ret = wolfSSL_CRYPTO_set_ex_data(&session->ex_data, idx, data); + } } #else (void)session; @@ -39483,6 +39511,7 @@ int wolfSSL_SESSION_set_ex_data_with_cleanup( void* wolfSSL_SESSION_get_ex_data(const WOLFSSL_SESSION* session, int idx) { + void* ret = NULL; WOLFSSL_ENTER("wolfSSL_SESSION_get_ex_data"); #ifdef HAVE_EX_DATA session = ClientSessionToSession(session); @@ -39490,57 +39519,20 @@ void* wolfSSL_SESSION_get_ex_data(const WOLFSSL_SESSION* session, int idx) #ifndef NO_SESSION_CACHE if (!session->ownExData) { /* Need to retrieve the data from the session cache */ - int row; - int i; - int error = 0; - SessionRow* sessRow = NULL; - const byte* id; - void* ret = NULL; - byte foundCache = 0; - - id = session->sessionID; - if (session->haveAltSessionID) - id = session->altSessionID; - - row = (int)(HashSession(id, ID_LEN, &error) % SESSION_ROWS); - if (error != 0) { - WOLFSSL_MSG("Hash session failed"); - return NULL; - } - - sessRow = &SessionCache[row]; - if (SESSION_ROW_LOCK(sessRow) != 0) { - WOLFSSL_MSG("Session row lock failed"); - return NULL; - } - - for (i = 0; i < SESSIONS_PER_ROW && i < sessRow->totalCount; i++) { - if (XMEMCMP(id, sessRow->Sessions[i].sessionID, ID_LEN) == 0) { - ret = wolfSSL_CRYPTO_get_ex_data( - &sessRow->Sessions[i].ex_data, idx); - foundCache = 1; - break; - } - } - SESSION_ROW_UNLOCK(sessRow); - /* If we don't have a session in cache then clear the cache and - * own it */ - if (!foundCache) { - WOLFSSL_SESSION* s = (WOLFSSL_SESSION*)session; - XMEMSET(&s->ex_data, 0, sizeof(WOLFSSL_CRYPTO_EX_DATA)); - s->ownExData = 1; - } - return ret; + SESSION_ex_data_cache_update((WOLFSSL_SESSION*)session, idx, NULL, + 1, &ret, NULL); } else #endif - return wolfSSL_CRYPTO_get_ex_data(&session->ex_data, idx); + { + ret = wolfSSL_CRYPTO_get_ex_data(&session->ex_data, idx); + } } #else (void)session; (void)idx; #endif - return NULL; + return ret; } #endif /* OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL || HAVE_EX_DATA */ diff --git a/tests/api.c b/tests/api.c index d1947aaab..0f29e6375 100644 --- a/tests/api.c +++ b/tests/api.c @@ -39996,7 +39996,7 @@ static int serverSessRemIdx = 3; static void SessRemCtxCb(WOLFSSL_CTX *ctx, WOLFSSL_SESSION *sess) { - int* mallocedData = SSL_SESSION_get_ex_data(sess, serverSessRemIdx); + int* mallocedData = (int*)SSL_SESSION_get_ex_data(sess, serverSessRemIdx); (void)ctx; AssertNotNull(mallocedData); if (!*mallocedData) diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 2a4c8d316..472306c76 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -4628,12 +4628,12 @@ WOLFSSL_API int wolfSSL_CTX_AsyncPoll(WOLFSSL_CTX* ctx, WOLF_EVENT** events, int WOLF_EVENT_FLAG flags, int* eventCount); #endif /* WOLFSSL_ASYNC_CRYPT */ +typedef void (*Rem_Sess_Cb)(WOLFSSL_CTX*, WOLFSSL_SESSION*); + #ifdef OPENSSL_EXTRA typedef void (*SSL_Msg_Cb)(int write_p, int version, int content_type, const void *buf, size_t len, WOLFSSL *ssl, void *arg); -typedef void (*Rem_Sess_Cb)(WOLFSSL_CTX*, WOLFSSL_SESSION*); - #if defined(HAVE_SECRET_CALLBACK) typedef void (*wolfSSL_CTX_keylog_cb_func) (const WOLFSSL* ssl, const char* line);