Add knobs for allowed modifications in ext_authz and ext_proc

[gbrail]

Title: Add knobs to control which headers ext_authz and ext_proc servers can modify

Description:

Today, the ext_authz filter allows the remote server to request that any header, including internal headers (prefixed with ":"), and x-envoy headers be modified.

The in-progress ext_proc filter does not allow these filters to be modified, but we expect that in some cases users will wish to support this.

We should give both of these filters configurable controls for this. One way would be to create a new enum and use it in the configuration of both filters.

I'd propose an enum with the following modes:

• DEFAULT: In this mode, headers may not be modified if they are expected to cause problems later that cannot be resolved. That means that no x-envoy header may be set, and a few specific headers such as ":scheme" may not be set. However, other internal headers, including ":path", ":method", ":authority", and "host" may be set. This may cause routes to have to be recalculated, which each filter should account for, ideally without requiring the user to do anything specifically. (i.e. the filter should know that, if ":path" was changed, that the route cache might need refreshing). This will be the default mode of the ext_proc filter.
• UNRESTRICTED: In this mode, any header may be set, including x-envoy, "host", and any internal headers. This will be the default mode for the ext_authz filter since this is how it works today.
• RESTRICTED: In this mode, no internal headers may be set, the host header may not be set, and no x-envoy header may be set. This provides the maximum safety for embedding in other products or for multi-tenant environments.

If these three options are not sufficient, then an alternative would be to support a set of booleans like:

• allow_internal_headers
• allow_x_envoy_headers
• allow_host_header

And if THAT is not sufficient then we could allow the user to specify an allow list and a deny list, complete with wildcards. However this might be overly complex.

[gbrail]

@alyssawilk @htuch we were discussing this for possible usage of ext_authz in an embedded product.

[gbrail]

Based on some other discussions, we might need another mode, like "DISABLED," which disallows all modifications and just lets the filter only decide whether to accept or reject.

[htuch]

The bools seem slightly better as they provide mechanism rather than policy, which is preferable in the API.

Arguably, having controls like this would also make sense for dynamic code like Lua/Wasm, since you could control what sandboxed code was capable of. But, if we design this right, and don't make the proto message restricted to ext_*, that can always be added later.

[alyssawilk]

As said on email earlier, I think this would be useful in a number of places. If we do booleans let's encapsulate in a base level proto so we can apply it to other places we do header modifications. I could imagine a cloud provider tacking on "no changing host" rules to ensure that client xDS config didn't change disallowed fields for per-route config for example.

Since I think bools are false by default, I'd be inclined to flip semantics and have
disallow_thing_a
disallow_thing_b
so by default everything is allowed and you filter by flipping explicit config, but you can also use Boolean and allow_thing.

cc @lizan and @PiotrSikora for wasm and @markdroth for gRPC thoughts (e.g. x-envoy: does gRPC have an equivalent?)

For naming, :scheme :method etc. are pseudo-headers (https://tools.ietf.org/html/rfc7540#section-8.1.2.1 : I think what you were calling internal?) and we should comment it also applies to http/1.1 equivalents like host:

For x-envoy there's the header_prefix knob in Envoy bootstrap that lets Envoy change its prefix from x-envoy to x-foo so we'd want to make sure we comment and sanitize based on the actual header prefix if it's not the default:
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/bootstrap/v3/bootstrap.proto#envoy-v3-api-msg-config-bootstrap-v3-bootstrap

[PiotrSikora]

See parameter sanitization in Proxy-Wasm capabilities (#13911).

[markdroth]

@alyssawilk gRPC does not have its own equivalent of x-envoy headers; we generally have APIs for the application to configure things instead. Although interestingly, as part of the xDS support we're adding, there are some cases where we're going to support some x-envoy headers for compatibility with Envoy. :/

Is the idea here that these knobs would be defined in some sort of wrapper that can be applied to any filter? How can the settings of these knobs be enforced that way? Or is the idea that we'd need each individual filter to support these knobs as appropriate?

CC @ejona86 @dfawley

[alyssawilk]

My mental model was we'd have a proto of "what is disallowed" (everything being allowed by default) and different areas of the code base could add it over time to things like request_headers_to_add, or ext_proc. There'd be a utility which took in that proto (or parsed config) and applied the checks, and that'd be added to sites with the API additions.
Totally open to options if there's a model which works better for grpc. For example rather than x-envoy header we could do a generic "no headers with this prefix" and use x-envoy in the examples.

[antoniovicente]

It may be useful to add some controls that specify what should be done if the external filter attempts to modify a disallowed header. Some options include:

1. emit some logging or stats
2. ignore the disallowed modifications
3. error out the request

[gbrail]

Check out #19141 which adds a proto to start to address this issue

[gbrail]

As of #19622, this has now been merged to main and is supported. See the new "mutation_rules" field in the filter configuration.