easegress icon indicating copy to clipboard operation
easegress copied to clipboard

The IP filter in HTTP Server rules works beyond its intended scope

Open llooper-dev opened this issue 3 years ago • 10 comments

配置一,与配置二,只是ipFilter位置不一样,但是效果就有差别,理论上,ipFilter不是只作用在配置的rule里面吗。

kind: HTTPServer name: facen-httpproxy port: 10080 keepAlive: true https: false rules:

  • paths:
    • pathPrefix: /facen/v2/ backend: facen-algapi-pipeline2
    • pathPrefix: /facen/v3/ backend: facen-algapi-pipeline3
  • ipFilter: blockByDefault: true allowIPs: - 127.0.0.1 blockIPs: - 192.168.9.45 paths:
    • pathPrefix: /facen/api/ backend: facen-algapi-pipeline
    • pathPrefix: /facen/v1/ backend: facen-algapi-pipeline

配置二:

kind: HTTPServer name: facen-httpproxy port: 10080 keepAlive: true https: false rules:

  • ipFilter: blockByDefault: true allowIPs: - 127.0.0.1 blockIPs: - 192.168.9.45 paths:
    • pathPrefix: /facen/api/ backend: facen-algapi-pipeline
    • pathPrefix: /facen/v1/ backend: facen-algapi-pipeline
  • paths:
    • pathPrefix: /facen/v2/ backend: facen-algapi-pipeline2
    • pathPrefix: /facen/v3/ backend: facen-algapi-pipeline3

llooper-dev avatar Aug 23 '21 08:08 llooper-dev

Hi @llooper-dev

  • First of all, I recommend writing the YAML configuration with the help of markdown format. I think these two configuration are
kind: HTTPServer
name: facen-httpproxy
port: 10080
keepAlive: true
https: false
rules:
  - paths:
      - pathPrefix: /facen/v2/
        backend: facen-algapi-pipeline2
      - pathPrefix: /facen/v3/
        backend: facen-algapi-pipeline3
  - ipFilter:
      blockByDefault: true
      allowIPs:
        - 127.0.0.1
      blockIPs:
        - 192.168.9.45
    paths:
      - pathPrefix: /facen/api/
        backend: facen-algapi-pipeline
      - pathPrefix: /facen/v1/
        backend: facen-algapi-pipeline
        

and

kind: HTTPServer
name: facen-httpproxy
port: 10080
keepAlive: true
https: false
rules:
  - ipFilter:
      blockByDefault: true
      allowIPs:
        - 127.0.0.1
      blockIPs:
        - 192.168.9.45
    paths:
      - pathPrefix: /facen/api/
        backend: facen-algapi-pipeline
      - pathPrefix: /facen/v1/
        backend: facen-algapi-pipeline
 - paths:
      - pathPrefix: /facen/v2/
        backend: facen-algapi-pipeline2
      - pathPrefix: /facen/v3/
        backend: facen-algapi-pipeline3		    

That's better for reading. :-)

  • From my understandings, one rule is for the /facen/api/ and /face/v1/ with IP filter enabled, the other rule is for /face/v2/ and /face/v3/ path without IP filter. Can u provide more details, maybe screenshots or tcpdump packages?
  • Easegress' HTTPServer detects the path matching according to the sequence of the rules array. And the IP filter in your configuration works for the /facen/api/ and /facen/v1/ rule, so I think the effect for IP blocking should be the same.
  • Also, I would like to know the purpose of using IP filter. Because HTTPServer has its own global IP filter too. Can u tell us the user scenario?
  • In the Easegress project, we prefer using English for issue discussion, bug reporting, and so on, Thx.

benja-wu avatar Aug 23 '21 09:08 benja-wu

@llooper-dev could you please tell us what the different behavior you observed? and what's the behavior you expected?

haoel avatar Aug 23 '21 13:08 haoel

不是国内的开源项目吗?我看了源码,发现你后面的过滤器继承了前面的过滤器,所以过滤器放在后面才达到 我的效果,可能是你们故意如此设计的吧?

llooper-dev avatar Aug 24 '21 14:08 llooper-dev

@llooper-dev As a programmer, I believe using English is more friendly for the community. And other users can be benefited from your issue reporting, or discussion by searching the keyword in Github easily. So English, please.

  • As for the Inherit behaviour u mentioned above, I can't get it. Is this about the httpserver/mut.go's cacheItem for rules? (But in your example YAML, the HTTPServer isn't enabled this cache feature.)
  • Can u share more details or which part of the codes is the root cause you found after code reading? what exactly does "your effect" mean?

benja-wu avatar Aug 25 '21 01:08 benja-wu

return &muxRule{
	ipFilter:      newIPFilter(rule.IPFilter),
	ipFilterChain: newIPFilterChain(parentIPFilters, rule.IPFilter),

	host:       rule.Host,
	hostRegexp: rule.HostRegexp,
	hostRE:     hostRE,
	paths:      paths,
}

llooper-dev avatar Aug 25 '21 02:08 llooper-dev

` // newIPFilterChain returns nil if the number of final filters is zero. func newIPFilterChain(parentIPFilters *ipfilter.IPFilters, childSpec *ipfilter.Spec) *ipfilter.IPFilters { var ipFilters *ipfilter.IPFilters if parentIPFilters != nil { ipFilters = ipfilter.NewIPFilters(parentIPFilters.Filters()...) } else { ipFilters = ipfilter.NewIPFilters() }

if childSpec != nil {
	ipFilters.Append(ipfilter.New(childSpec))
}

if len(ipFilters.Filters()) == 0 {
	return nil
}

return ipFilters

}

`

llooper-dev avatar Aug 25 '21 02:08 llooper-dev

ipFilters.Append(ipfilter.New(childSpec))

llooper-dev avatar Aug 25 '21 02:08 llooper-dev

That's probably how you designed it. The filter chain will inherit from the filter ?

llooper-dev avatar Aug 25 '21 02:08 llooper-dev

@llooper-dev OK, I got it. It's about the newIPFilterChain in HTTPServer.

  • Currently, HTTPServer has three layers of IP filter, in mux.go's line #365, line #375, and line #392. You can notice this in muxRules, muxRule, and muxPath's structure define. All of them have their own IP filter.
  • The ipFilterChain is an old design for aiming to filter all IP at the same place. But it is inactive right now. We are considering removing this chain structure at refactoring.
  • Still we haven't figured out the unexpected results of your example yet, perhaps u can join our production slack for more effective communication. Thx! (https://join.slack.com/t/openmegaease/shared_invite/zt-upo7v306-lYPHvVwKnvwlqR0Zl2vveA, it's also in our README.)

PS: I will modify your issue title to English, it's that OK for you?

benja-wu avatar Aug 25 '21 09:08 benja-wu

OK

llooper-dev avatar Aug 25 '21 10:08 llooper-dev