From b16b6ffaae54ec37eefbf210dcccd97a56a82efa Mon Sep 17 00:00:00 2001 From: Rene Meusel Date: Tue, 28 Mar 2023 18:31:24 +0200 Subject: [PATCH] Use new Session_Manager::find_some() efficiently --- src/lib/tls/tls_session_manager.cpp | 57 ++++++++++++++++++-------- src/lib/tls/tls_session_manager.h | 5 +++ src/tests/test_tls_rfc8448.cpp | 4 ++ src/tests/test_tls_session_manager.cpp | 6 +-- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/lib/tls/tls_session_manager.cpp b/src/lib/tls/tls_session_manager.cpp index a1621ab2379..5a2d3620dc9 100644 --- a/src/lib/tls/tls_session_manager.cpp +++ b/src/lib/tls/tls_session_manager.cpp @@ -90,23 +90,11 @@ std::optional Session_Manager::retrieve(const Session_Handle& handle, } } -std::vector Session_Manager::find(const Server_Information& info, - Callbacks& callbacks, - const Policy& policy) - { - auto allow_reusing_tickets = policy.reuse_session_tickets(); - - // Session_Manager::find() must be an atomic getter if ticket reuse is not - // allowed. I.e. each ticket handed to concurrently requesting threads must - // be unique. In that case we must hold a lock while retrieving a ticket. - // Otherwise, no locking is required on this level. - std::optional> lk; - if(!allow_reusing_tickets) - { lk.emplace(mutex()); } - - const auto max_sessions = std::max(policy.maximum_session_tickets_per_client_hello(), size_t(1000)); - auto sessions_and_handles = find_some(info, max_sessions); +std::vector Session_Manager::find_and_filter(const Server_Information& info, + Callbacks& callbacks, + const Policy& policy) + { // A value of '0' means: No policy restrictions. Session ticket lifetimes as // communicated by the server apply regardless. const std::chrono::seconds policy_lifetime = @@ -114,9 +102,23 @@ std::vector Session_Manager::find(const Server_Information& ? policy.session_ticket_lifetime() : std::chrono::seconds::max(); - if(!sessions_and_handles.empty()) + const size_t max_sessions_hint = std::max(policy.maximum_session_tickets_per_client_hello(), size_t(1)); + const auto now = callbacks.tls_current_timestamp(); + + // An arbitrary number of loop iterations to perform before giving up + // to avoid a potential endless loop with a misbehaving session manager. + constexpr unsigned int max_attempts = 10; + std::vector sessions_and_handles; + + // Query the session manager implementation for new sessions until at least + // one session passes the filter or no more sessions are found. + for(unsigned int attempt = 0; attempt < max_attempts && sessions_and_handles.empty(); ++attempt) { - const auto now = callbacks.tls_current_timestamp(); + sessions_and_handles = find_some(info, max_sessions_hint); + + // ... underlying implementation didn't find anything. Early exit. + if(sessions_and_handles.empty()) + { break; } // TODO: C++20, use std::ranges::remove_if() once XCode and Android NDK caught up. sessions_and_handles.erase( @@ -165,6 +167,25 @@ std::vector Session_Manager::find(const Server_Information& }), sessions_and_handles.end()); } + return sessions_and_handles; + } + +std::vector Session_Manager::find(const Server_Information& info, + Callbacks& callbacks, + const Policy& policy) + { + auto allow_reusing_tickets = policy.reuse_session_tickets(); + + // Session_Manager::find() must be an atomic getter if ticket reuse is not + // allowed. I.e. each ticket handed to concurrently requesting threads must + // be unique. In that case we must hold a lock while retrieving a ticket. + // Otherwise, no locking is required on this level. + std::optional> lk; + if(!allow_reusing_tickets) + { lk.emplace(mutex()); } + + auto sessions_and_handles = find_and_filter(info, callbacks, policy); + // std::vector::resize() cannot be used as the vector's members aren't // default constructible. const auto session_limit = policy.maximum_session_tickets_per_client_hello(); diff --git a/src/lib/tls/tls_session_manager.h b/src/lib/tls/tls_session_manager.h index 094954e79cc..45795095246 100644 --- a/src/lib/tls/tls_session_manager.h +++ b/src/lib/tls/tls_session_manager.h @@ -269,6 +269,11 @@ class BOTAN_PUBLIC_API(3, 0) Session_Manager */ recursive_mutex_type& mutex() { return m_mutex; } + private: + std::vector find_and_filter(const Server_Information& info, + Callbacks& callbacks, + const Policy& policy); + protected: std::shared_ptr m_rng; diff --git a/src/tests/test_tls_rfc8448.cpp b/src/tests/test_tls_rfc8448.cpp index f78ee0b3e06..992f7933ee0 100644 --- a/src/tests/test_tls_rfc8448.cpp +++ b/src/tests/test_tls_rfc8448.cpp @@ -1032,6 +1032,7 @@ class Test_TLS_RFC8448_Client : public Test_TLS_RFC8448 "tls_inspect_handshake_msg_client_hello", "tls_modify_extensions_client_hello", "tls_generate_ephemeral_key", + "tls_current_timestamp", }); result.test_eq("TLS client hello", ctx->pull_send_buffer(), vars.get_req_bin("Record_ClientHello_1")); @@ -1251,6 +1252,7 @@ class Test_TLS_RFC8448_Client : public Test_TLS_RFC8448 "tls_inspect_handshake_msg_client_hello", "tls_modify_extensions_client_hello", "tls_generate_ephemeral_key", + "tls_current_timestamp", }); result.test_eq("TLS client hello (1)", ctx->pull_send_buffer(), vars.get_req_bin("Record_ClientHello_1")); @@ -1361,6 +1363,7 @@ class Test_TLS_RFC8448_Client : public Test_TLS_RFC8448 "tls_inspect_handshake_msg_client_hello", "tls_modify_extensions_client_hello", "tls_generate_ephemeral_key", + "tls_current_timestamp", }); result.test_eq("Client Hello", ctx->pull_send_buffer(), vars.get_req_bin("Record_ClientHello_1")); @@ -1458,6 +1461,7 @@ class Test_TLS_RFC8448_Client : public Test_TLS_RFC8448 "tls_inspect_handshake_msg_client_hello", "tls_modify_extensions_client_hello", "tls_generate_ephemeral_key", + "tls_current_timestamp", }); }), diff --git a/src/tests/test_tls_session_manager.cpp b/src/tests/test_tls_session_manager.cpp index 5e2420d40c2..ce8234dae57 100644 --- a/src/tests/test_tls_session_manager.cpp +++ b/src/tests/test_tls_session_manager.cpp @@ -93,7 +93,7 @@ class Session_Manager_Policy : public Botan::TLS::Policy size_t maximum_session_tickets_per_client_hello() const override { return session_limit; } public: - size_t session_limit = 0; /// no limit + size_t session_limit = 1000; // basically 'no limit' bool allow_session_reuse = true; }; @@ -1027,8 +1027,8 @@ std::vector tls_session_manager_expiry() plcy.session_limit = 3; result.test_is_eq("find three", mgr->find(server_info, cbs, plcy).size(), size_t(plcy.session_limit)); - plcy.session_limit = 0; - result.test_is_eq("unlimited", mgr->find(server_info, cbs, plcy).size(), size_t(5)); + plcy.session_limit = 10; + result.test_is_eq("find all five", mgr->find(server_info, cbs, plcy).size(), size_t(5)); }), #if defined(BOTAN_HAS_TLS_13)