(rocketmq) branch develop updated: [ISSUE #8882] Change the compare method for acl signature to improve the security. (#8883)

[jinrongtong]

This is an automated email from the ASF dual-hosted git repository.

jinrongtong pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/rocketmq.git


The following commit(s) were added to refs/heads/develop by this push:
     new 851473443e [ISSUE #8882] Change the compare method for acl signature 
to improve the security. (#8883)
851473443e is described below

commit 851473443e88343c651ac203877330c6cbee3f42
Author: dingshuangxi888 <dingshuangxi...@gmail.com>
AuthorDate: Thu Nov 7 19:04:54 2024 +0800

    [ISSUE #8882] Change the compare method for acl signature to improve the 
security. (#8883)
    
    * Change the compare method for acl signature to improve the security.
    
    * Change the compare method for acl signature to improve the security.
---
 acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java       | 3 +--
 .../java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java   | 5 ++++-
 .../auth/authentication/chain/DefaultAuthenticationHandler.java      | 4 +++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java 
b/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java
index 937619beee..f32acaf2f7 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java
@@ -63,8 +63,7 @@ public class AclUtils {
     }
 
     public static String calSignature(byte[] data, String secretKey) {
-        String signature = AclSigner.calSignature(data, secretKey);
-        return signature;
+        return AclSigner.calSignature(data, secretKey);
     }
 
     public static void IPv6AddressCheck(String netAddress) {
diff --git 
a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java 
b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
index b075e5364e..daedc38f2e 100644
--- 
a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
+++ 
b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionManager.java
@@ -22,6 +22,7 @@ import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.security.MessageDigest;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -35,6 +36,7 @@ import org.apache.commons.lang3.StringUtils;
 import org.apache.rocketmq.acl.PermissionChecker;
 import org.apache.rocketmq.acl.common.AclConstants;
 import org.apache.rocketmq.acl.common.AclException;
+import org.apache.rocketmq.acl.common.AclSigner;
 import org.apache.rocketmq.acl.common.AclUtils;
 import org.apache.rocketmq.acl.common.Permission;
 import org.apache.rocketmq.common.AclConfig;
@@ -618,7 +620,8 @@ public class PlainPermissionManager {
 
         // Check the signature
         String signature = 
AclUtils.calSignature(plainAccessResource.getContent(), 
ownedAccess.getSecretKey());
-        if (!signature.equals(plainAccessResource.getSignature())) {
+        if (plainAccessResource.getSignature() == null
+            || 
!MessageDigest.isEqual(signature.getBytes(AclSigner.DEFAULT_CHARSET), 
plainAccessResource.getSignature().getBytes(AclSigner.DEFAULT_CHARSET))) {
             throw new AclException(String.format("Check signature failed for 
accessKey=%s", plainAccessResource.getAccessKey()));
         }
 
diff --git 
a/auth/src/main/java/org/apache/rocketmq/auth/authentication/chain/DefaultAuthenticationHandler.java
 
b/auth/src/main/java/org/apache/rocketmq/auth/authentication/chain/DefaultAuthenticationHandler.java
index 04f1316450..4b50de756a 100644
--- 
a/auth/src/main/java/org/apache/rocketmq/auth/authentication/chain/DefaultAuthenticationHandler.java
+++ 
b/auth/src/main/java/org/apache/rocketmq/auth/authentication/chain/DefaultAuthenticationHandler.java
@@ -16,6 +16,7 @@
  */
 package org.apache.rocketmq.auth.authentication.chain;
 
+import java.security.MessageDigest;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Supplier;
 import org.apache.commons.lang3.StringUtils;
@@ -62,7 +63,8 @@ public class DefaultAuthenticationHandler implements 
Handler<DefaultAuthenticati
             throw new AuthenticationException("User:{} is disabled.", 
context.getUsername());
         }
         String signature = AclSigner.calSignature(context.getContent(), 
user.getPassword());
-        if (!StringUtils.equals(signature, context.getSignature())) {
+        if (context.getSignature() == null
+            || 
!MessageDigest.isEqual(signature.getBytes(AclSigner.DEFAULT_CHARSET), 
context.getSignature().getBytes(AclSigner.DEFAULT_CHARSET))) {
             throw new AuthenticationException("check signature failed.");
         }
     }