Change ruleId if it exists (#628) (#635)

Signed-off-by: Ashish Agrawal <[email protected]> (cherry picked from commit 03573e9) Co-authored-by: Ashish Agrawal <[email protected]>
opensearch-project · Oct 4, 2023 · f20bada · f20bada
1 parent 639b072
commit f20bada
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 3 deletions.
diff --git a/src/main/java/org/opensearch/securityanalytics/util/RuleIndices.java b/src/main/java/org/opensearch/securityanalytics/util/RuleIndices.java
@@ -271,10 +271,16 @@ private String getRuleCategory(Path folderPath) {
     private void ingestQueries(Map<String, List<String>> logIndexToRules, WriteRequest.RefreshPolicy refreshPolicy, TimeValue indexTimeout, ActionListener<BulkResponse> listener) throws SigmaError, IOException {
         List<Rule> queries = new ArrayList<>();
 
-        for (Map.Entry<String, List<String>> logIndexToRule: logIndexToRules.entrySet()) {
-            Map<String, String> fieldMappings = logTypeService.getRuleFieldMappingsForBuiltinLogType(logIndexToRule.getKey());
+        // Moving others_cloud to the top so those queries are indexed first and can be overwritten if other categories
+        // contain the same rules. Tracking issue: https://github.com/opensearch-project/security-analytics/issues/630
+        List<String> categories = new ArrayList<>(logIndexToRules.keySet());
+        if (categories.remove("others_cloud")) {
+            categories.add(0, "others_cloud");
+        }
+        for (String category: categories) {
+            Map<String, String> fieldMappings = logTypeService.getRuleFieldMappingsForBuiltinLogType(category);
             final QueryBackend backend = new OSQueryBackend(fieldMappings, true, true);
-            queries.addAll(getQueries(backend, logIndexToRule.getKey(), logIndexToRule.getValue()));
+            queries.addAll(getQueries(backend, category, logIndexToRules.get(category)));
         }
         loadRules(queries, refreshPolicy, indexTimeout, listener, true);
     }

diff --git a/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java
@@ -21,12 +21,17 @@
 import org.opensearch.securityanalytics.SecurityAnalyticsPlugin;
 import org.opensearch.securityanalytics.SecurityAnalyticsRestTestCase;
 import org.opensearch.securityanalytics.config.monitors.DetectorMonitorConfig;
+import org.opensearch.securityanalytics.logtype.BuiltinLogTypeLoader;
 import org.opensearch.securityanalytics.model.Detector;
 import org.opensearch.securityanalytics.model.DetectorInput;
 import org.opensearch.securityanalytics.model.DetectorRule;
 import org.opensearch.securityanalytics.model.Rule;
 
+import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
@@ -191,6 +196,57 @@ public void testSearchingPrepackagedRules() throws IOException {
         Assert.assertEquals(5, ((Map<String, Object>) ((Map<String, Object>) responseBody.get("hits")).get("total")).get("value"));
     }
 
+    public void testSearchingForDuplicatedPrepackagedRules() throws IOException {
+        String gworkspaceRequest = "{\n" +
+                "  \"query\": {\n" +
+                "    \"nested\": {\n" +
+                "      \"path\": \"rule\",\n" +
+                "      \"query\": {\n" +
+                "        \"bool\": {\n" +
+                "          \"must\": [\n" +
+                "            { \"match\": {\"rule.category\": \"gworkspace\"}}\n" +
+                "          ]\n" +
+                "        }\n" +
+                "      }\n" +
+                "    }\n" +
+                "  }\n" +
+                "}";
+
+        Response gworkSpaceSearchResponse = makeRequest(client(), "POST", String.format(Locale.getDefault(), "%s/_search", SecurityAnalyticsPlugin.RULE_BASE_URI), Collections.singletonMap("pre_packaged", "true"),
+                new StringEntity(gworkspaceRequest), new BasicHeader("Content-Type", "application/json"));
+        Assert.assertEquals("Searching rules failed", RestStatus.OK, restStatus(gworkSpaceSearchResponse));
+
+        String azureRequest = "{\n" +
+                "  \"query\": {\n" +
+                "    \"nested\": {\n" +
+                "      \"path\": \"rule\",\n" +
+                "      \"query\": {\n" +
+                "        \"bool\": {\n" +
+                "          \"must\": [\n" +
+                "            { \"match\": {\"rule.category\": \"azure\"}}\n" +
+                "          ]\n" +
+                "        }\n" +
+                "      }\n" +
+                "    }\n" +
+                "  }\n" +
+                "}";
+
+        Response azureSearchResponse = makeRequest(client(), "POST", String.format(Locale.getDefault(), "%s/_search", SecurityAnalyticsPlugin.RULE_BASE_URI), Collections.singletonMap("pre_packaged", "true"),
+                new StringEntity(azureRequest), new BasicHeader("Content-Type", "application/json"));
+        Assert.assertEquals("Searching rules failed", RestStatus.OK, restStatus(azureSearchResponse));
+
+        ClassLoader classLoader = getClass().getClassLoader();
+        int gworkspaceFileCount = new File(classLoader.getResource("rules/gworkspace").getFile()).listFiles().length;
+        int azureFileCount = new File(classLoader.getResource("rules/azure").getFile()).listFiles().length;
+
+        // Verify azure and gworkspace categories have the right number of rules even though they
+        // conflict with others_cloud category
+        Map<String, Object> gworkspaceResponseBody = asMap(gworkSpaceSearchResponse);
+        Assert.assertEquals(gworkspaceFileCount, ((Map<String, Object>) ((Map<String, Object>) gworkspaceResponseBody.get("hits")).get("total")).get("value"));
+        Map<String, Object> azureResponseBody = asMap(azureSearchResponse);
+        Assert.assertEquals(azureFileCount, ((Map<String, Object>) ((Map<String, Object>) azureResponseBody.get("hits")).get("total")).get("value"));
+    }
+
     @SuppressWarnings("unchecked")
     public void testSearchingPrepackagedRulesByMitreAttackID() throws IOException {
         String request = "{\n" +