Skip to content

Commit 29ba67b

Browse files
j3grahamjzheaux
authored andcommitted
Remove dependency on commons-codec by using java.util.Base64
Closes gh-11318
1 parent cf69cdf commit 29ba67b

File tree

11 files changed

+156
-55
lines changed

11 files changed

+156
-55
lines changed

build.gradle

-6
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,6 @@ updateDependenciesSettings {
7373
alphaBetaVersions()
7474
snapshotVersions()
7575
addRule { components ->
76-
components.withModule("commons-codec:commons-codec") { selection ->
77-
ModuleComponentIdentifier candidate = selection.getCandidate();
78-
if (!candidate.getVersion().equals(selection.getCurrentVersion())) {
79-
selection.reject("commons-codec updates break saml tests");
80-
}
81-
}
8276
components.withModule("org.python:jython") { selection ->
8377
ModuleComponentIdentifier candidate = selection.getCandidate();
8478
if (!candidate.getVersion().equals(selection.getCurrentVersion())) {

dependencies/spring-security-dependencies.gradle

-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ dependencies {
2323
api "com.squareup.okhttp3:mockwebserver:3.14.9"
2424
api "com.squareup.okhttp3:okhttp:3.14.9"
2525
api "com.unboundid:unboundid-ldapsdk:4.0.14"
26-
api "commons-codec:commons-codec:1.15"
2726
api "commons-collections:commons-collections:3.2.2"
2827
api "io.mockk:mockk:1.12.3"
2928
api "io.projectreactor.tools:blockhound:1.0.6.RELEASE"

messaging/spring-security-messaging.gradle

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ dependencies {
1515
optional 'jakarta.servlet:jakarta.servlet-api'
1616

1717
testImplementation project(path: ':spring-security-core', configuration: 'tests')
18-
testImplementation 'commons-codec:commons-codec'
1918
testImplementation "org.assertj:assertj-core"
2019
testImplementation "org.junit.jupiter:junit-jupiter-api"
2120
testImplementation "org.junit.jupiter:junit-jupiter-params"

saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2AuthenticationTokenConverter.java

+62-4
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@
1818

1919
import java.io.ByteArrayOutputStream;
2020
import java.nio.charset.StandardCharsets;
21+
import java.util.Arrays;
22+
import java.util.Base64;
2123
import java.util.function.Function;
2224
import java.util.zip.Inflater;
2325
import java.util.zip.InflaterOutputStream;
2426

2527
import javax.servlet.http.HttpServletRequest;
2628

27-
import org.apache.commons.codec.CodecPolicy;
28-
import org.apache.commons.codec.binary.Base64;
29-
3029
import org.springframework.core.convert.converter.Converter;
3130
import org.springframework.http.HttpMethod;
3231
import org.springframework.security.saml2.core.Saml2Error;
@@ -49,7 +48,11 @@
4948
*/
5049
public final class Saml2AuthenticationTokenConverter implements AuthenticationConverter {
5150

52-
private static Base64 BASE64 = new Base64(0, new byte[] { '\n' }, false, CodecPolicy.STRICT);
51+
// MimeDecoder allows extra line-breaks as well as other non-alphabet values.
52+
// This matches the behaviour of the commons-codec decoder.
53+
private static final Base64.Decoder BASE64 = Base64.getMimeDecoder();
54+
55+
private static final Base64Checker BASE_64_CHECKER = new Base64Checker();
5356

5457
private final RelyingPartyRegistrationResolver relyingPartyRegistrationResolver;
5558

@@ -127,6 +130,7 @@ private String inflateIfRequired(HttpServletRequest request, byte[] b) {
127130

128131
private byte[] samlDecode(String base64EncodedPayload) {
129132
try {
133+
BASE_64_CHECKER.checkAcceptable(base64EncodedPayload);
130134
return BASE64.decode(base64EncodedPayload);
131135
}
132136
catch (Exception ex) {
@@ -149,4 +153,58 @@ private String samlInflate(byte[] b) {
149153
}
150154
}
151155

156+
static class Base64Checker {
157+
158+
private static final int[] values = genValueMapping();
159+
160+
Base64Checker() {
161+
162+
}
163+
164+
private static int[] genValueMapping() {
165+
byte[] alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"
166+
.getBytes(StandardCharsets.ISO_8859_1);
167+
168+
int[] values = new int[256];
169+
Arrays.fill(values, -1);
170+
for (int i = 0; i < alphabet.length; i++) {
171+
values[alphabet[i] & 0xff] = i;
172+
}
173+
return values;
174+
}
175+
176+
boolean isAcceptable(String s) {
177+
int goodChars = 0;
178+
int lastGoodCharVal = -1;
179+
180+
// count number of characters from Base64 alphabet
181+
for (int i = 0; i < s.length(); i++) {
182+
int val = values[0xff & s.charAt(i)];
183+
if (val != -1) {
184+
lastGoodCharVal = val;
185+
goodChars++;
186+
}
187+
}
188+
189+
// in cases of an incomplete final chunk, ensure the unused bits are zero
190+
switch (goodChars % 4) {
191+
case 0:
192+
return true;
193+
case 2:
194+
return (lastGoodCharVal & 0b1111) == 0;
195+
case 3:
196+
return (lastGoodCharVal & 0b11) == 0;
197+
default:
198+
return false;
199+
}
200+
}
201+
202+
void checkAcceptable(String ins) {
203+
if (!isAcceptable(ins)) {
204+
throw new IllegalArgumentException("Unaccepted Encoding");
205+
}
206+
}
207+
208+
}
209+
152210
}

web/spring-security-web.gradle

-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ dependencies {
2020
provided 'jakarta.servlet:jakarta.servlet-api'
2121

2222
testImplementation project(path: ':spring-security-core', configuration: 'tests')
23-
testImplementation 'commons-codec:commons-codec'
2423
testImplementation 'io.projectreactor:reactor-test'
2524
testImplementation 'jakarta.xml.bind:jakarta.xml.bind-api'
2625
testImplementation 'org.hamcrest:hamcrest'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2002-2022 the original author or authors.
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+
* https://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 org.springframework.security.test.web;
18+
19+
import java.util.Base64;
20+
21+
import org.springframework.util.DigestUtils;
22+
23+
public final class CodecTestUtils {
24+
25+
private CodecTestUtils() {
26+
}
27+
28+
public static String encodeBase64(String unencoded) {
29+
return Base64.getEncoder().encodeToString(unencoded.getBytes());
30+
}
31+
32+
public static String encodeBase64(byte[] unencoded) {
33+
return Base64.getEncoder().encodeToString(unencoded);
34+
}
35+
36+
public static String decodeBase64(String encoded) {
37+
return new String(Base64.getDecoder().decode(encoded));
38+
}
39+
40+
public static boolean isBase64(byte[] arrayOctet) {
41+
try {
42+
Base64.getMimeDecoder().decode(arrayOctet);
43+
return true;
44+
45+
}
46+
catch (Exception ex) {
47+
return false;
48+
}
49+
}
50+
51+
public static String md5Hex(String data) {
52+
return DigestUtils.md5DigestAsHex(data.getBytes());
53+
}
54+
55+
}

web/src/test/java/org/springframework/security/web/authentication/rememberme/TokenBasedRememberMeServicesTests.java

+9-10
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020

2121
import javax.servlet.http.Cookie;
2222

23-
import org.apache.commons.codec.binary.Base64;
24-
import org.apache.commons.codec.digest.DigestUtils;
2523
import org.junit.jupiter.api.BeforeEach;
2624
import org.junit.jupiter.api.Test;
2725

@@ -34,6 +32,7 @@
3432
import org.springframework.security.core.userdetails.UserDetails;
3533
import org.springframework.security.core.userdetails.UserDetailsService;
3634
import org.springframework.security.core.userdetails.UsernameNotFoundException;
35+
import org.springframework.security.test.web.CodecTestUtils;
3736
import org.springframework.util.StringUtils;
3837

3938
import static org.assertj.core.api.Assertions.assertThat;
@@ -77,7 +76,7 @@ void udsWillReturnNull() {
7776
}
7877

7978
private long determineExpiryTimeFromBased64EncodedToken(String validToken) {
80-
String cookieAsPlainText = new String(Base64.decodeBase64(validToken.getBytes()));
79+
String cookieAsPlainText = CodecTestUtils.decodeBase64(validToken);
8180
String[] cookieTokens = StringUtils.delimitedListToStringArray(cookieAsPlainText, ":");
8281
if (cookieTokens.length == 3) {
8382
try {
@@ -93,9 +92,9 @@ private String generateCorrectCookieContentForToken(long expiryTime, String user
9392
// format is:
9493
// username + ":" + expiryTime + ":" + Md5Hex(username + ":" + expiryTime + ":" +
9594
// password + ":" + key)
96-
String signatureValue = DigestUtils.md5Hex(username + ":" + expiryTime + ":" + password + ":" + key);
95+
String signatureValue = CodecTestUtils.md5Hex(username + ":" + expiryTime + ":" + password + ":" + key);
9796
String tokenValue = username + ":" + expiryTime + ":" + signatureValue;
98-
return new String(Base64.encodeBase64(tokenValue.getBytes()));
97+
return CodecTestUtils.encodeBase64(tokenValue);
9998
}
10099

101100
@Test
@@ -135,7 +134,7 @@ public void autoLoginReturnsNullForExpiredCookieAndClearsCookie() {
135134
@Test
136135
public void autoLoginReturnsNullAndClearsCookieIfMissingThreeTokensInCookieValue() {
137136
Cookie cookie = new Cookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY,
138-
new String(Base64.encodeBase64("x".getBytes())));
137+
CodecTestUtils.encodeBase64("x"));
139138
MockHttpServletRequest request = new MockHttpServletRequest();
140139
request.setCookies(cookie);
141140
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -176,7 +175,7 @@ public void autoLoginClearsCookieIfSignatureBlocksDoesNotMatchExpectedValue() {
176175
@Test
177176
public void autoLoginClearsCookieIfTokenDoesNotContainANumberInCookieValue() {
178177
Cookie cookie = new Cookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY,
179-
new String(Base64.encodeBase64("username:NOT_A_NUMBER:signature".getBytes())));
178+
CodecTestUtils.encodeBase64("username:NOT_A_NUMBER:signature"));
180179
MockHttpServletRequest request = new MockHttpServletRequest();
181180
request.setCookies(cookie);
182181
MockHttpServletResponse response = new MockHttpServletResponse();
@@ -276,7 +275,7 @@ public void loginSuccessNormalWithNonUserDetailsBasedPrincipalSetsExpectedCookie
276275
assertThat(Long.parseLong(expiryTime) > expectedExpiryTime - 10000).isTrue();
277276
assertThat(cookie).isNotNull();
278277
assertThat(cookie.getMaxAge()).isEqualTo(this.services.getTokenValiditySeconds());
279-
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
278+
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
280279
assertThat(new Date().before(new Date(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))).isTrue();
281280
}
282281

@@ -290,7 +289,7 @@ public void loginSuccessNormalWithUserDetailsBasedPrincipalSetsExpectedCookie()
290289
Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY);
291290
assertThat(cookie).isNotNull();
292291
assertThat(cookie.getMaxAge()).isEqualTo(this.services.getTokenValiditySeconds());
293-
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
292+
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
294293
assertThat(new Date().before(new Date(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))).isTrue();
295294
}
296295

@@ -316,7 +315,7 @@ public void negativeValidityPeriodIsSetOnCookieButExpiryTimeRemainsAtTwoWeeks()
316315
assertThat(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())
317316
- System.currentTimeMillis() > AbstractRememberMeServices.TWO_WEEKS_S - 50).isTrue();
318317
assertThat(cookie.getMaxAge()).isEqualTo(-1);
319-
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
318+
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
320319
}
321320

322321
}

web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationConverterTests.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import javax.servlet.http.HttpServletRequest;
2020

21-
import org.apache.commons.codec.binary.Base64;
2221
import org.junit.jupiter.api.BeforeEach;
2322
import org.junit.jupiter.api.Test;
2423
import org.junit.jupiter.api.extension.ExtendWith;
@@ -29,6 +28,7 @@
2928
import org.springframework.security.authentication.AuthenticationDetailsSource;
3029
import org.springframework.security.authentication.BadCredentialsException;
3130
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
31+
import org.springframework.security.test.web.CodecTestUtils;
3232

3333
import static org.assertj.core.api.Assertions.assertThat;
3434
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -57,7 +57,7 @@ public void setup() {
5757
public void testNormalOperation() {
5858
String token = "rod:koala";
5959
MockHttpServletRequest request = new MockHttpServletRequest();
60-
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
60+
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
6161
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
6262
verify(this.authenticationDetailsSource).buildDetails(any());
6363
assertThat(authentication).isNotNull();
@@ -68,7 +68,7 @@ public void testNormalOperation() {
6868
public void requestWhenAuthorizationSchemeInMixedCaseThenAuthenticates() {
6969
String token = "rod:koala";
7070
MockHttpServletRequest request = new MockHttpServletRequest();
71-
request.addHeader("Authorization", "BaSiC " + new String(Base64.encodeBase64(token.getBytes())));
71+
request.addHeader("Authorization", "BaSiC " + CodecTestUtils.encodeBase64(token));
7272
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
7373
verify(this.authenticationDetailsSource).buildDetails(any());
7474
assertThat(authentication).isNotNull();
@@ -88,7 +88,7 @@ public void testWhenUnsupportedAuthorizationHeaderThenIgnored() {
8888
public void testWhenInvalidBasicAuthorizationTokenThenError() {
8989
String token = "NOT_A_VALID_TOKEN_AS_MISSING_COLON";
9090
MockHttpServletRequest request = new MockHttpServletRequest();
91-
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
91+
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
9292
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.converter.convert(request));
9393
}
9494

@@ -103,7 +103,7 @@ public void testWhenInvalidBase64ThenError() {
103103
public void convertWhenEmptyPassword() {
104104
String token = "rod:";
105105
MockHttpServletRequest request = new MockHttpServletRequest();
106-
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
106+
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
107107
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
108108
verify(this.authenticationDetailsSource).buildDetails(any());
109109
assertThat(authentication).isNotNull();

0 commit comments

Comments
 (0)