Skip to content

Commit

Permalink
Deprecate realm names with a leading underscore (#73366)
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 6bc0916 commit 4880cea
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 9 deletions.
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 Function<String, Setting.AffixSetting<Boolean>> ENABLED_SETTING = affixSetting("enabled",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ private DeprecationChecks() {

static List<BiFunction<Settings, PluginsAndModules, DeprecationIssue>> NODE_SETTINGS_CHECKS = List.of(
NodeDeprecationChecks::checkSharedDataPathSetting,
NodeDeprecationChecks::checkReservedPrefixedRealmNames,
NodeDeprecationChecks::checkSingleDataNodeWatermarkSetting
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.RealmSettings;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Collectors;

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

public class NodeDeprecationChecks {

Expand Down Expand Up @@ -43,6 +51,35 @@ static DeprecationIssue checkSharedDataPathSetting(final Settings settings, fina
return null;
}

static DeprecationIssue checkReservedPrefixedRealmNames(final Settings settings, final PluginsAndModules pluginsAndModules) {
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 that start with [" + RESERVED_REALM_NAME_PREFIX + "] will not be permitted 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 checkSingleDataNodeWatermarkSetting(final Settings settings, final PluginsAndModules pluginsAndModules) {
if (DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE.exists(settings)) {
String key = DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE.getKey();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,25 @@

package org.elasticsearch.xpack.deprecation;

import static org.elasticsearch.xpack.deprecation.DeprecationChecks.NODE_SETTINGS_CHECKS;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.collection.IsIterableContainingInOrder.contains;

import java.util.List;

import org.elasticsearch.cluster.routing.allocation.decider.DiskThresholdDecider;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import static org.elasticsearch.xpack.deprecation.DeprecationChecks.NODE_SETTINGS_CHECKS;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.collection.IsIterableContainingInOrder.contains;

public class NodeDeprecationChecksTests extends ESTestCase {

public void testRemovedSettingNotSet() {
Expand Down Expand Up @@ -65,6 +68,57 @@ public void testSharedDataPathSetting() {
)));
}

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 List<DeprecationIssue> deprecationIssues = DeprecationChecks.filterChecks(NODE_SETTINGS_CHECKS, c -> c.apply(settings, null));

assertEquals(1, deprecationIssues.size());

final DeprecationIssue deprecationIssue = deprecationIssues.get(0);
assertEquals("Realm that start with [_] will not be permitted 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 testSingleDataNodeWatermarkSetting() {
Settings settings = Settings.builder()
.put(DiskThresholdDecider.ENABLE_FOR_SINGLE_DATA_NODE.getKey(), true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.CountDown;
import org.elasticsearch.common.util.concurrent.ThreadContext;
Expand Down Expand Up @@ -47,6 +49,7 @@
public class Realms implements Iterable<Realm> {

private static final Logger logger = LogManager.getLogger(Realms.class);
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(logger.getName());

private final Settings settings;
private final Environment env;
Expand Down Expand Up @@ -167,9 +170,13 @@ protected List<Realm> initRealms(List<RealmConfig> realmConfigs) throws Exceptio
List<Realm> realms = new ArrayList<>();
Map<String, Set<String>> nameToRealmIdentifier = new HashMap<>();
Map<Integer, Set<String>> orderToRealmName = new HashMap<>();
List<RealmConfig.RealmIdentifier> reservedPrefixedRealmIdentifiers = new ArrayList<>();
for (RealmConfig config: realmConfigs) {
Realm.Factory factory = factories.get(config.identifier().getType());
assert factory != null : "unknown realm type [" + config.identifier().getType() + "]";
if (config.identifier().getName().startsWith(RealmSettings.RESERVED_REALM_NAME_PREFIX)) {
reservedPrefixedRealmIdentifiers.add(config.identifier());
}
if (config.enabled() == false) {
if (logger.isDebugEnabled()) {
logger.debug("realm [{}] is disabled", config.identifier());
Expand Down Expand Up @@ -197,6 +204,7 @@ protected List<Realm> initRealms(List<RealmConfig> realmConfigs) throws Exceptio
if (Strings.hasText(duplicateRealms)) {
throw new IllegalArgumentException("Found multiple realms configured with the same name: " + duplicateRealms + "");
}
logDeprecationForReservedPrefixedRealmNames(reservedPrefixedRealmIdentifiers);
return Collections.unmodifiableList(realms);
}

Expand Down Expand Up @@ -337,6 +345,19 @@ private Set<String> findDisabledBasicRealmTypes(List<RealmConfig> realmConfigs)
.collect(Collectors.toUnmodifiableSet());
}

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("; ")));
}
}

private static void combineMaps(Map<String, Object> mapA, Map<String, Object> mapB) {
for (Entry<String, Object> entry : mapB.entrySet()) {
mapA.compute(entry.getKey(), (key, value) -> {
Expand Down
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 @@ -38,6 +39,8 @@
import java.util.Map;
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 @@ -583,6 +586,51 @@ public void testInitRealmsFailsForMultipleKerberosRealms() throws IOException {
"multiple realms [realm_1, realm_2] configured of type [kerberos], [kerberos] can only have one such realm configured")));
}

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 boolean randomDisableRealm(Settings.Builder builder, String type) {
final boolean disabled = randomBoolean();
if (disabled) {
Expand Down

0 comments on commit 4880cea

Please sign in to comment.