Merge tls into master: bring in subscription ownership, keep all TLS/stability work
tls and master diverged from a July 2024 fork point and never reconciled - tls carries ~19 months of real socket/TLS stability fixes (uninitialized tlsInfo, TCPServer::connectionRequest() unconditionally initializing TLS, the WANT_READ write busy-spin, the console authentication fix from this session), while master independently picked up a Subscription/SubscriptionManager rework (MString-based ownership tracking: owner/ownership/selection/time fields and a working subscriptionOwner() method) that tls never had. Every file both branches touched was resolved by keeping tls's side entirely (Command.h, Socket.cpp, TCPServer.h, TCPSession.cpp/h, TCPSocket.h were pure reformatting on master's side, or an alternate/superseded fix for a bug tls already fixed a different way - e.g. master's TCPSession uuid/alias handling vs tls's existing uuid_unparse-based fix), with two deliberate exceptions: - TCPServer.cpp's constructor was missing the commands.add(subscriptions, "publish"/"subscribe"/"catalog"/...) wiring master had - without it, SubscriptionManager existed but was completely unreachable through the normal command dispatch. Restored on top of tls's TLS-aware constructor. - SubscriptionManager.cpp's "invite" command handler had a real incompatibility: master's version assigned into an `invitee` member typed as coreutils::MString via a stray `(coreutils::MString *)` cast that doesn't match the member's actual (non-pointer) type, and called session.server.getSessionByAlias() against a TCPServer reference - tls made TCPSession::server a pointer (part of removing the separate TCPSession2 class), so that call site no longer compiles as written. Took tls's cleaner local-variable version instead (functionally identical - looks up a session by alias and forwards an invite message) and removed the now-unused `invitee` member from SubscriptionManager.h. One additional compile fix needed after resolving conflicts: Subscription::subscriptionOwners is an MString (master's ownership design), but TCPSession::alias is void* (tls's identity mechanism, unchanged and depended on elsewhere) - MString has an implicit constructor from void* already used successfully elsewhere in this same file (the "publish" handler's Subscription constructor call), but operator== doesn't accept the same implicit conversion, so removeSessionSubscriptions()'s ownership check needed an explicit coreutils::MString(session.alias) construction to compile. Also drops output/main (a stray checked-in compiled binary, already removed on tls) and the doxygen docs/html output (already removed on tls, was carried on master since the fork). Verified: full ServerCore rebuild (./compile) succeeds with zero errors or warnings beyond the one compile fix documented above.
This commit is contained in:
Brad Arant committed
commit
3609ed603e
2883 files changed
+5296
-84468
No files matched your search
@@ -0,0 +1,185 @@
|
||||
#include "TCPSocket.h"
|
||||
#include "EPoll.h"
|
||||
#include "Log.h"
|
||||
#include "Exception.h"
|
||||
#include "errno.h"
|
||||
#include <algorithm>
|
||||
#include <cstring>
|
||||
|
||||
namespace core {
|
||||
|
||||
TCPSocket::TCPSocket(EPoll &ePoll) : Socket(ePoll) {}
|
||||
|
||||
TCPSocket::TCPSocket(EPoll &ePoll, std::string text) : Socket(ePoll, text) {}
|
||||
|
||||
TCPSocket::TCPSocket(EPoll &ePoll, TLSInfo *tlsInfo, std::string text) : Socket(ePoll, text), TLS(tlsInfo), tlsInfo(tlsInfo) {}
|
||||
|
||||
TCPSocket::TCPSocket(EPoll &ePoll, SSL_CTX *ctx, std::string text) : Socket(ePoll, text), TLS(ctx) {}
|
||||
|
||||
TCPSocket::~TCPSocket() {}
|
||||
|
||||
void TCPSocket::connect(IPAddress &address) {
|
||||
setDescriptor(socket(AF_INET, SOCK_STREAM, 0));
|
||||
if(::connect(getDescriptor(), (struct sockaddr *)&address.addr, address.addressLength) == -1)
|
||||
throw coreutils::Exception("Error on connect to TCP socket." + errno);
|
||||
coreutils::Log(coreutils::LOG_DEBUG_3) << "Connected to IP..." << address.getClientAddressAndPort();
|
||||
}
|
||||
|
||||
void TCPSocket::output(std::stringstream &out) {
|
||||
out << "|" << ipAddress.getClientAddressAndPort();
|
||||
}
|
||||
|
||||
void TCPSocket::receiveData(coreutils::ZString &buffer) {
|
||||
// Dispatch on `ctx`, not `tlsInfo` - the SSL_CTX*-taking constructor
|
||||
// (used for every accepted TCPSession) never sets `tlsInfo` at all,
|
||||
// leaving it as uninitialized garbage that intermittently (whenever
|
||||
// that garbage happened to be non-zero) routed a plain, non-TLS
|
||||
// session's data through SSL_read()/SSL_write() against an
|
||||
// uninitialized SSL*, segfaulting inside libssl. `ctx` is the
|
||||
// reliable signal: real and non-NULL for an actual TLS server/
|
||||
// session (set in TLS::createContext(), inherited by sessions via
|
||||
// TLS(SSL_CTX*)), safely NULL by default otherwise (see TLS.h).
|
||||
if(ctx) {
|
||||
int len;
|
||||
{
|
||||
// Held only around the raw SSL_read call - released before
|
||||
// onDataReceived() so the request-handling chain it triggers
|
||||
// (which reaches Socket::write()/outlock) never runs while
|
||||
// ioLock is held. See ioLock's own doc comment in TCPSocket.h.
|
||||
std::lock_guard<std::mutex> ioGuard(ioLock);
|
||||
len = TLS::receiveData(buffer);
|
||||
}
|
||||
if(len >= 0) {
|
||||
coreutils::ZString zbuffer(buffer.getData(), len);
|
||||
onDataReceived(zbuffer);
|
||||
}
|
||||
} else {
|
||||
Socket::receiveData(buffer);
|
||||
}
|
||||
}
|
||||
|
||||
int TCPSocket::sendData(const char *data, int length) {
|
||||
if(ctx) {
|
||||
std::lock_guard<std::mutex> ioGuard(ioLock);
|
||||
return TLS::sendData(data, length);
|
||||
} else
|
||||
return Socket::sendData(data, length);
|
||||
}
|
||||
|
||||
void TCPSocket::performTLSHandshake() {
|
||||
// registerSocket() makes this fd eligible for epoll dispatch to ANY
|
||||
// worker thread immediately. ioLock MUST be acquired BEFORE that
|
||||
// call, not after - a first attempt here took the lock AFTER
|
||||
// registerSocket(), which still left a real window: another thread
|
||||
// could win the race for ioLock (via a read/write event on this
|
||||
// same brand-new connection) and have SSL_read/SSL_write silently
|
||||
// complete the handshake on OpenSSL's own initiative, and only
|
||||
// THEN would this thread acquire ioLock and run its own, now
|
||||
// redundant, acceptSocket() (SSL_accept) against an already-
|
||||
// negotiated SSL object - confirmed via gdb `thread apply all bt`
|
||||
// during a live crash: one thread mid-SSL_read, a second thread
|
||||
// simultaneously inside performTLSHandshake, both on the identical
|
||||
// object address. Acquiring ioLock first closes that window
|
||||
// entirely: no other thread can even begin touching this
|
||||
// connection's SSL object until this thread's own handshake
|
||||
// attempt has released the lock. acceptSocket() only attempts
|
||||
// SSL_accept() once and may leave the handshake incomplete
|
||||
// (WANT_READ/WANT_WRITE) - that's fine, OpenSSL transparently
|
||||
// finishes a pending handshake on the next SSL_read/SSL_write,
|
||||
// which ioLock already serializes against everything else.
|
||||
std::lock_guard<std::mutex> ioGuard(ioLock);
|
||||
registerSocket(getDescriptor());
|
||||
acceptSocket();
|
||||
}
|
||||
|
||||
void TCPSocket::onDataReceived(coreutils::ZString &data) {
|
||||
|
||||
if (data.getLength() > 0) {
|
||||
lineBuffer = (char *)realloc(lineBuffer, lineBufferSize + data.getLength());
|
||||
memcpy(lineBuffer + lineBufferSize, data.getData(), data.getLength());
|
||||
lineBufferSize += data.getLength();
|
||||
while (lineBufferSize > 0) {
|
||||
if (blockSize == 0) {
|
||||
// strcspn() treats its argument as a NUL-terminated C
|
||||
// string and keeps scanning past the end looking for one -
|
||||
// lineBuffer is a raw byte buffer sized via realloc() to
|
||||
// exactly lineBufferSize bytes, with no NUL terminator
|
||||
// appended anywhere. Whenever the buffered data doesn't yet
|
||||
// contain a CRLF (an ordinary, frequent case - a line split
|
||||
// across TCP reads, or mid-way through a long one, e.g. a
|
||||
// large email body), strcspn() read straight past
|
||||
// lineBufferSize into whatever heap memory happened to
|
||||
// follow, hunting for a byte that happened to match '\r',
|
||||
// '\n', or an accidental '\0' - producing a lineLength
|
||||
// larger than the buffer actually holds. Confirmed live via
|
||||
// gdb: SIGSEGV inside a LATER, unrelated realloc() call on
|
||||
// this same lineBuffer, well downstream of where the real
|
||||
// corruption happened (lineBufferSize -= lineLength
|
||||
// underflows when lineLength > lineBufferSize, and that
|
||||
// negative value gets handed to realloc() as a size_t -
|
||||
// realloc(ptr, SIZE_MAX-ish) - during a real inbound SMTP
|
||||
// message with a long quoted-printable line). memchr() is
|
||||
// the bounded equivalent - it never looks past the length
|
||||
// given, so "no CRLF yet" correctly comes back as "not
|
||||
// found" instead of reading off the end of the allocation.
|
||||
char *cr = (char *)memchr(lineBuffer, '\r', lineBufferSize);
|
||||
char *nl = (char *)memchr(lineBuffer, '\n', lineBufferSize);
|
||||
char *found = (cr && nl) ? std::min(cr, nl) : (cr ? cr : nl);
|
||||
lineLength = found ? (int)(found - lineBuffer) : lineBufferSize;
|
||||
if (lineLength == lineBufferSize)
|
||||
break;
|
||||
coreutils::ZString zLine(lineBuffer, lineLength);
|
||||
onLineReceived(zLine);
|
||||
// lineLength < lineBufferSize is guaranteed by the break
|
||||
// above, so this first index is always in bounds; bump
|
||||
// past a lone '\r' only if there's still at least one more
|
||||
// buffered byte to safely check for a following '\n' -
|
||||
// otherwise (a bare trailing '\r' with nothing buffered
|
||||
// after it yet) leave it for the next read to resolve,
|
||||
// rather than reading lineBuffer[lineBufferSize], one past
|
||||
// the allocation.
|
||||
if (lineBuffer[lineLength] == '\r')
|
||||
++lineLength;
|
||||
if (lineLength < lineBufferSize && lineBuffer[lineLength] == '\n')
|
||||
++lineLength;
|
||||
lineBufferSize -= lineLength;
|
||||
if (lineBufferSize > 0)
|
||||
memmove(lineBuffer, lineBuffer + lineLength, lineBufferSize);
|
||||
lineBuffer = (char *)realloc(lineBuffer, lineBufferSize);
|
||||
}
|
||||
else if (lineBufferSize >= blockSize) {
|
||||
coreutils::ZString zBlock(lineBuffer, blockSize);
|
||||
onBlockReceived(zBlock);
|
||||
lineBufferSize -= blockSize;
|
||||
if (lineBufferSize > 0)
|
||||
memmove(lineBuffer, lineBuffer + blockSize, lineBufferSize);
|
||||
lineBuffer = (char *)realloc(lineBuffer, lineBufferSize);
|
||||
}
|
||||
else
|
||||
// Not enough buffered data yet for a full block - stop and
|
||||
// wait for the next read, exactly like the line-mode
|
||||
// branch's own "no complete line yet" break above.
|
||||
// Without this, a block spanning more than one TCP read
|
||||
// (any IMAP APPEND literal bigger than a single read()'s
|
||||
// worth of socket buffer) sends this EPoll worker thread
|
||||
// into an unconditional busy-spin - lineBufferSize never
|
||||
// changes, so the while(lineBufferSize > 0) loop above
|
||||
// never yields back to let more data arrive.
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void TCPSocket::onLineReceived(coreutils::ZString &line) {
|
||||
if (term)
|
||||
shutdown("termination requested");
|
||||
}
|
||||
|
||||
void TCPSocket::onBlockReceived(coreutils::ZString &block) {}
|
||||
|
||||
void TCPSocket::setBlockSize(int blockSize) {
|
||||
this->blockSize = blockSize;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user