diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java index 6c3fd90eff..19a8fef087 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java @@ -489,7 +489,8 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationProvider implemen registeredClient); if (error.getErrorCode().equals(OAuth2ErrorCodes.INVALID_REQUEST) && (parameterName.equals(OAuth2ParameterNames.CLIENT_ID) - || parameterName.equals(OAuth2ParameterNames.STATE))) { + || parameterName.equals(OAuth2ParameterNames.STATE) + || parameterName.equals(OAuth2ParameterNames.REQUEST_URI))) { redirectUri = null; // Prevent redirects } diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java index 08af27f02f..a26fe0e2fb 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationValidator.java @@ -293,8 +293,10 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationValidator String redirectUri = StringUtils.hasText(authorizationCodeRequestAuthentication.getRedirectUri()) ? authorizationCodeRequestAuthentication.getRedirectUri() : registeredClient.getRedirectUris().iterator().next(); - if (error.getErrorCode().equals(OAuth2ErrorCodes.INVALID_REQUEST) - && parameterName.equals(OAuth2ParameterNames.REDIRECT_URI)) { + if ((error.getErrorCode().equals(OAuth2ErrorCodes.INVALID_REQUEST) + || error.getErrorCode().equals(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT)) + && (parameterName.equals(OAuth2ParameterNames.CLIENT_ID) + || parameterName.equals(OAuth2ParameterNames.REDIRECT_URI))) { redirectUri = null; // Prevent redirects } diff --git a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java index f5aac6dac6..307f7c8f1d 100644 --- a/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java +++ b/oauth2/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/OAuth2AuthorizationCodeRequestAuthenticationConverter.java @@ -123,46 +123,60 @@ public final class OAuth2AuthorizationCodeRequestAuthenticationConverter impleme principal = ANONYMOUS_AUTHENTICATION; } - // redirect_uri (OPTIONAL) - String redirectUri = parameters.getFirst(OAuth2ParameterNames.REDIRECT_URI); - if (StringUtils.hasText(redirectUri) && parameters.get(OAuth2ParameterNames.REDIRECT_URI).size() != 1) { - throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.REDIRECT_URI); + String redirectUri = null; + if (!StringUtils.hasText(requestUri)) { + // redirect_uri (OPTIONAL) + redirectUri = parameters.getFirst(OAuth2ParameterNames.REDIRECT_URI); + if (StringUtils.hasText(redirectUri) && parameters.get(OAuth2ParameterNames.REDIRECT_URI).size() != 1) { + throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.REDIRECT_URI); + } } - // scope (OPTIONAL) Set scopes = null; - String scope = parameters.getFirst(OAuth2ParameterNames.SCOPE); - if (StringUtils.hasText(scope) && parameters.get(OAuth2ParameterNames.SCOPE).size() != 1) { - throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.SCOPE); - } - if (StringUtils.hasText(scope)) { - scopes = new HashSet<>(Arrays.asList(StringUtils.delimitedListToStringArray(scope, " "))); + if (!StringUtils.hasText(requestUri)) { + // scope (OPTIONAL) + String scope = parameters.getFirst(OAuth2ParameterNames.SCOPE); + if (StringUtils.hasText(scope) && parameters.get(OAuth2ParameterNames.SCOPE).size() != 1) { + throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.SCOPE); + } + if (StringUtils.hasText(scope)) { + scopes = new HashSet<>(Arrays.asList(StringUtils.delimitedListToStringArray(scope, " "))); + } } - // state (RECOMMENDED) - String state = parameters.getFirst(OAuth2ParameterNames.STATE); - if (StringUtils.hasText(state) && parameters.get(OAuth2ParameterNames.STATE).size() != 1) { - throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.STATE); + String state = null; + if (!StringUtils.hasText(requestUri)) { + // state (RECOMMENDED) + state = parameters.getFirst(OAuth2ParameterNames.STATE); + if (StringUtils.hasText(state) && parameters.get(OAuth2ParameterNames.STATE).size() != 1) { + throwError(OAuth2ErrorCodes.INVALID_REQUEST, OAuth2ParameterNames.STATE); + } } - // code_challenge (REQUIRED for public clients) - RFC 7636 (PKCE) - String codeChallenge = parameters.getFirst(PkceParameterNames.CODE_CHALLENGE); - if (StringUtils.hasText(codeChallenge) && parameters.get(PkceParameterNames.CODE_CHALLENGE).size() != 1) { - throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE, PKCE_ERROR_URI); + if (!StringUtils.hasText(requestUri)) { + // code_challenge (REQUIRED for public clients) - RFC 7636 (PKCE) + String codeChallenge = parameters.getFirst(PkceParameterNames.CODE_CHALLENGE); + if (StringUtils.hasText(codeChallenge) && parameters.get(PkceParameterNames.CODE_CHALLENGE).size() != 1) { + throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE, PKCE_ERROR_URI); + } } - // code_challenge_method (OPTIONAL for public clients) - RFC 7636 (PKCE) - String codeChallengeMethod = parameters.getFirst(PkceParameterNames.CODE_CHALLENGE_METHOD); - if (StringUtils.hasText(codeChallengeMethod) - && parameters.get(PkceParameterNames.CODE_CHALLENGE_METHOD).size() != 1) { - throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE_METHOD, PKCE_ERROR_URI); + if (!StringUtils.hasText(requestUri)) { + // code_challenge_method (OPTIONAL for public clients) - RFC 7636 (PKCE) + String codeChallengeMethod = parameters.getFirst(PkceParameterNames.CODE_CHALLENGE_METHOD); + if (StringUtils.hasText(codeChallengeMethod) + && parameters.get(PkceParameterNames.CODE_CHALLENGE_METHOD).size() != 1) { + throwError(OAuth2ErrorCodes.INVALID_REQUEST, PkceParameterNames.CODE_CHALLENGE_METHOD, PKCE_ERROR_URI); + } } - // prompt (OPTIONAL for OpenID Connect 1.0 Authentication Request) - if (!CollectionUtils.isEmpty(scopes) && scopes.contains(OidcScopes.OPENID)) { - String prompt = parameters.getFirst("prompt"); - if (StringUtils.hasText(prompt) && parameters.get("prompt").size() != 1) { - throwError(OAuth2ErrorCodes.INVALID_REQUEST, "prompt"); + if (!StringUtils.hasText(requestUri)) { + // prompt (OPTIONAL for OpenID Connect 1.0 Authentication Request) + if (!CollectionUtils.isEmpty(scopes) && scopes.contains(OidcScopes.OPENID)) { + String prompt = parameters.getFirst("prompt"); + if (StringUtils.hasText(prompt) && parameters.get("prompt").size() != 1) { + throwError(OAuth2ErrorCodes.INVALID_REQUEST, "prompt"); + } } } diff --git a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java index 3af0793d52..3bd338494d 100644 --- a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java +++ b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProviderTests.java @@ -311,7 +311,7 @@ public class OAuth2AuthorizationCodeRequestAuthenticationProviderTests { assertThatExceptionOfType(OAuth2AuthorizationCodeRequestAuthenticationException.class) .isThrownBy(() -> this.authenticationProvider.authenticate(authentication)) .satisfies((ex) -> assertAuthenticationException(ex, OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, - OAuth2ParameterNames.CLIENT_ID, authentication.getRedirectUri())); + OAuth2ParameterNames.CLIENT_ID, null)); } @Test diff --git a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2PushedAuthorizationRequestAuthenticationProviderTests.java b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2PushedAuthorizationRequestAuthenticationProviderTests.java index b704062db3..64f3bf3728 100644 --- a/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2PushedAuthorizationRequestAuthenticationProviderTests.java +++ b/oauth2/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2PushedAuthorizationRequestAuthenticationProviderTests.java @@ -128,7 +128,7 @@ public class OAuth2PushedAuthorizationRequestAuthenticationProviderTests { assertThatExceptionOfType(OAuth2AuthorizationCodeRequestAuthenticationException.class) .isThrownBy(() -> this.authenticationProvider.authenticate(authentication)) .satisfies((ex) -> assertAuthenticationException(ex, OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, - OAuth2ParameterNames.CLIENT_ID, authentication.getRedirectUri())); + OAuth2ParameterNames.CLIENT_ID, null)); } @Test diff --git a/saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle b/saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle index 140f8a91e7..228548d34f 100644 --- a/saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle +++ b/saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle @@ -137,6 +137,14 @@ tasks.register("opensaml5Test", Test) { classpath = sourceSets.opensaml5Test.output + sourceSets.opensaml5Test.runtimeClasspath } -tasks.named("test") { - dependsOn opensaml5Test +tasks.register("bouncyCastleTest", Test) { + useJUnitPlatform() + testClassesDirs = sourceSets.test.output.classesDirs + classpath = sourceSets.test.runtimeClasspath + include "**/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.class" +} + +tasks.named("test") { + exclude "**/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.class" + dependsOn opensaml5Test, bouncyCastleTest } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/internal/Saml2Utils.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/internal/Saml2Utils.java index 91dae712f4..860dee0f97 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/internal/Saml2Utils.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/internal/Saml2Utils.java @@ -18,6 +18,7 @@ package org.springframework.security.saml2.internal; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; @@ -64,7 +65,7 @@ final class Saml2Utils { static String samlInflate(byte[] b) { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); - InflaterOutputStream iout = new InflaterOutputStream(out, new Inflater(true)); + InflaterOutputStream iout = new InflaterOutputStream(new CappedOutputStream(out), new Inflater(true)); iout.write(b); iout.finish(); return new String(out.toByteArray(), StandardCharsets.UTF_8); @@ -193,4 +194,27 @@ final class Saml2Utils { } + static class CappedOutputStream extends OutputStream { + + private static final long MAX_SIZE = 1024 * 1024; + + private final OutputStream delegate; + + private int size; + + CappedOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + @Override + public void write(int b) throws IOException { + if (this.size >= MAX_SIZE) { + throw new IOException("SAML payload exceeded maximum size of " + MAX_SIZE); + } + this.delegate.write(b); + this.size++; + } + + } + } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java index 3ac6f0845b..c278b594c3 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/BaseOpenSamlAuthenticationProvider.java @@ -319,62 +319,75 @@ class BaseOpenSamlAuthenticationProvider implements AuthenticationProvider { boolean responseSigned = response.isSigned(); ResponseToken responseToken = new ResponseToken(response, token); - Saml2ResponseValidatorResult result = this.responseSignatureValidator.convert(responseToken); + Collection responseSignatureErrors = this.responseSignatureValidator.convert(responseToken) + .getErrors(); + if (!responseSignatureErrors.isEmpty()) { + reportErrors(response, responseSignatureErrors); + return; + } + + Collection errors = new ArrayList<>(); if (responseSigned) { this.responseElementsDecrypter.accept(responseToken); } else if (!response.getEncryptedAssertions().isEmpty()) { - result = result.concat(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, + errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, "Did not decrypt response [" + response.getID() + "] since it is not signed")); } if (!this.validateResponseAfterAssertions) { - result = result.concat(this.responseValidator.convert(responseToken)); + errors.addAll(this.responseValidator.convert(responseToken).getErrors()); } boolean allAssertionsSigned = true; for (Assertion assertion : response.getAssertions()) { AssertionToken assertionToken = new AssertionToken(assertion, token); - result = result.concat(this.assertionSignatureValidator.convert(assertionToken)); + Collection assertionSignatureErrors = this.assertionSignatureValidator.convert(assertionToken) + .getErrors(); + errors.addAll(assertionSignatureErrors); allAssertionsSigned = allAssertionsSigned && assertion.isSigned(); + if (!assertionSignatureErrors.isEmpty()) { + continue; + } if (responseSigned || assertion.isSigned()) { this.assertionElementsDecrypter.accept(new AssertionToken(assertion, token)); } - result = result.concat(this.assertionValidator.convert(assertionToken)); + errors.addAll(this.assertionValidator.convert(assertionToken).getErrors()); } if (!responseSigned && !allAssertionsSigned) { String description = "Either the response or one of the assertions is unsigned. " + "Please either sign the response or all of the assertions."; - result = result.concat(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, description)); + errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_SIGNATURE, description)); } if (this.validateResponseAfterAssertions) { - result = result.concat(this.responseValidator.convert(responseToken)); + errors.addAll(this.responseValidator.convert(responseToken).getErrors()); } else { Assertion firstAssertion = CollectionUtils.firstElement(response.getAssertions()); if (firstAssertion != null && !hasName(firstAssertion)) { - Saml2Error error = new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, - "Assertion [" + firstAssertion.getID() + "] is missing a subject"); - result = result.concat(error); + errors.add(new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, + "Assertion [" + firstAssertion.getID() + "] is missing a subject")); } } - if (result.hasErrors()) { - Collection errors = result.getErrors(); - if (this.logger.isTraceEnabled()) { - this.logger.trace("Found " + errors.size() + " validation errors in SAML response [" + response.getID() - + "]: " + errors); - } - else if (this.logger.isDebugEnabled()) { - this.logger - .debug("Found " + errors.size() + " validation errors in SAML response [" + response.getID() + "]"); - } - Saml2Error first = errors.iterator().next(); - throw new Saml2AuthenticationException(first); - } - else { + reportErrors(response, errors); + } + + private void reportErrors(Response response, Collection errors) { + if (errors.isEmpty()) { if (this.logger.isDebugEnabled()) { this.logger.debug("Successfully processed SAML Response [" + response.getID() + "]"); } + return; } + if (this.logger.isTraceEnabled()) { + this.logger.trace("Found " + errors.size() + " validation errors in SAML response [" + response.getID() + + "]: " + errors); + } + else if (this.logger.isDebugEnabled()) { + this.logger + .debug("Found " + errors.size() + " validation errors in SAML response [" + response.getID() + "]"); + } + Saml2Error first = errors.iterator().next(); + throw new Saml2AuthenticationException(first); } private Converter createDefaultResponseSignatureValidator() { diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2Utils.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2Utils.java index 50d0d13967..162c56f569 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2Utils.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2Utils.java @@ -18,6 +18,7 @@ package org.springframework.security.saml2.provider.service.authentication; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; @@ -64,7 +65,7 @@ final class Saml2Utils { static String samlInflate(byte[] b) { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); - InflaterOutputStream iout = new InflaterOutputStream(out, new Inflater(true)); + InflaterOutputStream iout = new InflaterOutputStream(new CappedOutputStream(out), new Inflater(true)); iout.write(b); iout.finish(); return new String(out.toByteArray(), StandardCharsets.UTF_8); @@ -193,4 +194,27 @@ final class Saml2Utils { } + static class CappedOutputStream extends OutputStream { + + private static final long MAX_SIZE = 1024 * 1024; + + private final OutputStream delegate; + + private int size; + + CappedOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + @Override + public void write(int b) throws IOException { + if (this.size >= MAX_SIZE) { + throw new IOException("SAML payload exceeded maximum size of " + MAX_SIZE); + } + this.delegate.write(b); + this.size++; + } + + } + } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java index ca902cb14b..6a5dadf0c6 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java @@ -16,8 +16,9 @@ package org.springframework.security.saml2.provider.service.authentication.logout; +import java.util.ArrayList; import java.util.Collection; -import java.util.function.Consumer; +import java.util.Collections; import org.opensaml.saml.saml2.core.LogoutRequest; import org.opensaml.saml.saml2.core.NameID; @@ -57,84 +58,74 @@ class BaseOpenSamlLogoutRequestValidator implements Saml2LogoutRequestValidator LogoutRequest logoutRequest = this.saml.deserialize(Saml2Utils.withEncoded(request.getSamlRequest()) .inflate(request.getBinding() == Saml2MessageBinding.REDIRECT) .decode()); - return Saml2LogoutValidatorResult.withErrors() - .errors(verifySignature(request, logoutRequest, registration)) - .errors(validateRequest(logoutRequest, registration, authentication)) - .build(); + Collection errors = verifySignature(request, logoutRequest, registration); + if (!errors.isEmpty()) { + return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); + } + errors = validateRequest(logoutRequest, registration, authentication); + return errors.isEmpty() ? Saml2LogoutValidatorResult.success() + : Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); } - private Consumer> verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest, + private Collection verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest, RelyingPartyRegistration registration) { AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); Collection credentials = details.getVerificationX509Credentials(); VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId()); - return (errors) -> { - if (logoutRequest.isSigned()) { - errors.addAll(verify.verify(logoutRequest)); - } - else { - RedirectParameters params = new RedirectParameters(request.getParameters(), - request.getParametersQuery(), logoutRequest); - errors.addAll(verify.verify(params)); - } - }; + if (logoutRequest.isSigned()) { + return verify.verify(logoutRequest); + } + RedirectParameters params = new RedirectParameters(request.getParameters(), request.getParametersQuery(), + logoutRequest); + return verify.verify(params); } - private Consumer> validateRequest(LogoutRequest request, - RelyingPartyRegistration registration, Authentication authentication) { - return (errors) -> { - validateIssuer(request, registration).accept(errors); - validateDestination(request, registration).accept(errors); - validateSubject(request, registration, authentication).accept(errors); - }; + private Collection validateRequest(LogoutRequest request, RelyingPartyRegistration registration, + Authentication authentication) { + Collection errors = new ArrayList<>(); + errors.addAll(validateIssuer(request, registration)); + errors.addAll(validateDestination(request, registration)); + errors.addAll(validateSubject(request, registration, authentication)); + return errors; } - private Consumer> validateIssuer(LogoutRequest request, - RelyingPartyRegistration registration) { - return (errors) -> { - if (request.getIssuer() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest")); - return; - } - String issuer = request.getIssuer().getValue(); - if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { - errors - .add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); - } - }; + private Collection validateIssuer(LogoutRequest request, RelyingPartyRegistration registration) { + if (request.getIssuer() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest")); + } + String issuer = request.getIssuer().getValue(); + if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); + } + return Collections.emptyList(); } - private Consumer> validateDestination(LogoutRequest request, - RelyingPartyRegistration registration) { - return (errors) -> { - if (request.getDestination() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to find destination in LogoutRequest")); - return; - } - String destination = request.getDestination(); - if (!destination.equals(registration.getSingleLogoutServiceLocation())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to match destination to configured destination")); - } - }; + private Collection validateDestination(LogoutRequest request, RelyingPartyRegistration registration) { + if (request.getDestination() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, "Failed to find destination in LogoutRequest")); + } + String destination = request.getDestination(); + if (!destination.equals(registration.getSingleLogoutServiceLocation())) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to match destination to configured destination")); + } + return Collections.emptyList(); } - private Consumer> validateSubject(LogoutRequest request, - RelyingPartyRegistration registration, Authentication authentication) { - return (errors) -> { - if (authentication == null) { - return; - } - NameID nameId = getNameId(request, registration); - if (nameId == null) { - errors - .add(new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest")); - return; - } - - validateNameId(nameId, authentication, errors); - }; + private Collection validateSubject(LogoutRequest request, RelyingPartyRegistration registration, + Authentication authentication) { + if (authentication == null) { + return Collections.emptyList(); + } + NameID nameId = getNameId(request, registration); + if (nameId == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest")); + } + return validateNameId(nameId, authentication); } private NameID getNameId(LogoutRequest request, RelyingPartyRegistration registration) { @@ -142,13 +133,14 @@ class BaseOpenSamlLogoutRequestValidator implements Saml2LogoutRequestValidator return request.getNameID(); } - private void validateNameId(NameID nameId, Authentication authentication, Collection errors) { + private Collection validateNameId(NameID nameId, Authentication authentication) { String name = (authentication.getCredentials() instanceof Saml2ResponseAssertionAccessor assertion) ? assertion.getNameId() : authentication.getName(); if (!nameId.getValue().equals(name)) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST, + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST, "Failed to match subject in LogoutRequest with currently logged in user")); } + return Collections.emptyList(); } } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java index 92305d8610..73480fbdf9 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java @@ -16,8 +16,9 @@ package org.springframework.security.saml2.provider.service.authentication.logout; +import java.util.ArrayList; import java.util.Collection; -import java.util.function.Consumer; +import java.util.Collections; import org.opensaml.saml.saml2.core.LogoutResponse; import org.opensaml.saml.saml2.core.StatusCode; @@ -52,101 +53,90 @@ class BaseOpenSamlLogoutResponseValidator implements Saml2LogoutResponseValidato LogoutResponse logoutResponse = this.saml.deserialize(Saml2Utils.withEncoded(response.getSamlResponse()) .inflate(response.getBinding() == Saml2MessageBinding.REDIRECT) .decode()); - return Saml2LogoutValidatorResult.withErrors() - .errors(verifySignature(response, logoutResponse, registration)) - .errors(validateRequest(logoutResponse, registration)) - .errors(validateLogoutRequest(logoutResponse, request.getId())) - .build(); + Collection errors = verifySignature(response, logoutResponse, registration); + if (!errors.isEmpty()) { + return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); + } + errors = validateRequest(logoutResponse, registration, request.getId()); + return errors.isEmpty() ? Saml2LogoutValidatorResult.success() + : Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build(); } - private Consumer> verifySignature(Saml2LogoutResponse response, - LogoutResponse logoutResponse, RelyingPartyRegistration registration) { - return (errors) -> { - AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); - Collection credentials = details.getVerificationX509Credentials(); - VerificationConfigurer verify = this.saml.withVerificationKeys(credentials) - .entityId(details.getEntityId()) - .entityId(details.getEntityId()); - if (logoutResponse.isSigned()) { - errors.addAll(verify.verify(logoutResponse)); - } - else { - RedirectParameters params = new RedirectParameters(response.getParameters(), - response.getParametersQuery(), logoutResponse); - errors.addAll(verify.verify(params)); - } - }; - } - - private Consumer> validateRequest(LogoutResponse response, + private Collection verifySignature(Saml2LogoutResponse response, LogoutResponse logoutResponse, RelyingPartyRegistration registration) { - return (errors) -> { - validateIssuer(response, registration).accept(errors); - validateDestination(response, registration).accept(errors); - validateStatus(response).accept(errors); - }; + AssertingPartyMetadata details = registration.getAssertingPartyMetadata(); + Collection credentials = details.getVerificationX509Credentials(); + VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId()); + if (logoutResponse.isSigned()) { + return verify.verify(logoutResponse); + } + RedirectParameters params = new RedirectParameters(response.getParameters(), response.getParametersQuery(), + logoutResponse); + return verify.verify(params); } - private Consumer> validateIssuer(LogoutResponse response, - RelyingPartyRegistration registration) { - return (errors) -> { - if (response.getIssuer() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse")); - return; - } - String issuer = response.getIssuer().getValue(); - if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { - errors - .add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); - } - }; + private Collection validateRequest(LogoutResponse response, RelyingPartyRegistration registration, + String logoutRequestId) { + Collection errors = new ArrayList<>(); + errors.addAll(validateIssuer(response, registration)); + errors.addAll(validateDestination(response, registration)); + errors.addAll(validateStatus(response)); + errors.addAll(validateLogoutRequest(response, logoutRequestId)); + return errors; } - private Consumer> validateDestination(LogoutResponse response, - RelyingPartyRegistration registration) { - return (errors) -> { - if (response.getDestination() == null) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to find destination in LogoutResponse")); - return; - } - String destination = response.getDestination(); - if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) { - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, - "Failed to match destination to configured destination")); - } - }; + private Collection validateIssuer(LogoutResponse response, RelyingPartyRegistration registration) { + if (response.getIssuer() == null) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse")); + } + String issuer = response.getIssuer().getValue(); + if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) { + return Collections.singletonList( + new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer")); + } + return Collections.emptyList(); } - private Consumer> validateStatus(LogoutResponse response) { - return (errors) -> { - if (response.getStatus() == null) { - return; - } - if (response.getStatus().getStatusCode() == null) { - return; - } - if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) { - return; - } - if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) { - return; - } - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed")); - }; + private Collection validateDestination(LogoutResponse response, RelyingPartyRegistration registration) { + if (response.getDestination() == null) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to find destination in LogoutResponse")); + } + String destination = response.getDestination(); + if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) { + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, + "Failed to match destination to configured destination")); + } + return Collections.emptyList(); } - private Consumer> validateLogoutRequest(LogoutResponse response, String id) { - return (errors) -> { - if (response.getInResponseTo() == null) { - return; - } - if (response.getInResponseTo().equals(id)) { - return; - } - errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, - "LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest")); - }; + private Collection validateStatus(LogoutResponse response) { + if (response.getStatus() == null) { + return Collections.emptyList(); + } + if (response.getStatus().getStatusCode() == null) { + return Collections.emptyList(); + } + if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) { + return Collections.emptyList(); + } + if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) { + return Collections.emptyList(); + } + return Collections + .singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed")); + } + + private Collection validateLogoutRequest(LogoutResponse response, String id) { + if (response.getInResponseTo() == null) { + return Collections.emptyList(); + } + if (response.getInResponseTo().equals(id)) { + return Collections.emptyList(); + } + return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, + "LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest")); } } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/Saml2Utils.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/Saml2Utils.java index 75129fbeaf..c65295d033 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/Saml2Utils.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/Saml2Utils.java @@ -18,6 +18,7 @@ package org.springframework.security.saml2.provider.service.authentication.logou import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; @@ -64,7 +65,7 @@ final class Saml2Utils { static String samlInflate(byte[] b) { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); - InflaterOutputStream iout = new InflaterOutputStream(out, new Inflater(true)); + InflaterOutputStream iout = new InflaterOutputStream(new CappedOutputStream(out), new Inflater(true)); iout.write(b); iout.finish(); return new String(out.toByteArray(), StandardCharsets.UTF_8); @@ -193,4 +194,27 @@ final class Saml2Utils { } + static class CappedOutputStream extends OutputStream { + + private static final long MAX_SIZE = 1024 * 1024; + + private final OutputStream delegate; + + private int size; + + CappedOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + @Override + public void write(int b) throws IOException { + if (this.size >= MAX_SIZE) { + throw new IOException("SAML payload exceeded maximum size of " + MAX_SIZE); + } + this.delegate.write(b); + this.size++; + } + + } + } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/Saml2Utils.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/Saml2Utils.java index 0d1c1d33b6..0f873de8ed 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/Saml2Utils.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/metadata/Saml2Utils.java @@ -18,6 +18,7 @@ package org.springframework.security.saml2.provider.service.metadata; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; @@ -64,7 +65,7 @@ final class Saml2Utils { static String samlInflate(byte[] b) { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); - InflaterOutputStream iout = new InflaterOutputStream(out, new Inflater(true)); + InflaterOutputStream iout = new InflaterOutputStream(new CappedOutputStream(out), new Inflater(true)); iout.write(b); iout.finish(); return new String(out.toByteArray(), StandardCharsets.UTF_8); @@ -193,4 +194,27 @@ final class Saml2Utils { } + static class CappedOutputStream extends OutputStream { + + private static final long MAX_SIZE = 1024 * 1024; + + private final OutputStream delegate; + + private int size; + + CappedOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + @Override + public void write(int b) throws IOException { + if (this.size >= MAX_SIZE) { + throw new IOException("SAML payload exceeded maximum size of " + MAX_SIZE); + } + this.delegate.write(b); + this.size++; + } + + } + } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java index 23d9c24881..11221a013a 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java @@ -16,6 +16,11 @@ package org.springframework.security.saml2.provider.service.registration; +import java.io.IOException; +import java.io.InputStream; +import java.io.ObjectInputFilter; +import java.io.ObjectInputStream; +import java.security.Security; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Types; @@ -23,11 +28,16 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.function.Function; -import org.springframework.core.serializer.DefaultDeserializer; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.core.serializer.DefaultSerializer; import org.springframework.core.serializer.Deserializer; import org.springframework.core.serializer.Serializer; @@ -37,6 +47,7 @@ import org.springframework.jdbc.core.PreparedStatementSetter; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.SqlParameterValue; import org.springframework.security.saml2.core.Saml2X509Credential; +import org.springframework.security.saml2.provider.service.registration.JdbcAssertingPartyMetadataRepository.AssertingPartyMetadataRowMapper.Saml2X509CredentialCollectionDeserializer; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration.AssertingPartyDetails; import org.springframework.util.Assert; import org.springframework.util.function.ThrowingFunction; @@ -51,7 +62,7 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart private final JdbcOperations jdbcOperations; - private final RowMapper assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper(); + private RowMapper assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper(); private final AssertingPartyMetadataParametersMapper assertingPartyMetadataParametersMapper = new AssertingPartyMetadataParametersMapper(); @@ -145,13 +156,34 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart return this.jdbcOperations.update(UPDATE_CREDENTIAL_RECORD_SQL, parameters.toArray()); } + /** + * Set the {@link RowMapper} to use for reading {@link AssertingPartyMetadata} records + * from the database. By default, {@link AssertingPartyMetadataRowMapper} is used. + * + *

+ * Note that the default row mapper protects against insecure deserialization of + * credentials by way of {@link Saml2X509CredentialCollectionDeserializer}, which uses + * an allowlist of classes that can be deserialized. If a custom row mapper is used, + * it is the responsibility of the developer to ensure that any deserialization of + * credentials is done securely. + * @param rowMapper - the rowMapper to use + * @since 7.0.5 + * @see AssertingPartyMetadataRowMapper + */ + public void setRowMapper(RowMapper rowMapper) { + Assert.notNull(rowMapper, "rowMapper cannot be null"); + this.assertingPartyMetadataRowMapper = rowMapper; + } + /** * The default {@link RowMapper} that maps the current row in * {@code java.sql.ResultSet} to {@link AssertingPartyMetadata}. + * + * @since 7.0.5 */ - private static final class AssertingPartyMetadataRowMapper implements RowMapper { + public static final class AssertingPartyMetadataRowMapper implements RowMapper { - private final Deserializer deserializer = new DefaultDeserializer(); + private Deserializer> deserializer = new Saml2X509CredentialCollectionDeserializer(); @Override public AssertingPartyMetadata mapRow(ResultSet rs, int rowNum) throws SQLException { @@ -162,8 +194,7 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart List algorithms = List.of(rs.getString(COLUMN_NAMES[4]).split(",")); byte[] verificationCredentialsBytes = rs.getBytes(COLUMN_NAMES[5]); byte[] encryptionCredentialsBytes = rs.getBytes(COLUMN_NAMES[6]); - ThrowingFunction> credentials = ( - bytes) -> (Collection) this.deserializer.deserializeFromByteArray(bytes); + ThrowingFunction> credentials = this.deserializer::deserializeFromByteArray; AssertingPartyMetadata.Builder builder = new AssertingPartyDetails.Builder(); Collection verificationCredentials = credentials.apply(verificationCredentialsBytes); Collection encryptionCredentials = (encryptionCredentialsBytes != null) @@ -185,12 +216,121 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart return builder.build(); } + /** + * Set the {@link Deserializer} to use for reading the credential collection from + * the database. By default, {@link Saml2X509CredentialCollectionDeserializer} is + * used, which uses Java Object serialization. + * + *

+ * If a custom deserializer is used, it is the responsibility of the developer to + * ensure that any deserialization of credentials is done securely. + * @param deserializer the deserializer to use + */ + public void setCredentialsDeserializer(Deserializer> deserializer) { + Assert.notNull(deserializer, "deserializer cannot be null"); + this.deserializer = deserializer; + } + + /** + * The default deserializer for verification and encryption credentials. + * + *

+ * This is equipped with an allowlist of classes that can be deserialized. If you + * implement your own, you are responsible for to protecte against insecure + * deserialization. + * + * @since 7.0.5 + */ + public static final class Saml2X509CredentialCollectionDeserializer + implements Deserializer> { + + private static final AllowlistObjectInputFilter ALLOWLIST; + + static { + Set classes = new LinkedHashSet<>(); + classes.add(Saml2X509Credential.class.getName()); + classes.add(Saml2X509Credential.Saml2X509CredentialType.class.getName()); + classes.add(Enum.class.getName()); + classes.add("java.security.cert.Certificate$CertificateRep"); + classes.add("sun.security.x509.X509CertImpl"); + classes.add(LinkedHashSet.class.getName()); + classes.add(HashSet.class.getName()); + classes.add("java.util.Map$Entry"); + if (Security.getProvider("BC") != null) { + classes.add("org.bouncycastle.jcajce.provider.asymmetric.x509.X509CertificateObject"); + } + ALLOWLIST = new AllowlistObjectInputFilter(classes); + } + + @Override + @SuppressWarnings("unchecked") + public Collection deserialize(InputStream in) throws IOException { + ObjectInputStream oin = new ObjectInputStream(in); + oin.setObjectInputFilter(ALLOWLIST); + try { + Collection credentials = (Collection) oin.readObject(); + for (Object credential : credentials) { + Assert.isInstanceOf(Saml2X509Credential.class, credential, + "Deserialized object is not of type Saml2X509Credential"); + } + return credentials; + } + catch (ClassNotFoundException ex) { + throw new IOException("Failed to deserialize asserting party credential collection", ex); + } + } + + private static final class AllowlistObjectInputFilter implements ObjectInputFilter { + + private static final Log logger = LogFactory.getLog(JdbcAssertingPartyMetadataRepository.class); + + private static final int MAX_DEPTH = 20; + + private static final int MAX_REFS = 1000; + + private static final int MAX_ARRAY = 16384; + + private static final int MAX_BYTES = 1_048_576; + + private final ObjectInputFilter delegate; + + private AllowlistObjectInputFilter(Set allowlist) { + ObjectInputFilter pattern = ObjectInputFilter.Config + .createFilter(String.join(";", allowlist) + ";!*" + ";maxdepth=" + MAX_DEPTH + ";maxrefs=" + + MAX_REFS + ";maxarray=" + MAX_ARRAY + ";maxbytes=" + MAX_BYTES); + ObjectInputFilter global = ObjectInputFilter.Config.getSerialFilter(); + this.delegate = (global != null) ? ObjectInputFilter.merge(pattern, global) : pattern; + } + + @Override + public Status checkInput(FilterInfo info) { + Status status = this.delegate.checkInput(info); + if (status != Status.REJECTED) { + return status; + } + Class c = info.serialClass(); + if (c != null) { + logger.trace("Failed to deserialize due to [" + c.getName() + "] not being in the allowlist"); + } + else { + logger.trace("Failed to deserialize due to exceeding one the following limits: " + "depth=[" + + info.depth() + " < " + MAX_DEPTH + "]" + ", refs=[" + info.references() + " < " + + MAX_REFS + "]" + ", bytes=[" + info.streamBytes() + " < " + MAX_BYTES + "]" + + ", arrayLength=[" + info.arrayLength() + " < " + MAX_ARRAY + "]"); + } + return Status.REJECTED; + } + + } + + } + } - private static class AssertingPartyMetadataParametersMapper + private class AssertingPartyMetadataParametersMapper implements Function> { - private final Serializer serializer = new DefaultSerializer(); + private Serializer serializer = new DefaultSerializer(); @Override public List apply(AssertingPartyMetadata record) { diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/Saml2Utils.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/Saml2Utils.java index 6f8818605f..2c72a4df7b 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/Saml2Utils.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/Saml2Utils.java @@ -18,6 +18,7 @@ package org.springframework.security.saml2.provider.service.registration; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; @@ -64,7 +65,7 @@ final class Saml2Utils { static String samlInflate(byte[] b) { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); - InflaterOutputStream iout = new InflaterOutputStream(out, new Inflater(true)); + InflaterOutputStream iout = new InflaterOutputStream(new CappedOutputStream(out), new Inflater(true)); iout.write(b); iout.finish(); return new String(out.toByteArray(), StandardCharsets.UTF_8); @@ -193,4 +194,27 @@ final class Saml2Utils { } + static class CappedOutputStream extends OutputStream { + + private static final long MAX_SIZE = 1024 * 1024; + + private final OutputStream delegate; + + private int size; + + CappedOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + @Override + public void write(int b) throws IOException { + if (this.size >= MAX_SIZE) { + throw new IOException("SAML payload exceeded maximum size of " + MAX_SIZE); + } + this.delegate.write(b); + this.size++; + } + + } + } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2Utils.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2Utils.java index e923f1eab8..86bcb03cf9 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2Utils.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2Utils.java @@ -18,6 +18,7 @@ package org.springframework.security.saml2.provider.service.web; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Base64; @@ -64,7 +65,7 @@ final class Saml2Utils { static String samlInflate(byte[] b) { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); - InflaterOutputStream iout = new InflaterOutputStream(out, new Inflater(true)); + InflaterOutputStream iout = new InflaterOutputStream(new CappedOutputStream(out), new Inflater(true)); iout.write(b); iout.finish(); return new String(out.toByteArray(), StandardCharsets.UTF_8); @@ -193,4 +194,27 @@ final class Saml2Utils { } + static class CappedOutputStream extends OutputStream { + + private static final long MAX_SIZE = 1024 * 1024; + + private final OutputStream delegate; + + private int size; + + CappedOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + @Override + public void write(int b) throws IOException { + if (this.size >= MAX_SIZE) { + throw new IOException("SAML payload exceeded maximum size of " + MAX_SIZE); + } + this.delegate.write(b); + this.size++; + } + + } + } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilter.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilter.java index 591040b410..93c7df405b 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilter.java +++ b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilter.java @@ -24,17 +24,17 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.springframework.http.MediaType; import org.springframework.security.saml2.core.Saml2ParameterNames; import org.springframework.security.saml2.provider.service.authentication.AbstractSaml2AuthenticationRequest; import org.springframework.security.saml2.provider.service.authentication.Saml2PostAuthenticationRequest; import org.springframework.security.saml2.provider.service.authentication.Saml2RedirectAuthenticationRequest; import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration; import org.springframework.security.saml2.provider.service.web.authentication.Saml2AuthenticationRequestResolver; +import org.springframework.security.web.FormPostRedirectStrategy; +import org.springframework.security.web.RedirectStrategy; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; -import org.springframework.web.util.HtmlUtils; import org.springframework.web.util.UriComponentsBuilder; import org.springframework.web.util.UriUtils; @@ -67,6 +67,8 @@ public class Saml2WebSsoAuthenticationRequestFilter extends OncePerRequestFilter private Saml2AuthenticationRequestRepository authenticationRequestRepository = new HttpSessionSaml2AuthenticationRequestRepository(); + private final RedirectStrategy formPostRedirectStrategy = new FormPostRedirectStrategy(); + /** * Construct a {@link Saml2WebSsoAuthenticationRequestFilter} with the strategy for * resolving the {@code AuthnRequest} @@ -132,54 +134,11 @@ public class Saml2WebSsoAuthenticationRequestFilter extends OncePerRequestFilter private void sendPost(HttpServletRequest request, HttpServletResponse response, Saml2PostAuthenticationRequest authenticationRequest) throws IOException { this.authenticationRequestRepository.saveAuthenticationRequest(authenticationRequest, request, response); - String html = createSamlPostRequestFormData(authenticationRequest); - response.setContentType(MediaType.TEXT_HTML_VALUE); - response.getWriter().write(html); - } - - private String createSamlPostRequestFormData(Saml2PostAuthenticationRequest authenticationRequest) { - String authenticationRequestUri = authenticationRequest.getAuthenticationRequestUri(); - String relayState = authenticationRequest.getRelayState(); - String samlRequest = authenticationRequest.getSamlRequest(); - StringBuilder html = new StringBuilder(); - html.append("\n"); - html.append("\n").append(" \n"); - html.append(" (Map.of("not", "allowed"))); + this.jdbcOperations.update( + "UPDATE saml2_asserting_party_metadata SET verification_credentials = ? WHERE entity_id = ?", + notAllowed, this.metadata.getEntityId()); + + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> this.repository.findByEntityId(this.metadata.getEntityId())) + .withRootCauseInstanceOf(InvalidClassException.class); + } + + private static byte[] serialize(Object value) throws IOException { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(bytes)) { + oos.writeObject(value); + } + return bytes.toByteArray(); + } + + private static EmbeddedDatabase createDb() { + // @formatter:off + return new EmbeddedDatabaseBuilder() + .generateUniqueName(true) + .setType(EmbeddedDatabaseType.HSQL) + .setScriptEncoding("UTF-8") + .addScript(SCHEMA_SQL_RESOURCE) + .build(); + // @formatter:on + } + + private void assertAssertingPartyEquals(AssertingPartyMetadata found, AssertingPartyMetadata expected) { + assertThat(found).isNotNull(); + assertThat(found.getEntityId()).isEqualTo(expected.getEntityId()); + assertThat(found.getSingleSignOnServiceLocation()).isEqualTo(expected.getSingleSignOnServiceLocation()); + assertThat(found.getSingleSignOnServiceBinding()).isEqualTo(expected.getSingleSignOnServiceBinding()); + assertThat(found.getWantAuthnRequestsSigned()).isEqualTo(expected.getWantAuthnRequestsSigned()); + assertThat(found.getSingleLogoutServiceLocation()).isEqualTo(expected.getSingleLogoutServiceLocation()); + assertThat(found.getSingleLogoutServiceResponseLocation()) + .isEqualTo(expected.getSingleLogoutServiceResponseLocation()); + assertThat(found.getSingleLogoutServiceBinding()).isEqualTo(expected.getSingleLogoutServiceBinding()); + assertThat(found.getSigningAlgorithms()).containsAll(expected.getSigningAlgorithms()); + assertThat(found.getVerificationX509Credentials()).containsAll(expected.getVerificationX509Credentials()); + assertThat(found.getEncryptionX509Credentials()).containsAll(expected.getEncryptionX509Credentials()); + } + +} diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java index 5855462454..7d7a0654ba 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java @@ -16,7 +16,13 @@ package org.springframework.security.saml2.provider.service.registration; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InvalidClassException; +import java.io.ObjectOutputStream; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -24,12 +30,19 @@ import org.junit.jupiter.api.Test; import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; +import org.springframework.security.saml2.provider.service.registration.JdbcAssertingPartyMetadataRepository.AssertingPartyMetadataRowMapper; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; /** * Tests for {@link JdbcAssertingPartyMetadataRepository} @@ -115,6 +128,36 @@ class JdbcAssertingPartyMetadataRepositoryTests { assertThat(existing).isNotEqualTo(updated); } + @Test + void saveWhenCustomRowMapperThenUses() throws Exception { + RowMapper rowMapper = spy(new AssertingPartyMetadataRowMapper()); + this.repository.setRowMapper(rowMapper); + this.repository.save(this.metadata); + this.repository.findByEntityId(this.metadata.getEntityId()); + verify(rowMapper).mapRow(any(), eq(0)); + } + + @Test + void findByEntityIdWhenSerializedTypeNotInAllowlistThenFailsDeserialization() throws Exception { + this.repository.save(this.metadata); + byte[] notAllowed = serialize(new HashMap<>(Map.of("not", "allowed"))); + this.jdbcOperations.update( + "UPDATE saml2_asserting_party_metadata SET verification_credentials = ? WHERE entity_id = ?", + notAllowed, this.metadata.getEntityId()); + + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> this.repository.findByEntityId(this.metadata.getEntityId())) + .withRootCauseInstanceOf(InvalidClassException.class); + } + + private static byte[] serialize(Object value) throws IOException { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(bytes)) { + oos.writeObject(value); + } + return bytes.toByteArray(); + } + private static EmbeddedDatabase createDb() { return createDb(SCHEMA_SQL_RESOURCE); } diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilterTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilterTests.java index 35ac66dcdc..24bc97eddc 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilterTests.java +++ b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/web/Saml2WebSsoAuthenticationRequestFilterTests.java @@ -178,12 +178,10 @@ public class Saml2WebSsoAuthenticationRequestFilterTests { given(this.authenticationRequestResolver.resolve(any())).willReturn(request); this.filter.doFilterInternal(this.request, this.response, this.filterChain); assertThat(this.response.getHeader("Location")).isNull(); - assertThat(this.response.getContentAsString()).contains( - " ex.getSaml2Error().getErrorCode().equals(Saml2ErrorCodes.INVALID_REQUEST))); @@ -194,7 +194,7 @@ public class Saml2LogoutRequestFilterTests { this.logoutRequestProcessingFilter.doFilterInternal(request, response, new MockFilterChain()); - checkResponse(response.getContentAsString(), registration); + checkResponse(response, registration); verify(this.logoutResponseResolver).resolve(any(), any(), argThat((ex) -> ex.getSaml2Error().getErrorCode().equals(Saml2ErrorCodes.INVALID_DESTINATION))); verifyNoInteractions(this.logoutHandler); @@ -225,7 +225,7 @@ public class Saml2LogoutRequestFilterTests { this.logoutRequestProcessingFilter.doFilterInternal(request, response, new MockFilterChain()); - checkResponse(response.getContentAsString(), registration); + checkResponse(response, registration); verify(this.logoutResponseResolver).resolve(any(), any(), argThat((ex) -> ex.getSaml2Error().getErrorCode().equals(Saml2ErrorCodes.INVALID_REQUEST))); verifyNoInteractions(this.logoutHandler); @@ -259,15 +259,14 @@ public class Saml2LogoutRequestFilterTests { verifyNoInteractions(this.logoutHandler); } - private void checkResponse(String responseContent, RelyingPartyRegistration registration) { + private void checkResponse(MockHttpServletResponse response, RelyingPartyRegistration registration) + throws Exception { + String responseContent = response.getContentAsString(); assertThat(responseContent).contains(Saml2ParameterNames.SAML_RESPONSE); assertThat(responseContent) .contains(registration.getAssertingPartyMetadata().getSingleLogoutServiceResponseLocation()); - assertThat(responseContent).contains( - " Mono.empty()) - .map(URI::create); + .flatMap((decoded) -> isRelativePath(decoded) ? Mono.just(decoded) : Mono.empty()) + .map(URI::create) + .onErrorResume(IllegalArgumentException.class, (ex) -> Mono.empty()); + } + + private boolean isRelativePath(String uri) { + return uri.startsWith("/") && !uri.startsWith("//"); } @Override @@ -143,11 +149,13 @@ public class CookieServerRequestCache implements ServerRequestCache { } private static String encodeCookie(String cookieValue) { - return new String(Base64.getEncoder().encode(cookieValue.getBytes())); + return new String(Base64.getEncoder().encode(cookieValue.getBytes(StandardCharsets.UTF_8)), + StandardCharsets.UTF_8); } private static String decodeCookie(String encodedCookieValue) { - return new String(Base64.getDecoder().decode(encodedCookieValue.getBytes())); + return new String(Base64.getDecoder().decode(encodedCookieValue.getBytes(StandardCharsets.UTF_8)), + StandardCharsets.UTF_8); } private static ServerWebExchangeMatcher createDefaultRequestMatcher() { diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java index 8862e5ca94..2ea66283af 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java +++ b/web/src/test/java/org/springframework/security/web/savedrequest/CookieRequestCacheTests.java @@ -54,7 +54,7 @@ public class CookieRequestCacheTests { Cookie savedCookie = response.getCookie(DEFAULT_COOKIE_NAME); assertThat(savedCookie).isNotNull(); String redirectUrl = decodeCookie(savedCookie.getValue()); - assertThat(redirectUrl).isEqualTo("https://abc.com/destination?param1=a¶m2=b¶m3=1122"); + assertThat(redirectUrl).isEqualTo("/destination?param1=a¶m2=b¶m3=1122"); assertThat(savedCookie.getMaxAge()).isEqualTo(-1); assertThat(savedCookie.getPath()).isEqualTo("/"); assertThat(savedCookie.isHttpOnly()).isTrue(); @@ -101,18 +101,53 @@ public class CookieRequestCacheTests { public void getRequestWhenRequestContainsSavedRequestCookieThenReturnsSaveRequest() { CookieRequestCache cookieRequestCache = new CookieRequestCache(); MockHttpServletRequest request = new MockHttpServletRequest(); - String redirectUrl = "https://abc.com/destination?param1=a¶m2=b¶m3=1122"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setScheme("https"); + request.setServerName("abc.com"); + request.setServerPort(443); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination?param1=a¶m2=b¶m3=1122"))); SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); assertThat(savedRequest).isNotNull(); - assertThat(savedRequest.getRedirectUrl()).isEqualTo(redirectUrl); + assertThat(savedRequest.getRedirectUrl()) + .isEqualTo("https://abc.com/destination?param1=a¶m2=b¶m3=1122"); + } + + @Test + public void getRequestWhenRelativePathInCookieThenRedirectUrlUsesCurrentRequestOrigin() { + CookieRequestCache cookieRequestCache = new CookieRequestCache(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setScheme("https"); + request.setServerName("myapp.com"); + request.setServerPort(443); + request.setSecure(true); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/secret?token=abc"))); + SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); + assertThat(savedRequest).isNotNull(); + assertThat(savedRequest.getRedirectUrl()).isEqualTo("https://myapp.com/secret?token=abc"); + } + + @Test + public void getRequestWhenAbsoluteUrlInCookieThenReturnsNull() { + CookieRequestCache cookieRequestCache = new CookieRequestCache(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("https://evil.com/phishing"))); + SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); + assertThat(savedRequest).isNull(); + } + + @Test + public void getRequestWhenProtocolRelativeUrlInCookieThenReturnsNull() { + CookieRequestCache cookieRequestCache = new CookieRequestCache(); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("//evil.com/phishing"))); + SavedRequest savedRequest = cookieRequestCache.getRequest(request, new MockHttpServletResponse()); + assertThat(savedRequest).isNull(); } @Test public void getRequestWhenRequestContainsSavedRequestCookieThenSavedRequestContainsRequestParameters() { CookieRequestCache cookieRequestCache = new CookieRequestCache(); MockHttpServletRequest request = new MockHttpServletRequest(); - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("https://abc.com/destination"))); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination"))); request.setParameter("single", "first"); request.addParameter("multi", "second"); request.addParameter("multi", "third"); @@ -143,8 +178,7 @@ public class CookieRequestCacheTests { request.setServerName("abc.com"); request.setRequestURI("/destination"); request.setQueryString("param1=a¶m2=b¶m3=1122"); - String redirectUrl = "https://abc.com/destination?param1=a¶m2=b¶m3=1122"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination?param1=a¶m2=b¶m3=1122"))); MockHttpServletResponse response = new MockHttpServletResponse(); cookieRequestCache.getMatchingRequest(request, response); Cookie expiredCookie = response.getCookie(DEFAULT_COOKIE_NAME); @@ -162,8 +196,7 @@ public class CookieRequestCacheTests { request.setScheme("https"); request.setServerName("abc.com"); request.setRequestURI("/destination"); - String redirectUrl = "https://abc.com/api"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/api"))); MockHttpServletResponse response = new MockHttpServletResponse(); final HttpServletRequest matchingRequest = cookieRequestCache.getMatchingRequest(request, response); assertThat(matchingRequest).isNull(); @@ -182,8 +215,8 @@ public class CookieRequestCacheTests { request.setRequestURI("/destination"); request.setQueryString("goto=https%3A%2F%2Fstart.spring.io"); request.setParameter("goto", "https://start.spring.io"); - String redirectUrl = "https://abc.com/destination?goto=https%3A%2F%2Fstart.spring.io"; - request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); + request.setCookies( + new Cookie(DEFAULT_COOKIE_NAME, encodeCookie("/destination?goto=https%3A%2F%2Fstart.spring.io"))); MockHttpServletResponse response = new MockHttpServletResponse(); final HttpServletRequest matchingRequest = cookieRequestCache.getMatchingRequest(request, response); assertThat(matchingRequest).isNotNull(); @@ -212,7 +245,7 @@ public class CookieRequestCacheTests { request.setServerName("example.com"); request.setRequestURI("/destination"); request.setPreferredLocales(Arrays.asList(Locale.FRENCH, Locale.GERMANY)); - String redirectUrl = "https://example.com/destination"; + String redirectUrl = "/destination"; request.setCookies(new Cookie(DEFAULT_COOKIE_NAME, encodeCookie(redirectUrl))); MockHttpServletResponse response = new MockHttpServletResponse(); HttpServletRequest matchingRequest = cookieRequestCache.getMatchingRequest(request, response); diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java index a5416bb847..8f88af7a29 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java @@ -16,8 +16,6 @@ package org.springframework.security.web.savedrequest; -import java.util.Base64; - import jakarta.servlet.http.Cookie; import org.junit.jupiter.api.Test; @@ -53,14 +51,15 @@ public class RequestCacheAwareFilterTests { request.setScheme("https"); request.setServerPort(443); request.setSecure(true); - String encodedRedirectUrl = Base64.getEncoder().encodeToString("https://abc.com/destination".getBytes()); - Cookie savedRequest = new Cookie("REDIRECT_URI", encodedRedirectUrl); + MockHttpServletResponse response = new MockHttpServletResponse(); + cache.saveRequest(request, response); + Cookie savedRequest = response.getCookie("REDIRECT_URI"); savedRequest.setMaxAge(-1); savedRequest.setSecure(request.isSecure()); savedRequest.setPath("/"); savedRequest.setHttpOnly(true); request.setCookies(savedRequest); - MockHttpServletResponse response = new MockHttpServletResponse(); + response = new MockHttpServletResponse(); filter.doFilter(request, response, new MockFilterChain()); Cookie expiredCookie = response.getCookie("REDIRECT_URI"); assertThat(expiredCookie).isNotNull(); diff --git a/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java b/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java index 9dee46fe0b..1c3808d176 100644 --- a/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java +++ b/web/src/test/java/org/springframework/security/web/server/savedrequest/CookieServerRequestCacheTests.java @@ -117,6 +117,26 @@ public class CookieServerRequestCacheTests { assertThat(redirectUri).isNull(); } + @Test + public void getRedirectUriWhenCookieContainsAbsoluteUrlThenRedirectUriIsNull() { + String encodedAbsoluteUrl = Base64.getEncoder().encodeToString("https://evil.com/phishing".getBytes()); + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/login") + .accept(MediaType.TEXT_HTML) + .cookie(new HttpCookie("REDIRECT_URI", encodedAbsoluteUrl))); + URI redirectUri = this.cache.getRedirectUri(exchange).block(); + assertThat(redirectUri).isNull(); + } + + @Test + public void getRedirectUriWhenCookieContainsProtocolRelativeUrlThenRedirectUriIsNull() { + String encodedProtocolRelativeUrl = Base64.getEncoder().encodeToString("//evil.com/phishing".getBytes()); + MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/login") + .accept(MediaType.TEXT_HTML) + .cookie(new HttpCookie("REDIRECT_URI", encodedProtocolRelativeUrl))); + URI redirectUri = this.cache.getRedirectUri(exchange).block(); + assertThat(redirectUri).isNull(); + } + @Test public void getRedirectUriWhenNoCookieThenRedirectUriIsNull() { MockServerWebExchange exchange = MockServerWebExchange