openssl: SSL_Read failed with SSL_ERROR_SYSCALL when read timeout

@zaher it looks like your issue has been resolved; can this be closed now?

I am making this a documentation issue as it would be nice if this was documented somewhere.

I see the problem. You should avoid using setsockopt() for timeouts. You can get wacky behavior when using setsockopt() and openssl. Chances are that you are going to be operating with a block cipher, instead of a streaming cipher. Remember that SSL_read just returns what openssl has finished decrypting, which may be different than how much has come over the socket. You’re forcing a timeout at the socket level.

First, call SSL_has_pending to see if openssl has some decrypted data to read. If it does, just call SSL_read. If it doesn’t, then use select() with an appropriate timeout. Then, if that doesn’t time out, you can check SSL_has_pending again to see if openssl has something to read. If this second call to SSL_has_pending fails, treat it as a timeout.

Realistically, the second call to SSL_has_pending is kind of redundant. If select() doesn’t time out, then openssl should have something ready for you to read.

On Wed, May 17, 2023 at 11:40 AM Youda008 @.***> wrote:

When is this happening? Is it during the handshake SSL_connect() or something else?

We’re talking about SSL_read. Here’s an minimal example: https://pastebin.com/RG1T3d9s

— Reply to this email directly, view it on GitHub https://github.com/openssl/openssl/issues/12416#issuecomment-1551732467, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQJQ3OS2SXEWX7QWVEOP53XGT5PXANCNFSM4OW2V3XA . You are receiving this because you were mentioned.Message ID: @.***>

– John L Veazey

If you need a timeout, you need to do that at the application level. The SSL layer should only return an error in case something is wrong with SSL.

Understandable, but then OpenSSL shouldn’t touch errno / Windows last error, so that we can detect timeout by checking errno == ETIMEDOUT || errno == EWOULDBLOCK or WSAGetLastError() == WSAETIMEDOUT.

Using select() works, but it’s rather an ugly workaround than a correct solution.

OpenSSL should support the following workflow:

int ret = SSL_read( ssl, buffer, sizeof(buffer) );
if (ret <= 0)
{
	int error = SSL_get_error( ssl, ret );
	if (ret == SSL_ERROR_ZERO_RETURN)
		// server closed connection
	else if (ret == SSL_ERROR_SYSCALL && WSAGetLastError() == WSAETIMEDOUT)
		// timeout
	else
		// unexpected problem
}

I guess my question is more along the lines about how mnSockets.pas ties together with mnOpenSSL.pas.

In mnOpenSSL.pas, I see a method called TBIOStream.Read, but the only thing in it is SSL_read and I in mnSockets.pas I see a method called TmnSocketStream.WaitToRead. Where in the code can I see these two interact?

Edit2: I see where select is called, but there are problems with the code.

https://github.com/parmaja/minilib/blob/master/socket/source/mnWinSockets.pas

    begin
      TimeVal.tv_sec := Timeout div 1000;
      TimeVal.tv_usec := (Timeout mod 1000) * 1000;
      c := WinSock2.select(0, PSetRead, PSetWrite, nil, @TimeVal);
    end;
    if (c = SOCKET_ERROR) then
      Result := erInvalid
    else if (c = 0) then
      Result := erTimeout
    else
      Result := erSuccess;

Do not set both TimeVal.tv_sec and TimeVal.tv_usec. Use one or the other.

The code for determining a timeout does not match the C++ code I provided.

1. The call to select passes an fd_set into PSetRead and PsWrite. You need to pass in nil instead of PSetWrite. We aren’t testing whether you can write to the socket.
2. The 4th parameter, you pass in nil, instead of an fd_set. You need to pass something in here, since this parameter is how we can determine whether an error of some kind actually occurred (I’ll assume it is called PSetError).
3. Once select finishes, you need to call FD_ISSET for PSetRead and PSetError.
4. If FD_ISSET(socket, &read) returns true, there is no timeout.
5. if FD_ISSET(socket, &error) returns true, use getsockopt and/or WSAGetLastError and/or errno. If none of these returns useful information, assume WSAECONNRESET.
6. If select returns zero, FD_ISSET(socket, &read) returns false, and FD_ISSET(socket, &error) returns false, assume WSAETIMEDOUT

	int output = ERROR_SUCCESS;
	if (SOCKET_ERROR == select(static_cast<int>(socket+1), &read, nullptr, &error, &time))
		output = WSAGetLastError();
	else if (FD_ISSET(socket, &read))
		output = ERROR_SUCCESS;
	else if (FD_ISSET(socket, &error))
	{
		int length = sizeof(output);
		getsockopt(socket, SOL_SOCKET, SO_ERROR, reinterpret_cast<char*>(&output), &length);
		if (output == ERROR_SUCCESS)
			output = WSAGetLastError();
		if (output == ERROR_SUCCESS)
			output = WSAECONNRESET;
	}
	else
		output = WSAETIMEDOUT;

Looking at, I do see mnOpenSSL.pas where a socket is passed in, but you don’t do anything with it outside of SSL_set_fd and GetSocketError.

You need to use SSL_has_pending and select before calling SSL_read. Here’s a C++ example for using select to check for a read timeout. I’m not familiar enough to translate it to Delphi.

int ReadWait(SOCKET socket, const long timeout)
{
	fd_set read{};
	fd_set error{};
	timeval time{};

	FD_ZERO(&read);
	FD_SET(socket, &read);
	FD_ZERO(&error);
	FD_SET(socket, &error);
	time.tv_sec = timeout;
	time.tv_usec = 0;

	int output = ERROR_SUCCESS;
	if (SOCKET_ERROR == select(static_cast<int>(socket+1), &read, nullptr, &error, &time))
		output = WSAGetLastError();
	else if (FD_ISSET(socket, &read))
		output = ERROR_SUCCESS;
	else if (FD_ISSET(socket, &error))
	{
		int length = sizeof(output);
		getsockopt(socket, SOL_SOCKET, SO_ERROR, reinterpret_cast<char*>(&output), &length);
		if (output == ERROR_SUCCESS)
			output = WSAGetLastError();
		if (output == ERROR_SUCCESS)
			output = WSAECONNRESET;
	}
	else
		output = WSAETIMEDOUT;

	FD_ZERO(&read);
	FD_ZERO(&error);

	return output;
}