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

Fix get mappings view API incorrectly returning ecs path #867

Merged
merged 2 commits into from
Mar 11, 2024
Merged
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 @@ -346,15 +346,26 @@ private List<FieldMappingDoc> mergeFieldMappings(List<FieldMappingDoc> existingF
List<FieldMappingDoc> newFieldMappings = new ArrayList<>();
fieldMappingDocs.forEach( newFieldMapping -> {
Optional<FieldMappingDoc> foundFieldMappingDoc = Optional.empty();
for (FieldMappingDoc e: existingFieldMappings) {
if (e.getRawField().equals(newFieldMapping.getRawField())) {
for (FieldMappingDoc existingFieldMapping: existingFieldMappings) {
if (existingFieldMapping.getRawField().equals(newFieldMapping.getRawField())) {
if ((
e.get(defaultSchemaField) != null && newFieldMapping.get(defaultSchemaField) != null &&
e.get(defaultSchemaField).equals(newFieldMapping.get(defaultSchemaField))
existingFieldMapping.get(defaultSchemaField) != null && newFieldMapping.get(defaultSchemaField) != null &&
existingFieldMapping.get(defaultSchemaField).equals(newFieldMapping.get(defaultSchemaField))
) || (
e.get(defaultSchemaField) == null && newFieldMapping.get(defaultSchemaField) == null
existingFieldMapping.get(defaultSchemaField) == null && newFieldMapping.get(defaultSchemaField) == null
)) {
foundFieldMappingDoc = Optional.of(e);
foundFieldMappingDoc = Optional.of(existingFieldMapping);
}
// Grabs the right side of the ID with "|" as the delimiter if present representing the ecs field from predefined mappings
// Additional check to see if raw field path + log type combination is already in existingFieldMappings so a new one is not indexed
} else {
String id = existingFieldMapping.getId();
int indexOfPipe = id.indexOf("|");
if (indexOfPipe != -1) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this backwards compatible? Wondering if we will have duplicate mappings for a mapping that was generated before the pipe system was introduced (say an upgrade from an older to the current version).

I am also not very familiar with this part of the code though, so if that is not a concern, then I am good to approve.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Backwards compatibility should be fine because the mapping should not have been inserted in the first place but now we're ensuring that it doesn't get generated. If they encounter this bug though I think they may need to delete the log-type-config index so that we can automatically regenerate it with the correct mappings.

String ecsIdField = id.substring(indexOfPipe + 1);
if (ecsIdField.equals(newFieldMapping.getRawField()) && existingFieldMapping.getLogTypes().containsAll(newFieldMapping.getLogTypes())) {
foundFieldMappingDoc = Optional.of(existingFieldMapping);
}
}
}
}
Expand Down
29 changes: 29 additions & 0 deletions src/test/java/org/opensearch/securityanalytics/TestHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,35 @@ public static String randomRule() {
"level: high";
}

public static String randomRuleWithRawField() {
return "title: Remote Encrypting File System Abuse\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:/jsecurity101/MSRPC-to-ATTACK/blob/main/documents/MS-EFSR.md\n" +
" - https:/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" +
" eventName: testinghere\n" +
" condition: selection\n" +
"falsepositives:\n" +
" - Legitimate usage of remote file encryption\n" +
"level: high";
}

public static String randomRuleWithNotCondition() {
return "title: Remote Encrypting File System Abuse\n" +
"id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,52 @@ public void testOCSFCloudtrailGetMappingsViewApi() throws IOException {
assertEquals(24, unmappedFieldAliases.size());
}

@SuppressWarnings("unchecked")
public void testOCSFCloudtrailGetMappingsViewApiWithCustomRule() throws IOException {
String index = createTestIndex("cloudtrail", ocsfCloudtrailMappings());

Request request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
// both req params and req body are supported
request.addParameter("index_name", index);
request.addParameter("rule_topic", "cloudtrail");
Response response = client().performRequest(request);
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
Map<String, Object> respMap = responseAsMap(response);
// Verify alias mappings
Map<String, Object> props = (Map<String, Object>) respMap.get("properties");
Assert.assertEquals(18, props.size());
// Verify unmapped index fields
List<String> unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
assertEquals(20, unmappedIndexFields.size());
// Verify unmapped field aliases
List<String> unmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
assertEquals(25, unmappedFieldAliases.size());

// create a cloudtrail rule with a raw field
String rule = randomRuleWithRawField();
Response createResponse = makeRequest(client(), "POST", SecurityAnalyticsPlugin.RULE_BASE_URI, Collections.singletonMap("category", "cloudtrail"),
new StringEntity(rule), new BasicHeader("Content-Type", "application/json"));
Assert.assertEquals("Create rule failed", RestStatus.CREATED, restStatus(createResponse));

// check the mapping view API again to ensure it's the same after rule is created
Response response2 = client().performRequest(request);
assertEquals(HttpStatus.SC_OK, response2.getStatusLine().getStatusCode());
Map<String, Object> respMap2 = responseAsMap(response2);
// Verify alias mappings
Map<String, Object> props2 = (Map<String, Object>) respMap2.get("properties");
Assert.assertEquals(18, props2.size());
// Verify unmapped index fields
List<String> unmappedIndexFields2 = (List<String>) respMap2.get("unmapped_index_fields");
assertEquals(20, unmappedIndexFields2.size());
// Verify unmapped field aliases
List<String> unmappedFieldAliases2 = (List<String>) respMap2.get("unmapped_field_aliases");
assertEquals(25, unmappedFieldAliases2.size());
// Verify that first response and second response are the same after rule was indexed
assertEquals(props, props2);
assertEquals(unmappedIndexFields, unmappedIndexFields2);
assertEquals(unmappedFieldAliases, unmappedFieldAliases2);
}

@SuppressWarnings("unchecked")
public void testOCSFVpcflowGetMappingsViewApi() throws IOException {
String index = createTestIndex("vpcflow", ocsfVpcflowMappings());
Expand Down
Loading