From 1f0485039a276ad1c2fa3d53d284e3a87940ec77 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Sat, 6 Jun 2015 14:31:05 +0200 Subject: Convert all code to use StreamSocket::SendQueue Let OnStreamSocketWrite see the entire sendq instead of one element at a time --- src/modules/extra/m_ssl_gnutls.cpp | 10 ++++++---- src/modules/extra/m_ssl_openssl.cpp | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) (limited to 'src/modules/extra') diff --git a/src/modules/extra/m_ssl_gnutls.cpp b/src/modules/extra/m_ssl_gnutls.cpp index d33403aba..e142ead11 100644 --- a/src/modules/extra/m_ssl_gnutls.cpp +++ b/src/modules/extra/m_ssl_gnutls.cpp @@ -968,7 +968,7 @@ info_done_dealloc: } } - int OnStreamSocketWrite(StreamSocket* user, std::string& sendq) CXX11_OVERRIDE + int OnStreamSocketWrite(StreamSocket* user) CXX11_OVERRIDE { // Finish handshake if needed int prepret = PrepareIO(user); @@ -976,19 +976,21 @@ info_done_dealloc: return prepret; // Session is ready for transferring application data + StreamSocket::SendQueue& sendq = user->GetSendQ(); int ret = 0; { - ret = gnutls_record_send(this->sess, sendq.data(), sendq.length()); + const StreamSocket::SendQueue::Element& buffer = sendq.front(); + ret = gnutls_record_send(this->sess, buffer.data(), buffer.length()); - if (ret == (int)sendq.length()) + if (ret == (int)buffer.length()) { SocketEngine::ChangeEventMask(user, FD_WANT_NO_WRITE); return 1; } else if (ret > 0) { - sendq.erase(0, ret); + sendq.erase_front(ret); SocketEngine::ChangeEventMask(user, FD_WANT_SINGLE_WRITE); return 0; } diff --git a/src/modules/extra/m_ssl_openssl.cpp b/src/modules/extra/m_ssl_openssl.cpp index c8a035fac..c2a71eeca 100644 --- a/src/modules/extra/m_ssl_openssl.cpp +++ b/src/modules/extra/m_ssl_openssl.cpp @@ -601,7 +601,7 @@ class OpenSSLIOHook : public SSLIOHook } } - int OnStreamSocketWrite(StreamSocket* user, std::string& buffer) CXX11_OVERRIDE + int OnStreamSocketWrite(StreamSocket* user) CXX11_OVERRIDE { // Finish handshake if needed int prepret = PrepareIO(user); @@ -611,8 +611,10 @@ class OpenSSLIOHook : public SSLIOHook data_to_write = true; // Session is ready for transferring application data + StreamSocket::SendQueue& sendq = user->GetSendQ(); { ERR_clear_error(); + const StreamSocket::SendQueue::Element& buffer = sendq.front(); int ret = SSL_write(sess, buffer.data(), buffer.size()); #ifdef INSPIRCD_OPENSSL_ENABLE_RENEGO_DETECTION @@ -628,7 +630,7 @@ class OpenSSLIOHook : public SSLIOHook } else if (ret > 0) { - buffer.erase(0, ret); + sendq.erase_front(ret); SocketEngine::ChangeEventMask(user, FD_WANT_SINGLE_WRITE); return 0; } -- cgit v1.2.3 From e05c25865090790b4a60c376fcf630bb9466af72 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Sat, 6 Jun 2015 14:34:28 +0200 Subject: Add max outgoing record size option to sslprofile config --- src/modules/extra/m_ssl_gnutls.cpp | 12 ++++++++++-- src/modules/extra/m_ssl_openssl.cpp | 6 ++++++ 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'src/modules/extra') diff --git a/src/modules/extra/m_ssl_gnutls.cpp b/src/modules/extra/m_ssl_gnutls.cpp index e142ead11..f5e52b4e1 100644 --- a/src/modules/extra/m_ssl_gnutls.cpp +++ b/src/modules/extra/m_ssl_gnutls.cpp @@ -531,14 +531,20 @@ namespace GnuTLS */ Priority priority; + /** Rough max size of records to send + */ + const unsigned int outrecsize; + Profile(const std::string& profilename, const std::string& certstr, const std::string& keystr, std::auto_ptr& DH, unsigned int mindh, const std::string& hashstr, - const std::string& priostr, std::auto_ptr& CA, std::auto_ptr& CRL) + const std::string& priostr, std::auto_ptr& CA, std::auto_ptr& CRL, + unsigned int recsize) : name(profilename) , x509cred(certstr, keystr) , min_dh_bits(mindh) , hash(hashstr) , priority(priostr) + , outrecsize(recsize) { x509cred.SetDH(DH); x509cred.SetCA(CA, CRL); @@ -587,7 +593,8 @@ namespace GnuTLS crl.reset(new X509CRL(ReadFile(filename))); } - return new Profile(profilename, certstr, keystr, dh, mindh, hashstr, priostr, ca, crl); + unsigned int outrecsize = tag->getInt("outrecsize", 2048, 512, 16384); + return new Profile(profilename, certstr, keystr, dh, mindh, hashstr, priostr, ca, crl, outrecsize); } /** Set up the given session with the settings in this profile @@ -605,6 +612,7 @@ namespace GnuTLS const std::string& GetName() const { return name; } X509Credentials& GetX509Credentials() { return x509cred; } gnutls_digest_algorithm_t GetHash() const { return hash.get(); } + unsigned int GetOutgoingRecordSize() const { return outrecsize; } }; } diff --git a/src/modules/extra/m_ssl_openssl.cpp b/src/modules/extra/m_ssl_openssl.cpp index c2a71eeca..f4a661154 100644 --- a/src/modules/extra/m_ssl_openssl.cpp +++ b/src/modules/extra/m_ssl_openssl.cpp @@ -238,6 +238,10 @@ namespace OpenSSL */ const bool allowrenego; + /** Rough max size of records to send + */ + const unsigned int outrecsize; + static int error_callback(const char* str, size_t len, void* u) { Profile* profile = reinterpret_cast(u); @@ -278,6 +282,7 @@ namespace OpenSSL , ctx(SSL_CTX_new(SSLv23_server_method())) , clictx(SSL_CTX_new(SSLv23_client_method())) , allowrenego(tag->getBool("renegotiation", true)) + , outrecsize(tag->getInt("outrecsize", 2048, 512, 16384)) { if ((!ctx.SetDH(dh)) || (!clictx.SetDH(dh))) throw Exception("Couldn't set DH parameters"); @@ -337,6 +342,7 @@ namespace OpenSSL SSL* CreateClientSession() { return clictx.CreateClientSession(); } const EVP_MD* GetDigest() { return digest; } bool AllowRenegotiation() const { return allowrenego; } + unsigned int GetOutgoingRecordSize() const { return outrecsize; } }; } -- cgit v1.2.3 From d0556a2a598e207ab468b7ea4543e427205ef903 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Sat, 6 Jun 2015 14:42:59 +0200 Subject: Call OnStreamSocketWrite() once per write event Do sendq flattening in SSL modules, move code for it into class SSLIOHook from core --- include/modules/ssl.h | 25 +++++++++++++++++++++++++ src/inspsocket.cpp | 21 --------------------- src/modules/extra/m_ssl_gnutls.cpp | 9 +++++++-- src/modules/extra/m_ssl_openssl.cpp | 11 ++++++++--- 4 files changed, 40 insertions(+), 26 deletions(-) (limited to 'src/modules/extra') diff --git a/include/modules/ssl.h b/include/modules/ssl.h index 0f58e0b7b..67bfc7b2e 100644 --- a/include/modules/ssl.h +++ b/include/modules/ssl.h @@ -138,6 +138,31 @@ class SSLIOHook : public IOHook */ reference certificate; + /** Reduce elements in a send queue by appending later elements to the first element until there are no more + * elements to append or a desired length is reached + * @param sendq SendQ to work on + * @param targetsize Target size of the front element + */ + static void FlattenSendQueue(StreamSocket::SendQueue& sendq, size_t targetsize) + { + if ((sendq.size() <= 1) || (sendq.front().length() >= targetsize)) + return; + + // Avoid multiple repeated SSL encryption invocations + // This adds a single copy of the queue, but avoids + // much more overhead in terms of system calls invoked + // by an IOHook. + std::string tmp; + tmp.reserve(std::min(targetsize, sendq.bytes())+1); + do + { + tmp.append(sendq.front()); + sendq.pop_front(); + } + while (!sendq.empty() && tmp.length() < targetsize); + sendq.push_front(tmp); + } + public: SSLIOHook(IOHookProvider* hookprov) : IOHook(hookprov) diff --git a/src/inspsocket.cpp b/src/inspsocket.cpp index b8f8949dd..436cbb6bb 100644 --- a/src/inspsocket.cpp +++ b/src/inspsocket.cpp @@ -202,33 +202,12 @@ void StreamSocket::DoWrite() if (GetIOHook()) { { - while (error.empty() && !sendq.empty()) { - if (sendq.size() > 1 && sendq.front().length() < 1024) - { - // Avoid multiple repeated SSL encryption invocations - // This adds a single copy of the queue, but avoids - // much more overhead in terms of system calls invoked - // by the IOHook. - // - // The length limit of 1024 is to prevent merging strings - // more than once when writes begin to block. - std::string tmp; - tmp.reserve(1280); - while (!sendq.empty() && tmp.length() < 1024) - { - tmp.append(sendq.front()); - sendq.pop_front(); - } - sendq.push_front(tmp); - } - { int rv = GetIOHook()->OnStreamSocketWrite(this); if (rv > 0) { // consumed the entire string, and is ready for more - sendq.pop_front(); } else if (rv == 0) { diff --git a/src/modules/extra/m_ssl_gnutls.cpp b/src/modules/extra/m_ssl_gnutls.cpp index f5e52b4e1..935b0ee77 100644 --- a/src/modules/extra/m_ssl_gnutls.cpp +++ b/src/modules/extra/m_ssl_gnutls.cpp @@ -987,14 +987,16 @@ info_done_dealloc: StreamSocket::SendQueue& sendq = user->GetSendQ(); int ret = 0; + while (!sendq.empty()) { + FlattenSendQueue(sendq, profile->GetOutgoingRecordSize()); const StreamSocket::SendQueue::Element& buffer = sendq.front(); ret = gnutls_record_send(this->sess, buffer.data(), buffer.length()); if (ret == (int)buffer.length()) { - SocketEngine::ChangeEventMask(user, FD_WANT_NO_WRITE); - return 1; + // Wrote entire record, continue sending + sendq.pop_front(); } else if (ret > 0) { @@ -1014,6 +1016,9 @@ info_done_dealloc: return -1; } } + + SocketEngine::ChangeEventMask(user, FD_WANT_NO_WRITE); + return 1; } void TellCiphersAndFingerprint(LocalUser* user) diff --git a/src/modules/extra/m_ssl_openssl.cpp b/src/modules/extra/m_ssl_openssl.cpp index f4a661154..b037347f1 100644 --- a/src/modules/extra/m_ssl_openssl.cpp +++ b/src/modules/extra/m_ssl_openssl.cpp @@ -618,8 +618,10 @@ class OpenSSLIOHook : public SSLIOHook // Session is ready for transferring application data StreamSocket::SendQueue& sendq = user->GetSendQ(); + while (!sendq.empty()) { ERR_clear_error(); + FlattenSendQueue(sendq, profile->GetOutgoingRecordSize()); const StreamSocket::SendQueue::Element& buffer = sendq.front(); int ret = SSL_write(sess, buffer.data(), buffer.size()); @@ -630,9 +632,8 @@ class OpenSSLIOHook : public SSLIOHook if (ret == (int)buffer.length()) { - data_to_write = false; - SocketEngine::ChangeEventMask(user, FD_WANT_POLL_READ | FD_WANT_NO_WRITE); - return 1; + // Wrote entire record, continue sending + sendq.pop_front(); } else if (ret > 0) { @@ -666,6 +667,10 @@ class OpenSSLIOHook : public SSLIOHook } } } + + data_to_write = false; + SocketEngine::ChangeEventMask(user, FD_WANT_POLL_READ | FD_WANT_NO_WRITE); + return 1; } void TellCiphersAndFingerprint(LocalUser* user) -- cgit v1.2.3 From 042cd5e8e6edcf7a678c71e01919d9de319debc9 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Sat, 6 Jun 2015 15:13:31 +0200 Subject: m_ssl_gnutls Implement corking on GnuTLS 3.1.9 and later to avoid data copies done by sendq flattening --- src/modules/extra/m_ssl_gnutls.cpp | 107 ++++++++++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 18 deletions(-) (limited to 'src/modules/extra') diff --git a/src/modules/extra/m_ssl_gnutls.cpp b/src/modules/extra/m_ssl_gnutls.cpp index 935b0ee77..df676a252 100644 --- a/src/modules/extra/m_ssl_gnutls.cpp +++ b/src/modules/extra/m_ssl_gnutls.cpp @@ -92,6 +92,10 @@ typedef unsigned int inspircd_gnutls_session_init_flags_t; typedef gnutls_connection_end_t inspircd_gnutls_session_init_flags_t; #endif +#if INSPIRCD_GNUTLS_HAS_VERSION(3, 1, 9) +#define INSPIRCD_GNUTLS_HAS_CORK +#endif + class RandGen : public HandlerBase2 { public: @@ -593,7 +597,12 @@ namespace GnuTLS crl.reset(new X509CRL(ReadFile(filename))); } +#ifdef INSPIRCD_GNUTLS_HAS_CORK + // If cork support is available outrecsize represents the (rough) max amount of data we give GnuTLS while corked + unsigned int outrecsize = tag->getInt("outrecsize", 2048, 512); +#else unsigned int outrecsize = tag->getInt("outrecsize", 2048, 512, 16384); +#endif return new Profile(profilename, certstr, keystr, dh, mindh, hashstr, priostr, ca, crl, outrecsize); } @@ -622,6 +631,9 @@ class GnuTLSIOHook : public SSLIOHook gnutls_session_t sess; issl_status status; reference profile; +#ifdef INSPIRCD_GNUTLS_HAS_CORK + size_t gbuffersize; +#endif void CloseSession() { @@ -801,6 +813,43 @@ info_done_dealloc: return -1; } +#ifdef INSPIRCD_GNUTLS_HAS_CORK + int FlushBuffer(StreamSocket* sock) + { + // If GnuTLS has some data buffered, write it + if (gbuffersize) + return HandleWriteRet(sock, gnutls_record_uncork(this->sess, 0)); + return 1; + } +#endif + + int HandleWriteRet(StreamSocket* sock, int ret) + { + if (ret > 0) + { +#ifdef INSPIRCD_GNUTLS_HAS_CORK + gbuffersize -= ret; + if (gbuffersize) + { + SocketEngine::ChangeEventMask(sock, FD_WANT_SINGLE_WRITE); + return 0; + } +#endif + return ret; + } + else if (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED || ret == 0) + { + SocketEngine::ChangeEventMask(sock, FD_WANT_SINGLE_WRITE); + return 0; + } + else // (ret < 0) + { + sock->SetError(gnutls_strerror(ret)); + CloseSession(); + return -1; + } + } + static const char* UnknownIfNULL(const char* str) { return str ? str : "UNKNOWN"; @@ -921,6 +970,9 @@ info_done_dealloc: , sess(NULL) , status(ISSL_NONE) , profile(sslprofile) +#ifdef INSPIRCD_GNUTLS_HAS_CORK + , gbuffersize(0) +#endif { gnutls_init(&sess, flags); gnutls_transport_set_ptr(sess, reinterpret_cast(sock)); @@ -985,37 +1037,56 @@ info_done_dealloc: // Session is ready for transferring application data StreamSocket::SendQueue& sendq = user->GetSendQ(); + +#ifdef INSPIRCD_GNUTLS_HAS_CORK + while (true) + { + // If there is something in the GnuTLS buffer try to send() it + int ret = FlushBuffer(user); + if (ret <= 0) + return ret; // Couldn't flush entire buffer, retry later (or close on error) + + // GnuTLS buffer is empty, if the sendq is empty as well then break to set FD_WANT_NO_WRITE + if (sendq.empty()) + break; + + // GnuTLS buffer is empty but sendq is not, begin sending data from the sendq + gnutls_record_cork(this->sess); + while ((!sendq.empty()) && (gbuffersize < profile->GetOutgoingRecordSize())) + { + const StreamSocket::SendQueue::Element& elem = sendq.front(); + gbuffersize += elem.length(); + ret = gnutls_record_send(this->sess, elem.data(), elem.length()); + if (ret < 0) + { + CloseSession(); + return -1; + } + sendq.pop_front(); + } + } +#else int ret = 0; while (!sendq.empty()) { FlattenSendQueue(sendq, profile->GetOutgoingRecordSize()); const StreamSocket::SendQueue::Element& buffer = sendq.front(); - ret = gnutls_record_send(this->sess, buffer.data(), buffer.length()); + ret = HandleWriteRet(user, gnutls_record_send(this->sess, buffer.data(), buffer.length())); - if (ret == (int)buffer.length()) - { - // Wrote entire record, continue sending - sendq.pop_front(); - } - else if (ret > 0) + if (ret <= 0) + return ret; + else if (ret < (int)buffer.length()) { sendq.erase_front(ret); SocketEngine::ChangeEventMask(user, FD_WANT_SINGLE_WRITE); return 0; } - else if (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED || ret == 0) - { - SocketEngine::ChangeEventMask(user, FD_WANT_SINGLE_WRITE); - return 0; - } - else // (ret < 0) - { - user->SetError(gnutls_strerror(ret)); - CloseSession(); - return -1; - } + + // Wrote entire record, continue sending + sendq.pop_front(); } +#endif SocketEngine::ChangeEventMask(user, FD_WANT_NO_WRITE); return 1; -- cgit v1.2.3