[Backport 2.x] Add early rejection from RestHandler for unauthorized …

…requests (opensearch-project#3418) (opensearch-project#3496) Backport f7c47af from opensearch-project#3495 --------- Signed-off-by: Peter Nied <petern@amazon.com> Co-authored-by: Peter Nied <petern@amazon.com>
peternied · Oct 9, 2023 · 8b74d6f · 8b74d6f
1 parent 8cc4f00
commit 8b74d6f
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 15 deletions.
diff --git 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<Void> 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<MemoryPoolMXBean> 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<GarbageCollectorMXBean> 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
@@ -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
@@ -239,7 +239,7 @@ private Optional<SecurityResponse> 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
@@ -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
@@ -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<String, String> 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<String, String> 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
@@ -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
@@ -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