Copilot commented on code in PR #13889:
URL: https://github.com/apache/apisix/pull/13889#discussion_r3892961905
##########
t/plugin/jwe-decrypt.t:
##########
@@ -762,3 +762,123 @@ status: 400
}
--- response_body
status: 400
+
+
+
+=== TEST 31: RFC 7516 token authenticating the protected header is accepted
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+
+ -- generated with an independent JWE producer (python
cryptography),
+ -- so the tag covers the encoded protected header as the AES-GCM
AAD
+ local token =
"eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIiwia2lkIjoiandlLWZhaWwta2V5In0."
+ .. ".MTIzNDU2Nzg5MDEy.6JeRgm0.KaxbSD-kuYBVck03POSk7w"
+
+ local code = t('/jwe-decrypt-fail', ngx.HTTP_GET, nil, nil,
+ { Authorization = "Bearer " .. token })
+ ngx.say("status: ", code)
+ }
+ }
+--- response_body
+status: 200
+
+
+
+=== TEST 32: token without AAD is still accepted
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+
+ -- same payload, encrypted the way APISIX used to generate tokens
+ local token =
"eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIiwia2lkIjoiandlLWZhaWwta2V5In0."
+ .. ".MTIzNDU2Nzg5MDEy.6JeRgm0.rNt131nG5wMvUD1KXbwLGA"
+
+ local code = t('/jwe-decrypt-fail', ngx.HTTP_GET, nil, nil,
+ { Authorization = "Bearer " .. token })
+ ngx.say("status: ", code)
+ }
+ }
+--- response_body
+status: 200
+
+
+
+=== TEST 33: replacing the kid of an RFC 7516 token is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+
+ -- the TEST 26 token with its kid changed to another Consumer that
Review Comment:
This reference is stale: TEST 26 covers a missing token with `strict=false`,
while the RFC 7516 source token is now in TEST 31. Pointing to the current test
number avoids sending future maintainers to an unrelated case.
##########
apisix/plugins/jwe-decrypt.lua:
##########
@@ -138,6 +138,17 @@ local function load_jwe_token(jwe_token)
end
+-- the plugin only implements direct encryption with A256GCM; reject a token
+-- that asks for anything else instead of failing later with a decrypt error
+local function unsupported_header(header_obj)
+ if header_obj.alg and header_obj.alg ~= "dir" then
+ return true
+ end
+
+ return header_obj.enc and header_obj.enc ~= "A256GCM"
Review Comment:
The truthiness checks treat an explicit JSON `false` as if the field were
omitted. Consequently, a token with `"alg": false` or `"enc": false` can reach
decryption and be accepted, despite the new contract rejecting every supplied
value other than `dir`/`A256GCM`. Compare with `nil` so backward compatibility
applies only when the field is genuinely absent.
##########
t/plugin/jwe-decrypt.t:
##########
@@ -762,3 +762,123 @@ status: 400
}
--- response_body
status: 400
+
+
+
+=== TEST 31: RFC 7516 token authenticating the protected header is accepted
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+
+ -- generated with an independent JWE producer (python
cryptography),
+ -- so the tag covers the encoded protected header as the AES-GCM
AAD
+ local token =
"eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIiwia2lkIjoiandlLWZhaWwta2V5In0."
+ .. ".MTIzNDU2Nzg5MDEy.6JeRgm0.KaxbSD-kuYBVck03POSk7w"
+
+ local code = t('/jwe-decrypt-fail', ngx.HTTP_GET, nil, nil,
+ { Authorization = "Bearer " .. token })
+ ngx.say("status: ", code)
+ }
+ }
+--- response_body
+status: 200
+
+
+
+=== TEST 32: token without AAD is still accepted
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+
+ -- same payload, encrypted the way APISIX used to generate tokens
+ local token =
"eyJhbGciOiJkaXIiLCJlbmMiOiJBMjU2R0NNIiwia2lkIjoiandlLWZhaWwta2V5In0."
+ .. ".MTIzNDU2Nzg5MDEy.6JeRgm0.rNt131nG5wMvUD1KXbwLGA"
+
+ local code = t('/jwe-decrypt-fail', ngx.HTTP_GET, nil, nil,
+ { Authorization = "Bearer " .. token })
+ ngx.say("status: ", code)
+ }
+ }
+--- response_body
+status: 200
+
+
+
+=== TEST 33: replacing the kid of an RFC 7516 token is rejected
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+
+ -- the TEST 26 token with its kid changed to another Consumer that
+ -- happens to share the secret: the tag no longer covers the header
Review Comment:
This test does not actually exercise two Consumers sharing a secret. The
current `user-key` fixture is reconfigured at lines 398-406 with a different
base64-decoded key, while `jwe-fail-key` uses the literal 32-byte secret at
lines 539-544, so this request would be rejected for using the wrong key even
if the header were not authenticated. Create or update a second Consumer with
the exact same secret before targeting its `kid`, so the failure specifically
verifies AAD tamper protection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
