Skip to content

Commit f491589

Browse files
lukaszmigdalekjzheaux
authored andcommitted
Use Spec Order for Verifying Signatures
Closes gh-12346
1 parent ed0369a commit f491589

File tree

2 files changed

+51
-6
lines changed

2 files changed

+51
-6
lines changed

config/src/test/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurerTests.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,34 @@ public void saml2LogoutRequestWhenLowercaseEncodingThenLogsOutAndSendsLogoutResp
318318
verify(getBean(LogoutHandler.class)).logout(any(), any(), any());
319319
}
320320

321+
// gh-12346
322+
@Test
323+
public void saml2LogoutRequestWhenLowercaseEncodingAndDifferentQueryParamOrderThenLogsOutAndSendsLogoutResponse()
324+
throws Exception {
325+
this.spring.register(Saml2LogoutDefaultsConfig.class).autowire();
326+
String apLogoutRequest = "nZFNa4QwEIb/iuQeP6K7dYO6FKQg2B622x56G3WwgiY2E8v239fqCksPPfSWIXmfNw+THC9D73yi\r\n"
327+
+ "oU6rlAWuzxxUtW461abs5fzAY3bMEoKhF6Msdasne8KPCck6c1KRXK9SNhklNVBHUsGAJG0tn+8f\r\n"
328+
+ "SylcX45GW13rnjn5HOwU2KXt3dqRpOeZ0cULDGOPrjat1y8t3gL2zFrGnCJPWXkKcR8KCHY8xmrP\r\n"
329+
+ "Iz868OpOVLwO4wohggagmd8STVgosqBsyoQvBPd3XITnIJaRL8PYjcThjTmvm/f8SXa1lEvY3Nr9\r\n"
330+
+ "LQdEaH6EWAYjR2U7+8W7JvFucRv8aY4X+b/g03zaoCsmu46/FpN9Aw==";
331+
String apLogoutRequestRelayState = "d118dbd5-3853-4268-b3e5-c40fc033fa2f";
332+
String apLogoutRequestSignature = "VZ7rWa5u3hIX60fAQs/gBQZWDP2BAIlCMMrNrTHafoKKj0uXWnuITYLuL8NdsWmyQN0+fqWW4X05+BqiLpL80jHLmQR5RVqqL1EtVv1SpPUna938lgz2sOliuYmfQNj4Bmd+Z5G1K6QhbVrtfb7TQHURjUafzfRm8+jGz3dPjVBrn/rD/umfGoSn6RuWngugcMNL4U0A+JcEh1NSfSYNVz7y+MqlW1UhX2kF86rm97ERCrxay7Gh/bI2f3fJPJ1r+EyLjzrDUkqw5cva3rVlFgEQouMVu35lUJn7SFompW8oTxkI23oc/t+AGZqaBupNITNdjyGCBpfukZ69EZrj8g==";
333+
DefaultSaml2AuthenticatedPrincipal principal = new DefaultSaml2AuthenticatedPrincipal("user",
334+
Collections.emptyMap());
335+
principal.setRelyingPartyRegistrationId("get");
336+
Saml2Authentication user = new Saml2Authentication(principal, "response",
337+
AuthorityUtils.createAuthorityList("ROLE_USER"));
338+
MvcResult result = this.mvc
339+
.perform(get("/logout/saml2/slo").param("SAMLRequest", apLogoutRequest)
340+
.param("SigAlg", this.apLogoutRequestSigAlg).param("RelayState", apLogoutRequestRelayState)
341+
.param("Signature", apLogoutRequestSignature)
342+
.with(new SamlQueryStringRequestPostProcessor(true)).with(authentication(user)))
343+
.andExpect(status().isFound()).andReturn();
344+
String location = result.getResponse().getHeader("Location");
345+
assertThat(location).startsWith("https://ap.example.org/logout/saml2/response");
346+
verify(getBean(LogoutHandler.class)).logout(any(), any(), any());
347+
}
348+
321349
@Test
322350
public void saml2LogoutRequestWhenNoRegistrationThen400() throws Exception {
323351
this.spring.register(Saml2LogoutDefaultsConfig.class).autowire();

saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSamlVerificationUtils.java

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import java.util.Collection;
2222
import java.util.Collections;
2323
import java.util.HashSet;
24+
import java.util.Map;
25+
import java.util.Objects;
2426
import java.util.Set;
2527

2628
import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
@@ -191,9 +193,9 @@ private static class RedirectSignature {
191193
else {
192194
this.signature = null;
193195
}
194-
this.content = UriComponentsBuilder.newInstance().query(request.getParametersQuery())
195-
.replaceQueryParam(Saml2ParameterNames.SIGNATURE).build(true).toUriString().substring(1)
196-
.getBytes(StandardCharsets.UTF_8);
196+
Map<String, String> queryParams = UriComponentsBuilder.newInstance().query(request.getParametersQuery())
197+
.build(true).getQueryParams().toSingleValueMap();
198+
this.content = getContent(Saml2ParameterNames.SAML_REQUEST, request.getRelayState(), queryParams);
197199
}
198200

199201
RedirectSignature(Saml2LogoutResponse response) {
@@ -204,9 +206,24 @@ private static class RedirectSignature {
204206
else {
205207
this.signature = null;
206208
}
207-
this.content = UriComponentsBuilder.newInstance().query(response.getParametersQuery())
208-
.replaceQueryParam(Saml2ParameterNames.SIGNATURE).build(true).toUriString().substring(1)
209-
.getBytes(StandardCharsets.UTF_8);
209+
Map<String, String> queryParams = UriComponentsBuilder.newInstance()
210+
.query(response.getParametersQuery()).build(true).getQueryParams().toSingleValueMap();
211+
this.content = getContent(Saml2ParameterNames.SAML_RESPONSE, response.getRelayState(), queryParams);
212+
}
213+
214+
static byte[] getContent(String samlObject, String relayState, final Map<String, String> queryParams) {
215+
if (Objects.nonNull(relayState)) {
216+
return String
217+
.format("%s=%s&%s=%s&%s=%s", samlObject, queryParams.get(samlObject),
218+
Saml2ParameterNames.RELAY_STATE, queryParams.get(Saml2ParameterNames.RELAY_STATE),
219+
Saml2ParameterNames.SIG_ALG, queryParams.get(Saml2ParameterNames.SIG_ALG))
220+
.getBytes(StandardCharsets.UTF_8);
221+
}
222+
else {
223+
return String.format("%s=%s&%s=%s", samlObject, queryParams.get(samlObject),
224+
Saml2ParameterNames.SIG_ALG, queryParams.get(Saml2ParameterNames.SIG_ALG))
225+
.getBytes(StandardCharsets.UTF_8);
226+
}
210227
}
211228

212229
byte[] getContent() {

0 commit comments

Comments
 (0)