cmcfarlen commented on code in PR #12761:
URL: https://github.com/apache/trafficserver/pull/12761#discussion_r2640218622
##########
src/iocore/net/SNIActionPerformer.cc:
##########
@@ -417,7 +417,7 @@ SNI_IpAllow::SNIAction(SSL &ssl, ActionItem::Context const
& /* ctx ATS_UNUSED *
const sockaddr *client_ip = nullptr;
for (int i = 0; i < IpAllow::Subject::MAX_SUBJECTS; ++i) {
if (IpAllow::Subject::PEER == IpAllow::subjects[i]) {
- client_ip = ssl_vc->get_remote_addr();
+ client_ip = ssl_vc->get_client_addr();
Review Comment:
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.
##########
src/iocore/net/SSLClientUtils.cc:
##########
@@ -81,7 +81,7 @@ verify_callback(int signature_ok, X509_STORE_CTX *ctx)
Dbg(dbg_ctl_ssl_verify, "verification error:num=%d:%s:depth=%d", err,
X509_verify_cert_error_string(err), depth);
const char *sni_name;
char buff[INET6_ADDRSTRLEN];
- ats_ip_ntop(netvc->get_remote_addr(), buff, INET6_ADDRSTRLEN);
+ ats_ip_ntop(netvc->get_client_addr(), buff, INET6_ADDRSTRLEN);
Review Comment:
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`.
--
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]
