Skip to content

Commit 5f132e3

Browse files
tvernumkcm
tvernum
authored and
kcm
committed
Enable security automaton caching (#34028)
Building automatons can be costly. For the most part we cache things that use automatons so the cost is limited. However: - We don't (currently) do that everywhere (e.g. we don't cache role mappings) - It is sometimes necessary to clear some of those caches which can cause significant CPU overhead and processing delays. This commit introduces a new cache in the Automatons class to avoid unnecesarily recomputing automatons.
1 parent 07f2df4 commit 5f132e3

File tree

4 files changed

+184
-34
lines changed

4 files changed

+184
-34
lines changed

‎docs/reference/settings/security-settings.asciidoc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ Enables fips mode of operation. Set this to `true` if you run this {es} instance
5555
`xpack.security.authc.accept_default_password`::
5656
In `elasticsearch.yml`, set this to `false` to disable support for the default "changeme" password.
5757

58+
[float]
5859
[[password-hashing-settings]]
5960
==== Password hashing settings
6061
`xpack.security.authc.password_hashing.algorithm`::
@@ -82,6 +83,33 @@ resource. When set to `false`, an HTTP 401 response is returned and the user
8283
can provide credentials with the appropriate permissions to gain
8384
access. Defaults to `true`.
8485

86+
[float]
87+
[[security-automata-settings]]
88+
==== Automata Settings
89+
In places where {security} accepts wildcard patterns (e.g. index patterns in
90+
roles, group matches in the role mapping API), each pattern is compiled into
91+
an Automaton. The follow settings are available to control this behaviour.
92+
93+
`xpack.security.automata.max_determinized_states`::
94+
The upper limit on how many automaton states may be created by a single pattern.
95+
This protects against too-difficult (e.g. exponentially hard) patterns.
96+
Defaults to `100,000`.
97+
98+
`xpack.security.automata.cache.enabled`::
99+
Whether to cache the compiled automata. Compiling automata can be CPU intensive
100+
and may slowdown some operations. The cache reduces the frequency with which
101+
automata need to be compiled.
102+
Defaults to `true`.
103+
104+
`xpack.security.automata.cache.size`::
105+
The maximum number of items to retain in the automata cache.
106+
Defaults to `10,000`.
107+
108+
`xpack.security.automata.cache.ttl`::
109+
The length of time to retain in an item in the automata cache (based on most
110+
recent usage).
111+
Defaults to `48h` (48 hours).
112+
85113
[float]
86114
[[field-document-security-settings]]
87115
==== Document and field level security settings

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

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,18 @@
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.cache.Cache;
13+
import org.elasticsearch.common.cache.CacheBuilder;
1214
import org.elasticsearch.common.settings.Setting;
1315
import org.elasticsearch.common.settings.Settings;
16+
import org.elasticsearch.common.unit.TimeValue;
17+
import org.elasticsearch.common.util.set.Sets;
1418

1519
import java.util.ArrayList;
1620
import java.util.Arrays;
1721
import java.util.Collection;
1822
import java.util.List;
23+
import java.util.concurrent.ExecutionException;
1924
import java.util.function.Predicate;
2025

2126
import static org.apache.lucene.util.automaton.MinimizationOperations.minimize;
@@ -27,14 +32,23 @@
2732

2833
public final class Automatons {
2934

30-
public static final Setting<Integer> MAX_DETERMINIZED_STATES_SETTING =
35+
static final Setting<Integer> MAX_DETERMINIZED_STATES_SETTING =
3136
Setting.intSetting("xpack.security.automata.max_determinized_states", 100000, DEFAULT_MAX_DETERMINIZED_STATES,
3237
Setting.Property.NodeScope);
38+
39+
static final Setting<Boolean> CACHE_ENABLED =
40+
Setting.boolSetting("xpack.security.automata.cache.enabled", true, Setting.Property.NodeScope);
41+
static final Setting<Integer> CACHE_SIZE =
42+
Setting.intSetting("xpack.security.automata.cache.size", 10_000, Setting.Property.NodeScope);
43+
static final Setting<TimeValue> CACHE_TTL =
44+
Setting.timeSetting("xpack.security.automata.cache.ttl", TimeValue.timeValueHours(48), Setting.Property.NodeScope);
45+
3346
public static final Automaton EMPTY = Automata.makeEmpty();
3447
public static final Automaton MATCH_ALL = Automata.makeAnyString();
3548

36-
// this value is not final since we allow it to be set at runtime
49+
// these values are not final since we allow them to be set at runtime
3750
private static int maxDeterminizedStates = 100000;
51+
private static Cache<Object, Automaton> cache = buildCache(Settings.EMPTY);
3852

3953
static final char WILDCARD_STRING = '*'; // String equality with support for wildcards
4054
static final char WILDCARD_CHAR = '?'; // Char equality with support for wildcards
@@ -57,6 +71,18 @@ public static Automaton patterns(Collection<String> patterns) {
5771
if (patterns.isEmpty()) {
5872
return EMPTY;
5973
}
74+
if (cache == null) {
75+
return buildAutomaton(patterns);
76+
} else {
77+
try {
78+
return cache.computeIfAbsent(Sets.newHashSet(patterns), ignore -> buildAutomaton(patterns));
79+
} catch (ExecutionException e) {
80+
throw unwrapCacheException(e);
81+
}
82+
}
83+
}
84+
85+
private static Automaton buildAutomaton(Collection<String> patterns) {
6086
List<Automaton> automata = new ArrayList<>(patterns.size());
6187
for (String pattern : patterns) {
6288
final Automaton patternAutomaton = pattern(pattern);
@@ -69,11 +95,23 @@ public static Automaton patterns(Collection<String> patterns) {
6995
* Builds and returns an automaton that represents the given pattern.
7096
*/
7197
static Automaton pattern(String pattern) {
98+
if (cache == null) {
99+
return buildAutomaton(pattern);
100+
} else {
101+
try {
102+
return cache.computeIfAbsent(pattern, ignore -> buildAutomaton(pattern));
103+
} catch (ExecutionException e) {
104+
throw unwrapCacheException(e);
105+
}
106+
}
107+
}
108+
109+
private static Automaton buildAutomaton(String pattern) {
72110
if (pattern.startsWith("/")) { // it's a lucene regexp
73111
if (pattern.length() == 1 || !pattern.endsWith("/")) {
74112
throw new IllegalArgumentException("invalid pattern [" + pattern + "]. patterns starting with '/' " +
75-
"indicate regular expression pattern and therefore must also end with '/'." +
76-
" other patterns (those that do not start with '/') will be treated as simple wildcard patterns");
113+
"indicate regular expression pattern and therefore must also end with '/'." +
114+
" other patterns (those that do not start with '/') will be treated as simple wildcard patterns");
77115
}
78116
String regex = pattern.substring(1, pattern.length() - 1);
79117
return new RegExp(regex).toAutomaton();
@@ -84,16 +122,25 @@ static Automaton pattern(String pattern) {
84122
}
85123
}
86124

125+
private static RuntimeException unwrapCacheException(ExecutionException e) {
126+
final Throwable cause = e.getCause();
127+
if (cause instanceof RuntimeException) {
128+
return (RuntimeException) cause;
129+
} else {
130+
return new RuntimeException(cause);
131+
}
132+
}
133+
87134
/**
88135
* Builds and returns an automaton that represents the given pattern.
89136
*/
90137
@SuppressWarnings("fallthrough") // explicit fallthrough at end of switch
91138
static Automaton wildcard(String text) {
92139
List<Automaton> automata = new ArrayList<>();
93-
for (int i = 0; i < text.length();) {
140+
for (int i = 0; i < text.length(); ) {
94141
final char c = text.charAt(i);
95142
int length = 1;
96-
switch(c) {
143+
switch (c) {
97144
case WILDCARD_STRING:
98145
automata.add(Automata.makeAnyString());
99146
break;
@@ -138,8 +185,19 @@ public static Predicate<String> predicate(Automaton automaton) {
138185
return predicate(automaton, "Predicate for " + automaton);
139186
}
140187

141-
public static void updateMaxDeterminizedStates(Settings settings) {
188+
public static void updateConfiguration(Settings settings) {
142189
maxDeterminizedStates = MAX_DETERMINIZED_STATES_SETTING.get(settings);
190+
cache = buildCache(settings);
191+
}
192+
193+
private static Cache<Object, Automaton> buildCache(Settings settings) {
194+
if (CACHE_ENABLED.get(settings) == false) {
195+
return null;
196+
}
197+
return CacheBuilder.<Object, Automaton>builder()
198+
.setExpireAfterAccess(CACHE_TTL.get(settings))
199+
.setMaximumWeight(CACHE_SIZE.get(settings))
200+
.build();
143201
}
144202

145203
// accessor for testing
@@ -161,4 +219,11 @@ public String toString() {
161219
}
162220
};
163221
}
222+
223+
public static void addSettings(List<Setting<?>> settingsList) {
224+
settingsList.add(MAX_DETERMINIZED_STATES_SETTING);
225+
settingsList.add(CACHE_ENABLED);
226+
settingsList.add(CACHE_SIZE);
227+
settingsList.add(CACHE_TTL);
228+
}
164229
}

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

Lines changed: 82 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import static org.elasticsearch.xpack.core.security.support.Automatons.predicate;
2323
import static org.elasticsearch.xpack.core.security.support.Automatons.wildcard;
2424
import static org.hamcrest.Matchers.equalTo;
25+
import static org.hamcrest.Matchers.not;
26+
import static org.hamcrest.Matchers.sameInstance;
2527

2628
public class AutomatonsTests extends ESTestCase {
2729
public void testPatternsUnionOfMultiplePatterns() throws Exception {
@@ -70,29 +72,29 @@ public void testPredicateToString() throws Exception {
7072

7173
public void testPatternComplexity() {
7274
List<String> patterns = Arrays.asList("*", "filebeat*de-tst-chatclassification*",
73-
"metricbeat*de-tst-chatclassification*",
74-
"packetbeat*de-tst-chatclassification*",
75-
"heartbeat*de-tst-chatclassification*",
76-
"filebeat*documentationdev*",
77-
"metricbeat*documentationdev*",
78-
"packetbeat*documentationdev*",
79-
"heartbeat*documentationdev*",
80-
"filebeat*devsupport-website*",
81-
"metricbeat*devsupport-website*",
82-
"packetbeat*devsupport-website*",
83-
"heartbeat*devsupport-website*",
84-
".kibana-tcloud",
85-
".reporting-tcloud",
86-
"filebeat-app-ingress-*",
87-
"filebeat-app-tcloud-*",
88-
"filebeat*documentationprod*",
89-
"metricbeat*documentationprod*",
90-
"packetbeat*documentationprod*",
91-
"heartbeat*documentationprod*",
92-
"filebeat*bender-minio-test-1*",
93-
"metricbeat*bender-minio-test-1*",
94-
"packetbeat*bender-minio-test-1*",
95-
"heartbeat*bender-minio-test-1*");
75+
"metricbeat*de-tst-chatclassification*",
76+
"packetbeat*de-tst-chatclassification*",
77+
"heartbeat*de-tst-chatclassification*",
78+
"filebeat*documentationdev*",
79+
"metricbeat*documentationdev*",
80+
"packetbeat*documentationdev*",
81+
"heartbeat*documentationdev*",
82+
"filebeat*devsupport-website*",
83+
"metricbeat*devsupport-website*",
84+
"packetbeat*devsupport-website*",
85+
"heartbeat*devsupport-website*",
86+
".kibana-tcloud",
87+
".reporting-tcloud",
88+
"filebeat-app-ingress-*",
89+
"filebeat-app-tcloud-*",
90+
"filebeat*documentationprod*",
91+
"metricbeat*documentationprod*",
92+
"packetbeat*documentationprod*",
93+
"heartbeat*documentationprod*",
94+
"filebeat*bender-minio-test-1*",
95+
"metricbeat*bender-minio-test-1*",
96+
"packetbeat*bender-minio-test-1*",
97+
"heartbeat*bender-minio-test-1*");
9698
final Automaton automaton = Automatons.patterns(patterns);
9799
assertTrue(Operations.isTotal(automaton));
98100
assertTrue(automaton.isDeterministic());
@@ -137,7 +139,7 @@ public void testSettingMaxDeterminizedStates() {
137139
assertNotEquals(10000, Automatons.getMaxDeterminizedStates());
138140
// set to the min value
139141
Settings settings = Settings.builder().put(Automatons.MAX_DETERMINIZED_STATES_SETTING.getKey(), 10000).build();
140-
Automatons.updateMaxDeterminizedStates(settings);
142+
Automatons.updateConfiguration(settings);
141143
assertEquals(10000, Automatons.getMaxDeterminizedStates());
142144

143145
final List<String> names = new ArrayList<>(1024);
@@ -147,8 +149,63 @@ public void testSettingMaxDeterminizedStates() {
147149
TooComplexToDeterminizeException e = expectThrows(TooComplexToDeterminizeException.class, () -> Automatons.patterns(names));
148150
assertThat(e.getMaxDeterminizedStates(), equalTo(10000));
149151
} finally {
150-
Automatons.updateMaxDeterminizedStates(Settings.EMPTY);
152+
Automatons.updateConfiguration(Settings.EMPTY);
151153
assertEquals(100000, Automatons.getMaxDeterminizedStates());
152154
}
153155
}
156+
157+
public void testCachingOfAutomatons() {
158+
Automatons.updateConfiguration(Settings.EMPTY);
159+
160+
String pattern1 = randomAlphaOfLengthBetween(3, 8) + "*";
161+
String pattern2 = "/" + randomAlphaOfLengthBetween(1, 2) + "*" + randomAlphaOfLengthBetween(2, 4) + "/";
162+
163+
final Automaton a1 = Automatons.pattern(pattern1);
164+
final Automaton a2 = Automatons.pattern(pattern2);
165+
166+
assertThat(Automatons.pattern(pattern1), sameInstance(a1));
167+
assertThat(Automatons.pattern(pattern2), sameInstance(a2));
168+
169+
final Automaton a3 = Automatons.patterns(pattern1, pattern2);
170+
final Automaton a4 = Automatons.patterns(pattern2, pattern1);
171+
assertThat(a3, sameInstance(a4));
172+
}
173+
174+
public void testConfigurationOfCacheSize() {
175+
final Settings settings = Settings.builder()
176+
.put(Automatons.CACHE_SIZE.getKey(), 2)
177+
.build();
178+
Automatons.updateConfiguration(settings);
179+
180+
String pattern1 = "a";
181+
String pattern2 = "b";
182+
String pattern3 = "c";
183+
184+
final Automaton a1 = Automatons.pattern(pattern1);
185+
final Automaton a2 = Automatons.pattern(pattern2);
186+
187+
assertThat(Automatons.pattern(pattern1), sameInstance(a1));
188+
assertThat(Automatons.pattern(pattern2), sameInstance(a2));
189+
190+
final Automaton a3 = Automatons.pattern(pattern3);
191+
assertThat(Automatons.pattern(pattern3), sameInstance(a3));
192+
193+
// either pattern 1 or 2 should be evicted (in theory it should be 1, but we don't care about that level of precision)
194+
final Automaton a1b = Automatons.pattern(pattern1);
195+
final Automaton a2b = Automatons.pattern(pattern2);
196+
if (a1b == a1 && a2b == a2) {
197+
fail("Expected one of the existing automatons to be evicted, but both were still cached");
198+
}
199+
}
200+
201+
public void testDisableCache() {
202+
final Settings settings = Settings.builder()
203+
.put(Automatons.CACHE_ENABLED.getKey(), false)
204+
.build();
205+
Automatons.updateConfiguration(settings);
206+
207+
final String pattern = randomAlphaOfLengthBetween(5, 10);
208+
final Automaton automaton = Automatons.pattern(pattern);
209+
assertThat(Automatons.pattern(pattern), not(sameInstance(automaton)));
210+
}
154211
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ public Security(Settings settings, final Path configPath) {
307307
new FIPS140LicenseBootstrapCheck()));
308308
checks.addAll(InternalRealms.getBootstrapChecks(settings, env));
309309
this.bootstrapChecks = Collections.unmodifiableList(checks);
310-
Automatons.updateMaxDeterminizedStates(settings);
310+
Automatons.updateConfiguration(settings);
311311
} else {
312312
this.bootstrapChecks = Collections.emptyList();
313313
}
@@ -609,7 +609,7 @@ public static List<Setting<?>> getSettings(boolean transportClientMode, List<Sec
609609
ReservedRealm.addSettings(settingsList);
610610
AuthenticationService.addSettings(settingsList);
611611
AuthorizationService.addSettings(settingsList);
612-
settingsList.add(Automatons.MAX_DETERMINIZED_STATES_SETTING);
612+
Automatons.addSettings(settingsList);
613613
settingsList.add(CompositeRolesStore.CACHE_SIZE_SETTING);
614614
settingsList.add(FieldPermissionsCache.CACHE_SIZE_SETTING);
615615
settingsList.add(TokenService.TOKEN_EXPIRATION);

0 commit comments

Comments
 (0)