fix: properly reconnect on handshake error (#6950)

* fix: clear read buffer when closing connection

* fix: clear write buffer when closing connection

* fix: disable encryption when reconnecting

* refactor: dedupe code

* fix: maybe reconnect using MSE handshake if it was an utp failure

* chore: misc housekeeping

* chore: removed `tr_peerIo::utp_supported_`

* refactor: more logs in `tr_handshake::on_error()`

libtransmission/handshake.cc

@@ -627,7 +627,7 @@ void tr_handshake::on_error(tr_peerIo* io, tr_error const& error, void* vhandsha
 {
     auto* handshake = static_cast<tr_handshake*>(vhandshake);
 
-    auto const retry = [&]()
+    auto const retry_plain = [&]()
     {
         handshake->send_handshake(io);
         handshake->set_state(State::AwaitingHandshake);
@@ -638,6 +638,8 @@ void tr_handshake::on_error(tr_peerIo* io, tr_error const& error, void* vhandsha
         handshake->done(false);
     };
 
+    handshake->maybe_recycle_dh();
+
     if (io->is_utp() && !io->is_incoming() && handshake->is_state(State::AwaitingYb))
     {
         // the peer probably doesn't speak µTP.
@@ -653,7 +655,15 @@ void tr_handshake::on_error(tr_peerIo* io, tr_error const& error, void* vhandsha
 
         if (handshake->mediator_->allows_tcp() && io->reconnect())
         {
-            retry();
+            tr_logAddTraceHand(handshake, "uTP connection failed, trying TCP...");
+            if (handshake->encryption_mode_ != TR_CLEAR_PREFERRED)
+            {
+                handshake->send_ya(io);
+            }
+            else
+            {
+                retry_plain();
+            }
             return;
         }
 
@@ -667,8 +677,8 @@ void tr_handshake::on_error(tr_peerIo* io, tr_error const& error, void* vhandsha
     if (handshake->is_state(State::AwaitingYb) && handshake->encryption_mode_ != TR_ENCRYPTION_REQUIRED &&
         handshake->mediator_->allows_tcp() && io->reconnect())
     {
-        tr_logAddTraceHand(handshake, "handshake failed, trying plaintext...");
+        tr_logAddTraceHand(handshake, "MSE handshake failed, trying plaintext...");
-        retry();
+        retry_plain();
         return;
     }
 

libtransmission/peer-io.cc

@@ -229,6 +229,11 @@ void tr_peerIo::close()
     socket_.close();
     event_write_.reset();
     event_read_.reset();
+    inbuf_.clear();
+    outbuf_.clear();
+    outbuf_info_.clear();
+    encrypt_disable();
+    decrypt_disable();
 }
 
 void tr_peerIo::clear()
@@ -241,28 +246,20 @@ void tr_peerIo::clear()
 
 bool tr_peerIo::reconnect()
 {
-    TR_ASSERT(!this->is_incoming());
+    TR_ASSERT(!is_incoming());
-    TR_ASSERT(this->session_->allowsTCP());
+    TR_ASSERT(session_->allowsTCP());
 
-    short int const pending_events = this->pending_events_;
+    auto const pending_events = pending_events_;
     event_disable(EV_READ | EV_WRITE);
 
     close();
 
-    if (tr_peer_socket::limit_reached(session_))
-    {
-        return false;
-    }
-
     auto sock = tr_netOpenPeerSocket(session_, socket_address(), client_is_seed());
     if (!sock.is_tcp())
     {
         return false;
     }
-    socket_ = std::move(sock);
+    set_socket(std::move(sock));
-
-    this->event_read_.reset(event_new(session_->event_base(), socket_.handle.tcp, EV_READ, event_read_cb, this));
-    this->event_write_.reset(event_new(session_->event_base(), socket_.handle.tcp, EV_WRITE, event_write_cb, this));
 
     event_enable(pending_events);
 
@@ -609,23 +606,23 @@ void tr_peerIo::read_bytes(void* bytes, size_t n_bytes)
 {
     auto walk = reinterpret_cast<std::byte*>(bytes);
     n_bytes = std::min(n_bytes, std::size(inbuf_));
-    if (decrypt_remain_len_)
+    if (n_decrypt_remain_)
     {
-        if (*decrypt_remain_len_ <= n_bytes)
+        if (auto& n_remain = *n_decrypt_remain_; n_remain <= n_bytes)
         {
-            filter_.decrypt(std::data(inbuf_), *decrypt_remain_len_, walk);
+            filter_.decrypt(std::data(inbuf_), n_remain, walk);
-            inbuf_.drain(*decrypt_remain_len_);
+            inbuf_.drain(n_remain);
             if (walk != nullptr)
             {
-                walk += *decrypt_remain_len_;
+                walk += n_remain;
             }
-            n_bytes -= *decrypt_remain_len_;
+            n_bytes -= n_remain;
             filter_.decrypt_disable();
-            decrypt_remain_len_.reset();
+            n_decrypt_remain_.reset();
         }
         else
         {
-            *decrypt_remain_len_ -= n_bytes;
+            n_remain -= n_bytes;
         }
     }
     filter_.decrypt(std::data(inbuf_), n_bytes, walk);
@@ -655,7 +652,6 @@ void tr_peerIo::on_utp_state_change(int state)
     if (state == UTP_STATE_CONNECT)
     {
         tr_logAddTraceIo(this, "utp_on_state_change -- changed to connected");
-        utp_supported_ = true;
     }
     else if (state == UTP_STATE_WRITABLE)
     {

libtransmission/peer-io.h

@@ -256,11 +256,6 @@ public:
 
     ///
 
-    [[nodiscard]] constexpr auto supports_utp() const noexcept
-    {
-        return utp_supported_;
-    }
-
     [[nodiscard]] constexpr auto is_incoming() const noexcept
     {
         return is_incoming_;
@@ -290,14 +285,14 @@ public:
 
     void decrypt_init(bool is_incoming, DH const& dh, tr_sha1_digest_t const& info_hash)
     {
-        decrypt_remain_len_.reset();
+        n_decrypt_remain_.reset();
         filter_.decrypt_init(is_incoming, dh, info_hash);
     }
 
     TR_CONSTEXPR20 void decrypt_disable(size_t decrypt_len = 0U) noexcept
     {
         // optionally decrypt decrypt_len more bytes before disabling decryption
-        decrypt_remain_len_ = decrypt_len;
+        n_decrypt_remain_ = decrypt_len;
     }
 
     void encrypt_init(bool is_incoming, DH const& dh, tr_sha1_digest_t const& info_hash)
@@ -371,7 +366,7 @@ private:
         bool is_seed);
 
     Filter filter_;
-    std::optional<size_t> decrypt_remain_len_;
+    std::optional<size_t> n_decrypt_remain_;
 
     std::deque<std::pair<size_t /*n_bytes*/, bool /*is_piece_data*/>> outbuf_info_;
 
@@ -401,7 +396,6 @@ private:
     bool const client_is_seed_;
     bool const is_incoming_;
 
-    bool utp_supported_ = false;
     bool dht_supported_ = false;
     bool extended_protocol_supported_ = false;
     bool fast_extension_supported_ = false;