Stop exposing login AES secrets to anonymous callers

shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DashboardUserServiceImpl.java

@@ -52,9 +52,9 @@
 import org.apache.shenyu.admin.utils.WebI18nAssert;
 import org.apache.shenyu.common.constant.AdminConstants;
 import org.apache.shenyu.common.constant.Constants;
-import org.apache.shenyu.common.utils.AesUtils;
 import org.apache.shenyu.common.utils.DigestUtils;
 import org.apache.shenyu.common.utils.ListUtil;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.ldap.NameNotFoundException;
@@ -63,6 +63,13 @@
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.IvParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+import java.nio.charset.StandardCharsets;
+import java.security.Security;
+import java.util.Base64;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
@@ -77,6 +84,8 @@ public class DashboardUserServiceImpl implements DashboardUserService {
 
     private static final Logger LOG = LoggerFactory.getLogger(DashboardUserServiceImpl.class);
 
+    private static final int AES_BLOCK_SIZE = 16;
+
     private final DashboardUserMapper dashboardUserMapper;
 
     private final UserRoleMapper userRoleMapper;
@@ -279,12 +288,7 @@ public CommonPager<DashboardUserVO> listByPage(final DashboardUserQuery dashboar
     @Override
     public LoginDashboardUserVO login(final String userName, final String password, final String clientId) {
         DashboardUserVO dashboardUserVO = null;
-        final String cbcDecryptPassword;
-        if (StringUtils.isNotBlank(secretProperties.getKey()) && StringUtils.isNotBlank(secretProperties.getIv())) {
-            cbcDecryptPassword = AesUtils.cbcDecrypt(secretProperties.getKey(), secretProperties.getIv(), password);
-        } else {
-            cbcDecryptPassword = password;
-        }
+        final String cbcDecryptPassword = resolveLoginPassword(password);
 
         if (Objects.nonNull(ldapTemplate)) {
             dashboardUserVO = loginByLdap(userName, cbcDecryptPassword);
@@ -313,6 +317,43 @@ public LoginDashboardUserVO login(final String userName, final String password,
                 .orElse(null);
     }
 
+    private String resolveLoginPassword(final String password) {
+        if (StringUtils.isNotBlank(secretProperties.getKey()) && StringUtils.isNotBlank(secretProperties.getIv())
+                && isPotentialEncryptedPassword(password)) {
+            return tryDecryptPassword(password).orElse(password);
+        }
+        return password;
+    }
+
+    private boolean isPotentialEncryptedPassword(final String password) {
+        if (StringUtils.isBlank(password)) {
+            return false;
+        }
+        try {
+            byte[] decoded = Base64.getDecoder().decode(password);
+            return decoded.length > 0 && decoded.length % AES_BLOCK_SIZE == 0;
+        } catch (IllegalArgumentException ignored) {
+            return false;
+        }
+    }
+
+    private Optional<String> tryDecryptPassword(final String password) {
+        Security.addProvider(new BouncyCastleProvider());
+        byte[] secretKeyBytes = secretProperties.getKey().getBytes(StandardCharsets.UTF_8);
+        byte[] ivBytes = secretProperties.getIv().getBytes(StandardCharsets.UTF_8);
+        try {
+            SecretKey secretKey = new SecretKeySpec(secretKeyBytes, "AES");
+            Cipher cipher = Cipher.getInstance("AES/CBC/Pkcs7Padding");
+            IvParameterSpec ivParameterSpec = new IvParameterSpec(ivBytes);
+            cipher.init(Cipher.DECRYPT_MODE, secretKey, ivParameterSpec);
+            byte[] decryptedBytes = cipher.doFinal(Base64.getDecoder().decode(password));
+            return Optional.of(new String(decryptedBytes, StandardCharsets.UTF_8));
+        } catch (Exception e) {
+            LOG.debug("AES login password decrypt failed, falling back to plain text password", e);
+            return Optional.empty();
+        }
+    }
+
     /**
      * modify password.
      *

shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SecretServiceImpl.java

@@ -17,6 +17,7 @@
 
 package org.apache.shenyu.admin.service.impl;
 
+import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import org.apache.shenyu.admin.config.properties.SecretProperties;
 import org.apache.shenyu.admin.service.SecretService;
@@ -29,14 +30,13 @@
 @Service
 public class SecretServiceImpl implements SecretService {
 
-    private final SecretProperties secretProperties;
-
-    public SecretServiceImpl(final SecretProperties secretProperties) {
-        this.secretProperties = secretProperties;
-    }
+    private static final String SANITIZED_SECRET_VALUE = "";
 
     @Override
     public String info() {
-        return Base64.getEncoder().encodeToString(JsonUtils.toJson(secretProperties).getBytes());
+        SecretProperties sanitized = new SecretProperties();
+        sanitized.setKey(SANITIZED_SECRET_VALUE);
+        sanitized.setIv(SANITIZED_SECRET_VALUE);
+        return Base64.getEncoder().encodeToString(JsonUtils.toJson(sanitized).getBytes(StandardCharsets.UTF_8));
     }
 }

shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/PlatformControllerTest.java

@@ -52,6 +52,14 @@
 @ExtendWith(MockitoExtension.class)
 public final class PlatformControllerTest {
 
+    private static final String TEST_LOGIN_USER_NAME = "admin";
+
+    private static final String TEST_LOGIN_PASSWORD = "123456";
+
+    private static final String TEST_USER_ID = "1";
+
+    private static final String TEST_STORED_PASSWORD = "2095132720951327";
+
     private MockMvc mockMvc;
 
     @InjectMocks
@@ -69,8 +77,8 @@ public final class PlatformControllerTest {
     /**
      * dashboardUser mock data.
      */
-    private final DashboardUserVO dashboardUserVO = new DashboardUserVO("1", "admin", "2095132720951327",
-            1, true, "1", DateUtils.localDateTimeToString(LocalDateTime.now()),
+    private final DashboardUserVO dashboardUserVO = new DashboardUserVO(TEST_USER_ID, TEST_LOGIN_USER_NAME, TEST_STORED_PASSWORD,
+            1, true, TEST_USER_ID, DateUtils.localDateTimeToString(LocalDateTime.now()),
             DateUtils.localDateTimeToString(LocalDateTime.now()));
 
     /**
@@ -86,10 +94,10 @@ public void setUp() {
      */
     @Test
     public void testLoginDashboardUser() throws Exception {
-        final String loginUri = "/platform/login?userName=admin&password=123456";
+        final String loginUri = "/platform/login?userName=" + TEST_LOGIN_USER_NAME + "&password=" + TEST_LOGIN_PASSWORD;
 
         LoginDashboardUserVO loginDashboardUserVO = LoginDashboardUserVO.buildLoginDashboardUserVO(dashboardUserVO);
-        given(this.dashboardUserService.login(eq("admin"), eq("123456"), isNull())).willReturn(loginDashboardUserVO);
+        given(this.dashboardUserService.login(eq(TEST_LOGIN_USER_NAME), eq(TEST_LOGIN_PASSWORD), isNull())).willReturn(loginDashboardUserVO);
         this.mockMvc.perform(MockMvcRequestBuilders.request(HttpMethod.GET, loginUri))
                 .andExpect(status().isOk())
                 .andExpect(jsonPath("$.code", is(CommonErrorCode.SUCCESSFUL)))

shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DashboardUserServiceTest.java

@@ -77,6 +77,10 @@ public final class DashboardUserServiceTest {
 
     public static final String TEST_PASSWORD = "password";
 
+    private static final String TEST_AES_KEY = "2095132720951327";
+
+    private static final String TEST_AES_IV = "6075877187097700";
+
     @InjectMocks
     private DashboardUserServiceImpl dashboardUserService;
 
@@ -223,15 +227,19 @@ public void testLogin() {
 
         // test loginByDatabase AES password
         SecretProperties secretPropertiesTmp = new SecretProperties();
-        secretPropertiesTmp.setKey("2095132720951327");
-        secretPropertiesTmp.setIv("6075877187097700");
+        secretPropertiesTmp.setKey(TEST_AES_KEY);
+        secretPropertiesTmp.setIv(TEST_AES_IV);
         ReflectionTestUtils.setField(dashboardUserService, "secretProperties", secretPropertiesTmp);
         ReflectionTestUtils.setField(dashboardUserService, "ldapTemplate", null);
-        assertLoginSuccessful(dashboardUserDO, dashboardUserService.login(TEST_USER_NAME, AesUtils.cbcEncrypt("2095132720951327", "6075877187097700", TEST_PASSWORD), null));
+        assertLoginSuccessful(dashboardUserDO, dashboardUserService.login(TEST_USER_NAME, AesUtils.cbcEncrypt(TEST_AES_KEY, TEST_AES_IV, TEST_PASSWORD), null));
         verify(dashboardUserMapper, times(3)).findByQuery(eq(TEST_USER_NAME), anyString());
-        assertLoginSuccessful(dashboardUserDO, dashboardUserService.login(TEST_USER_NAME, AesUtils.cbcEncrypt("2095132720951327", "6075877187097700", TEST_PASSWORD), null));
+        assertLoginSuccessful(dashboardUserDO, dashboardUserService.login(TEST_USER_NAME, AesUtils.cbcEncrypt(TEST_AES_KEY, TEST_AES_IV, TEST_PASSWORD), null));
         verify(dashboardUserMapper, times(4)).findByQuery(eq(TEST_USER_NAME), anyString());
 
+        // test loginByDatabase plain password fallback when secret endpoint does not provide key material
+        assertLoginSuccessful(dashboardUserDO, dashboardUserService.login(TEST_USER_NAME, TEST_PASSWORD, null));
+        verify(dashboardUserMapper, times(5)).findByQuery(eq(TEST_USER_NAME), anyString());
+
     }
 
     private DashboardUserDO createDashboardUserDO() {

shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SecretServiceTest.java

@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shenyu.admin.service;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.shenyu.admin.config.properties.SecretProperties;
+import org.apache.shenyu.admin.service.impl.SecretServiceImpl;
+import org.junit.jupiter.api.Test;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+/**
+ * Test for {@link SecretServiceImpl}.
+ */
+public class SecretServiceTest {
+
+    private static final String TEST_AES_KEY = "2095132720951327";
+
+    private static final String TEST_AES_IV = "6075877187097700";
+
+    @Test
+    public void infoShouldNotExposeConfiguredSecrets() throws Exception {
+        SecretProperties secretProperties = new SecretProperties();
+        secretProperties.setKey(TEST_AES_KEY);
+        secretProperties.setIv(TEST_AES_IV);
+
+        SecretService secretService = new SecretServiceImpl();
+        String encoded = secretService.info();
+        String decoded = new String(Base64.getDecoder().decode(encoded), StandardCharsets.UTF_8);
+        @SuppressWarnings("unchecked")
+        Map<String, String> secretInfo = new ObjectMapper().readValue(decoded, Map.class);
+
+        assertNotEquals(TEST_AES_KEY, secretInfo.get("key"));
+        assertNotEquals(TEST_AES_IV, secretInfo.get("iv"));
+        assertEquals("", secretInfo.get("key"));
+        assertEquals("", secretInfo.get("iv"));
+    }
+}