Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Changes doc level query name field from id to rule name and adds validation #972

Merged
merged 5 commits into from
Apr 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,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(" || ");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,6 +25,10 @@
import java.util.Locale;
import java.util.Map;
import java.util.UUID;
import java.util.regex.Pattern;

import static org.opensearch.commons.utils.ValidationHelpersKt.getInvalidNameChars;
import static org.opensearch.commons.utils.ValidationHelpersKt.isValidName;

public class SigmaRule {

Expand Down Expand Up @@ -179,6 +184,15 @@ public static SigmaRule fromYaml(String rule, boolean collectErrors) throws Sigm
return fromDict(ruleMap, collectErrors);
}

public static void validateSigmaRuleTitle(String title, List<SigmaError> errors)
{
if (!isValidName(title))
{
errors.add(new SigmaTitleError("Sigma rule title may not start with [_, +, -], contain '..', or contain: " +
getInvalidNameChars().replace("\\", "")));
}
}

public String getTitle() {
return title;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -203,6 +202,7 @@ void prepareRuleIndexing() {
public void onResponse(Map<String, String> fieldMappings) {
try {
SigmaRule parsedRule = SigmaRule.fromYaml(rule, true);
SigmaRule.validateSigmaRuleTitle(parsedRule.getTitle(), parsedRule.getErrors());
if (parsedRule.getErrors() != null && parsedRule.getErrors().size() > 0) {
onFailures(parsedRule.getErrors().toArray(new SigmaError[]{}));
return;
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/org/opensearch/securityanalytics/TestHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -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" +
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.stream.Collectors;
import org.opensearch.securityanalytics.rules.exceptions.SigmaError;

import static org.opensearch.commons.utils.ValidationHelpersKt.getInvalidNameChars;
import static org.opensearch.securityanalytics.TestHelpers.randomDetectorType;
import static org.opensearch.securityanalytics.TestHelpers.countAggregationTestRule;
import static org.opensearch.securityanalytics.TestHelpers.randomDetectorWithInputs;
Expand All @@ -46,6 +47,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 {

Expand Down Expand Up @@ -157,15 +159,18 @@ public void testCreatingAggregationRule() throws SigmaError, IOException {

@SuppressWarnings("unchecked")
public void testCreatingARuleWithWrongSyntax() throws IOException {
String rule = randomRuleWithErrors();
String invalidSigmaRuleTitle = "_Invalid # Rule";
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 must have a log source\",\"error\":\"Sigma rule must have a detection definitions\"," +
"\"error\":\"Sigma rule title may not start with [_, +, -], contain '..', or contain: "+ getInvalidNameChars().replace("\\", "") + "\"}", reason);
}
}

Expand Down Expand Up @@ -403,6 +408,47 @@ 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 = "..Remote Encrypting File System Abuse";
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 may not start with [_, +, -], contain '..', or contain: " +
getInvalidNameChars().replace("\\", ""), reason);
}
}

public void testUpdatingARule_custom_category() throws IOException {
String index = createTestIndex(randomIndex(), windowsIndexMapping());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package org.opensearch.securityanalytics.rules.objects;

import org.junit.Assert;
import org.junit.Rule;
import org.opensearch.securityanalytics.rules.condition.ConditionOR;
import org.opensearch.securityanalytics.rules.exceptions.SigmaDateError;
import org.opensearch.securityanalytics.rules.exceptions.SigmaDetectionError;
Expand All @@ -15,6 +16,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;
Expand All @@ -25,6 +27,7 @@

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
Expand All @@ -34,6 +37,8 @@
import java.util.Map;
import java.util.UUID;

import static org.opensearch.commons.utils.ValidationHelpersKt.getInvalidNameChars;

public class SigmaRuleTests extends OpenSearchTestCase {

public void testSigmaRuleBadUuid() {
Expand Down Expand Up @@ -88,6 +93,22 @@ public void testSigmaRuleBadDate() {
});
}

public void testSigmaRuleBadTitle() {
String invalidSigmaRuleTitle = "_invalid ..title?";
List<SigmaError> errors = new ArrayList<>();
SigmaTitleError expectedError = new SigmaTitleError("Sigma rule title may not start with [_, +, -], contain '..', or contain: " + getInvalidNameChars().replace("\\", ""));

SigmaRule.validateSigmaRuleTitle(invalidSigmaRuleTitle, errors);

assertEquals(1, errors.size());
assertEquals(expectedError.getMessage(), errors.get(0).getMessage());

String validSigmaRuleTitle = "acceptable [title]";
errors.clear();
SigmaRule.validateSigmaRuleTitle(validSigmaRuleTitle, errors);
assertEquals(0, errors.size());
}

public void testSigmaRuleNoLogSource() {
Map<String, Object> sigmaRule = new HashMap<>();
sigmaRule.put("id", java.util.UUID.randomUUID().toString());
Expand Down
Loading