Skip to content

Commit 89055d4

Browse files
authored
Merge pull request #13222 from linwumingshi/fix/security-bcrypt
[ISSUE #13205] Implement `SafeBcryptPasswordEncoder` to address password length vulnerability
1 parent dfa9f6f commit 89055d4

File tree

6 files changed

+156
-5
lines changed

6 files changed

+156
-5
lines changed

Diff for: plugin-default-impl/nacos-default-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/NacosAuthConfig.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
4343
import org.springframework.security.config.annotation.web.configuration.WebSecurityCustomizer;
4444
import org.springframework.security.config.http.SessionCreationPolicy;
45-
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
4645
import org.springframework.security.crypto.password.PasswordEncoder;
4746
import org.springframework.security.web.SecurityFilterChain;
4847
import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
@@ -163,7 +162,7 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
163162

164163
@Bean
165164
public PasswordEncoder passwordEncoder() {
166-
return new BCryptPasswordEncoder();
165+
return new SafeBcryptPasswordEncoder();
167166
}
168167

169168
@Bean
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 1999-2025 Alibaba Group Holding Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.alibaba.nacos.plugin.auth.impl;
18+
19+
import com.alibaba.nacos.plugin.auth.impl.constant.AuthConstants;
20+
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
21+
22+
/**
23+
* BCrypt encoder that fixes the password length vulnerability.
24+
*
25+
* <p>Problem solved: When password length exceeds 72 characters, the original {@link BCryptPasswordEncoder}
26+
* only matches the first 72 characters, which could lead to different passwords being
27+
* validated as matching (e.g., passwords {@code "A".repeat(73)} and {@code "A".repeat(80)}
28+
* would be considered identical).
29+
*
30+
* <p>Fix logic: Adds length validation in {@link #matches(CharSequence, String)},
31+
* returning false directly if the password length exceeds 72.
32+
*
33+
* <p><strong>Recommendation:</strong> It is advised to add password length validation
34+
* during user registration/password modification to prevent login failures caused
35+
* by historical data issues.
36+
*
37+
* @see <a href="https://github.com/advisories/GHSA-mg83-c7gq-rv5c">Spring Security Password Length Vulnerability Advisory</a>
38+
* @author linwumignshi
39+
*/
40+
public class SafeBcryptPasswordEncoder extends BCryptPasswordEncoder {
41+
42+
@Override
43+
public boolean matches(CharSequence rawPassword, String encodedPassword) {
44+
// Reject excessively long passwords immediately
45+
if (rawPassword != null && rawPassword.length() > AuthConstants.MAX_PASSWORD_LENGTH) {
46+
return false;
47+
}
48+
return super.matches(rawPassword, encodedPassword);
49+
}
50+
}

Diff for: plugin-default-impl/nacos-default-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/constant/AuthConstants.java

+5
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,9 @@ public class AuthConstants {
7777
public static final String LDAP_DEFAULT_ENCODED_PASSWORD = PasswordEncoderUtil.encode(System.getProperty("ldap.default.password", "nacos"));
7878

7979
public static final String LDAP_PREFIX = "LDAP_";
80+
81+
/**
82+
* Maximum allowed password length.
83+
*/
84+
public static final int MAX_PASSWORD_LENGTH = 72;
8085
}

Diff for: plugin-default-impl/nacos-default-auth-plugin/src/main/java/com/alibaba/nacos/plugin/auth/impl/utils/PasswordEncoderUtil.java

+16-3
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616

1717
package com.alibaba.nacos.plugin.auth.impl.utils;
1818

19-
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
19+
import com.alibaba.nacos.plugin.auth.impl.SafeBcryptPasswordEncoder;
20+
import com.alibaba.nacos.plugin.auth.impl.constant.AuthConstants;
2021

2122
/**
2223
* Password encoder tool.
@@ -26,10 +27,22 @@
2627
public class PasswordEncoderUtil {
2728

2829
public static Boolean matches(String raw, String encoded) {
29-
return new BCryptPasswordEncoder().matches(raw, encoded);
30+
return new SafeBcryptPasswordEncoder().matches(raw, encoded);
3031
}
3132

33+
/**
34+
* Encode password.
35+
*
36+
* @param raw password
37+
* @return encoded password
38+
*/
3239
public static String encode(String raw) {
33-
return new BCryptPasswordEncoder().encode(raw);
40+
if (raw == null) {
41+
throw new IllegalArgumentException("Password cannot be null");
42+
}
43+
if (raw.length() > AuthConstants.MAX_PASSWORD_LENGTH) {
44+
throw new IllegalArgumentException("Password length must not exceed " + AuthConstants.MAX_PASSWORD_LENGTH + " characters");
45+
}
46+
return new SafeBcryptPasswordEncoder().encode(raw);
3447
}
3548
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Copyright 1999-2025 Alibaba Group Holding Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.alibaba.nacos.plugin.auth.impl;
18+
19+
import com.alibaba.nacos.plugin.auth.impl.constant.AuthConstants;
20+
import org.apache.commons.lang.StringUtils;
21+
import org.junit.jupiter.api.Test;
22+
23+
import static org.junit.jupiter.api.Assertions.assertFalse;
24+
import static org.junit.jupiter.api.Assertions.assertTrue;
25+
26+
/**
27+
* SafeBcryptPasswordEncoderTest.
28+
*
29+
* @author linuwmingshi
30+
*/
31+
public class SafeBcryptPasswordEncoderTest {
32+
33+
/**
34+
* SafeBCryptPasswordEncoder.
35+
*/
36+
private static final SafeBcryptPasswordEncoder ENCODER = new SafeBcryptPasswordEncoder();
37+
38+
@Test
39+
void testValidPasswordLength() {
40+
String rawPassword = StringUtils.repeat("A", AuthConstants.MAX_PASSWORD_LENGTH);
41+
String encodedPassword = ENCODER.encode(rawPassword);
42+
43+
assertTrue(ENCODER.matches(rawPassword, encodedPassword), "72-character rawPassword should match");
44+
}
45+
46+
@Test
47+
void testExcessivePasswordLength() {
48+
String rawPassword = StringUtils.repeat("A", AuthConstants.MAX_PASSWORD_LENGTH + 1);
49+
String encodedPassword = ENCODER.encode(rawPassword.substring(0, AuthConstants.MAX_PASSWORD_LENGTH));
50+
51+
assertFalse(ENCODER.matches(rawPassword, encodedPassword), "73-character rawPassword should be rejected");
52+
}
53+
54+
@Test
55+
void testEdgeCase() {
56+
String rawPassword72 = StringUtils.repeat("A", AuthConstants.MAX_PASSWORD_LENGTH);
57+
String rawPassword73 = rawPassword72 + "A";
58+
String encodedPassword = ENCODER.encode(rawPassword72);
59+
60+
assertTrue(ENCODER.matches(rawPassword72, encodedPassword), "72-character password must pass");
61+
assertFalse(ENCODER.matches(rawPassword73, encodedPassword), "73-character password must fail");
62+
}
63+
}

Diff for: plugin-default-impl/nacos-default-auth-plugin/src/test/java/com/alibaba/nacos/plugin/auth/impl/utils/PasswordEncoderUtilTest.java

+21
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,15 @@
1616

1717
package com.alibaba.nacos.plugin.auth.impl.utils;
1818

19+
import com.alibaba.nacos.plugin.auth.impl.SafeBcryptPasswordEncoder;
20+
import com.alibaba.nacos.plugin.auth.impl.constant.AuthConstants;
21+
import org.apache.commons.lang.StringUtils;
1922
import org.junit.jupiter.api.Test;
23+
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
2024

2125
import static org.junit.jupiter.api.Assertions.assertFalse;
2226
import static org.junit.jupiter.api.Assertions.assertNotEquals;
27+
import static org.junit.jupiter.api.Assertions.assertThrows;
2328
import static org.junit.jupiter.api.Assertions.assertTrue;
2429

2530
/**
@@ -50,4 +55,20 @@ void matches() {
5055
Boolean matches = PasswordEncoderUtil.matches("nacos", PasswordEncoderUtil.encode("nacos"));
5156
assertTrue(matches);
5257
}
58+
59+
@Test
60+
void enforcePasswordLength() {
61+
String raw72Password = StringUtils.repeat("A", AuthConstants.MAX_PASSWORD_LENGTH);
62+
String encodedPassword = PasswordEncoderUtil.encode(raw72Password);
63+
64+
assertThrows(IllegalArgumentException.class, () -> PasswordEncoderUtil.encode(null));
65+
66+
String raw73Password = raw72Password.concat("A");
67+
assertThrows(IllegalArgumentException.class, () -> PasswordEncoderUtil.encode(raw73Password));
68+
69+
assertTrue(new BCryptPasswordEncoder().matches(raw73Password, encodedPassword));
70+
assertFalse(new SafeBcryptPasswordEncoder().matches(raw73Password, encodedPassword));
71+
assertFalse(PasswordEncoderUtil.matches(raw73Password, encodedPassword));
72+
73+
}
5374
}

0 commit comments

Comments
 (0)