Add a setting to choose the data source of IP address for ACL (#12298)

maskit · web-flow · commit e1fd38e6c19e · 2025-06-16T20:07:59.000-05:00
* Add a setting to choose the data source of IP address for ACL

* Address isssues
diff --git a/doc/admin-guide/configuration/proxy-protocol.en.rst b/doc/admin-guide/configuration/proxy-protocol.en.rst
@@ -51,6 +51,10 @@ configured with :ts:cv:`proxy.config.http.proxy_protocol_allowlist`.
        :ts:cv:`proxy.config.http.server_ports` configuration, regardless of whether
        the connections have the Proxy Protocol header.
 
+By default, |TS| uses client's IP address that is from the peer when it applies ACL. If you configure a port to
+enable PROXY protocol and want to apply ACL against the IP address delivered by PROXY protocol, you need to have ``PROXY`` in
+:ts:cv:`proxy.config.acl.subjects`.
+
 1. HTTP Forwarded Header
 
 The client IP address in the PROXY protocol header is passed to the origin server via an HTTP `Forwarded:
diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst
@@ -2131,6 +2131,20 @@ IP Allow
    the use of this file, see :file:`ip_allow.yaml`. If this is a relative path,
    |TS| loads it relative to the ``SYSCONFDIR`` directory.
 
+.. ts:cv:: CONFIG proxy.config.acl.subjects STRING PEER
+
+   Specifies the list of data sources for getting client's IP address for ACL.
+   The value is a comma separated string, and the first available data source
+   will be used. If you configure a port to enable PROXY protocol, you probably
+   need to adjust this setting to have ``PROXY`` in the list.
+
+   ============= ======================================================================
+   Value         Description
+   ============= ======================================================================
+   ``PEER``      Use the IP address of the peer
+   ``PROXY``     Use the IP address from PROXY protocol
+   ============= ======================================================================
+
 
 Cache Control
 =============
diff --git a/include/proxy/IPAllow.h b/include/proxy/IPAllow.h
@@ -147,6 +147,8 @@ class IpAllow : public ConfigInfo
 
   static constexpr const char *MODULE_NAME = "IPAllow";
 
+  enum Subject { PEER, PROXY, MAX_SUBJECTS };
+
   /** An access control record and support data.
    * The primary point of this is to hold the backing configuration in memory while the ACL
    * is in use.
@@ -252,6 +254,8 @@ class IpAllow : public ConfigInfo
    */
   static bool has_no_rules();
 
+  static uint8_t subjects[Subject::MAX_SUBJECTS];
+
 private:
   static size_t       configid;         ///< Configuration ID for update management.
   static const Record ALLOW_ALL_RECORD; ///< Static record that allows all access.
diff --git a/src/iocore/net/SNIActionPerformer.cc b/src/iocore/net/SNIActionPerformer.cc
@@ -24,6 +24,7 @@
 #include "swoc/swoc_file.h"
 #include "swoc/BufferWriter.h"
 #include "tscore/Layout.h"
+#include "proxy/IPAllow.h"
 
 #include "SNIActionPerformer.h"
 
@@ -412,8 +413,19 @@ SNI_IpAllow::SNIAction(SSL &ssl, ActionItem::Context const & /* ctx ATS_UNUSED *
     return SSL_TLSEXT_ERR_OK;
   }
 
-  auto ssl_vc = SSLNetVCAccess(&ssl);
-  auto ip     = swoc::IPAddr(ssl_vc->get_remote_endpoint());
+  auto            ssl_vc    = SSLNetVCAccess(&ssl);
+  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();
+      break;
+    } else if (IpAllow::Subject::PROXY == IpAllow::subjects[i] &&
+               ssl_vc->get_proxy_protocol_version() != ProxyProtocolVersion::UNDEFINED) {
+      client_ip = ssl_vc->get_proxy_protocol_src_addr();
+      break;
+    }
+  }
+  swoc::IPAddr ip = swoc::IPAddr(client_ip);
 
   // check the allowed ips
   if (ip_addrs.contains(ip)) {
diff --git a/src/proxy/IPAllow.cc b/src/proxy/IPAllow.cc
@@ -24,6 +24,8 @@
   limitations under the License.
  */
 
+#include <string_view>
+
 #include "proxy/IPAllow.h"
 #include "records/RecCore.h"
 #include "swoc/Errata.h"
@@ -67,8 +69,9 @@ enum AclOp {
 const IpAllow::Record IpAllow::ALLOW_ALL_RECORD(ALL_METHOD_MASK);
 const IpAllow::ACL    IpAllow::DENY_ALL_ACL;
 
-size_t IpAllow::configid       = 0;
-bool   IpAllow::accept_check_p = true; // initializing global flag for fast deny
+size_t  IpAllow::configid       = 0;
+bool    IpAllow::accept_check_p = true; // initializing global flag for fast deny
+uint8_t IpAllow::subjects[Subject::MAX_SUBJECTS];
 
 static ConfigUpdateHandler<IpAllow> *ipAllowUpdate;
 
@@ -212,6 +215,32 @@ IpAllow::IpAllow(const char *ip_allow_config_var, const char *ip_categories_conf
   if (!path.empty()) {
     ip_categories_config_file = ats_scoped_str(path).get();
   }
+
+  RecString subjects_char;
+  REC_ReadConfigStringAlloc(subjects_char, "proxy.config.acl.subjects");
+  std::string_view            subjects_sv{subjects_char};
+  int                         i = 0;
+  std::string_view::size_type s, e;
+  for (s = 0, e = 0; s < subjects_sv.size() && e != subjects_sv.npos; s = e + 1) {
+    e                           = subjects_sv.find(",", s);
+    std::string_view subject_sv = subjects_sv.substr(s, e);
+    if (i >= MAX_SUBJECTS) {
+      Error("Too many ACL subjects were provided");
+    }
+    if (subject_sv == "PEER") {
+      subjects[i] = Subject::PEER;
+      ++i;
+    } else if (subject_sv == "PROXY") {
+      subjects[i] = Subject::PROXY;
+      ++i;
+    } else {
+      Dbg(dbg_ctl_ip_allow, "Unknown subject %.*s was ignored", static_cast<int>(subject_sv.length()), subject_sv.data());
+    }
+  }
+  if (i < Subject::MAX_SUBJECTS) {
+    subjects[i] = Subject::MAX_SUBJECTS;
+  }
+  ats_free(subjects_char);
 }
 
 BufferWriter &
diff --git a/src/proxy/http/HttpSessionAccept.cc b/src/proxy/http/HttpSessionAccept.cc
@@ -39,6 +39,16 @@ HttpSessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferReade
   IpAllow::ACL        acl;
   ip_port_text_buffer ipb;
 
+  for (int i = 0; i < IpAllow::Subject::MAX_SUBJECTS; ++i) {
+    if (IpAllow::Subject::PEER == IpAllow::subjects[i]) {
+      client_ip = netvc->get_remote_addr();
+      break;
+    } else if (IpAllow::Subject::PROXY == IpAllow::subjects[i] &&
+               netvc->get_proxy_protocol_version() != ProxyProtocolVersion::UNDEFINED) {
+      client_ip = netvc->get_proxy_protocol_src_addr();
+      break;
+    }
+  }
   acl = IpAllow::match(client_ip, IpAllow::SRC_ADDR);
   if (!acl.isValid()) { // if there's no ACL, it's a hard deny.
     Warning("client '%s' prohibited by ip-allow policy", ats_ip_ntop(client_ip, ipb, sizeof(ipb)));
diff --git a/src/proxy/http/remap/UrlRewrite.cc b/src/proxy/http/remap/UrlRewrite.cc
@@ -460,6 +460,26 @@ UrlRewrite::PerformACLFiltering(HttpTransact::State *s, const url_mapping *const
     int method_wksidx = (method != -1) ? (method - HTTP_WKSIDX_CONNECT) : -1;
 
     ink_release_assert(ats_is_ip(&s->client_info.src_addr));
+    const IpEndpoint    *src_addr   = nullptr;
+    const IpEndpoint    *local_addr = nullptr;
+    const ProxyProtocol &pp_info    = s->state_machine->get_ua_txn()->get_netvc()->get_proxy_protocol_info();
+    for (int i = 0; i < IpAllow::Subject::MAX_SUBJECTS; ++i) {
+      if (IpAllow::Subject::PEER == IpAllow::subjects[i]) {
+        src_addr   = &s->client_info.src_addr;
+        local_addr = &s->client_info.dst_addr;
+        break;
+      } else if (IpAllow::Subject::PROXY == IpAllow::subjects[i] && pp_info.version != ProxyProtocolVersion::UNDEFINED) {
+        src_addr   = &pp_info.src_addr;
+        local_addr = &pp_info.dst_addr;
+        break;
+      }
+    }
+
+    if (src_addr == nullptr) {
+      // Use addresses from peer if none of the configured sources are avaialable
+      src_addr   = &s->client_info.src_addr;
+      local_addr = &s->client_info.dst_addr;
+    }
 
     s->client_connection_allowed = true; // Default is that we allow things unless some filter matches
 
@@ -487,7 +507,7 @@ UrlRewrite::PerformACLFiltering(HttpTransact::State *s, const url_mapping *const
       if (rp->src_ip_valid) {
         bool src_ip_matches = false;
         for (int j = 0; j < rp->src_ip_cnt && !src_ip_matches; j++) {
-          bool in_range = rp->src_ip_array[j].contains(s->client_info.src_addr);
+          bool in_range = rp->src_ip_array[j].contains(*src_addr);
           if (rp->src_ip_array[j].invert) {
             if (!in_range) {
               src_ip_matches = true;
@@ -506,7 +526,7 @@ UrlRewrite::PerformACLFiltering(HttpTransact::State *s, const url_mapping *const
       if (ip_matches && rp->src_ip_category_valid) {
         bool category_ip_matches = false;
         for (int j = 0; j < rp->src_ip_category_cnt && !category_ip_matches; j++) {
-          bool in_category = rp->src_ip_category_array[j].contains(s->client_info.src_addr);
+          bool in_category = rp->src_ip_category_array[j].contains(*src_addr);
           if (rp->src_ip_category_array[j].invert) {
             if (!in_category) {
               category_ip_matches = true;
@@ -525,16 +545,14 @@ UrlRewrite::PerformACLFiltering(HttpTransact::State *s, const url_mapping *const
       if (ip_matches && rp->in_ip_valid) {
         bool in_ip_matches = false;
         for (int j = 0; j < rp->in_ip_cnt && !in_ip_matches; j++) {
-          IpEndpoint incoming_addr;
-          incoming_addr.assign(s->state_machine->get_ua_txn()->get_netvc()->get_local_addr());
           if (dbg_ctl_url_rewrite.on()) {
             char buf1[128], buf2[128], buf3[128];
-            ats_ip_ntop(incoming_addr, buf1, sizeof(buf1));
+            ats_ip_ntop(local_addr, buf1, sizeof(buf1));
             rp->in_ip_array[j].start.toString(buf2, sizeof(buf2));
             rp->in_ip_array[j].end.toString(buf3, sizeof(buf3));
             Dbg(dbg_ctl_url_rewrite, "Trying to match incoming address %s in range %s - %s.", buf1, buf2, buf3);
           }
-          bool in_range = rp->in_ip_array[j].contains(incoming_addr);
+          bool in_range = rp->in_ip_array[j].contains(*local_addr);
           if (rp->in_ip_array[j].invert) {
             if (!in_range) {
               in_ip_matches = true;
diff --git a/src/proxy/http2/Http2SessionAccept.cc b/src/proxy/http2/Http2SessionAccept.cc
@@ -42,8 +42,20 @@ Http2SessionAccept::~Http2SessionAccept() = default;
 bool
 Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferReader *reader)
 {
-  sockaddr const *client_ip   = netvc->get_remote_addr();
-  IpAllow::ACL    session_acl = IpAllow::match(client_ip, IpAllow::SRC_ADDR);
+  sockaddr const *client_ip = nullptr;
+
+  for (int i = 0; i < IpAllow::Subject::MAX_SUBJECTS; ++i) {
+    if (IpAllow::Subject::PEER == IpAllow::subjects[i]) {
+      client_ip = netvc->get_remote_addr();
+      break;
+    } else if (IpAllow::Subject::PROXY == IpAllow::subjects[i] &&
+               netvc->get_proxy_protocol_version() != ProxyProtocolVersion::UNDEFINED) {
+      client_ip = netvc->get_proxy_protocol_src_addr();
+      break;
+    }
+  }
+
+  IpAllow::ACL session_acl = IpAllow::match(client_ip, IpAllow::SRC_ADDR);
   if (!session_acl.isValid()) {
     ip_port_text_buffer ipb;
     Warning("HTTP/2 client '%s' prohibited by ip-allow policy", ats_ip_ntop(client_ip, ipb, sizeof(ipb)));
diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc
@@ -140,6 +140,14 @@ static const RecordElement RecordsConfig[] =
   {RECT_CONFIG, "proxy.config.srv_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
   ,
 
+  //##############################################################################
+  //#
+  //# ACL
+  //#
+  //##############################################################################
+  {RECT_CONFIG, "proxy.config.acl.subjects", RECD_STRING, "PEER", RECU_RESTART_TS, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
+  ,
+
   //##############################################################################
   //#
   //# Support for disabling check for Accept-* / Content-* header mismatch
diff --git a/tests/gold_tests/ip_allow/ip_allow.test.py b/tests/gold_tests/ip_allow/ip_allow.test.py
@@ -242,7 +242,8 @@ def _configure_traffic_server(self, tr: 'TestRun'):
 
         :param tr: The TestRun object to associate the ts process with.
         """
-        ts = tr.MakeATSProcess(f"ts-{Test_ip_allow.ts_counter}", enable_quic=self.is_h3, enable_tls=True)
+        ts = tr.MakeATSProcess(
+            f"ts-{Test_ip_allow.ts_counter}", enable_quic=self.is_h3, enable_tls=True, enable_proxy_protocol=True)
 
         Test_ip_allow.ts_counter += 1
         self._ts = ts
@@ -252,13 +253,14 @@ def _configure_traffic_server(self, tr: 'TestRun'):
         self._ts.Disk.records_config.update(
             {
                 'proxy.config.diags.debug.enabled': 1,
-                'proxy.config.diags.debug.tags': 'v_quic|quic|http|ip_allow',
+                'proxy.config.diags.debug.tags': 'v_quic|quic|http|ip_allow|proxyprotocol',
                 'proxy.config.http.push_method_enabled': 1,
                 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
                 'proxy.config.quic.no_activity_timeout_in': 0,
                 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
                 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE',
                 'proxy.config.http.connect_ports': f"{self._server.Variables.http_port}",
+                'proxy.config.acl.subjects': 'PROXY,PEER',
             })
 
         self._ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{self._server.Variables.http_port}')
@@ -278,6 +280,7 @@ def run(self):
         tr.AddVerifierClientProcess(
             f'client-{Test_ip_allow.client_counter}',
             self.replay_file,
+            http_ports=[self._ts.Variables.proxy_protocol_port],
             https_ports=[self._ts.Variables.ssl_port],
             http3_ports=[self._ts.Variables.ssl_port],
             keys=self.replay_keys)
@@ -333,3 +336,20 @@ def run(self):
     is_h3=False,
     expect_request_rejected=False)
 test_ip_allow_optional_methods.run()
+
+# TEST 7: Verify IP address from PROXY protocol is used.
+IP_ALLOW_CONFIG_PROXY_PROTOCOL = '''ip_allow:
+  - apply: in
+    ip_addrs: 1.2.3.4
+    action: allow
+  - apply: in
+    ip_addrs: 0/0
+    action: deny
+'''
+test_ip_allow_proxy_protocol = Test_ip_allow(
+    "ip_allow_proxy_protocol",
+    replay_file='replays/http_proxy_protocol.replay.yaml',
+    ip_allow_config=IP_ALLOW_CONFIG_PROXY_PROTOCOL,
+    is_h3=False,
+    expect_request_rejected=False)
+test_ip_allow_proxy_protocol.run()
diff --git a/tests/gold_tests/ip_allow/replays/http_proxy_protocol.replay.yaml b/tests/gold_tests/ip_allow/replays/http_proxy_protocol.replay.yaml
@@ -0,0 +1,54 @@
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+# The replay file executes various HTTP requests to verify the ip_allow policy
+# applies by default to all methods.
+
+meta:
+  version: "1.0"
+
+  blocks:
+  - standard_response: &standard_response
+      server-response:
+        status: 200
+        reason: OK
+        headers:
+          fields:
+          - [ Content-Length, 20 ]
+sessions:
+- protocol:
+  - name: http
+    version: 1
+  - name: proxy-protocol
+    version: 2
+    src-addr: "1.2.3.4:1111"
+    dst-addr: "5.6.7.8:2222"
+  transactions:
+
+  # GET
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /test/ip_allow/test_get
+      headers:
+        fields:
+        - [ Content-Length, 0 ]
+        - [ uuid, 1 ]
+
+    <<: *standard_response
+
+    proxy-response:
+      status: 200
diff --git a/tests/gold_tests/remap/remap_acl.test.py b/tests/gold_tests/remap/remap_acl.test.py
diff --git a/tests/gold_tests/remap/remap_acl_get_post_allowed_pp.replay.yaml b/tests/gold_tests/remap/remap_acl_get_post_allowed_pp.replay.yaml
diff --git a/tests/gold_tests/tls/replay/ip_allow_proxy.replay.yaml b/tests/gold_tests/tls/replay/ip_allow_proxy.replay.yaml
diff --git a/tests/gold_tests/tls/tls_sni_ip_allow.test.py b/tests/gold_tests/tls/tls_sni_ip_allow.test.py
