Improve SAML error handling by adding metadata (elastic#137598)

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 elastic#128179

Author: piotrsulkowski-elastic

docs/changelog/137598.yaml

@@ -0,0 +1,6 @@
+pr: 137598
+summary: Improve SAML error handling by adding metadata
+area: Authentication
+type: enhancement
+issues:
+ - 128179

x-pack/plugin/security/qa/saml-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/saml/SamlInResponseToIT.java

@@ -0,0 +1,112 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security.authc.saml;
+
+import org.apache.http.util.EntityUtils;
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.ResponseException;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.test.rest.ObjectPath;
+import org.elasticsearch.xcontent.json.JsonXContent;
+
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasEntry;
+import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.is;
+
+public class SamlInResponseToIT extends SamlRestTestCase {
+
+    private static final int REALM_NUMBER = 1;
+
+    public void testInResponseTo_matchingValues() throws Exception {
+        final String username = randomAlphaOfLengthBetween(4, 12);
+        String requestId = generateRandomRequestId();
+        var response = authUser(username, requestId, requestId);
+        assertThat(response, hasKey("access_token"));
+    }
+
+    public void testInResponseTo_requestAndTokenHaveDifferentValues() throws Exception {
+        final String username = randomAlphaOfLengthBetween(4, 12);
+        String requestIdFromRequest = generateRandomRequestId();
+        String requestIdFromToken = generateDifferentRandomRequestId(requestIdFromRequest);
+
+        var exception = expectThrows(ResponseException.class, () -> authUser(username, requestIdFromRequest, requestIdFromToken));
+        assertThat(exception.getResponse().getStatusLine().getStatusCode(), is(401));
+        String errorEntity = EntityUtils.toString(exception.getResponse().getEntity());
+        assertThat(errorEntity, containsString("\"security.saml.unsolicited_in_response_to\":\"" + requestIdFromToken + "\""));
+    }
+
+    public void testInResponseTo_requestNullTokenNotNull() throws Exception {
+        final String username = randomAlphaOfLengthBetween(4, 12);
+        String requestIdFromToken = generateRandomRequestId();
+
+        var exception = expectThrows(ResponseException.class, () -> authUser(username, null, requestIdFromToken));
+        assertThat(exception.getResponse().getStatusLine().getStatusCode(), is(401));
+        String errorEntity = EntityUtils.toString(exception.getResponse().getEntity());
+        assertThat(errorEntity, containsString("\"security.saml.unsolicited_in_response_to\":\"" + requestIdFromToken + "\""));
+    }
+
+    public void testInResponseTo_requestNotNullTokenNull() throws Exception {
+        final String username = randomAlphaOfLengthBetween(4, 12);
+        String requestIdFromRequest = generateRandomRequestId();
+
+        var response = authUser(username, requestIdFromRequest, null);
+        assertThat(response, hasKey("access_token"));
+    }
+
+    public void testInResponseTo_requestNullTokenNull() throws Exception {
+        final String username = randomAlphaOfLengthBetween(4, 12);
+        var response = authUser(username, null, null);
+        assertThat(response, hasKey("access_token"));
+    }
+
+    private String generateRandomRequestId() {
+        return randomAlphaOfLength(1) + randomAlphanumericOfLength(random().nextInt(10));
+    }
+
+    private String generateDifferentRandomRequestId(String existingId) {
+        String newId;
+        do {
+            newId = generateRandomRequestId();
+        } while (newId.equals(existingId));
+        return newId;
+    }
+
+    private Map<String, Object> authUser(String username, String inResponseToFromHeader, String inResponseToInSamlToken) throws Exception {
+
+        var httpsAddress = getAcsHttpsAddress();
+        var message = new SamlResponseBuilder().spEntityId("https://sp" + REALM_NUMBER + ".example.org/")
+            .idpEntityId(getIdpEntityId(REALM_NUMBER))
+            .acs(new URL("https://" + httpsAddress.getHostName() + ":" + httpsAddress.getPort() + "/acs/" + REALM_NUMBER))
+            .attribute("urn:oid:2.5.4.3", username)
+            .sign(getDataPath(SAML_SIGNING_CRT), getDataPath(SAML_SIGNING_KEY), new char[0])
+            .inResponseTo(inResponseToInSamlToken)
+            .asString();
+
+        final Map<String, Object> body = new HashMap<>();
+        body.put("content", Base64.getEncoder().encodeToString(message.getBytes(StandardCharsets.UTF_8)));
+        body.put("realm", getSamlRealmName(REALM_NUMBER));
+        if (inResponseToFromHeader != null) {
+            body.put("ids", inResponseToFromHeader);
+        }
+
+        var req = new Request("POST", "_security/saml/authenticate");
+        req.setJsonEntity(Strings.toString(JsonXContent.contentBuilder().map(body)));
+        var resp = entityAsMap(client().performRequest(req));
+        assertThat(resp, hasEntry("username", username));
+        assertThat(ObjectPath.evaluate(resp, "authentication.authentication_realm.name"), equalTo(getSamlRealmName(REALM_NUMBER)));
+        return resp;
+    }
+}

x-pack/plugin/security/qa/saml-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseBuilder.java

@@ -122,6 +122,11 @@ public SamlResponseBuilder attribute(String name, String value) {
         return this;
     }
 
+    public SamlResponseBuilder inResponseTo(String requestId) {
+        this.inResponseTo = requestId;
+        return this;
+    }
+
     public SamlResponseBuilder sign(Path certPath, Path keyPath, char[] keyPassword) throws GeneralSecurityException, IOException {
         var privateKey = PemUtils.readPrivateKey(keyPath, () -> keyPassword);
         var certificates = PemUtils.readCertificates(List.of(certPath));

x-pack/plugin/security/qa/saml-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/saml/SamlRestTestCase.java

@@ -0,0 +1,217 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security.authc.saml;
+
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpsServer;
+
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.core.PathUtils;
+import org.elasticsearch.http.PemHttpsConfigurator;
+import org.elasticsearch.mocksocket.MockHttpServer;
+import org.elasticsearch.test.cluster.ElasticsearchCluster;
+import org.elasticsearch.test.cluster.local.model.User;
+import org.elasticsearch.test.cluster.util.resource.Resource;
+import org.elasticsearch.test.junit.RunnableTestRuleAdapter;
+import org.elasticsearch.test.rest.ESRestTestCase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class SamlRestTestCase extends ESRestTestCase {
+    protected static final String SAML_SIGNING_CRT = "/saml/signing.crt";
+    protected static final String SAML_SIGNING_KEY = "/saml/signing.key";
+
+    private static HttpsServer httpsServer;
+    private static final Map<Integer, Boolean> metadataAvailable = new HashMap<>();
+    public static final ElasticsearchCluster cluster = initTestCluster();
+    private static Path caPath;
+
+    @ClassRule
+    public static TestRule ruleChain = RuleChain.outerRule(new RunnableTestRuleAdapter(SamlRestTestCase::initWebserver))
+        .around(cluster)
+        // during the startup, the metadata remains unavailable to prevent caching. After cluster init, make metadata available.
+        .around(new RunnableTestRuleAdapter(() -> makeMetadataAvailable(1, 2, 3)));
+
+    private static void initWebserver() {
+        try {
+            final InetSocketAddress address = new InetSocketAddress(InetAddress.getLoopbackAddress().getHostAddress(), 0);
+            final Path cert = getDataResource("/ssl/http.crt");
+            final Path key = getDataResource("/ssl/http.key");
+            httpsServer = MockHttpServer.createHttps(address, 0);
+            httpsServer.setHttpsConfigurator(new PemHttpsConfigurator(cert, key, new char[0]));
+            httpsServer.start();
+            List.of(1, 2, 3).forEach(realmNumber -> {
+                try {
+                    configureMetadataResource(realmNumber);
+                } catch (CertificateException | IOException | URISyntaxException e) {
+                    throw new RuntimeException("Cannot configure metadata for realm " + realmNumber, e);
+                }
+            });
+        } catch (URISyntaxException | IOException | GeneralSecurityException e) {
+            throw new RuntimeException("Failed to initialise mock web server", e);
+        }
+    }
+
+    @AfterClass
+    public static void shutdownWebserver() {
+        httpsServer.stop(0);
+        httpsServer = null;
+    }
+
+    private static ElasticsearchCluster initTestCluster() {
+        return ElasticsearchCluster.local()
+            .nodes(1)
+            .module("analysis-common")
+            .setting("xpack.license.self_generated.type", "trial")
+            .setting("xpack.security.enabled", "true")
+            .setting("xpack.security.authc.token.enabled", "true")
+            .setting("xpack.security.authc.api_key.enabled", "true")
+            .setting("xpack.security.http.ssl.enabled", "true")
+            .setting("xpack.security.http.ssl.certificate", "node.crt")
+            .setting("xpack.security.http.ssl.key", "node.key")
+            .setting("xpack.security.http.ssl.certificate_authorities", "ca.crt")
+            .setting("xpack.security.transport.ssl.enabled", "true")
+            .setting("xpack.security.transport.ssl.certificate", "node.crt")
+            .setting("xpack.security.transport.ssl.key", "node.key")
+            .setting("xpack.security.transport.ssl.certificate_authorities", "ca.crt")
+            .setting("xpack.security.transport.ssl.verification_mode", "certificate")
+            .keystore("bootstrap.password", "x-pack-test-password")
+            .user("test_admin", "x-pack-test-password", User.ROOT_USER_ROLE, true)
+            .user("rest_test", "rest_password", User.ROOT_USER_ROLE, false)
+            .configFile("node.key", Resource.fromClasspath("ssl/node.key"))
+            .configFile("node.crt", Resource.fromClasspath("ssl/node.crt"))
+            .configFile("ca.crt", Resource.fromClasspath("ssl/ca.crt"))
+            .settings(node -> {
+                var acsWebServerAddress = getAcsHttpsAddress();
+                var acsHttps = "https://" + acsWebServerAddress.getHostName() + ":" + acsWebServerAddress.getPort() + "/";
+                var idpWebServerAddress = getIdpHttpsAddress();
+                var idpHttps = "https://" + idpWebServerAddress.getHostName() + ":" + idpWebServerAddress.getPort() + "/";
+                var settings = new HashMap<String, String>();
+                for (int realmNumber : List.of(1, 2, 3)) {
+                    var prefix = "xpack.security.authc.realms.saml.saml" + realmNumber;
+                    var idpEntityId = getIdpEntityId(realmNumber);
+                    settings.put(prefix + ".order", String.valueOf(realmNumber));
+                    settings.put(prefix + ".idp.entity_id", idpEntityId);
+                    settings.put(prefix + ".idp.metadata.path", idpHttps + "metadata/" + realmNumber + ".xml");
+                    settings.put(prefix + ".sp.entity_id", "https://sp" + realmNumber + ".example.org/");
+                    settings.put(prefix + ".sp.acs", acsHttps + "acs/" + realmNumber);
+                    settings.put(prefix + ".attributes.principal", "urn:oid:2.5.4.3");
+                    settings.put(prefix + ".ssl.certificate_authorities", "ca.crt");
+                }
+                return settings;
+            })
+            .build();
+    }
+
+    private static Path getDataResource(String relativePath) throws URISyntaxException {
+        return PathUtils.get(SamlRestTestCase.class.getResource(relativePath).toURI());
+    }
+
+    protected static String getIdpEntityId(int realmNumber) {
+        return "https://idp" + realmNumber + ".example.org/";
+    }
+
+    protected static String getSamlRealmName(int realmNumber) {
+        return "saml" + realmNumber;
+    }
+
+    protected static void makeMetadataAvailable(int... realms) {
+        for (int r : realms) {
+            metadataAvailable.put(Integer.valueOf(r), true);
+        }
+    }
+
+    protected static void makeAllMetadataUnavailable() {
+        metadataAvailable.keySet().forEach(k -> metadataAvailable.put(k, false));
+    }
+
+    protected static InetSocketAddress getAcsHttpsAddress() {
+        return httpsServer.getAddress();
+    }
+
+    protected static InetSocketAddress getIdpHttpsAddress() {
+        return httpsServer.getAddress();
+    }
+
+    private static void configureMetadataResource(int realmNumber) throws CertificateException, IOException, URISyntaxException {
+        metadataAvailable.putIfAbsent(realmNumber, false);
+
+        var signingCert = getDataResource(SAML_SIGNING_CRT);
+        var metadataBody = new SamlIdpMetadataBuilder().entityId(getIdpEntityId(realmNumber)).sign(signingCert).asString();
+        httpsServer.createContext("/metadata/" + realmNumber + ".xml", http -> {
+            if (metadataAvailable.get(realmNumber)) {
+                sendXmlContent(metadataBody, http);
+            } else {
+                if (randomBoolean()) {
+                    http.sendResponseHeaders(randomFrom(404, 401, 403, 500), 0);
+                    http.getResponseBody().close();
+                } else {
+                    sendXmlContent("not valid xml", http);
+                }
+            }
+        });
+    }
+
+    private static void sendXmlContent(String bodyContent, HttpExchange http) throws IOException {
+        http.getResponseHeaders().add("Content-Type", "text/xml");
+        http.sendResponseHeaders(200, bodyContent.length());
+        try (var out = http.getResponseBody()) {
+            out.write(bodyContent.getBytes(StandardCharsets.UTF_8));
+        }
+    }
+
+    @BeforeClass
+    public static void loadCertificateAuthority() throws Exception {
+        URL resource = SamlRestTestCase.class.getResource("/ssl/ca.crt");
+        if (resource == null) {
+            throw new FileNotFoundException("Cannot find classpath resource /ssl/ca.crt");
+        }
+        caPath = PathUtils.get(resource.toURI());
+    }
+
+    @Override
+    protected String getTestRestCluster() {
+        return cluster.getHttpAddresses();
+    }
+
+    @Override
+    protected String getProtocol() {
+        return "https";
+    }
+
+    @Override
+    protected Settings restAdminSettings() {
+        final String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray()));
+        return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).put(CERTIFICATE_AUTHORITIES, caPath).build();
+    }
+
+    @Override
+    protected Settings restClientSettings() {
+        final String token = basicAuthHeaderValue("rest_test", new SecureString("rest_password".toCharArray()));
+        return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token).put(CERTIFICATE_AUTHORITIES, caPath).build();
+    }
+}