Skip to content

Commit e43375b

Browse files
authored
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.
1 parent 717df26 commit e43375b

File tree

3 files changed

+67
-14
lines changed

3 files changed

+67
-14
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java

+25-9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
import org.apache.lucene.util.automaton.Automaton;
1010
import org.apache.lucene.util.automaton.CharacterRunAutomaton;
1111
import org.apache.lucene.util.automaton.RegExp;
12+
import org.elasticsearch.common.settings.Setting;
13+
import org.elasticsearch.common.settings.Settings;
1214

1315
import java.util.ArrayList;
1416
import java.util.Arrays;
@@ -25,9 +27,15 @@
2527

2628
public final class Automatons {
2729

30+
public static final Setting<Integer> MAX_DETERMINIZED_STATES_SETTING =
31+
Setting.intSetting("xpack.security.automata.max_determinized_states", 100000, DEFAULT_MAX_DETERMINIZED_STATES,
32+
Setting.Property.NodeScope);
2833
public static final Automaton EMPTY = Automata.makeEmpty();
2934
public static final Automaton MATCH_ALL = Automata.makeAnyString();
3035

36+
// this value is not final since we allow it to be set at runtime
37+
private static int maxDeterminizedStates = 100000;
38+
3139
static final char WILDCARD_STRING = '*'; // String equality with support for wildcards
3240
static final char WILDCARD_CHAR = '?'; // Char equality with support for wildcards
3341
static final char WILDCARD_ESCAPE = '\\'; // Escape character
@@ -49,13 +57,12 @@ public static Automaton patterns(Collection<String> patterns) {
4957
if (patterns.isEmpty()) {
5058
return EMPTY;
5159
}
52-
Automaton automaton = null;
60+
List<Automaton> automata = new ArrayList<>(patterns.size());
5361
for (String pattern : patterns) {
54-
final Automaton patternAutomaton = minimize(pattern(pattern), DEFAULT_MAX_DETERMINIZED_STATES);
55-
automaton = automaton == null ? patternAutomaton : unionAndMinimize(Arrays.asList(automaton, patternAutomaton));
62+
final Automaton patternAutomaton = pattern(pattern);
63+
automata.add(patternAutomaton);
5664
}
57-
// the automaton is always minimized and deterministic
58-
return automaton;
65+
return unionAndMinimize(automata);
5966
}
6067

6168
/**
@@ -111,12 +118,12 @@ static Automaton wildcard(String text) {
111118

112119
public static Automaton unionAndMinimize(Collection<Automaton> automata) {
113120
Automaton res = union(automata);
114-
return minimize(res, DEFAULT_MAX_DETERMINIZED_STATES);
121+
return minimize(res, maxDeterminizedStates);
115122
}
116123

117124
public static Automaton minusAndMinimize(Automaton a1, Automaton a2) {
118-
Automaton res = minus(a1, a2, DEFAULT_MAX_DETERMINIZED_STATES);
119-
return minimize(res, DEFAULT_MAX_DETERMINIZED_STATES);
125+
Automaton res = minus(a1, a2, maxDeterminizedStates);
126+
return minimize(res, maxDeterminizedStates);
120127
}
121128

122129
public static Predicate<String> predicate(String... patterns) {
@@ -131,8 +138,17 @@ public static Predicate<String> predicate(Automaton automaton) {
131138
return predicate(automaton, "Predicate for " + automaton);
132139
}
133140

141+
public static void updateMaxDeterminizedStates(Settings settings) {
142+
maxDeterminizedStates = MAX_DETERMINIZED_STATES_SETTING.get(settings);
143+
}
144+
145+
// accessor for testing
146+
static int getMaxDeterminizedStates() {
147+
return maxDeterminizedStates;
148+
}
149+
134150
private static Predicate<String> predicate(Automaton automaton, final String toString) {
135-
CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton, DEFAULT_MAX_DETERMINIZED_STATES);
151+
CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton, maxDeterminizedStates);
136152
return new Predicate<String>() {
137153
@Override
138154
public boolean test(String s) {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/AutomatonsTests.java

+38
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
import org.apache.lucene.util.automaton.Automaton;
99
import org.apache.lucene.util.automaton.CharacterRunAutomaton;
1010
import org.apache.lucene.util.automaton.Operations;
11+
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
12+
import org.elasticsearch.common.settings.Settings;
1113
import org.elasticsearch.test.ESTestCase;
1214

15+
import java.util.ArrayList;
1316
import java.util.Arrays;
1417
import java.util.List;
1518

@@ -113,4 +116,39 @@ private void assertInvalidPattern(String text) {
113116
// expected
114117
}
115118
}
119+
120+
public void testLotsOfIndices() {
121+
final int numberOfIndices = scaledRandomIntBetween(512, 1024);
122+
final List<String> names = new ArrayList<>(numberOfIndices);
123+
for (int i = 0; i < numberOfIndices; i++) {
124+
names.add(randomAlphaOfLengthBetween(6, 48));
125+
}
126+
final Automaton automaton = Automatons.patterns(names);
127+
assertTrue(automaton.isDeterministic());
128+
129+
CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton);
130+
for (String name : names) {
131+
assertTrue(runAutomaton.run(name));
132+
}
133+
}
134+
135+
public void testSettingMaxDeterminizedStates() {
136+
try {
137+
assertNotEquals(10000, Automatons.getMaxDeterminizedStates());
138+
// set to the min value
139+
Settings settings = Settings.builder().put(Automatons.MAX_DETERMINIZED_STATES_SETTING.getKey(), 10000).build();
140+
Automatons.updateMaxDeterminizedStates(settings);
141+
assertEquals(10000, Automatons.getMaxDeterminizedStates());
142+
143+
final List<String> names = new ArrayList<>(1024);
144+
for (int i = 0; i < 1024; i++) {
145+
names.add(randomAlphaOfLength(48));
146+
}
147+
TooComplexToDeterminizeException e = expectThrows(TooComplexToDeterminizeException.class, () -> Automatons.patterns(names));
148+
assertThat(e.getMaxDeterminizedStates(), equalTo(10000));
149+
} finally {
150+
Automatons.updateMaxDeterminizedStates(Settings.EMPTY);
151+
assertEquals(100000, Automatons.getMaxDeterminizedStates());
152+
}
153+
}
116154
}

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
import org.elasticsearch.xpack.security.action.privilege.TransportPutPrivilegesAction;
127127
import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
128128
import org.elasticsearch.xpack.core.security.index.IndexAuditTrailField;
129+
import org.elasticsearch.xpack.core.security.support.Automatons;
129130
import org.elasticsearch.xpack.core.security.user.AnonymousUser;
130131
import org.elasticsearch.xpack.core.ssl.SSLConfiguration;
131132
import org.elasticsearch.xpack.core.ssl.SSLConfigurationSettings;
@@ -217,7 +218,6 @@
217218
import org.elasticsearch.xpack.security.transport.filter.IPFilter;
218219
import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4HttpServerTransport;
219220
import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4ServerTransport;
220-
import org.elasticsearch.xpack.core.template.TemplateUtils;
221221
import org.elasticsearch.xpack.security.transport.nio.SecurityNioHttpServerTransport;
222222
import org.elasticsearch.xpack.security.transport.nio.SecurityNioTransport;
223223
import org.joda.time.DateTime;
@@ -296,9 +296,6 @@ public Security(Settings settings, final Path configPath) {
296296
this.enabled = XPackSettings.SECURITY_ENABLED.get(settings);
297297
if (enabled && transportClientMode == false) {
298298
validateAutoCreateIndex(settings);
299-
}
300-
301-
if (enabled) {
302299
// we load them all here otherwise we can't access secure settings since they are closed once the checks are
303300
// fetched
304301
final List<BootstrapCheck> checks = new ArrayList<>();
@@ -313,6 +310,7 @@ public Security(Settings settings, final Path configPath) {
313310
new KerberosRealmBootstrapCheck(env)));
314311
checks.addAll(InternalRealms.getBootstrapChecks(settings, env));
315312
this.bootstrapChecks = Collections.unmodifiableList(checks);
313+
Automatons.updateMaxDeterminizedStates(settings);
316314
} else {
317315
this.bootstrapChecks = Collections.emptyList();
318316
}
@@ -607,13 +605,14 @@ public static List<Setting<?>> getSettings(boolean transportClientMode, List<Sec
607605
LoggingAuditTrail.registerSettings(settingsList);
608606
IndexAuditTrail.registerSettings(settingsList);
609607

610-
// authentication settings
608+
// authentication and authorization settings
611609
AnonymousUser.addSettings(settingsList);
612610
RealmSettings.addSettings(settingsList, securityExtensions);
613611
NativeRolesStore.addSettings(settingsList);
614612
ReservedRealm.addSettings(settingsList);
615613
AuthenticationService.addSettings(settingsList);
616614
AuthorizationService.addSettings(settingsList);
615+
settingsList.add(Automatons.MAX_DETERMINIZED_STATES_SETTING);
617616
settingsList.add(CompositeRolesStore.CACHE_SIZE_SETTING);
618617
settingsList.add(FieldPermissionsCache.CACHE_SIZE_SETTING);
619618
settingsList.add(TokenService.TOKEN_EXPIRATION);

0 commit comments

Comments
 (0)