allowed_upstream_headers in filters.http.ext_authz.v3.ExtAuthz removes repeated headers

[dastrobu]

Title: allowed_upstream_headers in filters.http.ext_authz.v3.ExtAuthz removes repeated headers

Description:

Configure an ExtAuthz filter which should allow some custom upstream headers from authz:

                    http_filters:
                      - name: envoy.filters.http.ext_authz
                        typed_config:
                          "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz
                          http_service:
                            server_uri:
                              uri: 127.0.0.1:1234
                              cluster: *authz-cluster
                              timeout: 20s
                            path_prefix: /authz
                            authorization_response:
                              allowed_upstream_headers:
                                patterns:
                                  - prefix: "foo-"
                                    ignore_case: true

It is expected that all client headers matching foo-* are removed from the request headers and all response headers from authz are added to the upstream request.

This is working, except for the case where the response headers of authz contain repeated values, such as:

foo-bar: value1
foo-bar: value2

In this case, envoy sets only one of the headervalues to the upstream headers.

envoy version is: v1.29.1

[kyessenov]

CC @tyxia

[tyxia]

@dastrobu For your example, do you see foo-bar: value1, value2 ?

That is expected, ext_authz merge duplicate header with , . This behavior strictly follow the RFC https://www.rfc-editor.org/rfc/rfc7230#section-3.2

Merging duplicate header in Envoy in general is a bit relaxed: It only comma concatenation for predefined inline headers

cced @yanavlasov who implemented the ext_authz behavior to fix CVE and @ggreenway who is code owner.

[dastrobu]

@tyxia thank you for this hint. I wouldn't expect envoy to merge herders, as this would be inconsistent with the behavior of allowed_upstream_headers_to_append. With this option, I get both headers consistently.

Also, consider custom headers that allow commas in their values (like the date header). One can certainly debate if this is allowed, but I'd say less magic from envoy side is better in this case. And consistency with allowed_upstream_headers_to_append would be best from my point of view.

When using allowed_upstream_headers, I only get the last value (foo-bar:value2).

Here is a SSCCE:

static_resources:

  listeners:
  - name: httpbin
    address:
      socket_address:
        address: 0.0.0.0
        port_value: 10000
    filter_chains:
    - filters:
      - name: envoy.filters.network.http_connection_manager
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          stat_prefix: ingress_http
          access_log:
          - name: envoy.access_loggers.stdout
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
          http_filters:
          - name: envoy.filters.http.ext_authz
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz
              http_service:
                  server_uri:
                    uri: 127.0.0.1:10003
                    cluster: ext-authz
                    timeout: 0.25s
                  authorization_response:
                    allowed_upstream_headers:
                      patterns:
                        - prefix: "foo-"
                          ignore_case: true
                    #allowed_upstream_headers_to_append:
                    #  patterns:
                    #    - exact: "foo-bar"
                    #      ignore_case: true
              include_peer_certificate: true
          - name: envoy.filters.http.lua
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
              default_source_code:
                inline_string: |
                  function envoy_on_request(request_handle)
                    local s = ""
                    for key, value in pairs(request_handle:headers()) do
                      s = s .. key .. ":" .. value .. "\n"
                    end
                    request_handle:respond(
                    {[":status"] = "200",
                    },
                    s)
                  end
          - name: envoy.filters.http.router
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              - match:
                  prefix: "/"
                route:
                  host_rewrite_literal: httpbin.org
                  cluster: httpbin
  - name: ext-authz
    address:
      socket_address:
        address: 0.0.0.0
        port_value: 10003
    filter_chains:
    - filters:
      - name: envoy.filters.network.http_connection_manager
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          stat_prefix: ingress_http
          access_log:
          - name: envoy.access_loggers.stdout
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StdoutAccessLog
          http_filters:
          - name: envoy.filters.http.lua
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
              default_source_code:
                inline_string: |
                  function envoy_on_request(request_handle)
                    request_handle:respond(
                    {[":status"] = "200",
                    },
                    "this is authz")
                  end
                  function envoy_on_response(response_handle)
                    response_handle:headers():add("foo-bar", "value1")
                    response_handle:headers():add("foo-bar", "value2")
                  end
          - name: envoy.filters.http.router
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]

  clusters:
  - name: httpbin
    type: LOGICAL_DNS
    # Comment out the following line to test on v6 networks
    dns_lookup_family: V4_ONLY
    load_assignment:
      cluster_name: service_envoyproxy_io
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: httpbin.org
                port_value: 80
  - name: ext-authz
    type: LOGICAL_DNS
    # Comment out the following line to test on v6 networks
    dns_lookup_family: V4_ONLY
    load_assignment:
      cluster_name: service_envoyproxy_io
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: localhost
                port_value: 10003

After staring it, you can verify the behavior of authz with:

$ curl localhost:10003 -i
HTTP/1.1 200 OK
content-length: 13
content-type: text/plain
foo-bar: value1
foo-bar: value2
date: Sat, 17 Feb 2024 16:29:38 GMT
server: envoy

this is authz

and used as filter:

$ curl localhost:10000
:authority:localhost:10000
:path:/
:method:GET
:scheme:http
user-agent:curl/8.4.0
accept:*/*
x-forwarded-proto:http
x-request-id:15f47be1-03aa-48b1-97e7-cb93de16339f
foo-bar:value2

[tyxia]

Ok. The merge behavior I was referring to is for check request to authz server. (sidestream). The issue you are discussing is client request to upstream. Those two are unrelated as they are requests to different destination.

Back to your original issue. The behavior you described seems to expected? allowed_upstream_headers : coexistent headers will be overridden (see here) allowed_upstream_headers_to_append: coexistent headers will be appended (see here)

[dastrobu]

I have reviewed the documentation, but I find it somewhat ambiguous. From my point of view, it should refer to the merge behavior of the client's original request headers and the authz's response headers. Typically, you would want to set some headers from authz, such as X-Scope-OrgID: A|B, which you can trust. Hence, any duplicate header from the client's request must be overridden.

In the above example, this would correspond to:

curl localhost:10000 -H 'foo-bar: client'

So, from my perspective, "coexistent headers will be overridden" should only refer to the merging behavior of client and authz headers.

If authz intentionally returns duplicate headers, I would not expect ext_authz to modify them in any way.

[tyxia]

Just to clarify in case there was miscommunication : allowed_upstream_headers (here) and allowed_upstream_headers_to_append (here) both describe the merge behavior of client's original request and the authz's response

If you want to have overridden merge behavior you can use allowed_upstream_headers. If you want to have append merge behavior, you can use allowed_upstream_headers_to_append. Does this work for you?

[dastrobu]

Ok, let me try to summarize.

The current behavior of allowed_upstream_headers is:

graph TB
  subgraph client [client request headers]
  c1[foo-bar: client1]
  c2[foo-bar: client2]
  end
  subgraph authz [authz response headers]
  a3[foo-bar: authz1]
  a1[foo-bar: authz2]
  end
  subgraph upstream [upstream request headers]
  u1[foo-bar: authz2]
    end
  client--->upstream
  authz--->upstream

The current behavior of allowed_upstream_headers_to_append is:

graph TB
  subgraph client [client request headers]
  c1[foo-bar: client1]
  c2[foo-bar: client2]
  end
  subgraph authz [authz response headers]
  a3[foo-bar: authz1]
  a1[foo-bar: authz2]
  end
  subgraph upstream [upstream request headers]
  u1[foo-bar: client1]
  u2[foo-bar: client2]
  u3[foo-bar: authz1]
  u4[foo-bar: authz2]
    end
  client--->upstream
  authz--->upstream

What I actually want is:

graph TB
  subgraph client [client request headers]
  c1[foo-bar: client1]
  c2[foo-bar: client2]
  end
  subgraph authz [authz response headers]
  a3[foo-bar: authz1]
  a1[foo-bar: authz2]
  end
  subgraph upstream [upstream request headers]
  u3[foo-bar: authz1]
  u4[foo-bar: authz2]
    end
  client--->upstream
  authz--->upstream

I believe this should be the behavior of allowed_upstream_headers but one could also argue that this should be a new option (maybe allowed_upstream_headers_to_replace?).

This actually the most desired behavior, as merging down the response headers from authz does not really make sense to me. I hope the illustration clarifies what I tried to explain earlier.

[tyxia]

Thanks for illustration! Nice diagram.

Looks like the behavior your described currently is not supported by allowed_upstream_headers and allowed_upstream_headers_to_append .

I am curious if your case can be achieved by headers_to_remove then + headers_to_append. You need to make sure remove old headers first then append.

[dastrobu]

@tyxia as far as I can see there is no way to configure headers_to_remove on external authz. This seems to be used internally, only.

The way we workaround this, is by combining with a HeaderMutation filter:

                    http_filters:
                      - name: envoy.filters.http.header_mutation
                        typed_config:
                          "@type": type.googleapis.com/envoy.extensions.filters.http.header_mutation.v3.HeaderMutation
                          mutations:
                            request_mutations:
                              - remove: "foo-bar"
                      - name: envoy.filters.http.ext_authz
                        typed_config:
                          "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz
                          transport_api_version: v3
                          http_service:
                            # ...
                            path_prefix: /authz
                            authorization_response:
                              allowed_upstream_headers_to_append:
                                patterns:
                                  - exact: "foo-bar"
                                    ignore_case: true

The major drawback is that it is a lot harder to understand the config and, more importantly, you cannot remove headers based on a prefix, as HeaderMutation only takes exact headers.

[tyxia]

@dastrobu I am thinking if authz_server can remove this header using this field

[dastrobu]

@tyxia interesting, seems to be an undocumented feature (?). I was able to remove the headers by setting x-envoy-auth-headers-to-remove: foo-bar. However, it removes all the headers, which does not surprise me, as this operation is applied after the authz call.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions[bot]]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.