diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 29f51dd27b668..4661be19535cf 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -5,6 +5,7 @@ Version history ================ * access loggers: access logger extensions use the "envoy.access_loggers" name space. A mapping of extension names is available in the :ref:`deprecated ` documentation. +* access log: fix %DOWSTREAM_DIRECT_REMOTE_ADDRESS% when used with PROXY protocol listener filter * adaptive concurrency: fixed bug that allowed concurrency limits to drop below the configured minimum. * aws_request_signing: a few fixes so that it works with S3. diff --git a/include/envoy/network/connection.h b/include/envoy/network/connection.h index b5ad324d9c49f..6e9667f778043 100644 --- a/include/envoy/network/connection.h +++ b/include/envoy/network/connection.h @@ -172,6 +172,13 @@ class Connection : public Event::DeferredDeletable, public FilterManager { */ virtual const Network::Address::InstanceConstSharedPtr& remoteAddress() const PURE; + /** + * @return The address of the remote directly connected peer. Note that this method + * will never return nullptr. This address is not affected or modified by PROXY protocol + * or any other listener filter. + */ + virtual const Network::Address::InstanceConstSharedPtr& directRemoteAddress() const PURE; + /** * Credentials of the peer of a socket as decided by SO_PEERCRED. */ diff --git a/include/envoy/network/listen_socket.h b/include/envoy/network/listen_socket.h index aa5957eae499d..89b8cdbebb23b 100644 --- a/include/envoy/network/listen_socket.h +++ b/include/envoy/network/listen_socket.h @@ -191,6 +191,12 @@ class ConnectionSocket : public virtual Socket { */ virtual const Address::InstanceConstSharedPtr& remoteAddress() const PURE; + /** + * @return the direct remote address of the socket. This is the address of the directly + * connected peer, and cannot be modified by listener filters. + */ + virtual const Address::InstanceConstSharedPtr& directRemoteAddress() const PURE; + /** * Restores the local address of the socket. On accepted sockets the local address defaults to the * one at which the connection was received at, which is the same as the listener's address, if diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 010c3fb9b9fa4..72639466378af 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -578,7 +578,7 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect stream_info_.setDownstreamLocalAddress( connection_manager_.read_callbacks_->connection().localAddress()); stream_info_.setDownstreamDirectRemoteAddress( - connection_manager_.read_callbacks_->connection().remoteAddress()); + connection_manager_.read_callbacks_->connection().directRemoteAddress()); // Initially, the downstream remote address is the source address of the // downstream connection. That can change later in the request's lifecycle, // based on XFF processing, but setting the downstream remote address here diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index 9042087853e98..60fc901a1331f 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -68,6 +68,9 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback const Address::InstanceConstSharedPtr& remoteAddress() const override { return socket_->remoteAddress(); } + const Address::InstanceConstSharedPtr& directRemoteAddress() const override { + return socket_->directRemoteAddress(); + } const Address::InstanceConstSharedPtr& localAddress() const override { return socket_->localAddress(); } diff --git a/source/common/network/listen_socket_impl.h b/source/common/network/listen_socket_impl.h index aa1173a7a68fc..ef417c77b1df5 100644 --- a/source/common/network/listen_socket_impl.h +++ b/source/common/network/listen_socket_impl.h @@ -120,13 +120,17 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { ConnectionSocketImpl(IoHandlePtr&& io_handle, const Address::InstanceConstSharedPtr& local_address, const Address::InstanceConstSharedPtr& remote_address) - : SocketImpl(std::move(io_handle), local_address), remote_address_(remote_address) {} + : SocketImpl(std::move(io_handle), local_address), remote_address_(remote_address), + direct_remote_address_(remote_address) {} // Network::Socket Address::SocketType socketType() const override { return Address::SocketType::Stream; } // Network::ConnectionSocket const Address::InstanceConstSharedPtr& remoteAddress() const override { return remote_address_; } + const Address::InstanceConstSharedPtr& directRemoteAddress() const override { + return direct_remote_address_; + } void restoreLocalAddress(const Address::InstanceConstSharedPtr& local_address) override { setLocalAddress(local_address); local_address_restored_ = true; @@ -158,6 +162,7 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket { protected: Address::InstanceConstSharedPtr remote_address_; + const Address::InstanceConstSharedPtr direct_remote_address_; bool local_address_restored_{false}; std::string transport_protocol_; std::vector application_protocols_; diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc index 62182d0ebe9c2..ded1b0ba5e8fa 100644 --- a/source/common/tcp_proxy/tcp_proxy.cc +++ b/source/common/tcp_proxy/tcp_proxy.cc @@ -255,6 +255,8 @@ void Filter::initialize(Network::ReadFilterCallbacks& callbacks, bool set_connec read_callbacks_->connection().enableHalfClose(true); getStreamInfo().setDownstreamLocalAddress(read_callbacks_->connection().localAddress()); getStreamInfo().setDownstreamRemoteAddress(read_callbacks_->connection().remoteAddress()); + getStreamInfo().setDownstreamDirectRemoteAddress( + read_callbacks_->connection().directRemoteAddress()); getStreamInfo().setDownstreamSslConnection(read_callbacks_->connection().ssl()); // Need to disable reads so that we don't write to an upstream that might fail diff --git a/source/extensions/filters/network/dubbo_proxy/active_message.cc b/source/extensions/filters/network/dubbo_proxy/active_message.cc index e672ef3437166..8af50551f6165 100644 --- a/source/extensions/filters/network/dubbo_proxy/active_message.cc +++ b/source/extensions/filters/network/dubbo_proxy/active_message.cc @@ -192,6 +192,7 @@ ActiveMessage::ActiveMessage(ConnectionManager& parent) parent_.stats().request_active_.inc(); stream_info_.setDownstreamLocalAddress(parent_.connection().localAddress()); stream_info_.setDownstreamRemoteAddress(parent_.connection().remoteAddress()); + stream_info_.setDownstreamDirectRemoteAddress(parent_.connection().directRemoteAddress()); } ActiveMessage::~ActiveMessage() { diff --git a/source/extensions/filters/network/thrift_proxy/conn_manager.h b/source/extensions/filters/network/thrift_proxy/conn_manager.h index 54331d020b385..7bbf35710ced4 100644 --- a/source/extensions/filters/network/thrift_proxy/conn_manager.h +++ b/source/extensions/filters/network/thrift_proxy/conn_manager.h @@ -164,6 +164,8 @@ class ConnectionManager : public Network::ReadFilter, stream_info_.setDownstreamLocalAddress(parent_.read_callbacks_->connection().localAddress()); stream_info_.setDownstreamRemoteAddress( parent_.read_callbacks_->connection().remoteAddress()); + stream_info_.setDownstreamDirectRemoteAddress( + parent_.read_callbacks_->connection().directRemoteAddress()); } ~ActiveRpc() override { request_timer_->complete(); diff --git a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.cc b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.cc index 8a871fcf186a3..270c7eec91fff 100644 --- a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.cc +++ b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.cc @@ -105,6 +105,13 @@ QuicFilterManagerConnectionImpl::remoteAddress() const { return quic_connection_->connectionSocket()->remoteAddress(); } +const Network::Address::InstanceConstSharedPtr& +QuicFilterManagerConnectionImpl::directRemoteAddress() const { + ASSERT(quic_connection_->connectionSocket() != nullptr, + "directRemoteAddress() should only be called after OnPacketHeader"); + return quic_connection_->connectionSocket()->directRemoteAddress(); +} + const Network::Address::InstanceConstSharedPtr& QuicFilterManagerConnectionImpl::localAddress() const { ASSERT(quic_connection_->connectionSocket() != nullptr, diff --git a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h index 9ba0282bf2394..1afcd7021a911 100644 --- a/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h +++ b/source/extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h @@ -44,6 +44,7 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase { void detectEarlyCloseWhenReadDisabled(bool /*value*/) override { NOT_REACHED_GCOVR_EXCL_LINE; } bool readEnabled() const override { return true; } const Network::Address::InstanceConstSharedPtr& remoteAddress() const override; + const Network::Address::InstanceConstSharedPtr& directRemoteAddress() const override; const Network::Address::InstanceConstSharedPtr& localAddress() const override; absl::optional unixSocketPeerCredentials() const override { diff --git a/source/server/api_listener_impl.h b/source/server/api_listener_impl.h index 9227f21810860..bf806341bed08 100644 --- a/source/server/api_listener_impl.h +++ b/source/server/api_listener_impl.h @@ -106,6 +106,9 @@ class ApiListenerImplBase : public ApiListener, const Network::Address::InstanceConstSharedPtr& remoteAddress() const override { return parent_.parent_.address(); } + const Network::Address::InstanceConstSharedPtr& directRemoteAddress() const override { + return parent_.parent_.address(); + } absl::optional unixSocketPeerCredentials() const override { return absl::nullopt; diff --git a/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc b/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc index 45506b58dadc4..ac788113cff02 100644 --- a/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc @@ -98,6 +98,7 @@ class TcpGrpcAccessLogIntegrationTest : public Grpc::GrpcClientIntegrationParamT // Clear fields which are not deterministic. auto* log_entry = request_msg.mutable_tcp_logs()->mutable_log_entry(0); clearPort(*log_entry->mutable_common_properties()->mutable_downstream_remote_address()); + clearPort(*log_entry->mutable_common_properties()->mutable_downstream_direct_remote_address()); clearPort(*log_entry->mutable_common_properties()->mutable_downstream_local_address()); clearPort(*log_entry->mutable_common_properties()->mutable_upstream_remote_address()); clearPort(*log_entry->mutable_common_properties()->mutable_upstream_local_address()); @@ -181,6 +182,9 @@ TEST_P(TcpGrpcAccessLogIntegrationTest, BasicAccessLogFlow) { socket_address: address: {} upstream_cluster: cluster_0 + downstream_direct_remote_address: + socket_address: + address: {} connection_properties: received_bytes: 3 sent_bytes: 5 @@ -188,6 +192,7 @@ TEST_P(TcpGrpcAccessLogIntegrationTest, BasicAccessLogFlow) { VersionInfo::version(), Network::Test::getLoopbackAddressString(ipVersion()), Network::Test::getLoopbackAddressString(ipVersion()), Network::Test::getLoopbackAddressString(ipVersion()), + Network::Test::getLoopbackAddressString(ipVersion()), Network::Test::getLoopbackAddressString(ipVersion())))); cleanup(); diff --git a/test/integration/proxy_proto_integration_test.cc b/test/integration/proxy_proto_integration_test.cc index fd17bed295d58..c10323e75737c 100644 --- a/test/integration/proxy_proto_integration_test.cc +++ b/test/integration/proxy_proto_integration_test.cc @@ -92,6 +92,33 @@ TEST_P(ProxyProtoIntegrationTest, RouterProxyUnknownLongRequestAndResponseWithBo testRouterRequestAndResponseWithBody(1024, 512, false, &creator); } +// Test that %DOWNSTREAM_DIRECT_REMOTE_ADDRESS%/%DOWNSTREAM_DIRECT_REMOTE_ADDRESS_WITHOUT_PORT% +// returns the direct address, and %DOWSTREAM_REMOTE_ADDRESS% returns the proxy-protocol-provided +// address. +TEST_P(ProxyProtoIntegrationTest, AccessLog) { + useAccessLog("%DOWNSTREAM_DIRECT_REMOTE_ADDRESS_WITHOUT_PORT% %DOWNSTREAM_REMOTE_ADDRESS%"); + + // Tell HCM to ignore x-forwarded-for so that the proxy-proto address is used. + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { hcm.mutable_use_remote_address()->set_value(true); }); + + ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr { + Network::ClientConnectionPtr conn = makeClientConnection(lookupPort("http")); + Buffer::OwnedImpl buf("PROXY TCP4 1.2.3.4 254.254.254.254 12345 1234\r\n"); + conn->write(buf, false); + return conn; + }; + + testRouterRequestAndResponseWithBody(1024, 512, false, &creator); + const std::string log_line = waitForAccessLog(access_log_name_); + const std::vector tokens = StringUtil::splitToken(log_line, " "); + + ASSERT_EQ(2, tokens.size()); + EXPECT_EQ(tokens[0], Network::Test::getLoopbackAddressString(GetParam())); + EXPECT_EQ(tokens[1], "1.2.3.4:12345"); +} + TEST_P(ProxyProtoIntegrationTest, DEPRECATED_FEATURE_TEST(OriginalDst)) { // Change the cluster to an original destination cluster. An original destination cluster // ignores the configured hosts, and instead uses the restored destination address from the diff --git a/test/mocks/network/connection.cc b/test/mocks/network/connection.cc index 915eb0e86c48a..4c46f6af97de7 100644 --- a/test/mocks/network/connection.cc +++ b/test/mocks/network/connection.cc @@ -62,6 +62,7 @@ template static void initializeMockConnection(T& connection) { connection.raiseEvent(Network::ConnectionEvent::LocalClose); })); ON_CALL(connection, remoteAddress()).WillByDefault(ReturnRef(connection.remote_address_)); + ON_CALL(connection, directRemoteAddress()).WillByDefault(ReturnRef(connection.remote_address_)); ON_CALL(connection, localAddress()).WillByDefault(ReturnRef(connection.local_address_)); ON_CALL(connection, id()).WillByDefault(Return(connection.next_id_)); ON_CALL(connection, state()).WillByDefault(ReturnPointee(&connection.state_)); diff --git a/test/mocks/network/connection.h b/test/mocks/network/connection.h index 18d660339831d..b66d9525d2686 100644 --- a/test/mocks/network/connection.h +++ b/test/mocks/network/connection.h @@ -67,6 +67,7 @@ class MockConnection : public Connection, public MockConnectionBase { MOCK_METHOD(void, detectEarlyCloseWhenReadDisabled, (bool)); MOCK_METHOD(bool, readEnabled, (), (const)); MOCK_METHOD(const Address::InstanceConstSharedPtr&, remoteAddress, (), (const)); + MOCK_METHOD(const Address::InstanceConstSharedPtr&, directRemoteAddress, (), (const)); MOCK_METHOD(absl::optional, unixSocketPeerCredentials, (), (const)); MOCK_METHOD(const Address::InstanceConstSharedPtr&, localAddress, (), (const)); @@ -112,6 +113,7 @@ class MockClientConnection : public ClientConnection, public MockConnectionBase MOCK_METHOD(void, detectEarlyCloseWhenReadDisabled, (bool)); MOCK_METHOD(bool, readEnabled, (), (const)); MOCK_METHOD(const Address::InstanceConstSharedPtr&, remoteAddress, (), (const)); + MOCK_METHOD(const Address::InstanceConstSharedPtr&, directRemoteAddress, (), (const)); MOCK_METHOD(absl::optional, unixSocketPeerCredentials, (), (const)); MOCK_METHOD(const Address::InstanceConstSharedPtr&, localAddress, (), (const)); @@ -160,6 +162,7 @@ class MockFilterManagerConnection : public FilterManagerConnection, public MockC MOCK_METHOD(void, detectEarlyCloseWhenReadDisabled, (bool)); MOCK_METHOD(bool, readEnabled, (), (const)); MOCK_METHOD(const Address::InstanceConstSharedPtr&, remoteAddress, (), (const)); + MOCK_METHOD(const Address::InstanceConstSharedPtr&, directRemoteAddress, (), (const)); MOCK_METHOD(absl::optional, unixSocketPeerCredentials, (), (const)); MOCK_METHOD(const Address::InstanceConstSharedPtr&, localAddress, (), (const)); diff --git a/test/mocks/network/mocks.cc b/test/mocks/network/mocks.cc index fd20cb5e5b11b..b0a47496cd11b 100644 --- a/test/mocks/network/mocks.cc +++ b/test/mocks/network/mocks.cc @@ -149,6 +149,7 @@ MockConnectionSocket::MockConnectionSocket() remote_address_(new Address::Ipv4Instance(80)) { ON_CALL(*this, localAddress()).WillByDefault(ReturnRef(local_address_)); ON_CALL(*this, remoteAddress()).WillByDefault(ReturnRef(remote_address_)); + ON_CALL(*this, directRemoteAddress()).WillByDefault(ReturnRef(remote_address_)); ON_CALL(*this, ioHandle()).WillByDefault(ReturnRef(*io_handle_)); ON_CALL(testing::Const(*this), ioHandle()).WillByDefault(ReturnRef(*io_handle_)); } diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 5e408d575c93e..131b6d220b02b 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -253,6 +253,7 @@ class MockConnectionSocket : public ConnectionSocket { MOCK_METHOD(bool, localAddressRestored, (), (const)); MOCK_METHOD(void, setRemoteAddress, (const Address::InstanceConstSharedPtr&)); MOCK_METHOD(const Address::InstanceConstSharedPtr&, remoteAddress, (), (const)); + MOCK_METHOD(const Address::InstanceConstSharedPtr&, directRemoteAddress, (), (const)); MOCK_METHOD(void, setDetectedTransportProtocol, (absl::string_view)); MOCK_METHOD(absl::string_view, detectedTransportProtocol, (), (const)); MOCK_METHOD(void, setRequestedApplicationProtocols, (const std::vector&)); diff --git a/test/server/filter_chain_benchmark_test.cc b/test/server/filter_chain_benchmark_test.cc index 19bd85d86f3a2..04e644c076558 100644 --- a/test/server/filter_chain_benchmark_test.cc +++ b/test/server/filter_chain_benchmark_test.cc @@ -67,6 +67,10 @@ class MockConnectionSocket : public Network::ConnectionSocket { return remote_address_; } + const Network::Address::InstanceConstSharedPtr& directRemoteAddress() const override { + return remote_address_; + } + const Network::Address::InstanceConstSharedPtr& localAddress() const override { return local_address_; }