Use ProxyProtocol SRC IP as Client IP

[cmcfarlen]

Restore old feature to copy PP src IP to remote ip of NetVConn, but this time the copy is configurable. Expand autest to cover new setup.

Edit: This doesn't restore the overwriting the remote_addr aspect of the old behavior, but does effectively replace the client IP with the proxy protocol address. /endedit

When the proxy protocol feature was added in #3958, the behavior was that if the server_port was configured for proxy protocol, and there was valid information available, the PP SRC IP address was alway copied to the NetVConnection's remote IP address. This meant that for proxy protocol ports, the client's IP (as seen by the API and logs) was not the direct peer host, but the peer of the prior HOPs connection (presumably the original requesting client IP). This might be desirable if ATS was used with some load balancer ingress to provide the PP and the ATS config was written without that hop in mind. As far as I can tell, this feature was introduced for ATS 9 and back ported to later versions of 8.

Later, #8544 was filed to request that the prior hop's IP address be made available to the log and an rchi be added to that end. PR #8893 was created and that changed the behavior of the proxy protocol feature to no longer copy the PP SRC IP to the remote ip address. This fixed the chi log to always report next hop, but removed the utility of having the "true" client IP for use in plugins, acls and logging.

This PR intends to revert to the mean and add the ability to restore the PP IP usage, but put that behavior behind a new server_port config. This new flag should allow the change without breaking compatability with 10, but also allow restoring the behavior that was in ATS 9.

Draft for now to finish docs and discussion.

[cmcfarlen]

After sleeping on this, I decided that I don't like that the meaning of remote address in NetVConnection is overloaded for proxy protocol. So, I reverted that and instead introduced get_client_addr and friends to return either the proxy protocol IP or the remote IP based on the config. This also provides the ability to implement the rchi log fields that was requested in #8544. Now the TSHttpTxnClientAddrGet calls the new get_client_addr and the remote address can still be acquired by TSNetVConnRemoteAddrGet without ambiguity.

[cmcfarlen]

autest failed: proxy_serve_stale_dns_fail

[cmcfarlen]

[approve ci autest 1]

[maskit]

Hmm, I don't think we should do this. If the setting value were CLIENT then this would make sense, but "PEER" was picked as a term that means the direct peer to distinguish it from the ambiguous term "client".

Also, this is unrelated to the change, I found that this if-else lacks the support for PLUGIN (verified address). I'll add it.

[cmcfarlen]

The point of this PR and the new pp-clnt flag (if configured) is to use the actual/true client address (from proxy protocol) for PEER or Client instead of the literal peer host's address. So, in all places where PEER is used, we need to check if the PP SRC IP should be used instead. This is what get_client_addr does. I'm open to better names for this function.

[maskit]

Looks like this function is for origin connections. If so, remote_addr means origin's address, and client_addr means ATS's address.

[maskit]

Actually, it may not. But it's confusing because the client is ATS on origin connections.

[cmcfarlen]

What is a better name? effective_remote_addr? I agree it's odd if the connection is not the incoming request one, but it looked like this function is generic. The only important function call change in this last commit is for ACLs. That is client side and does require get_client_addr.

[maskit]

Same as above. The name doesn't make sense on origin connections.

[bryancall]

Thanks for working on this feature, Chris. I've reviewed the changes and have some concerns that align with maskit's earlier comments.

Issues with SSLClientUtils.cc and SSLDiags.cc

The changes in these two files should be reverted to use get_remote_addr() instead of get_effective_remote_addr():

1. SSLClientUtils.cc - This file contains verify_callback() which is used for outbound origin connections (ATS verifying origin server certificates). On these connections:

   • The "remote" is the origin server, not a client
   • Proxy Protocol only applies to inbound client connections
   • Using get_effective_remote_addr() here would return an undefined/meaningless value since pp_info is never populated on origin connections

2. SSLDiags.cc - The SSLDiagnostic() function can be called for both client-side and origin-side SSL connections. When called for origin connections, get_effective_remote_addr() would log incorrect information.

Suggested Fix

Keep the get_effective_remote_addr() changes in:

• SNIActionPerformer.cc (inbound SNI handling)
• Http2SessionAccept.cc (inbound session handling)
• ProxySession.cc (client session handling)
• Logging code paths that specifically deal with client IPs

But revert to get_remote_addr() in:

• SSLClientUtils.cc (all 4 occurrences)
• SSLDiags.cc

The core issue is that these files handle origin-side SSL operations where Proxy Protocol doesn't apply.

[Copilot]

Pull request overview

This pull request adds a configurable flag pp-clnt to control whether the proxy protocol source IP address is used as the client IP for transactions. This restores functionality that was present in ATS 9 but was changed in version 10 to always use the direct peer's IP address.

Key changes:

• Introduces the pp-clnt server port configuration flag to enable using proxy protocol SRC IP as the client IP
• Adds new logging fields (rchi, rchp, rchh) to always log the actual remote peer's IP/port regardless of proxy protocol configuration
• Updates API to include TSNetVConnClientAddrGet() and internal methods to distinguish between client and remote addresses

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/gold_tests/proxy_protocol/proxy_protocol.test.py	Extended test to cover both proxy protocol configurations (with and without pp-clnt flag)
tests/gold_tests/proxy_protocol/gold/access.gold	Removed old gold file as tests now run two scenarios
tests/gold_tests/proxy_protocol/gold/access-nocp.gold	Gold file for proxy protocol without pp-clnt flag, showing chi equals remote peer IP
tests/gold_tests/proxy_protocol/gold/access-cp.gold	Gold file for proxy protocol with pp-clnt flag, showing chi uses proxy protocol SRC IP
tests/gold_tests/autest-site/trafficserver.test.ext	Added support for enable_proxy_protocol_cp_src parameter in test infrastructure
src/records/RecHttp.cc	Added OPT_PROXY_PROTO_CLIENT_SRC_IP constant and processing logic for pp-clnt flag
src/proxy/logging/LogAccess.cc	Implemented marshal functions for remote host IP/port and updated client host functions to use effective address
src/proxy/logging/Log.cc	Registered new logging fields (rchi, rchp, rchh) for remote host information
src/proxy/http2/Http2SessionAccept.cc	Updated to use get_effective_remote_addr() for proper IP resolution
src/proxy/http/HttpProxyServerMain.cc	Added f_proxy_protocol_client_src flag to accept options
src/proxy/ProxySession.cc	Implemented get_client_addr() and get_client_port() methods
src/iocore/net/UnixNetAccept.cc	Updated set_is_proxy_protocol() calls to pass cp_src_ip parameter
src/iocore/net/Server.cc	Enhanced debug logging for proxy protocol configuration
src/iocore/net/SSLNetVConnection.cc	Updated comment to clarify IP address usage
src/iocore/net/SSLDiags.cc	Changed to use get_effective_remote_addr() for diagnostic output
src/iocore/net/SSLClientUtils.cc	Updated to use get_effective_remote_addr() for SSL verification
src/iocore/net/SNIActionPerformer.cc	Changed to use get_effective_remote_addr() for IP allow checks
src/api/InkAPI.cc	Updated TSHttpSsnClientAddrGet() to use get_client_addr() and added TSNetVConnClientAddrGet()
include/ts/ts.h	Added TSNetVConnClientAddrGet() API function declaration
include/records/RecHttp.h	Added m_proxy_protocol_client_src member and OPT_PROXY_PROTO_CLIENT_SRC_IP constant
include/proxy/logging/LogAccess.h	Added declarations for marshal_remote_host_ip() and marshal_remote_host_port()
include/proxy/ProxyTransaction.h	Added get_client_addr() and get_client_port() method declarations
include/proxy/ProxySession.h	Added virtual get_client_addr() and get_client_port() methods
include/iocore/net/NetVConnection.h	Added get_effective_remote_addr(), get_client_endpoint(), and get_client_port() methods; updated set_is_proxy_protocol() signature
include/iocore/net/AcceptOptions.h	Added f_proxy_protocol_client_src flag to AcceptOptions struct
doc/admin-guide/logging/formatting.en.rst	Documented new logging fields (rchi, rchp, rchh) and updated chi field description
doc/admin-guide/files/records.yaml.en.rst	Documented pp-clnt server port configuration flag
doc/admin-guide/configuration/proxy-protocol.en.rst	Added explanation of pp-clnt flag behavior

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Spelling error: "alway" should be "always".

Suggested change

	rchi Remote Client This is alway the IP address of the inbound remote peer.
	rchi Remote Client This is always the IP address of the inbound remote peer.

[Copilot]

The flag name "pp-clnt" uses an abbreviation "clnt" which is inconsistent with the codebase naming pattern. Other related configuration options use full words like "pp", "ssl", "tr-in", "tr-out", etc. Consider using a more descriptive name like "pp-client-src" or "pp-use-src" to make the purpose clearer and maintain consistency with naming conventions.

[bryancall]

Update to my previous comment:

After further analysis, I realize the implementation is actually safe. The is_proxy_protocol_cp_src flag is only set on inbound connections via set_is_proxy_protocol() in UnixNetAccept.cc.

For outbound/origin connections:

• is_proxy_protocol_cp_src defaults to false and is never set
• So get_effective_remote_addr() will always return get_remote_addr() (the origin's address)

The changes in SSLClientUtils.cc and SSLDiags.cc are safe because on outbound VCs, the condition pp_info.version != ProxyProtocolVersion::UNDEFINED && is_proxy_protocol_cp_src will be false (since is_proxy_protocol_cp_src is false), causing it to fall through to get_remote_addr().

Maskit's concern about naming is still valid ("effective_remote_addr" is a bit confusing on origin connections), but the behavior is correct. My concerns about reverting those files were overly cautious.

[cmcfarlen]

[approve ci freebsd]