[BUG] Changes doc level query name field from id to rule name and add…

…s validation (#972) (#982) * change doc level query name field to rule name and add validation Signed-off-by: Joanne Wang <[email protected]> * call validation from common utils Signed-off-by: Joanne Wang <[email protected]> * change sigma title validation Signed-off-by: Joanne Wang <[email protected]> * add name to doc level query Signed-off-by: Joanne Wang <[email protected]> * fix unit tests Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> (cherry picked from commit b86ee63) Co-authored-by: Joanne Wang <[email protected]>
opensearch-project · Apr 13, 2024 · 14f1e20 · 14f1e20
1 parent ad63cd8
commit 14f1e20
Show file tree

Hide file tree

Showing 9 changed files with 156 additions and 7 deletions.
diff --git a/src/main/java/org/opensearch/securityanalytics/findings/FindingsService.java b/src/main/java/org/opensearch/securityanalytics/findings/FindingsService.java
@@ -299,7 +299,7 @@ public FindingDto mapFindingWithDocsToFindingDto(FindingWithDocs findingWithDocs
         if (docLevelQueries.isEmpty()) { // this is finding generated by a bucket level monitor
             for (Map.Entry<String, String> entry : detector.getRuleIdMonitorIdMap().entrySet()) {
                 if(entry.getValue().equals(findingWithDocs.getFinding().getMonitorId())) {
-                    docLevelQueries = Collections.singletonList(new DocLevelQuery(entry.getKey(),"", Collections.emptyList(),"",Collections.emptyList()));
+                    docLevelQueries = Collections.singletonList(new DocLevelQuery(entry.getKey(),"bucket_level_monitor", Collections.emptyList(),"",Collections.emptyList()));
                 }
             }
         }

diff --git a/src/main/java/org/opensearch/securityanalytics/model/DetectorTrigger.java b/src/main/java/org/opensearch/securityanalytics/model/DetectorTrigger.java
@@ -288,7 +288,7 @@ public Script convertToCondition() {
             StringBuilder ruleNameBuilder = new StringBuilder();
             size = ruleIds.size();
             for (int idx = 0; idx < size; ++idx) {
-                ruleNameBuilder.append(String.format(Locale.getDefault(), "query[name=%s]", ruleIds.get(idx)));
+                ruleNameBuilder.append(String.format(Locale.getDefault(), "query[id=%s]", ruleIds.get(idx)));
                 if (idx < size - 1) {
                     ruleNameBuilder.append(" || ");
                 }

diff --git a/src/main/java/org/opensearch/securityanalytics/rules/exceptions/SigmaTitleError.java b/src/main/java/org/opensearch/securityanalytics/rules/exceptions/SigmaTitleError.java
@@ -0,0 +1,12 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.opensearch.securityanalytics.rules.exceptions;
+
+public class SigmaTitleError extends SigmaError {
+
+    public SigmaTitleError(String message) {
+        super(message);
+    }
+}
diff --git a/src/main/java/org/opensearch/securityanalytics/rules/objects/SigmaRule.java b/src/main/java/org/opensearch/securityanalytics/rules/objects/SigmaRule.java
@@ -10,6 +10,7 @@
 import org.opensearch.securityanalytics.rules.exceptions.SigmaIdentifierError;
 import org.opensearch.securityanalytics.rules.exceptions.SigmaLevelError;
 import org.opensearch.securityanalytics.rules.exceptions.SigmaLogsourceError;
+import org.opensearch.securityanalytics.rules.exceptions.SigmaTitleError;
 import org.opensearch.securityanalytics.rules.exceptions.SigmaStatusError;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.LoaderOptions;
@@ -105,6 +106,17 @@ protected static SigmaRule fromDict(Map<String, Object> rule, boolean collectErr
             ruleId = null;
         }
 
+        String title;
+        if (rule.containsKey("title")) {
+            title = rule.get("title").toString();
+            if (!title.matches("^.{1,256}$"))
+            {
+                errors.add(new SigmaTitleError("Sigma rule title can be max 256 characters"));
+            }
+        } else {
+            title = "";
+        }
+
         SigmaLevel level;
         if (rule.containsKey("level")) {
             level = SigmaLevel.valueOf(rule.get("level").toString().toUpperCase(Locale.ROOT));
@@ -164,7 +176,7 @@ protected static SigmaRule fromDict(Map<String, Object> rule, boolean collectErr
             throw errors.get(0);
         }
 
-        return new SigmaRule(rule.get("title").toString(), logSource, detections, ruleId, status,
+        return new SigmaRule(title, logSource, detections, ruleId, status,
                 rule.get("description").toString(), rule.get("references") != null? (List<String>) rule.get("references"): null, ruleTags,
                 rule.get("author").toString(), ruleDate, rule.get("fields") != null? (List<String>) rule.get("fields"): null,
                 rule.get("falsepositives") != null? (List<String>) rule.get("falsepositives"): null, level, errors);

diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java
@@ -741,7 +741,7 @@ private IndexMonitorRequest createDocLevelMonitorRequest(List<Pair<String, Rule>
             String id = query.getLeft();
 
             Rule rule = query.getRight();
-            String name = query.getLeft();
+            String name = rule.getTitle();
             String actualQuery = rule.getQueries().get(0).getValue();
 
             List<String> tags = new ArrayList<>();

diff --git a/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java b/src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java
@@ -69,7 +69,6 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
 
 import static org.opensearch.securityanalytics.model.Detector.NO_ID;
 import static org.opensearch.securityanalytics.model.Detector.NO_VERSION;

diff --git a/src/test/java/org/opensearch/securityanalytics/TestHelpers.java b/src/test/java/org/opensearch/securityanalytics/TestHelpers.java
@@ -722,6 +722,35 @@ public static String randomEditedRule() {
                 "level: high";
     }
 
+    public static String randomEditedRuleInvalidSyntax(String title) {
+        return "title: " + title + "\n" +
+                "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\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" +
+                "    - 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.lateral_movement\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: 24\n" +
+                "    condition: selection\n" +
+                "falsepositives:\n" +
+                "    - Legitimate usage of remote file encryption\n" +
+                "level: high";
+    }
+
     public static String randomRuleWithErrors() {
         return "title: Remote Encrypting File System Abuse\n" +
                 "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" +
@@ -743,6 +772,27 @@ public static String randomRuleWithErrors() {
                 "level: high";
     }
 
+    public static String randomRuleWithErrors(String title) {
+        return "title: " + title + "\n" +
+                "id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\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" +
+                "    - 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.lateral_movement\n" +
+                "status: experimental\n" +
+                "author: Sagie Dulce, Dekel Paz\n" +
+                "date: 2022/01/01\n" +
+                "modified: 2022/01/01\n" +
+                "falsepositives:\n" +
+                "    - Legitimate usage of remote file encryption\n" +
+                "level: high";
+    }
+
     public static String toJsonStringWithUser(Detector detector) throws IOException {
         XContentBuilder builder = XContentFactory.jsonBuilder();
         builder = detector.toXContentWithUser(builder, ToXContent.EMPTY_PARAMS);

diff --git a/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java b/src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java
@@ -45,6 +45,7 @@
 import static org.opensearch.securityanalytics.TestHelpers.randomRuleForMappingView;
 import static org.opensearch.securityanalytics.TestHelpers.randomRuleWithErrors;
 import static org.opensearch.securityanalytics.TestHelpers.windowsIndexMapping;
+import static org.opensearch.securityanalytics.TestHelpers.randomEditedRuleInvalidSyntax;
 
 public class RuleRestApiIT extends SecurityAnalyticsRestTestCase {
 
@@ -156,15 +157,18 @@ public void testCreatingAggregationRule() throws SigmaError, IOException {
 
     @SuppressWarnings("unchecked")
     public void testCreatingARuleWithWrongSyntax() throws IOException {
-        String rule = randomRuleWithErrors();
+        String invalidSigmaRuleTitle = "a".repeat(257);
+        String rule = randomRuleWithErrors(invalidSigmaRuleTitle);
 
         try {
             makeRequest(client(), "POST", SecurityAnalyticsPlugin.RULE_BASE_URI, Collections.singletonMap("category", randomDetectorType()),
                     new StringEntity(rule), new BasicHeader("Content-Type", "application/json"));
+            fail("Invalid rule syntax, creation should have failed");
         } catch (ResponseException ex) {
             Map<String, Object> responseBody = asMap(ex.getResponse());
             String reason = ((Map<String, Object>) responseBody.get("error")).get("reason").toString();
-            Assert.assertEquals("{\"error\":\"Sigma rule must have a log source\",\"error\":\"Sigma rule must have a detection definitions\"}", reason);
+            Assert.assertEquals("{\"error\":\"Sigma rule title can be max 256 characters\",\"error\":\"Sigma rule must have a log source\"," +
+                    "\"error\":\"Sigma rule must have a detection definitions\"}", reason);
         }
     }
 
@@ -402,6 +406,46 @@ public void testUpdatingUnusedRule() throws IOException {
         Assert.assertEquals("Update rule failed", RestStatus.OK, restStatus(updateResponse));
     }
 
+    public void testUpdatingUnusedRuleWithWrongSyntax() throws IOException {
+        String index = createTestIndex(randomIndex(), windowsIndexMapping());
+
+        // Execute CreateMappingsAction to add alias mapping for index
+        Request createMappingRequest = new Request("POST", SecurityAnalyticsPlugin.MAPPER_BASE_URI);
+        // both req params and req body are supported
+        createMappingRequest.setJsonEntity(
+                "{ \"index_name\":\"" + index + "\"," +
+                        "  \"rule_topic\":\"" + randomDetectorType() + "\", " +
+                        "  \"partial\":true" +
+                        "}"
+        );
+
+        Response response = client().performRequest(createMappingRequest);
+        assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
+
+        String rule = randomRule();
+
+        Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.RULE_BASE_URI, Collections.singletonMap("category", randomDetectorType()),
+                new StringEntity(rule), new BasicHeader("Content-Type", "application/json"));
+        Assert.assertEquals("Create rule failed", RestStatus.CREATED, restStatus(createResponse));
+
+        // update rule with invalid syntax
+        Map<String, Object> responseBody = asMap(createResponse);
+        String createdId = responseBody.get("_id").toString();
+
+        String invalidSigmaRuleTitle = "a".repeat(257);
+        String updatedRule = randomEditedRuleInvalidSyntax(invalidSigmaRuleTitle);
+
+        try {
+            makeRequest(client(), "PUT", SecurityAnalyticsPlugin.RULE_BASE_URI + "/" + createdId, Map.of("category", randomDetectorType()),
+                    new StringEntity(updatedRule), new BasicHeader("Content-Type", "application/json"));
+            fail("Invalid rule name, updation should fail");
+        } catch (ResponseException ex) {
+            responseBody = asMap(ex.getResponse());
+            String reason = ((Map<String, Object>) responseBody.get("error")).get("reason").toString();
+            Assert.assertEquals("Sigma rule title can be max 256 characters", reason);
+        }
+    }
+
     public void testUpdatingARule_custom_category() throws IOException {
         String index = createTestIndex(randomIndex(), windowsIndexMapping());
 

diff --git a/src/test/java/org/opensearch/securityanalytics/rules/objects/SigmaRuleTests.java b/src/test/java/org/opensearch/securityanalytics/rules/objects/SigmaRuleTests.java
@@ -15,6 +15,7 @@
 import org.opensearch.securityanalytics.rules.exceptions.SigmaModifierError;
 import org.opensearch.securityanalytics.rules.exceptions.SigmaRegularExpressionError;
 import org.opensearch.securityanalytics.rules.exceptions.SigmaStatusError;
+import org.opensearch.securityanalytics.rules.exceptions.SigmaTitleError;
 import org.opensearch.securityanalytics.rules.exceptions.SigmaValueError;
 import org.opensearch.securityanalytics.rules.modifiers.SigmaContainsModifier;
 import org.opensearch.securityanalytics.rules.modifiers.SigmaEndswithModifier;
@@ -88,6 +89,37 @@ public void testSigmaRuleBadDate() {
         });
     }
 
+    public void testSigmaRuleBadTitle() {
+        Map<String, Object> sigmaRule = new HashMap<>();
+        sigmaRule.put("id", java.util.UUID.randomUUID().toString());
+        sigmaRule.put("level", "critical");
+        sigmaRule.put("status", "experimental");
+        sigmaRule.put("date", "2017/05/15");
+
+        // test empty string
+        String invalidSigmaRuleTitle = "";
+        sigmaRule.put("title", invalidSigmaRuleTitle);
+
+        Exception exception = assertThrows(SigmaTitleError.class, () -> {
+            SigmaRule.fromDict(sigmaRule, false);
+        });
+
+        String expectedMessage = "Sigma rule title can be max 256 characters";
+        String actualMessage = exception.getMessage();
+        assertTrue(actualMessage.contains(expectedMessage));
+
+        // test string over 256 chars
+        invalidSigmaRuleTitle = "a".repeat(257);
+        sigmaRule.put("title", invalidSigmaRuleTitle);
+
+        exception = assertThrows(SigmaTitleError.class, () -> {
+            SigmaRule.fromDict(sigmaRule, false);
+        });
+
+        actualMessage = exception.getMessage();
+        assertTrue(actualMessage.contains(expectedMessage));
+    }
+
     public void testSigmaRuleNoLogSource() {
         Map<String, Object> sigmaRule = new HashMap<>();
         sigmaRule.put("id", java.util.UUID.randomUUID().toString());