[Backport 2.x] Catch and wrap exceptions. #1198 (#1204) (#1205)

* Catch and wrap exceptions. (#1198) * Wrapping exceptions. Signed-off-by: AWSHurneyt <[email protected]> * Wrapping exceptions. Signed-off-by: AWSHurneyt <[email protected]> * Wrapping exceptions. Signed-off-by: AWSHurneyt <[email protected]> * Fixed exception wrapping. Signed-off-by: AWSHurneyt <[email protected]> * Fixed exception wrapping. Signed-off-by: AWSHurneyt <[email protected]> * Wrapping exceptions. Signed-off-by: AWSHurneyt <[email protected]> * Fixed exception wrapping. Signed-off-by: AWSHurneyt <[email protected]> * Adjusted test case to run with 1mil IOCs. Signed-off-by: AWSHurneyt <[email protected]> * Added logic to catch more exceptions, and wrap more exceptions. Signed-off-by: AWSHurneyt <[email protected]> * Updated jar to reflect changes in opensearch-project/security-analytics-commons#19. Signed-off-by: AWSHurneyt <[email protected]> * 2.16 release notes (#1196) Signed-off-by: Joanne Wang <[email protected]> * Updated release notes. Signed-off-by: AWSHurneyt <[email protected]> * Added catch for parsing errors. Signed-off-by: AWSHurneyt <[email protected]> * Addressed PR feedback. Signed-off-by: AWSHurneyt <[email protected]> * Addressed PR feedback. Signed-off-by: AWSHurneyt <[email protected]> * Added unit tests. Signed-off-by: AWSHurneyt <[email protected]> * Added integ test. Signed-off-by: AWSHurneyt <[email protected]> --------- Signed-off-by: AWSHurneyt <[email protected]> Signed-off-by: Joanne Wang <[email protected]> Co-authored-by: Joanne Wang <[email protected]> (cherry picked from commit f8b541d) Signed-off-by: AWSHurneyt <[email protected]> * Fixed cherry-pick. Signed-off-by: AWSHurneyt <[email protected]> --------- Signed-off-by: AWSHurneyt <[email protected]> Signed-off-by: Joanne Wang <[email protected]> Co-authored-by: Joanne Wang <[email protected]> (cherry picked from commit c6bff12) Co-authored-by: AWSHurneyt <[email protected]>
opensearch-project · Jul 30, 2024 · 1545a02 · 1545a02
1 parent 2ff8a9d
commit 1545a02
Show file tree

Hide file tree

Showing 18 changed files with 365 additions and 154 deletions.
diff --git a/release-notes/opensearch-security-analytics.release-notes-2.16.0.0.md b/release-notes/opensearch-security-analytics.release-notes-2.16.0.0.md
@@ -6,6 +6,7 @@ Compatible with OpenSearch 2.16.0
 * Threat Intel Analytics ([#1098](https://github.com/opensearch-project/security-analytics/pull/1098))
 
 ### Maintenance
+* Incremented version to 2.16.0. ([#1197](https://github.com/opensearch-project/security-analytics/pull/1197))
 * Fix build CI error due to action runner env upgrade node 20 ([#1143](https://github.com/opensearch-project/security-analytics/pull/1143))
 
 ### Enhancement
@@ -28,6 +29,7 @@ Compatible with OpenSearch 2.16.0
 * fix bug: threat intel monitor finding doesnt contain all doc_ids containing malicious IOC ([#1184](https://github.com/opensearch-project/security-analytics/pull/1184))
 * Fixed bulk indexing for IOCs ([#1187](https://github.com/opensearch-project/security-analytics/pull/1187))
 * Fix ioc upload update behavior and change error response ([#1192](https://github.com/opensearch-project/security-analytics/pull/1192))
+* Catch and wrap exceptions. ([#1198](https://github.com/opensearch-project/security-analytics/pull/1198))
 
 ### Documentation
 * Added 2.16.0 release notes. ([#1196](https://github.com/opensearch-project/security-analytics/pull/1196))
diff --git a/security-analytics-commons-1.0.0.jar b/security-analytics-commons-1.0.0.jar
diff --git a/src/main/java/org/opensearch/securityanalytics/model/STIX2IOC.java b/src/main/java/org/opensearch/securityanalytics/model/STIX2IOC.java
@@ -11,12 +11,14 @@
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.common.io.stream.Writeable;
+import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.core.xcontent.XContentParser;
 import org.opensearch.core.xcontent.XContentParserUtils;
 import org.opensearch.securityanalytics.commons.model.IOCType;
 import org.opensearch.securityanalytics.commons.model.STIX2;
+import org.opensearch.securityanalytics.util.SecurityAnalyticsException;
 import org.opensearch.securityanalytics.util.XContentUtils;
 
 import java.io.IOException;
@@ -205,7 +207,18 @@ public static STIX2IOC parse(XContentParser xcp, String id, Long version) throws
                     name = xcp.text();
                     break;
                 case TYPE_FIELD:
-                    type = new IOCType(xcp.text());
+                    String typeString = xcp.text();
+                    try {
+                        type = new IOCType(typeString);
+                    } catch (Exception e) {
+                        String error = String.format(
+                                "Couldn't parse IOC type '%s' while deserializing STIX2IOC with ID '%s': ",
+                                typeString,
+                                id
+                        );
+                        logger.error(error, e);
+                        throw new SecurityAnalyticsException(error, RestStatus.BAD_REQUEST, e);
+                    }
                     break;
                 case VALUE_FIELD:
                     value = xcp.text();

diff --git a/src/main/java/org/opensearch/securityanalytics/model/STIX2IOCDto.java b/src/main/java/org/opensearch/securityanalytics/model/STIX2IOCDto.java
@@ -5,26 +5,31 @@
 
 package org.opensearch.securityanalytics.model;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.common.io.stream.Writeable;
+import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.core.xcontent.XContentParser;
 import org.opensearch.core.xcontent.XContentParserUtils;
 import org.opensearch.securityanalytics.commons.model.IOCType;
 import org.opensearch.securityanalytics.commons.model.STIX2;
+import org.opensearch.securityanalytics.util.SecurityAnalyticsException;
 
 import java.io.IOException;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Locale;
 
 /**
  * A data transfer object for the [STIX2IOC] data model.
  */
 public class STIX2IOCDto implements Writeable, ToXContentObject {
+    private static final Logger logger = LogManager.getLogger(STIX2IOCDto.class);
+
     private String id;
     private String name;
     private IOCType type;
@@ -175,7 +180,18 @@ public static STIX2IOCDto parse(XContentParser xcp, String id, Long version) thr
                     name = xcp.text();
                     break;
                 case STIX2.TYPE_FIELD:
-                    type = new IOCType(xcp.text());
+                    String typeString = xcp.text();
+                    try {
+                        type = new IOCType(typeString);
+                    } catch (Exception e) {
+                        String error = String.format(
+                                "Couldn't parse IOC type '%s' while deserializing STIX2IOCDto with ID '%s': ",
+                                typeString,
+                                id
+                        );
+                        logger.error(error, e);
+                        throw new SecurityAnalyticsException(error, RestStatus.BAD_REQUEST, e);
+                    }
                     break;
                 case STIX2.VALUE_FIELD:
                     value = xcp.text();

diff --git a/src/main/java/org/opensearch/securityanalytics/services/STIX2IOCFetchService.java b/src/main/java/org/opensearch/securityanalytics/services/STIX2IOCFetchService.java
@@ -7,10 +7,12 @@
 
 import com.amazonaws.AmazonServiceException;
 import com.amazonaws.SdkClientException;
+import com.fasterxml.jackson.databind.RuntimeJsonMappingException;
 import org.apache.commons.csv.CSVParser;
 import org.apache.commons.csv.CSVRecord;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.opensearch.OpenSearchException;
 import org.opensearch.action.bulk.BulkRequest;
 import org.opensearch.client.Client;
 import org.opensearch.cluster.service.ClusterService;
@@ -24,6 +26,7 @@
 import org.opensearch.securityanalytics.action.TestS3ConnectionResponse;
 import org.opensearch.securityanalytics.commons.connector.Connector;
 import org.opensearch.securityanalytics.commons.connector.S3Connector;
+import org.opensearch.securityanalytics.commons.connector.exceptions.ConnectorParsingException;
 import org.opensearch.securityanalytics.commons.connector.factory.InputCodecFactory;
 import org.opensearch.securityanalytics.commons.connector.factory.S3ClientFactory;
 import org.opensearch.securityanalytics.commons.connector.factory.StsAssumeRoleCredentialsProviderFactory;
@@ -74,6 +77,9 @@ public class STIX2IOCFetchService {
     private final Logger log = LogManager.getLogger(STIX2IOCFetchService.class);
     private final String ENDPOINT_CONFIG_PATH = "/threatIntelFeed/internalAuthEndpoint.txt";
 
+    public final String REGION_REGEX = "^.{1,20}$";
+    public final String ROLE_ARN_REGEX = "^arn:aws:iam::\\d{12}:role/[\\w+=,.@-]{1,64}$";
+
     private Client client;
     private ClusterService clusterService;
     private STIX2IOCConnectorFactory connectorFactory;
@@ -105,11 +111,32 @@ public void onlyIndexIocs(SATIFSourceConfig saTifSourceConfig,
                               List<STIX2IOC> stix2IOCList,
                               ActionListener<STIX2IOCFetchResponse> listener) {
         STIX2IOCFeedStore feedStore = new STIX2IOCFeedStore(client, clusterService, saTifSourceConfig, listener);
+        Instant startTime = Instant.now();
+        Instant endTime;
+        Exception exception = null;
+        RestStatus restStatus = null;
         try {
+            log.info("Started IOC index step at {}.", startTime);
             feedStore.indexIocs(stix2IOCList);
+        } catch (IllegalArgumentException e) {
+            exception = e;
+            restStatus = RestStatus.BAD_REQUEST;
+        } catch (OpenSearchException e) {
+            exception = e;
+            restStatus = e.status();
         } catch (Exception e) {
-            log.error("Failed to index IOCs from source config", e);
-            listener.onFailure(e);
+            exception = e;
+            restStatus = RestStatus.INTERNAL_SERVER_ERROR;
+        }
+        endTime = Instant.now();
+        long took = Duration.between(startTime, endTime).toMillis();
+
+        if (exception != null && restStatus != null) {
+            String errorText = getErrorText(saTifSourceConfig, "index", took);
+            log.error(errorText, exception);
+            listener.onFailure(new SecurityAnalyticsException(errorText, restStatus, exception));
+        } else {
+            log.info("IOC index step took {} milliseconds.", took);
         }
     }
 
@@ -121,29 +148,65 @@ public void downloadAndIndexIOCs(SATIFSourceConfig saTifSourceConfig, ActionList
 
         Instant startTime = Instant.now();
         Instant endTime;
+        Exception exception = null;
+        RestStatus restStatus = null;
         try {
             log.info("Started IOC download step at {}.", startTime);
             s3Connector.load(consumer);
+        } catch (IllegalArgumentException | ConnectorParsingException | RuntimeJsonMappingException e) {
+            exception = e;
+            restStatus = RestStatus.BAD_REQUEST;
+        } catch (StsException | S3Exception e) {
+            exception = e;
+            restStatus = RestStatus.fromCode(e.statusCode());
+        } catch (AmazonServiceException e) {
+            exception = e;
+            restStatus = RestStatus.fromCode(e.getStatusCode());
+        } catch (SdkException | SdkClientException e) {
+            // SdkException is a RunTimeException that doesn't have a status code.
+            // Logging the full exception, and providing generic response as output.
+            exception = e;
+            restStatus = RestStatus.FORBIDDEN;
         } catch (Exception e) {
-            endTime = Instant.now();
-            log.error("Failed to download IOCs after {} milliseconds.", Duration.between(startTime, endTime).toMillis(), e);
-            listener.onFailure(e);
-            return;
+            exception = e;
+            restStatus = RestStatus.INTERNAL_SERVER_ERROR;
         }
         endTime = Instant.now();
-        log.info("IOC load step took {} milliseconds.", Duration.between(startTime, endTime).toMillis());
+        long took = Duration.between(startTime, endTime).toMillis();
+
+        if (exception != null && restStatus != null) {
+            String errorText = getErrorText(saTifSourceConfig, "download", took);
+            log.error(errorText, exception);
+            listener.onFailure(new SecurityAnalyticsException(errorText, restStatus, exception));
+            return;
+        } else {
+            log.info("IOC download step took {} milliseconds.", took);
+        }
 
         startTime = Instant.now();
         try {
             log.info("Started IOC flush at {}.", startTime);
             consumer.flushIOCs();
+        } catch (IllegalArgumentException e) {
+            exception = e;
+            restStatus = RestStatus.BAD_REQUEST;
+        } catch (OpenSearchException e) {
+            exception = e;
+            restStatus = e.status();
         } catch (Exception e) {
-            endTime = Instant.now();
-            log.error("Failed to flush IOCs queue after {} milliseconds.", Duration.between(startTime, endTime).toMillis(), e);
-            listener.onFailure(e);
+            exception = e;
+            restStatus = RestStatus.INTERNAL_SERVER_ERROR;
         }
         endTime = Instant.now();
-        log.info("IOC flush step took {} milliseconds.", Duration.between(startTime, endTime).toMillis());
+        took = Duration.between(startTime, endTime).toMillis();
+
+        if (exception != null && restStatus != null) {
+            String errorText = getErrorText(saTifSourceConfig, "index", took);
+            log.error(errorText, exception);
+            listener.onFailure(new SecurityAnalyticsException(errorText, restStatus, exception));
+        } else {
+            log.info("IOC flush step took {} milliseconds.", took);
+        }
     }
 
     public void testS3Connection(S3ConnectorConfig s3ConnectorConfig, ActionListener<TestS3ConnectionResponse> listener) {
@@ -160,24 +223,23 @@ private void testS3ClientConnection(S3ConnectorConfig s3ConnectorConfig, ActionL
             HeadObjectResponse response = connector.testS3Connection(s3ConnectorConfig);
             listener.onResponse(new TestS3ConnectionResponse(RestStatus.fromCode(response.sdkHttpResponse().statusCode()), ""));
         } catch (NoSuchKeyException noSuchKeyException) {
-            log.warn("S3Client connection test failed with NoSuchKeyException: ", noSuchKeyException);
+            log.error("S3Client connection test failed with NoSuchKeyException: ", noSuchKeyException);
             listener.onResponse(new TestS3ConnectionResponse(RestStatus.fromCode(noSuchKeyException.statusCode()), noSuchKeyException.awsErrorDetails().errorMessage()));
         } catch (S3Exception s3Exception) {
-            log.warn("S3Client connection test failed with S3Exception: ", s3Exception);
+            log.error("S3Client connection test failed with S3Exception: ", s3Exception);
             listener.onResponse(new TestS3ConnectionResponse(RestStatus.fromCode(s3Exception.statusCode()), "Resource not found."));
         } catch (StsException stsException) {
-            log.warn("S3Client connection test failed with StsException: ", stsException);
+            log.error("S3Client connection test failed with StsException: ", stsException);
             listener.onResponse(new TestS3ConnectionResponse(RestStatus.fromCode(stsException.statusCode()), stsException.awsErrorDetails().errorMessage()));
         } catch (SdkException sdkException) {
             // SdkException is a RunTimeException that doesn't have a status code.
             // Logging the full exception, and providing generic response as output.
-            log.warn("S3Client connection test failed with SdkException: ", sdkException);
+            log.error("S3Client connection test failed with SdkException: ", sdkException);
             listener.onResponse(new TestS3ConnectionResponse(RestStatus.FORBIDDEN, "Resource not found."));
         } catch (Exception e) {
-            log.warn("S3Client connection test failed with error: ", e);
+            log.error("S3Client connection test failed with error: ", e);
             listener.onFailure(SecurityAnalyticsException.wrap(e));
         }
-
     }
 
     private void testAmazonS3Connection(S3ConnectorConfig s3ConnectorConfig, ActionListener<TestS3ConnectionResponse> listener) {
@@ -186,15 +248,15 @@ private void testAmazonS3Connection(S3ConnectorConfig s3ConnectorConfig, ActionL
             boolean response = connector.testAmazonS3Connection(s3ConnectorConfig);
             listener.onResponse(new TestS3ConnectionResponse(response ? RestStatus.OK : RestStatus.FORBIDDEN, ""));
         } catch (AmazonServiceException e) {
-            log.warn("AmazonS3 connection test failed with AmazonServiceException: ", e);
+            log.error("AmazonS3 connection test failed with AmazonServiceException: ", e);
             listener.onResponse(new TestS3ConnectionResponse(RestStatus.fromCode(e.getStatusCode()), e.getErrorMessage()));
         } catch (SdkClientException e) {
             // SdkException is a RunTimeException that doesn't have a status code.
             // Logging the full exception, and providing generic response as output.
-            log.warn("AmazonS3 connection test failed with SdkClientException: ", e);
+            log.error("AmazonS3 connection test failed with SdkClientException: ", e);
             listener.onResponse(new TestS3ConnectionResponse(RestStatus.FORBIDDEN, "Resource not found."));
         } catch (Exception e) {
-            log.warn("AmazonS3 connection test failed with error: ", e);
+            log.error("AmazonS3 connection test failed with error: ", e);
             listener.onFailure(SecurityAnalyticsException.wrap(e));
         }
     }
@@ -229,12 +291,12 @@ private S3ConnectorConfig constructS3ConnectorConfig(SATIFSourceConfig saTifSour
     }
 
     private void validateS3ConnectorConfig(S3ConnectorConfig s3ConnectorConfig) {
-        if (s3ConnectorConfig.getRoleArn() == null || s3ConnectorConfig.getRoleArn().isEmpty()) {
-            throw new IllegalArgumentException("Role arn is required.");
+        if (s3ConnectorConfig.getRoleArn() == null || !s3ConnectorConfig.getRoleArn().matches(ROLE_ARN_REGEX)) {
+            throw new SecurityAnalyticsException("Role arn is empty or malformed.", RestStatus.BAD_REQUEST, new IllegalArgumentException());
         }
 
-        if (s3ConnectorConfig.getRegion() == null || s3ConnectorConfig.getRegion().isEmpty()) {
-            throw new IllegalArgumentException("Region is required.");
+        if (s3ConnectorConfig.getRegion() == null || !s3ConnectorConfig.getRegion().matches(REGION_REGEX)) {
+            throw new SecurityAnalyticsException("Region is empty or malformed.", RestStatus.BAD_REQUEST, new IllegalArgumentException());
         }
     }
 
@@ -276,13 +338,13 @@ public void downloadFromUrlAndIndexIOCs(SATIFSourceConfig saTifSourceConfig, Act
                     }
                 } catch (Exception e) {
                     log.error("Failed to download the IoCs in CSV format for source " + saTifSourceConfig.getId());
-                    listener.onFailure(e);
+                    listener.onFailure(SecurityAnalyticsException.wrap(e));
                     return;
                 }
                 break;
             default:
                 log.error("unsupported feed format for url download:" + source.getFeedFormat());
-                listener.onFailure(new UnsupportedOperationException("unsupported feed format for url download:" + source.getFeedFormat()));
+                listener.onFailure(SecurityAnalyticsException.wrap(new UnsupportedOperationException("unsupported feed format for url download:" + source.getFeedFormat())));
         }
     }
 
@@ -322,6 +384,23 @@ private void parseAndSaveThreatIntelFeedDataCSV(Iterator<CSVRecord> iterator, SA
         feedStore.indexIocs(iocs);
     }
 
+    /**
+     * Helper function for generating error message text.
+     * @param saTifSourceConfig The config for which IOCs are being downloaded/indexed.
+     * @param action The action that was being taken when the error occurred; e.g., "download", or "index".
+     * @param duration The amount of time, in milliseconds, it took for the action to fail.
+     * @return The error message text.
+     */
+    private String getErrorText(SATIFSourceConfig saTifSourceConfig, String action, long duration) {
+        return String.format(
+                "Failed to %s IOCs from source config '%s' with ID %s after %s milliseconds: ",
+                action,
+                saTifSourceConfig.getName(),
+                saTifSourceConfig.getId(),
+                duration
+        );
+    }
+
     public static class STIX2IOCFetchResponse extends ActionResponse implements ToXContentObject {
         public static String IOCS_FIELD = "iocs";
         public static String TOTAL_FIELD = "total";