Skip to content

Commit 1d6bbdc

Browse files
cmcfarlencmcfarlen
committed
Fix legacy acl (#12078)
* Update legacy mode and fix autests (cherry picked from commit 0b37b19db7ac5c258176c1d567893af48feb89c2) * match expected remap for legacy mode to 9.2 (cherry picked from commit 631ff2554c574981e41a4aa76412e0b77618b888) * add more tests with src_ip (cherry picked from commit 3d4ec00c8c54822ec97cfa782204e060b7e17dd5) --------- Co-authored-by: Chris McFarlen <cmcfarlen@apple.com> (cherry picked from commit 736f3c6)
1 parent 86c12d4 commit 1d6bbdc

4 files changed

Lines changed: 116 additions & 76 deletions

File tree

‎src/proxy/http/remap/UrlRewrite.cc‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,23 @@ UrlRewrite::PerformACLFiltering(HttpTransact::State *s, const url_mapping *const
557557
Dbg(dbg_ctl_url_rewrite, "%d: ACL filter %s rule matches by ip: %s, by method: %s", rule_index,
558558
(rp->allow_flag ? "allow" : "deny"), (ip_matches ? "true" : "false"), (method_matches ? "true" : "false"));
559559

560-
if (ip_matches) {
560+
if (_acl_behavior_policy == ACLBehaviorPolicy::ACL_BEHAVIOR_LEGACY) {
561+
s->skip_ip_allow_yaml = false;
562+
Dbg(dbg_ctl_url_rewrite, "Doing legacy filtering ip:%s method:%s", ip_matches ? "matched" : "didn't match",
563+
method_matches ? "matched" : "didn't match");
564+
bool match = ip_matches && method_matches;
565+
if (match && s->client_connection_allowed) { // make sure that a previous filter did not DENY
566+
Dbg(dbg_ctl_url_rewrite, "matched ACL filter rule, %s request", rp->allow_flag ? "allowing" : "denying");
567+
s->client_connection_allowed = rp->allow_flag ? true : false;
568+
} else {
569+
if (!s->client_connection_allowed) {
570+
Dbg(dbg_ctl_url_rewrite, "Previous ACL filter rule denied request, continuing to deny it");
571+
} else {
572+
Dbg(dbg_ctl_url_rewrite, "did NOT match ACL filter rule, %s request", rp->allow_flag ? "denying" : "allowing");
573+
s->client_connection_allowed = rp->allow_flag ? false : true;
574+
}
575+
}
576+
} else if (ip_matches) {
561577
// The rule matches. Handle the method according to the rule.
562578
if (method_matches) {
563579
// Did they specify allowing the listed methods, or denying them?

‎tests/gold_tests/remap/all_acl_combinations.py‎

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -56,50 +56,50 @@
5656
[ 1, "legacy", "", "", ALLOW_GET, 200, 403, ],
5757
[ 2, "legacy", "", "", DENY_GET, 403, 200, ],
5858
[ 3, "legacy", "", "", DENY_GET_AND_POST, 403, 403, ],
59-
[ 4, "legacy", "", "@action=allow @method=GET", ALLOW_GET_AND_POST, 200, 200, ],
59+
[ 4, "legacy", "", "@action=allow @method=GET", ALLOW_GET_AND_POST, 200, 403, ],
6060
[ 5, "legacy", "", "@action=allow @method=GET", ALLOW_GET, 200, 403, ],
61-
[ 6, "legacy", "", "@action=allow @method=GET", DENY_GET, 200, 200, ],
62-
[ 7, "legacy", "", "@action=allow @method=GET", DENY_GET_AND_POST, 200, 403, ],
61+
[ 6, "legacy", "", "@action=allow @method=GET", DENY_GET, 403, 403, ],
62+
[ 7, "legacy", "", "@action=allow @method=GET", DENY_GET_AND_POST, 403, 403, ],
6363
[ 8, "legacy", "", "@action=deny @method=GET", ALLOW_GET_AND_POST, 403, 200, ],
6464
[ 9, "legacy", "", "@action=deny @method=GET", ALLOW_GET, 403, 403, ],
6565
[ 10, "legacy", "", "@action=deny @method=GET", DENY_GET, 403, 200, ],
6666
[ 11, "legacy", "", "@action=deny @method=GET", DENY_GET_AND_POST, 403, 403, ],
67-
[ 12, "legacy", "@action=allow @method=GET", "", ALLOW_GET_AND_POST, 200, 200, ],
67+
[ 12, "legacy", "@action=allow @method=GET", "", ALLOW_GET_AND_POST, 200, 403, ],
6868
[ 13, "legacy", "@action=allow @method=GET", "", ALLOW_GET, 200, 403, ],
69-
[ 14, "legacy", "@action=allow @method=GET", "", DENY_GET, 200, 200, ],
70-
[ 15, "legacy", "@action=allow @method=GET", "", DENY_GET_AND_POST, 200, 403, ],
71-
[ 16, "legacy", "@action=allow @method=GET", "@action=allow @method=GET", ALLOW_GET_AND_POST, 200, 200, ],
69+
[ 14, "legacy", "@action=allow @method=GET", "", DENY_GET, 403, 403, ],
70+
[ 15, "legacy", "@action=allow @method=GET", "", DENY_GET_AND_POST, 403, 403, ],
71+
[ 16, "legacy", "@action=allow @method=GET", "@action=allow @method=GET", ALLOW_GET_AND_POST, 200, 403, ],
7272
[ 17, "legacy", "@action=allow @method=GET", "@action=allow @method=GET", ALLOW_GET, 200, 403, ],
73-
[ 18, "legacy", "@action=allow @method=GET", "@action=allow @method=GET", DENY_GET, 200, 200, ],
74-
[ 19, "legacy", "@action=allow @method=GET", "@action=allow @method=GET", DENY_GET_AND_POST, 200, 403, ],
75-
[ 20, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", ALLOW_GET_AND_POST, 200, 200, ],
76-
[ 21, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", ALLOW_GET, 200, 403, ],
77-
[ 22, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", DENY_GET, 200, 200, ],
78-
[ 23, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", DENY_GET_AND_POST, 200, 403, ],
79-
[ 24, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", ALLOW_GET_AND_POST, 200, 200, ],
80-
[ 25, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", ALLOW_GET, 200, 200, ],
81-
[ 26, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", DENY_GET, 200, 200, ],
82-
[ 27, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", DENY_GET_AND_POST, 200, 200, ],
73+
[ 18, "legacy", "@action=allow @method=GET", "@action=allow @method=GET", DENY_GET, 403, 403, ],
74+
[ 19, "legacy", "@action=allow @method=GET", "@action=allow @method=GET", DENY_GET_AND_POST, 403, 403, ],
75+
[ 20, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", ALLOW_GET_AND_POST, 403, 403, ],
76+
[ 21, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", ALLOW_GET, 403, 403, ],
77+
[ 22, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", DENY_GET, 403, 403, ],
78+
[ 23, "legacy", "@action=allow @method=GET", "@action=deny @method=GET", DENY_GET_AND_POST, 403, 403, ],
79+
[ 24, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", ALLOW_GET_AND_POST, 403, 403, ],
80+
[ 25, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", ALLOW_GET, 403, 403, ],
81+
[ 26, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", DENY_GET, 403, 403, ],
82+
[ 27, "legacy", "@action=allow @method=GET", "@action=allow @method=POST", DENY_GET_AND_POST, 403, 403, ],
8383
[ 28, "legacy", "@action=allow @method=GET", "@action=deny @method=POST", ALLOW_GET_AND_POST, 200, 403, ],
8484
[ 29, "legacy", "@action=allow @method=GET", "@action=deny @method=POST", ALLOW_GET, 200, 403, ],
85-
[ 30, "legacy", "@action=allow @method=GET", "@action=deny @method=POST", DENY_GET, 200, 403, ],
86-
[ 31, "legacy", "@action=allow @method=GET", "@action=deny @method=POST", DENY_GET_AND_POST, 200, 403, ],
85+
[ 30, "legacy", "@action=allow @method=GET", "@action=deny @method=POST", DENY_GET, 403, 403, ],
86+
[ 31, "legacy", "@action=allow @method=GET", "@action=deny @method=POST", DENY_GET_AND_POST, 403, 403, ],
8787
[ 32, "legacy", "@action=deny @method=GET", "", ALLOW_GET_AND_POST, 403, 200, ],
8888
[ 33, "legacy", "@action=deny @method=GET", "", ALLOW_GET, 403, 403, ],
8989
[ 34, "legacy", "@action=deny @method=GET", "", DENY_GET, 403, 200, ],
9090
[ 35, "legacy", "@action=deny @method=GET", "", DENY_GET_AND_POST, 403, 403, ],
91-
[ 36, "legacy", "@action=deny @method=GET", "@action=allow @method=GET", ALLOW_GET_AND_POST, 403, 200, ],
91+
[ 36, "legacy", "@action=deny @method=GET", "@action=allow @method=GET", ALLOW_GET_AND_POST, 403, 403, ],
9292
[ 37, "legacy", "@action=deny @method=GET", "@action=allow @method=GET", ALLOW_GET, 403, 403, ],
93-
[ 38, "legacy", "@action=deny @method=GET", "@action=allow @method=GET", DENY_GET, 403, 200, ],
93+
[ 38, "legacy", "@action=deny @method=GET", "@action=allow @method=GET", DENY_GET, 403, 403, ],
9494
[ 39, "legacy", "@action=deny @method=GET", "@action=allow @method=GET", DENY_GET_AND_POST, 403, 403, ],
9595
[ 40, "legacy", "@action=deny @method=GET", "@action=deny @method=GET", ALLOW_GET_AND_POST, 403, 200, ],
9696
[ 41, "legacy", "@action=deny @method=GET", "@action=deny @method=GET", ALLOW_GET, 403, 403, ],
9797
[ 42, "legacy", "@action=deny @method=GET", "@action=deny @method=GET", DENY_GET, 403, 200, ],
9898
[ 43, "legacy", "@action=deny @method=GET", "@action=deny @method=GET", DENY_GET_AND_POST, 403, 403, ],
9999
[ 44, "legacy", "@action=deny @method=GET", "@action=allow @method=POST", ALLOW_GET_AND_POST, 403, 200, ],
100-
[ 45, "legacy", "@action=deny @method=GET", "@action=allow @method=POST", ALLOW_GET, 403, 200, ],
100+
[ 45, "legacy", "@action=deny @method=GET", "@action=allow @method=POST", ALLOW_GET, 403, 403, ],
101101
[ 46, "legacy", "@action=deny @method=GET", "@action=allow @method=POST", DENY_GET, 403, 200, ],
102-
[ 47, "legacy", "@action=deny @method=GET", "@action=allow @method=POST", DENY_GET_AND_POST, 403, 200, ],
102+
[ 47, "legacy", "@action=deny @method=GET", "@action=allow @method=POST", DENY_GET_AND_POST, 403, 403, ],
103103
[ 48, "legacy", "@action=deny @method=GET", "@action=deny @method=POST", ALLOW_GET_AND_POST, 403, 403, ],
104104
[ 49, "legacy", "@action=deny @method=GET", "@action=deny @method=POST", ALLOW_GET, 403, 403, ],
105105
[ 50, "legacy", "@action=deny @method=GET", "@action=deny @method=POST", DENY_GET, 403, 403, ],
@@ -140,6 +140,22 @@
140140
[ 85, "modern", "@action=set_deny @method=GET", "@action=set_deny @method=GET", ALLOW_GET, 403, 200, ],
141141
[ 86, "modern", "@action=set_deny @method=GET", "@action=set_deny @method=GET", DENY_GET, 403, 200, ],
142142
[ 87, "modern", "@action=set_deny @method=GET", "@action=set_deny @method=GET", DENY_GET_AND_POST, 403, 200, ],
143+
[ 88, "legacy", "@action=allow @src_ip=127.0.0.1", "", ALLOW_GET_AND_POST, 200, 200, ],
144+
[ 89, "legacy", "@action=allow @src_ip=127.0.0.1", "", ALLOW_GET, 200, 403, ],
145+
[ 90, "legacy", "@action=allow @src_ip=127.0.0.1", "", DENY_GET, 403, 200, ],
146+
[ 91, "legacy", "@action=allow @src_ip=127.0.0.1", "", DENY_GET_AND_POST, 403, 403, ],
147+
[ 92, "legacy", "@action=deny @src_ip=127.0.0.1", "", ALLOW_GET_AND_POST, 403, 403, ],
148+
[ 93, "legacy", "@action=deny @src_ip=127.0.0.1", "", ALLOW_GET, 403, 403, ],
149+
[ 94, "legacy", "@action=deny @src_ip=127.0.0.1", "", DENY_GET, 403, 403, ],
150+
[ 95, "legacy", "@action=deny @src_ip=127.0.0.1", "", DENY_GET_AND_POST, 403, 403, ],
151+
[ 96, "legacy", "@action=allow @src_ip=192.0.2.1/24", "", ALLOW_GET_AND_POST, 403, 403, ],
152+
[ 97, "legacy", "@action=allow @src_ip=192.0.2.1/24", "", ALLOW_GET, 403, 403, ],
153+
[ 98, "legacy", "@action=allow @src_ip=192.0.2.0/24", "", DENY_GET, 403, 403, ],
154+
[ 99, "legacy", "@action=allow @src_ip=192.0.2.0/24", "", DENY_GET_AND_POST, 403, 403, ],
155+
[100, "legacy", "@action=deny @src_ip=192.0.2.1/24", "", ALLOW_GET_AND_POST, 200, 200, ],
156+
[101, "legacy", "@action=deny @src_ip=192.0.2.1/24", "", ALLOW_GET, 200, 403, ],
157+
[102, "legacy", "@action=deny @src_ip=192.0.2.0/24", "", DENY_GET, 403, 200, ],
158+
[103, "legacy", "@action=deny @src_ip=192.0.2.0/24", "", DENY_GET_AND_POST, 403, 403, ],
143159
]
144160
# yapf: enable
145161

0 commit comments

Comments
 (0)