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

Configs V1 API is faulty when using encryption-plugin #12046

Open
xyohn opened this issue Apr 29, 2024 · 13 comments
Open

Configs V1 API is faulty when using encryption-plugin #12046

xyohn opened this issue Apr 29, 2024 · 13 comments

Comments

@xyohn
Copy link
Contributor

xyohn commented Apr 29, 2024

Describe the bug
I have customized an encryption method through encryption SPI. When I create a config on console at first time, I found it work well, but getting error when I update it.
By debugging the code, I find there are some bugs in com.alibaba.nacos.config.server.controller.ConfigController#publishConfig

image

when I add a config on console, the request body do not contain encryptedDataKey field, so the method will call EncryptionHandler.encryptHandler(dataId, content) to encrypt data ,then it will obtain the encrypted data and encryptedDataKey. it can work well.
image
image

However, when I try to update this config, the console will send encryptedDataKey field, and the method only assign encryptedDataKey to encryptedDataKeyFinal. In the following logic, data which is unencrypted and the encryptedDataKey used in previous will be used in com.alibaba.nacos.config.server.service.ConfigOperationService#publishConfig.because of it, the data is unencrypted when it is stored in database as well as published to the client, and there may be causing exception if the client decrypt the unencrypted data by encryptedDataKey.
image

This problem can also be repeated by using official plugin(https://github.com/nacos-group/nacos-plugin/tree/develop/nacos-encryption-plugin-ext), which cause DecoderException. (Although it does not affect the actual business because of catching, I think it is a problem because the encryption and decryption function is disabled, which is not in line with expectations)

image

By contrast, v2 API(com.alibaba.nacos.config.server.controller.v2.ConfigControllerV2#publishConfig) do not have this problem because the data will be encrypted at any time. However, the console UI uses the v1 API.

BTW, I try to fix this bug by removing the if condition, but I found it was added on 26 May,2023(Revision Number 505c90b), which was about #10533 ,and the commit message did not explain why change it in this way.

So, is it a bug? If so, what is the community's opinion on how to solve this issue?

Thanks.

Expected behavior
A clear and concise description of what you expected to happen.

Whether I am creating or updating, the encryption plugin should work correctly.

Actually behavior
A clear and concise description of what you actually to happen.

It worked well when creating the config, but it didn't seem to work properly when updating it

How to Reproduce
Steps to reproduce the behavior:

  1. add a encryption plugin correctly
  2. create a config and it works well (encrypted data in database and the client receives encrypted data with its encryptedDataKey)
  3. update the config, then the data in database is unencrypted and the client receives unencrypted data with previous encryptedDataKey)

Desktop (please complete the following information):

  • OS: [e.g. Centos] Linux
  • Version [e.g. nacos-server 1.3.1, nacos-client 1.3.1] nacos-server 2.3.2,nacos-client 2.3.2,also be found in 2.4.0-SNAPSHOT
  • Module [e.g. naming/config] config
  • SDK [e.g. original, spring-cloud-alibaba-nacos, dubbo]

Additional context
Add any other context about the problem here.

@Daydreamer-ia
Copy link
Contributor

看了一下,发现在逻辑上确实存在点问题(在存在加密插件情况下):
1、v1版本的发布配置接口,接收前端的 encryptedDataKey,但是在存储时却是明文
2、v2版本的发布配置接口,不接受前端传 encryptedDataKey,配置使用的密钥完全在 server 上闭环,存储的时候是存密文
3、sdk的发布配置接口,接收端上传递的 encryptedDataKey,同v2版本接口存储也是密文 (区别在于网络传输也是密文,v2是明文)

问题貌似主要在v1版本接口,看下v1版本接口要不要和v2版本接口对齐,都不接受前端传递的 encryptedDataKey,而是去插件里获取,这样的话,就统一了,问 @KomachiSion

@xyohn
Copy link
Contributor Author

xyohn commented May 4, 2024

看了一下,发现在逻辑上确实存在点问题(在存在加密插件情况下): 1、v1版本的发布配置接口,接收前端的 encryptedDataKey,但是在存储时却是明文 2、v2版本的发布配置接口,不接受前端传 encryptedDataKey,配置使用的密钥完全在 server 上闭环,存储的时候是存密文 3、sdk的发布配置接口,接收端上传递的 encryptedDataKey,同v2版本接口存储也是密文 (区别在于网络传输也是密文,v2是明文)

问题貌似主要在v1版本接口,看下v1版本接口要不要和v2版本接口对齐,都不接受前端传递的 encryptedDataKey,而是去插件里获取,这样的话,就统一了,问 @KomachiSion

Yeah, and I would also like to make some additions about the current strategies for these three situations:

  1. For the v1 api, for data without an encryptedDataKey field, use EncryptionHandler.encryptHandler(dataId, content) to complete the encryption process on the data, and obtain the encrypted content and corresponding encryptedDataKey. For data with an encryptedDataKey field, directly use the content and encryptedDataKey input parameters.
  2. For the v2 api, it uses EncryptionHandler.encryptHandler(dataId, content) to complete the encryption process on the data anyway, and obtains the encrypted content and the corresponding encryptedDataKey.
  3. sdk (via grpc), no encryption processing is performed, and the input parameters content and encryptedDataKey (optional) are used directly

It is possible that the strategies are different due to different versions and services, but currently, there is a problem with the cooperation between console-ui and v1 API, which is this issue.


是的,我想再具体一下你说的这三个情况目前的策略:

  1. v1的api,对于没有encryptedDataKey字段的数据,对数据使用EncryptionHandler.encryptHandler(dataId, content)完成加密处理,得到加密了的content和对应的encryptedDataKey。对于有encryptedDataKey字段的数据,直接使用入参的contentencryptedDataKey
  2. v2的api,无论如何都对数据使用EncryptionHandler.encryptHandler(dataId, content)完成加密处理,得到加密了的content和对应的encryptedDataKey
  3. sdk(grpc),不进行处理,直接使用入参的contentencryptedDataKey(可选的)。

有可能因为版本以及面向的服务不同,策略有所不同,但目前看,console-ui和v1 API的配合上,是有问题的。

@shiyiyue1102
Copy link
Collaborator

这里v1 v2版本的后端处理接口不一致,是一个问题。
从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。
如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey
如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

@xyohn
Copy link
Contributor Author

xyohn commented May 6, 2024

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

Thanks for your answer.According to your reply, can I understand it this way:

  1. V2 API is not the latest. It should be consistent with v1 (if there has encryptedDataKey field, directly use the content and encryptedDataKey, no longer call EncryptionHandler.encryptHandler(dataId, content))
  2. console-ui should not offer the encryptedDataKey field during config updates

感谢,按照这个说法,是否可以这么理解:

  1. v2的接口不是最新的,其逻辑应该与v1的保持一致(如果存在encryptedDataKey,不再调用EncryptionHandler.encryptHandler(dataId, content)执行加密逻辑)
  2. console-ui在进行配置更新时,不应该传输encryptedDataKey字段

@shiyiyue1102
Copy link
Collaborator

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

Thanks for your answer.According to your reply, can I understand it this way:

  1. V2 API is not the latest. It should be consistent with v1 (if there has encryptedDataKey field, directly use the content and encryptedDataKey, no longer call EncryptionHandler.encryptHandler(dataId, content))
  2. console-ui should not offer the encryptedDataKey field during config updates

感谢,按照这个说法,是否可以这么理解:

  1. v2的接口不是最新的,其逻辑应该与v1的保持一致(如果存在encryptedDataKey,不再调用EncryptionHandler.encryptHandler(dataId, content)执行加密逻辑)
  2. console-ui在进行配置更新时,不应该传输encryptedDataKey字段

是应该这样处理,每个接口当前上层调用的入口不一致,所以逻辑没有统一,是需要统一下,我翻了下开源的console-ui的配置更新前端代码,好像没有看到前端会传encryptedDataKey 这个字段,你是修改了前端的代码么

@xyohn
Copy link
Contributor Author

xyohn commented May 6, 2024

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

Thanks for your answer.According to your reply, can I understand it this way:

  1. V2 API is not the latest. It should be consistent with v1 (if there has encryptedDataKey field, directly use the content and encryptedDataKey, no longer call EncryptionHandler.encryptHandler(dataId, content))
  2. console-ui should not offer the encryptedDataKey field during config updates

感谢,按照这个说法,是否可以这么理解:

  1. v2的接口不是最新的,其逻辑应该与v1的保持一致(如果存在encryptedDataKey,不再调用EncryptionHandler.encryptHandler(dataId, content)执行加密逻辑)
  2. console-ui在进行配置更新时,不应该传输encryptedDataKey字段

是应该这样处理,每个接口当前上层调用的入口不一致,所以逻辑没有统一,是需要统一下,我翻了下开源的console-ui的配置更新前端代码,好像没有看到前端会传encryptedDataKey 这个字段,你是修改了前端的代码么

No, I just used the official Docker image


没有哦,我用的官方的Docker镜像

@xyohn
Copy link
Contributor Author

xyohn commented May 6, 2024

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

Thanks for your answer.According to your reply, can I understand it this way:

  1. V2 API is not the latest. It should be consistent with v1 (if there has encryptedDataKey field, directly use the content and encryptedDataKey, no longer call EncryptionHandler.encryptHandler(dataId, content))
  2. console-ui should not offer the encryptedDataKey field during config updates

感谢,按照这个说法,是否可以这么理解:

  1. v2的接口不是最新的,其逻辑应该与v1的保持一致(如果存在encryptedDataKey,不再调用EncryptionHandler.encryptHandler(dataId, content)执行加密逻辑)
  2. console-ui在进行配置更新时,不应该传输encryptedDataKey字段

是应该这样处理,每个接口当前上层调用的入口不一致,所以逻辑没有统一,是需要统一下,我翻了下开源的console-ui的配置更新前端代码,好像没有看到前端会传encryptedDataKey 这个字段,你是修改了前端的代码么

May be because the API(show=all) for getting the configuration returns encryptedDataKey field, so it is also sent back by console-ui when updating.
770d17b9-8475-49c0-94c8-f037634515ba
97cc16d9-d356-49f5-b333-f963e83bb7fe

image

I tested it and found that even if I did not turn on encryption, console-ui would also provide the encryptedDataKey field (an empty string)


可能是因为获取配置的接口(show=all)返回了encryptedDataKey字段,所以更新的时候一并传回去了。
我测试了一下,即便我没有开启加密,console-ui也会提供encryptedDataKey字段(为空字符串)

@shiyiyue1102
Copy link
Collaborator

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

Thanks for your answer.According to your reply, can I understand it this way:

  1. V2 API is not the latest. It should be consistent with v1 (if there has encryptedDataKey field, directly use the content and encryptedDataKey, no longer call EncryptionHandler.encryptHandler(dataId, content))
  2. console-ui should not offer the encryptedDataKey field during config updates

感谢,按照这个说法,是否可以这么理解:

  1. v2的接口不是最新的,其逻辑应该与v1的保持一致(如果存在encryptedDataKey,不再调用EncryptionHandler.encryptHandler(dataId, content)执行加密逻辑)
  2. console-ui在进行配置更新时,不应该传输encryptedDataKey字段

是应该这样处理,每个接口当前上层调用的入口不一致,所以逻辑没有统一,是需要统一下,我翻了下开源的console-ui的配置更新前端代码,好像没有看到前端会传encryptedDataKey 这个字段,你是修改了前端的代码么

May be because the API(show=all) for getting the configuration returns encryptedDataKey field, so it is also sent back by console-ui when updating. 770d17b9-8475-49c0-94c8-f037634515ba 97cc16d9-d356-49f5-b333-f963e83bb7fe

image

I tested it and found that even if I did not turn on encryption, console-ui would also provide the encryptedDataKey field (an empty string)

可能是因为获取配置的接口(show=all)返回了encryptedDataKey字段,所以更新的时候一并传回去了。 我测试了一下,即便我没有开启加密,console-ui也会提供encryptedDataKey字段(为空字符串)

那这里是自动把前面的接口里的数据都原封不动地传入配置发布接口里了,那前端做下处理,把这个key给过滤掉就好了

@Daydreamer-ia
Copy link
Contributor

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

这里有个问题,目前控制台传递的 content 是明文的,只有 sdk 传递的会是密文。而且针对前端,是不知道配置内容和密钥是用什么加解密的,没办法加密 content

@xyohn
Copy link
Contributor Author

xyohn commented May 6, 2024

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

Thanks for your answer.According to your reply, can I understand it this way:

  1. V2 API is not the latest. It should be consistent with v1 (if there has encryptedDataKey field, directly use the content and encryptedDataKey, no longer call EncryptionHandler.encryptHandler(dataId, content))
  2. console-ui should not offer the encryptedDataKey field during config updates

感谢,按照这个说法,是否可以这么理解:

  1. v2的接口不是最新的,其逻辑应该与v1的保持一致(如果存在encryptedDataKey,不再调用EncryptionHandler.encryptHandler(dataId, content)执行加密逻辑)
  2. console-ui在进行配置更新时,不应该传输encryptedDataKey字段

是应该这样处理,每个接口当前上层调用的入口不一致,所以逻辑没有统一,是需要统一下,我翻了下开源的console-ui的配置更新前端代码,好像没有看到前端会传encryptedDataKey 这个字段,你是修改了前端的代码么

May be because the API(show=all) for getting the configuration returns encryptedDataKey field, so it is also sent back by console-ui when updating. 770d17b9-8475-49c0-94c8-f037634515ba 97cc16d9-d356-49f5-b333-f963e83bb7fe
image
I tested it and found that even if I did not turn on encryption, console-ui would also provide the encryptedDataKey field (an empty string)
可能是因为获取配置的接口(show=all)返回了encryptedDataKey字段,所以更新的时候一并传回去了。 我测试了一下,即便我没有开启加密,console-ui也会提供encryptedDataKey字段(为空字符串)

那这里是自动把前面的接口里的数据都原封不动地传入配置发布接口里了,那前端做下处理,把这个key给过滤掉就好了

Can I create a pull request to fix it ?

@xyohn
Copy link
Contributor Author

xyohn commented May 6, 2024

这里v1 v2版本的后端处理接口不一致,是一个问题。 从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。 如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey 如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey

这里有个问题,目前控制台传递的 content 是明文的,只有 sdk 传递的会是密文。而且针对前端,是不知道配置内容和密钥是用什么加解密的,没办法加密 content

In his opinion,what I understand is that console-ui do not need to send the content which is encrypted,and the encryptedDataKey need to be empty all the time.


我理解他的意见是 console-ui 传递的encryptedDataKey 需要为空,content为明文,不需要考虑加解密的事情。

@Daydreamer-ia
Copy link
Contributor

理解了。那你能提个pr?做两个事情:

  1. @shiyiyue1102 说的处理方式,把v2接口和v1接口同步一下
从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。
如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey
如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey
  1. console-ui上把 encryptedDataKey 过滤掉
  2. 单测

@xyohn
Copy link
Contributor Author

xyohn commented May 6, 2024

理解了。那你能提个pr?做两个事情:

  1. @shiyiyue1102 说的处理方式,把v2接口和v1接口同步一下
从后端接口的角度应该这样处理,发布接口接收content和encryptedDataKey。
如果encryptedDataKey 不为空,表示接口已经对明文进行过加密,则直接存储content(此时认为参数中的content已是密文)和encryptedDataKey
如果encryptedDataKey为空,则通过加解密插件对content(此时认为参数中的content为明文,根据插件中自定义规则判断是都需要加密)进行加密获得 加密后的content和encryptedDataKey
  1. console-ui上把 encryptedDataKey 过滤掉
  2. 单测

OK,I will resolve it. :-)

xyohn added a commit to xyohn/nacos that referenced this issue May 6, 2024
shiyiyue1102 pushed a commit that referenced this issue May 7, 2024
* #12046 console-ui should not offer encryptedDataKey field to API

* #12046 when containing encryptedDataKey,it should not execute encryption in v2 API (same as the v1 API)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants