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
1 change: 1 addition & 0 deletions apisix/utils/redis-schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ local policy_to_additional_properties = {
},
},
required = {"redis_host"},
encrypt_fields = {"redis_password"}
},
["redis-cluster"] = {
properties = {
Expand Down
8 changes: 8 additions & 0 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
Loading
Loading