diff --git a/include/envoy/api/io_error.h b/include/envoy/api/io_error.h index 247e102a49d59..ff65c3daac14d 100644 --- a/include/envoy/api/io_error.h +++ b/include/envoy/api/io_error.h @@ -31,6 +31,8 @@ class IoError { Interrupt, // Requested a nonexistent interface or a non-local source address. AddressNotAvailable, + // Connection reset by peer. + ConnectionResetByPeer, // Bad file descriptor. BadFd, // Other error codes cannot be mapped to any one above in getErrorCode(). diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index 07bf4319ef2d5..e6d2c4722c98a 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -147,6 +147,7 @@ struct msghdr { #define SOCKET_ERROR_ADDR_NOT_AVAIL WSAEADDRNOTAVAIL #define SOCKET_ERROR_INVAL WSAEINVAL #define SOCKET_ERROR_ADDR_IN_USE WSAEADDRINUSE +#define SOCKET_ERROR_ECONNRESET WSAECONNRESET #define HANDLE_ERROR_PERM ERROR_ACCESS_DENIED #define HANDLE_ERROR_INVALID ERROR_INVALID_HANDLE @@ -240,6 +241,7 @@ typedef int filesystem_os_id_t; // NOLINT(modernize-use-using) #define SOCKET_ERROR_ADDR_NOT_AVAIL EADDRNOTAVAIL #define SOCKET_ERROR_INVAL EINVAL #define SOCKET_ERROR_ADDR_IN_USE EADDRINUSE +#define SOCKET_ERROR_ECONNRESET ECONNRESET // Mapping POSIX file errors to common error names #define HANDLE_ERROR_PERM EACCES diff --git a/include/envoy/network/post_io_action.h b/include/envoy/network/post_io_action.h index 3b828bc1d5e7b..3c398601369b5 100644 --- a/include/envoy/network/post_io_action.h +++ b/include/envoy/network/post_io_action.h @@ -8,9 +8,11 @@ namespace Network { */ enum class PostIoAction { // Close the connection. - Close, + CloseGraceful, // Keep the connection open. - KeepOpen + KeepOpen, + // Close the connection because of an error. + CloseError, }; } // namespace Network diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 1a265cf10287b..f8fccc96e86b0 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -25,8 +25,12 @@ namespace { constexpr absl::string_view kTransportSocketConnectTimeoutTerminationDetails = "transport socket timeout was reached"; +constexpr absl::string_view kDownstreamConnectionTerminationDetails = + "downstream connection was terminated"; +constexpr absl::string_view kUpstreamConnectionTerminationDetails = + "upstream connection was terminated"; -} +} // namespace void ConnectionImplUtility::updateBufferStats(uint64_t delta, uint64_t new_total, uint64_t& previous_total, Stats::Counter& stat_total, @@ -585,7 +589,20 @@ void ConnectionImpl::onReadReady() { // a connection close. if ((!enable_half_close_ && result.end_stream_read_)) { result.end_stream_read_ = false; - result.action_ = PostIoAction::Close; + result.action_ = PostIoAction::CloseGraceful; + } + + if (result.action_ == PostIoAction::CloseError) { + if (dynamic_cast(this)) { + stream_info_.setConnectionTerminationDetails(kDownstreamConnectionTerminationDetails); + stream_info_.setResponseFlag(StreamInfo::ResponseFlag::DownstreamConnectionTermination); + } else { + stream_info_.setUpstreamTransportFailureReason(kUpstreamConnectionTerminationDetails); + stream_info_.setConnectionTerminationDetails(kUpstreamConnectionTerminationDetails); + stream_info_.setResponseFlag(StreamInfo::ResponseFlag::UpstreamConnectionTermination); + } + // Force "end_stream" so that filters can process this error. + result.end_stream_read_ = true; } read_end_stream_ |= result.end_stream_read_; @@ -598,7 +615,7 @@ void ConnectionImpl::onReadReady() { } // The read callback may have already closed the connection. - if (result.action_ == PostIoAction::Close || bothSidesHalfClosed()) { + if (result.action_ != PostIoAction::KeepOpen || bothSidesHalfClosed()) { ENVOY_CONN_LOG(debug, "remote close", *this); closeSocket(ConnectionEvent::RemoteClose); } @@ -651,11 +668,22 @@ void ConnectionImpl::onWriteReady() { uint64_t new_buffer_size = write_buffer_->length(); updateWriteBufferStats(result.bytes_processed_, new_buffer_size); + if (result.action_ == PostIoAction::CloseError) { + if (dynamic_cast(this)) { + stream_info_.setConnectionTerminationDetails(kDownstreamConnectionTerminationDetails); + stream_info_.setResponseFlag(StreamInfo::ResponseFlag::DownstreamConnectionTermination); + } else { + stream_info_.setUpstreamTransportFailureReason(kUpstreamConnectionTerminationDetails); + stream_info_.setConnectionTerminationDetails(kUpstreamConnectionTerminationDetails); + stream_info_.setResponseFlag(StreamInfo::ResponseFlag::UpstreamConnectionTermination); + } + } + // NOTE: If the delayed_close_timer_ is set, it must only trigger after a delayed_close_timeout_ // period of inactivity from the last write event. Therefore, the timer must be reset to its // original timeout value unless the socket is going to be closed as a result of the doWrite(). - if (result.action_ == PostIoAction::Close) { + if (result.action_ != PostIoAction::KeepOpen) { // It is possible (though unlikely) for the connection to have already been closed during the // write callback. This can happen if we manage to complete the SSL handshake in the write // callback, raise a connected event, and close the connection. diff --git a/source/common/network/io_socket_error_impl.cc b/source/common/network/io_socket_error_impl.cc index fcf1f1efcf42b..596c7b873b65e 100644 --- a/source/common/network/io_socket_error_impl.cc +++ b/source/common/network/io_socket_error_impl.cc @@ -26,6 +26,8 @@ Api::IoError::IoErrorCode IoSocketError::getErrorCode() const { return IoErrorCode::Interrupt; case SOCKET_ERROR_ADDR_NOT_AVAIL: return IoErrorCode::AddressNotAvailable; + case SOCKET_ERROR_ECONNRESET: + return IoErrorCode::ConnectionResetByPeer; default: ENVOY_LOG_MISC(debug, "Unknown error code {} details {}", errno_, getErrorDetails()); return IoErrorCode::UnknownError; diff --git a/source/common/network/raw_buffer_socket.cc b/source/common/network/raw_buffer_socket.cc index c539add2f71ad..bef85a2c23819 100644 --- a/source/common/network/raw_buffer_socket.cc +++ b/source/common/network/raw_buffer_socket.cc @@ -38,7 +38,7 @@ IoResult RawBufferSocket::doRead(Buffer::Instance& buffer) { ENVOY_CONN_LOG(trace, "read error: {}", callbacks_->connection(), result.err_->getErrorDetails()); if (result.err_->getErrorCode() != Api::IoError::IoErrorCode::Again) { - action = PostIoAction::Close; + action = PostIoAction::CloseError; } break; } @@ -73,7 +73,7 @@ IoResult RawBufferSocket::doWrite(Buffer::Instance& buffer, bool end_stream) { if (result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again) { action = PostIoAction::KeepOpen; } else { - action = PostIoAction::Close; + action = PostIoAction::CloseError; } break; } diff --git a/source/extensions/transport_sockets/alts/tsi_socket.cc b/source/extensions/transport_sockets/alts/tsi_socket.cc index 0fe5b752ceca3..0a92469060fea 100644 --- a/source/extensions/transport_sockets/alts/tsi_socket.cc +++ b/source/extensions/transport_sockets/alts/tsi_socket.cc @@ -83,7 +83,7 @@ Network::PostIoAction TsiSocket::doHandshakeNextDone(NextResultPtr&& next_result if (status != TSI_INCOMPLETE_DATA && status != TSI_OK) { ENVOY_CONN_LOG(debug, "TSI: Handshake failed: status: {}", callbacks_->connection(), status); - return Network::PostIoAction::Close; + return Network::PostIoAction::CloseError; } if (next_result->to_send_->length() > 0) { @@ -110,7 +110,7 @@ Network::PostIoAction TsiSocket::doHandshakeNextDone(NextResultPtr&& next_result } else { ENVOY_CONN_LOG(debug, "TSI: Handshake validation failed: {}", callbacks_->connection(), err); - return Network::PostIoAction::Close; + return Network::PostIoAction::CloseGraceful; } } else { ENVOY_CONN_LOG(debug, "TSI: Handshake validation skipped.", callbacks_->connection()); @@ -145,7 +145,7 @@ Network::PostIoAction TsiSocket::doHandshakeNextDone(NextResultPtr&& next_result if (read_error_ || (!handshake_complete_ && end_stream_read_)) { ENVOY_CONN_LOG(debug, "TSI: Handshake failed: end of stream without enough data", callbacks_->connection()); - return Network::PostIoAction::Close; + return Network::PostIoAction::CloseError; } if (raw_read_buffer_.length() > 0) { @@ -167,21 +167,21 @@ Network::IoResult TsiSocket::doRead(Buffer::Instance& buffer) { ENVOY_CONN_LOG(debug, "TSI: raw read result action {} bytes {} end_stream {}", callbacks_->connection(), enumToInt(result.action_), result.bytes_processed_, result.end_stream_read_); - if (result.action_ == Network::PostIoAction::Close && result.bytes_processed_ == 0) { + if (result.action_ != Network::PostIoAction::KeepOpen && result.bytes_processed_ == 0) { return result; } if (!handshake_complete_ && result.end_stream_read_ && result.bytes_processed_ == 0) { - return {Network::PostIoAction::Close, result.bytes_processed_, result.end_stream_read_}; + return {Network::PostIoAction::CloseError, result.bytes_processed_, result.end_stream_read_}; } end_stream_read_ = result.end_stream_read_; - read_error_ = result.action_ == Network::PostIoAction::Close; + read_error_ = result.action_ == Network::PostIoAction::CloseError; } if (!handshake_complete_) { Network::PostIoAction action = doHandshake(); - if (action == Network::PostIoAction::Close || !handshake_complete_) { + if (action != Network::PostIoAction::KeepOpen || !handshake_complete_) { return {action, 0, false}; } } @@ -244,7 +244,7 @@ void TsiSocket::onNextDone(NextResultPtr&& result) { handshaker_next_calling_ = false; Network::PostIoAction action = doHandshakeNextDone(std::move(result)); - if (action == Network::PostIoAction::Close) { + if (action != Network::PostIoAction::KeepOpen) { callbacks_->connection().close(Network::ConnectionCloseType::NoFlush); } } diff --git a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc index 3d4f716421e72..9e23a148b621e 100644 --- a/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc @@ -91,7 +91,7 @@ Network::IoResult UpstreamProxyProtocolSocket::writeHeader() { ENVOY_CONN_LOG(trace, "write error: {}", callbacks_->connection(), result.err_->getErrorDetails()); if (result.err_->getErrorCode() != Api::IoError::IoErrorCode::Again) { - action = Network::PostIoAction::Close; + action = Network::PostIoAction::CloseError; } break; } diff --git a/source/extensions/transport_sockets/tls/ssl_handshaker.cc b/source/extensions/transport_sockets/tls/ssl_handshaker.cc index 7d8def3ecfd1c..b130752f253b7 100644 --- a/source/extensions/transport_sockets/tls/ssl_handshaker.cc +++ b/source/extensions/transport_sockets/tls/ssl_handshaker.cc @@ -230,7 +230,7 @@ Network::PostIoAction SslHandshakerImpl::doHandshake() { // It's possible that we closed during the handshake callback. return handshake_callbacks_->connection().state() == Network::Connection::State::Open ? PostIoAction::KeepOpen - : PostIoAction::Close; + : PostIoAction::CloseGraceful; } else { int err = SSL_get_error(ssl(), rc); switch (err) { @@ -242,7 +242,7 @@ Network::PostIoAction SslHandshakerImpl::doHandshake() { return PostIoAction::KeepOpen; default: handshake_callbacks_->onFailure(); - return PostIoAction::Close; + return PostIoAction::CloseError; } } } diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index 50b2d27926e21..eaf062f1ab281 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -36,9 +36,11 @@ class NotReadySslSocket : public Network::TransportSocket { absl::string_view failureReason() const override { return NotReadyReason; } bool canFlushClose() override { return true; } void closeSocket(Network::ConnectionEvent) override {} - Network::IoResult doRead(Buffer::Instance&) override { return {PostIoAction::Close, 0, false}; } + Network::IoResult doRead(Buffer::Instance&) override { + return {PostIoAction::CloseError, 0, false}; + } Network::IoResult doWrite(Buffer::Instance&, bool) override { - return {PostIoAction::Close, 0, false}; + return {PostIoAction::CloseError, 0, false}; } void onConnected() override {} Ssl::ConnectionInfoConstSharedPtr ssl() const override { return nullptr; } @@ -110,7 +112,7 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { if (info_->state() != Ssl::SocketState::HandshakeComplete && info_->state() != Ssl::SocketState::ShutdownSent) { PostIoAction action = doHandshake(); - if (action == PostIoAction::Close || info_->state() != Ssl::SocketState::HandshakeComplete) { + if (action != PostIoAction::KeepOpen || info_->state() != Ssl::SocketState::HandshakeComplete) { // end_stream is false because either a hard error occurred (action == Close) or // the handshake isn't complete, so a half-close cannot occur yet. return {action, 0, false}; @@ -154,7 +156,7 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { // Renegotiation has started. We don't handle renegotiation so just fall through. default: drainErrorQueue(); - action = PostIoAction::Close; + action = PostIoAction::CloseError; break; } @@ -182,7 +184,7 @@ void SslSocket::onPrivateKeyMethodComplete() { // Resume handshake. PostIoAction action = doHandshake(); - if (action == PostIoAction::Close) { + if (action != PostIoAction::KeepOpen) { ENVOY_CONN_LOG(debug, "async handshake completion error", callbacks_->connection()); callbacks_->connection().close(Network::ConnectionCloseType::FlushWrite); } @@ -231,7 +233,7 @@ Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_st if (info_->state() != Ssl::SocketState::HandshakeComplete && info_->state() != Ssl::SocketState::ShutdownSent) { PostIoAction action = doHandshake(); - if (action == PostIoAction::Close || info_->state() != Ssl::SocketState::HandshakeComplete) { + if (action != PostIoAction::KeepOpen || info_->state() != Ssl::SocketState::HandshakeComplete) { return {action, 0, false}; } } @@ -270,7 +272,7 @@ Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_st // Renegotiation has started. We don't handle renegotiation so just fall through. default: drainErrorQueue(); - return {PostIoAction::Close, total_bytes_written, false}; + return {PostIoAction::CloseError, total_bytes_written, false}; } break; diff --git a/test/extensions/transport_sockets/alts/tsi_socket_test.cc b/test/extensions/transport_sockets/alts/tsi_socket_test.cc index 85f40ab3fcd8d..59cd03730d5fb 100644 --- a/test/extensions/transport_sockets/alts/tsi_socket_test.cc +++ b/test/extensions/transport_sockets/alts/tsi_socket_test.cc @@ -327,12 +327,13 @@ TEST_F(TsiSocketTest, HandshakeWithImmediateReadError) { initialize(nullptr, nullptr); EXPECT_CALL(*client_.raw_socket_, doRead(_)).WillOnce(Invoke([&](Buffer::Instance& buffer) { - Network::IoResult result = {Network::PostIoAction::Close, server_to_client_.length(), false}; + Network::IoResult result = {Network::PostIoAction::CloseError, server_to_client_.length(), + false}; buffer.move(server_to_client_); return result; })); EXPECT_CALL(*client_.raw_socket_, doWrite(_, false)).Times(0); - expectIoResult({Network::PostIoAction::Close, 0UL, false}, + expectIoResult({Network::PostIoAction::CloseError, 0UL, false}, client_.tsi_socket_->doRead(client_.read_buffer_)); EXPECT_EQ("", client_to_server_.toString()); EXPECT_EQ(0L, client_.read_buffer_.length()); @@ -344,7 +345,8 @@ TEST_F(TsiSocketTest, HandshakeWithReadError) { doFakeInitHandshake(); EXPECT_CALL(*client_.raw_socket_, doRead(_)).WillOnce(Invoke([&](Buffer::Instance& buffer) { - Network::IoResult result = {Network::PostIoAction::Close, server_to_client_.length(), false}; + Network::IoResult result = {Network::PostIoAction::CloseError, server_to_client_.length(), + false}; buffer.move(server_to_client_); return result; })); diff --git a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc index 16f56b4f94895..ae10186bb65c7 100644 --- a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc @@ -174,7 +174,7 @@ TEST_F(ProxyProtocolTest, ReturnsCloseWhenWriteErrorIsNotAgain) { } auto resp = proxy_protocol_socket_->doWrite(msg, false); - EXPECT_EQ(Network::PostIoAction::Close, resp.action_); + EXPECT_EQ(Network::PostIoAction::CloseError, resp.action_); } // Test injects V1 PROXY protocol using upstream addresses when transport options are null diff --git a/test/extensions/transport_sockets/tls/handshaker_test.cc b/test/extensions/transport_sockets/tls/handshaker_test.cc index 860eca5152fec..998f6bd5fc707 100644 --- a/test/extensions/transport_sockets/tls/handshaker_test.cc +++ b/test/extensions/transport_sockets/tls/handshaker_test.cc @@ -133,13 +133,13 @@ TEST_F(HandshakerTest, NormalOperation) { auto post_io_action = Network::PostIoAction::KeepOpen; // default enum // Run the handshakes from the client and server until SslHandshakerImpl decides - // we're done and returns PostIoAction::Close. - while (post_io_action != Network::PostIoAction::Close) { + // we're done and returns PostIoAction::CloseGraceful. + while (post_io_action == Network::PostIoAction::KeepOpen) { SSL_do_handshake(client_ssl_.get()); post_io_action = handshaker.doHandshake(); } - EXPECT_EQ(post_io_action, Network::PostIoAction::Close); + EXPECT_EQ(post_io_action, Network::PostIoAction::CloseGraceful); } // We induce some kind of BIO mismatch and force the SSL_do_handshake to @@ -159,13 +159,13 @@ TEST_F(HandshakerTest, ErrorCbOnAbnormalOperation) { auto post_io_action = Network::PostIoAction::KeepOpen; // default enum - while (post_io_action != Network::PostIoAction::Close) { + while (post_io_action == Network::PostIoAction::KeepOpen) { SSL_do_handshake(client_ssl_.get()); post_io_action = handshaker.doHandshake(); } // In the error case, SslHandshakerImpl also closes the connection. - EXPECT_EQ(post_io_action, Network::PostIoAction::Close); + EXPECT_EQ(post_io_action, Network::PostIoAction::CloseError); } // Example SslHandshakerImpl demonstrating special-case behavior which necessitates @@ -193,7 +193,7 @@ class SslHandshakerImplForTest : public SslHandshakerImpl { if (rc == 1) { setState(Ssl::SocketState::HandshakeComplete); handshakeCallbacks()->onSuccess(ssl()); - return Network::PostIoAction::Close; + return Network::PostIoAction::CloseGraceful; } else { switch (SSL_get_error(ssl(), rc)) { case SSL_ERROR_WANT_READ: @@ -206,7 +206,7 @@ class SslHandshakerImplForTest : public SslHandshakerImpl { return Network::PostIoAction::KeepOpen; default: handshakeCallbacks()->onFailure(); - return Network::PostIoAction::Close; + return Network::PostIoAction::CloseError; } } } @@ -231,12 +231,12 @@ TEST_F(HandshakerTest, NormalOperationWithSslHandshakerImplForTest) { auto post_io_action = Network::PostIoAction::KeepOpen; // default enum - while (post_io_action != Network::PostIoAction::Close) { + while (post_io_action == Network::PostIoAction::KeepOpen) { SSL_do_handshake(client_ssl_.get()); post_io_action = handshaker.doHandshake(); } - EXPECT_EQ(post_io_action, Network::PostIoAction::Close); + EXPECT_EQ(post_io_action, Network::PostIoAction::CloseGraceful); } } // namespace diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 403a08b61c3a5..e9495e32d2e04 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -2699,7 +2699,7 @@ TEST_P(SslSocketTest, ShutdownWithoutCloseNotify) { Buffer::OwnedImpl data("hello"); server_connection->write(data, false); EXPECT_EQ(data.length(), 0); - // Calling close(FlushWrite) in onEvent() callback results in PostIoAction::Close, + // Calling close(FlushWrite) in onEvent() callback results in PostIoAction::CloseGraceful, // after which the connection is closed without write ready event being delivered, // and with all outstanding data (here, "hello") being lost. })); @@ -4726,9 +4726,9 @@ TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { EXPECT_EQ(nullptr, transport_socket->ssl()); Buffer::OwnedImpl buffer; Network::IoResult result = transport_socket->doRead(buffer); - EXPECT_EQ(Network::PostIoAction::Close, result.action_); + EXPECT_EQ(Network::PostIoAction::CloseError, result.action_); result = transport_socket->doWrite(buffer, true); - EXPECT_EQ(Network::PostIoAction::Close, result.action_); + EXPECT_EQ(Network::PostIoAction::CloseError, result.action_); EXPECT_EQ("TLS error: Secret is not supplied by SDS", transport_socket->failureReason()); } @@ -4761,9 +4761,9 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { EXPECT_EQ(nullptr, transport_socket->ssl()); Buffer::OwnedImpl buffer; Network::IoResult result = transport_socket->doRead(buffer); - EXPECT_EQ(Network::PostIoAction::Close, result.action_); + EXPECT_EQ(Network::PostIoAction::CloseError, result.action_); result = transport_socket->doWrite(buffer, true); - EXPECT_EQ(Network::PostIoAction::Close, result.action_); + EXPECT_EQ(Network::PostIoAction::CloseError, result.action_); EXPECT_EQ("TLS error: Secret is not supplied by SDS", transport_socket->failureReason()); }