Security: fix joining cluster with production license (#31341)

The changes made to disable security for trial licenses unless security
is explicitly enabled caused issues when a 6.3 node attempts to join a
cluster that already has a production license installed. The new node
starts off with a trial license and `xpack.security.enabled` is not
set for the node, which causes the security code to skip attaching the
user to the request. The existing cluster has security enabled and the
lack of a user attached to the requests causes the request to be
rejected.

This commit changes the security code to check if the state has been
recovered yet when making the decision on whether or not to attach a
user. If the state has not yet been recovered, the code will attach
the user to the request in case security is enabled on the cluster
being joined.

Closes #31332

Author: jaymode

x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java

@@ -254,7 +254,11 @@ private static class Status {
 
     public XPackLicenseState(Settings settings) {
         this.isSecurityEnabled = XPackSettings.SECURITY_ENABLED.get(settings);
-        this.isSecurityExplicitlyEnabled = settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) && isSecurityEnabled;
+        // 6.0+ requires TLS for production licenses, so if TLS is enabled and security is enabled
+        // we can interpret this as an explicit enabling of security if the security enabled
+        // setting is not explicitly set
+        this.isSecurityExplicitlyEnabled = isSecurityEnabled &&
+            (settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) || XPackSettings.TRANSPORT_SSL_ENABLED.get(settings));
     }
 
     /** Updates the current state of the license, which will change what features are available. */

x-pack/plugin/core/src/test/java/org/elasticsearch/license/XPackLicenseStateTests.java

@@ -79,6 +79,16 @@ public void testSecurityDefaults() {
         assertThat(licenseState.allowedRealmType(), is(XPackLicenseState.AllowedRealmType.ALL));
         assertThat(licenseState.isCustomRoleProvidersAllowed(), is(true));
 
+        licenseState =
+            new XPackLicenseState(Settings.builder().put(XPackSettings.TRANSPORT_SSL_ENABLED.getKey(), true).build());
+        assertThat(licenseState.isAuthAllowed(), is(true));
+        assertThat(licenseState.isIpFilteringAllowed(), is(true));
+        assertThat(licenseState.isAuditingAllowed(), is(true));
+        assertThat(licenseState.isStatsAndHealthAllowed(), is(true));
+        assertThat(licenseState.isDocumentAndFieldLevelSecurityAllowed(), is(true));
+        assertThat(licenseState.allowedRealmType(), is(XPackLicenseState.AllowedRealmType.ALL));
+        assertThat(licenseState.isCustomRoleProvidersAllowed(), is(true));
+
         licenseState = new XPackLicenseState(Settings.EMPTY);
         assertThat(licenseState.isAuthAllowed(), is(true));
         assertThat(licenseState.isIpFilteringAllowed(), is(true));

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

@@ -492,8 +492,8 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
         ipFilter.set(new IPFilter(settings, auditTrailService, clusterService.getClusterSettings(), getLicenseState()));
         components.add(ipFilter.get());
         DestructiveOperations destructiveOperations = new DestructiveOperations(settings, clusterService.getClusterSettings());
-        securityInterceptor.set(new SecurityServerTransportInterceptor(settings, threadPool, authcService.get(), authzService, getLicenseState(),
-                getSslService(), securityContext.get(), destructiveOperations));
+        securityInterceptor.set(new SecurityServerTransportInterceptor(settings, threadPool, authcService.get(),
+                authzService, getLicenseState(), getSslService(), securityContext.get(), destructiveOperations, clusterService));
 
         final Set<RequestInterceptor> requestInterceptors;
         if (XPackSettings.DLS_FLS_ENABLED.get(settings)) {

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

@@ -9,12 +9,14 @@
 import org.elasticsearch.Version;
 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.component.AbstractComponent;
 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;
+import org.elasticsearch.gateway.GatewayService;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
@@ -72,14 +74,17 @@ public class SecurityServerTransportInterceptor extends AbstractComponent implem
     private final SecurityContext securityContext;
     private final boolean reservedRealmEnabled;
 
+    private volatile boolean isStateNotRecovered = true;
+
     public SecurityServerTransportInterceptor(Settings settings,
                                               ThreadPool threadPool,
                                               AuthenticationService authcService,
                                               AuthorizationService authzService,
                                               XPackLicenseState licenseState,
                                               SSLService sslService,
                                               SecurityContext securityContext,
-                                              DestructiveOperations destructiveOperations) {
+                                              DestructiveOperations destructiveOperations,
+                                              ClusterService clusterService) {
         super(settings);
         this.settings = settings;
         this.threadPool = threadPool;
@@ -90,6 +95,7 @@ public SecurityServerTransportInterceptor(Settings settings,
         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));
     }
 
     @Override
@@ -98,7 +104,13 @@ public AsyncSender interceptSender(AsyncSender sender) {
             @Override
             public <T extends TransportResponse> void sendRequest(Transport.Connection connection, String action, TransportRequest request,
                                                                   TransportRequestOptions options, TransportResponseHandler<T> handler) {
-                if (licenseState.isSecurityEnabled() && licenseState.isAuthAllowed()) {
+                // make a local copy of isStateNotRecovered as this is a volatile variable and it
+                // is used multiple times in the method. The copy to a local variable allows us to
+                // guarantee we use the same value wherever we would check the value for the state
+                // being recovered
+                final boolean stateNotRecovered = isStateNotRecovered;
+                final boolean sendWithAuth = (licenseState.isSecurityEnabled() && licenseState.isAuthAllowed()) || stateNotRecovered;
+                if (sendWithAuth) {
                     // the transport in core normally does this check, BUT since we are serializing to a string header we need to do it
                     // ourselves otherwise we wind up using a version newer than what we can actually send
                     final Version minVersion = Version.min(connection.getVersion(), Version.CURRENT);
@@ -108,20 +120,20 @@ public <T extends TransportResponse> void sendRequest(Transport.Connection conne
                     if (AuthorizationUtils.shouldReplaceUserWithSystem(threadPool.getThreadContext(), action)) {
                         securityContext.executeAsUser(SystemUser.INSTANCE, (original) -> sendWithUser(connection, action, request, options,
                                 new ContextRestoreResponseHandler<>(threadPool.getThreadContext().wrapRestorable(original)
-                                        , handler), sender), minVersion);
+                                        , handler), sender, stateNotRecovered), minVersion);
                     } else if (AuthorizationUtils.shouldSetUserBasedOnActionOrigin(threadPool.getThreadContext())) {
                         AuthorizationUtils.switchUserBasedOnActionOriginAndExecute(threadPool.getThreadContext(), securityContext,
                                 (original) -> sendWithUser(connection, action, request, options,
                                         new ContextRestoreResponseHandler<>(threadPool.getThreadContext().wrapRestorable(original)
-                                                , handler), sender));
+                                                , handler), sender, stateNotRecovered));
                     } else if (securityContext.getAuthentication() != null &&
                             securityContext.getAuthentication().getVersion().equals(minVersion) == false) {
                         // re-write the authentication since we want the authentication version to match the version of the connection
                         securityContext.executeAfterRewritingAuthentication(original -> sendWithUser(connection, action, request, options,
-                            new ContextRestoreResponseHandler<>(threadPool.getThreadContext().wrapRestorable(original), handler), sender),
-                            minVersion);
+                            new ContextRestoreResponseHandler<>(threadPool.getThreadContext().wrapRestorable(original), handler), sender,
+                            stateNotRecovered), minVersion);
                     } else {
-                        sendWithUser(connection, action, request, options, handler, sender);
+                        sendWithUser(connection, action, request, options, handler, sender, stateNotRecovered);
                     }
                 } else {
                     sender.sendRequest(connection, action, request, options, handler);
@@ -132,9 +144,10 @@ public <T extends TransportResponse> void sendRequest(Transport.Connection conne
 
     private <T extends TransportResponse> void sendWithUser(Transport.Connection connection, String action, TransportRequest request,
                                                             TransportRequestOptions options, TransportResponseHandler<T> handler,
-                                                            AsyncSender sender) {
-        // There cannot be a request outgoing from this node that is not associated with a user.
-        if (securityContext.getAuthentication() == null) {
+                                                            AsyncSender sender, final boolean stateNotRecovered) {
+        // There cannot be a request outgoing from this node that is not associated with a user
+        // unless we do not know the actual license of the cluster
+        if (securityContext.getAuthentication() == null && stateNotRecovered == false) {
             // we use an assertion here to ensure we catch this in our testing infrastructure, but leave the ISE for cases we do not catch
             // in tests and may be hit by a user
             assertNoAuthentication(action);

x-pack/plugin/security/src/test/java/org/elasticsearch/license/LicensingTests.java

@@ -24,11 +24,15 @@
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.discovery.DiscoveryModule;
+import org.elasticsearch.node.MockNode;
+import org.elasticsearch.node.Node;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.SecurityIntegTestCase;
 import org.elasticsearch.test.SecuritySettingsSource;
 import org.elasticsearch.test.SecuritySettingsSourceField;
+import org.elasticsearch.test.discovery.TestZenDiscovery;
 import org.elasticsearch.test.junit.annotations.TestLogging;
 import org.elasticsearch.transport.Netty4Plugin;
 import org.elasticsearch.transport.Transport;
@@ -42,7 +46,10 @@
 import org.junit.After;
 import org.junit.Before;
 
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -108,6 +115,7 @@ protected String configUsersRoles() {
     public Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder().put(super.nodeSettings(nodeOrdinal))
                 .put(NetworkModule.HTTP_ENABLED.getKey(), true)
+            .put(TestZenDiscovery.USE_MOCK_PINGS.getKey(), false)
                 .build();
     }
 
@@ -118,6 +126,11 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
         return plugins;
     }
 
+    @Override
+    protected int maxNumberOfNodes() {
+        return super.maxNumberOfNodes() + 1;
+    }
+
     @Before
     public void resetLicensing() {
         enableLicensing();
@@ -253,6 +266,33 @@ public void testTransportClientAuthenticationByLicenseType() throws Exception {
         }
     }
 
+    public void testNodeJoinWithoutSecurityExplicitlyEnabled() throws Exception {
+        License.OperationMode mode = randomFrom(License.OperationMode.GOLD, License.OperationMode.PLATINUM, License.OperationMode.STANDARD);
+        enableLicensing(mode);
+        ensureGreen();
+
+        Path home = createTempDir();
+        Path conf = home.resolve("config");
+        Files.createDirectories(conf);
+        Settings nodeSettings = Settings.builder()
+            .put(nodeSettings(maxNumberOfNodes() - 1).filter(s -> "xpack.security.enabled".equals(s) == false))
+            .put("node.name", "my-test-node")
+            .put("network.host", "localhost")
+            .put("cluster.name", internalCluster().getClusterName())
+            .put("discovery.zen.minimum_master_nodes",
+                internalCluster().getInstance(Settings.class).get("discovery.zen.minimum_master_nodes"))
+            .put("path.home", home)
+            .put(TestZenDiscovery.USE_MOCK_PINGS.getKey(), false)
+            .put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), "test-zen")
+            .put(DiscoveryModule.DISCOVERY_HOSTS_PROVIDER_SETTING.getKey(), "test-zen")
+            .build();
+        Collection<Class<? extends Plugin>> mockPlugins = Arrays.asList(LocalStateSecurity.class, TestZenDiscovery.TestPlugin.class);
+        try (Node node = new MockNode(nodeSettings, mockPlugins)) {
+            node.start();
+            ensureStableCluster(cluster().size() + 1);
+        }
+    }
+
     private static void assertElasticsearchSecurityException(ThrowingRunnable runnable) {
         ElasticsearchSecurityException ee = expectThrows(ElasticsearchSecurityException.class, runnable);
         assertThat(ee.getMetadata(LicenseUtils.EXPIRED_FEATURE_METADATA), hasItem(XPackField.SECURITY));