Skip to content

Commit 6b0b682

Browse files
cwperkspeternied
cwperks
and
peternied
authored
Add early rejection from RestHandler for unauthorized requests (opensearch-project#3418)
Previously unauthorized requests were fully processed and rejected once they reached the RestHandler. This allocations more memory and resources for these requests that might not be useful if they are already detected as unauthorized. Using the headerVerifer and decompressor customization from [1], perform an early authorization check when only the headers are available, save an 'early response' for transmission and do not perform the decompression on the request to speed up closing out the connection. - Resolves opensearch-project/OpenSearch#10260 Signed-off-by: Peter Nied <petern@amazon.com> Signed-off-by: Craig Perkins <cwperx@amazon.com> Signed-off-by: Craig Perkins <craig5008@gmail.com> Co-authored-by: Peter Nied <petern@amazon.com>
1 parent 0e4b8a4 commit 6b0b682

30 files changed

Lines changed: 947 additions & 275 deletions

‎.github/workflows/ci.yml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ on:
1010

1111
env:
1212
GRADLE_OPTS: -Dhttp.keepAlive=false
13+
CI_ENVIRONMENT: normal
1314

1415
jobs:
1516
generate-test-list:
@@ -108,6 +109,32 @@ jobs:
108109
arguments: |
109110
integrationTest -Dbuild.snapshot=false
110111
112+
resource-tests:
113+
env:
114+
CI_ENVIRONMENT: resource-test
115+
strategy:
116+
fail-fast: false
117+
matrix:
118+
jdk: [17]
119+
platform: [ubuntu-latest]
120+
runs-on: ${{ matrix.platform }}
121+
122+
steps:
123+
- name: Set up JDK for build and test
124+
uses: actions/setup-java@v3
125+
with:
126+
distribution: temurin # Temurin is a distribution of adoptium
127+
java-version: ${{ matrix.jdk }}
128+
129+
- name: Checkout security
130+
uses: actions/checkout@v4
131+
132+
- name: Build and Test
133+
uses: gradle/gradle-build-action@v2
134+
with:
135+
cache-disabled: true
136+
arguments: |
137+
integrationTest -Dbuild.snapshot=false --tests org.opensearch.security.ResourceFocusedTests
111138
backward-compatibility-build:
112139
runs-on: ubuntu-latest
113140
steps:

‎build.gradle‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -459,16 +459,27 @@ sourceSets {
459459

460460
//add new task that runs integration tests
461461
task integrationTest(type: Test) {
462+
doFirst {
463+
// Only run resources tests on resource-test CI environments or locally
464+
if (System.getenv('CI_ENVIRONMENT') == 'resource-test' || System.getenv('CI_ENVIRONMENT') == null) {
465+
include '**/ResourceFocusedTests.class'
466+
} else {
467+
exclude '**/ResourceFocusedTests.class'
468+
}
469+
// Only run with retries while in CI systems
470+
if (System.getenv('CI_ENVIRONMENT') == 'normal') {
471+
retry {
472+
failOnPassedAfterRetry = false
473+
maxRetries = 2
474+
maxFailures = 10
475+
}
476+
}
477+
}
462478
description = 'Run integration tests.'
463479
group = 'verification'
464480
systemProperty "java.util.logging.manager", "org.apache.logging.log4j.jul.LogManager"
465481
testClassesDirs = sourceSets.integrationTest.output.classesDirs
466482
classpath = sourceSets.integrationTest.runtimeClasspath
467-
retry {
468-
failOnPassedAfterRetry = false
469-
maxRetries = 2
470-
maxFailures = 10
471-
}
472483
//run the integrationTest task after the test task
473484
shouldRunAfter test
474485
}

‎src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java‎

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
package org.opensearch.security;
2+
3+
import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
4+
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
5+
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
6+
7+
import java.io.ByteArrayOutputStream;
8+
import java.io.IOException;
9+
import java.lang.management.GarbageCollectorMXBean;
10+
import java.lang.management.ManagementFactory;
11+
import java.lang.management.MemoryPoolMXBean;
12+
import java.lang.management.MemoryUsage;
13+
import java.nio.charset.StandardCharsets;
14+
import java.util.List;
15+
import java.util.Map;
16+
import java.util.concurrent.CompletableFuture;
17+
import java.util.concurrent.ForkJoinPool;
18+
import java.util.concurrent.TimeUnit;
19+
import java.util.function.Supplier;
20+
import java.util.stream.Collectors;
21+
import java.util.stream.IntStream;
22+
import java.util.zip.GZIPOutputStream;
23+
24+
import org.apache.hc.client5.http.classic.methods.HttpPost;
25+
import org.apache.hc.core5.http.ContentType;
26+
import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
27+
import org.apache.hc.core5.http.message.BasicHeader;
28+
import org.junit.BeforeClass;
29+
import org.junit.ClassRule;
30+
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
import org.opensearch.action.index.IndexRequest;
33+
import org.opensearch.client.Client;
34+
import org.opensearch.test.framework.TestSecurityConfig;
35+
import org.opensearch.test.framework.TestSecurityConfig.User;
36+
import org.opensearch.test.framework.cluster.ClusterManager;
37+
import org.opensearch.test.framework.cluster.LocalCluster;
38+
import org.opensearch.test.framework.cluster.TestRestClient;
39+
40+
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
41+
42+
@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
43+
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
44+
public class ResourceFocusedTests {
45+
private static final User ADMIN_USER = new User("admin").roles(ALL_ACCESS);
46+
private static final User LIMITED_USER = new User("limited_user").roles(
47+
new TestSecurityConfig.Role("limited-role").clusterPermissions(
48+
"indices:data/read/mget",
49+
"indices:data/read/msearch",
50+
"indices:data/read/scroll",
51+
"cluster:monitor/state",
52+
"cluster:monitor/health"
53+
)
54+
.indexPermissions(
55+
"indices:data/read/search",
56+
"indices:data/read/mget*",
57+
"indices:monitor/settings/get",
58+
"indices:monitor/stats"
59+
)
60+
.on("*")
61+
);
62+
63+
@ClassRule
64+
public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
65+
.authc(AUTHC_HTTPBASIC_INTERNAL)
66+
.users(ADMIN_USER, LIMITED_USER)
67+
.anonymousAuth(false)
68+
.doNotFailOnForbidden(true)
69+
.build();
70+
71+
@BeforeClass
72+
public static void createTestData() {
73+
try (Client client = cluster.getInternalNodeClient()) {
74+
client.index(new IndexRequest().setRefreshPolicy(IMMEDIATE).index("document").source(Map.of("foo", "bar", "abc", "xyz")))
75+
.actionGet();
76+
}
77+
}
78+
79+
@Test
80+
public void testUnauthenticatedFewBig() {
81+
// Tweaks:
82+
final RequestBodySize size = RequestBodySize.XLarge;
83+
final String requestPath = "/*/_search";
84+
final int parrallelism = 5;
85+
final int totalNumberOfRequests = 100;
86+
final boolean statsPrinter = false;
87+
88+
runResourceTest(size, requestPath, parrallelism, totalNumberOfRequests, statsPrinter);
89+
}
90+
91+
@Test
92+
public void testUnauthenticatedManyMedium() {
93+
// Tweaks:
94+
final RequestBodySize size = RequestBodySize.Medium;
95+
final String requestPath = "/*/_search";
96+
final int parrallelism = 20;
97+
final int totalNumberOfRequests = 10_000;
98+
final boolean statsPrinter = false;
99+
100+
runResourceTest(size, requestPath, parrallelism, totalNumberOfRequests, statsPrinter);
101+
}
102+
103+
@Test
104+
public void testUnauthenticatedTonsSmall() {
105+
// Tweaks:
106+
final RequestBodySize size = RequestBodySize.Small;
107+
final String requestPath = "/*/_search";
108+
final int parrallelism = 100;
109+
final int totalNumberOfRequests = 1_000_000;
110+
final boolean statsPrinter = false;
111+
112+
runResourceTest(size, requestPath, parrallelism, totalNumberOfRequests, statsPrinter);
113+
}
114+
115+
private Long runResourceTest(
116+
final RequestBodySize size,
117+
final String requestPath,
118+
final int parrallelism,
119+
final int totalNumberOfRequests,
120+
final boolean statsPrinter
121+
) {
122+
final byte[] compressedRequestBody = createCompressedRequestBody(size);
123+
try (final TestRestClient client = cluster.getRestClient(new BasicHeader("Content-Encoding", "gzip"))) {
124+
125+
if (statsPrinter) {
126+
printStats();
127+
}
128+
final HttpPost post = new HttpPost(client.getHttpServerUri() + requestPath);
129+
post.setEntity(new ByteArrayEntity(compressedRequestBody, ContentType.APPLICATION_JSON));
130+
131+
final ForkJoinPool forkJoinPool = new ForkJoinPool(parrallelism);
132+
133+
final List<CompletableFuture<Void>> waitingOn = IntStream.rangeClosed(1, totalNumberOfRequests)
134+
.boxed()
135+
.map(i -> CompletableFuture.runAsync(() -> client.executeRequest(post), forkJoinPool))
136+
.collect(Collectors.toList());
137+
Supplier<Long> getCount = () -> waitingOn.stream().filter(cf -> cf.isDone() && !cf.isCompletedExceptionally()).count();
138+
139+
CompletableFuture<Void> statPrinter = statsPrinter ? CompletableFuture.runAsync(() -> {
140+
while (true) {
141+
printStats();
142+
System.out.println(" & Succesful completions: " + getCount.get());
143+
try {
144+
Thread.sleep(500);
145+
} catch (Exception e) {
146+
break;
147+
}
148+
}
149+
}, forkJoinPool) : CompletableFuture.completedFuture(null);
150+
151+
final CompletableFuture<Void> allOfThem = CompletableFuture.allOf(waitingOn.toArray(new CompletableFuture[0]));
152+
153+
try {
154+
allOfThem.get(30, TimeUnit.SECONDS);
155+
statPrinter.cancel(true);
156+
} catch (final Exception e) {
157+
// Ignored
158+
}
159+
160+
if (statsPrinter) {
161+
printStats();
162+
System.out.println(" & Succesful completions: " + getCount.get());
163+
}
164+
return getCount.get();
165+
}
166+
}
167+
168+
static enum RequestBodySize {
169+
Small(1),
170+
Medium(1_000),
171+
XLarge(1_000_000);
172+
173+
public final int elementCount;
174+
175+
private RequestBodySize(final int elementCount) {
176+
this.elementCount = elementCount;
177+
}
178+
}
179+
180+
private byte[] createCompressedRequestBody(final RequestBodySize size) {
181+
final int repeatCount = size.elementCount;
182+
final String prefix = "{ \"items\": [";
183+
final String repeatedElement = IntStream.range(0, 20)
184+
.mapToObj(n -> ('a' + n) + "")
185+
.map(n -> '"' + n + '"' + ": 123")
186+
.collect(Collectors.joining(",", "{", "}"));
187+
final String postfix = "]}";
188+
long uncompressedBytesSize = 0;
189+
190+
try (
191+
final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
192+
final GZIPOutputStream gzipOutputStream = new GZIPOutputStream(byteArrayOutputStream)
193+
) {
194+
195+
final byte[] prefixBytes = prefix.getBytes(StandardCharsets.UTF_8);
196+
final byte[] repeatedElementBytes = repeatedElement.getBytes(StandardCharsets.UTF_8);
197+
final byte[] postfixBytes = postfix.getBytes(StandardCharsets.UTF_8);
198+
199+
gzipOutputStream.write(prefixBytes);
200+
uncompressedBytesSize = uncompressedBytesSize + prefixBytes.length;
201+
for (int i = 0; i < repeatCount; i++) {
202+
gzipOutputStream.write(repeatedElementBytes);
203+
uncompressedBytesSize = uncompressedBytesSize + repeatedElementBytes.length;
204+
}
205+
gzipOutputStream.write(postfixBytes);
206+
uncompressedBytesSize = uncompressedBytesSize + postfixBytes.length;
207+
gzipOutputStream.finish();
208+
209+
final byte[] compressedRequestBody = byteArrayOutputStream.toByteArray();
210+
System.out.println(
211+
"^^^"
212+
+ String.format(
213+
"Original size was %,d bytes, compressed to %,d bytes, ratio %,.2f",
214+
uncompressedBytesSize,
215+
compressedRequestBody.length,
216+
((double) uncompressedBytesSize / compressedRequestBody.length)
217+
)
218+
);
219+
return compressedRequestBody;
220+
} catch (final IOException ioe) {
221+
throw new RuntimeException(ioe);
222+
}
223+
}
224+
225+
private void printStats() {
226+
System.out.println("** Stats ");
227+
printMemory();
228+
printMemoryPools();
229+
printGCPools();
230+
}
231+
232+
private void printMemory() {
233+
final Runtime runtime = Runtime.getRuntime();
234+
235+
final long totalMemory = runtime.totalMemory(); // Total allocated memory
236+
final long freeMemory = runtime.freeMemory(); // Amount of free memory
237+
final long usedMemory = totalMemory - freeMemory; // Amount of used memory
238+
239+
System.out.println(" Memory Total: " + totalMemory + " Free:" + freeMemory + " Used:" + usedMemory);
240+
}
241+
242+
private void printMemoryPools() {
243+
List<MemoryPoolMXBean> memoryPools = ManagementFactory.getMemoryPoolMXBeans();
244+
for (MemoryPoolMXBean memoryPool : memoryPools) {
245+
MemoryUsage usage = memoryPool.getUsage();
246+
System.out.println(" " + memoryPool.getName() + " USED: " + usage.getUsed() + " MAX: " + usage.getMax());
247+
}
248+
}
249+
250+
private void printGCPools() {
251+
List<GarbageCollectorMXBean> garbageCollectors = ManagementFactory.getGarbageCollectorMXBeans();
252+
for (GarbageCollectorMXBean garbageCollector : garbageCollectors) {
253+
System.out.println(" " + garbageCollector.getName() + " COLLECTION TIME: " + garbageCollector.getCollectionTime());
254+
}
255+
}
256+
257+
}

‎src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ public void createRoleMapping(String backendRoleName, String roleName) {
268268
response.assertStatusCode(201);
269269
}
270270

271-
protected final String getHttpServerUri() {
271+
public final String getHttpServerUri() {
272272
return "http" + (enableHTTPClientSSL ? "s" : "") + "://" + nodeHttpAddress.getHostString() + ":" + nodeHttpAddress.getPort();
273273
}
274274

‎src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public class HTTPSamlAuthenticator implements HTTPAuthenticator, Destroyable {
7979
public static final String IDP_METADATA_FILE = "idp.metadata_file";
8080
public static final String IDP_METADATA_CONTENT = "idp.metadata_content";
8181

82-
private static final String API_AUTHTOKEN_SUFFIX = "api/authtoken";
82+
public static final String API_AUTHTOKEN_SUFFIX = "api/authtoken";
8383
private static final String AUTHINFO_SUFFIX = "authinfo";
8484
private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)";
8585
private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX);

‎src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
220220
public static final String PLUGINS_PREFIX = "_plugins/_security";
221221

222222
private boolean sslCertReloadEnabled;
223-
private volatile SecurityRestFilter securityRestHandler;
224223
private volatile SecurityInterceptor si;
225224
private volatile PrivilegesEvaluator evaluator;
226225
private volatile UserService userService;
@@ -914,7 +913,8 @@ public Map<String, Supplier<HttpServerTransport>> getHttpTransports(
914913
validatingDispatcher,
915914
clusterSettings,
916915
sharedGroupFactory,
917-
tracer
916+
tracer,
917+
securityRestHandler
918918
);
919919

920920
return Collections.singletonMap("org.opensearch.security.http.SecurityHttpServerTransport", () -> odshst);
@@ -930,7 +930,8 @@ public Map<String, Supplier<HttpServerTransport>> getHttpTransports(
930930
dispatcher,
931931
clusterSettings,
932932
sharedGroupFactory,
933-
tracer
933+
tracer,
934+
securityRestHandler
934935
)
935936
);
936937
}

‎src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,11 @@
3737
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
3838
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
3939
import org.apache.hc.core5.http.ContentType;
40-
import org.apache.hc.core5.http.HttpStatus;
4140
import org.apache.hc.core5.http.io.SocketConfig;
4241
import org.apache.hc.core5.http.io.entity.StringEntity;
4342
import org.apache.hc.core5.ssl.SSLContextBuilder;
4443
import org.apache.hc.core5.ssl.TrustStrategy;
45-
44+
import org.apache.http.HttpStatus;
4645
import org.opensearch.common.settings.Settings;
4746
import org.opensearch.core.common.Strings;
4847
import org.opensearch.security.auditlog.impl.AuditMessage;

0 commit comments

Comments
 (0)