diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java index 86474daa7ee03..53642a3c700d1 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterCredentialsManager.java @@ -18,7 +18,8 @@ import java.util.Collections; import java.util.Map; -import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import static org.elasticsearch.transport.RemoteClusterService.REMOTE_CLUSTER_CREDENTIALS; @@ -36,10 +37,16 @@ public synchronized UpdateRemoteClusterCredentialsResult updateClusterCredential final Map newClusterCredentials = REMOTE_CLUSTER_CREDENTIALS.getAsMap(settings); if (clusterCredentials.isEmpty()) { setCredentialsAndLog(newClusterCredentials); - return new UpdateRemoteClusterCredentialsResult(newClusterCredentials.keySet(), Collections.emptySet()); + return new UpdateRemoteClusterCredentialsResult(new TreeSet<>(clusterCredentials.keySet()), Collections.emptySortedSet()); } - final Set aliasesWithAddedCredentials = Sets.difference(newClusterCredentials.keySet(), clusterCredentials.keySet()); - final Set aliasesWithRemovedCredentials = Sets.difference(clusterCredentials.keySet(), newClusterCredentials.keySet()); + final SortedSet aliasesWithAddedCredentials = Sets.sortedDifference( + newClusterCredentials.keySet(), + clusterCredentials.keySet() + ); + final SortedSet aliasesWithRemovedCredentials = Sets.sortedDifference( + clusterCredentials.keySet(), + newClusterCredentials.keySet() + ); setCredentialsAndLog(newClusterCredentials); assert Sets.haveEmptyIntersection(aliasesWithRemovedCredentials, aliasesWithAddedCredentials); return new UpdateRemoteClusterCredentialsResult(aliasesWithAddedCredentials, aliasesWithRemovedCredentials); @@ -55,7 +62,12 @@ private void setCredentialsAndLog(Map newClusterCredential ); } - public record UpdateRemoteClusterCredentialsResult(Set aliasesWithAddedCredentials, Set aliasesWithRemovedCredentials) { + public record UpdateRemoteClusterCredentialsResult( + // Use sorted sets since we will iterate over these, and call a synchronized method. Establishing a deterministic order to prevent + // deadlocks + SortedSet aliasesWithAddedCredentials, + SortedSet aliasesWithRemovedCredentials + ) { int totalSize() { return aliasesWithAddedCredentials.size() + aliasesWithRemovedCredentials.size(); } diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java index 9f929c9951cf3..70f8773a0c21b 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java @@ -323,11 +323,11 @@ public synchronized void updateRemoteClusterCredentials(Settings settings, Actio }) ); for (var aliasWithAddedCredentials : result.aliasesWithAddedCredentials()) { - logger.debug("Rebuilding connection for remote cluster [{}] with added credential", aliasWithAddedCredentials); + logger.debug("Rebuilding connection for remote cluster [{}] with added credentials", aliasWithAddedCredentials); updateRemoteCluster(aliasWithAddedCredentials, settings, true, groupedListener); } for (var aliasWithRemovedCredentials : result.aliasesWithRemovedCredentials()) { - logger.debug("Rebuilding connection for remote cluster [{}] with added credential", aliasWithRemovedCredentials); + logger.debug("Rebuilding connection for remote cluster [{}] with removed credentials", aliasWithRemovedCredentials); updateRemoteCluster(aliasWithRemovedCredentials, settings, true, groupedListener); } } @@ -377,13 +377,15 @@ void updateRemoteCluster(String clusterAlias, Settings newSettings, ActionListen private synchronized void updateRemoteCluster( String clusterAlias, Settings newSettings, - boolean forceRebuild, + boolean credentialsChanged, ActionListener listener ) { if (LOCAL_CLUSTER_GROUP_KEY.equals(clusterAlias)) { throw new IllegalArgumentException("remote clusters must not have the empty string as its key"); } + // TODO: short-circuit case when credentials change, but there is no other remote cluster configuration yet + RemoteClusterConnection remote = this.remoteClusters.get(clusterAlias); if (RemoteConnectionStrategy.isConnectionEnabled(clusterAlias, newSettings) == false) { try { @@ -402,7 +404,7 @@ private synchronized void updateRemoteCluster( remote = new RemoteClusterConnection(finalSettings, clusterAlias, transportService, remoteClusterCredentialsManager); remoteClusters.put(clusterAlias, remote); remote.ensureConnected(listener.map(ignored -> RemoteClusterConnectionStatus.CONNECTED)); - } else if (remote.shouldRebuildConnection(newSettings) || forceRebuild) { + } else if (credentialsChanged || remote.shouldRebuildConnection(newSettings)) { // Changes to connection configuration. Must tear down existing connection try { IOUtils.close(remote); diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityTestCase.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityTestCase.java index d05c3c85cd07f..5a78dbbd8a369 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityTestCase.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityTestCase.java @@ -191,6 +191,19 @@ protected void configureRemoteCluster( boolean skipUnavailable ) throws Exception { // For configurable remote cluster security, this method assumes the cross cluster access API key is already configured in keystore + putRemoteClusterSettings(clusterAlias, targetFulfillingCluster, basicSecurity, isProxyMode, skipUnavailable); + + // Ensure remote cluster is connected + checkRemoteConnection(clusterAlias, targetFulfillingCluster, basicSecurity, isProxyMode); + } + + protected void putRemoteClusterSettings( + String clusterAlias, + ElasticsearchCluster targetFulfillingCluster, + boolean basicSecurity, + boolean isProxyMode, + boolean skipUnavailable + ) throws IOException { final Settings.Builder builder = Settings.builder(); final String remoteClusterEndpoint = basicSecurity ? targetFulfillingCluster.getTransportEndpoint(0) @@ -204,8 +217,14 @@ protected void configureRemoteCluster( } builder.put("cluster.remote." + clusterAlias + ".skip_unavailable", skipUnavailable); updateClusterSettings(builder.build()); + } - // Ensure remote cluster is connected + protected void checkRemoteConnection( + String clusterAlias, + ElasticsearchCluster targetFulfillingCluster, + boolean basicSecurity, + boolean isProxyMode + ) throws Exception { final Request remoteInfoRequest = new Request("GET", "/_remote/info"); assertBusy(() -> { final Response remoteInfoResponse = adminClient().performRequest(remoteInfoRequest); diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityReloadCredentialsRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityReloadCredentialsRestIT.java new file mode 100644 index 0000000000000..30b0efc7fb26e --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityReloadCredentialsRestIT.java @@ -0,0 +1,119 @@ +/* + * 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.remotecluster; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.MutableSettingsProvider; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.util.Locale; +import java.util.Map; + +public class RemoteClusterSecurityReloadCredentialsRestIT extends AbstractRemoteClusterSecurityTestCase { + private static final MutableSettingsProvider keystoreSettings = new MutableSettingsProvider(); + static { + fulfillingCluster = ElasticsearchCluster.local() + .name("fulfilling-cluster") + .apply(commonClusterConfig) + .setting("remote_cluster_server.enabled", "true") + .setting("remote_cluster.port", "0") + .setting("xpack.security.remote_cluster_server.ssl.enabled", "true") + .setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key") + .setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt") + .keystore("xpack.security.remote_cluster_server.ssl.secure_key_passphrase", "remote-cluster-password") + .build(); + + queryCluster = ElasticsearchCluster.local() + .name("query-cluster") + .apply(commonClusterConfig) + .setting("xpack.security.remote_cluster_client.ssl.enabled", "true") + .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") + .keystore(keystoreSettings) + .rolesFile(Resource.fromClasspath("roles.yml")) + .build(); + } + + @ClassRule + // Use a RuleChain to ensure that fulfilling cluster is started before query cluster + public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); + + public void testCredentialsReload() throws Exception { + final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleRequest.setJsonEntity(""" + { + "remote_indices": [ + { + "names": ["*"], + "privileges": ["read", "read_cross_cluster"], + "clusters": ["my_remote_cluster"] + } + ] + }"""); + assertOK(adminClient().performRequest(putRoleRequest)); + final var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); + putUserRequest.setJsonEntity(""" + { + "password": "x-pack-test-password", + "roles" : ["remote_search"] + }"""); + assertOK(adminClient().performRequest(putUserRequest)); + + final Map apiKeyMap = createCrossClusterAccessApiKey(""" + { + "search": [ + { + "names": ["*"] + } + ] + }"""); + + final boolean isProxyMode = randomBoolean(); + final boolean configureSettingsFirst = randomBoolean(); + // it's valid to first configure remote cluster, then credentials + if (configureSettingsFirst) { + putRemoteClusterSettings("my_remote_cluster", fulfillingCluster, false, isProxyMode, randomBoolean()); + } + + configureRemoteClusterCredentials("my_remote_cluster", (String) apiKeyMap.get("encoded")); + + // also valid to configure credentials, then cluster + if (false == configureSettingsFirst) { + configureRemoteCluster("my_remote_cluster"); + } else { + // now that credentials are configured, we expect a successful connection + checkRemoteConnection("my_remote_cluster", fulfillingCluster, false, isProxyMode); + } + + assertOK( + performRequestWithRemoteSearchUser( + new Request("GET", String.format(Locale.ROOT, "/my_remote_cluster:*/_search?ccs_minimize_roundtrips=%s", randomBoolean())) + ) + ); + } + + private void configureRemoteClusterCredentials(String clusterAlias, String credentials) throws IOException { + keystoreSettings.put("cluster.remote." + clusterAlias + ".credentials", credentials); + queryCluster.updateStoredSecureSettings(); + assertOK(adminClient().performRequest(new Request("POST", "/_nodes/reload_secure_settings"))); + } + + private Response performRequestWithRemoteSearchUser(final Request request) throws IOException { + request.setOptions( + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(REMOTE_SEARCH_USER, PASS)) + ); + return client().performRequest(request); + } + +}