Merge pull request #4787 from pocoproject/fix-ssl-shutdown

NetSSL_OpenSSL and NetSSL_Win: non-blocking support, shutdown behavior fix
pocoproject · Nov 29, 2024 · d8a423c · d8a423c
2 parents 0f71e6c + e83119b
commit d8a423c
Show file tree

Hide file tree

Showing 18 changed files with 707 additions and 583 deletions.
diff --git a/Net/include/Poco/Net/SocketImpl.h b/Net/include/Poco/Net/SocketImpl.h
@@ -170,12 +170,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 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
@@ -157,12 +157,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 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
@@ -69,8 +69,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/samples/HTTPTimeServer/src/HTTPTimeServer.cpp b/Net/samples/HTTPTimeServer/src/HTTPTimeServer.cpp
@@ -67,10 +67,11 @@ class TimeRequestHandler: public HTTPRequestHandler
 
 		response.setChunkedTransferEncoding(true);
 		response.setContentType("text/html");
+		response.set("Clear-Site-Data", "\"cookies\"");
 
 		std::ostream& ostr = response.send();
 		ostr << "<html><head><title>HTTPTimeServer powered by POCO C++ Libraries</title>";
-		ostr << "<meta http-equiv=\"refresh\" content=\"1\"></head>";
+		ostr << "</head>";
 		ostr << "<body><p style=\"text-align: center; font-size: 48px;\">";
 		ostr << dt;
 		ostr << "</p></body></html>";

diff --git 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
@@ -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
@@ -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
@@ -148,10 +148,11 @@ 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.
@@ -294,7 +295,6 @@ class NetSSL_API SecureSocketImpl
 	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
@@ -116,14 +116,25 @@ class NetSSL_API SecureStreamSocketImpl: public StreamSocketImpl
 		/// 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.
-
-	void shutdown() override;
+		/// 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.
+
+	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
@@ -253,69 +253,39 @@ 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
+			int rc = ::SSL_shutdown(_pSSL);
+			if (rc < 0) 
 			{
-				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()));
+				if (SocketImpl::lastError() == POCO_EWOULDBLOCK)
+					rc = SecureStreamSocket::ERR_SSL_WANT_WRITE;
+				else
+					rc = handleError(rc);
 			}
-#else
-			int rc = ::SSL_shutdown(_pSSL);
-#endif
-			if (rc < 0) 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 +377,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
@@ -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();
 }