From c1982d36ed5555f88df900539183b43bfc400294 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 14 Aug 2026 16:01:21 +0000 Subject: [PATCH] Match the full server ID length in the client session cache wolfSSL_GetSessionClient() compared only the requested number of bytes, so a short server ID could match a cached entry that merely started with the same bytes and landed in the same cache row. Applications partitioning the cache with wolfSSL_SetServerID() could then offer a ticket across partitions, against RFC 9846 Appendix C.4. Fixes https://github.com/wolfSSL/wolfssl/issues/11132 --- src/ssl_sess.c | 7 +++- tests/api/test_session.c | 79 ++++++++++++++++++++++++++++++++++++++++ tests/api/test_session.h | 4 +- 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/src/ssl_sess.c b/src/ssl_sess.c index 9677160825..1a25a9ecf9 100644 --- a/src/ssl_sess.c +++ b/src/ssl_sess.c @@ -1052,8 +1052,11 @@ WOLFSSL_SESSION* wolfSSL_GetSessionClient(WOLFSSL* ssl, const byte* id, int len) #else current = &sessRow->Sessions[clSess[idx].serverIdx]; #endif - if (current && XMEMCMP(current->serverID, id, - (unsigned long)len) == 0) { + /* Require the same length as well as the same bytes. Comparing only + * the requested length lets a short ID alias the prefix of a longer + * cached one, mixing sessions the application meant to keep apart. */ + if (current && current->idLen == (word16)len && + XMEMCMP(current->serverID, id, (unsigned long)len) == 0) { WOLFSSL_MSG("Found a serverid match for client"); if (LowResTimer() < (current->bornOn + current->timeout)) { WOLFSSL_MSG("Session valid"); diff --git a/tests/api/test_session.c b/tests/api/test_session.c index 3c08f32bef..5ad90c8eb8 100644 --- a/tests/api/test_session.c +++ b/tests/api/test_session.c @@ -1739,3 +1739,82 @@ int test_wolfSSL_GetSessionAtIndex(void) #endif /* SESSION_INDEX && HAVE_SESSION_TICKET && !NO_SESSION_CACHE && * !NO_WOLFSSL_CLIENT && !NO_TLS */ + +/* RFC 9846 Appendix C.4: client applications should not offer tickets across + * connections meant to be uncorrelated. wolfSSL_SetServerID() is how an + * application keeps such connections apart, so a shorter ID must not match a + * cached entry that merely starts with the same bytes. */ +int test_wolfSSL_client_cache_id_prefix(void) +{ + EXPECT_DECLS; +#if !defined(NO_SESSION_CACHE) && !defined(NO_CLIENT_CACHE) && \ + !defined(NO_TLS) && !defined(WOLFSSL_NO_TLS12) && \ + defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) + WOLFSSL_CTX* ctx_c = NULL; + WOLFSSL_CTX* ctx_s = NULL; + WOLFSSL* ssl_c = NULL; + WOLFSSL* ssl_s = NULL; + WOLFSSL* ssl = NULL; + struct test_memio_ctx test_ctx; + static const byte prefix[] = { 'w', 'o', 'l', 'f', 'S', 'S', 'L', ':' }; + byte id[sizeof(prefix) + 4]; + byte sessId[ID_LEN]; + word32 i; + /* The cache row is picked from a hash of the ID, so the prefix and any + * one long ID rarely share a row. Every long ID here starts with the + * prefix, and there are enough of them to cover all rows, so the prefix + * lookup lands on a row holding one of them. */ + const word32 fill = 4096; + + /* TLS 1.2 so the client has a complete session to cache as soon as the + * handshake is done, with or without session tickets. */ + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntEQ(ssl_c->session->isSetup, 1); + + XMEMCPY(id, prefix, sizeof(prefix)); + XMEMSET(sessId, 0, sizeof(sessId)); + for (i = 0; i < fill && EXPECT_SUCCESS(); i++) { + ClientSession* entry = NULL; + + c32toa(i, id + sizeof(prefix)); + c32toa(i, sessId); + ExpectIntEQ(wolfSSL_SetServerID(ssl_c, id, (int)sizeof(id), 1), + WOLFSSL_SUCCESS); + if (EXPECT_SUCCESS()) { + XMEMCPY(ssl_c->session->sessionID, sessId, ID_LEN); + XMEMCPY(ssl_c->session->altSessionID, sessId, ID_LEN); + ssl_c->session->sessionIDSz = ID_LEN; + } + ExpectIntEQ(AddSessionToCache(ctx_c, ssl_c->session, sessId, ID_LEN, + NULL, WOLFSSL_CLIENT_END, 0, &entry), 0); + ExpectNotNull(entry); + } + + /* The prefix is a distinct partition key: no cached session for it. */ + ExpectNotNull(ssl = wolfSSL_new(ctx_c)); + ExpectIntEQ(ssl->session->isSetup, 0); + ExpectIntEQ(wolfSSL_SetServerID(ssl, prefix, (int)sizeof(prefix), 0), + WOLFSSL_SUCCESS); + ExpectIntEQ(ssl->session->isSetup, 0); + ExpectIntEQ(ssl->session->idLen, (int)sizeof(prefix)); + wolfSSL_free(ssl); + ssl = NULL; + + /* Control: the ID it was cached under still finds it. */ + ExpectNotNull(ssl = wolfSSL_new(ctx_c)); + ExpectIntEQ(wolfSSL_SetServerID(ssl, id, (int)sizeof(id), 0), + WOLFSSL_SUCCESS); + ExpectIntEQ(ssl->session->isSetup, 1); + wolfSSL_free(ssl); + + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} diff --git a/tests/api/test_session.h b/tests/api/test_session.h index be3deb5f04..8781fd8ac4 100644 --- a/tests/api/test_session.h +++ b/tests/api/test_session.h @@ -37,6 +37,7 @@ int test_wolfSSL_CTX_sess_set_remove_cb(void); int test_wolfSSL_ticket_keys(void); int test_wolfSSL_SESSION_get_ex_new_index(void); int test_wolfSSL_GetSessionAtIndex(void); +int test_wolfSSL_client_cache_id_prefix(void); #define TEST_SESSION_DECLS \ TEST_DECL_GROUP("session", test_wolfSSL_CTX_add_session), \ @@ -51,6 +52,7 @@ int test_wolfSSL_GetSessionAtIndex(void); TEST_DECL_GROUP("session", test_wolfSSL_CTX_sess_set_remove_cb), \ TEST_DECL_GROUP("session", test_wolfSSL_ticket_keys), \ TEST_DECL_GROUP("session", test_wolfSSL_SESSION_get_ex_new_index), \ - TEST_DECL_GROUP("session", test_wolfSSL_GetSessionAtIndex) + TEST_DECL_GROUP("session", test_wolfSSL_GetSessionAtIndex), \ + TEST_DECL_GROUP("session", test_wolfSSL_client_cache_id_prefix) #endif /* WOLFCRYPT_TEST_SESSION_H */