Skip to content

jwt_authn: fix a bug where JWT with wrong issuer is allowed in allow_missing case#15194

Merged
asraa merged 3 commits intoenvoyproxy:mainfrom
qiwzhang:saved-jwt-allow-missing-bug
Feb 26, 2021
Merged

jwt_authn: fix a bug where JWT with wrong issuer is allowed in allow_missing case#15194
asraa merged 3 commits intoenvoyproxy:mainfrom
qiwzhang:saved-jwt-allow-missing-bug

Conversation

@qiwzhang
Copy link
Contributor

@qiwzhang qiwzhang commented Feb 25, 2021

When allow_missing is used inside RequiresAny, the requests with JWT with wrong issuer are accepted. This is a bug, allow_missing should only allow requests without any JWT.

This change fixed the above issue by preserving JwtUnknownIssuer in allow_missing case.

Change details:

  • JwtUnkownIssuer error got converted to JwtMissing in error aggregation inside VerifyAny object. This is the root cause. Such conversion is not needed.
  • Fix is to preserve JwtUknownIssuer in the error aggregation.

Risk Level: Low
Testing: unit-tested
Docs Changes: N/A
Release Notes: Yes

Signed-off-by: Wayne Zhang <qiwzhang@google.com>
Signed-off-by: Wayne Zhang <qiwzhang@google.com>
@qiwzhang qiwzhang requested a review from lizan as a code owner February 25, 2021 19:52
@qiwzhang
Copy link
Contributor Author

@yangminzhu could you help to review it?

@htuch
Copy link
Member

htuch commented Feb 25, 2021

@lizan tagging for a pass from you, thanks!

lizan
lizan previously approved these changes Feb 25, 2021
yangminzhu
yangminzhu previously approved these changes Feb 25, 2021
Signed-off-by: Wayne Zhang <qiwzhang@google.com>
@qiwzhang qiwzhang dismissed stale reviews from yangminzhu and lizan via 57a2fb4 February 25, 2021 20:59
Copy link
Contributor

@yangminzhu yangminzhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix!

@asraa asraa merged commit ea39e3c into envoyproxy:main Feb 26, 2021
@Shikugawa Shikugawa added the backport/approved Approved backports to stable releases label Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/approved Approved backports to stable releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants