From fc64e5ac278aefd34fb9b1710077dfa62f2d86de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnter=20Obiltschnig?= Date: Mon, 25 Nov 2024 21:08:20 +0100 Subject: [PATCH] enh(Net): non-blocking TLS shutdown --- Net/include/Poco/Net/SocketImpl.h | 14 ++++++-- Net/include/Poco/Net/StreamSocket.h | 14 ++++++-- Net/include/Poco/Net/WebSocketImpl.h | 4 +-- Net/src/SocketImpl.cpp | 6 ++-- Net/src/StreamSocket.cpp | 8 ++--- Net/src/WebSocketImpl.cpp | 8 ++--- .../include/Poco/Net/SecureSocketImpl.h | 10 ++++-- .../include/Poco/Net/SecureStreamSocketImpl.h | 19 ++++++++--- NetSSL_OpenSSL/src/SecureSocketImpl.cpp | 33 +++++++++++-------- NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp | 7 ++-- WebTunnel/src/SocketDispatcher.cpp | 22 +++++++++---- 11 files changed, 101 insertions(+), 44 deletions(-) diff --git a/Net/include/Poco/Net/SocketImpl.h b/Net/include/Poco/Net/SocketImpl.h index 1c7e5e44..febc7e88 100644 --- a/Net/include/Poco/Net/SocketImpl.h +++ b/Net/include/Poco/Net/SocketImpl.h @@ -151,12 +151,22 @@ class Net_API SocketImpl: public Poco::RefCountedObject virtual void shutdownReceive(); /// Shuts down the receiving part of the socket connection. - virtual void shutdownSend(); + virtual int shutdownSend(); /// Shuts down the sending part of the socket connection. + /// + /// Returns 0 for a non-blocking socket. May return + /// a negative value for a non-blocking socket in case + /// of a TLS connection. In that case, the operation should + /// be retried. - virtual void shutdown(); + virtual int shutdown(); /// Shuts down both the receiving and the sending part /// of the socket connection. + /// + /// Returns 0 for a non-blocking socket. May return + /// a negative value for a non-blocking socket in case + /// of a TLS connection. In that case, the operation should + /// be retried. virtual int sendBytes(const void* buffer, int length, int flags = 0); /// Sends the contents of the given buffer through diff --git a/Net/include/Poco/Net/StreamSocket.h b/Net/include/Poco/Net/StreamSocket.h index 1960362b..eb733546 100644 --- a/Net/include/Poco/Net/StreamSocket.h +++ b/Net/include/Poco/Net/StreamSocket.h @@ -90,12 +90,22 @@ class Net_API StreamSocket: public Socket void shutdownReceive(); /// Shuts down the receiving part of the socket connection. - void shutdownSend(); + int shutdownSend(); /// Shuts down the sending part of the socket connection. + /// + /// Returns 0 for a non-blocking socket. May return + /// a negative value for a non-blocking socket in case + /// of a TLS connection. In that case, the operation should + /// be retried. - void shutdown(); + int shutdown(); /// Shuts down both the receiving and the sending part /// of the socket connection. + /// + /// Returns 0 for a non-blocking socket. May return + /// a negative value for a non-blocking socket in case + /// of a TLS connection. In that case, the operation should + /// be retried. int sendBytes(const void* buffer, int length, int flags = 0); /// Sends the contents of the given buffer through diff --git a/Net/include/Poco/Net/WebSocketImpl.h b/Net/include/Poco/Net/WebSocketImpl.h index fc7229c3..3e3b09c7 100644 --- a/Net/include/Poco/Net/WebSocketImpl.h +++ b/Net/include/Poco/Net/WebSocketImpl.h @@ -60,8 +60,8 @@ class Net_API WebSocketImpl: public StreamSocketImpl virtual void listen(int backlog = 64); virtual void close(); virtual void shutdownReceive(); - virtual void shutdownSend(); - virtual void shutdown(); + virtual int shutdownSend(); + virtual int shutdown(); virtual int sendTo(const void* buffer, int length, const SocketAddress& address, int flags = 0); virtual int receiveFrom(void* buffer, int length, SocketAddress& address, int flags = 0); virtual void sendUrgent(unsigned char data); diff --git a/Net/src/SocketImpl.cpp b/Net/src/SocketImpl.cpp index 1ecdf726..962023ae 100644 --- a/Net/src/SocketImpl.cpp +++ b/Net/src/SocketImpl.cpp @@ -293,21 +293,23 @@ void SocketImpl::shutdownReceive() } -void SocketImpl::shutdownSend() +int SocketImpl::shutdownSend() { if (_sockfd == POCO_INVALID_SOCKET) throw InvalidSocketException(); int rc = ::shutdown(_sockfd, 1); if (rc != 0) error(); + return 0; } -void SocketImpl::shutdown() +int SocketImpl::shutdown() { if (_sockfd == POCO_INVALID_SOCKET) throw InvalidSocketException(); int rc = ::shutdown(_sockfd, 2); if (rc != 0) error(); + return 0; } diff --git a/Net/src/StreamSocket.cpp b/Net/src/StreamSocket.cpp index 46136da6..45e17236 100644 --- a/Net/src/StreamSocket.cpp +++ b/Net/src/StreamSocket.cpp @@ -97,15 +97,15 @@ void StreamSocket::shutdownReceive() } -void StreamSocket::shutdownSend() +int StreamSocket::shutdownSend() { - impl()->shutdownSend(); + return impl()->shutdownSend(); } -void StreamSocket::shutdown() +int StreamSocket::shutdown() { - impl()->shutdown(); + return impl()->shutdown(); } diff --git a/Net/src/WebSocketImpl.cpp b/Net/src/WebSocketImpl.cpp index 619972e5..d866f033 100644 --- a/Net/src/WebSocketImpl.cpp +++ b/Net/src/WebSocketImpl.cpp @@ -537,15 +537,15 @@ void WebSocketImpl::shutdownReceive() } -void WebSocketImpl::shutdownSend() +int WebSocketImpl::shutdownSend() { - _pStreamSocketImpl->shutdownSend(); + return _pStreamSocketImpl->shutdownSend(); } -void WebSocketImpl::shutdown() +int WebSocketImpl::shutdown() { - _pStreamSocketImpl->shutdown(); + return _pStreamSocketImpl->shutdown(); } diff --git a/NetSSL_OpenSSL/include/Poco/Net/SecureSocketImpl.h b/NetSSL_OpenSSL/include/Poco/Net/SecureSocketImpl.h index efd331ab..eb8ab860 100644 --- a/NetSSL_OpenSSL/include/Poco/Net/SecureSocketImpl.h +++ b/NetSSL_OpenSSL/include/Poco/Net/SecureSocketImpl.h @@ -148,13 +148,19 @@ class NetSSL_API SecureSocketImpl /// number of connections that can be queued /// for this socket. - void shutdown(); + int shutdown(); /// Shuts down the connection by attempting /// an orderly SSL shutdown, then actually - /// shutting down the TCP connection. + /// shutting down the TCP connection in the + /// send direction. void close(); /// Close the socket. + /// + /// If the SSL connection has not been shut down + /// yet, tries to perform a fast shutdown + /// (sending a close notify shutdown alert message, + /// but not waiting for the response). void abort(); /// Aborts the connection by closing the diff --git a/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h b/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h index 9b325232..1638ff81 100644 --- a/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h +++ b/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h @@ -116,14 +116,25 @@ class NetSSL_API SecureStreamSocketImpl: public StreamSocketImpl /// Since SSL does not support a half shutdown, this does /// nothing. - void shutdownSend(); + int shutdownSend(); /// Shuts down the receiving part of the socket connection. /// - /// Since SSL does not support a half shutdown, this does - /// nothing. + /// Sends a close notify shutdown alert message to the peer + /// (if not sent yet), then calls shutdownSend() on the + /// underlying socket. + /// + /// Returns 0 if the message has been sent. + /// Returns 1 if the message has been sent, but the peer + /// has not yet sent its shutdown alert message. + /// In case of a non-blocking socket, returns < 0 if the + /// message cannot be sent at the moment. In this case, + /// the call to shutdownSend() must be retried after the + /// underlying socket becomes writable again. - void shutdown(); + int shutdown(); /// Shuts down the SSL connection. + /// + /// Same as shutdownSend(). void abort(); /// Aborts the connection by closing the underlying diff --git a/NetSSL_OpenSSL/src/SecureSocketImpl.cpp b/NetSSL_OpenSSL/src/SecureSocketImpl.cpp index 8b41b41c..ac2d5970 100644 --- a/NetSSL_OpenSSL/src/SecureSocketImpl.cpp +++ b/NetSSL_OpenSSL/src/SecureSocketImpl.cpp @@ -237,30 +237,34 @@ void SecureSocketImpl::listen(int backlog) } -void SecureSocketImpl::shutdown() +int SecureSocketImpl::shutdown() { if (_pSSL) { - // Don't shut down the socket more than once. int shutdownState = SSL_get_shutdown(_pSSL); bool shutdownSent = (shutdownState & SSL_SENT_SHUTDOWN) == SSL_SENT_SHUTDOWN; if (!shutdownSent) { - // A proper clean shutdown would require us to - // retry the shutdown if we get a zero return - // value, until SSL_shutdown() returns 1. - // However, this will lead to problems with - // most web browsers, so we just set the shutdown - // flag by calling SSL_shutdown() once and be - // done with it. int rc = SSL_shutdown(_pSSL); - if (rc < 0) handleError(rc); - if (_pSocket->getBlocking()) + if (rc < 0) + { + if (SocketImpl::lastError() == POCO_EWOULDBLOCK) + rc = SecureStreamSocket::ERR_SSL_WANT_WRITE; + else + rc = handleError(rc); + } + if (rc >= 0) { - _pSocket->shutdown(); + _pSocket->shutdownSend(); } + return rc; + } + else + { + return (shutdownState & SSL_RECEIVED_SHUTDOWN) == SSL_RECEIVED_SHUTDOWN; } } + return 1; } @@ -500,7 +504,10 @@ int SecureSocketImpl::handleError(int rc) #endif if (socketError) { - SocketImpl::error(socketError); + if (socketError == POCO_EWOULDBLOCK) + return SSL_ERROR_WANT_READ; + else + SocketImpl::error(socketError); } // fallthrough default: diff --git a/NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp b/NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp index b90113b0..0eccc9c2 100644 --- a/NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp +++ b/NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp @@ -156,14 +156,15 @@ void SecureStreamSocketImpl::shutdownReceive() } -void SecureStreamSocketImpl::shutdownSend() +int SecureStreamSocketImpl::shutdownSend() { + return _impl.shutdown(); } -void SecureStreamSocketImpl::shutdown() +int SecureStreamSocketImpl::shutdown() { - _impl.shutdown(); + return _impl.shutdown(); } diff --git a/WebTunnel/src/SocketDispatcher.cpp b/WebTunnel/src/SocketDispatcher.cpp index 30fc6cc2..7ffe9c61 100644 --- a/WebTunnel/src/SocketDispatcher.cpp +++ b/WebTunnel/src/SocketDispatcher.cpp @@ -508,12 +508,17 @@ void SocketDispatcher::writable(const Poco::Net::Socket& socket, SocketDispatche PendingSend& pending = *pInfo->pendingSends.begin(); if (pending.options == PendingSend::OPT_SHUTDOWN) { - ss.shutdownSend(); - if (pInfo->pendingSends.size() > 1) + _logger.debug("Shutting down socket %?d..."s, ss.impl()->sockfd()); + if (ss.shutdownSend() >= 0) { - _logger.debug("Discarding pending writes after shutdown"); + _logger.debug("Shut down socket %?d."s, ss.impl()->sockfd()); + if (pInfo->pendingSends.size() > 1) + { + _logger.debug("Discarding pending writes after shutdown."s); + } + pInfo->pendingSends.clear(); } - pInfo->pendingSends.clear(); + else break; } else { @@ -667,7 +672,7 @@ void SocketDispatcher::sendBytesImpl(Poco::Net::StreamSocket& socket, Poco::Buff } else { - _logger.error("sendBytes() called with unknown socket."); + _logger.error("sendBytes() called with unknown socket."s); } } @@ -679,7 +684,12 @@ void SocketDispatcher::shutdownSendImpl(Poco::Net::StreamSocket& socket) { if (it->second->pendingSends.empty()) { - socket.shutdownSend(); + int rc = socket.shutdownSend(); + if (rc < 0) + { + // would block, try again later + it->second->pendingSends.emplace_back(PendingSend::OPT_SHUTDOWN); + } } else {