diff --git a/TCPServer.cpp b/TCPServer.cpp index 0e3d97d..6e6145f 100644 --- a/TCPServer.cpp +++ b/TCPServer.cpp @@ -3,6 +3,8 @@ #include "Exception.h" #include "Log.h" #include "TCPSession.h" +#include +#include namespace core { @@ -30,7 +32,14 @@ namespace core { if (bind(getDescriptor(), address.getPointer(), address.addressLength) < 0) throw coreutils::Exception("Error on bind to socket: " + std::to_string(errno)); - if (listen(getDescriptor(), 20) < 0) + // Was 20 - small enough that a burst of connection attempts + // arriving faster than this process calls accept() (a port scan, + // or just several real clients reconnecting around the same + // moment) can fill the kernel's own backlog before the + // application layer gets a chance to drain it, silently dropping + // the overflow SYNs rather than queuing them. 128 gives real + // bursts meaningfully more headroom; still bounded, not unlimited. + if (listen(getDescriptor(), 128) < 0) throw coreutils::Exception("Error on listen to socket"); // See isKnownListeningSocket()'s own doc comment - registers this @@ -49,6 +58,10 @@ namespace core { void TCPServer::connectionRequest() { coreutils::Log(coreutils::LOG_DEBUG_2) << "Connection request is being received on socket " << getDescriptor() << "."; + // Outside the lock.lock()/unlock() below, not nested inside it - + // sweepStaleSessions() takes the same `lock` itself (see its own + // comment), and this mutex isn't recursive. + sweepStaleSessions(); lock.lock(); TCPSession *session = accept(); // The shared_ptr constructed here IS the session's real, strong @@ -80,6 +93,33 @@ namespace core { lock.unlock(); } + void TCPServer::sweepStaleSessions() { + // 30 seconds is generous for completing a TCP handshake plus a + // TLS handshake plus sending a first real message over any real + // network - deliberately not aimed at catching anything borderline, + // only connections that never progressed at all. + static const std::chrono::seconds GRACE_PERIOD(30); + auto now = std::chrono::steady_clock::now(); + + // Collect under `lock` (sessions is mutated elsewhere under the + // same mutex - see its own doc comment), then drop the lock before + // actually removing anything. removeFromSessionList() takes `lock` + // itself for each call; calling it while still holding the guard + // below would deadlock on this non-recursive mutex. + std::vector stale; + { + std::lock_guard guard(lock); + for (auto &s : sessions) + if (!s->hasReceivedData && (now - s->acceptedAt) > GRACE_PERIOD) + stale.push_back(s.get()); + } + + for (auto *s : stale) { + coreutils::Log(coreutils::LOG_DEBUG_2) << "Reaping stale, never-active session " << s->getDescriptor() << "."; + removeFromSessionList(s); + } + } + void TCPServer::receiveData(coreutils::ZString &buffer) { // See this override's own doc comment in TCPServer.h - the // listening socket always uses the plain read()/ENOTCONN detection diff --git a/TCPServer.h b/TCPServer.h index febef22..8927f98 100644 --- a/TCPServer.h +++ b/TCPServer.h @@ -149,6 +149,31 @@ namespace core { void connectionRequest() override; + /// + /// Drops any session that completed a TCP accept() but has never + /// had a single byte of real data pass through onDataReceived() - + /// covers both a TLS handshake that never finishes and a plain + /// connection that never sends anything - once it's been open + /// longer than a short grace period. Nothing else in this class + /// ever revisits an already-accepted session once it stops + /// generating epoll events (Thread::run()'s dispatch loop only + /// acts on a session when a real event for it arrives), so a + /// connection that completes the handshake and then goes + /// permanently silent - exactly what a port scan/prober looks + /// like from here, as opposed to a normal scan that just connects + /// and immediately closes again, which the existing read-hangup + /// path already cleans up fine - would otherwise sit in `sessions` + /// forever, one TCPSession + one fd each. Called from + /// connectionRequest() (see its own call site) so the sweep keeps + /// pace with however fast new connections are actually arriving, + /// without needing a dedicated timer/thread of its own; a + /// legitimate, already-talking client is never at risk, since + /// hasReceivedData latches true permanently on its first real byte + /// and is never consulted again after that. + /// + + void sweepStaleSessions(); + /// /// The listening socket itself is never a TLS peer - accepting a /// connection on it surfaces as a plain read() failing with ENOTCONN diff --git a/TCPSocket.cpp b/TCPSocket.cpp index 6ab126a..9c0f83b 100644 --- a/TCPSocket.cpp +++ b/TCPSocket.cpp @@ -99,6 +99,7 @@ namespace core { void TCPSocket::onDataReceived(coreutils::ZString &data) { if (data.getLength() > 0) { + hasReceivedData = true; lineBuffer = (char *)realloc(lineBuffer, lineBufferSize + data.getLength()); memcpy(lineBuffer + lineBufferSize, data.getData(), data.getLength()); lineBufferSize += data.getLength(); diff --git a/TCPSocket.h b/TCPSocket.h index 049bcf3..5705f22 100644 --- a/TCPSocket.h +++ b/TCPSocket.h @@ -6,6 +6,7 @@ #include "TLS.h" #include "TLSInfo.h" #include "IPAddress.h" +#include namespace core { @@ -150,6 +151,41 @@ namespace core { void performTLSHandshake(); + /// + /// Set once, at construction, and never touched again - how long + /// this specific socket has been open since accept()/connect(), + /// regardless of whether anything has actually happened on it + /// since. TCPServer::sweepStaleSessions() reads this (alongside + /// hasReceivedData below) to find connections that completed a + /// TCP handshake but then never progressed - a scanner/prober + /// probing an open port, or a TLS handshake that never finished - + /// which otherwise sit in TCPServer::sessions forever: nothing in + /// the existing design ever revisits a socket that stops + /// generating epoll events (see sweepStaleSessions()'s own doc + /// comment for the full reasoning). + /// + + std::chrono::steady_clock::time_point acceptedAt = std::chrono::steady_clock::now(); + + /// + /// Flips to true the first time onDataReceived() sees a non-empty + /// read - true for both a plaintext socket's raw bytes and a TLS + /// session's already-decrypted ones, since both paths funnel + /// through the same onDataReceived() override below. A session + /// that reaches sweepStaleSessions()'s grace period with this + /// still false has never had a real exchange of any kind, TLS + /// handshake included (a stuck/incomplete SSL_accept() never + /// surfaces anything to onDataReceived) - the specific, narrow + /// condition that sweep treats as abandoned. Deliberately NOT + /// reset afterward and never consulted again once true, so a + /// legitimate client that sends something once and then goes + /// idle for a long time (an ordinary connected-but-idle game + /// session) is never at risk from this - only a connection that + /// NEVER said anything at all within the grace window is. + /// + + bool hasReceivedData = false; + protected: bool term = false; TLSInfo *tlsInfo;