7+ regression: acl filters in remap rules

[mlibbey]

in ATS <7, multiple acl_filters work in remap rules. For instance, this works

map http://example.com http://origin.example.com \
  @src_ip=192.168.0.0-192.168.255.255 @src_ip=10.0.0.0-10.255.255.255 @action=allow \
  @plugin=header_rewrite.so @pparam=someconfig.txt \
  @action=deny @method=CONNECT @method=POST @method=PUT @method=DELETE

(eg, only allow connections from 192.168.0.0/16 or 10.0.0.0/8, AND do not allow CONNECT/POST, etc methods.) In ATS 7+ that remap fails to have both ACLs.

In ATS7+:

• these work:

map http://example.com http://origin.example.com \
  @src_ip=192.168.0.0-192.168.255.255 @src_ip=10.0.0.0-10.255.255.255 @action=allow 

(eg, only allow connections from 192.168.0.0/16 or 10.0.0.0/8,)

map http://example.com http://origin.example.com \
  @action=deny @method=CONNECT @method=POST @method=PUT @method=DELETE

(eg, do not allow CONNECT/POST, etc methods)

• But, When switching the order of the rules, the action part from the IP filter is ignored -- a request from 192.168.0.2 is denied

map http://example.com http://origin.example.com \
  @action=deny @method=CONNECT @method=POST @method=PUT @method=DELETE \
  @src_ip=192.168.0.0-192.168.255.255 @src_ip=10.0.0.0-10.255.255.255 @action=allow 

• finally, multiple allow actions work (eg inverting the methods:

map http://example.com http://origin.example.com \
  @src_ip=192.168.0.0-192.168.255.255 @src_ip=10.0.0.0-10.255.255.255 @action=allow \
  @plugin=header_rewrite.so @pparam=someconfig.txt \
  @action=allow @method=GET @method=HEAD @method=ICP_QUERY @method=OPTIONS @method=TRACE @method=PUSH

[SolidWallOfCode]

I'm a bit confused - consider this example

map http://example.com http://origin.example.com \
  @action=deny @method=CONNECT @method=POST @method=PUT @method=DELETE \
  @src_ip=192.168.0.0-192.168.255.255 @src_ip=10.0.0.0-10.255.255.255 @action=allow 

The claim is this incorrectly denies the request, but doesn't @action=deny cause that? Or is that a GET request is denied?

[SolidWallOfCode]

I tested 5.3.x and 6.2.x - as far as I can tell the behavior is the same as 7.x in this regard. E.g. with previous rule

• GET from RFC addr - 403, denied.
• POST from RFC addr - allowed
• request from non-RFC - 403, denied.

Reading the documentation and examining the code, this seems expected. The first @action=deny is ignored because of the trailing @action=accept. The result is that requests from an RFC addr that are one of POST, CONNECT, PUT, DELETE are allowed. Everything else is denied. AFAICT this is the case with 7.x as well.

[mlibbey]

The feature we can't get to work is to have multiple rules. Eg, Only allow requests from 1.2.3.4 and Deny all POSTs.

Here are all my remap rule tests, their goals, and their results.

Goal: deny all POSTs

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @action=deny @method=POST

Result: successfully deny post, allow get
Success!

Goal: only allow requests from 1.2.3.4

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @src_ip=0.0.0.0-255.255.255.255 @action=deny

Result: successfully only allows requests from 1.2.3.4. Denies all requests from other IPs
Success!

---

Goal: only allow requests from 1.2.3.4. Deny all POSTs

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @src_ip=1.2.3.4 @method=POST @action=deny

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @src_ip=1.2.3.4 @action=deny @method=POST

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @src_ip=0.0.0.0-255.255.255.255 @method=POST @action=deny

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @src_ip=0.0.0.0-255.255.255.255 @action=deny @method=POST

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @method=POST @action=deny

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @action=deny @method=POST

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @src_ip=1.2.3.4 @method=POST @action=deny
 @src_ip=0.0.0.0-255.255.255.255 @method=POST @action=deny

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 @action=allow \
 @src_ip=1.2.3.4 @method=POST @action=deny \
 @src_ip=0.0.0.0-255.255.255.255 @action=deny

Result: All IPs can issue GETs (fails first goal)
Fail!

---

Goal: only allow requests from 1.2.3.4. Deny all POSTs

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=0.0.0.0-255.255.255.255 @method=POST @action=deny \
 @src_ip=1.2.3.4 @action=allow

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @method=POST @action=deny \
 @src_ip=1.2.3.4 @action=allow

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @action=deny @method=POST \
 @src_ip=1.2.3.4 @action=allow

map /acltest/ https://httpbin.org/ \
 @plugin=conf_remap.so @pparam=proxy.config.url_remap.pristine_host_hdr=0 \
 @src_ip=1.2.3.4 caction=deny @method=POST \
 @src_ip=1.2.3.4 @action=allow

Result: All GETs requests are rejected, including 'whitelisted' IP. Fail!

[zwoop]

Just to be sure I;m testing the same things; There's no global ip_allow.config rules here that would conflict / override right ?

[SolidWallOfCode]

I'll look at this some more. So far I see what the problem is, what I don't see is any evidence this ever worked as expected. I see why this happens, based on the code, but that code is the same in 5.3. I checked back to 5.0 looking at the code and I see the same logic. Basically, if any deny rule matches, the request is denied, and rules are checked until a deny is found or there are no more rules. Even if you could put an @action=deny as a trailer it wouldn't help because that would block everything. As far as I can tell, allow actions are simply useless and always have been. What would need to change at a minimum is to stop processing rules on a match, not on a deny. I am not sure how much of a change this is with regard to release issues.

[SolidWallOfCode]

Pondering this a bit, I would be tempted to do a PR with two changes:

• Add a config switch to enable "first match" rather than "deny match" for remap filters. In that case the first action that matched would control, so you could have an allow of what you wanted followed by a blanket deny. This would then work the same way as ip_allow.config.
• Fix the remap rule parser to support multiple rules. That might have gotten broken at some point, although again my code research suggests it hasn't worked since at least 5.3. Even if this isn't feasible, the former change would at least allow named filters to perform the task.

[zwoop]

I'm not convinced that adding yet another configuration just for backwards compatibility is all that great. Our configurations are already convoluted as they are :-).

[bryancall]

@vmamidi is going to look at this one some more.