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

Sigma Aggregation rule fixes #622

Merged
merged 3 commits into from
Oct 4, 2023
Merged

Sigma Aggregation rule fixes #622

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
add3527
Sigma Aggregation rule fixes
petardz Jun 22, 2023
3f2e23a
Sigma Aggregation rule fixes
sbcd90 Oct 4, 2023
64e5f91
ignore a failed security test
sbcd90 Oct 4, 2023
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
13 changes: 10 additions & 3 deletions src/main/java/org/opensearch/securityanalytics/findings/FindingsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
public void onResponse(GetDetectorResponse getDetectorResponse) {
// Get all monitor ids from detector
Detector detector = getDetectorResponse.getDetector();
List<String> monitorIds = detector.getMonitorIds();
ActionListener<GetFindingsResponse> getFindingsResponseListener = new ActionListener<>() {
@Override
public void onResponse(GetFindingsResponse resp) {
Expand Down Expand Up @@ -87,12 +86,20 @@
// monitor --> detectorId mapping
Map<String, Detector> monitorToDetectorMapping = new HashMap<>();
detector.getMonitorIds().forEach(
monitorId -> monitorToDetectorMapping.put(monitorId, detector)
monitorId -> {
if (detector.getRuleIdMonitorIdMap().containsKey("chained_findings_monitor")) {
if (!detector.getRuleIdMonitorIdMap().get("chained_findings_monitor").equals(monitorId)) {
monitorToDetectorMapping.put(monitorId, detector);

Check warning on line 92 in src/main/java/org/opensearch/securityanalytics/findings/FindingsService.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/findings/FindingsService.java#L92 

Added line #L92 was not covered by tests
}
} else {
monitorToDetectorMapping.put(monitorId, detector);
}
}
);
// Get findings for all monitor ids
FindingsService.this.getFindingsByMonitorIds(
monitorToDetectorMapping,
monitorIds,
new ArrayList<>(monitorToDetectorMapping.keySet()),
DetectorMonitorConfig.getAllFindingsIndicesPattern(detector.getDetectorType()),
table,
getFindingsResponseListener
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/opensearch/securityanalytics/model/Rule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@
for (SigmaCondition condition: sigmaRule.getDetection().getParsedCondition()) {
Pair<ConditionItem, AggregationItem> parsedItems = condition.parsed();
AggregationItem aggItem = parsedItems.getRight();
aggItem.setTimeframe(sigmaRule.getDetection().getTimeframe());

Check warning on line 484 in src/main/java/org/opensearch/securityanalytics/model/Rule.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/model/Rule.java#L484 

Added line #L484 was not covered by tests
aggregationItems.add(aggItem);
}
return aggregationItems;
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/opensearch/securityanalytics/rules/aggregation/AggregationItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

private Double threshold;

private String timeframe;

public void setAggFunction(String aggFunction) {
this.aggFunction = aggFunction;
}
Expand Down Expand Up @@ -59,4 +61,12 @@
public Double getThreshold() {
return threshold;
}

public void setTimeframe(String timeframe) {
this.timeframe = timeframe;
}

public String getTimeframe() {
return timeframe;

Check warning on line 70 in src/main/java/org/opensearch/securityanalytics/rules/aggregation/AggregationItem.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/rules/aggregation/AggregationItem.java#L70 

Added line #L70 was not covered by tests
}
}
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/securityanalytics/rules/backend/OSQueryBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,10 @@ public AggregationQueries convertAggregation(AggregationItem aggregation) {
fmtAggQuery = String.format(Locale.getDefault(), aggCountQuery, "result_agg", aggregation.getGroupByField());
}
aggBuilder.field(fieldName);
fmtBucketTriggerQuery = String.format(Locale.getDefault(), bucketTriggerQuery, "_cnt", "_cnt", "result_agg", "_cnt", aggregation.getCompOperator(), aggregation.getThreshold());
fmtBucketTriggerQuery = String.format(Locale.getDefault(), bucketTriggerQuery, "_cnt", "_count", "result_agg", "_cnt", aggregation.getCompOperator(), aggregation.getThreshold());

Script script = new Script(String.format(Locale.getDefault(), bucketTriggerScript, "_cnt", aggregation.getCompOperator(), aggregation.getThreshold()));
condition = new BucketSelectorExtAggregationBuilder(bucketTriggerSelectorId, Collections.singletonMap("_cnt", "_cnt"), script, "result_agg", null);
condition = new BucketSelectorExtAggregationBuilder(bucketTriggerSelectorId, Collections.singletonMap("_cnt", "_count"), script, "result_agg", null);
} else {
fmtAggQuery = String.format(Locale.getDefault(), aggQuery, "result_agg", aggregation.getGroupByField(), aggregation.getAggField(), aggregation.getAggFunction(), aggregation.getAggField());
fmtBucketTriggerQuery = String.format(Locale.getDefault(), bucketTriggerQuery, aggregation.getAggField(), aggregation.getAggField(), "result_agg", aggregation.getAggField(), aggregation.getCompOperator(), aggregation.getThreshold());
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/opensearch/securityanalytics/rules/backend/QueryBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ public List<Object> convertRule(SigmaRule rule) throws SigmaError {
}
queries.add(query);
if (aggItem != null) {
aggItem.setTimeframe(rule.getDetection().getTimeframe());
queries.add(convertAggregation(aggItem));
}
}
Expand Down
16 changes: 14 additions & 2 deletions src/main/java/org/opensearch/securityanalytics/rules/objects/SigmaDetections.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@ public class SigmaDetections {

private List<String> condition;

private String timeframe;

private List<SigmaCondition> parsedCondition;

public SigmaDetections(Map<String, SigmaDetection> detections, List<String> condition) throws SigmaDetectionError {
public SigmaDetections(Map<String, SigmaDetection> detections, List<String> condition, String timeframe) throws SigmaDetectionError {
this.detections = detections;
this.condition = condition;
this.timeframe = timeframe;

if (this.detections.isEmpty()) {
throw new SigmaDetectionError("No detections defined in Sigma rule");
Expand Down Expand Up @@ -55,7 +58,12 @@ protected static SigmaDetections fromDict(Map<String, Object> detectionMap) thro
}
}

return new SigmaDetections(detections, conditionList);
String timeframe = null;
if (detectionMap.containsKey("timeframe")) {
timeframe = detectionMap.get("timeframe").toString();
}

return new SigmaDetections(detections, conditionList, timeframe);
}

public Map<String, SigmaDetection> getDetections() {
Expand All @@ -69,4 +77,8 @@ public List<String> getCondition() {
public List<SigmaCondition> getParsedCondition() {
return parsedCondition;
}

public String getTimeframe() {
return timeframe;
}
}
6 changes: 4 additions & 2 deletions src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
import org.opensearch.securityanalytics.model.DetectorTrigger;
import org.opensearch.securityanalytics.model.Rule;
import org.opensearch.securityanalytics.model.Value;
import org.opensearch.securityanalytics.rules.aggregation.AggregationItem;
import org.opensearch.securityanalytics.rules.backend.OSQueryBackend;
import org.opensearch.securityanalytics.rules.backend.OSQueryBackend.AggregationQueries;
import org.opensearch.securityanalytics.rules.backend.QueryBackend;
Expand Down Expand Up @@ -784,7 +785,8 @@

List<String> indices = detector.getInputs().get(0).getIndices();

AggregationQueries aggregationQueries = queryBackend.convertAggregation(rule.getAggregationItemsFromRule().get(0));
AggregationItem aggItem = rule.getAggregationItemsFromRule().get(0);
AggregationQueries aggregationQueries = queryBackend.convertAggregation(aggItem);

Check warning on line 789 in src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/transport/TransportIndexDetectorAction.java#L788-L789 

Added lines #L788 - L789 were not covered by tests

SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder()
.seqNoAndPrimaryTerm(true)
Expand Down Expand Up @@ -814,7 +816,7 @@
? new BoolQueryBuilder()
: QueryBuilders.boolQuery().must(searchSourceBuilder.query());
RangeQueryBuilder timeRangeFilter = QueryBuilders.rangeQuery(TIMESTAMP_FIELD_ALIAS)
.gt("{{period_end}}||-1h")
.gt("{{period_end}}||-" + (aggItem.getTimeframe() != null? aggItem.getTimeframe(): "1h"))
.lte("{{period_end}}")
.format("epoch_millis");
boolQueryBuilder.must(timeRangeFilter);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/securityanalytics/util/RuleIndices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@

Rule ruleModel = new Rule(
rule.getId().toString(), NO_VERSION, rule, category,
ruleQueries,
ruleQueries.stream().map(Object::toString).collect(Collectors.toList()),

Check warning on line 298 in src/main/java/org/opensearch/securityanalytics/util/RuleIndices.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/util/RuleIndices.java#L298 

Added line #L298 was not covered by tests
new ArrayList<>(queryFieldNames),
ruleStr
);
Expand Down
30 changes: 30 additions & 0 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 @@ -88,6 +88,11 @@ public static Detector randomDetectorWithTriggers(List<String> rules, List<Detec
rules.stream().map(DetectorRule::new).collect(Collectors.toList()));
return randomDetector(null, null, null, List.of(input), triggers, null, null, null, null);
}
public static Detector randomDetectorWithTriggersAndScheduleAndEnabled(List<String> rules, List<DetectorTrigger> triggers, Schedule schedule, boolean enabled) {
DetectorInput input = new DetectorInput("windows detector for security analytics", List.of("windows"), Collections.emptyList(),
rules.stream().map(DetectorRule::new).collect(Collectors.toList()));
return randomDetector(null, null, null, List.of(input), triggers, schedule, enabled, null, null);
}

public static Detector randomDetectorWithTriggers(List<String> rules, List<DetectorTrigger> triggers, String detectorType, DetectorInput input) {
return randomDetector(null, detectorType, null, List.of(input), triggers, null, null, null, null);
Expand Down Expand Up @@ -349,6 +354,7 @@ public static String productIndexMaxAggRule() {

public static String randomProductDocument(){
return "{\n" +
" \"name\": \"laptop\",\n" +
" \"fieldA\": 123,\n" +
" \"mappedB\": 111,\n" +
" \"fieldC\": \"valueC\"\n" +
Expand Down Expand Up @@ -560,6 +566,9 @@ public static String netFlowMappings() {

public static String productIndexMapping(){
return "\"properties\":{\n" +
" \"name\":{\n" +
" \"type\":\"keyword\"\n" +
" },\n" +
" \"fieldA\":{\n" +
" \"type\":\"long\"\n" +
" },\n" +
Expand Down Expand Up @@ -588,13 +597,32 @@ public static String productIndexAvgAggRule(){
" category: test_category\n" +
" product: test_product\n" +
" detection:\n" +
" timeframe: 5m\n" +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to add documentation or does sigma have a format which we support for timevalue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigma has a format which supports timeframe. https://github.com/SigmaHQ/sigma-specification/blob/main/Sigma_specification.md#timeframe

" sel:\n" +
" fieldA: 123\n" +
" fieldB: 111\n" +
" fieldC: valueC\n" +
" condition: sel | avg(fieldA) by fieldC > 110";
}

public static String productIndexCountAggRule(){
return " title: Test\n" +
" id: 39f918f3-981b-4e6f-a975-8af7e507ef2b\n" +
" status: test\n" +
" level: critical\n" +
" description: Detects QuarksPwDump clearing access history in hive\n" +
" author: Florian Roth\n" +
" date: 2017/05/15\n" +
" logsource:\n" +
" category: test_category\n" +
" product: test_product\n" +
" detection:\n" +
" timeframe: 5m\n" +
" sel:\n" +
" name: laptop\n" +
" condition: sel | count(*) by name > 2";
}

public static String randomAggregationRule(String aggFunction, String signAndValue) {
String rule = "title: Remote Encrypting File System Abuse\n" +
"id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" +
Expand All @@ -616,6 +644,7 @@ public static String randomAggregationRule(String aggFunction, String signAndVa
" 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" +
" timeframe: 5m\n" +
" sel:\n" +
" Opcode: Info\n" +
" condition: sel | %s(SeverityValue) by Version %s\n" +
Expand Down Expand Up @@ -646,6 +675,7 @@ public static String randomAggregationRule(String aggFunction, String signAndVa
" 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" +
" timeframe: 5m\n" +
" sel:\n" +
" Opcode: %s\n" +
" condition: sel | %s(SeverityValue) by Version %s\n" +
Expand Down
15 changes: 4 additions & 11 deletions src/test/java/org/opensearch/securityanalytics/resthandler/DetectorMonitorRestApiIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1483,7 +1483,7 @@ public void testCreateDetector_verifyWorkflowExecutionBucketLevelDocLevelMonitor
Map<String, Object> getFindingsBody = entityAsMap(getFindingsResponse);

assertNotNull(getFindingsBody);
assertEquals(10, getFindingsBody.get("total_findings"));
assertEquals(6, getFindingsBody.get("total_findings"));

String findingDetectorId = ((Map<String, Object>)((List)getFindingsBody.get("findings")).get(0)).get("detectorId").toString();
assertEquals(detectorId, findingDetectorId);
Expand All @@ -1495,7 +1495,6 @@ public void testCreateDetector_verifyWorkflowExecutionBucketLevelDocLevelMonitor
List<Map<String, Object>> findings = (List)getFindingsBody.get("findings");

Set<String> docLevelRules = new HashSet<>(List.of(randomDocRuleId));
List<String> bucketLevelMonitorFindingDocs = new ArrayList<>();
for(Map<String, Object> finding : findings) {
List<Map<String, Object>> queries = (List<Map<String, Object>>) finding.get("queries");
Set<String> findingRules = queries.stream().map(it -> it.get("id").toString()).collect(Collectors.toSet());
Expand All @@ -1504,16 +1503,10 @@ public void testCreateDetector_verifyWorkflowExecutionBucketLevelDocLevelMonitor
docLevelFinding.addAll((List<String>) finding.get("related_doc_ids"));
} else {
List<String> findingDocs = (List<String>) finding.get("related_doc_ids");
if (((Map<String, Object>) ((List<Object>) finding.get("queries")).get(0)).get("query").equals("_id:*")) {
Assert.assertEquals(1, findingDocs.size());
bucketLevelMonitorFindingDocs.addAll(findingDocs);
} else {
Assert.assertEquals(4, findingDocs.size());
assertTrue(Arrays.asList("1", "2", "3", "4").containsAll(findingDocs));
}
Assert.assertEquals(4, findingDocs.size());
assertTrue(Arrays.asList("1", "2", "3", "4").containsAll(findingDocs));
}
}
assertTrue(bucketLevelMonitorFindingDocs.containsAll(Arrays.asList("1", "2", "3", "4")));
// Verify doc level finding
assertTrue(Arrays.asList("1", "2", "3", "4", "5").containsAll(docLevelFinding));
}
Expand Down Expand Up @@ -1652,7 +1645,7 @@ public void testCreateDetector_verifyWorkflowExecutionMultipleBucketLevelDocLeve

// Assert findings
assertNotNull(getFindingsBody);
assertEquals(33, getFindingsBody.get("total_findings"));
assertEquals(19, getFindingsBody.get("total_findings"));
}


Expand Down
Loading
Loading