Skip to content

Commit 41141a5

Browse files
committed
Check TooComplex exception for HasPrivileges body
The index pattern provided in the body of `_has_privileges` can trigger a `TooComplexToDeterminizeException` which is then bubbled up (badly). This change catches that exception and provides a better message
1 parent f988611 commit 41141a5

File tree

2 files changed

+46
-5
lines changed

2 files changed

+46
-5
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.apache.lucene.util.automaton.Automaton;
1010
import org.apache.lucene.util.automaton.Operations;
11+
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
1112
import org.elasticsearch.action.admin.indices.mapping.put.TransportAutoPutMappingAction;
1213
import org.elasticsearch.action.admin.indices.mapping.put.TransportPutMappingAction;
1314
import org.elasticsearch.action.support.IndexComponentSelector;
@@ -43,8 +44,10 @@
4344
import java.util.Set;
4445
import java.util.concurrent.ConcurrentHashMap;
4546
import java.util.function.BiPredicate;
47+
import java.util.function.Function;
4648
import java.util.function.Predicate;
4749
import java.util.function.Supplier;
50+
import java.util.stream.Collectors;
4851

4952
import static java.util.Collections.unmodifiableMap;
5053

@@ -330,11 +333,22 @@ public boolean checkResourcePrivileges(
330333
combineIndexGroups && checkForIndexPatterns.stream().anyMatch(Automatons::isLuceneRegex),
331334
IndexComponentSelector.FAILURES
332335
);
333-
for (String forIndexPattern : checkForIndexPatterns) {
334-
Automaton checkIndexAutomaton = Automatons.patterns(forIndexPattern);
335-
if (false == allowRestrictedIndices && false == isConcreteRestrictedIndex(forIndexPattern)) {
336-
checkIndexAutomaton = Automatons.minusAndMinimize(checkIndexAutomaton, restrictedIndices.getAutomaton());
337-
}
336+
Map<String, Automaton> checkIndexPatterns = checkForIndexPatterns.stream()
337+
.collect(Collectors.toMap(Function.identity(), pattern -> {
338+
try {
339+
Automaton automaton = Automatons.patterns(pattern);
340+
if (false == allowRestrictedIndices && false == isConcreteRestrictedIndex(pattern)) {
341+
automaton = Automatons.minusAndMinimize(automaton, restrictedIndices.getAutomaton());
342+
}
343+
return automaton;
344+
} catch (TooComplexToDeterminizeException e) {
345+
final String text = pattern.length() > 260 ? Strings.cleanTruncate(pattern, 256) + "..." : pattern;
346+
throw new IllegalArgumentException("the provided index pattern [" + text + "] is too complex to be evaluated", e);
347+
}
348+
}));
349+
for (var entry : checkIndexPatterns.entrySet()) {
350+
final String forIndexPattern = entry.getKey();
351+
final Automaton checkIndexAutomaton = entry.getValue();
338352
if (false == Operations.isEmpty(checkIndexAutomaton)) {
339353
Automaton allowedPrivilegesAutomatonForDataSelector = getIndexPrivilegesAutomaton(
340354
indexGroupAutomatonsForDataSelector,

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77
package org.elasticsearch.xpack.security.authz.accesscontrol;
88

9+
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
910
import org.elasticsearch.ElasticsearchSecurityException;
1011
import org.elasticsearch.TransportVersion;
1112
import org.elasticsearch.action.admin.indices.mapping.put.TransportAutoPutMappingAction;
@@ -55,6 +56,7 @@
5556
import static org.hamcrest.Matchers.containsString;
5657
import static org.hamcrest.Matchers.equalTo;
5758
import static org.hamcrest.Matchers.hasSize;
59+
import static org.hamcrest.Matchers.instanceOf;
5860
import static org.hamcrest.Matchers.is;
5961
import static org.hamcrest.Matchers.notNullValue;
6062
import static org.hamcrest.Matchers.nullValue;
@@ -1090,6 +1092,31 @@ public void testResourceAuthorizedPredicateAndWithFailures() {
10901092
assertThat(predicate.test(concreteIndexF, IndexComponentSelector.FAILURES), is(true));
10911093
}
10921094

1095+
public void testCheckResourcePrivilegesWithTooComplexAutomaton() {
1096+
IndicesPermission permission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup(
1097+
IndexPrivilege.ALL,
1098+
FieldPermissions.DEFAULT,
1099+
null,
1100+
false,
1101+
"my-index"
1102+
).build();
1103+
1104+
StringBuilder pattern = new StringBuilder("/");
1105+
for (int i = 0; i < 2048; i++) {
1106+
if (i > 0) {
1107+
pattern.append("|");
1108+
}
1109+
pattern.append(randomAlphaOfLength(64));
1110+
}
1111+
pattern.append("/");
1112+
var ex = expectThrows(
1113+
IllegalArgumentException.class,
1114+
() -> permission.checkResourcePrivileges(Set.of(pattern.toString()), false, Set.of("read"), null)
1115+
);
1116+
assertThat(ex.getMessage(), containsString("index pattern [/"));
1117+
assertThat(ex.getCause(), instanceOf(TooComplexToDeterminizeException.class));
1118+
}
1119+
10931120
private static IndexAbstraction concreteIndexAbstraction(String name) {
10941121
return new IndexAbstraction.ConcreteIndex(
10951122
IndexMetadata.builder(name).settings(indexSettings(IndexVersion.current(), 1, 0)).build()

0 commit comments

Comments
 (0)