Skip to content

Fix Proxy Protocol outbound #9632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bneradt merged 1 commit into from
Apr 24, 2023
Merged

Fix Proxy Protocol outbound #9632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions proxy/http/ConnectingEntry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,16 @@ ConnectingEntry::state_http_server_open(int event, void *data)
// The buffer we create will be handed over to the eventually created server session
_netvc_read_buffer = new_MIOBuffer(HTTP_SERVER_RESP_HDR_BUFFER_INDEX);
_netvc_reader = _netvc_read_buffer->alloc_reader();
netvc->do_io_write(this, 1, _netvc_reader);
ink_release_assert(!connect_sms.empty());
if (!connect_sms.empty()) {
HttpSM *prime_connect_sm = *(connect_sms.begin());
netvc->set_inactivity_timeout(prime_connect_sm->get_server_connect_timeout());
HttpSM *prime_connect_sm = *(connect_sms.begin());

int64_t nbytes = 1;
if (is_no_plugin_tunnel && prime_connect_sm->t_state.txn_conf->proxy_protocol_out >= 0) {
nbytes = do_outbound_proxy_protocol(_netvc_reader->mbuf, vc, ua_txn->get_netvc(),
prime_connect_sm->t_state.txn_conf->proxy_protocol_out);
Copy link
Member

@shinrich shinrich Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we need to move the outbound proxy protocol processing here so we set the initial write request to the correct length? If we wait until the EVENT_TXN is received by the state_http_server_open, the write operation will already have been set.

Copy link
Contributor Author

@bneradt bneradt Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, empirically anyway that's what I observe. By the time the EVENT_TXN is done (where it was before), it seems too late to add the Proxy Protocol header because no header is emitted if we attempt to add it there.

}
netvc->do_io_write(this, nbytes, _netvc_reader);
netvc->set_inactivity_timeout(prime_connect_sm->get_server_connect_timeout());
ink_release_assert(_pending_action == nullptr);
return 0;
}
Expand Down
4 changes: 3 additions & 1 deletion proxy/http/ConnectingEntry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ class ConnectingEntry : public Continuation
IpEndpoint ipaddr;
std::string hostname;
std::set<HttpSM *> connect_sms;
NetVConnection *netvc = nullptr;
ProxyTransaction *ua_txn = nullptr;
NetVConnection *netvc = nullptr;
bool is_no_plugin_tunnel = false;

private:
MIOBuffer *_netvc_read_buffer = nullptr;
Expand Down
29 changes: 6 additions & 23 deletions proxy/http/HttpSM.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,8 @@ std::atomic<int64_t> next_sm_id(0);
/// Buffer for some error logs.
thread_local std::string error_bw_buffer;

/**
Outbound PROXY Protocol
} // namespace

Write PROXY Protocol to the first block of given MIOBuffer
FIXME: make @vc_in const
*/
int64_t
do_outbound_proxy_protocol(MIOBuffer *miob, NetVConnection *vc_out, NetVConnection *vc_in, int conf)
{
Expand Down Expand Up @@ -148,8 +144,6 @@ do_outbound_proxy_protocol(MIOBuffer *miob, NetVConnection *vc_out, NetVConnecti
return len;
}

} // namespace

ClassAllocator<HttpSM> httpSMAllocator("httpSMAllocator");

HttpVCTable::HttpVCTable(HttpSM *mysm)
Expand Down Expand Up @@ -1931,7 +1925,6 @@ HttpSM::state_http_server_open(int event, void *data)
case CONNECT_EVENT_TXN:
SMDebug("http", "Connection handshake complete via CONNECT_EVENT_TXN");
if (this->create_server_txn(static_cast<PoolableSession *>(data))) {
write_outbound_proxy_protocol();
handle_http_server_open();
} else { // Failed to create transaction. Maybe too many active transactions already
// Try again (probably need a bounding counter here)
Expand All @@ -1944,7 +1937,6 @@ HttpSM::state_http_server_open(int event, void *data)
// Update the time out to the regular connection timeout.
SMDebug("http_ss", "Connection handshake complete");
this->create_server_txn(this->create_server_session(_netvc, _netvc_read_buffer, _netvc_reader));
write_outbound_proxy_protocol();
t_state.current.server->clear_connect_fail();
handle_http_server_open();
return 0;
Expand Down Expand Up @@ -5681,11 +5673,13 @@ HttpSM::do_http_server_open(bool raw, bool only_direct)
SMDebug("http_ss", "Queue multiplexed request");
new_entry = new ConnectingEntry();
new_entry->mutex = this->mutex;
new_entry->ua_txn = ua_txn;
new_entry->handler = (ContinuationHandler)&ConnectingEntry::state_http_server_open;
new_entry->ipaddr.assign(&t_state.current.server->dst_addr.sa);
new_entry->hostname = t_state.current.server->name;
new_entry->sni = this->get_outbound_sni();
new_entry->cert_name = this->get_outbound_cert();
new_entry->hostname = t_state.current.server->name;
new_entry->sni = this->get_outbound_sni();
new_entry->cert_name = this->get_outbound_cert();
new_entry->is_no_plugin_tunnel = plugin_tunnel_type == HTTP_NO_PLUGIN_TUNNEL;
this->t_state.set_connect_fail(EIO);
new_entry->connect_sms.insert(this);
ethread->connecting_pool->m_ip_pool.insert(std::make_pair(new_entry->ipaddr, new_entry));
Expand Down Expand Up @@ -6549,17 +6543,6 @@ HttpSM::write_header_into_buffer(HTTPHdr *h, MIOBuffer *b)
return dumpoffset;
}

void
HttpSM::write_outbound_proxy_protocol()
{
int64_t nbytes = 1;
if (t_state.txn_conf->proxy_protocol_out >= 0) {
nbytes = do_outbound_proxy_protocol(server_txn->get_remote_reader()->mbuf, server_txn->get_netvc(), ua_txn->get_netvc(),
t_state.txn_conf->proxy_protocol_out);
}
server_entry->write_vio = server_txn->do_io_write(this, nbytes, server_txn->get_remote_reader());
}

void
HttpSM::attach_server_session()
{
Expand Down
14 changes: 12 additions & 2 deletions proxy/http/HttpSM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ class PreWarmSM;
class HttpSM;
using HttpSMHandler = int (HttpSM::*)(int, void *);

/** Write Proxy Protocol to the first block of given MIOBuffer.
*
* @param[in] miob The MIOBuffer to write the Proxy Protocol to.
* @param[in] vc_out The outbound (server-side) VC.
* @param[in] vc_in The inbound (client-side) VC.
* @param[in] conf The configured Proxy Protocol version to write.
*
* @return The number of bytes written on the socket to write the Proxy
* Protocol.
*/
int64_t do_outbound_proxy_protocol(MIOBuffer *miob, NetVConnection *vc_out, NetVConnection *vc_in, int conf);

enum HttpVC_t {
HTTP_UNKNOWN = 0,
HTTP_UA_VC,
Expand Down Expand Up @@ -235,8 +247,6 @@ class HttpSM : public Continuation, public PluginUserArgs<TS_USER_ARGS_TXN>

ProxyTransaction *get_ua_txn();
ProxyTransaction *get_server_txn();
// Write out the proxy_protocol information on a new outbound connection
void write_outbound_proxy_protocol();

// Called by transact. Updates are fire and forget
// so there are no callbacks and are safe to do
Expand Down