Repository navigation
Conversation
connection_connect_timeout only covers the TCP connect poll in SocketConnect(). SSLSocket then runs a blocking SSL_connect() whose reads are bounded by SO_RCVTIMEO, i.e. the receive timeout, which defaults to infinite, so a peer that accepts the connection but never answers the ClientHello blocks the client forever and failover to the next endpoint never happens. I run the handshake non-blocking in SSLSocket::Handshake() and poll against the connect timeout, throwing std::system_error(ETIMEDOUT) on expiry so ResetConnectionEndpoint() moves on to the next endpoint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
connection_connect_timeoutonly bounds the TCP connect poll inSocketConnect().SSLSocketthen runs a blockingSSL_connect(), whose reads are bounded only bySO_RCVTIMEO, which comes from the receive timeout and defaults to infinite. So a peer that accepts TCP but never answers the ClientHello blocks the client forever, and failover to the next endpoint never runs. With a receive timeout set, expiry surfaces asOpenSSLErrorfromSSL_ERROR_WANT_READ, whichResetConnectionEndpoint()doesn't rotate on either.Fix:
SSLSocket::Handshake()runsSSL_connect()non-blocking, using the newSocket::SetNonBlocking/Socket::WaitFor, and polls against the connect timeout. On expiry it throwsstd::system_error(ETIMEDOUT)(WSAETIMEDOUTon Windows), which every rotation site already treats as a connection failure. The socket goes back to blocking mode after the handshake. A negative timeout still waits forever, like the TCP connect. A TLS endpoint can now take up to twice the connect timeout, and the option comment says so.Related to #486, which covers the plaintext Hello phase after the handshake; this PR doesn't change that phase.
Reproducer: point a TLS client with a short connect timeout at a listener that accepts and never writes.
Test:
OpenSSLConnection.HandshakeIsBoundedByConnectTimeoutuses two such tarpits, a 500 ms connect timeout and a 10 s receive timeout as a hang guard. On master it fails after about 10 s with an uncaughtOpenSSLError. Here it fails over both endpoints withETIMEDOUTin about 1 s. Only the Linux build was compiled and run; the Windows and macOS paths use the existingFIONBIO/fcntlandWSAPoll/pollwrappers.