Skip to content

Commit 69445a6

Browse files
nibixnibix
authored
Optimizations and simplifications for WildcardMatcher (#5470)
Signed-off-by: Nils Bandener <[email protected]>
1 parent 0d26b1f commit 69445a6

File tree

8 files changed

+482
-198
lines changed

8 files changed

+482
-198
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1717
* Moves OpenSAML jars to a Shadow Jar configuration to facilitate its use in FIPS enabled environments ([#5400](https://github.com/opensearch-project/security/pull/5404))
1818
* Replaced the standard distribution of BouncyCastle with BC-FIPS ([#5439](https://github.com/opensearch-project/security/pull/5439))
1919
* Introduced setting `plugins.security.privileges_evaluation.precomputed_privileges.enabled` ([#5465](https://github.com/opensearch-project/security/pull/5465))
20+
* Optimized wildcard matching runtime performance ([#5470](https://github.com/opensearch-project/security/pull/5470))
21+
2022
### Bug Fixes
2123

2224
* Fix compilation issue after change to Subject interface in core and bump to 3.2.0 ([#5423](https://github.com/opensearch-project/security/pull/5423))

‎src/integrationTest/java/org/opensearch/security/support/WildcardMatcherTest.java‎

Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
12+
package org.opensearch.security.support;
13+
14+
import java.util.Arrays;
15+
import java.util.Collection;
16+
import java.util.Collections;
17+
import java.util.stream.Collectors;
18+
import java.util.stream.Stream;
19+
import java.util.stream.StreamSupport;
20+
21+
import org.junit.Test;
22+
import org.junit.runner.RunWith;
23+
import org.junit.runners.Parameterized;
24+
25+
import static org.junit.Assert.assertEquals;
26+
import static org.junit.Assert.assertFalse;
27+
import static org.junit.Assert.assertNotEquals;
28+
import static org.junit.Assert.assertSame;
29+
import static org.junit.Assert.assertTrue;
30+
31+
@RunWith(Parameterized.class)
32+
public class WildcardMatcherTest {
33+
boolean ignoreCase;
34+
35+
@Test
36+
public void any() {
37+
WildcardMatcher subject = applyCase(WildcardMatcher.ANY);
38+
assertTrue(subject.test("any_string"));
39+
assertTrue(subject.test(""));
40+
assertTrue(subject.matchAny("any_string"));
41+
assertTrue(subject.matchAny(Arrays.asList("any_string")));
42+
assertTrue(subject.matchAny(Stream.of("any_string")));
43+
assertEquals("*", subject.toString());
44+
}
45+
46+
@Test
47+
public void none() {
48+
WildcardMatcher subject = applyCase(WildcardMatcher.NONE);
49+
assertFalse(subject.test("any_string"));
50+
assertFalse(subject.test(""));
51+
assertFalse(subject.matchAny("any_string"));
52+
assertFalse(subject.matchAny(Arrays.asList("any_string")));
53+
assertFalse(subject.matchAny(Stream.of("any_string")));
54+
assertEquals("<NONE>", subject.toString());
55+
}
56+
57+
@Test
58+
public void anyFromStar() {
59+
assertSame(WildcardMatcher.ANY, applyCase(WildcardMatcher.from("*")));
60+
}
61+
62+
@Test
63+
public void noneFromNull() {
64+
assertSame(WildcardMatcher.NONE, applyCase(WildcardMatcher.from()));
65+
assertSame(WildcardMatcher.NONE, applyCase(WildcardMatcher.from((String) null)));
66+
}
67+
68+
@Test
69+
public void exact() {
70+
String base = "exact_string";
71+
WildcardMatcher subject = applyCase(WildcardMatcher.from(base));
72+
assertTrue(subject instanceof WildcardMatcher.Exact);
73+
assertTrue(subject.test(base));
74+
assertFalse(subject.test(base + "_x"));
75+
assertEquals(ignoreCase, subject.test(base.toUpperCase()));
76+
assertTrue(subject.matchAny(base));
77+
assertTrue(subject.matchAny(base, "foo"));
78+
assertFalse(subject.matchAny("foo"));
79+
assertTrue(subject.matchAny(Arrays.asList(base, "foo")));
80+
assertFalse(subject.matchAny(Arrays.asList("foo")));
81+
assertEquals(base, subject.toString());
82+
assertEquals(applyCase(WildcardMatcher.from(base)), subject);
83+
assertNotEquals(applyCase(WildcardMatcher.from(base + "x")), subject);
84+
assertNotEquals(applyCase(WildcardMatcher.from(base + "*")), subject);
85+
assertEquals(Arrays.asList(base), subject.getMatchAny(Arrays.asList(base, "other"), Collectors.toList()));
86+
}
87+
88+
@Test
89+
public void prefix() {
90+
String base = "prefix_string";
91+
WildcardMatcher subject = applyCase(WildcardMatcher.from(base + "*"));
92+
assertTrue(subject instanceof WildcardMatcher.PrefixMatcher);
93+
assertTrue(subject.test(base + "_more"));
94+
assertTrue(subject.test(base));
95+
assertFalse(subject.test(base.substring(0, 5)));
96+
assertFalse(subject.test("more_" + base));
97+
assertEquals(ignoreCase, subject.test(base.toUpperCase()));
98+
assertEquals(ignoreCase, subject.test(base.toUpperCase() + "_more"));
99+
assertTrue(subject.matchAny(base));
100+
assertTrue(subject.matchAny(base, "foo"));
101+
assertFalse(subject.matchAny("foo"));
102+
assertEquals(base + "*", subject.toString());
103+
assertEquals(applyCase(WildcardMatcher.from(subject.toString())), subject);
104+
assertNotEquals(applyCase(WildcardMatcher.from(subject.toString() + "x")), subject);
105+
assertEquals(
106+
Arrays.asList(base, base + "_more"),
107+
subject.getMatchAny(Arrays.asList(base, base + "_more", "other"), Collectors.toList())
108+
);
109+
assertEquals(
110+
Arrays.asList(base, base + "_more"),
111+
StreamSupport.stream(subject.iterateMatching(Arrays.asList(base, base + "_more", "other")).spliterator(), false).toList()
112+
);
113+
assertEquals(Arrays.asList(base, base + "_more"), subject.matching(Arrays.asList(base, base + "_more", "other")));
114+
}
115+
116+
@Test
117+
public void contains() {
118+
String base = "contains_string";
119+
WildcardMatcher subject = applyCase(WildcardMatcher.from("*" + base + "*"));
120+
assertTrue(subject instanceof WildcardMatcher.ContainsMatcher);
121+
assertTrue(subject.test(base));
122+
assertTrue(subject.test(base + "_a"));
123+
assertTrue(subject.test("a_" + base));
124+
assertTrue(subject.test("a_" + base + "_a"));
125+
assertFalse(subject.test("string".substring(0, 5)));
126+
assertEquals(ignoreCase, subject.test(base.toUpperCase()));
127+
assertEquals(ignoreCase, subject.test("a_" + base.toUpperCase() + "_a"));
128+
assertEquals("*" + base + "*", subject.toString());
129+
assertEquals(applyCase(WildcardMatcher.from(subject.toString())), subject);
130+
assertNotEquals(applyCase(WildcardMatcher.from(subject.toString() + "x")), subject);
131+
}
132+
133+
@Test
134+
public void simple() {
135+
String base1 = "my";
136+
String base2 = "string";
137+
WildcardMatcher subject = applyCase(WildcardMatcher.from(base1 + "*" + base2 + "*"));
138+
assertTrue(subject instanceof WildcardMatcher.SimpleMatcher);
139+
assertTrue(subject.test(base1 + base2));
140+
assertTrue(subject.test(base1 + "_x_" + base2));
141+
assertTrue(subject.test(base1 + "_x_" + base2 + "_y"));
142+
assertFalse(subject.test("x_" + base1 + base2));
143+
assertEquals(ignoreCase, subject.test(base1 + base2.toUpperCase()));
144+
}
145+
146+
@Test
147+
public void simple_withQuestionMark() {
148+
String base = "string";
149+
WildcardMatcher subject = applyCase(WildcardMatcher.from("?" + base));
150+
assertTrue(subject instanceof WildcardMatcher.SimpleMatcher);
151+
assertFalse(subject.test(base));
152+
assertTrue(subject.test("." + base));
153+
assertFalse(subject.test(".." + base));
154+
assertFalse(subject.test("." + base + "."));
155+
assertEquals(ignoreCase, subject.test("." + base.toUpperCase()));
156+
}
157+
158+
@Test
159+
public void simple_withQuestionMarkAndStar() {
160+
String base1 = "my";
161+
String base2 = "string";
162+
WildcardMatcher subject = applyCase(WildcardMatcher.from(base1 + "?" + base2 + "*"));
163+
assertTrue(subject instanceof WildcardMatcher.SimpleMatcher);
164+
assertFalse(subject.test(base1 + base2));
165+
assertTrue(subject.test(base1 + "_" + base2));
166+
assertTrue(subject.test(base1 + "_" + base2 + "_y"));
167+
assertFalse(subject.test("x_" + base1 + base2));
168+
assertEquals(ignoreCase, subject.test(base1 + "_" + base2.toUpperCase()));
169+
}
170+
171+
@Test
172+
public void simple_questionMark() {
173+
WildcardMatcher subject = applyCase(WildcardMatcher.from("?"));
174+
assertTrue(subject instanceof WildcardMatcher.SimpleMatcher);
175+
assertFalse(subject.test(""));
176+
assertTrue(subject.test("."));
177+
assertFalse(subject.test(".."));
178+
}
179+
180+
@Test
181+
public void regex() {
182+
String base1 = "my";
183+
String base2 = "string";
184+
WildcardMatcher subject = applyCase(WildcardMatcher.from("/" + base1 + ".*" + base2 + "./"));
185+
assertTrue(subject instanceof WildcardMatcher.RegexMatcher);
186+
assertFalse(subject.test(base1 + base2));
187+
assertTrue(subject.test(base1 + base2 + "x"));
188+
assertTrue(subject.test(base1 + "_x_" + base2 + "x"));
189+
assertFalse(subject.test("x_" + base1 + base2 + "x"));
190+
assertEquals(ignoreCase, subject.test(base1 + base2.toUpperCase() + "x"));
191+
assertEquals("/" + base1 + ".*" + base2 + "./", subject.toString());
192+
assertEquals(applyCase(WildcardMatcher.from(subject.toString())), subject);
193+
assertNotEquals(applyCase(WildcardMatcher.from(subject.toString() + "x")), subject);
194+
}
195+
196+
@Test
197+
public void combined() {
198+
String base1 = "string";
199+
String base2 = "other";
200+
WildcardMatcher subject = applyCase(WildcardMatcher.from(base1 + "*", base2));
201+
assertTrue(subject instanceof WildcardMatcher.MatcherCombiner);
202+
assertTrue(subject.test(base1 + "_more"));
203+
assertTrue(subject.test(base1));
204+
assertTrue(subject.test(base2));
205+
assertFalse(subject.test(base2 + "_more"));
206+
assertEquals(ignoreCase, subject.test(base1.toUpperCase()));
207+
assertEquals(ignoreCase, subject.test(base1.toUpperCase() + "_more"));
208+
assertEquals(ignoreCase, subject.test(base2.toUpperCase()));
209+
assertEquals("[" + base1 + "*, " + base2 + "]", subject.toString());
210+
assertEquals(applyCase(WildcardMatcher.from(base1 + "*", base2)), subject);
211+
assertNotEquals(applyCase(WildcardMatcher.from(base1 + "*", base2, "x")), subject);
212+
assertNotEquals(applyCase(WildcardMatcher.from(base1 + "*")), subject);
213+
}
214+
215+
@Test
216+
public void concat() {
217+
String base1 = "string";
218+
String base2 = "other";
219+
WildcardMatcher subject1 = applyCase(WildcardMatcher.from(base1 + "*"));
220+
assertSame(subject1, subject1.concat(Collections.emptyList()));
221+
WildcardMatcher subject2 = subject1.concat(Collections.singleton(applyCase(WildcardMatcher.from(base2))));
222+
assertTrue(subject2 instanceof WildcardMatcher.MatcherCombiner);
223+
assertTrue(subject2.test(base1));
224+
assertTrue(subject2.test(base2));
225+
assertFalse(subject2.test(base2 + "_more"));
226+
}
227+
228+
public WildcardMatcherTest(boolean ignoreCase) {
229+
this.ignoreCase = ignoreCase;
230+
}
231+
232+
@Parameterized.Parameters(name = "ignoreCase: {0}")
233+
public static Collection<Object[]> params() {
234+
return Arrays.asList(new Object[] { false }, new Object[] { true });
235+
}
236+
237+
private WildcardMatcher applyCase(WildcardMatcher wildcardMatcher) {
238+
if (ignoreCase) {
239+
return wildcardMatcher.ignoreCase();
240+
} else {
241+
return wildcardMatcher;
242+
}
243+
}
244+
}

‎src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public final class AuditMessage {
6464
private static final Logger log = LogManager.getLogger(AuditMessage.class);
6565

6666
// clustername and cluster uuid
67-
private static final WildcardMatcher AUTHORIZATION_HEADER = WildcardMatcher.from("Authorization", false);
67+
private static final WildcardMatcher AUTHORIZATION_HEADER = WildcardMatcher.from("Authorization").ignoreCase();
6868
private static final String SENSITIVE_KEY = "password";
6969
private static final String SENSITIVE_REPLACEMENT_VALUE = "__SENSITIVE__";
7070

‎src/main/java/org/opensearch/security/rest/SecurityWhoAmIAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void accept(RestChannel channel) throws Exception {
111111

112112
final String dn = sslInfo.getPrincipal();
113113
final boolean isAdmin = adminDns.isAdminDN(dn);
114-
final boolean isNodeCertificateRequest = dn != null && WildcardMatcher.from(nodesDn, true).matchAny(dn);
114+
final boolean isNodeCertificateRequest = dn != null && WildcardMatcher.from(nodesDn).ignoreCase().matchAny(dn);
115115

116116
builder.startObject();
117117
builder.field("dn", dn);

‎src/main/java/org/opensearch/security/securityconf/DynamicConfigFactory.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,9 @@ public Map<String, WildcardMatcher> getNodesDn() {
399399
return this.configuration.getCEntries()
400400
.entrySet()
401401
.stream()
402-
.collect(ImmutableMap.toImmutableMap(Entry::getKey, entry -> WildcardMatcher.from(entry.getValue().getNodesDn(), false)));
402+
.collect(
403+
ImmutableMap.toImmutableMap(Entry::getKey, entry -> WildcardMatcher.from(entry.getValue().getNodesDn()).ignoreCase())
404+
);
403405
}
404406
}
405407
}

0 commit comments

Comments
 (0)