Security: fix joining cluster with production license

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

@@ -472,7 +472,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
         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));
+                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

@@ -23,11 +23,16 @@
 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.MockHttpTransport;
 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;
@@ -41,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;
@@ -115,6 +123,18 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
         return plugins;
     }
 
+    @Override
+    protected int maxNumberOfNodes() {
+        return super.maxNumberOfNodes() + 1;
+    }
+
+    @Override
+    public Settings nodeSettings(int nodeOrdinal) {
+        return Settings.builder().put(super.nodeSettings(nodeOrdinal))
+            .put(TestZenDiscovery.USE_MOCK_PINGS.getKey(), false)
+            .build();
+    }
+
     @Before
     public void resetLicensing() {
         enableLicensing();
@@ -250,6 +270,34 @@ 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,
+            MockHttpTransport.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));