From 25ce19427343723875b56314166a36d093482598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnter=20Obiltschnig?= Date: Sat, 23 Nov 2024 11:10:53 +0100 Subject: [PATCH] fix(NetSSL): shutdown behavior --- Net/include/Poco/Net/SocketImpl.h | 14 ++++- Net/include/Poco/Net/StreamSocket.h | 14 ++++- Net/include/Poco/Net/WebSocketImpl.h | 4 +- .../HTTPTimeServer/src/HTTPTimeServer.cpp | 3 +- Net/src/SocketImpl.cpp | 6 +- Net/src/StreamSocket.cpp | 8 +-- Net/src/WebSocketImpl.cpp | 8 +-- .../include/Poco/Net/SecureSocketImpl.h | 6 +- .../include/Poco/Net/SecureStreamSocketImpl.h | 19 +++++-- NetSSL_OpenSSL/src/SecureSocketImpl.cpp | 57 ++++--------------- NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp | 7 ++- .../include/Poco/Net/SecureSocketImpl.h | 5 +- .../include/Poco/Net/SecureStreamSocketImpl.h | 19 +++++-- NetSSL_Win/src/SecureSocketImpl.cpp | 18 ++++-- NetSSL_Win/src/SecureStreamSocketImpl.cpp | 3 +- 15 files changed, 104 insertions(+), 87 deletions(-) diff --git a/Net/include/Poco/Net/SocketImpl.h b/Net/include/Poco/Net/SocketImpl.h index 2a71877e1..a2d4c8e62 100644 --- a/Net/include/Poco/Net/SocketImpl.h +++ b/Net/include/Poco/Net/SocketImpl.h @@ -170,12 +170,22 @@ public: 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 once the underlying socket becomes writable. - 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 once the underlying socket becomes writable. 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 662fb6dd3..fa5706316 100644 --- a/Net/include/Poco/Net/StreamSocket.h +++ b/Net/include/Poco/Net/StreamSocket.h @@ -157,12 +157,22 @@ public: 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 once the underlying socket becomes writable. - 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 once the underlying socket becomes writable. 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 4e53d3871..ee8ede76d 100644 --- a/Net/include/Poco/Net/WebSocketImpl.h +++ b/Net/include/Poco/Net/WebSocketImpl.h @@ -69,8 +69,8 @@ public: 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/samples/HTTPTimeServer/src/HTTPTimeServer.cpp b/Net/samples/HTTPTimeServer/src/HTTPTimeServer.cpp index 3289c3d1e..9f4ee7667 100644 --- a/Net/samples/HTTPTimeServer/src/HTTPTimeServer.cpp +++ b/Net/samples/HTTPTimeServer/src/HTTPTimeServer.cpp @@ -67,10 +67,11 @@ public: response.setChunkedTransferEncoding(true); response.setContentType("text/html"); + response.set("Clear-Site-Data", "\"cookies\""); std::ostream& ostr = response.send(); ostr << "HTTPTimeServer powered by POCO C++ Libraries"; - ostr << ""; + ostr << ""; ostr << "

"; ostr << dt; ostr << "

"; diff --git a/Net/src/SocketImpl.cpp b/Net/src/SocketImpl.cpp index 579dfc656..b0c828fb3 100644 --- a/Net/src/SocketImpl.cpp +++ b/Net/src/SocketImpl.cpp @@ -327,21 +327,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 806c56516..dc8e84858 100644 --- a/Net/src/StreamSocket.cpp +++ b/Net/src/StreamSocket.cpp @@ -146,15 +146,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 23759d54c..fa2a27fb2 100644 --- a/Net/src/WebSocketImpl.cpp +++ b/Net/src/WebSocketImpl.cpp @@ -539,15 +539,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 10e9c1aee..477718618 100644 --- a/NetSSL_OpenSSL/include/Poco/Net/SecureSocketImpl.h +++ b/NetSSL_OpenSSL/include/Poco/Net/SecureSocketImpl.h @@ -148,10 +148,11 @@ public: /// 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. @@ -294,7 +295,6 @@ private: bool _needHandshake; std::string _peerHostName; Session::Ptr _pSession; - bool _bidirectShutdown = true; mutable MutexT _mutex; friend class SecureStreamSocketImpl; diff --git a/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h b/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h index 942a36c45..083399166 100644 --- a/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h +++ b/NetSSL_OpenSSL/include/Poco/Net/SecureStreamSocketImpl.h @@ -116,14 +116,25 @@ public: /// Since SSL does not support a half shutdown, this does /// nothing. - void shutdownSend() override; + int shutdownSend() override; /// 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() override; + int shutdown() override; /// 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 2d70fd57b..a68cb917a 100644 --- a/NetSSL_OpenSSL/src/SecureSocketImpl.cpp +++ b/NetSSL_OpenSSL/src/SecureSocketImpl.cpp @@ -253,69 +253,33 @@ void SecureSocketImpl::listen(int backlog) } -void SecureSocketImpl::shutdown() +int SecureSocketImpl::shutdown() { if (_pSSL) { UnLockT l(_mutex); - // 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. -#if OPENSSL_VERSION_NUMBER >= 0x30000000L - int rc = 0; - if (!_bidirectShutdown) - rc = ::SSL_shutdown(_pSSL); - else - { - Poco::Timespan recvTimeout = _pSocket->getReceiveTimeout(); - Poco::Timespan pollTimeout(0, 100000); - Poco::Timestamp tsNow; - do - { - rc = ::SSL_shutdown(_pSSL); - if (rc == 1) break; - if (rc < 0) - { - int err = ::SSL_get_error(_pSSL, rc); - if (err == SSL_ERROR_WANT_READ) - _pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_READ); - else if (err == SSL_ERROR_WANT_WRITE) - _pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_WRITE); - else - { - int socketError = SocketImpl::lastError(); - long lastError = ::ERR_get_error(); - if ((err == SSL_ERROR_SSL) && (socketError == 0) && (lastError == 0x0A000123)) - rc = 0; - break; - } - } - else _pSocket->poll(pollTimeout, Poco::Net::Socket::SELECT_READ); - } while (!tsNow.isElapsed(recvTimeout.totalMicroseconds())); - } -#else int rc = ::SSL_shutdown(_pSSL); -#endif - if (rc < 0) handleError(rc); + if (rc < 0) rc = handleError(rc); l.unlock(); - if (_pSocket->getBlocking()) + if (rc >= 0) { - _pSocket->shutdown(); + _pSocket->shutdownSend(); } + return rc; + } + else + { + return (shutdownState & SSL_RECEIVED_SHUTDOWN) == SSL_RECEIVED_SHUTDOWN; } } + return 1; } @@ -407,7 +371,6 @@ int SecureSocketImpl::receiveBytes(void* buffer, int length, int flags) if (tsStart.isElapsed(recvTimeout.totalMicroseconds())) throw Poco::TimeoutException(); }; - _bidirectShutdown = false; if (rc <= 0) { return handleError(rc); diff --git a/NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp b/NetSSL_OpenSSL/src/SecureStreamSocketImpl.cpp index 289279e37..3523831e8 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/NetSSL_Win/include/Poco/Net/SecureSocketImpl.h b/NetSSL_Win/include/Poco/Net/SecureSocketImpl.h index b3fa77c29..9fc60b4b3 100644 --- a/NetSSL_Win/include/Poco/Net/SecureSocketImpl.h +++ b/NetSSL_Win/include/Poco/Net/SecureSocketImpl.h @@ -154,10 +154,11 @@ public: /// 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. diff --git a/NetSSL_Win/include/Poco/Net/SecureStreamSocketImpl.h b/NetSSL_Win/include/Poco/Net/SecureStreamSocketImpl.h index 243f0a28a..6760d6b7d 100644 --- a/NetSSL_Win/include/Poco/Net/SecureStreamSocketImpl.h +++ b/NetSSL_Win/include/Poco/Net/SecureStreamSocketImpl.h @@ -117,14 +117,25 @@ public: /// 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_Win/src/SecureSocketImpl.cpp b/NetSSL_Win/src/SecureSocketImpl.cpp index fb62c3ea4..c9ea76b83 100644 --- a/NetSSL_Win/src/SecureSocketImpl.cpp +++ b/NetSSL_Win/src/SecureSocketImpl.cpp @@ -265,7 +265,7 @@ void SecureSocketImpl::listen(int backlog) } -void SecureSocketImpl::shutdown() +int SecureSocketImpl::shutdown() { if (_mode == MODE_SERVER) serverDisconnect(&_hCreds, &_hContext); @@ -273,16 +273,22 @@ void SecureSocketImpl::shutdown() clientDisconnect(&_hCreds, &_hContext); _pSocket->shutdown(); + return 0; } void SecureSocketImpl::close() { - if (_mode == MODE_SERVER) - serverDisconnect(&_hCreds, &_hContext); - else - clientDisconnect(&_hCreds, &_hContext); - + try + { + if (_mode == MODE_SERVER) + serverDisconnect(&_hCreds, &_hContext); + else + clientDisconnect(&_hCreds, &_hContext); + } + catch (Poco::Exception&) + { + } _pSocket->close(); cleanup(); } diff --git a/NetSSL_Win/src/SecureStreamSocketImpl.cpp b/NetSSL_Win/src/SecureStreamSocketImpl.cpp index e90a03059..ca7048c9b 100644 --- a/NetSSL_Win/src/SecureStreamSocketImpl.cpp +++ b/NetSSL_Win/src/SecureStreamSocketImpl.cpp @@ -151,12 +151,13 @@ void SecureStreamSocketImpl::shutdownReceive() void SecureStreamSocketImpl::shutdownSend() { + return _impl.shutdown(); } void SecureStreamSocketImpl::shutdown() { - _impl.shutdown(); + return _impl.shutdown(); }