envoy icon indicating copy to clipboard operation
envoy copied to clipboard

JWT Claim to Header extraction fails when claim is a URL-like string

Open CelsoSantos opened this issue 2 years ago • 3 comments

When attempting to perform a JWT Claim to Header extraction, envoy will fail to do the extraction if the claim is a URL-like string, for instance http://example.org/some_claim, and the fail is possibly silently, as I've been unable to locate logs with the failure.

Given a claim like

{
  "iss": "http://example.org/",
  "sub": "[email protected]",
  "iat": 1712240289,
  "exp": 1743776289,
  "aud": "http://example.org/",
  "flavour": "chocolate",
  "parent_token": "abc",
  "some_url_value": "http://example.org/about",
  "http://example.org/parent_token": "xyz"
}

on the resulting request headers, the claim "http://example.org/parent_token": "xyz" will not have been extracted.

Here's the claim_to_headers block:

claim_to_headers:
- header_name: cookie
  claim_name: flavour
- header_name: x-subject
  claim_name: sub
- header_name: x-simple-claim
  claim_name: parent_token
- header_name: x-url-value-claim
  claim_name: some_url_value
- header_name: x-url-key-claim
  claim_name: http://example.org/parent_token
- header_name: x-quoted-claim
  claim_name: 'http://example.org/parent_token'
- header_name: x-regex-1-claim
  claim_name: http:\/\/example.org\/parent_token
- header_name: x-regex-2-claim
  claim_name: http:\\/\\/example\\.org\\/parent_token

In order to show this, I've created a docker compose based reproduction at CelsoSantos/envoy-jwt-claim-extraction, which contains a README.md detailing the steps to reproduce and showcase the issue.

Now, there are some open questions here, namely:

  1. Is this not expected functionality?
  2. If it IS expected functionality, where and why is it breaking? It's not clear from the jwt logs nor others inspected. Which log should expose the error message?
  3. If it needs to be treated as a regex, what is the correct way to escape the string? go-format? javascript? Could an example be provided?

CelsoSantos avatar Apr 17 '24 11:04 CelsoSantos

cc @taoxuy @lizan as codeowners

adisuissa avatar Apr 19 '24 18:04 adisuissa

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] avatar May 19 '24 20:05 github-actions[bot]

i'll volunteer to take a look into this

derekargueta avatar May 21 '24 01:05 derekargueta

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] avatar Jun 20 '24 08:06 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.

github-actions[bot] avatar Jun 27 '24 12:06 github-actions[bot]

I'm going to have some free time soon (in between jobs). Can I help to assess this in any way?

@derekargueta, how can I help? The issue became stale in the meantime...

CelsoSantos avatar Jul 04 '24 18:07 CelsoSantos

This is because of a poorly documented feature of "nested claim" support. Dot is interpreted as a separator. https://github.com/google/jwt_verify_lib/blob/master/src/struct_utils.cc#L108

kyessenov avatar Jul 09 '24 23:07 kyessenov

What would be the best way to handle this then? Should the jet_verify_lib handle these situations or should it be handled before reaching it?

I think ideally in the base lib, but I'm afraid I don't know enough C++ to handle that change nor do I know how receptive google is to PRs

CelsoSantos avatar Jul 25 '24 19:07 CelsoSantos

I think the base library should not be interpreting the keys and let Envoy handle tree-walking.

kyessenov avatar Jul 25 '24 22:07 kyessenov

Turns out, there's actually an open PR on the jwt_verify_lib repo

CelsoSantos avatar Oct 04 '24 11:10 CelsoSantos