Remap ACL changes for 10

doc/admin-guide/files/remap.config.en.rst

@@ -435,17 +435,18 @@ You may configure Nexthop or Parent hierarchical caching rules by remap using th
 **@strategy** tag.  See :doc:`../configuration/hierarchical-caching.en` and :doc:`strategies.yaml.en`
 for configuration details and examples.
 
-Acl Filters
+ACL Filters
 ===========
 
-Acl filters can be created to control access of specific remap lines. The markup
+ACL filters can be created to control access of specific remap lines. The markup
 is very similar to that of :file:`ip_allow.yaml`, with slight changes to
 accommodate remap markup.
 
 **Note:** As of ATS v10.x, these filters are applied just as :file:`ip_allow.yaml`,
-this means once a filter matches the request, the action for that rule takes effect.
+meaning once a filter matches the request, the action for that rule takes effect.
 In previous versions, all filters for a remap rule were evaluated, and the ``deny``
-action took priority.
+action took priority. Also, if an ACL filter matches, then :file:`ip_allow.yaml` rules
+will not not apply to the request because the matched rule is the ACL filter.
 
 Examples
 --------
@@ -464,11 +465,14 @@ Examples
 
     map http://foo.example.com/  http://foo.example.com/ @action=allow @src_ip_category=ACME_INTERNAL @method=post @method=get @method=head
 
-Note that these Acl filters will return a 403 response if the resource is restricted.
+Note that these ACL filters will return a 403 response if the resource is restricted.
 
 The difference between ``@src_ip`` and ``@in_ip`` is that the ``@src_ip`` is the client
 ip and the ``in_ip`` is the ip address the client is connecting to (the incoming address).
 ``@src_ip_category`` functions like ``ip_category`` described in :file:`ip_allow.yaml`.
+If no IP address is specified for ``@src_ip``, ``@src_ip_category``, or
+``@in_ip``, the filter will implicitly apply to all incoming IP addresses. This
+can be explicitly stated with ``@src_ip=all``.
 
 Named Filters
 =============
@@ -486,6 +490,7 @@ is helpful for remapping internal requests without allowing access
 to external users. By default both internal and external requests
 are allowed.
 
+In-line ACL filters take priority over named active ACL filters.
 
 Examples
 --------

include/proxy/http/HttpTransact.h

@@ -682,7 +682,7 @@ class HttpTransact
     bool is_upgrade_request           = false;
     bool is_websocket                 = false;
     bool did_upgrade_succeed          = false;
-    bool client_connection_enabled    = true;
+    bool client_connection_allowed    = true;
     bool acl_filtering_performed      = false;
     bool api_cleanup_cache_read       = false;
     bool api_server_response_no_store = false;

include/proxy/http/remap/AclFiltering.h

@@ -40,22 +40,27 @@ static int const ACL_FILTER_MAX_IN_IP  = 8;
 static int const ACL_FILTER_MAX_ARGV   = 512;
 
 struct src_ip_info_t {
-  IpAddr start; ///< Minimum value in range.
-  IpAddr end;   ///< Maximum value in range.
-  bool invert;  ///< Should we "invert" the meaning of this IP range ("not in range")
+  IpAddr start;             ///< Minimum value in range.
+  IpAddr end;               ///< Maximum value in range.
+  bool invert;              ///< Should we "invert" the meaning of this IP range ("not in range")
+  bool match_all_addresses; ///< This rule should match all IP addresses.
 
   void
   reset()
   {
     start.invalidate();
     end.invalidate();
-    invert = false;
+    invert              = false;
+    match_all_addresses = false;
   }
 
   /// @return @c true if @a ip is inside @a this range.
   bool
   contains(IpEndpoint const &ip) const
   {
+    if (match_all_addresses) {
+      return true;
+    }
     IpAddr addr{ip};
     return addr.cmp(start) >= 0 && addr.cmp(end) <= 0;
   }
@@ -95,11 +100,11 @@ class acl_filter_rule
   acl_filter_rule *next = nullptr;
   char *filter_name     = nullptr; // optional filter name
   unsigned int allow_flag : 1,     // action allow deny
-    src_ip_valid          : 1,     // src_ip range valid
-    src_ip_category_valid : 1,     // src_ip range valid
-    in_ip_valid           : 1,
-    active_queue_flag     : 1, // filter is in active state (used by .useflt directive)
-    internal              : 1; // filter internal HTTP requests
+    src_ip_valid          : 1,     // src_ip (client's src IP) range is specified and valid
+    src_ip_category_valid : 1,     // src_ip_category (client's src IP category) is specified and valid
+    in_ip_valid           : 1,     // in_ip (client's dest IP) range is specified and valid
+    active_queue_flag     : 1,     // filter is in active state (used by .useflt directive)
+    internal              : 1;     // filter internal HTTP requests
 
   // we need arguments as string array for directive processing
   int argc = 0;                    // argument counter (only for filter defs)

include/proxy/http/remap/UrlMapping.h

@@ -116,7 +116,7 @@ class url_mapping
   referer_info *referer_list         = nullptr;
   redirect_tag_str *redir_chunk_list = nullptr;
   bool ip_allow_check_enabled_p      = false;
-  acl_filter_rule *filter            = nullptr; // acl filtering (list of rules)
+  acl_filter_rule *filter            = nullptr; // acl filtering (linked list of rules)
   LINK(url_mapping, link);                      // For use with the main Queue linked list holding all the mapping
   std::shared_ptr<NextHopSelectionStrategy> strategy = nullptr;
   std::string remapKey;

src/proxy/http/HttpSM.cc

@@ -4303,7 +4303,7 @@ HttpSM::check_sni_host()
         swoc::bwprint(error_bw_buffer, "No SNI for TLS request: connecting to {} for host='{}', returning a 403",
                       t_state.client_info.dst_addr, std::string_view{host_name, static_cast<size_t>(host_len)});
         Log::error("%s", error_bw_buffer.c_str());
-        this->t_state.client_connection_enabled = false;
+        this->t_state.client_connection_allowed = false;
       }
     } else if (strncasecmp(host_name, sni_value, host_len) != 0) { // Name mismatch
       Warning("SNI/hostname mismatch sni=%s host=%.*s action=%s", sni_value, host_len, host_name, action_value);
@@ -4312,7 +4312,7 @@ HttpSM::check_sni_host()
         swoc::bwprint(error_bw_buffer, "SNI/hostname mismatch: connecting to {} for host='{}' sni='{}', returning a 403",
                       t_state.client_info.dst_addr, std::string_view{host_name, static_cast<size_t>(host_len)}, sni_value);
         Log::error("%s", error_bw_buffer.c_str());
-        this->t_state.client_connection_enabled = false;
+        this->t_state.client_connection_allowed = false;
       }
     } else {
       SMDbg(dbg_ctl_ssl_sni, "SNI/hostname successfully match sni=%s host=%.*s", sni_value, host_len, host_name);

src/proxy/http/HttpTransact.cc

@@ -1107,7 +1107,7 @@ HttpTransact::EndRemapRequest(State *s)
   /////////////////////////////////////////////////////////////////////////
   // We must close this connection if client_connection_enabled == false //
   /////////////////////////////////////////////////////////////////////////
-  if (!s->client_connection_enabled) {
+  if (!s->client_connection_allowed) {
     build_error_response(s, HTTP_STATUS_FORBIDDEN, "Access Denied", "access#denied");
     s->reverse_proxy = false;
     goto done;
@@ -6489,7 +6489,7 @@ void
 HttpTransact::process_quick_http_filter(State *s, int method)
 {
   // connection already disabled by previous ACL filtering, don't modify it.
-  if (!s->client_connection_enabled) {
+  if (!s->client_connection_allowed) {
     return;
   }
 
@@ -6525,7 +6525,7 @@ HttpTransact::process_quick_http_filter(State *s, int method)
         TxnDbg(dbg_ctl_ip_allow, "Line %d denial for '%.*s' from %s", acl.source_line(), method_str_len, method_str,
                ats_ip_ntop(&s->client_info.src_addr.sa, ipb, sizeof(ipb)));
       }
-      s->client_connection_enabled = false;
+      s->client_connection_allowed = false;
     }
   }
 }

src/proxy/http/remap/AclFiltering.cc

@@ -55,6 +55,7 @@ acl_filter_rule::reset()
   for (i = (src_ip_cnt = 0); i < ACL_FILTER_MAX_SRC_IP; i++) {
     src_ip_array[i].reset();
   }
+  src_ip_category_valid = 0;
   for (i = (src_ip_category_cnt = 0); i < ACL_FILTER_MAX_SRC_IP; i++) {
     src_ip_category_array[i].reset();
   }
@@ -66,7 +67,7 @@ acl_filter_rule::reset()
   internal    = 0;
 }
 
-acl_filter_rule::acl_filter_rule() : allow_flag(1), src_ip_valid(0), active_queue_flag(0), internal(0)
+acl_filter_rule::acl_filter_rule() : allow_flag(1), src_ip_valid(0), src_ip_category_valid(0), active_queue_flag(0), internal(0)
 {
   standard_method_lookup.resize(HTTP_WKSIDX_METHODS_CNT);
   ink_zero(argv);
@@ -108,9 +109,11 @@ acl_filter_rule::print()
 {
   int i;
   printf("-----------------------------------------------------------------------------------------\n");
-  printf("Filter \"%s\" status: allow_flag=%s, src_ip_valid=%s, in_ip_valid=%s, internal=%s, active_queue_flag=%d\n",
+  printf("Filter \"%s\" status: allow_flag=%s, src_ip_valid=%s, src_ip_category_valid=%s, in_ip_valid=%s, internal=%s, "
+         "active_queue_flag=%d\n",
          filter_name ? filter_name : "<NONAME>", allow_flag ? "true" : "false", src_ip_valid ? "true" : "false",
-         in_ip_valid ? "true" : "false", internal ? "true" : "false", static_cast<int>(active_queue_flag));
+         src_ip_category_valid ? "true" : "false", in_ip_valid ? "true" : "false", internal ? "true" : "false",
+         static_cast<int>(active_queue_flag));
   printf("standard methods=");
   for (i = 0; i < HTTP_WKSIDX_METHODS_CNT; i++) {
     if (standard_method_lookup[i]) {
@@ -126,18 +129,20 @@ acl_filter_rule::print()
   printf("src_ip_cnt=%d\n", src_ip_cnt);
   for (i = 0; i < src_ip_cnt; i++) {
     ip_text_buffer b1, b2;
-    printf("%s - %s, ", src_ip_array[i].start.toString(b1, sizeof(b1)), src_ip_array[i].end.toString(b2, sizeof(b2)));
+    printf("%s - %s/invert=%s, ", src_ip_array[i].start.toString(b1, sizeof(b1)), src_ip_array[i].end.toString(b2, sizeof(b2)),
+           src_ip_array[i].invert ? "true" : "false");
   }
   printf("\n");
   printf("src_ip_category_cnt=%d\n", src_ip_category_cnt);
   for (i = 0; i < src_ip_category_cnt; i++) {
-    printf("%s, ", src_ip_category_array[i].category.c_str());
+    printf("%s/invert=%s, ", src_ip_category_array[i].category.c_str(), src_ip_category_array[i].invert ? "true" : "false");
   }
   printf("\n");
   printf("in_ip_cnt=%d\n", in_ip_cnt);
   for (i = 0; i < in_ip_cnt; i++) {
     ip_text_buffer b1, b2;
-    printf("%s - %s, ", in_ip_array[i].start.toString(b1, sizeof(b1)), in_ip_array[i].end.toString(b2, sizeof(b2)));
+    printf("%s - %s/invert=%s, ", in_ip_array[i].start.toString(b1, sizeof(b1)), in_ip_array[i].end.toString(b2, sizeof(b2)),
+           in_ip_array[i].invert ? "true" : "false");
   }
   printf("\n");
   for (i = 0; i < argc; i++) {

src/proxy/http/remap/RemapConfig.cc

@@ -21,6 +21,7 @@
  *  limitations under the License.
  */
 
+#include "proxy/http/remap/AclFiltering.h"
 #include "swoc/swoc_file.h"
 
 #include "proxy/http/remap/RemapConfig.h"
@@ -111,24 +112,31 @@ process_filter_opt(url_mapping *mp, const BUILD_TABLE_INFO *bti, char *errStrBuf
     Debug("url_rewrite", "[process_filter_opt] Invalid argument(s)");
     return (const char *)"[process_filter_opt] Invalid argument(s)";
   }
-  for (rp = bti->rules_list; rp; rp = rp->next) {
-    if (rp->active_queue_flag) {
-      Debug("url_rewrite", "[process_filter_opt] Add active main filter \"%s\" (argc=%d)",
-            rp->filter_name ? rp->filter_name : "<nullptr>", rp->argc);
-      for (rpp = &mp->filter; *rpp; rpp = &((*rpp)->next)) {
-        ;
-      }
-      if ((errStr = remap_validate_filter_args(rpp, (const char **)rp->argv, rp->argc, errStrBuf, errStrBufSize)) != nullptr) {
-        break;
-      }
-    }
-  }
+  // ACLs are processed in this order:
+  // 1. A remap.config ACL line for an individual remap rule.
+  // 2. All named ACLs in remap.config.
+  // 3. Rules as specified in ip_allow.yaml.
   if (!errStr && (bti->remap_optflg & REMAP_OPTFLG_ALL_FILTERS) != 0) {
     Debug("url_rewrite", "[process_filter_opt] Add per remap filter");
     for (rpp = &mp->filter; *rpp; rpp = &((*rpp)->next)) {
       ;
     }
     errStr = remap_validate_filter_args(rpp, (const char **)bti->argv, bti->argc, errStrBuf, errStrBufSize);
+    for (rp = bti->rules_list; rp; rp = rp->next) {
+      for (rpp = &mp->filter; *rpp; rpp = &((*rpp)->next)) {
+        ;
+      }
+      if (rp->active_queue_flag) {
+        Debug("url_rewrite", "[process_filter_opt] Add active main filter \"%s\" (argc=%d)",
+              rp->filter_name ? rp->filter_name : "<nullptr>", rp->argc);
+        for (rpp = &mp->filter; *rpp; rpp = &((*rpp)->next)) {
+          ;
+        }
+        if ((errStr = remap_validate_filter_args(rpp, (const char **)rp->argv, rp->argc, errStrBuf, errStrBufSize)) != nullptr) {
+          break;
+        }
+      }
+    }
   }
   // Set the ip allow flag for this rule to the current ip allow flag state
   mp->ip_allow_check_enabled_p = bti->ip_allow_check_enabled_p;
@@ -449,6 +457,7 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
     Debug("url_rewrite", "[validate_filter_args] new acl_filter_rule class was created during remap rule processing");
   }
 
+  bool ip_is_listed = false;
   for (i = 0; i < argc; i++) {
     unsigned long ul;
     bool hasarg;
@@ -508,7 +517,10 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
       if (ul & REMAP_OPTFLG_INVERT) {
         ipi->invert = true;
       }
-      if (ats_ip_range_parse(argptr, ipi->start, ipi->end) != 0) {
+      std::string_view arg{argptr};
+      if (arg == "all") {
+        ipi->match_all_addresses = true;
+      } else if (ats_ip_range_parse(arg, ipi->start, ipi->end) != 0) {
         Debug("url_rewrite", "[validate_filter_args] Unable to parse IP value in %s", argv[i]);
         snprintf(errStrBuf, errStrBufSize, "Unable to parse IP value in %s", argv[i]);
         errStrBuf[errStrBufSize - 1] = 0;
@@ -528,6 +540,7 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
       if (ipi) {
         rule->src_ip_cnt++;
         rule->src_ip_valid = 1;
+        ip_is_listed       = true;
       }
     }
 
@@ -543,6 +556,7 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
         return (const char *)errStrBuf;
       }
       src_ip_category_info_t *ipi = &rule->src_ip_category_array[rule->src_ip_category_cnt];
+      ipi->category.assign(argptr);
       if (ul & REMAP_OPTFLG_INVERT) {
         ipi->invert = true;
       }
@@ -556,10 +570,11 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
       if (ipi) {
         rule->src_ip_category_cnt++;
         rule->src_ip_category_valid = 1;
+        ip_is_listed                = true;
       }
     }
 
-    if (ul & REMAP_OPTFLG_IN_IP) { /* "dest_ip=" option */
+    if (ul & REMAP_OPTFLG_IN_IP) { /* "in_ip=" option */
       if (rule->in_ip_cnt >= ACL_FILTER_MAX_IN_IP) {
         Debug("url_rewrite", "[validate_filter_args] Too many \"in_ip=\" filters");
         snprintf(errStrBuf, errStrBufSize, "Defined more than %d \"in_ip=\" filters!", ACL_FILTER_MAX_IN_IP);
@@ -575,7 +590,10 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
         ipi->invert = true;
       }
       // important! use copy of argument
-      if (ats_ip_range_parse(argptr, ipi->start, ipi->end) != 0) {
+      std::string_view arg{argptr};
+      if (arg == "all") {
+        ipi->match_all_addresses = true;
+      } else if (ats_ip_range_parse(arg, ipi->start, ipi->end) != 0) {
         Debug("url_rewrite", "[validate_filter_args] Unable to parse IP value in %s", argv[i]);
         snprintf(errStrBuf, errStrBufSize, "Unable to parse IP value in %s", argv[i]);
         errStrBuf[errStrBufSize - 1] = 0;
@@ -595,6 +613,7 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
       if (ipi) {
         rule->in_ip_cnt++;
         rule->in_ip_valid = 1;
+        ip_is_listed      = true;
       }
     }
 
@@ -620,6 +639,15 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg
     }
   }
 
+  if (!ip_is_listed) {
+    // If no IP addresses are listed, treat that like `@src_ip=all`.
+    ink_release_assert(rule->src_ip_valid == 0 && rule->src_ip_cnt == 0);
+    src_ip_info_t *ipi       = &rule->src_ip_array[rule->src_ip_cnt];
+    ipi->match_all_addresses = true;
+    rule->src_ip_cnt++;
+    rule->src_ip_valid = 1;
+  }
+
   if (is_debug_tag_set("url_rewrite")) {
     rule->print();
   }
@@ -675,7 +703,7 @@ remap_check_option(const char **argv, int argc, unsigned long findmode, int *_re
           *argptr = &argv[i][8];
         }
         ret_flags |= (REMAP_OPTFLG_SRC_IP | REMAP_OPTFLG_INVERT);
-      } else if (!strncasecmp(argv[i], "src_ip_category=~", 8)) {
+      } else if (!strncasecmp(argv[i], "src_ip_category=~", 17)) {
         if ((findmode & REMAP_OPTFLG_SRC_IP_CATEGORY) != 0) {
           idx = i;
         }
@@ -691,6 +719,14 @@ remap_check_option(const char **argv, int argc, unsigned long findmode, int *_re
           *argptr = &argv[i][7];
         }
         ret_flags |= REMAP_OPTFLG_SRC_IP;
+      } else if (!strncasecmp(argv[i], "src_ip_category=", 16)) {
+        if ((findmode & REMAP_OPTFLG_SRC_IP_CATEGORY) != 0) {
+          idx = i;
+        }
+        if (argptr) {
+          *argptr = &argv[i][16];
+        }
+        ret_flags |= REMAP_OPTFLG_SRC_IP_CATEGORY;
       } else if (!strncasecmp(argv[i], "in_ip=~", 7)) {
         if ((findmode & REMAP_OPTFLG_IN_IP) != 0) {
           idx = i;