fix(jwe-decrypt): reject tokens that fail to decrypt#13404
Open
shreemaan-abhishek wants to merge 2 commits into
Open
fix(jwe-decrypt): reject tokens that fail to decrypt#13404shreemaan-abhishek wants to merge 2 commits into
shreemaan-abhishek wants to merge 2 commits into
Conversation
jwe_decrypt_with_obj() returned only the decrypted value, so the error from the AES-256-GCM decryption was always discarded. rewrite() then never entered the "failed to decrypt JWE token" branch: a well-formed JWE whose ciphertext could not be decrypted was accepted and forwarded upstream instead of being rejected. Propagate the decryption error and return 400 when decryption does not yield a plaintext. Add a test covering a well-formed token whose ciphertext fails to decrypt.
The JWE token used by the header-parsing tests was generated before the IV handling was corrected, so it no longer decrypts under the current code. These tests previously passed only because a failed decryption did not stop the request. Now that a failed decryption is rejected, the token is regenerated so it decrypts successfully.
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness issue in the jwe-decrypt auth plugin where AES-256-GCM decryption errors were dropped, allowing structurally valid but undecryptable JWE tokens to pass through and be forwarded upstream. It propagates the decryption error and rejects requests when decryption fails, with a regression test ensuring undecryptable ciphertext is denied.
Changes:
- Propagate the AES-256-GCM decrypt error from
jwe_decrypt_with_obj()to its caller. - Update
rewrite()to reject requests when decryption yields no plaintext (returning HTTP 400). - Add a test case covering a well-formed JWE that fails integrity/authentication during decryption.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
apisix/plugins/jwe-decrypt.lua |
Return decrypt errors from the AES-GCM decrypt call and fail closed in rewrite() when plaintext is not produced. |
t/plugin/jwe-decrypt.t |
Update token fixtures and add a regression test ensuring undecryptable (but well-formed) JWE tokens are rejected with 400. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
jwe-decrypt'sjwe_decrypt_with_obj()returned only the decrypted text, so the error from the AES-256-GCM decryption was always discarded.rewrite()therefore never reached thefailed to decrypt JWE tokenbranch, and a well-formed JWE whose ciphertext could not actually be decrypted was treated as a success and forwarded upstream.This PR propagates the decryption error and returns
400when decryption does not produce a plaintext, so only tokens that genuinely decrypt are accepted.A test case was added to
t/plugin/jwe-decrypt.tcovering a well-formed token whose ciphertext fails to decrypt.This is backward compatible: tokens that decrypt successfully are unaffected; only tokens that never decrypted, which previously slipped through, are now correctly rejected.
Which issue(s) this PR fixes:
Checklist