Skip to content

Commit

Permalink
Security: revert to old way of merging automata (#32254)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jaymode
jaymode authored Jul 24, 2018
1 parent 717df26 commit e43375b
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 14 deletions.
34 changes: 25 additions & 9 deletions ...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Automatons.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
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;
Expand All @@ -25,9 +27,15 @@

public final class Automatons {

public static final Setting<Integer> 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
Expand All @@ -49,13 +57,12 @@ public static Automaton patterns(Collection<String> patterns) {
if (patterns.isEmpty()) {
return EMPTY;
}
Automaton automaton = null;
List<Automaton> 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);
}

/**
Expand Down Expand Up @@ -111,12 +118,12 @@ static Automaton wildcard(String text) {

public static Automaton unionAndMinimize(Collection<Automaton> 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<String> predicate(String... patterns) {
Expand All @@ -131,8 +138,17 @@ public static Predicate<String> predicate(Automaton automaton) {
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<String> predicate(Automaton automaton, final String toString) {
CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton, DEFAULT_MAX_DETERMINIZED_STATES);
CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton, maxDeterminizedStates);
return new Predicate<String>() {
@Override
public boolean test(String s) {
Expand Down
38 changes: 38 additions & 0 deletions ...gin/core/src/test/java/org/elasticsearch/xpack/core/security/support/AutomatonsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
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;

Expand Down Expand Up @@ -113,4 +116,39 @@ private void assertInvalidPattern(String text) {
// expected
}
}

public void testLotsOfIndices() {
final int numberOfIndices = scaledRandomIntBetween(512, 1024);
final List<String> 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<String> 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());
}
}
}
9 changes: 4 additions & 5 deletions x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
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;
Expand Down Expand Up @@ -217,7 +218,6 @@
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;
Expand Down Expand Up @@ -296,9 +296,6 @@ public Security(Settings settings, final Path configPath) {
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<BootstrapCheck> checks = new ArrayList<>();
Expand All @@ -313,6 +310,7 @@ public Security(Settings settings, final Path configPath) {
new KerberosRealmBootstrapCheck(env)));
checks.addAll(InternalRealms.getBootstrapChecks(settings, env));
this.bootstrapChecks = Collections.unmodifiableList(checks);
Automatons.updateMaxDeterminizedStates(settings);
} else {
this.bootstrapChecks = Collections.emptyList();
}
Expand Down Expand Up @@ -607,13 +605,14 @@ public static List<Setting<?>> getSettings(boolean transportClientMode, List<Sec
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);
Expand Down

0 comments on commit e43375b

Please sign in to comment.