diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index bfee20b0dfe2c..bf6c1dbedd10f 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -4,3 +4,4 @@ Changes ------- * listener: fix crash when disabling or re-enabling listeners due to overload while processing LDS updates. +* proxy_proto: fixed a bug where network filters would not have the correct downstreamRemoteAddress() when accessed from the StreamInfo. This could result in incorrect enforcement of RBAC rules in the RBAC network filter (but not in the RBAC HTTP filter), or incorrect access log addresses from tcp_proxy. diff --git a/test/integration/BUILD b/test/integration/BUILD index 82695f39c4d3f..d60c17f3c091a 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -837,10 +837,14 @@ envoy_cc_test( ":http_integration_lib", "//source/common/buffer:buffer_lib", "//source/common/http:codec_client_lib", + "//source/extensions/access_loggers/file:config", "//source/extensions/filters/listener/proxy_protocol:config", + "//source/extensions/filters/network/tcp_proxy:config", "//test/test_common:utility_lib", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", + "@envoy_api//envoy/config/filter/network/tcp_proxy/v2:pkg_cc_proto", + "@envoy_api//envoy/extensions/access_loggers/file/v3:pkg_cc_proto", ], ) diff --git a/test/integration/proxy_proto_integration_test.cc b/test/integration/proxy_proto_integration_test.cc index 92c4bc90b39d6..287e7542fc1ec 100644 --- a/test/integration/proxy_proto_integration_test.cc +++ b/test/integration/proxy_proto_integration_test.cc @@ -2,6 +2,8 @@ #include "envoy/config/bootstrap/v3/bootstrap.pb.h" #include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.pb.h" +#include "envoy/extensions/access_loggers/file/v3/file.pb.h" #include "common/buffer/buffer_impl.h" @@ -14,6 +16,24 @@ namespace Envoy { +static void +insertProxyProtocolFilterConfigModifier(envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + ::envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proxy_protocol; + auto rule = proxy_protocol.add_rules(); + rule->set_tlv_type(0x02); + rule->mutable_on_tlv_present()->set_key("PP2TypeAuthority"); + + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + auto* ppv_filter = listener->add_listener_filters(); + ppv_filter->set_name("envoy.listener.proxy_protocol"); + ppv_filter->mutable_typed_config()->PackFrom(proxy_protocol); +} + +ProxyProtoIntegrationTest::ProxyProtoIntegrationTest() + : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam()) { + config_helper_.addConfigModifier(insertProxyProtocolFilterConfigModifier); +} + INSTANTIATE_TEST_SUITE_P(IpVersions, ProxyProtoIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); @@ -201,4 +221,58 @@ TEST_P(ProxyProtoIntegrationTest, ClusterProvided) { testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator); } +ProxyProtoTcpIntegrationTest::ProxyProtoTcpIntegrationTest() + : BaseIntegrationTest(GetParam(), ConfigHelper::tcpProxyConfig()) { + config_helper_.addConfigModifier(insertProxyProtocolFilterConfigModifier); + config_helper_.renameListener("tcp_proxy"); +} + +INSTANTIATE_TEST_SUITE_P(IpVersions, ProxyProtoTcpIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +// This tests that the StreamInfo contains the correct addresses. +TEST_P(ProxyProtoTcpIntegrationTest, AccessLog) { + std::string access_log_path = TestEnvironment::temporaryPath( + fmt::format("access_log{}.txt", version_ == Network::Address::IpVersion::v4 ? "v4" : "v6")); + config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + auto* filter_chain = listener->mutable_filter_chains(0); + auto* config_blob = filter_chain->mutable_filters(0)->mutable_typed_config(); + + ASSERT_TRUE( + config_blob->Is()); + auto tcp_proxy_config = MessageUtil::anyConvert(*config_blob); + + auto* access_log = tcp_proxy_config.add_access_log(); + access_log->set_name("accesslog"); + envoy::extensions::access_loggers::file::v3::FileAccessLog access_log_config; + access_log_config.set_path(access_log_path); + access_log_config.mutable_log_format()->set_text_format( + "remote=%DOWNSTREAM_REMOTE_ADDRESS% local=%DOWNSTREAM_LOCAL_ADDRESS%"); + access_log->mutable_typed_config()->PackFrom(access_log_config); + config_blob->PackFrom(tcp_proxy_config); + }); + initialize(); + + IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy")); + ASSERT_TRUE(tcp_client->write("PROXY TCP4 1.2.3.4 254.254.254.254 12345 1234\r\nhello", false)); + + FakeRawConnectionPtr fake_upstream_connection; + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection)); + ASSERT_TRUE(fake_upstream_connection->waitForData(5)); + ASSERT_TRUE(fake_upstream_connection->close()); + tcp_client->close(); + ASSERT_TRUE(fake_upstream_connection->waitForDisconnect()); + + std::string log_result; + // Access logs only get flushed to disk periodically, so poll until the log is non-empty + do { + log_result = api_->fileSystem().fileReadToEnd(access_log_path); + } while (log_result.empty()); + + EXPECT_EQ(log_result, "remote=1.2.3.4:12345 local=254.254.254.254:1234"); +} + } // namespace Envoy diff --git a/test/integration/proxy_proto_integration_test.h b/test/integration/proxy_proto_integration_test.h index 140d5b63d3f70..3a719ad44aaf0 100644 --- a/test/integration/proxy_proto_integration_test.h +++ b/test/integration/proxy_proto_integration_test.h @@ -16,19 +16,13 @@ namespace Envoy { class ProxyProtoIntegrationTest : public testing::TestWithParam, public HttpIntegrationTest { public: - ProxyProtoIntegrationTest() : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam()) { - config_helper_.addConfigModifier( - [&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { - ::envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proxy_protocol; - auto rule = proxy_protocol.add_rules(); - rule->set_tlv_type(0x02); - rule->mutable_on_tlv_present()->set_key("PP2TypeAuthority"); - - auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); - auto* ppv_filter = listener->add_listener_filters(); - ppv_filter->set_name("envoy.listener.proxy_protocol"); - ppv_filter->mutable_typed_config()->PackFrom(proxy_protocol); - }); - } + ProxyProtoIntegrationTest(); }; + +class ProxyProtoTcpIntegrationTest : public testing::TestWithParam, + public BaseIntegrationTest { +public: + ProxyProtoTcpIntegrationTest(); +}; + } // namespace Envoy