You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "nic-6443 (via GitHub)" <gi...@apache.org> on 2026/06/11 09:40:24 UTC
[PR] fix(admin): encrypt_fields get double-encrypted on every PATCH [apisix]
nic-6443 opened a new pull request, #13525:
URL: https://github.com/apache/apisix/pull/13525
### Description
When data encryption (`data_encryption.enable_encrypt_fields`, enabled by default since 3.10.0) is on, every Admin API PATCH request re-encrypts the plugin fields declared in `encrypt_fields`, even though the stored values are already ciphertext. The ciphertext grows with every PATCH, while the data plane decrypts exactly once, so plugins end up running with a corrupted secret. For example, after PATCHing an unrelated field on a route that uses `ai-proxy`, the gateway starts sending a garbled API key upstream and gets 401 responses.
Root cause: the PATCH handler in `apisix/admin/resource.lua` reads the raw resource from etcd (where `encrypt_fields` hold ciphertext), merges the patch into it, and then passes the merged value to `check_conf`, which unconditionally calls `encrypt_conf`. So the already-encrypted fields get wrapped in another encryption layer on every PATCH.
Fix: decrypt the stored plugin conf right after fetching it from etcd and before the merge, reusing the same decryption helpers the GET path uses (`utils.decrypt_params` + `plugin.decrypt_conf`). The merged value is then plaintext, and `check_conf` re-encrypts it exactly once on write. Both the whole-body PATCH and the sub-path PATCH variants are covered.
Affected resource types: all PATCH-able resources whose `check_conf` encrypts plugin fields — routes, services, plugin_configs, global_rules and consumer_groups. Consumers, credentials and plugin_metadata do not support PATCH. Upstream `tls.client_key` is not affected, since its encryption is guarded by a PEM-prefix check and is therefore already idempotent.
#### Which issue(s) this PR fixes:
Fixes #13351
### 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
- [ ] 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)
--
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
Re: [PR] fix(admin): encrypt_fields get double-encrypted on every PATCH [apisix]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13525:
URL: https://github.com/apache/apisix/pull/13525#discussion_r3395420565
##########
apisix/admin/resource.lua:
##########
@@ -421,6 +422,13 @@ function _M:patch(id, conf, sub_path, args)
local node_value = res_old.body.node.value
local modified_index = res_old.body.node.modifiedIndex
+ -- the encrypt_fields of the stored plugin conf are ciphertext, decrypt
+ -- them before merging, otherwise the check_conf below will encrypt them
+ -- again, resulting in fields that are encrypted multiple times
+ if apisix_plugin.enable_gde() then
+ utils.decrypt_params(apisix_plugin.decrypt_conf, res_old.body)
+ end
Review Comment:
Decrypting the stored plugin configs here makes `node_value` plaintext until `check_conf()` re-encrypts it. This changes the security properties of later log statements in this function (e.g. the `core.log.info("new conf: ...")` log), which will now include decrypted secrets from plugins' `encrypt_fields` when log level includes `info`. Please avoid logging the merged plaintext config; move that log after `check_conf()` (so values are encrypted again), remove it, or log a redacted/encrypted copy.
--
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
Re: [PR] fix(admin): encrypt_fields get double-encrypted on every PATCH [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 merged PR #13525:
URL: https://github.com/apache/apisix/pull/13525
--
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
Re: [PR] fix(admin): encrypt_fields get double-encrypted on every PATCH [apisix]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #13525:
URL: https://github.com/apache/apisix/pull/13525#discussion_r3397104201
##########
apisix/admin/resource.lua:
##########
@@ -421,6 +422,13 @@ function _M:patch(id, conf, sub_path, args)
local node_value = res_old.body.node.value
local modified_index = res_old.body.node.modifiedIndex
+ -- the encrypt_fields of the stored plugin conf are ciphertext, decrypt
+ -- them before merging, otherwise the check_conf below will encrypt them
+ -- again, resulting in fields that are encrypted multiple times
+ if apisix_plugin.enable_gde() then
+ utils.decrypt_params(apisix_plugin.decrypt_conf, res_old.body)
+ end
Review Comment:
Good point — moved the "new conf" info log to after check_conf in 308df55c2, so it logs the re-encrypted value.
--
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