Skip to content

Commit 77a59ad

Browse files
tvernumtvernum
authored
Extract security wildcard pattern matching (#65404)
This commit moves the implementation of wildcard pattern matching into a standalone utility class ("StringMatcher"). In general, we rely on lucene Automaton objects to implement pattern matching (wildcards and regexp) within Elasticsearch security - for example in Index name patterns within a role. The IndicesPermission class also has a special optimisation for exact string matches (that is raw index names that contain no wildcards) as using String.equals / Set.contains is more efficient for this common case. All of the above functionality has now been extracted into the StringMatcher class, and it is now used in several places where it may be more efficient that the previous use of raw Automaton objects. A future change will expand this StringMatcher class with additional optimisations for common use cases that are poorly handled within our existing automaton compilation process. Relates: #36062
1 parent 00c9533 commit 77a59ad

File tree

8 files changed

+380
-75
lines changed

8 files changed

+380
-75
lines changed

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

Lines changed: 13 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,21 @@
55
*/
66
package org.elasticsearch.xpack.core.security.authz.permission;
77

8-
import org.apache.logging.log4j.LogManager;
98
import org.apache.lucene.util.automaton.Automaton;
109
import org.apache.lucene.util.automaton.Operations;
11-
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
12-
import org.elasticsearch.ElasticsearchSecurityException;
1310
import org.elasticsearch.action.admin.indices.mapping.put.AutoPutMappingAction;
1411
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction;
1512
import org.elasticsearch.cluster.metadata.IndexAbstraction;
1613
import org.elasticsearch.cluster.metadata.IndexMetadata;
1714
import org.elasticsearch.common.Nullable;
18-
import org.elasticsearch.common.Strings;
1915
import org.elasticsearch.common.bytes.BytesReference;
2016
import org.elasticsearch.common.logging.DeprecationLogger;
2117
import org.elasticsearch.common.regex.Regex;
2218
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
2319
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
2420
import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames;
2521
import org.elasticsearch.xpack.core.security.support.Automatons;
22+
import org.elasticsearch.xpack.core.security.support.StringMatcher;
2623

2724
import java.util.ArrayList;
2825
import java.util.Arrays;
@@ -60,65 +57,18 @@ public IndicesPermission(Group... groups) {
6057
this.groups = groups;
6158
}
6259

63-
private static Predicate<String> indexMatcher(Collection<String> ordinaryIndices, Collection<String> restrictedIndices) {
64-
Predicate<String> namePredicate;
60+
private static StringMatcher indexMatcher(Collection<String> ordinaryIndices, Collection<String> restrictedIndices) {
61+
StringMatcher matcher;
6562
if (ordinaryIndices.isEmpty()) {
66-
namePredicate = indexMatcher(restrictedIndices);
63+
matcher = StringMatcher.of(restrictedIndices);
6764
} else {
68-
namePredicate = indexMatcher(ordinaryIndices)
69-
.and(index -> false == RestrictedIndicesNames.isRestricted(index));
65+
matcher = StringMatcher.of(ordinaryIndices)
66+
.and("<not-restricted>", index -> false == RestrictedIndicesNames.isRestricted(index));
7067
if (restrictedIndices.isEmpty() == false) {
71-
namePredicate = indexMatcher(restrictedIndices).or(namePredicate);
68+
matcher = StringMatcher.of(restrictedIndices).or(matcher);
7269
}
7370
}
74-
return namePredicate;
75-
}
76-
77-
/**
78-
* Given a collection of index names and patterns, this constructs a {@code Predicate} that tests
79-
* {@code true} for the names in the collection as well as for any names matching the patterns in the collection.
80-
*/
81-
public static Predicate<String> indexMatcher(Collection<String> indices) {
82-
Set<String> exactMatch = new HashSet<>();
83-
List<String> nonExactMatch = new ArrayList<>();
84-
for (String indexPattern : indices) {
85-
if (indexPattern.startsWith("/") || indexPattern.contains("*") || indexPattern.contains("?")) {
86-
nonExactMatch.add(indexPattern);
87-
} else {
88-
exactMatch.add(indexPattern);
89-
}
90-
}
91-
92-
if (exactMatch.isEmpty() && nonExactMatch.isEmpty()) {
93-
return s -> false;
94-
} else if (exactMatch.isEmpty()) {
95-
return buildAutomataPredicate(nonExactMatch);
96-
} else if (nonExactMatch.isEmpty()) {
97-
return buildExactMatchPredicate(exactMatch);
98-
} else {
99-
return buildExactMatchPredicate(exactMatch).or(buildAutomataPredicate(nonExactMatch));
100-
}
101-
}
102-
103-
private static Predicate<String> buildExactMatchPredicate(Set<String> indices) {
104-
if (indices.size() == 1) {
105-
final String singleValue = indices.iterator().next();
106-
return singleValue::equals;
107-
}
108-
return indices::contains;
109-
}
110-
111-
private static Predicate<String> buildAutomataPredicate(List<String> indices) {
112-
try {
113-
return Automatons.predicate(indices);
114-
} catch (TooComplexToDeterminizeException e) {
115-
LogManager.getLogger(IndicesPermission.class).debug("Index pattern automaton [{}] is too complex", indices);
116-
String description = Strings.collectionToCommaDelimitedString(indices);
117-
if (description.length() > 80) {
118-
description = Strings.cleanTruncate(description, 80) + "...";
119-
}
120-
throw new ElasticsearchSecurityException("The set of permitted index patterns [{}] is too complex to evaluate", e, description);
121-
}
71+
return matcher;
12272
}
12373

12474
public Group[] groups() {
@@ -373,7 +323,7 @@ public Group(IndexPrivilege privilege, FieldPermissions fieldPermissions, @Nulla
373323
this.privilege = privilege;
374324
this.actionMatcher = privilege.predicate();
375325
this.indices = indices;
376-
this.indexNameMatcher = indexMatcher(Arrays.asList(indices));
326+
this.indexNameMatcher = StringMatcher.of(Arrays.asList(indices));
377327
this.fieldPermissions = Objects.requireNonNull(fieldPermissions);
378328
this.query = query;
379329
this.allowRestrictedIndices = allowRestrictedIndices;
@@ -445,14 +395,14 @@ private static Predicate<IndexAbstraction> buildIndexMatcherPredicateForAction(S
445395
}
446396
}
447397
}
448-
final Predicate<String> namePredicate = indexMatcher(ordinaryIndices, restrictedIndices);
449-
final Predicate<String> bwcSpecialCaseNamePredicate = indexMatcher(grantMappingUpdatesOnIndices,
398+
final StringMatcher nameMatcher = indexMatcher(ordinaryIndices, restrictedIndices);
399+
final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices,
450400
grantMappingUpdatesOnRestrictedIndices);
451401
return indexAbstraction -> {
452-
return namePredicate.test(indexAbstraction.getName()) ||
402+
return nameMatcher.test(indexAbstraction.getName()) ||
453403
(indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM &&
454404
(indexAbstraction.getParentDataStream() == null) &&
455-
bwcSpecialCaseNamePredicate.test(indexAbstraction.getName()));
405+
bwcSpecialCaseMatcher.test(indexAbstraction.getName()));
456406
};
457407
}
458408
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import org.elasticsearch.xpack.core.security.action.privilege.ApplicationPrivilegesRequest;
2020
import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission;
2121
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege.Category;
22-
import org.elasticsearch.xpack.core.security.support.Automatons;
22+
import org.elasticsearch.xpack.core.security.support.StringMatcher;
2323
import org.elasticsearch.xpack.core.security.xcontent.XContentUtils;
2424

2525
import java.io.IOException;
@@ -133,7 +133,7 @@ public static class ManageApplicationPrivileges implements ConfigurableClusterPr
133133

134134
public ManageApplicationPrivileges(Set<String> applicationNames) {
135135
this.applicationNames = Collections.unmodifiableSet(applicationNames);
136-
this.applicationPredicate = Automatons.predicate(applicationNames);
136+
this.applicationPredicate = StringMatcher.of(applicationNames);
137137
this.requestPredicate = request -> {
138138
if (request instanceof ApplicationPrivilegesRequest) {
139139
final ApplicationPrivilegesRequest privRequest = (ApplicationPrivilegesRequest) request;

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/SystemPrivilege.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import org.elasticsearch.index.seqno.RetentionLeaseSyncAction;
1111
import org.elasticsearch.persistent.CompletionPersistentTaskAction;
1212
import org.elasticsearch.transport.TransportActionProxy;
13-
import org.elasticsearch.xpack.core.security.support.Automatons;
13+
import org.elasticsearch.xpack.core.security.support.StringMatcher;
1414

1515
import java.util.Collections;
1616
import java.util.function.Predicate;
@@ -19,7 +19,7 @@ public final class SystemPrivilege extends Privilege {
1919

2020
public static SystemPrivilege INSTANCE = new SystemPrivilege();
2121

22-
private static final Predicate<String> ALLOWED_ACTIONS = Automatons.predicate(
22+
private static final Predicate<String> ALLOWED_ACTIONS = StringMatcher.of(
2323
"internal:*",
2424
"indices:monitor/*", // added for monitoring
2525
"cluster:monitor/*", // added for monitoring

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

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.core.security.support;
8+
9+
import org.apache.logging.log4j.LogManager;
10+
import org.apache.logging.log4j.Logger;
11+
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
12+
import org.elasticsearch.ElasticsearchSecurityException;
13+
import org.elasticsearch.common.Strings;
14+
15+
import java.util.ArrayList;
16+
import java.util.Collection;
17+
import java.util.HashSet;
18+
import java.util.LinkedHashSet;
19+
import java.util.List;
20+
import java.util.Objects;
21+
import java.util.Set;
22+
import java.util.function.Predicate;
23+
import java.util.stream.Collectors;
24+
25+
/**
26+
* This class acts as a facade / encapsulation around the expression and testing of string-based patterns within Elasticsearch security.
27+
* Security supports "wildcards" in a number of places (e.g. index names within roles). These cases also support
28+
* {@link org.apache.lucene.util.automaton.RegExp Lucene-syntax regular expressions} and are implemented via Lucene
29+
* {@link org.apache.lucene.util.automaton.Automaton} objects.
30+
* However, it can be more efficient to have special handling and avoid {@code Automata} for particular cases such as exact string matches.
31+
* This class handles that logic, an provides a clean interface for
32+
* <em>test whether a provided string matches one of an existing set of patterns</em> that hides the possible implementation options.
33+
*/
34+
public class StringMatcher implements Predicate<String> {
35+
36+
private static final StringMatcher MATCH_NOTHING = new StringMatcher("(empty)", s -> false);
37+
38+
private final String description;
39+
private final Predicate<String> predicate;
40+
private static final Logger LOGGER = LogManager.getLogger(StringMatcher.class);
41+
42+
private StringMatcher(String description, Predicate<String> predicate) {
43+
this.description = description;
44+
this.predicate = predicate;
45+
}
46+
47+
public static StringMatcher of(Iterable<String> patterns) {
48+
return StringMatcher.builder().includeAll(patterns).build();
49+
}
50+
51+
public static StringMatcher of(String... patterns) {
52+
return StringMatcher.builder().includeAll(patterns).build();
53+
}
54+
55+
public static Builder builder() {
56+
return new Builder();
57+
}
58+
59+
@Override
60+
public String toString() {
61+
return description;
62+
}
63+
64+
@Override
65+
public boolean test(String s) {
66+
return predicate.test(s);
67+
}
68+
69+
@Override
70+
public StringMatcher or(Predicate<? super String> other) {
71+
Objects.requireNonNull(other);
72+
return new StringMatcher(description + "|" + other, this.predicate.or(other));
73+
}
74+
75+
@Override
76+
public StringMatcher and(Predicate<? super String> other) {
77+
return this.and(String.valueOf(other), other);
78+
}
79+
80+
public StringMatcher and(String otherDescription, Predicate<? super String> otherPredicate) {
81+
Objects.requireNonNull(otherPredicate);
82+
return new StringMatcher(this.description + "&" + otherDescription, this.predicate.and(otherPredicate));
83+
}
84+
85+
public static class Builder {
86+
private final List<String> allText = new ArrayList<>();
87+
private final Set<String> exactMatch = new HashSet<>();
88+
private final Set<String> nonExactMatch = new LinkedHashSet<>();
89+
90+
public Builder include(String pattern) {
91+
allText.add(pattern);
92+
if (pattern.startsWith("/") || pattern.contains("*") || pattern.contains("?")) {
93+
nonExactMatch.add(pattern);
94+
} else {
95+
exactMatch.add(pattern);
96+
}
97+
return this;
98+
}
99+
100+
public Builder includeAll(String... patterns) {
101+
for (String pattern : patterns) {
102+
include(pattern);
103+
}
104+
return this;
105+
}
106+
107+
public Builder includeAll(Iterable<String> patterns) {
108+
for (String pattern : patterns) {
109+
include(pattern);
110+
}
111+
return this;
112+
}
113+
114+
public StringMatcher build() {
115+
if (allText.isEmpty()) {
116+
return MATCH_NOTHING;
117+
}
118+
119+
final String description = describe(allText);
120+
if (exactMatch.isEmpty()) {
121+
return new StringMatcher(description, buildAutomataPredicate(nonExactMatch));
122+
}
123+
if (nonExactMatch.isEmpty()) {
124+
return new StringMatcher(description, buildExactMatchPredicate(exactMatch));
125+
}
126+
final Predicate<String> predicate = buildExactMatchPredicate(exactMatch).or(buildAutomataPredicate(nonExactMatch));
127+
return new StringMatcher(description, predicate);
128+
}
129+
130+
private static String describe(List<String> strings) {
131+
if (strings.size() == 1) {
132+
return strings.get(0);
133+
}
134+
final int totalLength = strings.stream().map(String::length).reduce(0, Math::addExact);
135+
if (totalLength < 250) {
136+
return Strings.collectionToDelimitedString(strings, "|");
137+
}
138+
final int maxItemLength = Math.max(16, 250 / strings.size());
139+
return strings.stream().map(s -> {
140+
if (s.length() > maxItemLength) {
141+
return Strings.cleanTruncate(s, maxItemLength - 3) + "...";
142+
} else {
143+
return s;
144+
}
145+
}).collect(Collectors.joining("|"));
146+
}
147+
148+
private static Predicate<String> buildExactMatchPredicate(Set<String> stringValues) {
149+
if (stringValues.size() == 1) {
150+
final String singleValue = stringValues.iterator().next();
151+
return singleValue::equals;
152+
}
153+
return stringValues::contains;
154+
}
155+
156+
private static Predicate<String> buildAutomataPredicate(Collection<String> patterns) {
157+
try {
158+
return Automatons.predicate(patterns);
159+
} catch (TooComplexToDeterminizeException e) {
160+
LOGGER.debug("Pattern automaton [{}] is too complex", patterns);
161+
String description = Strings.collectionToCommaDelimitedString(patterns);
162+
if (description.length() > 80) {
163+
description = Strings.cleanTruncate(description, 80) + "...";
164+
}
165+
throw new ElasticsearchSecurityException("The set patterns [{}] is too complex to evaluate", e, description);
166+
}
167+
}
168+
}
169+
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@
99
import org.apache.lucene.util.automaton.CharacterRunAutomaton;
1010
import org.apache.lucene.util.automaton.Operations;
1111
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
12+
import org.apache.lucene.util.automaton.Transition;
1213
import org.elasticsearch.common.settings.Settings;
1314
import org.elasticsearch.test.ESTestCase;
1415

16+
import java.io.IOException;
1517
import java.util.ArrayList;
1618
import java.util.Arrays;
1719
import java.util.List;
20+
import java.util.Locale;
1821

1922
import static org.apache.lucene.util.automaton.Operations.DEFAULT_MAX_DETERMINIZED_STATES;
2023
import static org.elasticsearch.xpack.core.security.support.Automatons.pattern;
@@ -64,6 +67,13 @@ public void testWildcard() throws Exception {
6467
assertMatch(wildcard("t\\*st"), "t*st");
6568
}
6669

70+
public void testUnicode() throws Exception {
71+
assertMatch(wildcard("*ξη"), "λέξη");
72+
assertMatch(wildcard("сл*во"), "слово");
73+
assertMatch(wildcard("စကာ*"), "စကားလုံး");
74+
assertMatch(wildcard("וואָרט"), "וואָרט");
75+
}
76+
6777
public void testPredicateToString() throws Exception {
6878
assertThat(predicate("a.*z").toString(), equalTo("a.*z"));
6979
assertThat(predicate("a.*z", "A.*Z").toString(), equalTo("a.*z|A.*Z"));
@@ -208,4 +218,21 @@ public void testDisableCache() {
208218
final Automaton automaton = Automatons.pattern(pattern);
209219
assertThat(Automatons.pattern(pattern), not(sameInstance(automaton)));
210220
}
221+
222+
// This isn't use directly in the code, but it's sometimes needed when debugging failing tests
223+
// (and it is annoying to have to rewrite it each time it's needed)
224+
public static <A extends Appendable> A debug(Automaton a, A out) throws IOException {
225+
out.append("Automaton {");
226+
out.append(String.format(Locale.ROOT, "States:%d Deterministic:%s", a.getNumStates(), a.isDeterministic()));
227+
for (int s = 0; s < a.getNumStates(); s++) {
228+
out.append(String.format(Locale.ROOT, " [State#%d %s", s, a.isAccept(s) ? "(accept)" : ""));
229+
for (int t = 0; t < a.getNumTransitions(s); t++) {
230+
Transition transition = new Transition();
231+
a.getTransition(s, t, transition);
232+
out.append(String.format(Locale.ROOT, " (%05d - %05d => %s)", transition.min, transition.max, transition.dest));
233+
}
234+
out.append("]");
235+
}
236+
return out;
237+
}
211238
}

0 commit comments

Comments
 (0)