Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support more sensitive fields for encryption #11095

Merged
merged 15 commits into from
Mar 29, 2024
30 changes: 21 additions & 9 deletions apisix/plugins/jwe-decrypt.lua
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ local consumer_schema = {
is_base64_encoded = { type = "boolean" },
},
required = { "key", "secret" },
encrypt_fields = { "key", "secret" },
}


Expand All @@ -71,15 +72,26 @@ function _M.check_schema(conf, schema_type)
return false, err
end

-- restrict the length of secret, we use A256GCM for encryption,
-- so the length should be 32 chars only
if conf.is_base64_encoded then
if #base64.decode_base64url(conf.secret) ~= 32 then
return false, "the secret length after base64 decode should be 32 chars"
end
else
if #conf.secret ~= 32 then
return false, "the secret length should be 32 chars"
local local_conf, err = core.config.local_conf(true)
if not local_conf then
return false, "failed to load the configuration file: " .. err
end

local encrypted = core.table.try_read_attr(local_conf, "apisix", "data_encryption",
"enable_encrypt_fields") and (core.config.type == "etcd")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to add this ?
I think the check_schema is run before the fields encrypted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if data encryption is enabled then the secret length will be more than 32. So we should not check the length if data encryption is on.

I think the check_schema is run before the fields encrypted.

yes. This is why we cannot use the code in plugin.lua, so I just copied the logic 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

if data encryption is enabled then the secret length will be more than 32.

Why the secret length will be more than 32. Is the secret is encrypted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, exactly.


-- if encrypted, the secret length will exceed 32 so don't check
if not encrypted then
-- restrict the length of secret, we use A256GCM for encryption,
-- so the length should be 32 chars only
if conf.is_base64_encoded then
if #base64.decode_base64url(conf.secret) ~= 32 then
return false, "the secret length after base64 decode should be 32 chars"
end
else
if #conf.secret ~= 32 then
return false, "the secret length should be 32 chars"
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion apisix/plugins/openid-connect.lua
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ local schema = {
}
}
},
encrypt_fields = {"client_secret"},
encrypt_fields = {"client_secret", "client_rsa_private_key"},
required = {"client_id", "client_secret", "discovery"}
}

Expand Down
3 changes: 2 additions & 1 deletion apisix/plugins/openwhisk.lua
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ local schema = {
keepalive_timeout = {type = "integer", minimum = 1000, default = 60000},
keepalive_pool = {type = "integer", minimum = 1, default = 5}
},
required = {"api_host", "service_token", "namespace", "action"}
required = {"api_host", "service_token", "namespace", "action"},
encrypt_fields = {"service_token"}
}


Expand Down
66 changes: 47 additions & 19 deletions t/plugin/jwe-decrypt.t
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ done


=== TEST 4: secret length too long
--- yaml_config
apisix:
data_encryption:
enable_encrypt_fields: false
--- config
location /t {
content_by_lua_block {
Expand All @@ -115,6 +119,10 @@ done


=== TEST 5: secret length too long(base64 encode)
--- yaml_config
apisix:
data_encryption:
enable_encrypt_fields: false
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -163,7 +171,27 @@ passed



=== TEST 7: enable jwe-decrypt plugin using admin api
=== TEST 7: verify encrypted field
--- config
location /t {
content_by_lua_block {
local json = require("toolkit.json")
local t = require("lib.test_admin").test

-- get plugin conf from etcd, secret and key is encrypted
local etcd = require("apisix.core.etcd")
local res = assert(etcd.get('/consumers/jack'))
ngx.say(res.body.node.value.plugins["jwe-decrypt"].key)
ngx.say(res.body.node.value.plugins["jwe-decrypt"].secret)
}
}
--- response_body
XU29sA3FEVF68hGcdPo7sg==
f9pGB0Dt4gYNCLKiINPfVSviKjQs2zfkBCT4+XZ3mDABZkJTr0orzYRD5CptDKMc



=== TEST 8: enable jwe-decrypt plugin using admin api
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -198,7 +226,7 @@ passed



=== TEST 8: create public API route (jwe-decrypt sign)
=== TEST 9: create public API route (jwe-decrypt sign)
--- config
location /t {
content_by_lua_block {
Expand All @@ -224,7 +252,7 @@ passed



=== TEST 9: sign / verify in argument
=== TEST 10: sign / verify in argument
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -254,14 +282,14 @@ hello world



=== TEST 10: test for unsupported method
=== TEST 11: test for unsupported method
--- request
PATCH /apisix/plugin/jwe/encrypt?key=user-key
--- error_code: 404



=== TEST 11: verify, missing token
=== TEST 12: verify, missing token
--- request
GET /hello
--- error_code: 403
Expand All @@ -270,7 +298,7 @@ GET /hello



=== TEST 12: verify: invalid JWE token
=== TEST 13: verify: invalid JWE token
--- request
GET /hello
--- more_headers
Expand All @@ -281,7 +309,7 @@ Authorization: invalid-eyJhbGciOiJkaXIiLCJraWQiOiJ1c2VyLWtleSIsImVuYyI6IkEyNTZHQ



=== TEST 13: verify (in header)
=== TEST 14: verify (in header)
--- request
GET /hello
--- more_headers
Expand All @@ -291,7 +319,7 @@ hello world



=== TEST 14: verify (in header without Bearer)
=== TEST 15: verify (in header without Bearer)
--- request
GET /hello
--- more_headers
Expand All @@ -301,7 +329,7 @@ hello world



=== TEST 15: verify (header with bearer)
=== TEST 16: verify (header with bearer)
--- request
GET /hello
--- more_headers
Expand All @@ -311,7 +339,7 @@ hello world



=== TEST 16: verify (invalid bearer token)
=== TEST 17: verify (invalid bearer token)
--- request
GET /hello
--- more_headers
Expand All @@ -322,7 +350,7 @@ Authorization: bearer invalid-eyJhbGciOiJkaXIiLCJraWQiOiJ1c2VyLWtleSIsImVuYyI6Ik



=== TEST 17: delete a exist consumer
=== TEST 18: delete a exist consumer
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -372,7 +400,7 @@ code: true body: passed



=== TEST 18: add consumer with username and plugins with base64 secret
=== TEST 19: add consumer with username and plugins with base64 secret
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -402,7 +430,7 @@ passed



=== TEST 19: enable jwt decrypt plugin with base64 secret
=== TEST 20: enable jwt decrypt plugin with base64 secret
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -436,7 +464,7 @@ passed



=== TEST 20: create public API route (jwe-decrypt sign)
=== TEST 21: create public API route (jwe-decrypt sign)
--- config
location /t {
content_by_lua_block {
Expand All @@ -462,7 +490,7 @@ passed



=== TEST 21: sign / verify in argument
=== TEST 22: sign / verify in argument
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -494,7 +522,7 @@ hello world



=== TEST 22: verify (in header)
=== TEST 23: verify (in header)
--- request
GET /hello
--- more_headers
Expand All @@ -504,7 +532,7 @@ hello world



=== TEST 23: verify (in header without Bearer)
=== TEST 24: verify (in header without Bearer)
--- request
GET /hello
--- more_headers
Expand All @@ -514,7 +542,7 @@ hello world



=== TEST 24: enable jwt decrypt plugin with test upstream route
=== TEST 25: enable jwt decrypt plugin with test upstream route
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -548,7 +576,7 @@ passed



=== TEST 25: verify in upstream header
=== TEST 26: verify in upstream header
--- request
GET /headers
--- more_headers
Expand Down
Loading
Loading