Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect behavior with Routing based on JWT Token Dynamic Metadata #19910

Closed
mantasmatelis opened this issue Feb 10, 2022 · 17 comments
Closed
Labels
area/http area/jwt_authn bug stale stalebot believes this issue/PR has not been touched recently

Comments

@mantasmatelis
Copy link

mantasmatelis commented Feb 10, 2022

Title: Incorrect behavior with Routing based on JWT Token Dynamic Metadata

Description:
When using the payload_in_metadata flag of the JWT Authentication filter, dynamic metadata is attached to the stream and visible in logging. However, this dynamic metadata cannot normally be used for HTTP routing.

If I set an unrelated header in lua, the dynamic metadata can be used for routing, see the repro steps below.

Repro steps:
Run envoy (version: a9d72603c68da3a10a1c0d021d01c7877e6f2a30/1.21.0/Clean/RELEASE/BoringSSL) with the config provided pointing to the provided keys.json. Send an example request with a valid JWT and shard_id in the payload of either 1 or 2. Example requests that are valid with the provided keys.json are below.

(if you use the config as shown below)

$ curl <ip> -H "Authorization: Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTY0NDM0Mjg1Miwic2hhcmRfaWQiOiIyIn0.Y0iDgULUicqYV9v3-q_JulhonBtEjEFS4aawPj5sVU4cNVqE2sCQ9YtzO7LxqiL5FN4lcD7D__JsJ7swyB6Wpqf-DEqpE_vKsRCtPllBhArsDpNXiozWlBaJrhIM0L3OI9pMDQlQxU6BI3ZMux8K6OsaTTlqKnpcNnZ7BHTiYRIcbWzvNtQBaul-whdSpZwDH_cKzeoEAHeJyzMZ9iMSP_srlwXjI1iFHQt9BWXLnKDxUvosmg2ffrkDIiPmywdUTpoUU-FKnwahAoDb7h7Jvw3XcX0bobQ9JKKvp9bY8F7VDqy0KbQuw9DkB_aG754_KqMHuwSJX63sbJ_xFDKTww"
did not get to a specific cluster

(if you uncomment the single lua line request_handle:headers():add("a-header-that-doesnt-matter", "nop-value"))

$ curl  <ip> -H "Authorization: Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTY0NDM0Mjg1Miwic2hhcmRfaWQiOiIyIn0.Y0iDgULUicqYV9v3-q_JulhonBtEjEFS4aawPj5sVU4cNVqE2sCQ9YtzO7LxqiL5FN4lcD7D__JsJ7swyB6Wpqf-DEqpE_vKsRCtPllBhArsDpNXiozWlBaJrhIM0L3OI9pMDQlQxU6BI3ZMux8K6OsaTTlqKnpcNnZ7BHTiYRIcbWzvNtQBaul-whdSpZwDH_cKzeoEAHeJyzMZ9iMSP_srlwXjI1iFHQt9BWXLnKDxUvosmg2ffrkDIiPmywdUTpoUU-FKnwahAoDb7h7Jvw3XcX0bobQ9JKKvp9bY8F7VDqy0KbQuw9DkB_aG754_KqMHuwSJX63sbJ_xFDKTww"
this is cluster 2

I expect to get this is cluster 2 each time, as shard_id is set to 2 in the payload, but the routing based on dynamic metadata only works if the lua line is uncommented.

Config:

admin:
  address:
    socket_address:
      address: 127.0.0.1
      port_value: 9090
static_resources:
  listeners:
  - address:
      socket_address:
        address: 0.0.0.0
        port_value: 80
    per_connection_buffer_limit_bytes: 32768 # 32 KiB
    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: http
          use_remote_address: true
          normalize_path: true
          merge_slashes: true
          path_with_escaped_slashes_action: UNESCAPE_AND_REDIRECT
          http2_protocol_options:
            max_concurrent_streams: 100
            initial_stream_window_size: 65536 # 64 KiB
            initial_connection_window_size: 1048576 # 1 MiB
          server_header_transformation: PASS_THROUGH
          common_http_protocol_options:
            idle_timeout: 900s
          stream_idle_timeout: 300s # 5 mins, must be disabled for long-lived and streaming requests
          request_timeout: 120s # 2 mins, must be disabled for long-lived and streaming requests
          request_headers_timeout: 15s
          stream_error_on_invalid_http_message: true
          route_config:
            virtual_hosts:
            - name: default
              domains:
              - "*"
              response_headers_to_remove:
                - x-envoy-upstream-service-time
              routes:
              - match:
                  prefix: /
                  dynamic_metadata:
                    - filter: envoy.filters.http.jwt_authn
                      path:
                      - key: payload
                      - key: shard_id
                      value:
                        string_match:
                          exact: "1"
                direct_response:
                  status: 200
                  body:
                    inline_string: |
                      this is cluster 1
              - match:
                  prefix: /
                  dynamic_metadata:
                    - filter: envoy.filters.http.jwt_authn
                      path:
                      - key: payload
                      - key: shard_id
                      value:
                        string_match:
                          exact: "2"
                direct_response:
                  status: 200
                  body:
                    inline_string: |
                      this is cluster 2
              - match:
                  prefix: /
                direct_response:
                  status: 200
                  body:
                    inline_string: |
                      did not get to a specific cluster
          access_log:
            - name: envoy.access_loggers.stderr
              typed_config:
                "@type": type.googleapis.com/envoy.extensions.access_loggers.stream.v3.StderrAccessLog
                log_format:
                  text_format: '[%START_TIME%] "%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH)% %PROTOCOL%" %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% "%REQ(X-FORWARDED-FOR)%" "%REQ(USER-AGENT)%" "%REQ(X-REQUEST-ID)%" "%REQ(:AUTHORITY)%" "%UPSTREAM_HOST%" "%DYNAMIC_METADATA(envoy.filters.http.jwt_authn)%"\n'
          http_filters:
          - name: envoy.filters.http.grpc_json_transcoder
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.grpc_json_transcoder.v3.GrpcJsonTranscoder
              proto_descriptor: "/config/unrelated_descriptor_set.protoset"
          - name: envoy.filters.http.jwt_authn
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.JwtAuthentication
              providers:
                provider:
                  local_jwks:
                    filename: /config/keys.json
                  payload_in_metadata: payload
              rules:
                - match:
                    prefix: /
                  requires:
                    provider_name: provider
          - name: envoy.filters.http.lua
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
              inline_code: |
                function envoy_on_request(request_handle)
                  -- request_handle:headers():add("a-header-that-doesnt-matter", "nop-value")
                end
          - name: envoy.filters.http.router

keys.json:

{
  "keys": [
    {
      "kty": "RSA",
      "n": "u1SU1LfVLPHCozMxH2Mo4lgOEePzNm0tRgeLezV6ffAt0gunVTLw7onLRnrq0_IzW7yWR7QkrmBL7jTKEn5u-qKhbwKfBstIs-bMY2Zkp18gnTxKLxoS2tFczGkPLPgizskuemMghRniWaoLcyehkd3qqGElvW_VDL5AaWTg0nLVkjRo9z-40RQzuVaE8AkAFmxZzow3x-VJYKdjykkJ0iT9wCS0DRTXu269V264Vf_3jvredZiKRkgwlL9xNAwxXFg0x_XFw005UWVRIkdgcKWTjpBP2dPwVZ4WWC-9aGVd-Gyn1o0CLelf4rEjGoXbAAEgAqeGUxrcIlbjXfbcmw",
      "e": "AQAB",
      "alg": "RS256",
      "use": "sig"
    }
  ]
}

Logs:
(no relevant logs in output)

@mantasmatelis mantasmatelis added bug triage Issue requires triage labels Feb 10, 2022
@htuch
Copy link
Member

htuch commented Feb 11, 2022

CC @qiwzhang

@htuch htuch added area/http area/jwt_authn and removed triage Issue requires triage labels Feb 11, 2022
@daixiang0
Copy link
Member

Test at local, it does not work when remove lua filter.

@lambdai
Copy link
Contributor

lambdai commented Feb 18, 2022

the jwt filter is configured to populate metadata per config payload_in_metadata: payload
I believe the added metadata is to guide a new route
However, the refresh new route is not in the current jwt_authn filter.

THe lua filter trigger the fresh new route because lua filter after any header operation.

We should add a new config in the jwt_authn that clear the snapped route entry

@daixiang0
Copy link
Member

daixiang0 commented Feb 18, 2022

Is it a bug as jwt filter do not guide a right route?

@lambdai
Copy link
Contributor

lambdai commented Feb 18, 2022

Is it a bug as jwt filter do not guide a right route?

It was not a bug before route filter can route upon filter state.

In short term, we'd better leave the clear/re-resolve route decision to the user of jwt filter

@daixiang0
Copy link
Member

The result looks like the jwt filter chooses the wrong route, I try to read codes to understand the flow between filters and routes but still not clear, could you tell more details about it, like regular flow and correct jwt filter flow?

@yangminzhu
Copy link
Contributor

yangminzhu commented Feb 23, 2022

We should add a new config in the jwt_authn that clear the snapped route entry

+1, the jwt filter currently does not clear the route which is needed by the dynamic_metadata to work, it should just be added to the jwt filter API. Meanwhile I believe you could workaround this by having a separate filter to clear the route after the jwt filter.

@github-actions
Copy link

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 github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 26, 2022
@mantasmatelis
Copy link
Author

not stale

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Mar 31, 2022
@lucallero
Copy link

I got the same problem but using an external auth-server, which adds the user-id to the dynamic metadata. For later on routing based on user-ids patterns, using the dynamic_metadata match. However, it doesn't work as expected, I managed to print the user-id with Lua, but the matcher apparently is not seeing the user-id metadata, it never matches.

This Lua is reading and logging the user-id from the dynamic metadata.

function envoy_on_response(response_handle)
    local meta = response_handle:streamInfo():dynamicMetadata():get("envoy.filters.http.ext_authz")["user-id"]
    response_handle:logInfo("user-id: " .. meta)
end

Output:
...[info][lua] [source/extensions/filters/http/lua/lua_filter.cc:796] script log: user-id: A11111

And this is the snippet of the dynamic_metadata that is not matching :

- match:
    prefix: /
    dynamic_metadata:
      - filter: envoy.filters.http.ext_authz
        path:
        - key: user-id
        value:
          string_match:
            prefix: "A"
  route:
    cluster: a_upstream

I believe it's the same problem, and it is related to the dynamic_metadata matching feature. That's why I posted here. Otherwise let me know if you want this to be reported as another bug.

@theganyo
Copy link
Contributor

theganyo commented Apr 6, 2022

I'm seeing the same thing when just using Lua to set the metadata string_match value.

@github-actions
Copy link

github-actions bot commented May 7, 2022

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 github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label May 7, 2022
@lucallero
Copy link

not stale

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label May 7, 2022
@github-actions
Copy link

github-actions bot commented Jun 6, 2022

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 github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jun 6, 2022
@github-actions
Copy link

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.

@jparklab
Copy link
Contributor

I also hit this issue. I am using ext_auth filter to set dynamic metadata and envoy fails to match route using the dynamic metadata unless I add the lua filter that sets a dummy header. I've tested with envoy 1.20 and 1.21 and both showed the issue

@jparklab
Copy link
Contributor

jparklab commented Jun 28, 2022

In my case, it happened because clear_route_cache is not triggered when there is no additional header set by the filter(see https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/ext_authz/v3/ext_authz.proto.html), but I only set metadata from ext_authz filter. Route matching works as expected after I set a header from ext_authz filter.
@lucallero you'd want to check if you are setting a header from the filter.
(I think route should also be cleared when dynamic metadata is set, but it is a different issue than this one)

I believe this issue happens because envoy finds route before applying filters, and does not re-match route unless route is cleared(https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/http/http_filters#filter-route-mutation). The workaround using lua filter works because it clears routes if header is modifed (https://github.com/envoyproxy/envoy/blob/main/source/extensions/filters/http/lua/lua_filter.h#L509)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/http area/jwt_authn bug stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

No branches or pull requests

8 participants