Startup check for security implicit behavior change

server/src/main/java/org/elasticsearch/env/NodeEnvironment.java

@@ -484,6 +484,23 @@ private static NodeMetadata loadNodeMetadata(Settings settings, Logger logger,
         return metadata;
     }
 
+    /**
+     * loads existing node metadata from disk without attempting to upgrade to current version
+     */
+    public NodeMetadata loadLastKnownMetadata() {
+        try {
+            final Path path = nodePath.path;
+            NodeMetadata metadata = PersistedClusterStateService.nodeMetadata(path);
+            if (metadata == null) {
+                return NodeMetadata.FORMAT.loadLatestState(logger, NamedXContentRegistry.EMPTY, path);
+            }
+            return metadata;
+        } catch (IOException e) {
+            logger.warn("Failed to load node metadata from disk", e);
+            return null;
+        }
+    }
+
     public static String generateNodeId(Settings settings) {
         Random random = Randomness.get(settings, NODE_ID_SEED_SETTING);
         return UUIDs.randomBase64UUID(random);

server/src/main/java/org/elasticsearch/env/NodeMetadata.java

@@ -28,35 +28,43 @@ public final class NodeMetadata {
 
     static final String NODE_ID_KEY = "node_id";
     static final String NODE_VERSION_KEY = "node_version";
+    static final String PREVIOUS_NODE_VERSION_KEY = "previous_node_version";
 
     private final String nodeId;
 
     private final Version nodeVersion;
 
-    public NodeMetadata(final String nodeId, final Version nodeVersion) {
+    private final Version previousNodeVersion;
+
+    public NodeMetadata(final String nodeId, final Version nodeVersion, final Version previousNodeVersion) {
         this.nodeId = Objects.requireNonNull(nodeId);
         this.nodeVersion = Objects.requireNonNull(nodeVersion);
+        this.previousNodeVersion = Objects.requireNonNull(previousNodeVersion);
     }
 
-    @Override
-    public boolean equals(Object o) {
+    public NodeMetadata(final String nodeId, final Version nodeVersion) {
+       this(nodeId, nodeVersion, nodeVersion);
+    }
+
+    @Override public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         NodeMetadata that = (NodeMetadata) o;
-        return nodeId.equals(that.nodeId) &&
-            nodeVersion.equals(that.nodeVersion);
+        return nodeId.equals(that.nodeId) && nodeVersion.equals(that.nodeVersion) && Objects.equals(
+            previousNodeVersion,
+            that.previousNodeVersion);
     }
 
-    @Override
-    public int hashCode() {
-        return Objects.hash(nodeId, nodeVersion);
+    @Override public int hashCode() {
+        return Objects.hash(nodeId, nodeVersion, previousNodeVersion);
     }
 
     @Override
     public String toString() {
         return "NodeMetadata{" +
             "nodeId='" + nodeId + '\'' +
             ", nodeVersion=" + nodeVersion +
+            ", previousNodeVersion=" + previousNodeVersion +
             '}';
     }
 
@@ -68,10 +76,14 @@ public Version nodeVersion() {
         return nodeVersion;
     }
 
+    public Version previousNodeVersion() {
+        return previousNodeVersion;
+    }
+
     public NodeMetadata upgradeToCurrentVersion() {
         if (nodeVersion.equals(Version.V_EMPTY)) {
             assert Version.CURRENT.major <= Version.V_7_0_0.major + 1 : "version is required in the node metadata from v9 onwards";
-            return new NodeMetadata(nodeId, Version.CURRENT);
+            return new NodeMetadata(nodeId, Version.CURRENT, Version.V_EMPTY);
         }
 
         if (nodeVersion.before(Version.CURRENT.minimumIndexCompatibilityVersion())) {
@@ -84,12 +96,13 @@ public NodeMetadata upgradeToCurrentVersion() {
                 "cannot downgrade a node from version [" + nodeVersion + "] to version [" + Version.CURRENT + "]");
         }
 
-        return nodeVersion.equals(Version.CURRENT) ? this : new NodeMetadata(nodeId, Version.CURRENT);
+        return nodeVersion.equals(Version.CURRENT) ? this : new NodeMetadata(nodeId, Version.CURRENT, nodeVersion);
     }
 
     private static class Builder {
         String nodeId;
         Version nodeVersion;
+        Version previousNodeVersion;
 
         public void setNodeId(String nodeId) {
             this.nodeId = nodeId;
@@ -99,6 +112,10 @@ public void setNodeVersionId(int nodeVersionId) {
             this.nodeVersion = Version.fromId(nodeVersionId);
         }
 
+        public void setPreviousNodeVersionId(int previousNodeVersionId) {
+            this.previousNodeVersion = Version.fromId(previousNodeVersionId);
+        }
+
         public NodeMetadata build() {
             final Version nodeVersion;
             if (this.nodeVersion == null) {
@@ -107,8 +124,11 @@ public NodeMetadata build() {
             } else {
                 nodeVersion = this.nodeVersion;
             }
+            if (this.previousNodeVersion == null) {
+                previousNodeVersion = nodeVersion;
+            }
 
-            return new NodeMetadata(nodeId, nodeVersion);
+            return new NodeMetadata(nodeId, nodeVersion, previousNodeVersion);
         }
     }
 
@@ -125,6 +145,7 @@ static class NodeMetadataStateFormat extends MetadataStateFormat<NodeMetadata> {
             objectParser = new ObjectParser<>("node_meta_data", ignoreUnknownFields, Builder::new);
             objectParser.declareString(Builder::setNodeId, new ParseField(NODE_ID_KEY));
             objectParser.declareInt(Builder::setNodeVersionId, new ParseField(NODE_VERSION_KEY));
+            objectParser.declareInt(Builder::setPreviousNodeVersionId, new ParseField(PREVIOUS_NODE_VERSION_KEY));
         }
 
         @Override
@@ -138,6 +159,7 @@ protected XContentBuilder newXContentBuilder(XContentType type, OutputStream str
         public void toXContent(XContentBuilder builder, NodeMetadata nodeMetadata) throws IOException {
             builder.field(NODE_ID_KEY, nodeMetadata.nodeId);
             builder.field(NODE_VERSION_KEY, nodeMetadata.nodeVersion.id);
+            builder.field(PREVIOUS_NODE_VERSION_KEY, nodeMetadata.previousNodeVersion.id);
         }
 
         @Override

server/src/test/java/org/elasticsearch/env/NodeMetadataTests.java

@@ -97,6 +97,14 @@ public void testDoesNotUpgradeAncientVersion() {
             allOf(startsWith("cannot upgrade a node from version ["), endsWith("] directly to version [" + Version.CURRENT + "]")));
     }
 
+    public void testUpgradeMarksPreviousVersion() {
+        final String nodeId = randomAlphaOfLength(10);
+        final Version version = VersionUtils.randomVersionBetween(random(), Version.V_7_3_0, Version.V_7_16_0);
+        final NodeMetadata nodeMetadata = new NodeMetadata(nodeId, version).upgradeToCurrentVersion();
+        assertThat(nodeMetadata.nodeVersion(), equalTo(Version.CURRENT));
+        assertThat(nodeMetadata.previousNodeVersion(), equalTo(version));
+    }
+
     public static Version tooNewVersion() {
         return Version.fromId(between(Version.CURRENT.id + 1, 99999999));
     }

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

@@ -58,7 +58,7 @@ public class LicensesMetadata extends AbstractNamedDiffable<Metadata.Custom> imp
     @Nullable
     private Version trialVersion;
 
-    LicensesMetadata(License license, Version trialVersion) {
+    public LicensesMetadata(License license, Version trialVersion) {
         this.license = license;
         this.trialVersion = trialVersion;
     }

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

@@ -47,6 +47,7 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.NodeEnvironment;
+import org.elasticsearch.env.NodeMetadata;
 import org.elasticsearch.http.HttpServerTransport;
 import org.elasticsearch.index.IndexModule;
 import org.elasticsearch.indices.ExecutorNames;
@@ -402,9 +403,6 @@ public Security(Settings settings, final Path configPath) {
         this.enabled = XPackSettings.SECURITY_ENABLED.get(settings);
         if (enabled) {
             runStartupChecks(settings);
-            // we load them all here otherwise we can't access secure settings since they are closed once the checks are
-            // fetched
-
             Automatons.updateConfiguration(settings);
         } else {
             this.bootstrapChecks.set(Collections.emptyList());
@@ -436,7 +434,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
                                                Supplier<RepositoriesService> repositoriesServiceSupplier) {
         try {
             return createComponents(client, threadPool, clusterService, resourceWatcherService, scriptService, xContentRegistry,
-                environment, expressionResolver);
+                environment, nodeEnvironment.loadLastKnownMetadata(), expressionResolver);
         } catch (final Exception e) {
             throw new IllegalStateException("security initialization failed", e);
         }
@@ -445,7 +443,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
     // pkg private for testing - tests want to pass in their set of extensions hence we are not using the extension service directly
     Collection<Object> createComponents(Client client, ThreadPool threadPool, ClusterService clusterService,
                                         ResourceWatcherService resourceWatcherService, ScriptService scriptService,
-                                        NamedXContentRegistry xContentRegistry, Environment environment,
+                                        NamedXContentRegistry xContentRegistry, Environment environment, NodeMetadata nodeMetadata,
                                         IndexNameExpressionResolver expressionResolver) throws Exception {
         logger.info("Security is {}", enabled ? "enabled" : "disabled");
         if (enabled == false) {
@@ -459,6 +457,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
         checks.addAll(Arrays.asList(
             new TokenSSLBootstrapCheck(),
             new PkiRealmBootstrapCheck(getSslService()),
+            new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata),
             new TLSLicenseBootstrapCheck()));
         checks.addAll(InternalRealms.getBootstrapChecks(settings, environment));
         this.bootstrapChecks.set(Collections.unmodifiableList(checks));
@@ -493,7 +492,6 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
 
         // realms construction
         final NativeUsersStore nativeUsersStore = new NativeUsersStore(settings, client, securityIndex.get());
-
         final NativeRoleMappingStore nativeRoleMappingStore = new NativeRoleMappingStore(settings, client, securityIndex.get(),
             scriptService);
         final AnonymousUser anonymousUser = new AnonymousUser(settings);

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

@@ -0,0 +1,55 @@
+/*
+ * 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;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.bootstrap.BootstrapCheck;
+import org.elasticsearch.bootstrap.BootstrapContext;
+import org.elasticsearch.env.NodeMetadata;
+import org.elasticsearch.license.License;
+import org.elasticsearch.license.LicenseService;
+import org.elasticsearch.xpack.core.XPackSettings;
+
+public class SecurityImplicitBehaviorBootstrapCheck implements BootstrapCheck {
+
+    private final NodeMetadata nodeMetadata;
+
+    public SecurityImplicitBehaviorBootstrapCheck(NodeMetadata nodeMetadata) {
+        this.nodeMetadata = nodeMetadata;
+    }
+
+    @Override
+    public BootstrapCheckResult check(BootstrapContext context) {
+        if (nodeMetadata == null) {
+            return BootstrapCheckResult.success();
+        }
+        final License license = LicenseService.getLicense(context.metadata());
+        final Version lastKnownVersion = nodeMetadata.previousNodeVersion();
+        // pre v7.2.0 nodes have Version.EMPTY and its id is 0, so Version#before handles this successfully
+        if (lastKnownVersion.before(Version.V_8_0_0)
+            && XPackSettings.SECURITY_ENABLED.exists(context.settings()) == false
+            && (license.operationMode() == License.OperationMode.BASIC || license.operationMode() == License.OperationMode.TRIAL)) {
+            return BootstrapCheckResult.failure(
+                "The default value for ["
+                    + XPackSettings.SECURITY_ENABLED.getKey()
+                    + "] has changed. See https://www.elastic.co/guide/en/elasticsearch/reference/"
+                    + Version.CURRENT.major
+                    + "."
+                    + Version.CURRENT.minor
+                    + "/security-minimal-setup.html to enable security, or explicitly disable security by "
+                    + "setting [xpack.security.enabled] to \"false\" in elasticsearch.yml before restarting the node"
+            );
+        } else {
+            return BootstrapCheckResult.success();
+        }
+    }
+
+    public boolean alwaysEnforce() {
+        return true;
+    }
+}

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityImplicitBehaviorBootstrapCheckTests.java

@@ -0,0 +1,87 @@
+/*
+ * 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;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.bootstrap.BootstrapCheck;
+import org.elasticsearch.cluster.metadata.Metadata;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.env.NodeMetadata;
+import org.elasticsearch.license.License;
+import org.elasticsearch.license.LicensesMetadata;
+import org.elasticsearch.license.TestUtils;
+import org.elasticsearch.test.AbstractBootstrapCheckTestCase;
+import org.elasticsearch.test.VersionUtils;
+import org.elasticsearch.xpack.core.XPackSettings;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+
+public class SecurityImplicitBehaviorBootstrapCheckTests extends AbstractBootstrapCheckTestCase {
+
+    public void testFailureUpgradeFrom7xWithImplicitSecuritySettings() throws Exception {
+        final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_7_2_0, Version.V_7_16_0);
+        final NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), Version.CURRENT, previousVersion);
+        BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
+            createTestContext(Settings.EMPTY, createLicensesMetadata(previousVersion))
+        );
+        assertThat(result.isFailure(), is(true));
+        assertThat(
+            result.getMessage(),
+            equalTo(
+                "The default value for ["
+                    + XPackSettings.SECURITY_ENABLED.getKey()
+                    + "] has changed. See https://www.elastic.co/guide/en/elasticsearch/reference/"
+                    + Version.CURRENT.major
+                    + "."
+                    + Version.CURRENT.minor
+                    + "/security-minimal-setup.html to enable security, or explicitly disable security by "
+                    + "setting [xpack.security.enabled] to \"false\" in elasticsearch.yml before restarting the node"
+            )
+        );
+    }
+
+    public void testUpgradeFrom7xWithExplicitSecuritySettings() throws Exception {
+        final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_7_2_0, Version.V_7_16_0);
+        final NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), Version.CURRENT, previousVersion);
+        BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
+            createTestContext(
+                Settings.builder().put(XPackSettings.SECURITY_ENABLED.getKey(), true).build(),
+                createLicensesMetadata(previousVersion)
+            )
+        );
+        assertThat(result.isSuccess(), is(true));
+    }
+
+    public void testUpgradeFrom8xWithImplicitSecuritySettings() throws Exception {
+        final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, null);
+        final NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), Version.CURRENT, previousVersion);
+        BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
+            createTestContext(Settings.EMPTY, createLicensesMetadata(previousVersion))
+        );
+        assertThat(result.isSuccess(), is(true));
+    }
+
+    public void testUpgradeFrom8xWithExplicitSecuritySettings() throws Exception {
+        final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, null);
+        final NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), Version.CURRENT, previousVersion);
+        BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
+            createTestContext(
+                Settings.builder().put(XPackSettings.SECURITY_ENABLED.getKey(), true).build(),
+                createLicensesMetadata(previousVersion)
+            )
+        );
+        assertThat(result.isSuccess(), is(true));
+    }
+
+    private Metadata createLicensesMetadata(Version version) throws Exception {
+        License license = TestUtils.generateSignedLicense(randomFrom("basic", "trial"), TimeValue.timeValueHours(2));
+        return Metadata.builder().putCustom(LicensesMetadata.TYPE, new LicensesMetadata(license, version)).build();
+    }
+}

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java

@@ -31,6 +31,7 @@
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.NodeMetadata;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.index.IndexModule;
 import org.elasticsearch.index.IndexSettings;
@@ -128,6 +129,7 @@ public Map<String, Realm.Factory> getRealms(SecurityComponents components) {
 
     private Collection<Object> createComponentsUtil(Settings settings, SecurityExtension... extensions) throws Exception {
         Environment env = TestEnvironment.newEnvironment(settings);
+        NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(8), Version.CURRENT);
         licenseState = new TestUtils.UpdatableLicenseState(settings);
         SSLService sslService = new SSLService(env);
         security = new Security(settings, null, Arrays.asList(extensions)) {
@@ -155,7 +157,7 @@ protected SSLService getSslService() {
         when(client.threadPool()).thenReturn(threadPool);
         when(client.settings()).thenReturn(settings);
         return security.createComponents(client, threadPool, clusterService, mock(ResourceWatcherService.class), mock(ScriptService.class),
-            xContentRegistry(), env, TestIndexNameExpressionResolver.newInstance(threadContext));
+            xContentRegistry(), env, nodeMetadata, TestIndexNameExpressionResolver.newInstance(threadContext));
     }
 
     private Collection<Object> createComponents(Settings testSettings, SecurityExtension... extensions) throws Exception {