membphis commented on PR #13912:
URL: https://github.com/apache/apisix/pull/13912#issuecomment-5521872290

   Reviewed commit: `04fd78a199924450d1f97b5bb3fa6ec86d9052f0`.
   
   ### [P1] Fix the invalid upstream PROXY protocol header in mixed mode
   
   With both `tls: true` and `tls_passthrough: true`, enabling 
`proxy_protocol_to_upstream` (directly or through the global default) causes 
the inner server to generate another PROXY protocol header after the Unix 
socket hop.
   
   `set_real_ip_from unix:` restores the client address, but the destination 
remains the Unix socket. Assigning `api_ctx.var.server_addr` / `server_port` 
only updates the Lua variable cache; NGINX's PROXY protocol writer still reads 
`c->local_sockaddr` / `c->local_socklen`.
   
   An isolated stream/realip reproducer built from the NGINX bundled in 
OpenResty 1.29.2.4, using the same Unix socket forwarding pattern, captured a 
header of this form:
   
   ```text
   PROXY TCP4 127.0.0.1 unix:/tmp/stls-.../inner.sock <client-port> 0
   ```
   
   The destination is not a valid PROXY v1 TCP4/TCP6 address. A strict PROXY 
protocol backend rejects it before it can process the application data or TLS 
handshake. This affects both the terminating and passthrough branches of a 
mixed listener when upstream PROXY protocol is enabled. Dedicated 
passthrough/terminating listeners do not take this Unix socket path.
   
   Please fix the upstream header so that it carries valid original 
source/destination addresses and ports, and add regression tests for **both 
mixed-mode branches against a backend that actually parses PROXY protocol**. 
The existing rendering check and real-client-IP test do not exercise this 
combination.
   
   The reproduction validates the NGINX transport behavior; it was not a full 
APISIX/EE end-to-end run.
   
   Code references: [mixed listener 
template](https://github.com/apache/apisix/blob/04fd78a199924450d1f97b5bb3fa6ec86d9052f0/apisix/cli/ngx_tpl.lua#L270-L336),
 [Lua-only destination 
restoration](https://github.com/apache/apisix/blob/04fd78a199924450d1f97b5bb3fa6ec86d9052f0/apisix/init.lua#L1434-L1442).
   
   ### [P2] Additional socket forwarding in mixed mode: author assessment 
requested
   
   Please assess this point separately from the P1 correctness fix above.
   
   For a fixed-IP upstream, without retries or additional plugin calls, the 
mixed path has two outbound connections per stream session: the outer 
`proxy_pass $stream_tls_target` connects to an internal Unix socket, then the 
inner `proxy_pass apisix_backend` connects to the business upstream. Compared 
with the direct listener path:
   
   - Business-upstream connection: `1 -> 1`.
   - Internal Unix socket connection: `0 -> 1`.
   - Total outbound socket connections: `1 -> 2`.
   
   This is a local forwarding hop, not an extra remote-service lookup. It adds 
socket, buffering/copying, and scheduling work for both mixed-mode branches; 
dedicated passthrough/terminating listeners retain their direct path. The 
topology is verified from the implementation, but no latency or throughput 
regression has been measured here.
   
   Could you confirm whether sharing one listener requires retaining this hop, 
evaluate connection-rate/throughput and resource usage against the direct 
modes, and state whether to keep this trade-off or use separate listener modes 
/ an implementation without the extra hop? This item needs your explicit 
assessment; it is separate from the required PROXY protocol fix.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to