From 8b74d6fd3201929cb9aa99755bf69ff6081ecef9 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 9 Oct 2023 13:42:54 -0400 Subject: [PATCH] [Backport 2.x] Add early rejection from RestHandler for unauthorized requests (#3418) (#3496) Backport f7c47af244cbe0c0f20fe708d7d8124090b76a9f from #3495 --------- Signed-off-by: Peter Nied Co-authored-by: Peter Nied --- .../security/ResourceFocusedTests.java | 14 ++++++------ .../framework/cluster/TestRestClient.java | 2 +- .../http/saml/AuthTokenProcessorHandler.java | 2 +- .../security/auth/BackendRegistry.java | 4 ++-- .../security/filter/SecurityResponse.java | 22 +++++++++++++++++++ .../security/filter/SecurityRestFilter.java | 6 ++--- .../Netty4HttpRequestHeaderVerifier.java | 2 +- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java b/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java index 2ab19d15ce..3899dfa111 100644 --- a/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java +++ b/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java @@ -139,7 +139,7 @@ private Long runResourceTest( CompletableFuture statPrinter = statsPrinter ? CompletableFuture.runAsync(() -> { while (true) { printStats(); - System.out.println(" & Succesful completions: " + getCount.get()); + System.err.println(" & Succesful completions: " + getCount.get()); try { Thread.sleep(500); } catch (Exception e) { @@ -159,7 +159,7 @@ private Long runResourceTest( if (statsPrinter) { printStats(); - System.out.println(" & Succesful completions: " + getCount.get()); + System.err.println(" & Succesful completions: " + getCount.get()); } return getCount.get(); } @@ -207,7 +207,7 @@ private byte[] createCompressedRequestBody(final RequestBodySize size) { gzipOutputStream.finish(); final byte[] compressedRequestBody = byteArrayOutputStream.toByteArray(); - System.out.println( + System.err.println( "^^^" + String.format( "Original size was %,d bytes, compressed to %,d bytes, ratio %,.2f", @@ -223,7 +223,7 @@ private byte[] createCompressedRequestBody(final RequestBodySize size) { } private void printStats() { - System.out.println("** Stats "); + System.err.println("** Stats "); printMemory(); printMemoryPools(); printGCPools(); @@ -236,21 +236,21 @@ private void printMemory() { final long freeMemory = runtime.freeMemory(); // Amount of free memory final long usedMemory = totalMemory - freeMemory; // Amount of used memory - System.out.println(" Memory Total: " + totalMemory + " Free:" + freeMemory + " Used:" + usedMemory); + System.err.println(" Memory Total: " + totalMemory + " Free:" + freeMemory + " Used:" + usedMemory); } private void printMemoryPools() { List memoryPools = ManagementFactory.getMemoryPoolMXBeans(); for (MemoryPoolMXBean memoryPool : memoryPools) { MemoryUsage usage = memoryPool.getUsage(); - System.out.println(" " + memoryPool.getName() + " USED: " + usage.getUsed() + " MAX: " + usage.getMax()); + System.err.println(" " + memoryPool.getName() + " USED: " + usage.getUsed() + " MAX: " + usage.getMax()); } } private void printGCPools() { List garbageCollectors = ManagementFactory.getGarbageCollectorMXBeans(); for (GarbageCollectorMXBean garbageCollector : garbageCollectors) { - System.out.println(" " + garbageCollector.getName() + " COLLECTION TIME: " + garbageCollector.getCollectionTime()); + System.err.println(" " + garbageCollector.getName() + " COLLECTION TIME: " + garbageCollector.getCollectionTime()); } } diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index 0e29f3b472..1d7c686a18 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -395,7 +395,7 @@ private JsonNode getJsonNodeAt(String jsonPointer) { try { return toJsonNode().at(jsonPointer); } catch (IOException e) { - throw new IllegalArgumentException("Cound not convert response body to JSON node ", e); + throw new IllegalArgumentException("Cound not convert response body to JSON node '" + getBody() + "'", e); } } diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java index a344c653f3..f545c2425b 100644 --- a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java +++ b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java @@ -239,7 +239,7 @@ private Optional handleLowLevel(RestRequest restRequest) throw return Optional.of(new SecurityResponse(HttpStatus.SC_OK, SecurityResponse.CONTENT_TYPE_APP_JSON, responseBodyString)); } catch (JsonProcessingException e) { log.warn("Error while parsing JSON for /_opendistro/_security/api/authtoken", e); - return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, null, "JSON could not be parsed")); + return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, new Exception("JSON could not be parsed"))); } } diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index f4abb5361a..5824348225 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -204,7 +204,7 @@ public boolean authenticate(final SecurityRequestChannel request) { log.debug("Rejecting REST request because of blocked address: {}", request.getRemoteAddress().orElse(null)); } - request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, null, "Authentication finally failed")); + request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, new Exception("Authentication finally failed"))); return false; } @@ -226,7 +226,7 @@ public boolean authenticate(final SecurityRequestChannel request) { if (!isInitialized()) { log.error("Not yet initialized (you may need to run securityadmin)"); - request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, null, "OpenSearch Security not initialized.")); + request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, new Exception("OpenSearch Security not initialized."))); return false; } diff --git a/src/main/java/org/opensearch/security/filter/SecurityResponse.java b/src/main/java/org/opensearch/security/filter/SecurityResponse.java index be80a5d4d4..14c21a9385 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityResponse.java +++ b/src/main/java/org/opensearch/security/filter/SecurityResponse.java @@ -11,9 +11,11 @@ package org.opensearch.security.filter; +import java.io.IOException; import java.util.Map; import org.apache.http.HttpHeaders; +import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.rest.RestStatus; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestResponse; @@ -26,6 +28,12 @@ public class SecurityResponse { private final Map headers; private final String body; + public SecurityResponse(final int status, final Exception e) { + this.status = status; + this.headers = CONTENT_TYPE_APP_JSON; + this.body = generateFailureMessage(e); + } + public SecurityResponse(final int status, final Map headers, final String body) { this.status = status; this.headers = headers; @@ -52,4 +60,18 @@ public RestResponse asRestResponse() { return restResponse; } + protected String generateFailureMessage(final Exception e) { + try { + return XContentFactory.jsonBuilder() + .startObject() + .startObject("error") + .field("status", "error") + .field("reason", e.getMessage()) + .endObject() + .endObject() + .toString(); + } catch (final IOException ioe) { + throw new RuntimeException(ioe); + } + } } diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index 87fe7f5323..b6475e5ec3 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -247,7 +247,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t log.error(exception.toString()); auditLog.logBadHeaders(requestChannel); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString())); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception)); return; } @@ -256,7 +256,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t log.error(exception.toString()); auditLog.logBadHeaders(requestChannel); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString())); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception)); return; } @@ -276,7 +276,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t } catch (SSLPeerUnverifiedException e) { log.error("No ssl info", e); auditLog.logSSLException(requestChannel, e); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, null)); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, new Exception("No ssl info"))); return; } diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java index 16e23ea361..5112ceced3 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java @@ -119,7 +119,7 @@ public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) thro ctx.channel().attr(IS_AUTHENTICATED).set(Boolean.TRUE); } } catch (final OpenSearchSecurityException e) { - final SecurityResponse earlyResponse = new SecurityResponse(ExceptionsHelper.status(e).getStatus(), null, e.getMessage()); + final SecurityResponse earlyResponse = new SecurityResponse(ExceptionsHelper.status(e).getStatus(), e); ctx.channel().attr(EARLY_RESPONSE).set(earlyResponse); } catch (final SecurityRequestChannelUnsupported srcu) { // Use defaults for unsupported channels