This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 1de573558186b761b514cf1ee39c0aaae4fb4e6a Author: Brian Neradt <[email protected]> AuthorDate: Thu Jul 25 09:13:58 2024 -0500 add_allow and add_deny ACL filter actions (#11568) This adds the add_allow and add_deny filter actions. These actions behave just like the allow and deny actions from pre-10 ACL filters, but makes the difference of their behavior from ip_allow.yaml allow and deny actions explicit. The allow and deny ACL filter actions are also kept, but their behavior depends upon which proxy.config.url_remap.acl_matching_policy is set: * If the policy is 0 (the default), then the allow and deny ACL filter actions act as synonyms for add_allow and add_deny since their behavior is compatible with the pre-10 release. * If the policy is 1 (the new 10 behavior), then the allow and deny ACL filter actions behave like their ip_allow.yaml counterparts. These new actions supplement the additive nature of ACL filters to the proxy.config.url_remap.acl_matching_policy 1 configured value, while also allowing users to explicitly specify this additive nature when the ACL matching policy is set to 0. (cherry picked from commit edc3a0964895efd7851d04c03821d70c3b9fd9f9) --- doc/admin-guide/files/remap.config.en.rst | 23 +++++++++++++++++++ include/proxy/http/remap/AclFiltering.h | 9 +++++++- src/proxy/http/remap/AclFiltering.cc | 32 ++++++++++++++++++++++----- src/proxy/http/remap/RemapConfig.cc | 9 ++++++-- src/proxy/http/remap/UrlRewrite.cc | 10 +++++++-- tests/gold_tests/remap/deactivate_ip_allow.py | 22 ++++++++++++++++++ tests/gold_tests/remap/remap_acl.test.py | 30 +++++++++++++++++++++++++ 7 files changed, 124 insertions(+), 11 deletions(-) diff --git a/doc/admin-guide/files/remap.config.en.rst b/doc/admin-guide/files/remap.config.en.rst index 32f97a3db9..46d4e594cf 100644 --- a/doc/admin-guide/files/remap.config.en.rst +++ b/doc/admin-guide/files/remap.config.en.rst @@ -447,6 +447,29 @@ In-line filters can be created to control access of specific remap lines. The ma is very similar to that of :file:`ip_allow.yaml`, with slight changes to accommodate remap markup. +Actions +~~~~~~~ + +As is the case with :file:`ip_allow.yaml` rules, each ACL filter takes one of a number of actions. They are specified as +``@action=<action>``, such as ``@action=add_allow``. There are four possible actions: + +- ``allow``: This behaves like the ``allow`` action in :file:`ip_allow.yaml` in which a list of allowed methods are + provided. Any request with a method in the list is allowed, while any request with a method not in the list is denied. + The exception to this is if :ts:cv:`proxy.config.url_remap.acl_matching_policy` is set to ``0``. In this case, the + ``allow`` action is a synonym for ``add_allow``, described below. +- ``add_allow``: This action adds a list of allowed methods to whatever other methods are allowed in a subsequently + matched ACL filter or :file:`ip_allow.yaml` rule. Thus, if an ``add_allow`` ACL filter specifies the ``POST`` method, + and a subsequently matching :file:`ip_allow.yaml` rule allows the ``GET`` and ``HEAD`` methods, then any requests that + have ``POST``, ``GET``, or ``HEAD`` methods will be allowed while all others will be denied. +- ``deny``: This behaves like the ``deny`` action in :file:`ip_allow.yaml` in which a list of denied methods are + provided. Any request with a method in the list is denied, while any request with a method not in the list is allowed. + The exception to this is if :ts:cv:`proxy.config.url_remap.acl_matching_policy` is set to ``0``. In this case, the + ``deny`` action is a synonym for ``add_deny``, described below. +- ``add_deny``: This action adds a list of denied methods to whatever other methods are denied in a subsequently matched + ACL filter or :file:`ip_allow.yaml` rule. Thus, if an ``add_deny`` ACL filter specifies the ``POST`` method, and a + matching :file:`ip_allow.yaml` rule allows the ``GET``, ``HEAD``, and ``POST`` methods, then this ACL filter + effectively removes ``POST`` from the allowed method list. Thus only requests with the ``GET`` and ``HEAD`` methods + will be allowed. Examples ~~~~~~~~ diff --git a/include/proxy/http/remap/AclFiltering.h b/include/proxy/http/remap/AclFiltering.h index 618fdc5689..32fe3c0c7a 100644 --- a/include/proxy/http/remap/AclFiltering.h +++ b/include/proxy/http/remap/AclFiltering.h @@ -99,7 +99,8 @@ private: public: acl_filter_rule *next = nullptr; char *filter_name = nullptr; // optional filter name - unsigned int allow_flag : 1, // action allow deny + unsigned int allow_flag : 1, // action allow or add_allow (1); or deny or add_deny (0) + add_flag : 1, // add_allow/add_deny (1) or allow/deny (0) 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 @@ -134,6 +135,12 @@ public: int add_argv(int _argc, char *_argv[]); void print(); + /** Return a description of the action. + * + * @return "allow", "add_allow", "deny", or "add_deny", as appropriate. + */ + char const *get_action_description() const; + static acl_filter_rule *find_byname(acl_filter_rule *list, const char *name); static void delete_byname(acl_filter_rule **list, const char *name); static void requeue_in_active_list(acl_filter_rule **list, acl_filter_rule *rp); diff --git a/src/proxy/http/remap/AclFiltering.cc b/src/proxy/http/remap/AclFiltering.cc index 7dade7d666..db48d09d32 100644 --- a/src/proxy/http/remap/AclFiltering.cc +++ b/src/proxy/http/remap/AclFiltering.cc @@ -67,7 +67,8 @@ acl_filter_rule::reset() 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) +acl_filter_rule::acl_filter_rule() + : allow_flag(1), add_flag(0), 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); @@ -109,11 +110,12 @@ acl_filter_rule::print() { int i; printf("-----------------------------------------------------------------------------------------\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", - src_ip_category_valid ? "true" : "false", in_ip_valid ? "true" : "false", internal ? "true" : "false", - static_cast<int>(active_queue_flag)); + printf( + "Filter \"%s\" status: allow_flag=%s, add_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", add_flag ? "true" : "false", + src_ip_valid ? "true" : "false", 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]) { @@ -150,6 +152,24 @@ acl_filter_rule::print() } } +char const * +acl_filter_rule::get_action_description() const +{ + if (allow_flag) { + if (add_flag) { + return "add_allow"; + } else { + return "allow"; + } + } else { // denying + if (add_flag) { + return "add_deny"; + } else { + return "deny"; + } + } +} + acl_filter_rule * acl_filter_rule::find_byname(acl_filter_rule *list, const char *_name) { diff --git a/src/proxy/http/remap/RemapConfig.cc b/src/proxy/http/remap/RemapConfig.cc index 7c48e44325..0617f6b151 100644 --- a/src/proxy/http/remap/RemapConfig.cc +++ b/src/proxy/http/remap/RemapConfig.cc @@ -628,9 +628,14 @@ remap_validate_filter_args(acl_filter_rule **rule_pp, const char **argv, int arg } if (ul & REMAP_OPTFLG_ACTION) { /* "action=" option */ - if (is_inkeylist(argptr, "0", "off", "deny", "disable", nullptr)) { + if (is_inkeylist(argptr, "add_allow", "add_deny", nullptr)) { + rule->add_flag = 1; + } else { + rule->add_flag = 0; + } + if (is_inkeylist(argptr, "0", "off", "deny", "add_deny", "disable", nullptr)) { rule->allow_flag = 0; - } else if (is_inkeylist(argptr, "1", "on", "allow", "enable", nullptr)) { + } else if (is_inkeylist(argptr, "1", "on", "allow", "add_allow", "enable", nullptr)) { rule->allow_flag = 1; } else { Dbg(dbg_ctl_url_rewrite, "[validate_filter_args] Unknown argument \"%s\"", argv[i]); diff --git a/src/proxy/http/remap/UrlRewrite.cc b/src/proxy/http/remap/UrlRewrite.cc index ccda792b69..8bd0676454 100644 --- a/src/proxy/http/remap/UrlRewrite.cc +++ b/src/proxy/http/remap/UrlRewrite.cc @@ -561,10 +561,16 @@ UrlRewrite::PerformACLFiltering(HttpTransact::State *s, const url_mapping *const break; } - if (_acl_matching_policy == ACLMatchingPolicy::MATCH_ON_IP_ONLY) { + // @action=add_allow and @action=add_deny behave the same for each ACL + // policy behavior. The difference in behavior applies to @action=allow + // and @action=deny. For these, in Match on IP and Method mode they are + // synonyms for @action=add_allow and @action=add_deny because that is + // how they behaved pre-10.x. For the Match on IP Only behavior, they + // behave like the corresponding ip_allow actions. + if (!rp->add_flag && _acl_matching_policy == ACLMatchingPolicy::MATCH_ON_IP_ONLY) { // Flipping the action for unspecified methods. Dbg(dbg_ctl_url_rewrite, "ACL rule matched on IP but not on method, action: %s, %s the request", - (rp->allow_flag ? "allow" : "deny"), (rp->allow_flag ? "denying" : "allowing")); + rp->get_action_description(), (rp->allow_flag ? "denying" : "allowing")); s->client_connection_allowed = !rp->allow_flag; // Since IP match and configured policy is MATCH_ON_IP_ONLY, no need to process other filters nor ip_allow.yaml rules. diff --git a/tests/gold_tests/remap/deactivate_ip_allow.py b/tests/gold_tests/remap/deactivate_ip_allow.py index 45093a2332..7cfabcbc42 100644 --- a/tests/gold_tests/remap/deactivate_ip_allow.py +++ b/tests/gold_tests/remap/deactivate_ip_allow.py @@ -91,6 +91,28 @@ deactivate_ip_allow_combinations = [ [ 27, "ip_and_method", "@action=deny @method=GET", "", True, DENY_GET, 403, 200, ], [ 28, "ip_and_method", "@action=deny @method=GET", "", True, DENY_GET_AND_POST, 403, 200, ], [ 29, "ip_and_method", "@action=deny @method=GET", "", True, DENY_ALL, 403, 200, ], + + # Verify in ip_and_method mode that add_allow acts just like allow, and add_deny acts just like deny. + [ 30, "ip_and_method", "@action=add_allow @method=GET", "", False, ALLOW_GET_AND_POST, 200, 200, ], + [ 31, "ip_and_method", "@action=add_allow @method=GET", "", False, ALLOW_GET, 200, 403, ], + [ 32, "ip_and_method", "@action=add_allow @method=GET", "", False, DENY_GET, 200, 200, ], + [ 33, "ip_and_method", "@action=add_allow @method=GET", "", False, DENY_GET_AND_POST, 200, 403, ], + [ 34, "ip_and_method", "@action=add_allow @method=GET", "", False, DENY_ALL, None, None, ], + [ 35, "ip_and_method", "@action=add_allow @method=GET", "", True, ALLOW_GET_AND_POST, 200, 200, ], + [ 36, "ip_and_method", "@action=add_allow @method=GET", "", True, ALLOW_GET, 200, 200, ], + [ 37, "ip_and_method", "@action=add_allow @method=GET", "", True, DENY_GET, 200, 200, ], + [ 38, "ip_and_method", "@action=add_allow @method=GET", "", True, DENY_GET_AND_POST, 200, 200, ], + [ 39, "ip_and_method", "@action=add_allow @method=GET", "", True, DENY_ALL, 200, 200, ], + [ 40, "ip_and_method", "@action=add_deny @method=GET", "", False, ALLOW_GET_AND_POST, 403, 200, ], + [ 41, "ip_and_method", "@action=add_deny @method=GET", "", False, ALLOW_GET, 403, 403, ], + [ 42, "ip_and_method", "@action=add_deny @method=GET", "", False, DENY_GET, 403, 200, ], + [ 43, "ip_and_method", "@action=add_deny @method=GET", "", False, DENY_GET_AND_POST, 403, 403, ], + [ 44, "ip_and_method", "@action=add_deny @method=GET", "", False, DENY_ALL, None, None, ], + [ 45, "ip_and_method", "@action=add_deny @method=GET", "", True, ALLOW_GET_AND_POST, 403, 200, ], + [ 46, "ip_and_method", "@action=add_deny @method=GET", "", True, ALLOW_GET, 403, 200, ], + [ 47, "ip_and_method", "@action=add_deny @method=GET", "", True, DENY_GET, 403, 200, ], + [ 48, "ip_and_method", "@action=add_deny @method=GET", "", True, DENY_GET_AND_POST, 403, 200, ], + [ 49, "ip_and_method", "@action=add_deny @method=GET", "", True, DENY_ALL, 403, 200, ], ] all_deactivate_ip_allow_tests = [dict(zip(keys, test)) for test in deactivate_ip_allow_combinations] # yapf: enable diff --git a/tests/gold_tests/remap/remap_acl.test.py b/tests/gold_tests/remap/remap_acl.test.py index cd718e2b2f..57b24b3f76 100644 --- a/tests/gold_tests/remap/remap_acl.test.py +++ b/tests/gold_tests/remap/remap_acl.test.py @@ -161,6 +161,26 @@ test_ip_allow_optional_methods = Test_remap_acl( named_acls=[], expected_responses=[200, 200, 403, 403, 403]) +test_ip_allow_optional_methods = Test_remap_acl( + "Verify add_allow adds an allowed method.", + replay_file='remap_acl_get_post_allowed.replay.yaml', + ip_allow_content=IP_ALLOW_CONTENT, + deactivate_ip_allow=False, + acl_matching_policy=1, + acl_configuration='@action=add_allow @src_ip=127.0.0.1 @method=POST', + named_acls=[], + expected_responses=[200, 200, 403, 403, 403]) + +test_ip_allow_optional_methods = Test_remap_acl( + "Verify add_allow adds allowed methods.", + replay_file='remap_acl_get_post_allowed.replay.yaml', + ip_allow_content=IP_ALLOW_CONTENT, + deactivate_ip_allow=False, + acl_matching_policy=1, + acl_configuration='@action=add_allow @src_ip=127.0.0.1 @method=GET @method=POST', + named_acls=[], + expected_responses=[200, 200, 403, 403, 403]) + test_ip_allow_optional_methods = Test_remap_acl( "Verify if no ACLs match, ip_allow.yaml is used.", replay_file='remap_acl_get_allowed.replay.yaml', @@ -211,6 +231,16 @@ test_ip_allow_optional_methods = Test_remap_acl( named_acls=[], expected_responses=[403, 403, 200, 200, 400]) +test_ip_allow_optional_methods = Test_remap_acl( + "Verify add_deny adds blocked methods.", + replay_file='remap_acl_all_denied.replay.yaml', + ip_allow_content=IP_ALLOW_CONTENT, + deactivate_ip_allow=False, + acl_matching_policy=1, + acl_configuration='@action=add_deny @src_ip=127.0.0.1 @method=GET', + named_acls=[], + expected_responses=[403, 403, 403, 403, 403]) + test_ip_allow_optional_methods = Test_remap_acl( "Verify a default deny filter rule works.", replay_file='remap_acl_all_denied.replay.yaml',
