diff --git a/docs/changelog/137598.yaml b/docs/changelog/137598.yaml new file mode 100644 index 0000000000000..1c00356a109bf --- /dev/null +++ b/docs/changelog/137598.yaml @@ -0,0 +1,6 @@ +pr: 137598 +summary: In-response-to in saml error response +area: Authentication +type: enhancement +issues: + - 128179 diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java index 484af268afca0..e670d619eab1f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandler.java @@ -19,6 +19,7 @@ import java.util.Collection; import static org.elasticsearch.xpack.security.authc.saml.SamlUtils.samlException; +import static org.elasticsearch.xpack.security.authc.saml.SamlUtils.samlUnsolicitedInResponseToException; public class SamlResponseHandler extends SamlObjectHandler { public SamlResponseHandler(Clock clock, IdpConfiguration idp, SpConfiguration sp, TimeValue maxSkew) { @@ -32,11 +33,7 @@ protected void checkInResponseTo(StatusResponseType response, Collection + "incorrectly populates the InResponseTo attribute", response.getID() ); - throw samlException( - "SAML content is in-response-to [{}] but expected one of {} ", - response.getInResponseTo(), - allowedSamlRequestIds - ); + throw samlUnsolicitedInResponseToException(response.getInResponseTo(), allowedSamlRequestIds); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java index 37461b3261922..5a8e786cf96f7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java @@ -42,6 +42,7 @@ import java.security.SecureRandom; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; @@ -62,6 +63,7 @@ public class SamlUtils { private static final String SAML_EXCEPTION_KEY = "es.security.saml"; + private static final String SAML_UNSOLICITED_RESPONSE_KEY = "es.security.saml.unsolicited_in_response_to"; private static final String SAML_MARSHALLING_ERROR_STRING = "_unserializable_"; private static final AtomicBoolean INITIALISED = new AtomicBoolean(false); @@ -115,6 +117,26 @@ public static ElasticsearchSecurityException samlException(String msg, Exception return exception; } + /** + * Constructs exception for a specific case where the in-response-to value in the SAML content does not match any of the values + * provided by the client. One example situation when this can happen is when user spent too much time on the IdP site and the + * initial SAML request has expired. In that case the IdP would send a SAML response which content includes an in-response-to value + * matching the initial request, however that initial request is now gone, so the client sends an empty in-response-to parameter causing + * a mismatch between the two. + */ + public static ElasticsearchSecurityException samlUnsolicitedInResponseToException( + String samlContentInResponseTo, + Collection expectedInResponseTos + ) { + final ElasticsearchSecurityException exception = samlException( + "SAML content is in-response-to [{}] but expected one of {} ", + samlContentInResponseTo, + expectedInResponseTos + ); + exception.addMetadata(SAML_UNSOLICITED_RESPONSE_KEY, samlContentInResponseTo); + return exception; + } + /** * @see #samlException(String, Object...) */ diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java index a46d79329751c..b139275d37034 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java @@ -85,6 +85,7 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsInRelativeOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; @@ -1414,6 +1415,20 @@ public void testDescribeVeryLongIssuer() { assertThat(description, endsWith("...")); } + public void testUnsolicitedResponse() throws Exception { + final String xml = getSimpleResponseAsString(clock.instant()); + + // response with valid in-response-to, while allowedRequestIds is empty (i.e. unsolicited response) + final SamlToken token = token(signResponse(xml), Collections.emptyList()); + ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token)); + assertThat(exception.getCause(), nullValue()); + assertThat(exception.getMessage(), containsString("SAML content is in-response-to")); + + final String EXPECTED_METADATA_KEY = "es.security.saml.unsolicited_in_response_to"; + assertThat(exception.getMetadataKeys(), containsInRelativeOrder(EXPECTED_METADATA_KEY)); + assertThat(exception.getMetadata(EXPECTED_METADATA_KEY), equalTo(Collections.singletonList(requestId))); + } + private interface CryptoTransform { String transform(String xml, Tuple keyPair) throws Exception; } @@ -1743,11 +1758,15 @@ private String getSimpleResponseFromXmlTemplate( } private SamlToken token(String content) { - return token(content.getBytes(StandardCharsets.UTF_8)); + return token(content.getBytes(StandardCharsets.UTF_8), singletonList(requestId)); + } + + private SamlToken token(String content, List allowedRequestIds) { + return token(content.getBytes(StandardCharsets.UTF_8), allowedRequestIds); } - private SamlToken token(byte[] content) { - return new SamlToken(content, singletonList(requestId), null); + private SamlToken token(byte[] content, List allowedRequestIds) { + return new SamlToken(content, allowedRequestIds, null); } }