AlinsRan commented on code in PR #13889:
URL: https://github.com/apache/apisix/pull/13889#discussion_r3893521221
##########
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:
Fixed. Both checks now compare against `nil`, so a header carrying `"alg":
false` or `"enc": false` is rejected instead of falling into the backward
compatible path. TEST 37 pins it.
##########
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:
Fixed, the comment now points at TEST 31.
##########
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:
Right, `user-key` was reconfigured with a different secret in TEST 17, so
the request failed on the key rather than on the AAD. TEST 23 now also creates
a `jwe-fail-key-twin` Consumer holding the same secret, and the tampered token
targets that kid, so the 400 can only come from the tag no longer covering the
header.
--
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]
