This is an automated email from the ASF dual-hosted git repository.
hefengen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new 31f0a4d8a7 Stop exposing login AES secrets to anonymous callers (#6323)
31f0a4d8a7 is described below
commit 31f0a4d8a7e2ae99beafcb2523e67419ac11b81a
Author: aias00 <[email protected]>
AuthorDate: Wed Apr 15 19:39:54 2026 +0800
Stop exposing login AES secrets to anonymous callers (#6323)
* goalx: snapshot before shenyu-analysis
* Stop exposing login AES secrets to anonymous callers
The login page was fetching /platform/secretInfo anonymously and receiving
the long-lived AES key material used for password encryption. This
change sanitizes the endpoint output so no real key/IV is returned and
adds a server-side fallback to accept plain-text passwords when the
client no longer encrypts before login.
Constraint: Preserve compatibility with the existing bundled login page
without rebuilding the frontend assets
Rejected: Remove the endpoint or require auth only | would leave the
current login bundle unable to encrypt and cause login failures without a
backend fallback
Confidence: medium
Scope-risk: moderate
Reversibility: clean
Directive: Treat any future login bootstrap material as public-only data;
never return reusable symmetric secrets to the client
Tested: mvn -pl shenyu-admin
-Dtest=SecretServiceTest,DashboardUserServiceTest,PlatformControllerTest test
Not-tested: End-to-end browser login flow against a running admin instance
and non-TLS deployment behavior
* Refine login secret exposure fix after review
This follow-up removes an unused constructor dependency, fixes the
secret payload encoding charset explicitly, and narrows AES decrypt
attempts to inputs that look like ciphertext so normal plain-text login
fallback no longer produces avoidable decrypt failures.
Constraint: Maintain compatibility with the current login bundle while
reducing operational noise from the fallback path
Confidence: high
Scope-risk: moderate
Reversibility: clean
Directive: Avoid using exception-driven control flow for common login paths
when ciphertext can be identified up front
Tested: mvn -pl shenyu-admin
-Dtest=SecretServiceTest,DashboardUserServiceTest,PlatformControllerTest test
Not-tested: End-to-end browser login against a running admin instance
* Reduce login fallback log noise after review
This follow-up removes the exception-driven decrypt path from normal
plain-text login fallback and switches secret bootstrap encoding to an
explicit UTF-8 byte conversion while keeping the sanitized response.
Constraint: Preserve compatibility with the current bundled login page
while avoiding production error-log spam on valid fallback flows
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Do not route expected login fallback behavior through utilities
that log hard failures by default
Tested: mvn -pl shenyu-admin
-Dtest=SecretServiceTest,DashboardUserServiceTest,PlatformControllerTest test
Not-tested: End-to-end browser login against a running admin instance
* Replace login secret magic values with named constants
This follow-up names the AES block size and the sanitized secret
placeholder in production code, and replaces repeated literal key/iv
and credential strings in the related tests with explicit constants.
Constraint: Keep the cleanup local to the active login-secret fix to avoid
unrelated churn across other PR branches
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Prefer named constants for repeated protocol/security values
even in tests when they carry semantic meaning
Tested: mvn -pl shenyu-admin
-Dtest=SecretServiceTest,DashboardUserServiceTest,PlatformControllerTest test
Not-tested: Full shenyu-admin test suite
* Refine login secret fixes after latest review
This follow-up removes the fully qualified SecretProperties usage in
SecretServiceImpl and factors the login password normalization into a
named helper so the plain-text fallback path reads clearly in the main
login flow.
Constraint: Keep the update local to the existing login-secret fix without
changing the external API shape
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep security-sensitive fallback logic explicit and readable;
avoid hiding behavior in dense Optional chains
Tested: mvn -pl shenyu-admin
-Dtest=SecretServiceTest,DashboardUserServiceTest,PlatformControllerTest test
Not-tested: End-to-end browser login against a running admin instance
---
.../service/impl/DashboardUserServiceImpl.java | 55 +++++++++++++++++---
.../admin/service/impl/SecretServiceImpl.java | 12 ++---
.../admin/controller/PlatformControllerTest.java | 16 ++++--
.../admin/service/DashboardUserServiceTest.java | 16 ++++--
.../shenyu/admin/service/SecretServiceTest.java | 58 ++++++++++++++++++++++
5 files changed, 136 insertions(+), 21 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DashboardUserServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DashboardUserServiceImpl.java
index 18b41cd7c0..87ea9546c2 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DashboardUserServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/DashboardUserServiceImpl.java
@@ -52,9 +52,9 @@ import org.apache.shenyu.admin.utils.SessionUtil;
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.ldap.support.LdapEncoder;
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 class DashboardUserServiceImpl implements
DashboardUserService {
@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 class DashboardUserServiceImpl implements
DashboardUserService {
.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.
*
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SecretServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SecretServiceImpl.java
index 9e1a2596c4..aad23dc6ec 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SecretServiceImpl.java
+++
b/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 @@ import org.springframework.stereotype.Service;
@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));
}
}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/PlatformControllerTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/PlatformControllerTest.java
index e78bbe9a16..25dfbba730 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/PlatformControllerTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/PlatformControllerTest.java
@@ -52,6 +52,14 @@ import static
org.springframework.test.web.servlet.result.MockMvcResultMatchers.
@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 final class PlatformControllerTest {
*/
@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)))
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DashboardUserServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DashboardUserServiceTest.java
index d1ca7ed01b..791496e8c2 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/DashboardUserServiceTest.java
+++
b/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 final class DashboardUserServiceTest {
// 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() {
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SecretServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SecretServiceTest.java
new file mode 100644
index 0000000000..0840f89099
--- /dev/null
+++
b/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"));
+ }
+}
