apache / shenyu

Apache ShenYu is a Java native API Gateway for service proxy, protocol conversion and API governance.
https://shenyu.apache.org/
Apache License 2.0
8.44k stars 2.93k forks source link

Optimize the password encryption and password change logic for the admin user #1888

Closed dengliming closed 2 years ago

dengliming commented 3 years ago

Task

Currently, We use the AES algorithm to encrypt the password and provide the password back to the front end. I think It's not safe.

image

I suggest the following optimization:

  1. The API does not return the password field. See org.apache.shenyu.admin.controller.DashboardUserController#detailDashboardUser
  2. Consider Using sha512 algorithm to encrypt the password (Need to discuss)

Please read the Contribution Guideline before submitting the PR

qurenneng commented 3 years ago

I'll take care of this

dengliming commented 3 years ago

@qurenneng Hi~ any progress?

qurenneng commented 3 years ago

Solved in the last two days

At 2021-08-21 10:21:31, "dengliming" @.***> wrote:

@qurenneng Hi~ any progress?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

qurenneng commented 3 years ago

Solved in the last two days

dengliming commented 3 years ago

which pr?

qurenneng commented 3 years ago

the password (Need to discuss) This is what we need to do here

erdengk commented 2 years ago

Hi, @dengliming It seems that no one has contributed to this issue yet. I have done some research and have ideas for improvement. I would like to ask you to review it.

  1. Regarding the first point you mentioned

The API does not return the password field

I looked at your proposed method and detailDashboardUser doesn't return a password field. (Maybe I read the code wrong, please help me)

  1. Consider Using sha512 algorithm to encrypt the password

The password encryption algorithm used in shenyu (v2.4.2) is the AES algorithm, which is an algorithm that supports decryption. If the user's database is leaked, the user name and password may be cracked.

The SHA-512 algorithm is an irreversible algorithm. Compared with the MD5 algorithm, the SHA-512 algorithm achieves that two different data have different hash values. Therefore, the SHA-512 algorithm has a higher level of security.

I noticed that when shenyu changes the password, it does not verify the previous password, and directly overwrites it, so the encrypted password stored in the database does not need to be decrypted.

So I think it's ok to encrypt the password with SHA-512.

If the above thinking is correct. I want to make the following changes to shenyu

  1. Add SHA-512 encryption algorithm tool class
  2. Update the method of creating users. Use SHA-512 for encryption when storing passwords

https://github.com/apache/incubator-shenyu/blob/8f536623c993f0bbbc087a6eb776c66fff1afc5d/shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/DashboardUserController.java#L113-L121

  1. Modify the method of updating users, directly encrypt the new password and store it in the database

https://github.com/apache/incubator-shenyu/blob/8f536623c993f0bbbc087a6eb776c66fff1afc5d/shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/DashboardUserController.java#L132-L141

  1. Delete the previous aes tool class
dengliming commented 2 years ago

2、encrypt the password with SHA-512

We need to discuss about this first. @yu199195

erdengk commented 2 years ago

hi, @dengliming Please check this email.

https://lists.apache.org/thread/dpwllrrpsqt1svyy0f05p20l2kzrmtdh

This is the result of my discussion with him. Can I submit the code as discussed in the email?

dengliming commented 2 years ago

@erdengk Sure. Feel free to go. : )

erdengk commented 2 years ago

Hi,@qurenneng @yu199195 It's not very convenient to add the viewing code to the email, let's discuss it here.

(I searched github and other sites and found no useful solution)

I completed the password encryption task this afternoon, updated functions such as login, password modification, etc. And also ran the application (without any errors), and all functions can complete the task as expected.

but when I use mvn clean install -Dmaven.javadoc.skip=true I always get these three errors.

2022-02-23 01:09:28 [main] WARN  org.apache.shenyu.admin.exception.ExceptionHandlers - http request method not supported
org.springframework.web.HttpRequestMethodNotSupportedException: null
2022-02-23 01:09:28 [main] ERROR org.apache.shenyu.admin.exception.ExceptionHandlers - Test shenyuException message!
org.apache.shenyu.common.exception.ShenyuException: Test shenyuException message!
        at org.apache.shenyu.admin.exception.ExceptionHandlersTest.testServerExceptionHandlerByShenyuException(ExceptionHandlersTest.java:94)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)

[INFO] Running org.apache.shenyu.admin.shiro.config.ShiroRealmTest
2022-02-23 01:09:34 [main] INFO  org.apache.shenyu.admin.utils.JwtUtils - jwt decode fail, token: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwidXNlck5hbWUiOiJKb2huIERvZSIsImlhdCI6MTUxNjIzOTAyMn0.vZiLpzbncmNC5KL1idgfapCOTsRC0h_5XnqxaGfcLlM 
com.auth0.jwt.exceptions.SignatureVerificationException: The Token's Signature resulted invalid when verified using the Algorithm: HmacSHA256
        at com.auth0.jwt.algorithms.HMACAlgorithm.verify(HMACAlgorithm.java:56)
        at com.auth0.jwt.JWTVerifier.verify(JWTVerifier.java:287)
        at com.auth0.jwt.JWTVerifier.verify(JWTVerifier.java:271)
        at org.apache.shenyu.admin.utils.JwtUtils.verifyToken(JwtUtils.java:98)
        at org.apache.shenyu.admin.shiro.config.ShiroRealm.doGetAuthenticationInfo(ShiroRealm.java:91)
        at org.apache.shenyu.admin.shiro.config.ShiroRealmTest.lambda$testDoGetAuthenticationInfo$2(ShiroRealmTest.java:121)
        at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:55)

So I undo all the changes and re-use mvn clean install -Dmaven.javadoc.skip=true

maven returns BUILD SUCCESS

I started to go through it step by step, trying to figure out what was wrong. Each time an operation is performed, the following command is used to see if the current operation has an effect. mvn clean install -pl shenyu-admin -Dmaven.javadoc.skip=true

In the last email, you mentioned that you want to overwrite sql, when I overwrite the original encrypted password with the encrypted password, the above error occurred when building shenyu-admin.

So I did three tests

  1. No modification at all, use directly mvn clean install -pl shenyu-admin -Dmaven.javadoc.skip=true

maven returns success

  1. The new encrypted password overwrites the previous password, return build failed
    
    before

INSERT IGNORE INTO dashboard_user (id, user_name, password, role, enabled, date_created, date_updated) VALUES ('1','admin','bbiB8zbUo3z3oA0VqEB/IA==', '1', '1', '2018-06-23 15:12:22', '2018-06-23 15:12:23');;

new encrypted password

ba3253876aed6bc22d4a6ff53d8406c6ad864195ed144ab5c87621b6c233b548baeae6956df346ec8c17f5ea10f35ee3cbc514797ed7ddd3145464e2a0bab413

INSERT IGNORE INTO dashboard_user (id, user_name, password, role, enabled, date_created, date_updated) VALUES ('1','admin', 'ba3253876aed6bc22d4a6ff53d8406c6ad864195ed144ab5c87621b6c233b548baeae6956df346ec8c17f5ea10f35ee3cbc514797ed7ddd3145464e2a0bab413', '1', '1', '2018-06-23 15:12:22', '2018-06-23 15:12:23');


**I guess it should be the modification of the initial encryption password, which caused problems with other configurations, but I did not find any code in shiro and jwt that might be affected by this. (Maybe my technology is not good enough, I couldn't find the corresponding code, please help me)**

3. The encryption password is not changed, only a new java class is added. (not used anywhere, just added to)

Still returns the three errors above

```java
2022-02-23 01:14:46 [main] ERROR org.apache.shenyu.admin.exception.ExceptionHandlers - unauthorized exception
org.apache.shiro.authz.UnauthorizedException: null
2022-02-23 01:14:47 [main] ERROR org.apache.shenyu.admin.exception.ExceptionHandlers - duplicate key exception 
org.springframework.dao.DuplicateKeyException: Test duplicateKeyException message!
        at org.apache.shenyu.admin.exception.ExceptionHandlersTest.testServerExceptionHandlerByDuplicateKeyException(ExceptionHandlersTest.java:102)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
        at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:125)
        at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:132)

2022-02-23 01:14:53 [main] ERROR org.apache.shenyu.admin.exception.ExceptionHandlers - null
java.lang.Exception: null
        at org.apache.shenyu.admin.exception.ExceptionHandlersTest.testServerExceptionHandlerByException(ExceptionHandlersTest.java:86)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
        at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)

[INFO] Rat check: Summary over all files. Unapproved: 1, unknown: 1, generated: 0, approved: 414 licenses.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  03:46 min
[INFO] Finished at: 2022-02-23T01:15:17+08:00

I have very little understanding of shenyu. I understand why this problem is caused by just adding an unused java file, please help me improve this function.

(I searched github and other sites and found no useful solution)

I completed the password encryption task this afternoon, updated functions such as login, password modification, etc., and also ran the application (without any errors), and all functions can complete the task as expected.

but when I use mvn clean install -Dmaven.javadoc.skip=true When testing, I always get these three errors.

So I undo all the changes and re-use mvn clean install -Dmaven.javadoc.skip=true

maven returns BUILD SUCCESS

I started to go through it step by step, trying to figure out what was wrong. Each time an operation is performed, the following command is used to see if the current operation has an effect. mvn clean install -pl shenyu-admin -Dmaven.javadoc.skip=true

In the last email, you mentioned that you want to overwrite sql, when I overwrite the original encrypted password with the encrypted password, the above error occurred when building shenyu-admin.

So I did three tests

  1. No modification at all, use directly mvn clean install -pl shenyu-admin -Dmaven.javadoc.skip=true maven returns success

  2. The new encrypted password overwrites the previous password, return build failed

I guess it should be the modification of the initial encryption password, which caused problems with other configurations, but I did not find any code in shiro and jwt that might be affected by this. (Maybe my technology is not good enough, I couldn't find the corresponding code, please help me)

  1. The encryption password is not changed, only a new encryption method tool class is added. (not referenced anywhere, just added to)

image

Running org.apache.shenyu.admin.exception.ExceptionHandlersTest
2022-02-23 01:38:06 [main] ERROR org.apache.shenyu.admin.exception.ExceptionHandlers - unauthorized exception
org.apache.shiro.authz.UnauthorizedException: null
2022-02-23 01:38:06 [main] ERROR org.apache.shenyu.admin.exception.ExceptionHandlers - duplicate key exception 
org.springframework.dao.DuplicateKeyException: Test duplicateKeyException message!
        at org.apache.shenyu.admin.exception.ExceptionHandlersTest.testServerExceptionHandlerByDuplicateKeyException(ExceptionHandlersTest.java:102)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
        at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:125)
        at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:132)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:124)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:74)
        at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
        at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
 Running org.apache.shenyu.admin.shiro.config.ShiroRealmTest
2022-02-23 01:38:19 [main] INFO  org.apache.shenyu.admin.utils.JwtUtils - jwt decode fail, token: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwidXNlck5hbWUiOiJKb2huIERvZSIsImlhdCI6MTUxNjIzOTAyMn0.vZiLpzbncmNC5KL1idgfapCOTsRC0h_5XnqxaGfcLlM 
com.auth0.jwt.exceptions.SignatureVerificationException: The Token's Signature resulted invalid when verified using the Algorithm: HmacSHA256
        at com.auth0.jwt.algorithms.HMACAlgorithm.verify(HMACAlgorithm.java:56)

Still returns the three errors above

When I delete the added files above and revert to a version where nothing has changed, mvn clean install -pl shenyu-admin -Dmaven.javadoc.skip=true maven returns success

My computer is a mac with M1 chip, shenyu (v2.4.2)

I have very little understanding of shenyu. I understand why this problem is caused by just adding an unused java file, please help me sovle this function.

Thank you for reading my long comment.😊

HessTina-YuI commented 2 years ago

The new class is lack of apache protocol.

erdengk commented 2 years ago

After testing, the problem is that the apache protocol is not added.

After adding, the above errors will appear. But maven builds successfully

2022-02-23 02:13:23 [main] ERROR org.apache.shenyu.admin.exception.ExceptionHandlers - null pointer exception 
java.lang.NullPointerException: null
[INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.718 s - in org.apache.shenyu.admin.exception.ExceptionHandlersTest
[INFO] Running org.apache.shenyu.admin.spring.ShenyuApplicationContextAwareTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.07 s - in org.apache.shenyu.admin.spring.ShenyuApplicationContextAwareTest
[INFO] Running org.apache.shenyu.admin.spring.SpringBeanUtilsTest
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.042 s - in org.apache.shenyu.admin.spring.SpringBeanUtilsTest
[INFO] Running org.apache.shenyu.admin.shiro.config.ShiroRealmTest
2022-02-23 02:13:29 [main] INFO  org.apache.shenyu.admin.utils.JwtUtils - jwt decode fail, token: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwidXNlck5hbWUiOiJKb2huIERvZSIsImlhdCI6MTUxNjIzOTAyMn0.vZiLpzbncmNC5KL1idgfapCOTsRC0h_5XnqxaGfcLlM 
com.auth0.jwt.exceptions.SignatureVerificationException: The Token's Signature resulted invalid when verified using the Algorithm: HmacSHA256
        at com.auth0.jwt.algorithms.HMACAlgorithm.verify(HMACAlgorithm.java:56)
        at com.auth0.jwt.JWTVerifier.verify(JWTVerifier.java:287)
        at com.auth0.jwt.JWTVerifier.verify(JWTVerifier.java:271)
        at org.apache.shenyu.admin.utils.JwtUtils.verifyToken(JwtUtils.java:98)
        at org.apache.shenyu.admin.shiro.config.ShiroRealm.doGetAuthenticationInfo(ShiroRealm.java:91)
        at org.apache.shenyu.admin.shiro.config.ShiroRealmTest.lambda$testDoGetAuthenticationInfo$2(ShiroRealmTest.java:121)
        at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:55)
        at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:37)

image