solo-io / gloo

The Feature-rich, Kubernetes-native, Next-Generation API Gateway Built on Envoy
https://docs.solo.io/
Apache License 2.0
4.09k stars 441 forks source link

Don't add unused filters to the filter chain #5651

Closed Spoonbender closed 2 years ago

Spoonbender commented 2 years ago

Is your feature request related to a problem? Please describe. Currently, the filter chains constructed by Gloo Edge include various filters which we don't actually use, such as: -io.solo.filters.http.solo_jwt_authn_staged -envoy.filters.http.ext_authz -envoy.filters.http.grpc_web -envoy.filters.http.ratelimit -io.solo.filters.http.graphql

This is poses two issues:

  1. Security: having additional, unneeded filters in the chain increases the attack surface.
    • Even if the filters supposedly don't do anything, it is easier to argue about the security of the system if the filters are not included in the chain at all
  2. Performance: there is some performance penalty for executing these filters for each HTTP request
    • Even if they do nothing and return almost immediately, this is still a function call with prologue and epilogue etc.

Notice that in a "vanilla" envoy - no filters are added to the chain unless explicitly configured to.

Describe the solution you'd like Gloo should only add filters to the chain if they are actually in use. For example:

Describe alternatives you've considered

  1. Enabling users to modify the global list of filters Gloo adds to every filter chain
  2. Enabling users to configure the list of filters Gloo applies to each VirtualService
  3. Enabling users to disable specific features, such as rate-limit or GraphQL, which would in turn exclude these filters from the chain by implication
chrisgaun commented 2 years ago

related to https://github.com/solo-io/gloo/issues/5650

nrjpoddar commented 2 years ago

@nfuden can you share what user config are you planning to expose to make this configurable?

nfuden commented 2 years ago

@nrjpoddar its a good question and one that is not entirely set.

The current statements are under consideration.

  1. We don't want to break backwards compatibility by changing our plugin interface so it is likely to be advantageous to have the configuration setting be usable in the future
  2. We want likely want this toggleable at a per plugin level (this may not be true but is likely true)

Current thought is to have an option in GlooOptions which would have information for allow-listing/deny-listing and an array of plugins to allow/disallow. Similar to the basic flow for DisableGrpcWeb. Would be distinct from a disable flag.

So far I've been digging in on a per filter to see what it means for a filter to not be needed in a given configuration and need to get some feedback on the settings naming/structuring.

nfuden commented 2 years ago

Is there a specific version requested for backports? Or are we fine with it being in 1.11?

nfuden commented 2 years ago

Bulldozer closed when it merged to OSS, still needs to be brought into Enterprise so reopening.