Security: minimize automatons as they are combined (elastic/x-pack-elasticsearch#4300)
This commit changes the combination of multiple automatons representing a pattern so that the result of each step is minimal. Previously, the code unioned the automata and performed the minimization operation after all of the automata had been combined. This resulted in patterns with lots of overlap causing a TooComplexToDeterminizeException even though the end result could be a automaton that is total. Minimizing the automata as we go, allows us to build an automata that could not previously be built at the cost of additional operations. Automata are typically cached in the security code, so the net performance impact should be minimal. Original commit: elastic/x-pack-elasticsearch@b59fe8d690
This commit is contained in:
parent
be92ee1fb1
commit
b4bf9ed87e
|
@ -51,13 +51,11 @@ public final class Automatons {
|
|||
}
|
||||
Automaton automaton = null;
|
||||
for (String pattern : patterns) {
|
||||
if (automaton == null) {
|
||||
automaton = pattern(pattern);
|
||||
} else {
|
||||
automaton = union(automaton, pattern(pattern));
|
||||
}
|
||||
final Automaton patternAutomaton = minimize(pattern(pattern), DEFAULT_MAX_DETERMINIZED_STATES);
|
||||
automaton = automaton == null ? patternAutomaton : unionAndMinimize(Arrays.asList(automaton, patternAutomaton));
|
||||
}
|
||||
return minimize(automaton, DEFAULT_MAX_DETERMINIZED_STATES); // minimal is also deterministic
|
||||
// the automaton is always minimized and deterministic
|
||||
return automaton;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -72,8 +70,11 @@ public final class Automatons {
|
|||
}
|
||||
String regex = pattern.substring(1, pattern.length() - 1);
|
||||
return new RegExp(regex).toAutomaton();
|
||||
} else if (pattern.equals("*")) {
|
||||
return MATCH_ALL;
|
||||
} else {
|
||||
return wildcard(pattern);
|
||||
}
|
||||
return wildcard(pattern);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -7,8 +7,12 @@ 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.elasticsearch.test.ESTestCase;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
|
||||
import static org.apache.lucene.util.automaton.Operations.DEFAULT_MAX_DETERMINIZED_STATES;
|
||||
import static org.elasticsearch.xpack.core.security.support.Automatons.pattern;
|
||||
import static org.elasticsearch.xpack.core.security.support.Automatons.patterns;
|
||||
|
@ -61,6 +65,36 @@ public class AutomatonsTests extends ESTestCase {
|
|||
assertThat(predicate("a.*z", "A.*Z", "Α.*Ω").toString(), equalTo("a.*z|A.*Z|Α.*Ω"));
|
||||
}
|
||||
|
||||
public void testPatternComplexity() {
|
||||
List<String> patterns = Arrays.asList("*", "filebeat*de-tst-chatclassification*",
|
||||
"metricbeat*de-tst-chatclassification*",
|
||||
"packetbeat*de-tst-chatclassification*",
|
||||
"heartbeat*de-tst-chatclassification*",
|
||||
"filebeat*documentationdev*",
|
||||
"metricbeat*documentationdev*",
|
||||
"packetbeat*documentationdev*",
|
||||
"heartbeat*documentationdev*",
|
||||
"filebeat*devsupport-website*",
|
||||
"metricbeat*devsupport-website*",
|
||||
"packetbeat*devsupport-website*",
|
||||
"heartbeat*devsupport-website*",
|
||||
".kibana-tcloud",
|
||||
".reporting-tcloud",
|
||||
"filebeat-app-ingress-*",
|
||||
"filebeat-app-tcloud-*",
|
||||
"filebeat*documentationprod*",
|
||||
"metricbeat*documentationprod*",
|
||||
"packetbeat*documentationprod*",
|
||||
"heartbeat*documentationprod*",
|
||||
"filebeat*bender-minio-test-1*",
|
||||
"metricbeat*bender-minio-test-1*",
|
||||
"packetbeat*bender-minio-test-1*",
|
||||
"heartbeat*bender-minio-test-1*");
|
||||
final Automaton automaton = Automatons.patterns(patterns);
|
||||
assertTrue(Operations.isTotal(automaton));
|
||||
assertTrue(automaton.isDeterministic());
|
||||
}
|
||||
|
||||
private void assertMatch(Automaton automaton, String text) {
|
||||
CharacterRunAutomaton runAutomaton = new CharacterRunAutomaton(automaton, DEFAULT_MAX_DETERMINIZED_STATES);
|
||||
assertTrue(runAutomaton.run(text));
|
||||
|
|
|
@ -12,6 +12,7 @@ import org.elasticsearch.action.search.SearchAction;
|
|||
import org.elasticsearch.cluster.metadata.AliasMetaData;
|
||||
import org.elasticsearch.cluster.metadata.IndexMetaData;
|
||||
import org.elasticsearch.cluster.metadata.MetaData;
|
||||
import org.elasticsearch.common.Strings;
|
||||
import org.elasticsearch.common.bytes.BytesArray;
|
||||
import org.elasticsearch.common.bytes.BytesReference;
|
||||
import org.elasticsearch.common.io.stream.BytesStreamOutput;
|
||||
|
@ -29,7 +30,9 @@ import org.elasticsearch.xpack.core.security.authz.permission.Role;
|
|||
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
|
@ -253,19 +256,15 @@ public class IndicesPermissionTests extends ESTestCase {
|
|||
}
|
||||
|
||||
public void testErrorMessageIfIndexPatternIsTooComplex() {
|
||||
final IndicesPermission.Group[] groups = new IndicesPermission.Group[26];
|
||||
for (int i = 0; i < 26; i++) {
|
||||
final char ch = (char) ('a' + i);
|
||||
String index = Character.toString(ch);
|
||||
for (int j = 0; j < 250; j++) {
|
||||
index += "*" + ch;
|
||||
}
|
||||
groups[i] = new IndicesPermission.Group(IndexPrivilege.ALL, new FieldPermissions(), null, index);
|
||||
List<String> indices = new ArrayList<>();
|
||||
for (int i = 0; i < 10; i++) {
|
||||
String prefix = randomAlphaOfLengthBetween(4, 12);
|
||||
String suffixBegin = randomAlphaOfLengthBetween(12, 36);
|
||||
indices.add("*" + prefix + "*" + suffixBegin + "*");
|
||||
}
|
||||
final IndicesPermission permission = new IndicesPermission(groups);
|
||||
final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class,
|
||||
() -> permission.allowedIndicesMatcher(GetAction.NAME));
|
||||
assertThat(e.getMessage(), containsString("a*a*a*a*a"));
|
||||
() -> new IndicesPermission.Group(IndexPrivilege.ALL, new FieldPermissions(), null, indices.toArray(Strings.EMPTY_ARRAY)));
|
||||
assertThat(e.getMessage(), containsString(indices.get(0)));
|
||||
assertThat(e.getMessage(), containsString("too complex to evaluate"));
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue