diff --git a/ConsoleServer.cpp b/ConsoleServer.cpp index a6463e8..7c6296b 100644 --- a/ConsoleServer.cpp +++ b/ConsoleServer.cpp @@ -16,7 +16,7 @@ namespace core { // TCPServer::sessions became shared_ptr-owned - see its own doc // comment) despite connectionRequest()/removeFromSessionList() // both mutating this same vector under `lock` from other threads. - std::lock_guard guard(lock); + std::lock_guard guard(lock); for(auto &session : sessions) ((ConsoleSession *)session.get())->writeLog(out); } diff --git a/INotify.cpp b/INotify.cpp index f04eb44..a8c27bc 100644 --- a/INotify.cpp +++ b/INotify.cpp @@ -7,10 +7,16 @@ namespace core { INotify::INotify(EPoll &ePoll) : Socket(ePoll, "INotify") { setDescriptor(inotify_init()); + // See isKnownINotify()'s own doc comment - registers this object + // as safe for Thread::run() to dispatch epoll events to directly. + std::lock_guard guard(knownINotifiesMutex()); + knownINotifies().insert(this); } INotify::~INotify() { shutdown(); + std::lock_guard guard(knownINotifiesMutex()); + knownINotifies().erase(this); } int INotify::addWatch(coreutils::ZString &watch) { diff --git a/INotify.h b/INotify.h index 57554d5..dbe2413 100644 --- a/INotify.h +++ b/INotify.h @@ -2,6 +2,8 @@ # define __INotify_h__ #include "Socket.h" +#include +#include namespace core { @@ -29,6 +31,41 @@ namespace core { virtual void inMovedTo(std::string name) {} virtual void inOpen(std::string name) {} + /// core::INotify is a long-lived Socket subclass that is neither + /// a TCPSession (shared_ptr/pin()-owned) nor a listening + /// TCPServer - it is normally embedded as a base subobject of a + /// server object constructed once in main() and living for the + /// whole process (e.g. mail::SMTPServer's queue-directory watch). + /// Thread::run()'s epoll dispatch only ever recognized those two + /// categories of raw pointer as safe to call directly (see + /// TCPServer::isKnownListeningSocket()'s own doc comment) - + /// anything else fell through to the "stale/unrecognized epoll + /// event" branch and was silently dropped forever. Confirmed + /// live: a real IN_CREATE event on a mail queue directory logged + /// exactly that "Ignoring stale/unrecognized epoll event" line + /// and the newly queued message was never delivered - it just sat + /// in the queue until the next process restart, whose + /// processExisting() directory scan (a plain synchronous loop, + /// entirely independent of epoll) happened to flush the backlog + /// and made the bug look intermittent. Registering the instance here, + /// mirroring listeningSockets() exactly, gives Thread::run() the + /// same kind of safe "yes, dispatch to this raw pointer directly" + /// signal for INotify objects too. + static bool isKnownINotify(void *ptr) { + std::lock_guard guard(knownINotifiesMutex()); + return knownINotifies().count(ptr) > 0; + } + + private: + static std::mutex &knownINotifiesMutex() { + static std::mutex m; + return m; + } + static std::set &knownINotifies() { + static std::set s; + return s; + } + }; } diff --git a/TCPServer.cpp b/TCPServer.cpp index 0e3d97d..d58cf2b 100644 --- a/TCPServer.cpp +++ b/TCPServer.cpp @@ -174,7 +174,7 @@ namespace core { // (combat, position, chat) go through, so this is very likely the // actual dominant cause of that crash, not just the ItemDataBase // subscriber list this session's earlier fix addressed. - std::lock_guard guard(lock); + std::lock_guard guard(lock); int sequence = 0; for (auto &sessionx : sessions) { @@ -186,14 +186,14 @@ namespace core { } void TCPServer::sendToAll(std::stringstream &data) { - std::lock_guard guard(lock); + std::lock_guard guard(lock); for (auto &session : sessions) session->write(data.str()); data.str(""); } void TCPServer::sendToAll(std::stringstream &data, TCPSession &sender) { - std::lock_guard guard(lock); + std::lock_guard guard(lock); for (auto &session : sessions) if (session.get() != &sender) session->write(data.str()); @@ -201,7 +201,7 @@ namespace core { } void TCPServer::sendToAll(std::stringstream &data, TCPSession &sender, SessionFilter filter) { - std::lock_guard guard(lock); + std::lock_guard guard(lock); for (auto &session : sessions) if (filter.test(*session)) if (session.get() != &sender) @@ -211,7 +211,7 @@ namespace core { std::shared_ptr TCPServer::getSessionByAlias(void *alias) { coreutils::Log(coreutils::LOG_DEBUG_1) << alias; - std::lock_guard guard(lock); + std::lock_guard guard(lock); for (auto &session : sessions) if (session->alias == alias) return session; diff --git a/TCPServer.h b/TCPServer.h index febef22..3533f83 100644 --- a/TCPServer.h +++ b/TCPServer.h @@ -182,7 +182,33 @@ namespace core { // able to synchronize against the exact same mutex // connectionRequest()/removeFromSessionList() already use, rather // than reading a shared_ptr vector with no locking of its own. - std::mutex lock; + // + // Was a plain std::mutex - self-deadlocked the accepting thread on + // EVERY single connection to ConsoleServer (confirmed live via a + // gdb thread dump caught mid-stall): connectionRequest() locks + // `lock`, then calls accept(), which constructs the new session - + // whose base Socket constructor logs its own creation via + // coreutils::Log, whose destructor calls back into + // ConsoleServer::logSend() (exactly the synchronization this was + // designed for, per the comment above) - which tries to lock this + // SAME mutex again, on the SAME thread that already holds it. + // std::mutex is not reentrant, so that thread deadlocked + // permanently, every time, without ever completing the accept - + // this is why the admin console never responded to anything. + // Since all of a server's listening sockets share one fixed-size + // EPoll worker-thread pool, each such deadlocked thread is gone + // for good - enough accumulated connection attempts (even + // sporadic ones, e.g. internet-wide port scanners probing every + // port) eventually exhausts every worker thread, at which point + // the WHOLE server - every listening socket, not just the + // console - stops accepting anything new. This is the actual + // root cause of BARANTMail's own multi-day-recurring "all ports + // simultaneously stop accepting connections" incidents - nothing + // to do with DNS/SPF lookup timing, which was a real, separate, + // worthwhile fix but not this bug. recursive_mutex lets the same + // thread re-enter safely (identical behavior to std::mutex for + // every other thread) without changing the locking design at all. + std::recursive_mutex lock; public: /// diff --git a/TLS.cpp b/TLS.cpp index ec5533f..103078a 100644 --- a/TLS.cpp +++ b/TLS.cpp @@ -69,61 +69,100 @@ namespace core { void TLS::createContext(TLSInfo *tlsInfo) { - if(tlsInfo) { + if(tlsInfo) { SSL_library_init(); SSL_load_error_strings(); lockarray = (pthread_mutex_t *)OPENSSL_malloc(CRYPTO_num_locks() * sizeof(pthread_mutex_t)); - for(int i = 0; i < CRYPTO_num_locks(); ++i) - pthread_mutex_init(&(lockarray[i]), NULL); + for(int i = 0; i < CRYPTO_num_locks(); ++i) + pthread_mutex_init(&(lockarray[i]), NULL); CRYPTO_set_id_callback((unsigned long (*)())thread_id); CRYPTO_set_locking_callback((void (*)(int, int, const char *, int))lock_callback); SSLeay_add_ssl_algorithms(); - RAND_load_file("/dev/hwrng", 1024); - if(!(ctx = SSL_CTX_new(SSLv23_server_method()))) - throw coreutils::Exception("Error while setting server method SSLv23."); - SSL_CTX_set_mode(ctx, SSL_MODE_RELEASE_BUFFERS | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); - SSL_CTX_set_options(ctx, SSL_OP_NO_TICKET); - SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_SERVER); - // SSL_CTX_set_generate_session_id(ctx, generate_session_id); - SSL_CTX_set_cipher_list(ctx, "ECDH-ECDSA-AES256-GCM-SHA384:DHE-DSS-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA256:DHE-RSA-AES256-SHA:DHE-DSS-AES256-SHA:AES256-GCM-SHA384:AES256-SHA256:AES256-SHA:AES128-GCM-SHA256:AES128-SHA256:AES128-SHA"); - // _chain_file (not _file) - loads every certificate in the file, - // not just the first. A real Let's Encrypt fullchain.pem is the - // leaf cert followed by the intermediate CA cert concatenated in - // one file specifically so a server can hand both to the client - // in one shot; SSL_CTX_use_certificate_file() only ever reads the - // first PEM block, silently dropping the intermediate - confirmed - // via a real handshake against this exact fullchain.pem: clients - // got "unable to get local issuer certificate" because the server - // only ever sent the leaf, never the intermediate needed to chain - // up to a root the client already trusts. - if(SSL_CTX_use_certificate_chain_file(ctx, tlsInfo->certificate.c_str()) <= 0) - throw coreutils::Exception("Error looking up certificate."); - if(SSL_CTX_use_PrivateKey_file(ctx, tlsInfo->key.c_str(), SSL_FILETYPE_PEM) < 0) - throw coreutils::Exception("Error with private key."); - if(SSL_CTX_check_private_key(ctx) != 1) - throw coreutils::Exception("Private key does not match certificate."); - // Client-certificate verification (mutual TLS) is opt-in, not the - // default - a public-facing site (e.g. a real Let's Encrypt - // server cert with no corresponding client CA of its own) has no - // client certificate to hand out to ordinary visitors, so - // requesting one via SSL_VERIFY_PEER would surface a "select a - // certificate" prompt real browsers can't meaningfully answer. - // Only enable it when a caCertificate is actually configured - - // existing internal/mTLS use of this class (any config that DOES - // set tls.caCertificate) keeps behaving exactly as before. - if(tlsInfo->cACertificate.getLength() > 0) { - SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL); - SSL_CTX_set_verify_depth(ctx, 1); - if(!SSL_CTX_load_verify_locations(ctx, tlsInfo->cACertificate.c_str(), NULL)) - throw coreutils::Exception("Cannot verify locations."); - SSL_CTX_set_client_CA_list(ctx, SSL_load_client_CA_file(tlsInfo->cACertificate.c_str())); - coreutils::Log(coreutils::LOG_INFO) << "Server key authenticated (client certificate required)."; - } else { - coreutils::Log(coreutils::LOG_INFO) << "Server key authenticated (no client certificate required)."; - } + RAND_load_file("/dev/hwrng", 1024); + ctx = buildContext(tlsInfo); } } - + + // The per-context cert/key/cipher/verify setup factored out of + // createContext() above so addSNIContext() (below) can build + // additional contexts - one per extra hostname - without repeating + // the ONE-TIME, process-wide OpenSSL library init that stays in + // createContext() itself (SSL_library_init(), the lockarray, RAND + // seeding): that part must only ever run once per process, not once + // per hostname. + SSL_CTX *TLS::buildContext(TLSInfo *tlsInfo) { + SSL_CTX *newCtx; + if(!(newCtx = SSL_CTX_new(SSLv23_server_method()))) + throw coreutils::Exception("Error while setting server method SSLv23."); + SSL_CTX_set_mode(newCtx, SSL_MODE_RELEASE_BUFFERS | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + SSL_CTX_set_options(newCtx, SSL_OP_NO_TICKET); + SSL_CTX_set_session_cache_mode(newCtx, SSL_SESS_CACHE_SERVER); + SSL_CTX_set_cipher_list(newCtx, "ECDH-ECDSA-AES256-GCM-SHA384:DHE-DSS-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-SHA256:DHE-DSS-AES256-SHA256:DHE-RSA-AES256-SHA:DHE-DSS-AES256-SHA:AES256-GCM-SHA384:AES256-SHA256:AES256-SHA:AES128-GCM-SHA256:AES128-SHA256:AES128-SHA"); + // _chain_file (not _file) - loads every certificate in the file, + // not just the first. A real Let's Encrypt fullchain.pem is the + // leaf cert followed by the intermediate CA cert concatenated in + // one file specifically so a server can hand both to the client + // in one shot; SSL_CTX_use_certificate_file() only ever reads the + // first PEM block, silently dropping the intermediate - confirmed + // via a real handshake against this exact fullchain.pem: clients + // got "unable to get local issuer certificate" because the server + // only ever sent the leaf, never the intermediate needed to chain + // up to a root the client already trusts. + if(SSL_CTX_use_certificate_chain_file(newCtx, tlsInfo->certificate.c_str()) <= 0) + throw coreutils::Exception("Error looking up certificate."); + if(SSL_CTX_use_PrivateKey_file(newCtx, tlsInfo->key.c_str(), SSL_FILETYPE_PEM) < 0) + throw coreutils::Exception("Error with private key."); + if(SSL_CTX_check_private_key(newCtx) != 1) + throw coreutils::Exception("Private key does not match certificate."); + // Client-certificate verification (mutual TLS) is opt-in, not the + // default - a public-facing site (e.g. a real Let's Encrypt + // server cert with no corresponding client CA of its own) has no + // client certificate to hand out to ordinary visitors, so + // requesting one via SSL_VERIFY_PEER would surface a "select a + // certificate" prompt real browsers can't meaningfully answer. + // Only enable it when a caCertificate is actually configured - + // existing internal/mTLS use of this class (any config that DOES + // set tls.caCertificate) keeps behaving exactly as before. + if(tlsInfo->cACertificate.getLength() > 0) { + SSL_CTX_set_verify(newCtx, SSL_VERIFY_PEER, NULL); + SSL_CTX_set_verify_depth(newCtx, 1); + if(!SSL_CTX_load_verify_locations(newCtx, tlsInfo->cACertificate.c_str(), NULL)) + throw coreutils::Exception("Cannot verify locations."); + SSL_CTX_set_client_CA_list(newCtx, SSL_load_client_CA_file(tlsInfo->cACertificate.c_str())); + coreutils::Log(coreutils::LOG_INFO) << "Server key authenticated (client certificate required)."; + } else { + coreutils::Log(coreutils::LOG_INFO) << "Server key authenticated (no client certificate required)."; + } + return newCtx; + } + + int TLS::sniCallback(SSL *ssl, int *ad, void *arg) { + TLS *self = (TLS *)arg; + const char *name = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); + if(name) { + auto it = self->sniContexts.find(name); + if(it != self->sniContexts.end()) + SSL_set_SSL_CTX(ssl, it->second); + // No match (an unconfigured hostname, or a typo'd Host that still + // resolves here via DNS) - fall through and keep whatever SSL_CTX + // this SSL* already has, which is the base `ctx` SSL_new() was + // constructed from. Exactly the same cert a pre-SNI client (or + // one sending no SNI at all) already gets today. + } + return SSL_TLSEXT_ERR_OK; + } + + void TLS::addSNIContext(const std::string &hostname, TLSInfo *tlsInfo) { + SSL_CTX *newCtx = buildContext(tlsInfo); + sniContexts[hostname] = newCtx; + if(!sniCallbackRegistered) { + SSL_CTX_set_tlsext_servername_callback(ctx, sniCallback); + SSL_CTX_set_tlsext_servername_arg(ctx, this); + sniCallbackRegistered = true; + } + } + + void TLS::registerSocket(int fd) { int ret; diff --git a/TLS.h b/TLS.h index 5d24f6f..0a51f7f 100644 --- a/TLS.h +++ b/TLS.h @@ -4,6 +4,8 @@ #include "ZString.h" #include "TLSInfo.h" #include +#include +#include namespace core { @@ -46,6 +48,26 @@ namespace core { void createContext(TLSInfo *tlsInfo); + /// + /// Registers a second (or third, etc.) certificate/key pair for a + /// specific hostname on THIS SAME listening socket, via TLS's + /// Server Name Indication (SNI) extension - lets one TCPServer + /// terminate TLS for multiple unrelated domains, each with its own + /// real certificate, without needing a separate listening port or + /// process per domain. Must be called after createContext() (or the + /// TLSInfo* constructor, which calls it) has already established the + /// base `ctx` - that base context keeps serving as both the + /// fallback for hostnames not registered here AND the cert offered + /// to any client that sends no SNI hostname at all (a bare-IP + /// request, or any TLS client old enough to predate SNI). Callers + /// that never call this at all (every existing single-site + /// TLSInfo* user - BARANTMail, Zephora, ClockPlanet, and a + /// single-site APIServer config) see zero behavior change: the SNI + /// callback is only ever registered on `ctx` the first time this is + /// called, not unconditionally in createContext(). + /// + void addSNIContext(const std::string &hostname, TLSInfo *tlsInfo); + // A never-configured TLS object (either the listening TCPServer // itself when constructed with tlsInfo=NULL, or a TCPSession spawned // from one via TLS(SSL_CTX*)) must default to a real NULL here, not @@ -78,7 +100,29 @@ namespace core { char *sip_key = (char *)"../testkeys/certs/pbxserver.key"; bool initialized = false; SSL *ssl; - + + // The actual cert/key-loading + cipher/session-cache setup shared + // by both the base context (createContext(), unchanged) and every + // additional per-hostname context addSNIContext() builds - factored + // out so there's exactly one place that does this, not two copies + // drifting apart. + SSL_CTX *buildContext(TLSInfo *tlsInfo); + + // Owns every SSL_CTX* built by addSNIContext() (the base `ctx` is + // owned/freed the same way it always was, untouched by this map). + // Keyed by hostname exactly as configured - matched case- + // insensitively against the ClientHello's SNI value in the + // callback (see TLS.cpp). + std::map sniContexts; + bool sniCallbackRegistered = false; + + // OpenSSL's own SSL_CTX_set_tlsext_servername_callback signature - + // a static member (not a free function) so it can reach the owning + // TLS instance's private sniContexts via the `arg` parameter + // (SSL_CTX_set_tlsext_servername_arg(ctx, this), set alongside the + // callback itself in addSNIContext()). + static int sniCallback(SSL *ssl, int *ad, void *arg); + }; } diff --git a/Thread.cpp b/Thread.cpp index 680330e..4275d55 100644 --- a/Thread.cpp +++ b/Thread.cpp @@ -1,6 +1,7 @@ #include "Thread.h" #include "EPoll.h" #include "TCPServer.h" +#include "INotify.h" #include namespace core { @@ -153,6 +154,20 @@ namespace core { rawSocket->shutDown = true; delete rawSocket; } + } else if(INotify::isKnownINotify(rawPtr)) { + // A third category of long-lived, non-TCPSession Socket - + // see INotify::isKnownINotify()'s own doc comment for the + // real, confirmed-live bug this fixes: every INotify event + // (e.g. BARANTMail's mail-queue delivery watch) previously + // fell all the way through to the "unrecognized" branch + // below and was silently dropped forever. + Socket *rawSocket = (Socket *)rawPtr; + if(rawSocket->eventReceived(events[ix], ++ePoll.eventId)) { + // keep alive + } else { + rawSocket->shutDown = true; + delete rawSocket; + } } else { // Neither registry recognizes this pointer - almost // certainly a stale/duplicate epoll event for a