From e43375bf9abaa7825eec69831d5ba92047896586 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Tue, 24 Jul 2018 16:26:50 -0600 Subject: [PATCH] Security: revert to old way of merging automata (#32254) This commit reverts to the pre-6.3 way of merging automata as the change in 6.3 significantly impacts the performance for roles with a large number of concrete indices. In addition, the maximum number of states for security automata has been increased to 100,000 in order to allow users to use roles that caused problems pre-6.3 and 6.3 fixed. As an escape hatch, the maximum number of states is configurable with a setting so that users with complex patterns in roles can increase the states with the knowledge that there is more memory usage. --- .../core/security/support/Automatons.java | 34 ++++++++++++----- .../security/support/AutomatonsTests.java | 38 +++++++++++++++++++ .../xpack/security/Security.java | 9 ++--- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java index 36e0b8ddb00..b11867f8365 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java @@ -9,6 +9,8 @@ import org.apache.lucene.util.automaton.Automata; import org.apache.lucene.util.automaton.Automaton; import org.apache.lucene.util.automaton.CharacterRunAutomaton; import org.apache.lucene.util.automaton.RegExp; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; import java.util.ArrayList; import java.util.Arrays; @@ -25,9 +27,15 @@ import static org.elasticsearch.common.Strings.collectionToDelimitedString; public final class Automatons { + public static final Setting MAX_DETERMINIZED_STATES_SETTING = + Setting.intSetting("xpack.security.automata.max_determinized_states", 100000, DEFAULT_MAX_DETERMINIZED_STATES, + Setting.Property.NodeScope); public static final Automaton EMPTY = Automata.makeEmpty(); public static final Automaton MATCH_ALL = Automata.makeAnyString(); + // this value is not final since we allow it to be set at runtime + private static int maxDeterminizedStates = 100000; + static final char WILDCARD_STRING = '*'; // String equality with support for wildcards static final char WILDCARD_CHAR = '?'; // Char equality with support for wildcards static final char WILDCARD_ESCAPE = '\\'; // Escape character @@ -49,13 +57,12 @@ public final class Automatons { if (patterns.isEmpty()) { return EMPTY; } - Automaton automaton = null; + List automata = new ArrayList<>(patterns.size()); for (String pattern : patterns) { - final Automaton patternAutomaton = minimize(pattern(pattern), DEFAULT_MAX_DETERMINIZED_STATES); - automaton = automaton == null ? patternAutomaton : unionAndMinimize(Arrays.asList(automaton, patternAutomaton)); + final Automaton patternAutomaton = pattern(pattern); + automata.add(patternAutomaton); } - // the automaton is always minimized and deterministic - return automaton; + return unionAndMinimize(automata); } /** @@ -111,12 +118,12 @@ public final class Automatons { public static Automaton unionAndMinimize(Collection automata) { Automaton res = union(automata); - return minimize(res, DEFAULT_MAX_DETERMINIZED_STATES); + return minimize(res, maxDeterminizedStates); } public static Automaton minusAndMinimize(Automaton a1, Automaton a2) { - Automaton res = minus(a1, a2, DEFAULT_MAX_DETERMINIZED_STATES); - return minimize(res, DEFAULT_MAX_DETERMINIZED_STATES); + Automaton res = minus(a1, a2, maxDeterminizedStates); + return minimize(res, maxDeterminizedStates); } public static Predicate predicate(String... patterns) { @@ -131,8 +138,17 @@ public final class Automatons { return predicate(automaton, "Predicate for " + automaton); } + public static void updateMaxDeterminizedStates(Settings settings) { + maxDeterminizedStates = MAX_DETERMINIZED_STATES_SETTING.get(settings); + } + + // accessor for testing + static int getMaxDeterminizedStates() { + return maxDeterminizedStates; + } + private static Predicate predicate(Automaton automaton, final String toString) { - CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton, DEFAULT_MAX_DETERMINIZED_STATES); + CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton, maxDeterminizedStates); return new Predicate() { @Override public boolean test(String s) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/AutomatonsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/AutomatonsTests.java index 92c3b1d7713..72c988fc227 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/AutomatonsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/AutomatonsTests.java @@ -8,8 +8,11 @@ package org.elasticsearch.xpack.core.security.support; import org.apache.lucene.util.automaton.Automaton; import org.apache.lucene.util.automaton.CharacterRunAutomaton; import org.apache.lucene.util.automaton.Operations; +import org.apache.lucene.util.automaton.TooComplexToDeterminizeException; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -113,4 +116,39 @@ public class AutomatonsTests extends ESTestCase { // expected } } + + public void testLotsOfIndices() { + final int numberOfIndices = scaledRandomIntBetween(512, 1024); + final List names = new ArrayList<>(numberOfIndices); + for (int i = 0; i < numberOfIndices; i++) { + names.add(randomAlphaOfLengthBetween(6, 48)); + } + final Automaton automaton = Automatons.patterns(names); + assertTrue(automaton.isDeterministic()); + + CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton); + for (String name : names) { + assertTrue(runAutomaton.run(name)); + } + } + + public void testSettingMaxDeterminizedStates() { + try { + assertNotEquals(10000, Automatons.getMaxDeterminizedStates()); + // set to the min value + Settings settings = Settings.builder().put(Automatons.MAX_DETERMINIZED_STATES_SETTING.getKey(), 10000).build(); + Automatons.updateMaxDeterminizedStates(settings); + assertEquals(10000, Automatons.getMaxDeterminizedStates()); + + final List names = new ArrayList<>(1024); + for (int i = 0; i < 1024; i++) { + names.add(randomAlphaOfLength(48)); + } + TooComplexToDeterminizeException e = expectThrows(TooComplexToDeterminizeException.class, () -> Automatons.patterns(names)); + assertThat(e.getMaxDeterminizedStates(), equalTo(10000)); + } finally { + Automatons.updateMaxDeterminizedStates(Settings.EMPTY); + assertEquals(100000, Automatons.getMaxDeterminizedStates()); + } + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index b9cdd22ba47..f4bb4b7eb3b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -126,6 +126,7 @@ import org.elasticsearch.xpack.security.action.privilege.TransportGetPrivilegesA import org.elasticsearch.xpack.security.action.privilege.TransportPutPrivilegesAction; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.core.security.index.IndexAuditTrailField; +import org.elasticsearch.xpack.core.security.support.Automatons; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings; @@ -217,7 +218,6 @@ import org.elasticsearch.xpack.security.transport.SecurityServerTransportInterce import org.elasticsearch.xpack.security.transport.filter.IPFilter; import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4HttpServerTransport; import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4ServerTransport; -import org.elasticsearch.xpack.core.template.TemplateUtils; import org.elasticsearch.xpack.security.transport.nio.SecurityNioHttpServerTransport; import org.elasticsearch.xpack.security.transport.nio.SecurityNioTransport; import org.joda.time.DateTime; @@ -296,9 +296,6 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw this.enabled = XPackSettings.SECURITY_ENABLED.get(settings); if (enabled && transportClientMode == false) { validateAutoCreateIndex(settings); - } - - if (enabled) { // we load them all here otherwise we can't access secure settings since they are closed once the checks are // fetched final List checks = new ArrayList<>(); @@ -313,6 +310,7 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw new KerberosRealmBootstrapCheck(env))); checks.addAll(InternalRealms.getBootstrapChecks(settings, env)); this.bootstrapChecks = Collections.unmodifiableList(checks); + Automatons.updateMaxDeterminizedStates(settings); } else { this.bootstrapChecks = Collections.emptyList(); } @@ -607,13 +605,14 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw LoggingAuditTrail.registerSettings(settingsList); IndexAuditTrail.registerSettings(settingsList); - // authentication settings + // authentication and authorization settings AnonymousUser.addSettings(settingsList); RealmSettings.addSettings(settingsList, securityExtensions); NativeRolesStore.addSettings(settingsList); ReservedRealm.addSettings(settingsList); AuthenticationService.addSettings(settingsList); AuthorizationService.addSettings(settingsList); + settingsList.add(Automatons.MAX_DETERMINIZED_STATES_SETTING); settingsList.add(CompositeRolesStore.CACHE_SIZE_SETTING); settingsList.add(FieldPermissionsCache.CACHE_SIZE_SETTING); settingsList.add(TokenService.TOKEN_EXPIRATION);