From d54d2892a9bbf46adc3430871fb74502917f9ec3 Mon Sep 17 00:00:00 2001 From: AnotherTest Date: Sat, 30 May 2020 19:53:07 +0430 Subject: [PATCH] LibTLS: Avoid busy-wait between ClientHello and ServerHello This commit also adds a timeout timer to cancel the connection if the server does not respond to the hello request in 10 seconds. --- Libraries/LibTLS/ClientHandshake.cpp | 10 ++++++++ Libraries/LibTLS/Handshake.cpp | 7 ++++++ Libraries/LibTLS/Record.cpp | 4 +--- Libraries/LibTLS/Socket.cpp | 36 ++++++++++++++++++++-------- Libraries/LibTLS/TLSv12.h | 10 ++++++-- 5 files changed, 52 insertions(+), 15 deletions(-) diff --git a/Libraries/LibTLS/ClientHandshake.cpp b/Libraries/LibTLS/ClientHandshake.cpp index dcf20543d96..28d27215dc9 100644 --- a/Libraries/LibTLS/ClientHandshake.cpp +++ b/Libraries/LibTLS/ClientHandshake.cpp @@ -238,6 +238,16 @@ ssize_t TLSv12::handle_finished(const ByteBuffer& buffer, WritePacketStage& writ #endif m_context.connection_status = ConnectionStatus::Established; + if (m_handshake_timeout_timer) { + // Disable the handshake timeout timer as handshake has been established. + m_handshake_timeout_timer->stop(); + m_handshake_timeout_timer->remove_from_parent(); + m_handshake_timeout_timer = nullptr; + } + + if (on_tls_ready_to_write) + on_tls_ready_to_write(*this); + return handle_message(buffer); } diff --git a/Libraries/LibTLS/Handshake.cpp b/Libraries/LibTLS/Handshake.cpp index e209f11d8a9..99165fec5b1 100644 --- a/Libraries/LibTLS/Handshake.cpp +++ b/Libraries/LibTLS/Handshake.cpp @@ -167,4 +167,11 @@ ByteBuffer TLSv12::build_finished() return packet; } +void TLSv12::alert(AlertLevel level, AlertDescription code) +{ + auto the_alert = build_alert(level == AlertLevel::Critical, (u8)code); + write_packet(the_alert); + flush(); +} + } diff --git a/Libraries/LibTLS/Record.cpp b/Libraries/LibTLS/Record.cpp index ee624264232..19fb65f4406 100644 --- a/Libraries/LibTLS/Record.cpp +++ b/Libraries/LibTLS/Record.cpp @@ -311,9 +311,7 @@ ssize_t TLSv12::handle_message(const ByteBuffer& buffer) if (code == 0) { // close notify res += 2; - auto closure_alert = build_alert(true, (u8)AlertDescription::CloseNotify); - write_packet(closure_alert); - flush(); + alert(AlertLevel::Critical, AlertDescription::CloseNotify); m_context.connection_finished = true; } m_context.error_code = (Error)code; diff --git a/Libraries/LibTLS/Socket.cpp b/Libraries/LibTLS/Socket.cpp index 62d95d43d03..b5f4fcf3ca1 100644 --- a/Libraries/LibTLS/Socket.cpp +++ b/Libraries/LibTLS/Socket.cpp @@ -24,6 +24,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include #include #include #include @@ -110,14 +111,32 @@ bool TLSv12::common_connect(const struct sockaddr* saddr, socklen_t length) } } - auto packet = build_hello(); - write_packet(packet); - Core::Socket::on_connected = [this] { Core::Socket::on_ready_to_read = [this] { read_from_socket(); }; - write_into_socket(); + + auto packet = build_hello(); + write_packet(packet); + + deferred_invoke([&](auto&) { + m_handshake_timeout_timer = Core::Timer::create_single_shot( + m_max_wait_time_for_handshake_in_seconds * 1000, [&] { + // The server did not respond fast enough, + // time the connection out. + alert(AlertLevel::Critical, AlertDescription::UserCanceled); + m_context.connection_finished = true; + m_context.tls_buffer.clear(); + m_context.error_code = Error::TimedOut; + m_context.critical_error = (u8)Error::TimedOut; + check_connection_state(false); // Notify the client. + }, + this); + write_into_socket(); + m_handshake_timeout_timer->start(); + }); + m_has_scheduled_write_flush = true; + if (on_tls_connected) on_tls_connected(); }; @@ -138,7 +157,7 @@ void TLSv12::read_from_socket() if (!check_connection_state(true)) return; - flush(); + consume(Core::Socket::read(4096)); } @@ -152,13 +171,10 @@ void TLSv12::write_into_socket() return; flush(); - if (!is_established()) { - deferred_invoke([this](auto&) { write_into_socket(); }); - m_has_scheduled_write_flush = true; + if (!is_established()) return; - } - if (is_established() && !m_context.application_buffer.size()) // hey client, you still have stuff to read... + if (!m_context.application_buffer.size()) // hey client, you still have stuff to read... if (on_tls_ready_to_write) on_tls_ready_to_write(*this); } diff --git a/Libraries/LibTLS/TLSv12.h b/Libraries/LibTLS/TLSv12.h index 6a104092eb0..ab9ca33ff5c 100644 --- a/Libraries/LibTLS/TLSv12.h +++ b/Libraries/LibTLS/TLSv12.h @@ -142,6 +142,7 @@ enum class Error : i8 { FeatureNotSupported = -17, DecryptionFailed = -20, NeedMoreData = -21, + TimedOut = -22, }; enum class AlertLevel : u8 { @@ -293,6 +294,8 @@ struct Context { StringView negotiated_alpn; size_t send_retries { 0 }; + + time_t handshake_initiation_timestamp { 0 }; }; class TLSv12 : public Core::Socket { @@ -335,7 +338,7 @@ public: ByteBuffer read(size_t max_size); bool write(const ByteBuffer& buffer); - void alert(bool critical, u8 code); + void alert(AlertLevel, AlertDescription); bool can_read_line() const { return m_context.application_buffer.size() && memchr(m_context.application_buffer.data(), '\n', m_context.application_buffer.size()); } bool can_read() const { return m_context.application_buffer.size() > 0; } @@ -467,7 +470,10 @@ private: OwnPtr m_aes_local; OwnPtr m_aes_remote; - bool m_has_scheduled_write_flush = false; + bool m_has_scheduled_write_flush { false }; + i32 m_max_wait_time_for_handshake_in_seconds { 10 }; + + RefPtr m_handshake_timeout_timer; }; namespace Constants {