Skip to content

Commit 31b51c4

Browse files
in-response-to in saml error response
in case when there is a mismatch between the expected request ID (that comes from a cookie) and the actual in-response-to field in the SAML content, return more specific error to allow for better error handling at the client closes #128179
1 parent 4a6fd8a commit 31b51c4

File tree

3 files changed

+46
-5
lines changed

3 files changed

+46
-5
lines changed

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.Collection;
2020

2121
import static org.elasticsearch.xpack.security.authc.saml.SamlUtils.samlException;
22+
import static org.elasticsearch.xpack.security.authc.saml.SamlUtils.samlUnsolicitedInResponseToException;
2223

2324
public class SamlResponseHandler extends SamlObjectHandler {
2425
public SamlResponseHandler(Clock clock, IdpConfiguration idp, SpConfiguration sp, TimeValue maxSkew) {
@@ -32,8 +33,7 @@ protected void checkInResponseTo(StatusResponseType response, Collection<String>
3233
+ "incorrectly populates the InResponseTo attribute",
3334
response.getID()
3435
);
35-
throw samlException(
36-
"SAML content is in-response-to [{}] but expected one of {} ",
36+
throw samlUnsolicitedInResponseToException(
3737
response.getInResponseTo(),
3838
allowedSamlRequestIds
3939
);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.security.SecureRandom;
4343
import java.util.ArrayList;
4444
import java.util.Arrays;
45+
import java.util.Collection;
4546
import java.util.List;
4647
import java.util.Objects;
4748
import java.util.concurrent.atomic.AtomicBoolean;
@@ -62,6 +63,7 @@
6263
public class SamlUtils {
6364

6465
private static final String SAML_EXCEPTION_KEY = "es.security.saml";
66+
private static final String SAML_UNSOLICITED_RESPONSE_KEY = "es.security.saml.unsolicited_in_response_to";
6567
private static final String SAML_MARSHALLING_ERROR_STRING = "_unserializable_";
6668

6769
private static final AtomicBoolean INITIALISED = new AtomicBoolean(false);
@@ -115,6 +117,26 @@ public static ElasticsearchSecurityException samlException(String msg, Exception
115117
return exception;
116118
}
117119

120+
/**
121+
* Constructs exception for a specific case where the in-response-to value in the SAML content does not match any of the values
122+
* provided by the client. One example situation when this can happen is when user spent too much time on the IdP site and the
123+
* initial SAML request has expired. In that case the IdP would send a SAML response which content includes an in-response-to value
124+
* matching the initial request, however that initial request is now gone, so the client sends an empty in-response-to parameter causing
125+
* a mismatch between the two.
126+
*/
127+
public static ElasticsearchSecurityException samlUnsolicitedInResponseToException(
128+
String samlContentInResponseTo,
129+
Collection<String> expectedInResponseTos
130+
) {
131+
final ElasticsearchSecurityException exception = samlException(
132+
"SAML content is in-response-to [{}] but expected one of {} ",
133+
samlContentInResponseTo,
134+
expectedInResponseTos
135+
);
136+
exception.addMetadata(SAML_UNSOLICITED_RESPONSE_KEY, samlContentInResponseTo);
137+
return exception;
138+
}
139+
118140
/**
119141
* @see #samlException(String, Object...)
120142
*/

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import static org.hamcrest.CoreMatchers.not;
8686
import static org.hamcrest.Matchers.contains;
8787
import static org.hamcrest.Matchers.containsInAnyOrder;
88+
import static org.hamcrest.Matchers.containsInRelativeOrder;
8889
import static org.hamcrest.Matchers.containsString;
8990
import static org.hamcrest.Matchers.endsWith;
9091
import static org.hamcrest.Matchers.equalTo;
@@ -1414,6 +1415,20 @@ public void testDescribeVeryLongIssuer() {
14141415
assertThat(description, endsWith("..."));
14151416
}
14161417

1418+
public void testUnsolicitedResponse() throws Exception {
1419+
final String xml = getSimpleResponseAsString(clock.instant());
1420+
1421+
// response with valid in-response-to, while allowedRequestIds is empty (i.e. unsolicited response)
1422+
final SamlToken token = token(signResponse(xml), Collections.emptyList());
1423+
ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
1424+
assertThat(exception.getCause(), nullValue());
1425+
assertThat(exception.getMessage(), containsString("SAML content is in-response-to"));
1426+
1427+
final String EXPECTED_METADATA_KEY = "es.security.saml.unsolicited_in_response_to";
1428+
assertThat(exception.getMetadataKeys(), containsInRelativeOrder(EXPECTED_METADATA_KEY));
1429+
assertThat(exception.getMetadata(EXPECTED_METADATA_KEY), equalTo(Collections.singletonList(requestId)));
1430+
}
1431+
14171432
private interface CryptoTransform {
14181433
String transform(String xml, Tuple<X509Certificate, PrivateKey> keyPair) throws Exception;
14191434
}
@@ -1743,11 +1758,15 @@ private String getSimpleResponseFromXmlTemplate(
17431758
}
17441759

17451760
private SamlToken token(String content) {
1746-
return token(content.getBytes(StandardCharsets.UTF_8));
1761+
return token(content.getBytes(StandardCharsets.UTF_8), singletonList(requestId));
1762+
}
1763+
1764+
private SamlToken token(String content, List<String> allowedRequestIds) {
1765+
return token(content.getBytes(StandardCharsets.UTF_8), allowedRequestIds);
17471766
}
17481767

1749-
private SamlToken token(byte[] content) {
1750-
return new SamlToken(content, singletonList(requestId), null);
1768+
private SamlToken token(byte[] content, List<String> allowedRequestIds) {
1769+
return new SamlToken(content, allowedRequestIds, null);
17511770
}
17521771

17531772
}

0 commit comments

Comments
 (0)