diff --git a/src/internal.c b/src/internal.c index e32fe421eb..76c5f43123 100644 --- a/src/internal.c +++ b/src/internal.c @@ -19568,6 +19568,21 @@ int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, word32 size, if (ConstantCompare(input + *inOutIdx, (const byte*)&ssl->hsHashes->verifyHashes, (int)size) != 0) { WOLFSSL_MSG("Verify finished error on hashes"); +#ifdef HAVE_SESSION_TICKET + /* Drop the unverified ticket (SetTicket() made the session + * unique, so this cannot clear one shared with the app). */ + if (ssl->options.side == WOLFSSL_CLIENT_END && + ssl->msgsReceived.got_session_ticket) { + ForceZero(ssl->session->ticket, ssl->session->ticketLen); + if (ssl->session->ticketLenAlloc > 0) { + XFREE(ssl->session->ticket, ssl->heap, + DYNAMIC_TYPE_SESSION_TICK); + ssl->session->ticket = ssl->session->staticTicket; + ssl->session->ticketLenAlloc = 0; + } + ssl->session->ticketLen = 0; + } +#endif WOLFSSL_ERROR_VERBOSE(VERIFY_FINISHED_ERROR); return VERIFY_FINISHED_ERROR; } @@ -19606,6 +19621,21 @@ int DoFinished(WOLFSSL* ssl, const byte* input, word32* inOutIdx, word32 size, ssl->cbmode = WOLFSSL_CB_MODE_WRITE; ssl->options.clientState = CLIENT_FINISHED_COMPLETE; #endif + /* The server is authenticated only now, so this is the first point at + * which the session may be cached (a full handshake, or a resumption + * that renewed the ticket, from RFC 5246 Section 7.2.2). */ + if (sniff == NO_SNIFF && (!ssl->options.resuming +#ifdef HAVE_SESSION_TICKET + /* A renewal only: an empty ticket clears ticketLen. */ + || (ssl->msgsReceived.got_session_ticket && + ssl->session->ticketLen > 0) +#endif + )) { + SetupSession(ssl); +#ifndef NO_SESSION_CACHE + AddSession(ssl); +#endif + } if (!ssl->options.resuming) { #ifdef OPENSSL_EXTRA if (ssl->CBIS != NULL) { @@ -27302,11 +27332,13 @@ int SendFinished(WOLFSSL* ssl) return BUILD_MSG_ERROR; if (!ssl->options.resuming) { - SetupSession(ssl); + /* Client side is cached by DoFinished(), which is the first point at + * which the server Finished has been verified. */ + if (ssl->options.side == WOLFSSL_SERVER_END) { + SetupSession(ssl); #ifndef NO_SESSION_CACHE - AddSession(ssl); + AddSession(ssl); #endif - if (ssl->options.side == WOLFSSL_SERVER_END) { #ifdef OPENSSL_EXTRA ssl->options.serverState = SERVER_FINISHED_COMPLETE; ssl->cbmode = WOLFSSL_CB_MODE_WRITE; @@ -38288,6 +38320,14 @@ int SetTicket(WOLFSSL* ssl, const byte* ticket, word32 length) else #endif { + /* A server issuing a ticket sends no session ID, so keep caching + * under a generated one rather than under the ticket bytes. */ + if (!ssl->session->haveAltSessionID && + ssl->arrays->sessionIDSz == 0 && + wc_RNG_GenerateBlock(ssl->rng, ssl->session->altSessionID, + ID_LEN) == 0) { + ssl->session->haveAltSessionID = 1; + } XMEMSET(ssl->arrays->sessionID, 0, ID_LEN); XMEMCPY(ssl->arrays->sessionID, ssl->session->ticket + length - sessIdLen, @@ -38317,7 +38357,8 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } /* A renewed ticket while resuming confirms resumption; check before the - * SetupSession() below refreshes the cached suite/EMS and masks a downgrade. + * SetupSession() in DoFinished refreshes the cached suite/EMS and masks a + * downgrade. * (The ChangeCipherSpec check covers the no-renewal case.) */ if (ssl->options.resuming) { ret = CheckResumptionConsistency(ssl); @@ -38344,11 +38385,10 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, return ret; *inOutIdx += length; if (length > 0) { + /* The session is not cached here, the + * server Finished is still unverified. + * DoFinished() caches once it verifies. */ ssl->timeout = lifetime; - SetupSession(ssl); -#ifndef NO_SESSION_CACHE - AddSession(ssl); -#endif } ssl->expect_session_ticket = 0; diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index c9c0c8a44b..f47e0c5396 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -2231,6 +2231,196 @@ int test_tls12_resume_ticket_decline_fallback(void) return EXPECT_RESULT(); } +/* A TLS 1.2 client that rejects the server Finished keeps no ticket, and + * leaves none in the cache for a later connection. */ +int test_tls12_ticket_dropped_on_bad_finished(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SESSION_TICKET) && \ + !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && !defined(NO_SESSION_CACHE) && \ + !defined(NO_CLIENT_CACHE) && !defined(NO_SESSION_CACHE_REF) && \ + !defined(NO_WOLFSSL_CLIENT) && \ + !defined(NO_WOLFSSL_SERVER) + const byte serverID[] = "tls12-ticket-cache-test"; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL, *ssl_c2 = NULL; + struct test_memio_ctx test_ctx; + const char* msg = NULL; + int msgSz = 0; + + 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(wolfSSL_UseSessionTicket(ssl_c), WOLFSSL_SUCCESS); + /* Register the session under a server ID so a later connection can look it + * up in the client cache. */ + ExpectIntEQ(wolfSSL_SetServerID(ssl_c, serverID, (int)sizeof(serverID), 1), + WOLFSSL_SUCCESS); + + /* ClientHello */ + ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + /* ServerHello .. ServerHelloDone */ + ExpectIntNE(wolfSSL_accept(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_get_error(ssl_s, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + /* ClientKeyExchange, ChangeCipherSpec, Finished */ + ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + /* NewSessionTicket, ChangeCipherSpec, Finished */ + ExpectIntEQ(wolfSSL_accept(ssl_s), WOLFSSL_SUCCESS); + + /* Flip the last byte of the plaintext NewSessionTicket record. */ + ExpectIntEQ(test_memio_get_message(&test_ctx, 1, &msg, &msgSz, 0), 0); + ExpectIntGT(msgSz, RECORD_HEADER_SZ); + if (EXPECT_SUCCESS()) { + int off = (int)(msg - (const char*)test_ctx.c_buff); + word16 recSz = 0; + + ExpectIntEQ((byte)msg[0], handshake); + ExpectIntEQ((byte)msg[RECORD_HEADER_SZ], session_ticket); + ato16((const byte*)msg + 3, &recSz); + ExpectIntGT(recSz, 0); + ExpectIntGE(msgSz, RECORD_HEADER_SZ + (int)recSz); + if (EXPECT_SUCCESS()) + test_ctx.c_buff[off + RECORD_HEADER_SZ + recSz - 1] ^= 0xFF; + } + + /* Client takes the ticket, then rejects the Finished and sends a fatal + * alert. */ + ExpectIntNE(wolfSSL_connect(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_get_error(ssl_c, WOLFSSL_FATAL_ERROR), + WC_NO_ERR_TRACE(VERIFY_FINISHED_ERROR)); + /* Not a vacuous pass: the ticket really was processed. */ + ExpectIntEQ(ssl_c->msgsReceived.got_session_ticket, 1); + /* The unverified ticket is dropped from the session too. */ + ExpectIntEQ(ssl_c->session->ticketLen, 0); + + /* The failed handshake must leave no ticket for the next connection. */ + ExpectNotNull(ssl_c2 = wolfSSL_new(ctx_c)); + ExpectIntEQ(wolfSSL_SetServerID(ssl_c2, serverID, (int)sizeof(serverID), 0), + WOLFSSL_SUCCESS); + ExpectNotNull(ssl_c2->session); + ExpectIntEQ(ssl_c2->session->ticketLen, 0); + + wolfSSL_free(ssl_c2); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + +/* A TLS 1.2 client that completes the handshake leaves its ticket session in + * the cache for a later connection. */ +int test_tls12_ticket_cached_after_finished(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SESSION_TICKET) && \ + !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && !defined(NO_SESSION_CACHE) && \ + !defined(NO_CLIENT_CACHE) && !defined(NO_SESSION_CACHE_REF) && \ + !defined(NO_WOLFSSL_CLIENT) && \ + !defined(NO_WOLFSSL_SERVER) + const byte serverID[] = "tls12-ticket-cache-ok"; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL, *ssl_c2 = NULL; + struct test_memio_ctx test_ctx; + + 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(wolfSSL_UseSessionTicket(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_SetServerID(ssl_c, serverID, (int)sizeof(serverID), 1), + WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntGT(ssl_c->session->ticketLen, 0); + /* Cached under a generated ID, not under the ticket bytes. */ + ExpectIntEQ(ssl_c->session->haveAltSessionID, 1); + + ExpectNotNull(ssl_c2 = wolfSSL_new(ctx_c)); + ExpectIntEQ(wolfSSL_SetServerID(ssl_c2, serverID, (int)sizeof(serverID), 0), + WOLFSSL_SUCCESS); + ExpectNotNull(ssl_c2->session); + ExpectIntGT(ssl_c2->session->ticketLen, 0); + + wolfSSL_free(ssl_c2); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + +/* A resumed TLS 1.2 handshake in which the server sends an empty + * NewSessionTicket leaves the client's cached ticket in place. */ +int test_tls12_empty_ticket_keeps_cached(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SESSION_TICKET) && \ + !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && !defined(NO_SESSION_CACHE) && \ + !defined(NO_CLIENT_CACHE) && !defined(NO_SESSION_CACHE_REF) && \ + !defined(NO_WOLFSSL_CLIENT) && \ + !defined(NO_WOLFSSL_SERVER) && !defined(WOLFSSL_TICKET_DECRYPT_NO_CREATE) + const byte serverID[] = "tls12-empty-ticket-test"; + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + WOLFSSL *ssl_c2 = NULL, *ssl_s2 = NULL, *ssl_c3 = NULL; + struct test_memio_ctx test_ctx; + struct test_memio_ctx test_ctx2; + + /* Full handshake, so the client caches a session holding a ticket. */ + 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(wolfSSL_UseSessionTicket(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_SetServerID(ssl_c, serverID, (int)sizeof(serverID), 1), + WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + ExpectIntGT(ssl_c->session->ticketLen, 0); + + /* A hint of half the ticket key lifetime or more makes the server send an + * empty ticket instead of renewing. */ + ExpectIntEQ(wolfSSL_CTX_set_TicketHint(ctx_s, WOLFSSL_TICKET_KEY_LIFETIME), + WOLFSSL_SUCCESS); + + /* Resume against the same server CTX, so the ticket is accepted. */ + XMEMSET(&test_ctx2, 0, sizeof(test_ctx2)); + ExpectIntEQ(test_memio_setup(&test_ctx2, &ctx_c, &ctx_s, &ssl_c2, &ssl_s2, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + ExpectIntEQ(wolfSSL_UseSessionTicket(ssl_c2), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_SetServerID(ssl_c2, serverID, (int)sizeof(serverID), 0), + WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c2, ssl_s2, 10, NULL), 0); + ExpectIntEQ(wolfSSL_session_reused(ssl_c2), 1); + /* The empty ticket cleared this connection's copy. */ + ExpectIntEQ(ssl_c2->msgsReceived.got_session_ticket, 1); + ExpectIntEQ(ssl_c2->session->ticketLen, 0); + + /* The cached ticket must not have been overwritten. */ + ExpectNotNull(ssl_c3 = wolfSSL_new(ctx_c)); + ExpectIntEQ(wolfSSL_SetServerID(ssl_c3, serverID, (int)sizeof(serverID), 0), + WOLFSSL_SUCCESS); + ExpectNotNull(ssl_c3->session); + ExpectIntGT(ssl_c3->session->ticketLen, 0); + + wolfSSL_free(ssl_c3); + wolfSSL_free(ssl_c2); + wolfSSL_free(ssl_s2); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + /* wolfSSL_set_session() must reject a TLS 1.2 session when minDowngrade is * set to TLS 1.3. */ int test_tls_set_session_min_downgrade(void) diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index 6c3fda8996..e82dd4c323 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -49,6 +49,9 @@ int test_tls_version_error_alert_mapping(void); int test_tls12_etm_failed_resumption(void); int test_tls12_resume_ticket_wrong_suite(void); int test_tls12_resume_ticket_decline_fallback(void); +int test_tls12_ticket_dropped_on_bad_finished(void); +int test_tls12_ticket_cached_after_finished(void); +int test_tls12_empty_ticket_keeps_cached(void); int test_tls_set_session_min_downgrade(void); int test_tls12_session_id_resumption_sni_mismatch(void); int test_tls13_session_resumption_sni_mismatch(void); @@ -96,6 +99,9 @@ int test_wolfSSL_get_shared_ciphers(void); TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption), \ TEST_DECL_GROUP("tls", test_tls12_resume_ticket_wrong_suite), \ TEST_DECL_GROUP("tls", test_tls12_resume_ticket_decline_fallback), \ + TEST_DECL_GROUP("tls", test_tls12_ticket_dropped_on_bad_finished), \ + TEST_DECL_GROUP("tls", test_tls12_ticket_cached_after_finished), \ + TEST_DECL_GROUP("tls", test_tls12_empty_ticket_keeps_cached), \ TEST_DECL_GROUP("tls", test_tls_set_session_min_downgrade), \ TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \ TEST_DECL_GROUP("tls", test_tls13_session_resumption_sni_mismatch), \