Thank you for starting this discussion. I have three comments: # 1. "Match on IP and Method Policy"
Let me clarify one thing about the current default confing called "Match on IP and Method Policy". It employs the "first match wins" policy. This is an improvement from ATS 9.2.x, making Inline Filter, Named Filter, and ip_allow.yaml work well. - e.g. deny the PUT method in ip_allow.yaml for general cases, but allow it on a specific remap rule. # 2. ATS 10.0.0 I would prefer to avoid making any additional changes of ACL in the last minuetes of ATS 10.0.0 release unless there is a serious bug. The target of this discussion should be ATS 10.1.0 or 11.0.0. # 3. `add_allow` and `add_deny` actions I understand that if the "Match on IP only Policy" has `add_allow` and `add_deny` actions, it can be superset. However, I don't see any reason to make "Match on IP only Policy" as the default config and deprecate the "Match on IP and Method Policy". The biggest concern with having "Match on IP only Policy" as the default is, it might accidentaly allow methods that can be a security hole. For example, when a user configures a remap rule like below, as you described, ``` map http://www.example.com/ http://internal.example.com/ \ @action=deny @method=POST ``` This allows ALL requests except POST request with "Match on IP only Policy". That means PURGE and PUSH requests are allowed despite they're denied in ip_allow.yaml in default. I assume, you're proposing the `add_deny` action for this case and suggesting use it instead of the `deny` action. However, even if it has additional knobs, this is still a trap. Thanks, Masaori On Wed, Jul 17, 2024 at 6:32 AM Brian Neradt <brian.ner...@gmail.com> wrote: > > dev@trafficserver.apache.org: > > Matty Williams, Masaori Koshiba, and I have been working on polishing up > ACL filter rules for 10.0.x. Masaori and Matty worked on a configuration > that keeps 10.0.x ACL filter action behavior, by default, the same as it > was for ATS 9.x via the following PRs: > > > - https://github.com/apache/trafficserver/pull/11433 > - https://github.com/apache/trafficserver/pull/11448 > > While collaborating with them on that work, it occurred to me that the > additive action behavior of 9.x ACL filter `allow` and `deny` actions can > be valuable for the new 10.x behavior as well. As a result, I suggest the > following incremental change upon our work so far to ACL filters for 10.x: > > - Name our two ACL action modes "legacy" and "modern". The legacy policy > makes the allow and deny ACL filter actions behave as they do for 9.x: they > additively layer allowed or denied methods on top of other ip_allow.yaml > rules. The modern behavior makes the allow and deny rules act like ip_allow > rules: they fully specify how requests will be accepted or denied in the > rule itself. > - Add `add_allow` and `add_deny` ACL filter actions in addition to the > already existing `allow` and `deny` actions. These actions behave the same > between both legacy and modern ACL filter action behavior modes: they act > like `allow` and `deny` actions behaved in 9.x and before, adding methods > to allow or deny lists. > - The legacy mode will be introduced as deprecated for 10.x and will be > removed in 11.x. ACL rules will behave as modern from 11.x on. > > Note that with this change, modern mode is a feature superset on top of > legacy: that is, it allows a user to specify both ip_allow like rules via > `allow` and `deny` actions while additively setting allowed or denied > methods via `add_allow` and `add_deny` actions. > > To give some examples to illustrate these behaviors (these examples are > given in our docs in the draft PR mentioned below): > > map http://www.example.com/ http://internal.example.com/ @action=deny > > @method=POST > > > In legacy mode, this behaves just like it does in 9.x: it adds denial of > POST to requests with the http://www.example.com target, while all other > requests will be handled by other ip_allow or other ACL filter rules. This > is not changed by my suggestion here (other than calling it "legacy" mode). > > In modern mode, this behaves like an ip_allow.yaml rule: POST requests to > http://www.example.com will be denied, while all other requests will be > allowed. (Naturally, this will be an unlikely rule for anyone to configure, > either as a modern ACL filter or ip_allow rule, but for this is helpful for > the purpose of comparison between the two behaviors.) > > The same rule using an `add_deny` action would be the following: > > map http://www.example.com/ http://internal.example.com/ @action=add_deny > > @method=POST > > > In both legacy and modern modes, this rule behaves the same: it acts like a > `deny` rule would have for 9.x. That is, it adds denial of POST requests on > top of any other ip_allow or ACL filter rule that my otherwise apply to > requests to http://www.example.com. > > In case it is helpful, here is a draft PR for the implementation of this > suggestion: > https://github.com/apache/trafficserver/pull/11555 > > Note that the production change is almost trivial: most of the change > involves renaming the configuration and its associated member variables or > updating some tests. > > Please let me know if you have any thoughts or concerns. > > Thank you, > Brian Neradt > -- > "Come to Me, all who are weary and heavy-laden, and I will > give you rest. Take My yoke upon you and learn from Me, for > I am gentle and humble in heart, and you will find rest for > your souls. For My yoke is easy and My burden is light." > > ~ Matthew 11:28-30
