You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/12/08 10:15:52 UTC
[GitHub] [apisix] tzssangglass opened a new pull request, #8487: feat: data encryption support more plugins
tzssangglass opened a new pull request, #8487:
URL: https://github.com/apache/apisix/pull/8487
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Fixes #8407
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046708488
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and not core.table.isempty(schema.encrypt_fields) then
+ for _, key in ipairs(schema.encrypt_fields) do
+ if conf[key] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[key] = encrypted
+ end
+ elseif core.string.find(key, ".") then
+ -- encrypt fields has indents
+ local res, err = re_split(key, "\\.", "jo")
+ if not res then
+ core.log.warn("failed to split key [", key, "], err: ", err)
+ return
+ end
+
+ -- we only support two levels
Review Comment:
So many limitations, maybe we should optimize this feature later when we have time
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1048421447
##########
docs/zh/latest/plugins/key-auth.md:
##########
@@ -41,8 +41,6 @@ Consumer 端：
| ---- | ------ | ------ | ------------------------------------------------------------------------------------------------------------- |
| key | string | 是 | 不同的 Consumer 应有不同的 `key`，它应当是唯一的。如果多个 Consumer 使用了相同的 `key`，将会出现请求匹配异常。 |
-注意：`key` 的 schema 中还定义了 `encrypted = true`，这意味着该字段将会被加密存储在 etcd 中。具体参考 [加密存储字段](../plugin-develop.md#加密存储字段)。
Review Comment:
typo, fix it
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1050304381
##########
docs/zh/latest/plugins/key-auth.md:
##########
@@ -41,6 +41,8 @@ Consumer 端：
| ---- | ------ | ------ | ------------------------------------------------------------------------------------------------------------- |
| key | string | 是 | 不同的 Consumer 应有不同的 `key`，它应当是唯一的。如果多个 Consumer 使用了相同的 `key`，将会出现请求匹配异常。 |
+注意：schema 中还定义了 `encrypt_fields = {"client_secret"}`，这意味着该字段将会被加密存储在 etcd 中。具体参考 [加密存储字段](../plugin-develop.md#加密存储字段)。
Review Comment:
fix
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
soulbird commented on PR #8487:
URL: https://github.com/apache/apisix/pull/8487#issuecomment-1343808375
It is best to update the documentation of each plugin to indicate the fields that will be encrypted.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046755567
##########
docs/en/latest/plugins/csrf.md:
##########
@@ -41,6 +41,8 @@ This Plugin considers the `GET`, `HEAD` and `OPTIONS` methods to be safe operati
| expires | number | False | `7200` | Expiration time in seconds of the CSRF cookie. Set to `0` to skip checking expiration time. |
| key | string | True | | Secret key used to encrypt the cookie. |
+NOTE: `encrypt_fields = {"key"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
yes, what is stored in etcd is the encrypted
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1048421040
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and not core.table.isempty(schema.encrypt_fields) then
+ for _, key in ipairs(schema.encrypt_fields) do
+ if conf[key] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[key] = encrypted
+ end
+ elseif core.string.find(key, ".") then
+ -- encrypt fields has indents
+ local res, err = re_split(key, "\\.", "jo")
+ if not res then
+ core.log.warn("failed to split key [", key, "], err: ", err)
+ return
+ end
+
+ -- we only support two levels
+ if conf[res[1]] and conf[res[1]][res[2]] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(
+ conf[res[1]][res[2]], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[res[1]][res[2]] = encrypted
+ end
+ end
+ end
end
end
end
_M.encrypt_conf = encrypt_conf
+check_plugin_metadata = function(item)
+ local ok, err = check_single_plugin_schema(item.id, item,
+ core.schema.TYPE_METADATA, true)
+ if ok and enable_gde()then
Review Comment:
done
##########
docs/en/latest/plugins/key-auth.md:
##########
@@ -41,8 +41,6 @@ For Consumer:
|------|--------|-------------|----------------------------|
| key | string | required | Unique key for a Consumer. |
-NOTE: The schema for `key` also defines `encrypted = true`, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
type, fix it
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1049233555
##########
docs/zh/latest/plugins/key-auth.md:
##########
@@ -41,6 +41,8 @@ Consumer 端：
| ---- | ------ | ------ | ------------------------------------------------------------------------------------------------------------- |
| key | string | 是 | 不同的 Consumer 应有不同的 `key`，它应当是唯一的。如果多个 Consumer 使用了相同的 `key`，将会出现请求匹配异常。 |
+注意：schema 中还定义了 `encrypt_fields = {"client_secret"}`，这意味着该字段将会被加密存储在 etcd 中。具体参考 [加密存储字段](../plugin-develop.md#加密存储字段)。
Review Comment:
Bad field name?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1045619558
##########
docs/zh/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,8 @@ description: OpenID Connect（OIDC）是基于 OAuth 2.0 的身份认证协议
| session | object | 否 | | | 当设置 bearer_only 为 false 时，openid-connect 插件将使用 Authorization Code 在 IDP 上进行认证，因此你必须设置 session 相关设置。 |
| session.secret | string | 是 | 自动生成 | 16 个以上字符 | 用于 session 加密和 HMAC 计算的密钥。 |
+NOTE: `encrypt_fields = {"client_secret"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
done
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,8 @@ description: OpenID Connect allows the client to obtain user information from th
| session | object | False | | | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
| session.secret | string | True | Automatic generation | 16 or more characters | The key used for session encrypt and HMAC operation. |
+注意：schema 中还定义了 `encrypt_fields = {"client_secret"}`，这意味着该字段将会被加密存储在 etcd 中。具体参考 [加密存储字段](../plugin-develop.md#加密存储字段)。
Review Comment:
done
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046722410
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and not core.table.isempty(schema.encrypt_fields) then
+ for _, key in ipairs(schema.encrypt_fields) do
+ if conf[key] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[key] = encrypted
+ end
+ elseif core.string.find(key, ".") then
+ -- encrypt fields has indents
+ local res, err = re_split(key, "\\.", "jo")
+ if not res then
+ core.log.warn("failed to split key [", key, "], err: ", err)
+ return
+ end
+
+ -- we only support two levels
Review Comment:
I'd like to optimize when the schema must be expressed in more than two levels.
For now, I think two levels are enough(In fact, I hate nesting more than two levels in a schema, it's unreadable.)
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046708488
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and not core.table.isempty(schema.encrypt_fields) then
+ for _, key in ipairs(schema.encrypt_fields) do
+ if conf[key] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[key] = encrypted
+ end
+ elseif core.string.find(key, ".") then
+ -- encrypt fields has indents
+ local res, err = re_split(key, "\\.", "jo")
+ if not res then
+ core.log.warn("failed to split key [", key, "], err: ", err)
+ return
+ end
+
+ -- we only support two levels
Review Comment:
too many limitations, maybe we should optimize this feature later when we have time
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046774463
##########
docs/en/latest/plugins/csrf.md:
##########
@@ -41,6 +41,8 @@ This Plugin considers the `GET`, `HEAD` and `OPTIONS` methods to be safe operati
| expires | number | False | `7200` | Expiration time in seconds of the CSRF cookie. Set to `0` to skip checking expiration time. |
| key | string | True | | Secret key used to encrypt the cookie. |
+NOTE: `encrypt_fields = {"key"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
Is the sentence better `The field will be encrypted and stored in etcd `
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
spacewander merged PR #8487:
URL: https://github.com/apache/apisix/pull/8487
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046720719
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and not core.table.isempty(schema.encrypt_fields) then
+ for _, key in ipairs(schema.encrypt_fields) do
+ if conf[key] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[key] = encrypted
+ end
+ elseif core.string.find(key, ".") then
+ -- encrypt fields has indents
+ local res, err = re_split(key, "\\.", "jo")
+ if not res then
+ core.log.warn("failed to split key [", key, "], err: ", err)
+ return
+ end
+
+ -- we only support two levels
Review Comment:
Why add this limitation, can we parse the conf recursively
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046721982
##########
docs/en/latest/plugins/csrf.md:
##########
@@ -41,6 +41,8 @@ This Plugin considers the `GET`, `HEAD` and `OPTIONS` methods to be safe operati
| expires | number | False | `7200` | Expiration time in seconds of the CSRF cookie. Set to `0` to skip checking expiration time. |
| key | string | True | | Secret key used to encrypt the cookie. |
+NOTE: `encrypt_fields = {"key"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
The field will be encrypted and stored in etcd ?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1048077562
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and not core.table.isempty(schema.encrypt_fields) then
+ for _, key in ipairs(schema.encrypt_fields) do
+ if conf[key] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[key] = encrypted
+ end
+ elseif core.string.find(key, ".") then
+ -- encrypt fields has indents
+ local res, err = re_split(key, "\\.", "jo")
+ if not res then
+ core.log.warn("failed to split key [", key, "], err: ", err)
+ return
+ end
+
+ -- we only support two levels
+ if conf[res[1]] and conf[res[1]][res[2]] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(
+ conf[res[1]][res[2]], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[res[1]][res[2]] = encrypted
+ end
+ end
+ end
end
end
end
_M.encrypt_conf = encrypt_conf
+check_plugin_metadata = function(item)
+ local ok, err = check_single_plugin_schema(item.id, item,
+ core.schema.TYPE_METADATA, true)
+ if ok and enable_gde()then
Review Comment:
```suggestion
if ok and enable_gde() then
```
##########
docs/en/latest/plugins/key-auth.md:
##########
@@ -41,8 +41,6 @@ For Consumer:
|------|--------|-------------|----------------------------|
| key | string | required | Unique key for a Consumer. |
-NOTE: The schema for `key` also defines `encrypted = true`, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
Why just remove it?
##########
docs/zh/latest/plugins/key-auth.md:
##########
@@ -41,8 +41,6 @@ Consumer 端：
| ---- | ------ | ------ | ------------------------------------------------------------------------------------------------------------- |
| key | string | 是 | 不同的 Consumer 应有不同的 `key`，它应当是唯一的。如果多个 Consumer 使用了相同的 `key`，将会出现请求匹配异常。 |
-注意：`key` 的 schema 中还定义了 `encrypted = true`，这意味着该字段将会被加密存储在 etcd 中。具体参考 [加密存储字段](../plugin-develop.md#加密存储字段)。
Review Comment:
Ditto
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046708488
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and not core.table.isempty(schema.encrypt_fields) then
+ for _, key in ipairs(schema.encrypt_fields) do
+ if conf[key] then
+ local encrypted, err = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
+ if not encrypted then
+ core.log.warn("failed to encrypt the conf of plugin [", name,
+ "] key [", key, "], err: ", err)
+ else
+ conf[key] = encrypted
+ end
+ elseif core.string.find(key, ".") then
+ -- encrypt fields has indents
+ local res, err = re_split(key, "\\.", "jo")
+ if not res then
+ core.log.warn("failed to split key [", key, "], err: ", err)
+ return
+ end
+
+ -- we only support two levels
Review Comment:
So many limitations, is it written in doc?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1046874823
##########
docs/en/latest/plugins/csrf.md:
##########
@@ -41,6 +41,8 @@ This Plugin considers the `GET`, `HEAD` and `OPTIONS` methods to be safe operati
| expires | number | False | `7200` | Expiration time in seconds of the CSRF cookie. Set to `0` to skip checking expiration time. |
| key | string | True | | Secret key used to encrypt the cookie. |
+NOTE: `encrypt_fields = {"key"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
I guess there is no difference? For this field, the only encrypted state is in etcd. They are decrypted when APISIX is running and when fetched via the admin API.
I think the way I wrote it better conveys the idea that it is only encrypted in etcd.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #8487:
URL: https://github.com/apache/apisix/pull/8487#issuecomment-1345812353
> It is best to update the documentation of each plugin to indicate the fields that will be encrypted.
done
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1045444109
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and core.table.nkeys(schema.encrypt_fields) > 0 then
Review Comment:
Better to use `core.table.isempty`?
##########
docs/zh/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,8 @@ description: OpenID Connect（OIDC）是基于 OAuth 2.0 的身份认证协议
| session | object | 否 | | | 当设置 bearer_only 为 false 时，openid-connect 插件将使用 Authorization Code 在 IDP 上进行认证，因此你必须设置 session 相关设置。 |
| session.secret | string | 是 | 自动生成 | 16 个以上字符 | 用于 session 加密和 HMAC 计算的密钥。 |
+NOTE: `encrypt_fields = {"client_secret"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).
Review Comment:
Let's use Chinese
##########
docs/en/latest/plugin-develop.md:
##########
@@ -300,13 +300,21 @@ Specify the parameters to be stored encrypted. (Requires APISIX version >= 3.1.0
Some plugins require parameters to be stored encrypted, such as the `password` parameter of the `basic-auth` plugin. This plugin needs to specify in the `schema` which parameters need to be stored encrypted.
```lua
-password = { type = "string", encrypted = true },
+encrypt_fields = {"password"}
```
-Parameters can be stored encrypted by specifying `encrypted = true` in the `schema`. APISIX will provide the following functionality.
+If it is a nested parameter, such as the `clickhouse.password` parameter of the `error-log-logger` plugin, it needs to be separated by `.`:
-- When adding and updating resources via the `Admin API`, APISIX automatically encrypts parameters with `encrypted = true` and stores them in etcd
-- When fetching resources via the `Admin API` and when running the plugin, APISIX automatically decrypts the `encrypted = true` parameter
+```lua
+encrypt_fields = {"clickhouse.password"}
+```
+
+Currently only two levels of nesting are supported.
Review Comment:
Let's make it clear that we don't support fields in array
##########
docs/en/latest/plugins/openid-connect.md:
##########
@@ -61,6 +61,8 @@ description: OpenID Connect allows the client to obtain user information from th
| session | object | False | | | When bearer_only is set to false, openid-connect will use Authorization Code flow to authenticate on the IDP, so you need to set the session-related configuration. |
| session.secret | string | True | Automatic generation | 16 or more characters | The key used for session encrypt and HMAC operation. |
+注意：schema 中还定义了 `encrypt_fields = {"client_secret"}`，这意味着该字段将会被加密存储在 etcd 中。具体参考 [加密存储字段](../plugin-develop.md#加密存储字段)。
Review Comment:
Let's use English
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8487: feat: data encryption support more plugins
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8487:
URL: https://github.com/apache/apisix/pull/8487#discussion_r1045619361
##########
docs/en/latest/plugin-develop.md:
##########
@@ -300,13 +300,21 @@ Specify the parameters to be stored encrypted. (Requires APISIX version >= 3.1.0
Some plugins require parameters to be stored encrypted, such as the `password` parameter of the `basic-auth` plugin. This plugin needs to specify in the `schema` which parameters need to be stored encrypted.
```lua
-password = { type = "string", encrypted = true },
+encrypt_fields = {"password"}
```
-Parameters can be stored encrypted by specifying `encrypted = true` in the `schema`. APISIX will provide the following functionality.
+If it is a nested parameter, such as the `clickhouse.password` parameter of the `error-log-logger` plugin, it needs to be separated by `.`:
-- When adding and updating resources via the `Admin API`, APISIX automatically encrypts parameters with `encrypted = true` and stores them in etcd
-- When fetching resources via the `Admin API` and when running the plugin, APISIX automatically decrypts the `encrypted = true` parameter
+```lua
+encrypt_fields = {"clickhouse.password"}
+```
+
+Currently only two levels of nesting are supported.
Review Comment:
done
##########
apisix/plugin.lua:
##########
@@ -903,19 +926,57 @@ _M.decrypt_conf = decrypt_conf
local function encrypt_conf(name, conf, schema_type)
local schema = get_plugin_schema_for_gde(name, schema_type)
if not schema then
+ core.log.warn("failed to get schema for plugin: ", name)
return
end
- for key, props in pairs(schema.properties) do
- if props.type == "string" and props.encrypted and conf[key] then
- local encrypted = apisix_ssl.aes_encrypt_pkey(conf[key], "data_encrypt")
- conf[key] = encrypted
+ if schema.encrypt_fields and core.table.nkeys(schema.encrypt_fields) > 0 then
Review Comment:
done
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org