Skip to content

Remove dependency on commons-codec by using java.util.Base64 (for 5.8.x) #11322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,6 @@ updateDependenciesSettings {
alphaBetaVersions()
snapshotVersions()
addRule { components ->
components.withModule("commons-codec:commons-codec") { selection ->
ModuleComponentIdentifier candidate = selection.getCandidate();
if (!candidate.getVersion().equals(selection.getCurrentVersion())) {
selection.reject("commons-codec updates break saml tests");
}
}
components.withModule("org.python:jython") { selection ->
ModuleComponentIdentifier candidate = selection.getCandidate();
if (!candidate.getVersion().equals(selection.getCurrentVersion())) {
Expand Down
1 change: 0 additions & 1 deletion dependencies/spring-security-dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ dependencies {
api "com.squareup.okhttp3:mockwebserver:3.14.9"
api "com.squareup.okhttp3:okhttp:3.14.9"
api "com.unboundid:unboundid-ldapsdk:4.0.14"
api "commons-codec:commons-codec:1.15"
api "commons-collections:commons-collections:3.2.2"
api "io.mockk:mockk:1.12.3"
api "io.projectreactor.tools:blockhound:1.0.6.RELEASE"
Expand Down
1 change: 0 additions & 1 deletion messaging/spring-security-messaging.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ dependencies {
optional 'jakarta.servlet:jakarta.servlet-api'

testImplementation project(path: ':spring-security-core', configuration: 'tests')
testImplementation 'commons-codec:commons-codec'
testImplementation "org.assertj:assertj-core"
testImplementation "org.junit.jupiter:junit-jupiter-api"
testImplementation "org.junit.jupiter:junit-jupiter-params"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,14 @@

import java.io.ByteArrayOutputStream;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Base64;
import java.util.function.Function;
import java.util.zip.Inflater;
import java.util.zip.InflaterOutputStream;

import javax.servlet.http.HttpServletRequest;

import org.apache.commons.codec.CodecPolicy;
import org.apache.commons.codec.binary.Base64;

import org.springframework.core.convert.converter.Converter;
import org.springframework.http.HttpMethod;
import org.springframework.security.saml2.core.Saml2Error;
Expand All @@ -49,7 +48,11 @@
*/
public final class Saml2AuthenticationTokenConverter implements AuthenticationConverter {

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

private static final Base64Checker BASE_64_CHECKER = new Base64Checker();

private final RelyingPartyRegistrationResolver relyingPartyRegistrationResolver;

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

private byte[] samlDecode(String base64EncodedPayload) {
try {
BASE_64_CHECKER.checkAcceptable(base64EncodedPayload);
return BASE64.decode(base64EncodedPayload);
}
catch (Exception ex) {
Expand All @@ -149,4 +153,58 @@ private String samlInflate(byte[] b) {
}
}

static class Base64Checker {

private static final int[] values = genValueMapping();

Base64Checker() {

}

private static int[] genValueMapping() {
byte[] alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"
.getBytes(StandardCharsets.ISO_8859_1);

int[] values = new int[256];
Arrays.fill(values, -1);
for (int i = 0; i < alphabet.length; i++) {
values[alphabet[i] & 0xff] = i;
}
return values;
}

boolean isAcceptable(String s) {
int goodChars = 0;
int lastGoodCharVal = -1;

// count number of characters from Base64 alphabet
for (int i = 0; i < s.length(); i++) {
int val = values[0xff & s.charAt(i)];
if (val != -1) {
lastGoodCharVal = val;
goodChars++;
}
}

// in cases of an incomplete final chunk, ensure the unused bits are zero
switch (goodChars % 4) {
case 0:
return true;
case 2:
return (lastGoodCharVal & 0b1111) == 0;
case 3:
return (lastGoodCharVal & 0b11) == 0;
default:
return false;
}
}

void checkAcceptable(String ins) {
if (!isAcceptable(ins)) {
throw new IllegalArgumentException("Unaccepted Encoding");
}
}

}

}
1 change: 0 additions & 1 deletion web/spring-security-web.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ dependencies {
provided 'jakarta.servlet:jakarta.servlet-api'

testImplementation project(path: ':spring-security-core', configuration: 'tests')
testImplementation 'commons-codec:commons-codec'
testImplementation 'io.projectreactor:reactor-test'
testImplementation 'jakarta.xml.bind:jakarta.xml.bind-api'
testImplementation 'org.hamcrest:hamcrest'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2002-2022 the original author or authors.
*
* Licensed 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
*
* https://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.springframework.security.test.web;

import java.util.Base64;

import org.springframework.util.DigestUtils;

public final class CodecTestUtils {

private CodecTestUtils() {
}

public static String encodeBase64(String unencoded) {
return Base64.getEncoder().encodeToString(unencoded.getBytes());
}

public static String encodeBase64(byte[] unencoded) {
return Base64.getEncoder().encodeToString(unencoded);
}

public static String decodeBase64(String encoded) {
return new String(Base64.getDecoder().decode(encoded));
}

public static boolean isBase64(byte[] arrayOctet) {
try {
Base64.getMimeDecoder().decode(arrayOctet);
return true;

}
catch (Exception ex) {
return false;
}
}

public static String md5Hex(String data) {
return DigestUtils.md5DigestAsHex(data.getBytes());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

import javax.servlet.http.Cookie;

import org.apache.commons.codec.binary.Base64;
import org.apache.commons.codec.digest.DigestUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -34,6 +32,7 @@
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.security.test.web.CodecTestUtils;
import org.springframework.util.StringUtils;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -77,7 +76,7 @@ void udsWillReturnNull() {
}

private long determineExpiryTimeFromBased64EncodedToken(String validToken) {
String cookieAsPlainText = new String(Base64.decodeBase64(validToken.getBytes()));
String cookieAsPlainText = CodecTestUtils.decodeBase64(validToken);
String[] cookieTokens = StringUtils.delimitedListToStringArray(cookieAsPlainText, ":");
if (cookieTokens.length == 3) {
try {
Expand All @@ -93,9 +92,9 @@ private String generateCorrectCookieContentForToken(long expiryTime, String user
// format is:
// username + ":" + expiryTime + ":" + Md5Hex(username + ":" + expiryTime + ":" +
// password + ":" + key)
String signatureValue = DigestUtils.md5Hex(username + ":" + expiryTime + ":" + password + ":" + key);
String signatureValue = CodecTestUtils.md5Hex(username + ":" + expiryTime + ":" + password + ":" + key);
String tokenValue = username + ":" + expiryTime + ":" + signatureValue;
return new String(Base64.encodeBase64(tokenValue.getBytes()));
return CodecTestUtils.encodeBase64(tokenValue);
}

@Test
Expand Down Expand Up @@ -135,7 +134,7 @@ public void autoLoginReturnsNullForExpiredCookieAndClearsCookie() {
@Test
public void autoLoginReturnsNullAndClearsCookieIfMissingThreeTokensInCookieValue() {
Cookie cookie = new Cookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY,
new String(Base64.encodeBase64("x".getBytes())));
CodecTestUtils.encodeBase64("x"));
MockHttpServletRequest request = new MockHttpServletRequest();
request.setCookies(cookie);
MockHttpServletResponse response = new MockHttpServletResponse();
Expand Down Expand Up @@ -176,7 +175,7 @@ public void autoLoginClearsCookieIfSignatureBlocksDoesNotMatchExpectedValue() {
@Test
public void autoLoginClearsCookieIfTokenDoesNotContainANumberInCookieValue() {
Cookie cookie = new Cookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY,
new String(Base64.encodeBase64("username:NOT_A_NUMBER:signature".getBytes())));
CodecTestUtils.encodeBase64("username:NOT_A_NUMBER:signature"));
MockHttpServletRequest request = new MockHttpServletRequest();
request.setCookies(cookie);
MockHttpServletResponse response = new MockHttpServletResponse();
Expand Down Expand Up @@ -276,7 +275,7 @@ public void loginSuccessNormalWithNonUserDetailsBasedPrincipalSetsExpectedCookie
assertThat(Long.parseLong(expiryTime) > expectedExpiryTime - 10000).isTrue();
assertThat(cookie).isNotNull();
assertThat(cookie.getMaxAge()).isEqualTo(this.services.getTokenValiditySeconds());
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
assertThat(new Date().before(new Date(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())))).isTrue();
}

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

Expand All @@ -316,7 +315,7 @@ public void negativeValidityPeriodIsSetOnCookieButExpiryTimeRemainsAtTwoWeeks()
assertThat(determineExpiryTimeFromBased64EncodedToken(cookie.getValue())
- System.currentTimeMillis() > AbstractRememberMeServices.TWO_WEEKS_S - 50).isTrue();
assertThat(cookie.getMaxAge()).isEqualTo(-1);
assertThat(Base64.isArrayByteBase64(cookie.getValue().getBytes())).isTrue();
assertThat(CodecTestUtils.isBase64(cookie.getValue().getBytes())).isTrue();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import javax.servlet.http.HttpServletRequest;

import org.apache.commons.codec.binary.Base64;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand All @@ -29,6 +28,7 @@
import org.springframework.security.authentication.AuthenticationDetailsSource;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.test.web.CodecTestUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand Down Expand Up @@ -57,7 +57,7 @@ public void setup() {
public void testNormalOperation() {
String token = "rod:koala";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
verify(this.authenticationDetailsSource).buildDetails(any());
assertThat(authentication).isNotNull();
Expand All @@ -68,7 +68,7 @@ public void testNormalOperation() {
public void requestWhenAuthorizationSchemeInMixedCaseThenAuthenticates() {
String token = "rod:koala";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "BaSiC " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "BaSiC " + CodecTestUtils.encodeBase64(token));
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
verify(this.authenticationDetailsSource).buildDetails(any());
assertThat(authentication).isNotNull();
Expand All @@ -88,7 +88,7 @@ public void testWhenUnsupportedAuthorizationHeaderThenIgnored() {
public void testWhenInvalidBasicAuthorizationTokenThenError() {
String token = "NOT_A_VALID_TOKEN_AS_MISSING_COLON";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.converter.convert(request));
}

Expand All @@ -103,7 +103,7 @@ public void testWhenInvalidBase64ThenError() {
public void convertWhenEmptyPassword() {
String token = "rod:";
MockHttpServletRequest request = new MockHttpServletRequest();
request.addHeader("Authorization", "Basic " + new String(Base64.encodeBase64(token.getBytes())));
request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token));
UsernamePasswordAuthenticationToken authentication = this.converter.convert(request);
verify(this.authenticationDetailsSource).buildDetails(any());
assertThat(authentication).isNotNull();
Expand Down
Loading