Skip to content

Commit 50c47e9

Browse files
authored
reverse_tunnels: intercept RPING on upstream reverse connections (envoyproxy#43666)
## Commit Message Intercept RPING on upstream reverse connections ## Additional Description If a RPING is sent and then the connection is upgraded to http2 via the conn pool the downstream reverse connection io handle's rping response causes a 502 protocol error. More specifically: Remote peer returned unexpected data while we expected SETTINGS frame Additional logs: ``` 2026-02-26 17:01:22.937 trace connection[thread=41 func=onReadReady file=source/common/network/connection_impl.cc:721] [Tags: "ConnectionId":"10614"] read ready. dispatch_buffered_data=0 2026-02-26 17:01:22.937 info misc[thread=41 func=read file=./source/extensions/bootstrap/reverse_tunnel/upstream_socket_interface/reverse_connection_io_handle.h:89] ALERT GOT SOME RPING Bytes: RPING 2026-02-26 17:01:22.937 trace connection[thread=41 func=doRead file=source/common/network/raw_buffer_socket.cc:25] [Tags: "ConnectionId":"10614"] read returns: 20745 2026-02-26 17:01:22.937 info misc[thread=41 func=read file=./source/extensions/bootstrap/reverse_tunnel/upstream_socket_interface/reverse_connection_io_handle.h:89] ALERT GOT SOME RPING Bytes: RPING 2026-02-26 17:01:22.937 trace connection[thread=41 func=doRead file=source/common/network/raw_buffer_socket.cc:39] [Tags: "ConnectionId":"10614"] read error: Resource temporarily unavailable, code: 0 2026-02-26 17:01:22.937 trace http2[thread=41 func=dispatch file=source/common/http/http2/codec_impl.cc:1077] [Tags: "ConnectionId":"10614"] dispatching 20745 bytes 2026-02-26 17:01:22.937 debug http2[thread=41 func=onError file=source/common/http/http2/codec_impl.cc:1395] [Tags: "ConnectionId":"10614"] invalid http2: Remote peer returned unexpected data while we expected SETTINGS frame. Perhaps, peer does not support HTTP/2 properly. ``` The Alert was a custom log line added by me for easier debugging. Its better for both the downstream and upstream rc io handles to share a common read implementation. ## Testing Unit tests. Additionally, manually validated the example in the docs which also works after the change. Signed-off-by: aakugan <aakashganapathy2@gmail.com>
1 parent 93795f9 commit 50c47e9

File tree

12 files changed

+445
-82
lines changed

12 files changed

+445
-82
lines changed

source/extensions/bootstrap/reverse_tunnel/common/BUILD

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
load(
22
"//bazel:envoy_build_system.bzl",
33
"envoy_cc_extension",
4+
"envoy_cc_library",
45
"envoy_extension_package",
56
)
67

@@ -22,3 +23,13 @@ envoy_cc_extension(
2223
"//source/common/http:headers_lib",
2324
],
2425
)
26+
27+
envoy_cc_library(
28+
name = "rping_interceptor_lib",
29+
srcs = ["rping_interceptor.cc"],
30+
hdrs = ["rping_interceptor.h"],
31+
deps = [
32+
":reverse_connection_utility_lib",
33+
"//source/common/network:default_socket_interface_lib",
34+
],
35+
)
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#include "source/extensions/bootstrap/reverse_tunnel/common/rping_interceptor.h"
2+
3+
#include "source/extensions/bootstrap/reverse_tunnel/common/reverse_connection_utility.h"
4+
5+
namespace Envoy {
6+
namespace Extensions {
7+
namespace Bootstrap {
8+
namespace ReverseConnection {
9+
10+
Api::IoCallUint64Result RpingInterceptor::read(Buffer::Instance& buffer,
11+
absl::optional<uint64_t> max_length) {
12+
// Perform the actual read first.
13+
Api::IoCallUint64Result result = IoSocketHandleImpl::read(buffer, max_length);
14+
ENVOY_LOG(trace, "RpingInterceptor: read result: {}", result.return_value_);
15+
16+
// If RPING keepalives are still active, check whether the incoming data is a RPING message.
17+
if (ping_echo_active_ && result.err_ == nullptr && result.return_value_ > 0) {
18+
const uint64_t expected = ReverseConnectionUtility::PING_MESSAGE.size();
19+
20+
// Compare up to the expected size using a zero-copy view.
21+
const uint64_t len = std::min<uint64_t>(buffer.length(), expected);
22+
const char* data = static_cast<const char*>(buffer.linearize(len));
23+
absl::string_view peek_sv{data, static_cast<size_t>(len)};
24+
25+
// Check if we have a complete RPING message.
26+
if (len == expected && ReverseConnectionUtility::isPingMessage(peek_sv)) {
27+
// Found a complete RPING. Echo and drain it from the buffer.
28+
buffer.drain(expected);
29+
onPingMessage();
30+
31+
// If buffer only contained RPING, return showing we processed it.
32+
if (buffer.length() == 0) {
33+
return Api::IoCallUint64Result{expected, Api::IoError::none()};
34+
}
35+
36+
// RPING followed by application data. Disable echo and return the remaining data.
37+
ENVOY_LOG(trace,
38+
"RpingInterceptor: received application data after RPING, "
39+
"disabling RPING echo for FD: {}",
40+
fd_);
41+
ping_echo_active_ = false;
42+
// The adjusted return value is the number of bytes excluding the drained RPING. It should be
43+
// transparent to upper layers that the RPING was processed.
44+
const uint64_t adjusted =
45+
(result.return_value_ >= expected) ? (result.return_value_ - expected) : 0;
46+
return Api::IoCallUint64Result{adjusted, Api::IoError::none()};
47+
}
48+
49+
// If partial data could be the start of RPING (only when fewer than expected bytes).
50+
if (len < expected) {
51+
const absl::string_view rping_prefix =
52+
ReverseConnectionUtility::PING_MESSAGE.substr(0, static_cast<size_t>(len));
53+
if (peek_sv == rping_prefix) {
54+
ENVOY_LOG(trace,
55+
"RpingInterceptor: partial RPING received ({} bytes), waiting "
56+
"for more.",
57+
len);
58+
return result; // Wait for more data.
59+
}
60+
}
61+
62+
// Data is not RPING (complete or partial). Disable echo permanently.
63+
ENVOY_LOG(trace,
64+
"RpingInterceptor: received application data ({} bytes), "
65+
"disabling RPING echo for FD: {}",
66+
len, fd_);
67+
ping_echo_active_ = false;
68+
}
69+
70+
return result;
71+
}
72+
73+
} // namespace ReverseConnection
74+
} // namespace Bootstrap
75+
} // namespace Extensions
76+
} // namespace Envoy
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#pragma once
2+
3+
#include "source/common/network/io_socket_handle_impl.h"
4+
5+
namespace Envoy {
6+
namespace Extensions {
7+
namespace Bootstrap {
8+
namespace ReverseConnection {
9+
10+
class RpingInterceptor : public virtual Network::IoSocketHandleImpl {
11+
public:
12+
// Intercept reads to handle reverse connection keep-alive pings.
13+
Api::IoCallUint64Result read(Buffer::Instance& buffer,
14+
absl::optional<uint64_t> max_length) override;
15+
16+
virtual void onPingMessage() PURE;
17+
18+
protected:
19+
// Whether to actively echo RPING messages while the connection is idle.
20+
// Disabled permanently after the first non-RPING application byte is observed.
21+
bool ping_echo_active_{true};
22+
};
23+
24+
} // namespace ReverseConnection
25+
} // namespace Bootstrap
26+
} // namespace Extensions
27+
} // namespace Envoy

source/extensions/bootstrap/reverse_tunnel/downstream_socket_interface/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ envoy_cc_library(
8989
"//source/common/tls:ssl_handshaker_lib",
9090
"//source/common/upstream:load_balancer_context_base_lib",
9191
"//source/extensions/bootstrap/reverse_tunnel/common:reverse_connection_utility_lib",
92+
"//source/extensions/bootstrap/reverse_tunnel/common:rping_interceptor_lib",
9293
],
9394
)
9495

source/extensions/bootstrap/reverse_tunnel/downstream_socket_interface/downstream_reverse_connection_io_handle.cc

Lines changed: 8 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#include "source/extensions/bootstrap/reverse_tunnel/downstream_socket_interface/downstream_reverse_connection_io_handle.h"
22

33
#include "source/common/common/logger.h"
4-
#include "source/extensions/bootstrap/reverse_tunnel/common/reverse_connection_utility.h"
54
#include "source/extensions/bootstrap/reverse_tunnel/downstream_socket_interface/reverse_connection_io_handle.h"
65

76
#include "absl/strings/string_view.h"
@@ -31,79 +30,15 @@ DownstreamReverseConnectionIOHandle::~DownstreamReverseConnectionIOHandle() {
3130
fd_, connection_key_);
3231
}
3332

34-
Api::IoCallUint64Result
35-
DownstreamReverseConnectionIOHandle::read(Buffer::Instance& buffer,
36-
absl::optional<uint64_t> max_length) {
37-
// Perform the actual read first.
38-
Api::IoCallUint64Result result = IoSocketHandleImpl::read(buffer, max_length);
39-
ENVOY_LOG(trace, "DownstreamReverseConnectionIOHandle: read result: {}", result.return_value_);
40-
41-
// If RPING keepalives are still active, check whether the incoming data is a RPING message.
42-
if (ping_echo_active_ && result.err_ == nullptr && result.return_value_ > 0) {
43-
const uint64_t expected =
44-
::Envoy::Extensions::Bootstrap::ReverseConnection::ReverseConnectionUtility::PING_MESSAGE
45-
.size();
46-
47-
// Compare up to the expected size using a zero-copy view.
48-
const uint64_t len = std::min<uint64_t>(buffer.length(), expected);
49-
const char* data = static_cast<const char*>(buffer.linearize(len));
50-
absl::string_view peek_sv{data, static_cast<size_t>(len)};
51-
52-
// Check if we have a complete RPING message.
53-
if (len == expected &&
54-
::Envoy::Extensions::Bootstrap::ReverseConnection::ReverseConnectionUtility::isPingMessage(
55-
peek_sv)) {
56-
// Found a complete RPING. Echo and drain it from the buffer.
57-
buffer.drain(expected);
58-
auto echo_rc = ::Envoy::Extensions::Bootstrap::ReverseConnection::ReverseConnectionUtility::
59-
sendPingResponse(*this);
60-
if (!echo_rc.ok()) {
61-
ENVOY_LOG(trace, "DownstreamReverseConnectionIOHandle: failed to send RPING echo on FD: {}",
62-
fd_);
63-
} else {
64-
ENVOY_LOG(trace, "DownstreamReverseConnectionIOHandle: echoed RPING on FD: {}", fd_);
65-
}
66-
67-
// If buffer only contained RPING, return showing we processed it.
68-
if (buffer.length() == 0) {
69-
return Api::IoCallUint64Result{expected, Api::IoError::none()};
70-
}
71-
72-
// RPING followed by application data. Disable echo and return the remaining data.
73-
ENVOY_LOG(trace,
74-
"DownstreamReverseConnectionIOHandle: received application data after RPING, "
75-
"disabling RPING echo for FD: {}",
76-
fd_);
77-
ping_echo_active_ = false;
78-
// The adjusted return value is the number of bytes excluding the drained RPING. It should be
79-
// transparent to upper layers that the RPING was processed.
80-
const uint64_t adjusted =
81-
(result.return_value_ >= expected) ? (result.return_value_ - expected) : 0;
82-
return Api::IoCallUint64Result{adjusted, Api::IoError::none()};
83-
}
84-
85-
// If partial data could be the start of RPING (only when fewer than expected bytes).
86-
if (len < expected) {
87-
const absl::string_view rping_prefix =
88-
ReverseConnectionUtility::PING_MESSAGE.substr(0, static_cast<size_t>(len));
89-
if (peek_sv == rping_prefix) {
90-
ENVOY_LOG(trace,
91-
"DownstreamReverseConnectionIOHandle: partial RPING received ({} bytes), waiting "
92-
"for more.",
93-
len);
94-
return result; // Wait for more data.
95-
}
96-
}
97-
98-
// Data is not RPING (complete or partial). Disable echo permanently.
99-
ENVOY_LOG(trace,
100-
"DownstreamReverseConnectionIOHandle: received application data ({} bytes), "
101-
"disabling RPING echo for FD: {}",
102-
len, fd_);
103-
ping_echo_active_ = false;
104-
}
33+
void DownstreamReverseConnectionIOHandle::onPingMessage() {
34+
auto echo_rc = ReverseConnectionUtility::sendPingResponse(*this);
10535

106-
return result;
36+
if (!echo_rc.ok()) {
37+
ENVOY_LOG(trace, "DownstreamReverseConnectionIOHandle: failed to send RPING echo on FD: {}",
38+
fd_);
39+
} else {
40+
ENVOY_LOG(trace, "DownstreamReverseConnectionIOHandle: echoed RPING on FD: {}", fd_);
41+
}
10742
}
10843

10944
// DownstreamReverseConnectionIOHandle close() implementation.

source/extensions/bootstrap/reverse_tunnel/downstream_socket_interface/downstream_reverse_connection_io_handle.h

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
#include "source/common/common/logger.h"
99
#include "source/common/network/io_socket_handle_impl.h"
10+
#include "source/extensions/bootstrap/reverse_tunnel/common/reverse_connection_utility.h"
11+
#include "source/extensions/bootstrap/reverse_tunnel/common/rping_interceptor.h"
1012

1113
namespace Envoy {
1214
namespace Extensions {
@@ -21,7 +23,7 @@ class ReverseConnectionIOHandle;
2123
* This class is used internally by ReverseConnectionIOHandle to manage the lifecycle
2224
* of accepted downstream connections.
2325
*/
24-
class DownstreamReverseConnectionIOHandle : public Network::IoSocketHandleImpl {
26+
class DownstreamReverseConnectionIOHandle : public RpingInterceptor {
2527
public:
2628
/**
2729
* Constructor that takes ownership of the socket and stores parent pointer and connection key.
@@ -33,12 +35,13 @@ class DownstreamReverseConnectionIOHandle : public Network::IoSocketHandleImpl {
3335
~DownstreamReverseConnectionIOHandle() override;
3436

3537
// Network::IoHandle overrides.
36-
// Intercept reads to handle reverse connection keep-alive pings.
37-
Api::IoCallUint64Result read(Buffer::Instance& buffer,
38-
absl::optional<uint64_t> max_length) override;
3938
Api::IoCallUint64Result close() override;
4039
Api::SysCallIntResult shutdown(int how) override;
4140

41+
// RPING Interceptor overrides.
42+
// Send the RPING response from here.
43+
void onPingMessage() override;
44+
4245
/**
4346
* Tell this IO handle to ignore close() and shutdown() calls.
4447
* This is called by the HTTP filter during socket hand-off to prevent
@@ -60,10 +63,6 @@ class DownstreamReverseConnectionIOHandle : public Network::IoSocketHandleImpl {
6063
std::string connection_key_;
6164
// Flag to ignore close and shutdown calls during socket hand-off.
6265
bool ignore_close_and_shutdown_{false};
63-
64-
// Whether to actively echo RPING messages while the connection is idle.
65-
// Disabled permanently after the first non-RPING application byte is observed.
66-
bool ping_echo_active_{true};
6766
};
6867

6968
} // namespace ReverseConnection

source/extensions/bootstrap/reverse_tunnel/upstream_socket_interface/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ envoy_cc_extension(
2929
"//source/common/config:utility_lib",
3030
"//source/common/network:default_socket_interface_lib",
3131
"//source/extensions/bootstrap/reverse_tunnel/common:reverse_connection_utility_lib",
32+
"//source/extensions/bootstrap/reverse_tunnel/common:rping_interceptor_lib",
3233
"@envoy_api//envoy/extensions/bootstrap/reverse_tunnel/upstream_socket_interface/v3:pkg_cc_proto",
3334
],
3435
)

source/extensions/bootstrap/reverse_tunnel/upstream_socket_interface/reverse_connection_io_handle.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "envoy/network/socket.h"
77

88
#include "source/common/network/io_socket_handle_impl.h"
9+
#include "source/extensions/bootstrap/reverse_tunnel/common/rping_interceptor.h"
910

1011
namespace Envoy {
1112
namespace Extensions {
@@ -17,7 +18,7 @@ namespace ReverseConnection {
1718
* This class implements RAII principles to ensure proper socket cleanup and provides
1819
* reverse connection semantics where the connection is already established.
1920
*/
20-
class UpstreamReverseConnectionIOHandle : public Network::IoSocketHandleImpl {
21+
class UpstreamReverseConnectionIOHandle : public RpingInterceptor {
2122
public:
2223
/**
2324
* Constructs an UpstreamReverseConnectionIOHandle that takes ownership of a socket.
@@ -71,6 +72,10 @@ class UpstreamReverseConnectionIOHandle : public Network::IoSocketHandleImpl {
7172
*/
7273
void releaseSocketForTest() { owned_socket_.reset(); }
7374

75+
// Ignore all ping messages.
76+
// The connection is passed on to the http2 codec.
77+
void onPingMessage() override {}
78+
7479
private:
7580
// The name of the cluster this reverse connection belongs to.
7681
std::string cluster_name_;

test/extensions/bootstrap/reverse_tunnel/common/BUILD

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,14 @@ envoy_cc_test(
2020
"//test/test_common:test_runtime_lib",
2121
],
2222
)
23+
24+
envoy_cc_test(
25+
name = "rping_interceptor_test",
26+
size = "medium",
27+
srcs = ["rping_interceptor_test.cc"],
28+
deps = [
29+
"//source/common/buffer:buffer_lib",
30+
"//source/extensions/bootstrap/reverse_tunnel/common:reverse_connection_utility_lib",
31+
"//source/extensions/bootstrap/reverse_tunnel/common:rping_interceptor_lib",
32+
],
33+
)

0 commit comments

Comments
 (0)