Skip to content

Commit

Permalink
Startup check for security implicit behavior change (#76879)
Browse files Browse the repository at this point in the history
In the security ON by default project, we introduced a breaking change 
for the xpack.security.enabled setting. While we do expose necessary
deprecation warnings and release notes, there might still be a case
where a deployment that is

- On basic or trial license with `xpack.security.enabled` not set
- A single node cluster or a multi-node, single-host cluster

gets upgraded to 8.x in place without using the Upgrade Assistant or 
consulting the release notes. In this case, we elect to stop the node
from starting via a newly introduced BootstrapCheck, so that we can
 notify the user that the implicit behavior for security has changed. 
If we don't do that, the upgrade can seemingly succeed but the user 
will have no way to interact with the upgraded cluster as security is 
enabled and they have no credentials. 

This is a best effort check in the sense that: 

- LicenseState might not be correct that early in the 
node lifecycle, so we might not be able to know if this node was on
basic/trial
- A grow-and-shrink upgrade would bypass this check since new
nodes start with empty state on disk
- A user might change the configuration and remove the explicit
xpack.security.enabled configuration _while_ upgrading the node
to 8.x
  • Loading branch information
jkakavas committed Oct 25, 2021
1 parent 1c77944 commit 8b3a615
Show file tree
Hide file tree
Showing 8 changed files with 221 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,13 @@ public String nodeId() {
return nodeMetadata.nodeId();
}

/**
* Returns the loaded NodeMetadata for this node
*/
public NodeMetadata nodeMetadata() {
return nodeMetadata;
}

/**
* Returns an array of all of the {@link NodePath}s.
*/
Expand Down
47 changes: 36 additions & 11 deletions server/src/main/java/org/elasticsearch/env/NodeMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,37 @@ public final class NodeMetadata {

private final Version nodeVersion;

public NodeMetadata(final String nodeId, final Version nodeVersion) {
private final Version previousNodeVersion;

private 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 +
'}';
}

Expand All @@ -68,10 +75,20 @@ public Version nodeVersion() {
return nodeVersion;
}

/**
* When a node starts we read the existing node metadata from disk (see NodeEnvironment@loadNodeMetadata), store a reference to the
* node version that we read from there in {@code previousNodeVersion} and then proceed to upgrade the version to
* the current version of the node ({@link NodeMetadata#upgradeToCurrentVersion()} before storing the node metadata again on disk.
* In doing so, {@code previousNodeVersion} refers to the previously last known version that this node was started on.
*/
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())) {
Expand All @@ -84,12 +101,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;
Expand All @@ -99,6 +117,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) {
Expand All @@ -107,8 +129,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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.elasticsearch.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;
Expand Down Expand Up @@ -432,9 +433,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());
Expand Down Expand Up @@ -465,7 +463,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.nodeMetadata(), expressionResolver);
} catch (final Exception e) {
throw new IllegalStateException("security initialization failed", e);
}
Expand All @@ -474,7 +472,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) {
Expand All @@ -488,6 +486,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
checks.addAll(Arrays.asList(
new TokenSSLBootstrapCheck(),
new PkiRealmBootstrapCheck(getSslService()),
new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata),
new TransportTLSBootstrapCheck()));
checks.addAll(InternalRealms.getBootstrapChecks(settings, environment));
this.bootstrapChecks.set(Collections.unmodifiableList(checks));
Expand Down Expand Up @@ -522,7 +521,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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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 in the current version. "
+ " Security features were implicitly disabled for this node but they would now be enabled, possibly"
+ " preventing access to the node. "
+ "See https://www.elastic.co/guide/en/elasticsearch/reference/"
+ Version.CURRENT.major
+ "."
+ Version.CURRENT.minor
+ "/security-minimal-setup.html to configure 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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* 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);
NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), previousVersion);
nodeMetadata = nodeMetadata.upgradeToCurrentVersion();
BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
createTestContext(Settings.EMPTY, createLicensesMetadata(previousVersion, randomFrom("basic", "trial")))
);
assertThat(result.isFailure(), is(true));
assertThat(
result.getMessage(),
equalTo(
"The default value for ["
+ XPackSettings.SECURITY_ENABLED.getKey()
+ "] has changed in the current version. "
+ " Security features were implicitly disabled for this node but they would now be enabled, possibly"
+ " preventing access to the node. "
+ "See https://www.elastic.co/guide/en/elasticsearch/reference/"
+ Version.CURRENT.major
+ "."
+ Version.CURRENT.minor
+ "/security-minimal-setup.html to configure security, or explicitly disable security by "
+ "setting [xpack.security.enabled] to \"false\" in elasticsearch.yml before restarting the node."
)
);
}

public void testUpgradeFrom7xWithImplicitSecuritySettingsOnGoldPlus() throws Exception {
final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_7_2_0, Version.V_7_16_0);
NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), previousVersion);
nodeMetadata = nodeMetadata.upgradeToCurrentVersion();
BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
createTestContext(Settings.EMPTY, createLicensesMetadata(previousVersion, randomFrom("gold", "platinum")))
);
assertThat(result.isSuccess(), is(true));
}

public void testUpgradeFrom7xWithExplicitSecuritySettings() throws Exception {
final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_7_2_0, Version.V_7_16_0);
NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), previousVersion);
nodeMetadata = nodeMetadata.upgradeToCurrentVersion();
BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
createTestContext(
Settings.builder().put(XPackSettings.SECURITY_ENABLED.getKey(), true).build(),
createLicensesMetadata(previousVersion, randomFrom("basic", "trial"))
)
);
assertThat(result.isSuccess(), is(true));
}

public void testUpgradeFrom8xWithImplicitSecuritySettings() throws Exception {
final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, null);
NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), previousVersion);
nodeMetadata = nodeMetadata.upgradeToCurrentVersion();
BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
createTestContext(Settings.EMPTY, createLicensesMetadata(previousVersion, randomFrom("basic", "trial")))
);
assertThat(result.isSuccess(), is(true));
}

public void testUpgradeFrom8xWithExplicitSecuritySettings() throws Exception {
final Version previousVersion = VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, null);
NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(10), previousVersion);
nodeMetadata = nodeMetadata.upgradeToCurrentVersion();
BootstrapCheck.BootstrapCheckResult result = new SecurityImplicitBehaviorBootstrapCheck(nodeMetadata).check(
createTestContext(
Settings.builder().put(XPackSettings.SECURITY_ENABLED.getKey(), true).build(),
createLicensesMetadata(previousVersion, randomFrom("basic", "trial"))
)
);
assertThat(result.isSuccess(), is(true));
}

private Metadata createLicensesMetadata(Version version, String licenseMode) throws Exception {
License license = TestUtils.generateSignedLicense(licenseMode, TimeValue.timeValueHours(2));
return Metadata.builder().putCustom(LicensesMetadata.TYPE, new LicensesMetadata(license, version)).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 8b3a615

Please sign in to comment.