Envoy: Question: set-cookie on client response in successful ExtAuthz response

[mikehuston]

When using ExtAuthz HttpService, is there a way to set set-cookie header on the client response based on the same header in a response from a successful ExtAuthz request?

allowed_client_headers is close to what I want, but my read of the doc comments (and rudimentary manual tests) is that these response headers are only transferred to the client response for unsuccessful ExtAuthz requests, whereas I would like to set the set-cookie header on the client response for a successful ExtAuthz request. One use case for this is to extend a session cookie validity as the user remains active.

Could someone confirm this is correct, and perhaps suggest a workaround for this using other Envoy primitives? Ideally in order to best follow principle of least privilege, I'd prefer that the set-cookie header only be sent to the client and not upstream.

[mattklein123]

cc @dio @gsagula

[stale[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 other activity occurs. Thank you for your contributions.

[stale[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". Thank you for your contributions.

[dio]

I'm investigating this. Once it is possible to re-open the issue, I'll do it.

[dio]

There are two things here.

1. You need to set the response_header_to_add for the router. This can be done Today, through a config, e.g.

          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              request_headers_to_remove:
              - i-dont-want-upstream-to-see-this
              response_headers_to_add:
              - header:
                  key: "foo"
                  value: "%REQ(foo)%"

However, you need to send that foo header to "upstream" through:

          - name: envoy.ext_authz
            config:
              http_service:
                server_uri:
                  uri: http://authorization-service:8080
                  cluster: service_authz
                  timeout: 2s
                authorization_response:
                  allowed_upstream_headers:
                    patterns:
                    - exact: foo

which probably you don't want to (hence the request_headers_to_remove). So the ask here, how to pass the intended headers (the response header from the ext_authz service) to, most likely, the router (so it understands the response headers to add (which are provided by ext_authz service) when responding to the client).

1. The config of response_header_to_add can't deal (yet) with repeated header key, which is normal for set-cookie.

              response_headers_to_add:
              - header:
                  key: "set-cookie"
                  value: "%REQ(set-cookie)%"

[stale[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 other activity occurs. Thank you for your contributions.

[stale[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". Thank you for your contributions.

[RichiCoder1]

@dio About to give this a try myself, so hopefully can answer if it worked well. That said, is there a way this be added as a built in feature to add authz responses headers to the proxy response rather then having to dive into transformers?

[dio]

Hum, true. I think if we can flag allowed_client_headers to always be considered when sending back the response to the client, probably a good solution for it.

[RichiCoder1]

@dio My only worry with that is you might end up including headers that make sense in a 401 response, but not in a normal succesful response. I can't currently think of a specific situation where that might pop up though.

[Bibob7]

I solved it by passing the set-cookie header to the upstream and applied a second lua filter that copies the set-cookie request header to the response. This is also a workaround, but at least, it works:

- applyTo: HTTP_FILTER
      match:
        context: SIDECAR_INBOUND
        listener:
          filterChain:
            filter:
              name: envoy.http_connection_manager
              subFilter:
                name: envoy.filters.http.jwt_authn
      patch:
        operation: INSERT_AFTER
        value:
          name: envoy.lua
          typed_config:
            "@type": "type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua"
            inlineCode: |
              function envoy_on_request(request_handle)
                local headers = request_handle:headers()
                request_handle:streamInfo():dynamicMetadata():set("envoy.filters.http.lua", "cookies", headers:get("set-cookie"))
              end

              function envoy_on_response(response_handle)
                local cookies = response_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.lua")["cookies"]
                response_handle:headers():add("set-cookie", cookies)
              end

[hisener]

@dio any update on this issue?

@Bibob7 thanks for the workaround. However, this doesn't work well when there are multiple set-cookie headers. IIUC, allowed_upstream_headers overrides the existing header, at least in the response I get only one one of them. So this forces me to use allowed_upstream_headers_to_append but that's also problematic as the RFC states.

   Origin servers SHOULD NOT fold multiple Set-Cookie header fields into
   a single header field.  The usual mechanism for folding HTTP headers
   fields (i.e., as defined in [RFC2616]) might change the semantics of
   the Set-Cookie header field because the %x2C (",") character is used
   by Set-Cookie in a way that conflicts with such folding.

Poor man's solution (wouldn't work if a cookie value contains ,):

function envoy_on_request(request_handle)
  local set_cookie_headers = request_handle:headers():get("set-cookie")
  request_handle:streamInfo():dynamicMetadata():set("envoy.filters.http.lua", "set_cookie_headers", set_cookie_headers)
end
function envoy_on_response(response_handle)
  local set_cookie_headers = response_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.lua")["set_cookie_headers"]
  if (set_cookie_headers ~= nil and set_cookie_headers ~= "") then
    for cookie in string.gmatch(set_cookie_headers, "([^,]+)") do
      response_handle:headers():add("set-cookie", cookie)
    end
  end
end

IMHO, this is a common case and should be supported either by extending allowed_client_headers or adding a new option like allowed_client_headers_on_success.

Similar report: https://github.com/envoyproxy/envoy/issues/10396.

[dio]

@hisener I removed the "stale" label at #10396 and put a help wanted label. I'll probably have time later this week, but just in case someone wants to do it before me.

[hisener]

@dio, should we open this issue or https://github.com/envoyproxy/envoy/issues/10396 to get some attention?

[esmet]

I think allowed_client_headers_on_success is the right option to add, and we can implement it by saving the external authorization server's response and adding those headers to the downstream client request during encodeHeaders(). Thoughts @RichiCoder1 + @hisener ?

@dio can you assign this one to me?

[peppered]

@Bibob7 , @hisener thanks for your solutions! I use Istio and I'm totally new to Envoy filters. Based on your suggestions I was able to write Istio EnvoyFilter for ext_authz that talks to oauth2-proxy:

apiVersion: networking.istio.io/v1alpha3
kind: EnvoyFilter
metadata:
  name: istio-ingressgateway
  namespace: istio-system
spec:
  workloadSelector:
    labels:
      app: istio-ingressgateway
  configPatches:
    - applyTo: HTTP_FILTER
      match:
        context: GATEWAY
        listener:
          filterChain:
            filter:
              name: envoy.http_connection_manager
              subFilter:
                name: envoy.filters.http.jwt_authn
      patch:
        operation: INSERT_BEFORE
        value:
          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: http://oauth2-proxy.oauth2-proxy.svc.cluster.local:4180/oauth2/auth
                cluster: outbound|4180||oauth2-proxy.oauth2-proxy.svc.cluster.local
                timeout: 10s
              authorizationRequest:
                allowedHeaders:
                  patterns:
                    - exact: "cookie"
              authorizationResponse:
                allowedUpstreamHeaders:
                  patterns:
                    - exact: authorization
                allowedUpstreamHeadersToAppend:
                  patterns:
                    - exact: set-cookie
                allowed_client_headers:
                  patterns:
                    - exact: set-cookie
    - applyTo: HTTP_FILTER
      match:
        context: GATEWAY
        listener:
          filterChain:
            filter:
              name: envoy.http_connection_manager
              subFilter:
                name: envoy.filters.http.jwt_authn
      patch:
        operation: INSERT_AFTER
        value:
          name: envoy.lua
          typed_config:
            "@type": "type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua"
            # language=lua
            inlineCode: |
              function envoy_on_request(request_handle)
                local headers = request_handle:headers()
                local cookies = {}
                for key, value in pairs(headers) do
                  if key == "set-cookie" then
                    cookies[#cookies + 1] = value
                  end
                end
                request_handle:streamInfo():dynamicMetadata():set("envoy.filters.http.lua", "cookies", cookies)
              end

              function envoy_on_response(response_handle)
                local cookies = response_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.lua")["cookies"]
                for _, cookie in pairs(cookies) do
                  response_handle:headers():add("set-cookie", cookie)
                end
              end

With @hisener solution I got browser error related to 4kb cookie limit. That was due to the Lua filter that put multiple set-cookies headers into one. This solution treats each set-cookie header separately.