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

set custom rule doc id to the id passed in rule yaml instead of autogenerated id #1302

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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 @@ -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,14 +216,16 @@ 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 | CompositeSigmaErrors e) {
onFailures(e);
} catch (Exception e) {
//TODO change to catching only DocIdAlreadyExistsException
}
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
});
}
Expand Down
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
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
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 @@ -784,7 +796,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