Skip to content

Commit

Permalink
Proxy protocol: sanitise non utf8 chars in TLVs
Browse files Browse the repository at this point in the history
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
  • Loading branch information
Kateryna Nezdolii authored and lgadban committed Jan 27, 2024
1 parent 3986fc8 commit 3e6e83d
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 1 deletion.
4 changes: 4 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ bug_fixes:
change: |
Fix crash due to uncaught exception when the operating system does not support an address type (such as IPv6) that is
received in a proxy protocol header. Connections will instead be dropped/reset.
- area: proxy_protocol
change: |
Fixed a bug where TLVs with non utf8 characters were inserted as protobuf values into filter metadata circumventing
ext_authz checks when ``failure_mode_allow`` is set to ``true``.
- area: tls
change: |
Fix crash due to uncaught exception when the operating system does not support an address type (such as IPv6) that is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "source/common/network/address_impl.h"
#include "source/common/network/proxy_protocol_filter_state.h"
#include "source/common/network/utility.h"
#include "source/common/protobuf/utility.h"
#include "source/extensions/common/proxy_protocol/proxy_protocol_header.h"

using envoy::config::core::v3::ProxyProtocolPassThroughTLVs;
Expand Down Expand Up @@ -440,7 +441,9 @@ bool Filter::parseTlvs(const uint8_t* buf, size_t len) {
auto key_value_pair = config_->isTlvTypeNeeded(tlv_type);
if (nullptr != key_value_pair) {
ProtobufWkt::Value metadata_value;
metadata_value.set_string_value(tlv_value.data(), tlv_value.size());
// Sanitize any non utf8 characters.
auto sanitised_tlv_value = MessageUtil::sanitizeUtf8String(tlv_value);
metadata_value.set_string_value(sanitised_tlv_value.data(), sanitised_tlv_value.size());

std::string metadata_key = key_value_pair->metadata_namespace().empty()
? "envoy.filters.listener.proxy_protocol"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1583,6 +1583,70 @@ TEST_P(ProxyProtocolTest, V2ExtractMultipleTlvsOfInterest) {
disconnect();
}

TEST_P(ProxyProtocolTest, V2ExtractMultipleTlvsOfInterestAndSanitiseNonUtf8) {
// A well-formed ipv4/tcp with a pair of TLV extensions is accepted.
constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49,
0x54, 0x0a, 0x21, 0x11, 0x00, 0x39, 0x01, 0x02, 0x03, 0x04,
0x00, 0x01, 0x01, 0x02, 0x03, 0x05, 0x00, 0x02};
// A TLV of type 0x00 with size of 4 (1 byte is value).
constexpr uint8_t tlv1[] = {0x00, 0x00, 0x01, 0xff};
// A TLV of type 0x02 with size of 10 bytes (7 bytes are value). Second and last bytes in the
// value are non utf8 characters.
constexpr uint8_t tlv_type_authority[] = {0x02, 0x00, 0x07, 0x66, 0xfe,
0x6f, 0x2e, 0x63, 0x6f, 0xc1};
// A TLV of type 0x0f with size of 6 bytes (3 bytes are value).
constexpr uint8_t tlv3[] = {0x0f, 0x00, 0x03, 0xf0, 0x00, 0x0f};
// A TLV of type 0xea with size of 25 bytes (22 bytes are value). 7th and 21st bytes are non utf8
// characters.
constexpr uint8_t tlv_vpc_id[] = {0xea, 0x00, 0x16, 0x01, 0x76, 0x70, 0x63, 0x2d, 0x30,
0xc0, 0x35, 0x74, 0x65, 0x73, 0x74, 0x32, 0x66, 0x61,
0x36, 0x63, 0x36, 0x33, 0x68, 0xf9, 0x37};
constexpr uint8_t data[] = {'D', 'A', 'T', 'A'};

envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
auto rule_type_authority = proto_config.add_rules();
rule_type_authority->set_tlv_type(0x02);
rule_type_authority->mutable_on_tlv_present()->set_key("PP2 type authority");

auto rule_vpc_id = proto_config.add_rules();
rule_vpc_id->set_tlv_type(0xea);
rule_vpc_id->mutable_on_tlv_present()->set_key("PP2 vpc id");

connect(true, &proto_config);
write(buffer, sizeof(buffer));
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);

write(tlv1, sizeof(tlv1));
write(tlv_type_authority, sizeof(tlv_type_authority));
write(tlv3, sizeof(tlv3));
write(tlv_vpc_id, sizeof(tlv_vpc_id));
write(data, sizeof(data));
expectData("DATA");

EXPECT_EQ(1, server_connection_->streamInfo().dynamicMetadata().filter_metadata_size());

auto metadata = server_connection_->streamInfo().dynamicMetadata().filter_metadata();
EXPECT_EQ(1, metadata.size());
EXPECT_EQ(1, metadata.count(ProxyProtocol));

auto fields = metadata.at(ProxyProtocol).fields();
EXPECT_EQ(2, fields.size());
EXPECT_EQ(1, fields.count("PP2 type authority"));
EXPECT_EQ(1, fields.count("PP2 vpc id"));

const char replacement = 0x21;
auto value_type_authority = fields.at("PP2 type authority").string_value();
// Non utf8 characters have been replaced with `0x21` (`!` character).
ASSERT_THAT(value_type_authority,
ElementsAre(0x66, replacement, 0x6f, 0x2e, 0x63, 0x6f, replacement));

auto value_vpc_id = fields.at("PP2 vpc id").string_value();
ASSERT_THAT(value_vpc_id,
ElementsAre(0x01, 0x76, 0x70, 0x63, 0x2d, 0x30, replacement, 0x35, 0x74, 0x65, 0x73,
0x74, 0x32, 0x66, 0x61, 0x36, 0x63, 0x36, 0x33, 0x68, replacement, 0x37));
disconnect();
}

TEST_P(ProxyProtocolTest, V2WillNotOverwriteTLV) {
// A well-formed ipv4/tcp with a pair of TLV extensions is accepted
constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49,
Expand Down

0 comments on commit 3e6e83d

Please sign in to comment.