Skip to content

Commit

Permalink
Deprecate realm names with a leading underscore (#73366) (#73319)
Browse files Browse the repository at this point in the history
Deprecation warning is now issued if any realm is configured with a name
prefixed with an underscore. This applies to all realms regardless
whether they are enabled or not.

Relates: #73250
  • Loading branch information
ywangd authored Jun 15, 2021
1 parent 4ed2ec9 commit 59a64a0
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 0 deletions.
35 changes: 35 additions & 0 deletions docs/reference/migration/migrate_7_14.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,38 @@ has no effect and you should discontinue its use.
====

// end::notable-breaking-changes[]

[discrete]
[[deprecated-7.14]]
=== Deprecations

The following functionality has been deprecated in {es} 7.14
and will be removed in 8.0.
While this won't have an immediate impact on your applications,
we strongly encourage you take the described steps to update your code
after upgrading to 7.14.

NOTE: Significant changes in behavior are deprecated in a minor release and
the old behavior is supported until the next major release.
To find out if you are using any deprecated functionality,
enable <<deprecation-logging, deprecation logging>>.

[discrete]
[[breaking_714_security_changes]]
==== Security deprecations

[[reserved-prefixed-realm-names]]
.Configuring a realm name with a leading underscore is deprecated.
[%collapsible]
====
*Details* +
Elasticsearch creates "synthetic" realm names on the fly for services like API keys.
These synthetic realm names are prefixed with an underscore.
Currently, user configured realms can also be given a name with a leading underscore.
This creates confusion since realm names are meant to be unique for a node.
*Impact* +
Configuring a realm name with a leading underscore is deprecated. In a future release of {es}
it will result in an error on startup if any user configured realm has a name
with a leading underscore.
====
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/
public class RealmSettings {

public static final String RESERVED_REALM_NAME_PREFIX = "_";
public static final String PREFIX = "xpack.security.authc.realms.";
public static final String ENABLED_SETTING_KEY = "enabled";
public static final String ORDER_SETTING_KEY = "order";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ private DeprecationChecks() {
NodeDeprecationChecks::checkMissingRealmOrders,
NodeDeprecationChecks::checkUniqueRealmOrders,
NodeDeprecationChecks::checkImplicitlyDisabledBasicRealms,
NodeDeprecationChecks::checkReservedPrefixedRealmNames,
(settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkThreadPoolListenerQueueSize(settings),
(settings, pluginsAndModules, cs) -> NodeDeprecationChecks.checkThreadPoolListenerSize(settings),
NodeDeprecationChecks::checkClusterRemoteConnectSetting,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
Expand All @@ -42,6 +43,8 @@
import java.util.function.BiFunction;
import java.util.stream.Collectors;

import static org.elasticsearch.xpack.core.security.authc.RealmSettings.RESERVED_REALM_NAME_PREFIX;

class NodeDeprecationChecks {

static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndModules pluginsAndModules,
Expand Down Expand Up @@ -175,6 +178,37 @@ static DeprecationIssue checkImplicitlyDisabledBasicRealms(final Settings settin

}

static DeprecationIssue checkReservedPrefixedRealmNames(final Settings settings, final PluginsAndModules pluginsAndModules,
final ClusterState clusterState) {
final Map<RealmConfig.RealmIdentifier, Settings> realmSettings = RealmSettings.getRealmSettings(settings);
if (realmSettings.isEmpty()) {
return null;
}
List<RealmConfig.RealmIdentifier> reservedPrefixedRealmIdentifiers = new ArrayList<>();
for (RealmConfig.RealmIdentifier realmIdentifier: realmSettings.keySet()) {
if (realmIdentifier.getName().startsWith(RESERVED_REALM_NAME_PREFIX)) {
reservedPrefixedRealmIdentifiers.add(realmIdentifier);
}
}
if (reservedPrefixedRealmIdentifiers.isEmpty()) {
return null;
} else {
return new DeprecationIssue(
DeprecationIssue.Level.CRITICAL,
"Realm names cannot start with [" + RESERVED_REALM_NAME_PREFIX + "] in a future major release.",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.14/deprecated-7.14.html#reserved-prefixed-realm-names",
String.format(Locale.ROOT, "Found realm " + (reservedPrefixedRealmIdentifiers.size() == 1 ? "name" : "names")
+ " with reserved prefix [%s]: [%s]. "
+ "In a future major release, node will fail to start if any realm names start with reserved prefix.",
RESERVED_REALM_NAME_PREFIX,
reservedPrefixedRealmIdentifiers.stream()
.map(rid -> RealmSettings.PREFIX + rid.getType() + "." + rid.getName())
.sorted()
.collect(Collectors.joining("; ")))
);
}
}

static DeprecationIssue checkThreadPoolListenerQueueSize(final Settings settings) {
return checkThreadPoolListenerSetting("thread_pool.listener.queue_size", settings);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
Expand All @@ -29,6 +30,7 @@
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.RealmSettings;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -302,6 +304,56 @@ public void testCheckImplicitlyDisabledBasicRealms() {
}
}

public void testCheckReservedPrefixedRealmNames() {
final Settings.Builder builder = Settings.builder();
final boolean invalidFileRealmName = randomBoolean();
final boolean invalidNativeRealmName = randomBoolean();
final boolean invalidOtherRealmName = (false == invalidFileRealmName && false == invalidNativeRealmName) || randomBoolean();

final List<String> invalidRealmNames = new ArrayList<>();

final String fileRealmName = randomAlphaOfLengthBetween(4, 12);
if (invalidFileRealmName) {
builder.put("xpack.security.authc.realms.file." + "_" + fileRealmName + ".order", -20);
invalidRealmNames.add("xpack.security.authc.realms.file." + "_" + fileRealmName);
} else {
builder.put("xpack.security.authc.realms.file." + fileRealmName + ".order", -20);
}

final String nativeRealmName = randomAlphaOfLengthBetween(4, 12);
if (invalidNativeRealmName) {
builder.put("xpack.security.authc.realms.native." + "_" + nativeRealmName + ".order", -10);
invalidRealmNames.add("xpack.security.authc.realms.native." + "_" + nativeRealmName);
} else {
builder.put("xpack.security.authc.realms.native." + nativeRealmName + ".order", -10);
}

final int otherRealmId = randomIntBetween(0, 9);
final String otherRealmName = randomAlphaOfLengthBetween(4, 12);
if (invalidOtherRealmName) {
builder.put("xpack.security.authc.realms.type_" + otherRealmId + "." + "_" + otherRealmName + ".order", 0);
invalidRealmNames.add("xpack.security.authc.realms.type_" + otherRealmId + "." + "_" + otherRealmName);
} else {
builder.put("xpack.security.authc.realms.type_" + otherRealmId + "." + otherRealmName + ".order", 0);
}

final Settings settings = builder.build();
final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList());
final List<DeprecationIssue> deprecationIssues = getDeprecationIssues(settings, pluginsAndModules);

assertEquals(1, deprecationIssues.size());

final DeprecationIssue deprecationIssue = deprecationIssues.get(0);
assertEquals("Realm names cannot start with [_] in a future major release.", deprecationIssue.getMessage());
assertEquals("https://www.elastic.co/guide/en/elasticsearch/reference" +
"/7.14/deprecated-7.14.html#reserved-prefixed-realm-names", deprecationIssue.getUrl());
assertEquals("Found realm " + (invalidRealmNames.size() == 1 ? "name" : "names")
+ " with reserved prefix [_]: ["
+ Strings.collectionToDelimitedString(invalidRealmNames.stream().sorted().collect(Collectors.toList()), "; ") + "]. "
+ "In a future major release, node will fail to start if any realm names start with reserved prefix.",
deprecationIssue.getDetails());
}

public void testThreadPoolListenerQueueSize() {
final int size = randomIntBetween(1, 4);
final Settings settings = Settings.builder().put("thread_pool.listener.queue_size", size).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ protected List<Realm> initRealms() throws Exception {
Map<String, Set<String>> nameToRealmIdentifier = new HashMap<>();
Set<String> missingOrderRealmSettingKeys = new TreeSet<>();
Map<String, Set<String>> orderToRealmOrderSettingKeys = new HashMap<>();
List<RealmConfig.RealmIdentifier> reservedPrefixedRealmIdentifiers = new ArrayList<>();
Set<String> unconfiguredBasicRealms = new HashSet<>(
org.elasticsearch.core.Set.of(FileRealmSettings.TYPE, NativeRealmSettings.TYPE));
for (final Map.Entry<RealmConfig.RealmIdentifier, Settings> entry: realmsSettings.entrySet()) {
Expand All @@ -195,6 +196,9 @@ protected List<Realm> initRealms() throws Exception {
if (factory == null) {
throw new IllegalArgumentException("unknown realm type [" + identifier.getType() + "] for realm [" + identifier + "]");
}
if (identifier.getName().startsWith(RealmSettings.RESERVED_REALM_NAME_PREFIX)) {
reservedPrefixedRealmIdentifiers.add(identifier);
}
RealmConfig config = new RealmConfig(identifier, settings, env, threadContext);
unconfiguredBasicRealms.remove(identifier.getType());
if (config.enabled() == false) {
Expand Down Expand Up @@ -244,6 +248,7 @@ protected List<Realm> initRealms() throws Exception {
}

logDeprecationIfFound(missingOrderRealmSettingKeys, orderToRealmOrderSettingKeys);
logDeprecationForReservedPrefixedRealmNames(reservedPrefixedRealmIdentifiers);

return Collections.unmodifiableList(realms);
}
Expand Down Expand Up @@ -405,4 +410,17 @@ private void logDeprecationForImplicitlyDisabledBasicRealms(List<Realm> realms,
);
}
}

private void logDeprecationForReservedPrefixedRealmNames(List<RealmConfig.RealmIdentifier> realmIdentifiers) {
if (false == realmIdentifiers.isEmpty()) {
deprecationLogger.deprecate(DeprecationCategory.SECURITY, "realm_name_with_reserved_prefix",
"Found realm " + (realmIdentifiers.size() == 1 ? "name" : "names") + " with reserved prefix [{}]: [{}]. " +
"In a future major release, node will fail to start if any realm names start with reserved prefix.",
RealmSettings.RESERVED_REALM_NAME_PREFIX,
realmIdentifiers.stream()
.map(rid -> RealmSettings.PREFIX + rid.getType() + "." + rid.getName())
.sorted()
.collect(Collectors.joining("; ")));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.env.Environment;
Expand Down Expand Up @@ -39,6 +40,7 @@
import java.util.Map.Entry;
import java.util.TreeMap;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -751,6 +753,51 @@ public void testWarningsForImplicitlyDisabledBasicRealms() throws Exception {
}
}

public void testWarningsForReservedPrefixedRealmNames() throws Exception {
Settings.Builder builder = Settings.builder()
.put("path.home", createTempDir());
final boolean invalidFileRealmName = randomBoolean();
final boolean invalidNativeRealmName = randomBoolean();
// Ensure at least one realm has invalid name
final int upperBound = (invalidFileRealmName || invalidNativeRealmName) ? randomRealmTypesCount : randomRealmTypesCount - 1;
final int invalidOtherRealmNameIndex = randomIntBetween(0, upperBound);

final List<String> invalidRealmNames = new ArrayList<>();
if (invalidFileRealmName) {
builder.put("xpack.security.authc.realms.file._default_file.order", -20);
invalidRealmNames.add("xpack.security.authc.realms.file._default_file");
} else {
builder.put("xpack.security.authc.realms.file.default_file.order", -20);
}

if (invalidNativeRealmName) {
builder.put("xpack.security.authc.realms.native._default_native.order", -10);
invalidRealmNames.add("xpack.security.authc.realms.native._default_native");
} else {
builder.put("xpack.security.authc.realms.native.default_native.order", -10);
}

IntStream.range(0, randomRealmTypesCount).forEach(i -> {
if (i != invalidOtherRealmNameIndex) {
builder.put("xpack.security.authc.realms.type_" + i + ".realm_" + i + ".order", i)
.put("xpack.security.authc.realms.type_" + i + ".realm_" + i + ".enabled", randomBoolean());
} else {
builder.put("xpack.security.authc.realms.type_" + i + "._realm_" + i + ".order", i)
.put("xpack.security.authc.realms.type_" + i + "._realm_" + i + ".enabled", randomBoolean());
invalidRealmNames.add("xpack.security.authc.realms.type_" + i + "._realm_" + i);
}
});

Settings settings = builder.build();
Environment env = TestEnvironment.newEnvironment(settings);
new Realms(settings, env, factories, licenseState, threadContext, reservedRealm);

assertWarnings("Found realm " + (invalidRealmNames.size() == 1 ? "name" : "names")
+ " with reserved prefix [_]: ["
+ Strings.collectionToDelimitedString(invalidRealmNames.stream().sorted().collect(Collectors.toList()), "; ") + "]. "
+ "In a future major release, node will fail to start if any realm names start with reserved prefix.");
}

private void disableFileAndNativeRealms(Settings.Builder builder) {
builder.put("xpack.security.authc.realms.file.default_file.enabled", false)
.put("xpack.security.authc.realms.native.default_native.enabled", false);
Expand Down

0 comments on commit 59a64a0

Please sign in to comment.