Update envoy to jwt clock_skew change#420
Update envoy to jwt clock_skew change#420qiwzhang merged 2 commits intoGoogleCloudPlatform:masterfrom
Conversation
Signed-off-by: Wayne Zhang <qiwzhang@google.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiwzhang, TAOXUY The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
Signed-off-by: Wayne Zhang <qiwzhang@google.com>
|
New changes are detected. LGTM label has been removed. |
|
/test ESPv2-presubmit-asan |
1 similar comment
|
/test ESPv2-presubmit-asan |
| }, | ||
| { | ||
| desc: "Failed, token with wrong issuer", | ||
| desc: "Successed, token with wrong issuer is considered as missing", |
There was a problem hiding this comment.
Hi @qiwzhang, was there a change in upstream envoy that changes this test? Just curious why it changed, it's not obvious.
There was a problem hiding this comment.
Yes, it is due to my recently submitted pr envoyproxy/envoy#13839
If a Jwt has "iss" and "iss" doesn't matched the one specified in the provider, it will return JwtUnknownIssuer.
In this PR, JwtUnkownIssuer is treated as JwtMissing.
This is the part I am debating, if the spec wants to verify JWT with "iss1" and allow_missing. But a JWT with "iss2" comes, should we reject it? Is this consider as "missing"?
For now, it is considered as "missing" and it is accepted.
There was a problem hiding this comment.
I see, that is interesting. JWT requirement config allows both allow_missing and allow_missing_or_failed. Wouldn't the issuer mismatch be a verification failure? Then allow_missing should reject it, but allow_missing_or_failed should pass through.
There was a problem hiding this comment.
Well, maybe you could clarify what the term "verification" means in this documentation. Verifying the entire token (including the issuer), or just verifying the signature matches?
There was a problem hiding this comment.
We verify signature, "aud" and "iss".
Signed-off-by: Wayne Zhang qiwzhang@google.com
It is to fix: #369