Skip to content

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Feb 2, 2024

This continues in the vein of #9631 by implementing the following ACL changes for ATS 10 which makes remap.config ACLs behave more similarly to ip_allow.yaml:

  1. Fix @action=allow to deny transactions that are not in the allow list. Transactions with non-allowed methods simply didn't match the rule before, so they were not denied. This seems like a bug to me.
  2. Change in-line ACL rules to match before activated named ACL rules.
  3. If an ACL matches and is run, implicitly stop processing ip_allow.yaml rules.

@bneradt bneradt added this to the 10.0.0 milestone Feb 2, 2024
@bneradt bneradt self-assigned this Feb 2, 2024
@bneradt bneradt force-pushed the remap_acl_changes_for_10 branch 3 times, most recently from 66aa325 to 2bcadc6 Compare February 2, 2024 19:30
@bryancall bryancall requested a review from zwoop February 5, 2024 23:25
If a user specifies an @action=allow remap.config ACL rule, then the
implication is that requests with methods not in the allow list would be
denied. Before this patch, allow ACL rules would just never deny. This
fixes that behavior so that allow ACL rules that match on IP but not on
method deny.
This switches the order of how ACls are processed. Before this patch,
defined rules were matched before remap ACL lines. Then ip_allow.yaml
rules were run. This does two things:

1. Make remap.config ACL lines run before defined names.
2. If an ACL rule matches, it turns off ip_allow.yaml processing since a
   rule was already matched.

This will likely obviate the need for a lot of uses of
`.deactivatefilter ip_allow` since a matched rule will turn off
ip_allow.yaml processing.
@bneradt bneradt force-pushed the remap_acl_changes_for_10 branch 2 times, most recently from 86b4755 to 17168c2 Compare February 8, 2024 23:29
@bneradt bneradt force-pushed the remap_acl_changes_for_10 branch from 17168c2 to f4b98d7 Compare February 8, 2024 23:55
@bneradt bneradt merged commit 762d87c into apache:master Feb 11, 2024
@bneradt bneradt deleted the remap_acl_changes_for_10 branch February 11, 2024 02:48
bryancall added a commit to bryancall/trafficserver that referenced this pull request Aug 12, 2024
Documentation for these PRs:

 apache#10910 - Change default C++ standard to 20
 apache#11033 - Remap ACL changes for 10
 apache#11045 - Deprecate the support for NPN
 apache#11171 - Remove symbols with prefix INKUDP from plugin API
bryancall added a commit that referenced this pull request Aug 15, 2024
Documentation for these PRs:

 #10910 - Change default C++ standard to 20
 #11033 - Remap ACL changes for 10
 #11045 - Deprecate the support for NPN
 #11171 - Remove symbols with prefix INKUDP from plugin API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants