From d5b7271b42c3e3245b7c8176ff77f6f472e451f4 Mon Sep 17 00:00:00 2001 From: Surya Sashank Nistala Date: Thu, 12 Sep 2024 17:10:35 -0700 Subject: [PATCH 1/3] set custom rule doc id to the id passed in rule yaml instead of autogenerated name Signed-off-by: Surya Sashank Nistala --- .../transport/TransportIndexRuleAction.java | 26 ++++++++++++++-- .../securityanalytics/TestHelpers.java | 30 +++++++++++++++++++ .../resthandler/RuleRestApiIT.java | 14 ++++++++- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java index 059845995..5abfc08f7 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java @@ -5,11 +5,15 @@ package org.opensearch.securityanalytics.transport; import java.util.Set; + +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.search.join.ScoreMode; import org.opensearch.OpenSearchStatusException; +import org.opensearch.ResourceAlreadyExistsException; import org.opensearch.action.ActionRunnable; +import org.opensearch.action.DocWriteRequest; import org.opensearch.action.admin.indices.create.CreateIndexResponse; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.index.IndexResponse; @@ -31,6 +35,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.index.engine.VersionConflictEngineException; import org.opensearch.index.query.QueryBuilder; import org.opensearch.index.query.QueryBuilders; import org.opensearch.rest.RestRequest; @@ -211,7 +216,7 @@ public void onResponse(Map fieldMappings) { List queries = backend.convertRule(parsedRule); Set queryFieldNames = backend.getQueryFields().keySet(); Rule ruleDoc = new Rule( - NO_ID, NO_VERSION, parsedRule, category, + parsedRule.getId()!=null ? parsedRule.getId().toString() : NO_ID, NO_VERSION, parsedRule, category, queries, new ArrayList<>(queryFieldNames), rule @@ -219,6 +224,8 @@ public void onResponse(Map fieldMappings) { indexRule(ruleDoc, fieldMappings); } catch (IOException | SigmaError e) { onFailures(e); + } catch (Exception e) { + //TODO change to catching only DocIdAlreadyExistsException } } @@ -284,8 +291,10 @@ public void onFailure(Exception e) { IndexRequest indexRequest = new IndexRequest(Rule.CUSTOM_RULES_INDEX) .setRefreshPolicy(request.getRefreshPolicy()) .source(rule.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Map.of("with_type", "true")))) + .opType(DocWriteRequest.OpType.CREATE) .timeout(indexTimeout); - + if(StringUtils.isNotBlank(rule.getId())) + indexRequest.id(rule.getId()); client.index(indexRequest, new ActionListener<>() { @Override public void onResponse(IndexResponse response) { @@ -299,7 +308,18 @@ public void onResponse(IndexResponse response) { @Override public void onFailure(Exception e) { - onFailures(e); + if (e instanceof VersionConflictEngineException || e.getCause() instanceof VersionConflictEngineException) { + log.error(String.format("Cannot create rule. Rule with id %s already exists", rule.getId()), e); + onFailures( // don't throw original exception as it will expose rules index name + SecurityAnalyticsException.wrap( + new IllegalArgumentException( + String.format("Cannot create rule. Rule with id %s already exists", rule.getId()) + ) + ) + ); + } else { + onFailures(e); + } } }); } diff --git a/src/test/java/org/opensearch/securityanalytics/TestHelpers.java b/src/test/java/org/opensearch/securityanalytics/TestHelpers.java index 78044e6d0..5269a2dbe 100644 --- a/src/test/java/org/opensearch/securityanalytics/TestHelpers.java +++ b/src/test/java/org/opensearch/securityanalytics/TestHelpers.java @@ -286,6 +286,36 @@ public static String randomRule() { "level: high"; } + + public static String randomRule(String id) { + return "title: Remote Encrypting File System Abuse\n" + + String.format("id: %s\n", id) + + "description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" + + "references:\n" + + " - https://attack.mitre.org/tactics/TA0008/\n" + + " - https://msrc.microsoft.com/update-guide/vulnerability/CVE-2021-36942\n" + + " - https://github.com/jsecurity101/MSRPC-to-ATTACK/blob/main/documents/MS-EFSR.md\n" + + " - https://github.com/zeronetworks/rpcfirewall\n" + + " - https://zeronetworks.com/blog/stopping_lateral_movement_via_the_rpc_firewall/\n" + + "tags:\n" + + " - attack.defense_evasion\n" + + "status: experimental\n" + + "author: Sagie Dulce, Dekel Paz\n" + + "date: 2022/01/01\n" + + "modified: 2022/01/01\n" + + "logsource:\n" + + " product: rpc_firewall\n" + + " category: application\n" + + " definition: 'Requirements: install and apply the RPC Firewall to all processes with \"audit:true action:block uuid:df1941c5-fe89-4e79-bf10-463657acf44d or c681d488-d850-11d0-8c52-00c04fd90f7e'\n" + + "detection:\n" + + " selection:\n" + + " EventID: 22\n" + + " condition: selection\n" + + "falsepositives:\n" + + " - Legitimate usage of remote file encryption\n" + + "level: high"; + } + public static String randomRuleWithRawField() { return "title: Remote Encrypting File System Abuse\n" + "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" + diff --git a/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java index 9c258e0e8..1892604f0 100644 --- a/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java +++ b/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.UUID; import java.util.stream.Collectors; import org.opensearch.securityanalytics.rules.exceptions.SigmaError; @@ -50,7 +51,8 @@ public class RuleRestApiIT extends SecurityAnalyticsRestTestCase { public void testCreatingARule() throws IOException { - String rule = randomRule(); + String id = UUID.randomUUID().toString(); + String rule = randomRule(id); Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.RULE_BASE_URI, Collections.singletonMap("category", randomDetectorType()), new StringEntity(rule), new BasicHeader("Content-Type", "application/json")); @@ -59,6 +61,16 @@ public void testCreatingARule() throws IOException { Map responseBody = asMap(createResponse); String createdId = responseBody.get("_id").toString(); + assertEquals(createdId, id); + try { + makeRequest(client(), "POST", SecurityAnalyticsPlugin.RULE_BASE_URI, Collections.singletonMap("category", randomDetectorType()), + new StringEntity(rule), new BasicHeader("Content-Type", "application/json")); + fail("Rule creation should have failed due to duplicate id"); + } catch(Exception e) { + //expect failure due to creation of duplicate rule + assertTrue(e.getMessage().contains("Cannot create rule")); + assertTrue(e.getMessage().contains("already exists")); + } int createdVersion = Integer.parseInt(responseBody.get("_version").toString()); Assert.assertNotEquals("response is missing Id", Detector.NO_ID, createdId); Assert.assertTrue("incorrect version", createdVersion > 0); From e0c0de7394e6e47e8341a429dc91de57d8a32f20 Mon Sep 17 00:00:00 2001 From: Surya Sashank Nistala Date: Tue, 8 Oct 2024 15:43:07 -0700 Subject: [PATCH 2/3] fix test Signed-off-by: Surya Sashank Nistala --- .../SecurityAnalyticsRestTestCase.java | 2 +- .../opensearch/securityanalytics/TestHelpers.java | 12 +++++++----- .../securityanalytics/resthandler/RuleRestApiIT.java | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java index 26ffe4344..4f2ea2515 100644 --- a/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java +++ b/src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java @@ -1471,7 +1471,7 @@ protected void tryDeletingRole(String name) throws IOException { @Override protected boolean preserveIndicesUponCompletion() { - return true; + return false; } boolean preserveODFEIndicesAfterTest() { diff --git a/src/test/java/org/opensearch/securityanalytics/TestHelpers.java b/src/test/java/org/opensearch/securityanalytics/TestHelpers.java index 5269a2dbe..ef4f77a7e 100644 --- a/src/test/java/org/opensearch/securityanalytics/TestHelpers.java +++ b/src/test/java/org/opensearch/securityanalytics/TestHelpers.java @@ -54,8 +54,10 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.UUID; import java.util.stream.Collectors; +import static org.opensearch.test.OpenSearchTestCase.randomAlphaOfLength; import static org.opensearch.test.OpenSearchTestCase.randomInt; public class TestHelpers { @@ -259,7 +261,7 @@ public static CorrelationRule randomCorrelationRuleWithTrigger(String name) { public static String randomRule() { return "title: Remote Encrypting File System Abuse\n" + - "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" + + "id: " + UUID.randomUUID() + "\n" + "description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" + "references:\n" + " - https://attack.mitre.org/tactics/TA0008/\n" + @@ -380,7 +382,7 @@ public static String randomRuleWithNotCondition() { public static String randomRuleWithCriticalSeverity() { return "title: Remote Encrypting File System Abuse\n" + - "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" + + "id: " + UUID.randomUUID() + "\n" + "description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" + "references:\n" + " - https://attack.mitre.org/tactics/TA0008/\n" + @@ -471,7 +473,7 @@ public static String randomNullRule() { } public static String randomCloudtrailRuleForCorrelations(String value) { - return "id: 5f92fff9-82e2-48ab-8fc1-8b133556a551\n" + + return "id: " + UUID.randomUUID() + "\n" + "logsource:\n" + " product: cloudtrail\n" + "title: AWS User Created\n" + @@ -524,7 +526,7 @@ public static String randomRuleForMappingView(String field) { public static String randomRuleForCustomLogType() { return "title: Remote Encrypting File System Abuse\n" + - "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" + + "id: " + UUID.randomUUID() + "\n" + "description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" + "references:\n" + " - https://attack.mitre.org/tactics/TA0008/\n" + @@ -1104,7 +1106,7 @@ public static String randomAggregationRule(String aggFunction, String signAndVal public static String randomAggregationRule(String aggFunction, String signAndValue, String opCode) { String rule = "title: Remote Encrypting File System Abuse\n" + - "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" + + "id: " + UUID.randomUUID() + "\n" + "description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" + "references:\n" + " - https://attack.mitre.org/tactics/TA0008/\n" + diff --git a/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java index 1892604f0..3a9d97db3 100644 --- a/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java +++ b/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java @@ -785,7 +785,7 @@ public void testDeletingNonExistingCustomRule() throws IOException { public void testCustomRuleValidation() throws IOException { String rule1 = "title: Remote Encrypting File System Abuse\n" + - "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" + + "id: " + UUID.randomUUID() + "\n" + "description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" + "references:\n" + " - https://attack.mitre.org/tactics/TA0008/\n" + From 1c80f4205c6f503fce823a3739cdfbda82fe1d8e Mon Sep 17 00:00:00 2001 From: Surya Sashank Nistala Date: Tue, 8 Oct 2024 15:59:54 -0700 Subject: [PATCH 3/3] handle generic Exception Signed-off-by: Surya Sashank Nistala --- .../securityanalytics/transport/TransportIndexRuleAction.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java index 5abfc08f7..e58a58258 100644 --- a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java +++ b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java @@ -222,10 +222,8 @@ public void onResponse(Map fieldMappings) { rule ); indexRule(ruleDoc, fieldMappings); - } catch (IOException | SigmaError e) { - onFailures(e); } catch (Exception e) { - //TODO change to catching only DocIdAlreadyExistsException + onFailures(e); } }