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.

Sign up for GitHub

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

[2.17] set custom rule doc id to the id passed in rule yaml instead of autogenerated name #1347

Merged
merged 3 commits into from
Oct 9, 2024
Merged

[2.17] set custom rule doc id to the id passed in rule yaml instead of autogenerated name #1347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8eba9d5
set custom rule doc id to the id passed in rule yaml instead of autog…
eirsep Sep 13, 2024
c1a983a
fix test
eirsep Oct 8, 2024
8464f01
handle generic Exception
eirsep Oct 8, 2024
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
26 changes: 22 additions & 4 deletions src/main/java/org/opensearch/securityanalytics/transport/TransportIndexRuleAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -211,13 +216,13 @@ public void onResponse(Map<String, String> fieldMappings) {
List<Object> queries = backend.convertRule(parsedRule);
Set<String> 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
);
indexRule(ruleDoc, fieldMappings);
} catch (IOException | SigmaError e) {
} catch (Exception e) {
onFailures(e);
}
}
Expand Down Expand Up @@ -284,8 +289,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) {
Expand All @@ -299,7 +306,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);
}
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/securityanalytics/SecurityAnalyticsRestTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1470,7 +1470,7 @@ protected void tryDeletingRole(String name) throws IOException {

@Override
protected boolean preserveIndicesUponCompletion() {
return true;
return false;
}

boolean preserveODFEIndicesAfterTest() {
Expand Down
42 changes: 37 additions & 5 deletions src/test/java/org/opensearch/securityanalytics/TestHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -259,7 +261,37 @@ 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" +
" - 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 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" +
Expand Down Expand Up @@ -350,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" +
Expand Down Expand Up @@ -441,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" +
Expand Down Expand Up @@ -494,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" +
Expand Down Expand Up @@ -1074,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" +
Expand Down
16 changes: 14 additions & 2 deletions src/test/java/org/opensearch/securityanalytics/resthandler/RuleRestApiIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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"));
Expand All @@ -59,6 +61,16 @@ public void testCreatingARule() throws IOException {
Map<String, Object> 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);
Expand Down Expand Up @@ -773,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" +
Expand Down
Loading