-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
JWT metadata broken in Envoyfilter since 1.21.0 #50246
Comments
This appears to be an issue in the proxy, rather than the control plane. |
I think the key under meta is now simply |
https://istio.io/latest/news/releases/1.21.x/announcing-1.21/change-notes/#security talks about some changes to the dynamic metadata key
So there's definitely been some jwt changes. However this doesn't explain the issue i'm having (we were using |
I can work around this by doing:
But would be good to know if this change is intentional? If so; it probably should be on the release notes somewhere. |
@kyessenov was looking into something similar I think |
@Stono I think it was not under
The reason for |
@kyessenov I'm not sure I understand you, we have a perfectly working envoyfilter on 1.20; which has the claims under |
Ah, yes, you are right, the payload was placed under the hard-coded issuer key previously. Now the key is always "payload" since it simplifies the configuration. |
@keithmattix comment regarding the breakage only impacts you if you have an Authorization policy using JWT claims. If you do your own logic with Lua, the bug doesn't affect you. We can update the notes to mention the internal change to xDS, but we usually reserve the right to do small changes like this. |
Agree, it's simpler.
That's why I suggested it might simply be a release note thing. I'm willing to bet I'm not the only user reading jwt claims in a lua envoyfilter? |
Oh I missed that this was Lua; my bad |
Does this also mean the apiVersion: security.istio.io/v1beta1
kind: AuthorizationPolicy
metadata:
name: allow-ingress-infra-w-token
spec:
action: ALLOW
rules:
- from:
- source:
namespaces:
- istio-system
principals:
- cluster.local/ns/istio-system/sa/foo-ingressgateway-service-account
requestPrincipals:
# i RequestAuthentication URL (<ISS>/<SUB>)
- 'https://token.actions.githubusercontent.com/repo:ORGNAME/REPONAME:ref:refs/heads/main' Downgrading the proxy worked. cc @larhauga |
@chlunde The format is the same. However, it's interpreted as two equalities "iss=A" and "sub=B" for a value "A/B". When you have slashes in the subject like in your cause, the slash is chosen at the wrong position, e.g. "sub=main" above because it becomes ambiguous where the issuer ends. JWT spec seems to allow arbitrary strings in the issuer:
I think for your case, it's probably better to explicitly specify "iss" and "sub" claim equality with conditions. |
Not sure if I fully understand, @kyessenov is this a regression? Should it be fixed? If not, would it be best to deprecate |
Yes, we should update the documentation, this is a regression or at the very least under-specified API. |
If someone stumbles upon this issue in the meantime, here's what we changed to: - from:
- source:
namespaces:
- istio-system
principals:
- cluster.local/ns/istio-system/sa/FOO-ingressgateway-service-account
requestPrincipals: ['*']
when:
- key: request.auth.claims[iss]
values: [https://token.actions.githubusercontent.com]
- key: request.auth.claims[sub]
values:
- repo:ORG/REPO:ref:refs/heads/main |
This is a pretty huge change for us to consume (in terms of # of affected Just to confirm, is #49913 is contributing to the above? My interpretation of @kyessenov comment above is that requestPrincipal is parsing out For context, we're testing |
The issue @chlunde run into is that the subject contains a slash. If you do not have a slash, I think the matching should work as expected when it includes |
Is this the right place to submit this?
Bug Description
Attempted to update to 1.21 today; and our envoyfilter which extracts JWT metadata is failing, specifically we do this:
In this case,
claims
is empty, whereas on1.20
is was correctly populated.We have the following
RequestAuthentication
filter:Version
Additional Information
No response
The text was updated successfully, but these errors were encountered: