fix(jwe-decrypt): accept JWE tokens that authenticate the protected header - #13889
Open
AlinsRan wants to merge 3 commits into
Open
fix(jwe-decrypt): accept JWE tokens that authenticate the protected header#13889AlinsRan wants to merge 3 commits into
AlinsRan wants to merge 3 commits into
Conversation
dosubot
Bot
added
the
size:L
AlinsRan
force-pushed
the
feat/jwe-decrypt-rfc7516-aad
branch
from
August 27, 2026 09:34
6cee77e to
7011949
Compare
nic-6443
previously approved these changes
Aug 28, 2026
…eader RFC 7516 section 5.1 makes the encoded protected header the AES-GCM additional authenticated data, so every JWE library computes the tag over it. The plugin decrypts with no AAD, so a token produced by a compliant library never authenticates and is rejected with 400. Try the RFC 7516 form first and fall back to decrypting without AAD, so tokens generated the way APISIX itself used to generate them keep working. Authenticating the header also makes `kid` tamper-proof for compliant tokens: replacing it now breaks the tag even when the two Consumers share a secret. Also reject a header that asks for an `alg` or `enc` the plugin does not implement, instead of reporting a decryption failure for it.
Only a header naming an algorithm the plugin does not implement is rejected; an absent alg or enc stays accepted, since such tokens decrypt today and nothing in the plugin branches on either field.
AlinsRan
force-pushed
the
feat/jwe-decrypt-rfc7516-aad
branch
from
August 31, 2026 03:55
7011949 to
005d148
Compare
nic-6443
previously approved these changes
Aug 31, 2026
shreemaan-abhishek
previously approved these changes
Aug 31, 2026
There was a problem hiding this comment.
Pull request overview
Adds RFC 7516-compatible protected-header authentication while retaining legacy token support.
Changes:
- Uses the protected header as AES-GCM AAD, with a legacy fallback.
- Validates
algandenc. - Adds interoperability tests and updates documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
apisix/plugins/jwe-decrypt.lua |
Implements AAD decryption and algorithm validation. |
t/plugin/jwe-decrypt.t |
Adds compatibility, tampering, and validation tests. |
docs/en/latest/plugins/jwe-decrypt.md |
Documents standard and legacy JWE behavior. |
docs/zh/latest/plugins/jwe-decrypt.md |
Updates Chinese JWE guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A JSON false decodes to a Lua false, so the truthiness check let a header carrying "alg": false through the algorithm validation. Compare against nil so the backward compatible path only covers a genuinely absent field. Also make the AAD tamper test point at the token it is derived from and target a Consumer that really shares the secret, so the rejection can only come from the header no longer being authenticated.
AlinsRan
dismissed stale reviews from shreemaan-abhishek, membphis, and nic-6443
via
August 31, 2026 10:13
4121e57
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-decryptdecrypts the token with no additional authenticated data:RFC 7516 §5.1 makes the encoded protected header the AES-GCM AAD for compact serialization, so every JWE library computes the tag over it. A token from a compliant producer therefore never authenticates and is rejected with
400 failed to decrypt JWE token, even though the key, IV and ciphertext are all correct.Reproduced against master with a token generated by python
cryptography(AESGCM(key).encrypt(iv, payload, aad=protected_header)): the identical token with the tag computed without AAD is accepted, the compliant one is rejected.This PR tries the RFC 7516 form first and falls back to decrypting without AAD, so tokens generated the way APISIX itself used to generate them — including the one in the plugin documentation and everything produced by the
/apisix/plugin/jwe/encryptendpoint removed in #13464 — keep working. Authenticating the header also makeskidtamper-proof for compliant tokens: swapping it breaks the tag even when the two Consumers share a secret.It also rejects a header that asks for an
algorencthe plugin does not implement (onlydir/A256GCMare), instead of letting it fail later as a decryption error. Tokens that omit either field keep working, and a token that names another algorithm cannot decrypt today anyway, so no working configuration changes.Trade-off worth naming: a legacy token now costs two GCM operations instead of one, since the AAD attempt runs first.
Checklist
Tests: TEST 26 accepts an RFC 7516 token produced by an independent library, TEST 27 pins the legacy no-AAD token, TEST 28 shows a
kidswap on a compliant token is rejected across Consumers sharing a secret, TEST 29/30 cover the unsupportedalg/enc. TEST 26, 29 and 30 fail on master.Note: this touches
jwe_decrypt_with_obj(), the same function as #13844; whichever lands first, I will rebase the other.