Skip to content

Commit

Permalink
tls_inspector: Fix invalid ALPN extension in test (#34300)
Browse files Browse the repository at this point in the history
This commit stops generateClientHelloFromJA3Fingerprint() generating client
hellos containing an invalid ALPN extension. It also updates relevant
tls_inspector_test functions to check the ALPN value, if expected.

When the generateClientHelloFromJA3Fingerprint() function was asked to include
an ALPN extension (16) in the generated client hello, it was generating a
default empty extension with the correct id (16) but a zero length. While this
is technically a valid extension, it is not a valid ALPN extension, which must
include a list of the client's preferred protocol(s).

This was causing test failures in the envoy-openssl repo because OpenSSL
responds to the malformed ALPN extension by sending a TLS alert 50 (Decode
Error) which causes many of the tls_inspector_test functions to fail.

Signed-off-by: Ted Poole <[email protected]>
  • Loading branch information
tedjpoole authored May 22, 2024
1 parent 7dca2c8 commit 72362c1
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class TlsInspectorTest : public testing::TestWithParam<std::tuple<uint16_t, uint
}

void testJA3(const std::string& fingerprint, bool expect_server_name = true,
const std::string& hash = {});
const std::string& hash = {}, bool expect_alpn = true);

NiceMock<Api::MockOsSysCalls> os_sys_calls_;
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls_{&os_sys_calls_};
Expand Down Expand Up @@ -311,7 +311,7 @@ TEST_P(TlsInspectorTest, ConnectionFingerprint) {
}

void TlsInspectorTest::testJA3(const std::string& fingerprint, bool expect_server_name,
const std::string& hash) {
const std::string& hash, bool expect_alpn) {
envoy::extensions::filters::listener::tls_inspector::v3::TlsInspector proto_config;
proto_config.mutable_enable_ja3_fingerprinting()->set_value(true);
cfg_ = std::make_shared<Config>(*store_.rootScope(), proto_config);
Expand All @@ -328,7 +328,11 @@ void TlsInspectorTest::testJA3(const std::string& fingerprint, bool expect_serve
if (expect_server_name) {
EXPECT_CALL(socket_, setRequestedServerName(absl::string_view("www.envoyproxy.io")));
}
EXPECT_CALL(socket_, setRequestedApplicationProtocols(_)).Times(0);
if (expect_alpn) {
EXPECT_CALL(socket_, setRequestedApplicationProtocols(testing::Contains("HTTP/1.1")));
} else {
EXPECT_CALL(socket_, setRequestedApplicationProtocols(_)).Times(0);
}
// EXPECT_CALL(cb_, continueFilterChain(true));
EXPECT_CALL(socket_, setDetectedTransportProtocol(absl::string_view("tls")));
EXPECT_CALL(socket_, detectedTransportProtocol()).Times(::testing::AnyNumber());
Expand Down Expand Up @@ -387,7 +391,7 @@ TEST_P(TlsInspectorTest, ConnectionJA3HashNoEllipticCurvesOrPointFormats) {
TEST_P(TlsInspectorTest, ConnectionJA3HashTls10NoExtensions) {
testJA3("769,49162-49157-49161-49156-49159-49154-49160-49155-49172-49167-49171-49166-49169-49164-"
"49170-49165-57-51-53-47-5-4-10,,,",
false);
false, "", false);
}

// Test that the filter sets the correct `JA3` hash with TLS1.1.
Expand Down
16 changes: 16 additions & 0 deletions test/extensions/filters/listener/tls_inspector/tls_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ std::vector<uint8_t> generateClientHelloFromJA3Fingerprint(const std::string& ja
// algorithm
0x04, 0x03};

// ALPN extension
const uint16_t alpn_id = 0x10;
std::vector<uint8_t> alpn_extension = {(alpn_id & 0xff00) >> 8, alpn_id & 0xff,
// length
0x00, 0x0b,
// list length
0x00, 0x09,
// protocol length
0x08,
// protocol name
'H', 'T', 'T', 'P', '/', '1', '.', '1'};

// extensions
values = absl::StrSplit(fingerprint[2], '-', absl::SkipEmpty());
std::vector<uint8_t> extensions;
Expand All @@ -141,6 +153,10 @@ std::vector<uint8_t> generateClientHelloFromJA3Fingerprint(const std::string& ja
std::end(signature_algorithms));
break;
}
case alpn_id: {
extensions.insert(std::end(extensions), std::begin(alpn_extension), std::end(alpn_extension));
break;
}
default: {
uint16_t extension_id = std::stoi(v, nullptr);
extensions.push_back((extension_id & 0xff00) >> 8);
Expand Down

0 comments on commit 72362c1

Please sign in to comment.