From 090e73d82462f46d32c112554206a5ed0861eba2 Mon Sep 17 00:00:00 2001 From: Florent Lecoultre <104759954+fl0Lec@users.noreply.github.com> Date: Mon, 18 Nov 2024 18:48:26 +0100 Subject: [PATCH] feature: make always accessible the original downstream local address (#36920) Signed-off-by: Florent Lecoultre --- .../observability/access_log/usage.rst | 38 ++++++++++- envoy/network/socket.h | 6 ++ .../common/formatter/stream_info_formatter.cc | 26 ++++++++ source/common/http/filter_manager.h | 3 + source/common/network/socket_impl.h | 8 ++- .../extensions/filters/http/lua/wrappers.cc | 7 ++ source/extensions/filters/http/lua/wrappers.h | 8 +++ .../formatter/substitution_formatter_test.cc | 66 +++++++++++++++++-- test/common/router/header_formatter_test.cc | 3 + .../filters/http/lua/lua_integration_test.cc | 9 +++ 10 files changed, 163 insertions(+), 11 deletions(-) diff --git a/docs/root/configuration/observability/access_log/usage.rst b/docs/root/configuration/observability/access_log/usage.rst index 90ed759fc93b..13aaeeb9eab7 100644 --- a/docs/root/configuration/observability/access_log/usage.rst +++ b/docs/root/configuration/observability/access_log/usage.rst @@ -735,17 +735,51 @@ UDP .. note:: This may not be the physical remote address of the peer if the address has been inferred from - :ref:`Proxy Protocol filter ` or :ref:`x-forwarded-for - `. + :ref:`Proxy Protocol filter `. + +%DOWNSTREAM_DIRECT_LOCAL_ADDRESS% + Direct local address of the downstream connection. + + .. note:: + + This is always the physical local address even if the downstream remote address has been inferred from + :ref:`Proxy Protocol filter `. %DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT% Local address of the downstream connection, without any port component. IP addresses are the only address type with a port component. + .. note:: + + This may not be the physical local address if the downstream local address has been inferred from + :ref:`Proxy Protocol filter `. + +%DOWNSTREAM_DIRECT_LOCAL_ADDRESS_WITHOUT_PORT% + Direct local address of the downstream connection, without any port component. + + .. note:: + + This is always the physical local address even if the downstream local address has been inferred from + :ref:`Proxy Protocol filter `. + %DOWNSTREAM_LOCAL_PORT% Local port of the downstream connection. IP addresses are the only address type with a port component. + .. note:: + + This may not be the physical port if the downstream local address has been inferred from + :ref:`Proxy Protocol filter `. + +%DOWNSTREAM_DIRECT_LOCAL_PORT% + Direct local port of the downstream connection. + IP addresses are the only address type with a port component. + + .. note:: + + This is always the listener port even if the downstream local address has been inferred from + :ref:`Proxy Protocol filter `. + .. _config_access_log_format_connection_id: %CONNECTION_ID% diff --git a/envoy/network/socket.h b/envoy/network/socket.h index bed7b7be35aa..db00010cfa5b 100644 --- a/envoy/network/socket.h +++ b/envoy/network/socket.h @@ -225,6 +225,12 @@ class ConnectionInfoProvider { */ virtual const Address::InstanceConstSharedPtr& localAddress() const PURE; + /** + * @return the direct local address of the socket. This is the listener address and it can not be + * modified by listener filters. + */ + virtual const Address::InstanceConstSharedPtr& directLocalAddress() const PURE; + /** * @return true if the local address has been restored to a value that is different from the * address the socket was initially accepted at. diff --git a/source/common/formatter/stream_info_formatter.cc b/source/common/formatter/stream_info_formatter.cc index b4c3aa35414c..b071119b699d 100644 --- a/source/common/formatter/stream_info_formatter.cc +++ b/source/common/formatter/stream_info_formatter.cc @@ -823,6 +823,7 @@ using StreamInfoFormatterProviderLookupTable = absl::flat_hash_map>; +// clang-format off const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProviders() { CONSTRUCT_ON_FIRST_USE( StreamInfoFormatterProviderLookupTable, @@ -1298,6 +1299,14 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide return stream_info.downstreamAddressProvider().localAddress(); }); }}}, + {"DOWNSTREAM_DIRECT_LOCAL_ADDRESS", + {CommandSyntaxChecker::COMMAND_ONLY, + [](absl::string_view, absl::optional) { + return StreamInfoAddressFormatterProvider::withPort( + [](const StreamInfo::StreamInfo& stream_info) { + return stream_info.downstreamAddressProvider().directLocalAddress(); + }); + }}}, {"DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT", {CommandSyntaxChecker::COMMAND_ONLY, [](absl::string_view, absl::optional) { @@ -1306,6 +1315,14 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide return stream_info.downstreamAddressProvider().localAddress(); }); }}}, + {"DOWNSTREAM_DIRECT_LOCAL_ADDRESS_WITHOUT_PORT", + {CommandSyntaxChecker::COMMAND_ONLY, + [](absl::string_view, absl::optional) { + return StreamInfoAddressFormatterProvider::withoutPort( + [](const Envoy::StreamInfo::StreamInfo& stream_info) { + return stream_info.downstreamAddressProvider().directLocalAddress(); + }); + }}}, {"DOWNSTREAM_LOCAL_PORT", {CommandSyntaxChecker::COMMAND_ONLY, [](absl::string_view, absl::optional) { @@ -1314,6 +1331,14 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide return stream_info.downstreamAddressProvider().localAddress(); }); }}}, + {"DOWNSTREAM_DIRECT_LOCAL_PORT", + {CommandSyntaxChecker::COMMAND_ONLY, + [](absl::string_view, absl::optional) { + return StreamInfoAddressFormatterProvider::justPort( + [](const Envoy::StreamInfo::StreamInfo& stream_info) { + return stream_info.downstreamAddressProvider().directLocalAddress(); + }); + }}}, {"DOWNSTREAM_REMOTE_ADDRESS", {CommandSyntaxChecker::COMMAND_ONLY, [](absl::string_view, absl::optional) { @@ -1861,6 +1886,7 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide }}}, }); } +// clang-format on class BuiltInStreamInfoCommandParser : public StreamInfoCommandParser { public: diff --git a/source/common/http/filter_manager.h b/source/common/http/filter_manager.h index 81bc003e86ab..36e240084d36 100644 --- a/source/common/http/filter_manager.h +++ b/source/common/http/filter_manager.h @@ -579,6 +579,9 @@ class OverridableRemoteConnectionInfoSetterStreamInfo : public StreamInfo::Strea const Network::Address::InstanceConstSharedPtr& localAddress() const override { return StreamInfoImpl::downstreamAddressProvider().localAddress(); } + const Network::Address::InstanceConstSharedPtr& directLocalAddress() const override { + return StreamInfoImpl::downstreamAddressProvider().directLocalAddress(); + } bool localAddressRestored() const override { return StreamInfoImpl::downstreamAddressProvider().localAddressRestored(); } diff --git a/source/common/network/socket_impl.h b/source/common/network/socket_impl.h index 1121cebb1fe2..17837b400be6 100644 --- a/source/common/network/socket_impl.h +++ b/source/common/network/socket_impl.h @@ -14,8 +14,8 @@ class ConnectionInfoSetterImpl : public ConnectionInfoSetter { public: ConnectionInfoSetterImpl(const Address::InstanceConstSharedPtr& local_address, const Address::InstanceConstSharedPtr& remote_address) - : local_address_(local_address), remote_address_(remote_address), - direct_remote_address_(remote_address) {} + : local_address_(local_address), direct_local_address_(local_address), + remote_address_(remote_address), direct_remote_address_(remote_address) {} void setDirectRemoteAddressForTest(const Address::InstanceConstSharedPtr& direct_remote_address) { direct_remote_address_ = direct_remote_address; @@ -32,6 +32,9 @@ class ConnectionInfoSetterImpl : public ConnectionInfoSetter { // ConnectionInfoSetter const Address::InstanceConstSharedPtr& localAddress() const override { return local_address_; } + const Address::InstanceConstSharedPtr& directLocalAddress() const override { + return direct_local_address_; + } void setLocalAddress(const Address::InstanceConstSharedPtr& local_address) override { local_address_ = local_address; } @@ -89,6 +92,7 @@ class ConnectionInfoSetterImpl : public ConnectionInfoSetter { private: Address::InstanceConstSharedPtr local_address_; + Address::InstanceConstSharedPtr direct_local_address_; bool local_address_restored_{false}; Address::InstanceConstSharedPtr remote_address_; Address::InstanceConstSharedPtr direct_remote_address_; diff --git a/source/extensions/filters/http/lua/wrappers.cc b/source/extensions/filters/http/lua/wrappers.cc index 2824dcd82676..dc3cd331ec1d 100644 --- a/source/extensions/filters/http/lua/wrappers.cc +++ b/source/extensions/filters/http/lua/wrappers.cc @@ -202,6 +202,13 @@ int StreamInfoWrapper::luaDownstreamLocalAddress(lua_State* state) { return 1; } +int StreamInfoWrapper::luaDownstreamDirectLocalAddress(lua_State* state) { + const std::string& local_address = + stream_info_.downstreamAddressProvider().directLocalAddress()->asString(); + lua_pushlstring(state, local_address.data(), local_address.size()); + return 1; +} + int StreamInfoWrapper::luaDownstreamDirectRemoteAddress(lua_State* state) { const std::string& direct_remote_address = stream_info_.downstreamAddressProvider().directRemoteAddress()->asString(); diff --git a/source/extensions/filters/http/lua/wrappers.h b/source/extensions/filters/http/lua/wrappers.h index 2bf474caa7b5..0e2ca9abbadb 100644 --- a/source/extensions/filters/http/lua/wrappers.h +++ b/source/extensions/filters/http/lua/wrappers.h @@ -269,6 +269,7 @@ class StreamInfoWrapper : public Filters::Common::Lua::BaseLuaObjectlocalAddress(); + stream_info.downstream_connection_info_provider_->setLocalAddress(address); + EXPECT_EQ("127.0.0.2:0", format.formatWithContext({}, stream_info)); + EXPECT_THAT(format.formatValueWithContext({}, stream_info), + ProtoEq(ValueUtil::stringValue("127.0.0.2:0"))); + stream_info.downstream_connection_info_provider_->setLocalAddress(original_address); + } + { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT"); EXPECT_EQ("127.0.0.2", upstream_format.formatWithContext({}, stream_info)); @@ -871,31 +883,71 @@ TEST(SubstitutionFormatterTest, streamInfoFormatter) { } { - StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_PORT"); + StreamInfoFormatter format("DOWNSTREAM_DIRECT_LOCAL_ADDRESS_WITHOUT_PORT"); + EXPECT_EQ("127.0.0.2", format.formatWithContext({}, stream_info)); + EXPECT_THAT(format.formatValueWithContext({}, stream_info), + ProtoEq(ValueUtil::stringValue("127.0.0.2"))); + } + + { + StreamInfoFormatter format("DOWNSTREAM_DIRECT_LOCAL_ADDRESS_WITHOUT_PORT"); + auto address = Network::Address::InstanceConstSharedPtr{ + new Network::Address::Ipv4Instance("127.1.2.3", 8900)}; + stream_info.downstream_connection_info_provider_->setLocalAddress(address); + EXPECT_EQ("127.0.0.2", format.formatWithContext({}, stream_info)); + EXPECT_THAT(format.formatValueWithContext({}, stream_info), + ProtoEq(ValueUtil::stringValue("127.0.0.2"))); + } + + { + StreamInfoFormatter format("DOWNSTREAM_DIRECT_LOCAL_PORT"); + EXPECT_EQ("0", format.formatWithContext({}, stream_info)); + EXPECT_THAT(format.formatValueWithContext({}, stream_info), ProtoEq(ValueUtil::numberValue(0))); + } + + { + StreamInfoFormatter downstream_local_port_format("DOWNSTREAM_LOCAL_PORT"), + downstream_direct_downstream_local_port_format("DOWNSTREAM_DIRECT_LOCAL_PORT"); // Validate for IPv4 address auto address = Network::Address::InstanceConstSharedPtr{ new Network::Address::Ipv4Instance("127.1.2.3", 8443)}; stream_info.downstream_connection_info_provider_->setLocalAddress(address); - EXPECT_EQ("8443", upstream_format.formatWithContext({}, stream_info)); - EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info), + EXPECT_EQ("8443", downstream_local_port_format.formatWithContext({}, stream_info)); + EXPECT_THAT(downstream_local_port_format.formatValueWithContext({}, stream_info), ProtoEq(ValueUtil::numberValue(8443))); + EXPECT_EQ("0", + downstream_direct_downstream_local_port_format.formatWithContext({}, stream_info)); + EXPECT_THAT( + downstream_direct_downstream_local_port_format.formatValueWithContext({}, stream_info), + ProtoEq(ValueUtil::numberValue(0))); + // Validate for IPv6 address address = Network::Address::InstanceConstSharedPtr{new Network::Address::Ipv6Instance("::1", 9443)}; stream_info.downstream_connection_info_provider_->setLocalAddress(address); - EXPECT_EQ("9443", upstream_format.formatWithContext({}, stream_info)); - EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info), + EXPECT_EQ("9443", downstream_local_port_format.formatWithContext({}, stream_info)); + EXPECT_THAT(downstream_local_port_format.formatValueWithContext({}, stream_info), ProtoEq(ValueUtil::numberValue(9443))); + EXPECT_EQ("0", + downstream_direct_downstream_local_port_format.formatWithContext({}, stream_info)); + EXPECT_THAT( + downstream_direct_downstream_local_port_format.formatValueWithContext({}, stream_info), + ProtoEq(ValueUtil::numberValue(0))); // Validate for Pipe address = Network::Address::InstanceConstSharedPtr{*Network::Address::PipeInstance::create("/foo")}; stream_info.downstream_connection_info_provider_->setLocalAddress(address); - EXPECT_EQ("", upstream_format.formatWithContext({}, stream_info)); - EXPECT_THAT(upstream_format.formatValueWithContext({}, stream_info), + EXPECT_EQ("", downstream_local_port_format.formatWithContext({}, stream_info)); + EXPECT_THAT(downstream_local_port_format.formatValueWithContext({}, stream_info), ProtoEq(ValueUtil::nullValue())); + EXPECT_EQ("0", + downstream_direct_downstream_local_port_format.formatWithContext({}, stream_info)); + EXPECT_THAT( + downstream_direct_downstream_local_port_format.formatValueWithContext({}, stream_info), + ProtoEq(ValueUtil::numberValue(0))); } { diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index 0e27b66d6939..44edc3cd2825 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -70,6 +70,9 @@ TEST(HeaderParserTest, TestParse) { {"%DOWNSTREAM_LOCAL_ADDRESS%", {"127.0.0.2:0"}, {}}, {"%DOWNSTREAM_LOCAL_PORT%", {"0"}, {}}, {"%DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT%", {"127.0.0.2"}, {}}, + {"%DOWNSTREAM_DIRECT_LOCAL_ADDRESS%", {"127.0.0.2:0"}, {}}, + {"%DOWNSTREAM_DIRECT_LOCAL_PORT%", {"0"}, {}}, + {"%DOWNSTREAM_DIRECT_LOCAL_ADDRESS_WITHOUT_PORT%", {"127.0.0.2"}, {}}, {"%UPSTREAM_METADATA([\"ns\", \"key\"])%", {"value"}, {}}, {"[%UPSTREAM_METADATA([\"ns\", \"key\"])%", {"[value"}, {}}, {"%UPSTREAM_METADATA([\"ns\", \"key\"])%]", {"value]"}, {}}, diff --git a/test/extensions/filters/http/lua/lua_integration_test.cc b/test/extensions/filters/http/lua/lua_integration_test.cc index 970d1cd81112..66922e83044e 100644 --- a/test/extensions/filters/http/lua/lua_integration_test.cc +++ b/test/extensions/filters/http/lua/lua_integration_test.cc @@ -307,6 +307,8 @@ name: lua end request_handle:headers():add("request_protocol", request_handle:streamInfo():protocol()) request_handle:headers():add("request_dynamic_metadata_value", dynamic_metadata_value) + request_handle:headers():add("request_downstream_direct_local_address_value", + request_handle:streamInfo():downstreamDirectLocalAddress()) request_handle:headers():add("request_downstream_local_address_value", request_handle:streamInfo():downstreamLocalAddress()) request_handle:headers():add("request_downstream_directremote_address_value", @@ -408,6 +410,13 @@ name: lua ->value() .getStringView()); + EXPECT_TRUE(absl::StrContains( + upstream_request_->headers() + .get(Http::LowerCaseString("request_downstream_direct_local_address_value"))[0] + ->value() + .getStringView(), + GetParam() == Network::Address::IpVersion::v4 ? "127.0.0.1:" : "[::1]:")); + EXPECT_TRUE( absl::StrContains(upstream_request_->headers() .get(Http::LowerCaseString("request_downstream_local_address_value"))[0]