Browse Source

Remove the client transport profile filter (#43236)

Now that the transport client has been removed, the client transport
profile filter can be removed from security. This filter prevented node
actions from being executed using a transport client.
Jay Modi 6 years ago
parent
commit
0a41b13cd8

+ 8 - 0
docs/reference/migration/migrate_8_0/security.asciidoc

@@ -33,3 +33,11 @@ The `elasticsearch-migrate` tool provided a way to convert file
 realm users and roles into the native realm. It has been deprecated
 since 7.2.0. Users and roles should now be created in the native
 realm directly.
+
+[float]
+[[separating-node-and-client-traffic]]
+==== The `transport.profiles.*.xpack.security.type` setting has been removed
+
+The `transport.profiles.*.xpack.security.type` setting has been removed since
+the Transport Client has been removed and therefore all client traffic now uses
+the HTTP transport. Transport profiles using this setting should be removed.

+ 0 - 68
docs/reference/security/securing-communications/separating-node-client-traffic.asciidoc

@@ -1,68 +0,0 @@
-[role="xpack"]
-[[separating-node-client-traffic]]
-=== Separating node-to-node and client traffic
-
-Elasticsearch has the feature of so called 
-{ref}/modules-transport.html[TCP transport profiles]
-that allows it to bind to several ports and addresses. The {es}
-{security-features} extend on this functionality to enhance the security of the
-cluster by enabling the separation of node-to-node transport traffic from client
-transport traffic. This is important if the client transport traffic is not
-trusted and could potentially be malicious. To separate the node-to-node traffic
-from the client traffic, add the following to `elasticsearch.yml`:
-
-[source, yaml]
---------------------------------------------------
-transport.profiles.client: <1>
-  port: 9500-9600 <2>
-  xpack.security:
-    type: client <3>
---------------------------------------------------
-<1> `client` is the name of this example profile
-<2> The port range that will be used by transport clients to communicate with
-    this cluster
-<3> Categorizes the profile as a `client`. This accounts for additional security
-    filters by denying request attempts on for internal cluster operations
-    (e.g shard level actions and ping requests) from this profile.
-
-If supported by your environment, an internal network can be used for node-to-node
-traffic and public network can be used for client traffic by adding the following
-to `elasticsearch.yml`:
-
-[source, yaml]
---------------------------------------------------
-transport.profiles.default.bind_host: 10.0.0.1 <1>
-transport.profiles.client.bind_host: 1.1.1.1 <2>
---------------------------------------------------
-<1> The bind address for the network that will be used for node-to-node communication
-<2> The bind address for the network used for client communication
-
-If separate networks are not available, then
-{stack-ov}/ip-filtering.html[IP Filtering] can
-be enabled to limit access to the profiles.
-
-When using SSL for transport, a different set of certificates can also be used
-for the client traffic by adding the following to `elasticsearch.yml`:
-
-[source, yaml]
---------------------------------------------------
-transport.profiles.client.xpack.security.ssl.truststore:
-  path: /path/to/another/truststore
-  password: x-pack-test-password
-
-transport.profiles.client.xpack.security.ssl.keystore:
-  path: /path/to/another/keystore
-  password: x-pack-test-password
---------------------------------------------------
-
-To change the default behavior that requires certificates for transport clients,
-set the following value in the `elasticsearch.yml` file:
-
-[source, yaml]
---------------------------------------------------
-transport.profiles.client.xpack.security.ssl.client_authentication: none
---------------------------------------------------
-
-This setting keeps certificate authentication active for node-to-node traffic,
-but removes the requirement to distribute a signed certificate to transport
-clients.

+ 0 - 3
x-pack/docs/en/security/configuring-es.asciidoc

@@ -148,9 +148,6 @@ include::{es-repo-dir}/security/securing-communications/configuring-tls-docker.a
 :edit_url: https://github.com/elastic/elasticsearch/edit/{branch}/docs/reference/security/securing-communications/enabling-cipher-suites.asciidoc
 include::{es-repo-dir}/security/securing-communications/enabling-cipher-suites.asciidoc[]
 
-:edit_url: https://github.com/elastic/elasticsearch/edit/{branch}/docs/reference/security/securing-communications/separating-node-client-traffic.asciidoc
-include::{es-repo-dir}/security/securing-communications/separating-node-client-traffic.asciidoc[]
-
 :edit_url:
 include::authentication/configuring-active-directory-realm.asciidoc[]
 include::authentication/configuring-file-realm.asciidoc[]

+ 0 - 5
x-pack/docs/en/security/securing-communications.asciidoc

@@ -24,8 +24,3 @@ include::{es-repo-dir}/security/securing-communications/setting-up-ssl.asciidoc[
 === Enabling cipher suites for stronger encryption
 
 See {ref}/ciphers.html[Enabling Cipher Suites for Stronger Encryption].
-
-[[separating-node-client-traffic]]
-=== Separating node-to-node and client traffic
-
-See {ref}/separating-node-client-traffic.html[Separating node-to-node and client traffic].

+ 0 - 1
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -604,7 +604,6 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw
         settingsList.add(TokenService.TOKEN_EXPIRATION);
         settingsList.add(TokenService.DELETE_INTERVAL);
         settingsList.add(TokenService.DELETE_TIMEOUT);
-        settingsList.add(SecurityServerTransportInterceptor.TRANSPORT_TYPE_PROFILE_SETTING);
         settingsList.addAll(SSLConfigurationSettings.getProfileSettings());
         settingsList.add(ApiKeyService.PASSWORD_HASHING_ALGORITHM);
         settingsList.add(ApiKeyService.DELETE_TIMEOUT);

+ 2 - 29
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java

@@ -12,7 +12,6 @@ import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.DestructiveOperations;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.CheckedConsumer;
-import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -45,24 +44,13 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.Executor;
-import java.util.function.Function;
 
 import static org.elasticsearch.xpack.core.security.SecurityField.setting;
 
 public class SecurityServerTransportInterceptor implements TransportInterceptor {
 
-    private static final Function<String, Setting<String>> TRANSPORT_TYPE_SETTING_TEMPLATE = key -> new Setting<>(key, "node", v -> {
-        if (v.equals("node") || v.equals("client")) {
-            return v;
-        }
-        throw new IllegalArgumentException("type must be one of [client, node]");
-    }, Setting.Property.NodeScope);
-    private static final String TRANSPORT_TYPE_SETTING_KEY = "xpack.security.type";
     private static final Logger logger = LogManager.getLogger(SecurityServerTransportInterceptor.class);
 
-    public static final Setting.AffixSetting<String> TRANSPORT_TYPE_PROFILE_SETTING = Setting.affixKeySetting("transport.profiles.",
-            TRANSPORT_TYPE_SETTING_KEY, TRANSPORT_TYPE_SETTING_TEMPLATE);
-
     private final AuthenticationService authcService;
     private final AuthorizationService authzService;
     private final SSLService sslService;
@@ -71,7 +59,6 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
     private final ThreadPool threadPool;
     private final Settings settings;
     private final SecurityContext securityContext;
-    private final boolean reservedRealmEnabled;
 
     private volatile boolean isStateNotRecovered = true;
 
@@ -92,7 +79,6 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
         this.sslService = sslService;
         this.securityContext = securityContext;
         this.profileFilters = initializeProfileFilters(destructiveOperations);
-        this.reservedRealmEnabled = XPackSettings.RESERVED_REALM_ENABLED_SETTING.get(settings);
         clusterService.addListener(e -> isStateNotRecovered = e.state().blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK));
     }
 
@@ -187,21 +173,8 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
         for (Map.Entry<String, SSLConfiguration> entry : profileConfigurations.entrySet()) {
             final SSLConfiguration profileConfiguration = entry.getValue();
             final boolean extractClientCert = transportSSLEnabled && sslService.isSSLClientAuthEnabled(profileConfiguration);
-            final String type = TRANSPORT_TYPE_PROFILE_SETTING.getConcreteSettingForNamespace(entry.getKey()).get(settings);
-            switch (type) {
-                case "client":
-                    profileFilters.put(entry.getKey(), new ServerTransportFilter.ClientProfile(authcService, authzService,
-                            threadPool.getThreadContext(), extractClientCert, destructiveOperations, reservedRealmEnabled,
-                            securityContext, licenseState));
-                    break;
-                case "node":
-                    profileFilters.put(entry.getKey(), new ServerTransportFilter.NodeProfile(authcService, authzService,
-                            threadPool.getThreadContext(), extractClientCert, destructiveOperations, reservedRealmEnabled,
-                            securityContext, licenseState));
-                    break;
-                default:
-                   throw new IllegalStateException("unknown profile type: " + type);
-            }
+            profileFilters.put(entry.getKey(), new ServerTransportFilter(authcService, authzService, threadPool.getThreadContext(),
+                extractClientCert, destructiveOperations, securityContext, licenseState));
         }
 
         return Collections.unmodifiableMap(profileFilters);

+ 69 - 116
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java

@@ -32,137 +32,90 @@ import org.elasticsearch.xpack.security.action.SecurityActionMapper;
 import org.elasticsearch.xpack.security.authc.AuthenticationService;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
 
-import java.io.IOException;
-
-import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
-
 /**
- * This interface allows to intercept messages as they come in and execute logic
- * This is used in x-pack security to execute the authentication/authorization on incoming
- * messages.
- * Note that this filter only applies for nodes, but not for clients.
+ * The server transport filter that should be used in nodes as it ensures that an incoming
+ * request is properly authenticated and authorized
  */
-public interface ServerTransportFilter {
+final class ServerTransportFilter {
+
+    private static final Logger logger = LogManager.getLogger(ServerTransportFilter.class);
+
+    private final AuthenticationService authcService;
+    private final AuthorizationService authzService;
+    private final SecurityActionMapper actionMapper = new SecurityActionMapper();
+    private final ThreadContext threadContext;
+    private final boolean extractClientCert;
+    private final DestructiveOperations destructiveOperations;
+    private final SecurityContext securityContext;
+    private final XPackLicenseState licenseState;
+
+    ServerTransportFilter(AuthenticationService authcService, AuthorizationService authzService,
+                ThreadContext threadContext, boolean extractClientCert, DestructiveOperations destructiveOperations,
+                SecurityContext securityContext, XPackLicenseState licenseState) {
+        this.authcService = authcService;
+        this.authzService = authzService;
+        this.threadContext = threadContext;
+        this.extractClientCert = extractClientCert;
+        this.destructiveOperations = destructiveOperations;
+        this.securityContext = securityContext;
+        this.licenseState = licenseState;
+    }
 
     /**
      * Called just after the given request was received by the transport. Any exception
      * thrown by this method will stop the request from being handled and the error will
      * be sent back to the sender.
      */
-    void inbound(String action, TransportRequest request, TransportChannel transportChannel, ActionListener<Void> listener)
-            throws IOException;
-
-    /**
-     * The server transport filter that should be used in nodes as it ensures that an incoming
-     * request is properly authenticated and authorized
-     */
-    class NodeProfile implements ServerTransportFilter {
-        private static final Logger logger = LogManager.getLogger(NodeProfile.class);
-
-        private final AuthenticationService authcService;
-        private final AuthorizationService authzService;
-        private final SecurityActionMapper actionMapper = new SecurityActionMapper();
-        private final ThreadContext threadContext;
-        private final boolean extractClientCert;
-        private final DestructiveOperations destructiveOperations;
-        private final boolean reservedRealmEnabled;
-        private final SecurityContext securityContext;
-        private final XPackLicenseState licenseState;
-
-        NodeProfile(AuthenticationService authcService, AuthorizationService authzService,
-                    ThreadContext threadContext, boolean extractClientCert, DestructiveOperations destructiveOperations,
-                    boolean reservedRealmEnabled, SecurityContext securityContext, XPackLicenseState licenseState) {
-            this.authcService = authcService;
-            this.authzService = authzService;
-            this.threadContext = threadContext;
-            this.extractClientCert = extractClientCert;
-            this.destructiveOperations = destructiveOperations;
-            this.reservedRealmEnabled = reservedRealmEnabled;
-            this.securityContext = securityContext;
-            this.licenseState = licenseState;
-        }
-
-        @Override
-        public void inbound(String action, TransportRequest request, TransportChannel transportChannel, ActionListener<Void> listener)
-                throws IOException {
-            if (CloseIndexAction.NAME.equals(action) || OpenIndexAction.NAME.equals(action) || DeleteIndexAction.NAME.equals(action)) {
-                IndicesRequest indicesRequest = (IndicesRequest) request;
-                try {
-                    destructiveOperations.failDestructive(indicesRequest.indices());
-                } catch(IllegalArgumentException e) {
-                    listener.onFailure(e);
-                    return;
-                }
-            }
-            /*
-             here we don't have a fallback user, as all incoming request are
-             expected to have a user attached (either in headers or in context)
-             We can make this assumption because in nodes we make sure all outgoing
-             requests from all the nodes are attached with a user (either a serialize
-             user an authentication token
-             */
-            String securityAction = actionMapper.action(action, request);
-
-            TransportChannel unwrappedChannel = transportChannel;
-            if (unwrappedChannel instanceof TaskTransportChannel) {
-                unwrappedChannel = ((TaskTransportChannel) unwrappedChannel).getChannel();
+    void inbound(String action, TransportRequest request, TransportChannel transportChannel,ActionListener<Void> listener) {
+        if (CloseIndexAction.NAME.equals(action) || OpenIndexAction.NAME.equals(action) || DeleteIndexAction.NAME.equals(action)) {
+            IndicesRequest indicesRequest = (IndicesRequest) request;
+            try {
+                destructiveOperations.failDestructive(indicesRequest.indices());
+            } catch(IllegalArgumentException e) {
+                listener.onFailure(e);
+                return;
             }
+        }
+        /*
+         here we don't have a fallback user, as all incoming request are
+         expected to have a user attached (either in headers or in context)
+         We can make this assumption because in nodes we make sure all outgoing
+         requests from all the nodes are attached with a user (either a serialize
+         user an authentication token
+         */
+        String securityAction = actionMapper.action(action, request);
+
+        TransportChannel unwrappedChannel = transportChannel;
+        if (unwrappedChannel instanceof TaskTransportChannel) {
+            unwrappedChannel = ((TaskTransportChannel) unwrappedChannel).getChannel();
+        }
 
-            if (extractClientCert && (unwrappedChannel instanceof TcpTransportChannel)) {
-                TcpChannel tcpChannel = ((TcpTransportChannel) unwrappedChannel).getChannel();
-                if (tcpChannel instanceof Netty4TcpChannel || tcpChannel instanceof NioTcpChannel) {
-                    if (tcpChannel.isOpen()) {
-                        SSLEngineUtils.extractClientCertificates(logger, threadContext, tcpChannel);
-                    }
+        if (extractClientCert && (unwrappedChannel instanceof TcpTransportChannel)) {
+            TcpChannel tcpChannel = ((TcpTransportChannel) unwrappedChannel).getChannel();
+            if (tcpChannel instanceof Netty4TcpChannel || tcpChannel instanceof NioTcpChannel) {
+                if (tcpChannel.isOpen()) {
+                    SSLEngineUtils.extractClientCertificates(logger, threadContext, tcpChannel);
                 }
             }
+        }
 
-            final Version version = transportChannel.getVersion();
-            authcService.authenticate(securityAction, request, (User)null, ActionListener.wrap((authentication) -> {
-                if (authentication != null) {
-                    if (securityAction.equals(TransportService.HANDSHAKE_ACTION_NAME) &&
-                        SystemUser.is(authentication.getUser()) == false) {
-                        securityContext.executeAsUser(SystemUser.INSTANCE, (ctx) -> {
-                            final Authentication replaced = Authentication.getAuthentication(threadContext);
-                            authzService.authorize(replaced, securityAction, request, listener);
-                        }, version);
-                    } else {
-                        authzService.authorize(authentication, securityAction, request, listener);
-                    }
-                } else if (licenseState.isAuthAllowed() == false) {
-                    listener.onResponse(null);
+        final Version version = transportChannel.getVersion();
+        authcService.authenticate(securityAction, request, (User)null, ActionListener.wrap((authentication) -> {
+            if (authentication != null) {
+                if (securityAction.equals(TransportService.HANDSHAKE_ACTION_NAME) &&
+                    SystemUser.is(authentication.getUser()) == false) {
+                    securityContext.executeAsUser(SystemUser.INSTANCE, (ctx) -> {
+                        final Authentication replaced = Authentication.getAuthentication(threadContext);
+                        authzService.authorize(replaced, securityAction, request, listener);
+                    }, version);
                 } else {
-                    listener.onFailure(new IllegalStateException("no authentication present but auth is allowed"));
+                    authzService.authorize(authentication, securityAction, request, listener);
                 }
-            }, listener::onFailure));
-        }
-    }
-
-    /**
-     * A server transport filter rejects internal calls, which should be used on connections
-     * where only clients connect to. This ensures that no client can send any internal actions
-     * or shard level actions. As it extends the NodeProfile the authentication/authorization is
-     * done as well
-     */
-    class ClientProfile extends NodeProfile {
-
-        ClientProfile(AuthenticationService authcService, AuthorizationService authzService,
-                             ThreadContext threadContext, boolean extractClientCert, DestructiveOperations destructiveOperations,
-                             boolean reservedRealmEnabled, SecurityContext securityContext, XPackLicenseState licenseState) {
-            super(authcService, authzService, threadContext, extractClientCert, destructiveOperations, reservedRealmEnabled,
-                securityContext, licenseState);
-        }
-
-        @Override
-        public void inbound(String action, TransportRequest request, TransportChannel transportChannel, ActionListener<Void> listener)
-                throws IOException {
-            // TODO is ']' sufficient to mark as shard action?
-            final boolean isInternalOrShardAction = action.startsWith("internal:") || action.endsWith("]");
-            if (isInternalOrShardAction && TransportService.HANDSHAKE_ACTION_NAME.equals(action) == false) {
-                throw authenticationError("executing internal/shard actions is considered malicious and forbidden");
+            } else if (licenseState.isAuthAllowed() == false) {
+                listener.onResponse(null);
+            } else {
+                listener.onFailure(new IllegalStateException("no authentication present but auth is allowed"));
             }
-            super.inbound(action, request, transportChannel, listener);
-        }
+        }, listener::onFailure));
     }
-
 }

+ 0 - 208
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ServerTransportFilterIntegrationTests.java

@@ -1,208 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.security.transport;
-
-import org.elasticsearch.ElasticsearchSecurityException;
-import org.elasticsearch.Version;
-import org.elasticsearch.cluster.action.index.NodeMappingRefreshAction;
-import org.elasticsearch.cluster.node.DiscoveryNode;
-import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.common.network.NetworkAddress;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.transport.TransportAddress;
-import org.elasticsearch.node.MockNode;
-import org.elasticsearch.node.Node;
-import org.elasticsearch.node.NodeValidationException;
-import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.test.MockHttpTransport;
-import org.elasticsearch.test.SecurityIntegTestCase;
-import org.elasticsearch.test.SecuritySettingsSourceField;
-import org.elasticsearch.threadpool.ThreadPool;
-import org.elasticsearch.transport.ConnectionProfile;
-import org.elasticsearch.transport.Transport;
-import org.elasticsearch.transport.TransportException;
-import org.elasticsearch.transport.TransportRequestOptions;
-import org.elasticsearch.transport.TransportResponse;
-import org.elasticsearch.transport.TransportResponseHandler;
-import org.elasticsearch.transport.TransportService;
-import org.elasticsearch.xpack.core.XPackSettings;
-import org.elasticsearch.xpack.core.security.SecurityField;
-import org.elasticsearch.xpack.security.LocalStateSecurity;
-import org.junit.BeforeClass;
-
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.concurrent.CountDownLatch;
-
-import static org.elasticsearch.discovery.SettingsBasedSeedHostsProvider.DISCOVERY_SEED_HOSTS_SETTING;
-import static org.elasticsearch.test.SecuritySettingsSource.addSSLSettingsForNodePEMFiles;
-import static org.elasticsearch.test.SecuritySettingsSource.addSSLSettingsForPEMFiles;
-import static org.elasticsearch.xpack.security.test.SecurityTestUtils.writeFile;
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.instanceOf;
-
-public class ServerTransportFilterIntegrationTests extends SecurityIntegTestCase {
-    private static int randomClientPort;
-
-    @BeforeClass
-    public static void getRandomPort() {
-        randomClientPort = randomIntBetween(49000, 65500); // ephemeral port
-    }
-
-    @Override
-    public boolean transportSSLEnabled() {
-        return true;
-    }
-
-    @Override
-    protected Settings nodeSettings(int nodeOrdinal) {
-        Settings.Builder settingsBuilder = Settings.builder().put(super.nodeSettings(nodeOrdinal));
-        String randomClientPortRange = randomClientPort + "-" + (randomClientPort+100);
-        addSSLSettingsForNodePEMFiles(settingsBuilder, "transport.profiles.client.xpack.security.", true);
-        Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt");
-        settingsBuilder.putList("transport.profiles.client.xpack.security.ssl.certificate_authorities",
-            Collections.singletonList(certPath.toString())) // settings for client truststore
-            .put("transport.profiles.client.xpack.security.type", "client")
-            .put("transport.profiles.client.port", randomClientPortRange)
-            // make sure this is "localhost", no matter if ipv4 or ipv6, but be consistent
-            .put("transport.profiles.client.bind_host", "localhost")
-            .put("xpack.security.audit.enabled", false)
-            .put(XPackSettings.WATCHER_ENABLED.getKey(), false);
-        if (randomBoolean()) {
-            settingsBuilder.put("transport.profiles.default.xpack.security.type", "node"); // this is default lets set it randomly
-        }
-
-        return settingsBuilder.build();
-    }
-
-    public void testThatConnectionToServerTypeConnectionWorks() throws IOException, NodeValidationException {
-        Path home = createTempDir();
-        Path xpackConf = home.resolve("config");
-        Files.createDirectories(xpackConf);
-
-        Transport transport = internalCluster().getMasterNodeInstance(Transport.class);
-        TransportAddress transportAddress = transport.boundAddress().publishAddress();
-        String unicastHost = NetworkAddress.format(transportAddress.address());
-
-        // test that starting up a node works
-        Settings.Builder nodeSettings = Settings.builder()
-            .put("node.name", "my-test-node")
-            .put("network.host", "localhost")
-            .put("cluster.name", internalCluster().getClusterName())
-            .put(DISCOVERY_SEED_HOSTS_SETTING.getKey(), unicastHost)
-            .put("xpack.security.enabled", true)
-            .put("xpack.security.audit.enabled", false)
-            .put("xpack.security.transport.ssl.enabled", true)
-            .put(XPackSettings.WATCHER_ENABLED.getKey(), false)
-            .put("path.home", home)
-            .put(Node.NODE_MASTER_SETTING.getKey(), false);
-        Collection<Class<? extends Plugin>> mockPlugins = Arrays.asList(LocalStateSecurity.class, MockHttpTransport.TestPlugin.class);
-        addSSLSettingsForPEMFiles(
-            nodeSettings,
-            "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem",
-            "testnode",
-            "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt",
-            List.of("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt",
-                "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode_ec.crt"));
-        try (Node node = new MockNode(nodeSettings.build(), mockPlugins)) {
-            node.start();
-            ensureStableCluster(cluster().size() + 1);
-        }
-    }
-
-    public void testThatConnectionToClientTypeConnectionIsRejected() throws IOException, NodeValidationException, InterruptedException {
-        Path home = createTempDir();
-        Path xpackConf = home.resolve("config");
-        Files.createDirectories(xpackConf);
-        writeFile(xpackConf, "users", configUsers());
-        writeFile(xpackConf, "users_roles", configUsersRoles());
-        writeFile(xpackConf, "roles.yml", configRoles());
-
-        Transport transport = internalCluster().getMasterNodeInstance(Transport.class);
-        TransportAddress transportAddress = transport.profileBoundAddresses().get("client").publishAddress();
-        String unicastHost = NetworkAddress.format(transportAddress.address());
-
-        // test that starting up a node works
-        Settings.Builder nodeSettings = Settings.builder()
-            .put("xpack.security.authc.realms.file.file.order", 0)
-            .put("node.name", "my-test-node")
-            .put(SecurityField.USER_SETTING.getKey(), "test_user:" + SecuritySettingsSourceField.TEST_PASSWORD)
-            .put("cluster.name", internalCluster().getClusterName())
-            .put(DISCOVERY_SEED_HOSTS_SETTING.getKey(), unicastHost)
-            .put("xpack.security.enabled", true)
-            .put("xpack.security.audit.enabled", false)
-            .put("xpack.security.transport.ssl.enabled", true)
-            .put(XPackSettings.WATCHER_ENABLED.getKey(), false)
-            .put("discovery.initial_state_timeout", "0s")
-            .put("path.home", home)
-            .put(Node.NODE_MASTER_SETTING.getKey(), false);
-        Collection<Class<? extends Plugin>> mockPlugins = Arrays.asList(LocalStateSecurity.class, MockHttpTransport.TestPlugin.class);
-        addSSLSettingsForPEMFiles(
-            nodeSettings,
-            "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem",
-            "testnode",
-            "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt",
-            List.of("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt",
-                "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode_ec.crt"));
-        try (Node node = new MockNode(nodeSettings.build(), mockPlugins)) {
-            node.start();
-            TransportService instance = node.injector().getInstance(TransportService.class);
-            try (Transport.Connection connection = instance.openConnection(new DiscoveryNode("theNode", transportAddress, Version.CURRENT),
-                    ConnectionProfile.buildSingleChannelProfile(TransportRequestOptions.Type.REG))) {
-                // handshake should be ok
-                final DiscoveryNode handshake = instance.handshake(connection, 10000);
-                assertEquals(transport.boundAddress().publishAddress(), handshake.getAddress());
-                CountDownLatch latch = new CountDownLatch(1);
-                instance.sendRequest(connection, NodeMappingRefreshAction.ACTION_NAME,
-                        new NodeMappingRefreshAction.NodeMappingRefreshRequest("foo", "bar", "baz"),
-                        TransportRequestOptions.EMPTY,
-                        new TransportResponseHandler<TransportResponse>() {
-                    @Override
-                    public TransportResponse read(StreamInput in) {
-                        try {
-                            fail("never get that far");
-                        } finally {
-                            latch.countDown();
-                        }
-                        return null;
-                    }
-
-                    @Override
-                    public void handleResponse(TransportResponse response) {
-                        try {
-                            fail("never get that far");
-                        } finally {
-                            latch.countDown();
-                        }
-                    }
-
-                    @Override
-                    public void handleException(TransportException exp) {
-                        try {
-                            assertThat(exp.getCause(), instanceOf(ElasticsearchSecurityException.class));
-                            assertThat(exp.getCause().getMessage(),
-                                    equalTo("executing internal/shard actions is considered malicious and forbidden"));
-                        } finally {
-                            latch.countDown();
-                        }
-                    }
-
-                    @Override
-                    public String executor() {
-                        return ThreadPool.Names.SAME;
-                    }
-                });
-                latch.await();
-            }
-        }
-    }
-
-}

+ 9 - 34
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ServerTransportFilterTests.java

@@ -33,7 +33,6 @@ import org.elasticsearch.xpack.security.authc.AuthenticationService;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
 import org.junit.Before;
 
-import java.io.IOException;
 import java.util.Collections;
 
 import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
@@ -83,7 +82,7 @@ public class ServerTransportFilterTests extends ESTestCase {
             callback.onResponse(authentication);
             return Void.TYPE;
         }).when(authcService).authenticate(eq("_action"), eq(request), eq((User)null), any(ActionListener.class));
-        ServerTransportFilter filter = getClientOrNodeFilter();
+        ServerTransportFilter filter = getNodeFilter();
         PlainActionFuture<Void> future = new PlainActionFuture<>();
         filter.inbound("_action", request, channel, future);
         //future.get(); // don't block it's not called really just mocked
@@ -104,7 +103,7 @@ public class ServerTransportFilterTests extends ESTestCase {
             callback.onResponse(authentication);
             return Void.TYPE;
         }).when(authcService).authenticate(eq(action), eq(request), eq((User)null), any(ActionListener.class));
-        ServerTransportFilter filter = getClientOrNodeFilter();
+        ServerTransportFilter filter = getNodeFilter();
         PlainActionFuture listener = mock(PlainActionFuture.class);
         filter.inbound(action, request, channel, listener);
         if (failDestructiveOperations) {
@@ -124,7 +123,7 @@ public class ServerTransportFilterTests extends ESTestCase {
             callback.onFailure(authE);
             return Void.TYPE;
         }).when(authcService).authenticate(eq("_action"), eq(request), eq((User)null), any(ActionListener.class));
-        ServerTransportFilter filter = getClientOrNodeFilter();
+        ServerTransportFilter filter = getNodeFilter();
         try {
             PlainActionFuture<Void> future = new PlainActionFuture<>();
             filter.inbound("_action", request, channel, future);
@@ -137,7 +136,7 @@ public class ServerTransportFilterTests extends ESTestCase {
     }
 
     public void testInboundAuthorizationException() throws Exception {
-        ServerTransportFilter filter = getClientOrNodeFilter();
+        ServerTransportFilter filter = getNodeFilter();
         TransportRequest request = mock(TransportRequest.class);
         Authentication authentication = mock(Authentication.class);
         doAnswer((i) -> {
@@ -158,23 +157,10 @@ public class ServerTransportFilterTests extends ESTestCase {
         assertThat(e.getMessage(), equalTo("authz failed"));
     }
 
-    public void testClientProfileRejectsNodeActions() throws Exception {
-        TransportRequest request = mock(TransportRequest.class);
-        ServerTransportFilter filter = getClientFilter(true);
-        ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class,
-                () -> filter.inbound("internal:foo/bar", request, channel, new PlainActionFuture<>()));
-        assertEquals("executing internal/shard actions is considered malicious and forbidden", e.getMessage());
-        e = expectThrows(ElasticsearchSecurityException.class,
-                () -> filter.inbound("indices:action" + randomFrom("[s]", "[p]", "[r]", "[n]", "[s][p]", "[s][r]", "[f]"),
-                        request, channel, new PlainActionFuture<>()));
-        assertEquals("executing internal/shard actions is considered malicious and forbidden", e.getMessage());
-        verifyZeroInteractions(authcService);
-    }
-
-    public void testNodeProfileAllowsNodeActions() throws Exception {
+    public void testAllowsNodeActions() throws Exception {
         final String internalAction = "internal:foo/bar";
         final String nodeOrShardAction = "indices:action" + randomFrom("[s]", "[p]", "[r]", "[n]", "[s][p]", "[s][r]", "[f]");
-        ServerTransportFilter filter = getNodeFilter(true);
+        ServerTransportFilter filter = getNodeFilter();
         TransportRequest request = mock(TransportRequest.class);
         Authentication authentication = new Authentication(new User("test", "superuser"), new RealmRef("test", "test", "node1"), null);
         doAnswer((i) -> {
@@ -200,21 +186,10 @@ public class ServerTransportFilterTests extends ESTestCase {
         verifyNoMoreInteractions(authcService, authzService);
     }
 
-    private ServerTransportFilter getClientOrNodeFilter() throws IOException {
-        return randomBoolean() ? getNodeFilter(true) : getClientFilter(true);
-    }
-
-    private ServerTransportFilter.ClientProfile getClientFilter(boolean reservedRealmEnabled) throws IOException {
-        Settings settings = Settings.builder().put("path.home", createTempDir()).build();
-        ThreadContext threadContext = new ThreadContext(settings);
-        return new ServerTransportFilter.ClientProfile(authcService, authzService, threadContext, false, destructiveOperations,
-                reservedRealmEnabled, new SecurityContext(settings, threadContext), new XPackLicenseState(settings));
-    }
-
-    private ServerTransportFilter.NodeProfile getNodeFilter(boolean reservedRealmEnabled) throws IOException {
+    private ServerTransportFilter getNodeFilter() {
         Settings settings = Settings.builder().put("path.home", createTempDir()).build();
         ThreadContext threadContext = new ThreadContext(settings);
-        return new ServerTransportFilter.NodeProfile(authcService, authzService, threadContext, false, destructiveOperations,
-                reservedRealmEnabled, new SecurityContext(settings, threadContext), new XPackLicenseState(settings));
+        return new ServerTransportFilter(authcService, authzService, threadContext, false, destructiveOperations,
+                new SecurityContext(settings, threadContext), new XPackLicenseState(settings));
     }
 }