Skip to content

Commit 4139155

Browse files
riysaxen-amznjowg-amazoneirsep
authored
Backport 2.9 fix (#937)
* Fix duplicate ecs mappings which returns incorrect log index field in mapping view API (#786) (#788) * field mapping changes Signed-off-by: Joanne Wang <[email protected]> * add integ test Signed-off-by: Joanne Wang <[email protected]> * turn unmappedfieldaliases as set and add integ test Signed-off-by: Joanne Wang <[email protected]> * add comments Signed-off-by: Joanne Wang <[email protected]> * fix integ tests Signed-off-by: Joanne Wang <[email protected]> * moved logic to method for better readability Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> * Fix get mappings view API incorrectly returning ecs path (#867) * add logic and integ tests to not add duplicate to log-types-config index Signed-off-by: Joanne Wang <[email protected]> * change naming for existingFieldMapping and change contains to equals Signed-off-by: Joanne Wang <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> * fix integ test (#918) Signed-off-by: Joanne Wang <[email protected]> * fix detector writeTo() method missing fields (#695) * fix detector writeTo() method missing fields Signed-off-by: Surya Sashank Nistala <[email protected]> * fix test Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]> * removing threatIntel related code Signed-off-by: Riya Saxena <[email protected]> * removing threatIntel related code Signed-off-by: Riya Saxena <[email protected]> * ignore flaky tests Signed-off-by: Riya Saxena <[email protected]> --------- Signed-off-by: Joanne Wang <[email protected]> Signed-off-by: Surya Sashank Nistala <[email protected]> Signed-off-by: Riya Saxena <[email protected]> Co-authored-by: Joanne Wang <[email protected]> Co-authored-by: Surya Sashank Nistala <[email protected]>
1 parent e44e5d4 commit 4139155

File tree

7 files changed

+367
-15
lines changed

7 files changed

+367
-15
lines changed

‎src/main/java/org/opensearch/securityanalytics/logtype/LogTypeService.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,15 +344,26 @@ private List<FieldMappingDoc> mergeFieldMappings(List<FieldMappingDoc> existingF
344344
List<FieldMappingDoc> newFieldMappings = new ArrayList<>();
345345
fieldMappingDocs.forEach( newFieldMapping -> {
346346
Optional<FieldMappingDoc> foundFieldMappingDoc = Optional.empty();
347-
for (FieldMappingDoc e: existingFieldMappings) {
348-
if (e.getRawField().equals(newFieldMapping.getRawField())) {
347+
for (FieldMappingDoc existingFieldMapping: existingFieldMappings) {
348+
if (existingFieldMapping.getRawField().equals(newFieldMapping.getRawField())) {
349349
if ((
350-
e.get(defaultSchemaField) != null && newFieldMapping.get(defaultSchemaField) != null &&
351-
e.get(defaultSchemaField).equals(newFieldMapping.get(defaultSchemaField))
350+
existingFieldMapping.get(defaultSchemaField) != null && newFieldMapping.get(defaultSchemaField) != null &&
351+
existingFieldMapping.get(defaultSchemaField).equals(newFieldMapping.get(defaultSchemaField))
352352
) || (
353-
e.get(defaultSchemaField) == null && newFieldMapping.get(defaultSchemaField) == null
353+
existingFieldMapping.get(defaultSchemaField) == null && newFieldMapping.get(defaultSchemaField) == null
354354
)) {
355-
foundFieldMappingDoc = Optional.of(e);
355+
foundFieldMappingDoc = Optional.of(existingFieldMapping);
356+
}
357+
// Grabs the right side of the ID with "|" as the delimiter if present representing the ecs field from predefined mappings
358+
// Additional check to see if raw field path + log type combination is already in existingFieldMappings so a new one is not indexed
359+
} else {
360+
String id = existingFieldMapping.getId();
361+
int indexOfPipe = id.indexOf("|");
362+
if (indexOfPipe != -1) {
363+
String ecsIdField = id.substring(indexOfPipe + 1);
364+
if (ecsIdField.equals(newFieldMapping.getRawField()) && existingFieldMapping.getLogTypes().containsAll(newFieldMapping.getLogTypes())) {
365+
foundFieldMappingDoc = Optional.of(existingFieldMapping);
366+
}
356367
}
357368
}
358369
}

‎src/main/java/org/opensearch/securityanalytics/model/Detector.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@ public Detector(StreamInput sin) throws IOException {
159159
sin.readList(DetectorInput::readFrom),
160160
sin.readList(DetectorTrigger::readFrom),
161161
sin.readStringList(),
162-
sin.readString(),
163-
sin.readString(),
164-
sin.readString(),
165-
sin.readString(),
166-
sin.readString(),
167-
sin.readString(),
162+
sin.readOptionalString(),
163+
sin.readOptionalString(),
164+
sin.readOptionalString(),
165+
sin.readOptionalString(),
166+
sin.readOptionalString(),
167+
sin.readOptionalString(),
168168
sin.readMap(StreamInput::readString, StreamInput::readString)
169169
);
170170
}
@@ -197,8 +197,12 @@ public void writeTo(StreamOutput out) throws IOException {
197197
it.writeTo(out);
198198
}
199199
out.writeStringCollection(monitorIds);
200-
out.writeString(ruleIndex);
201-
200+
out.writeOptionalString(ruleIndex);
201+
out.writeOptionalString(alertsIndex);
202+
out.writeOptionalString(alertsHistoryIndex);
203+
out.writeOptionalString(alertsHistoryIndexPattern);
204+
out.writeOptionalString(findingsIndex);
205+
out.writeOptionalString(findingsIndexPattern);
202206
out.writeMap(ruleIdMonitorIdMap, StreamOutput::writeString, StreamOutput::writeString);
203207
}
204208

‎src/test/java/org/opensearch/securityanalytics/TestHelpers.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,100 @@ public static String randomRule() {
208208
" - Legitimate usage of remote file encryption\n" +
209209
"level: high";
210210
}
211+
public static String randomRuleWithRawField() {
212+
return "title: Remote Encrypting File System Abuse\n" +
213+
"id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" +
214+
"description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" +
215+
"references:\n" +
216+
" - https://attack.mitre.org/tactics/TA0008/\n" +
217+
" - https://msrc.microsoft.com/update-guide/vulnerability/CVE-2021-36942\n" +
218+
" - https://github.com/jsecurity101/MSRPC-to-ATTACK/blob/main/documents/MS-EFSR.md\n" +
219+
" - https://github.com/zeronetworks/rpcfirewall\n" +
220+
" - https://zeronetworks.com/blog/stopping_lateral_movement_via_the_rpc_firewall/\n" +
221+
"tags:\n" +
222+
" - attack.defense_evasion\n" +
223+
"status: experimental\n" +
224+
"author: Sagie Dulce, Dekel Paz\n" +
225+
"date: 2022/01/01\n" +
226+
"modified: 2022/01/01\n" +
227+
"logsource:\n" +
228+
" product: rpc_firewall\n" +
229+
" category: application\n" +
230+
" 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" +
231+
"detection:\n" +
232+
" selection:\n" +
233+
" eventName: testinghere\n" +
234+
" condition: selection\n" +
235+
"falsepositives:\n" +
236+
" - Legitimate usage of remote file encryption\n" +
237+
"level: high";
238+
}
239+
240+
public static String randomRuleWithNotCondition() {
241+
return "title: Remote Encrypting File System Abuse\n" +
242+
"id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" +
243+
"description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" +
244+
"references:\n" +
245+
" - https://attack.mitre.org/tactics/TA0008/\n" +
246+
" - https://msrc.microsoft.com/update-guide/vulnerability/CVE-2021-36942\n" +
247+
" - https://github.com/jsecurity101/MSRPC-to-ATTACK/blob/main/documents/MS-EFSR.md\n" +
248+
" - https://github.com/zeronetworks/rpcfirewall\n" +
249+
" - https://zeronetworks.com/blog/stopping_lateral_movement_via_the_rpc_firewall/\n" +
250+
"tags:\n" +
251+
" - attack.defense_evasion\n" +
252+
"status: experimental\n" +
253+
"author: Sagie Dulce, Dekel Paz\n" +
254+
"date: 2022/01/01\n" +
255+
"modified: 2022/01/01\n" +
256+
"logsource:\n" +
257+
" product: rpc_firewall\n" +
258+
" category: application\n" +
259+
" 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" +
260+
"detection:\n" +
261+
" selection1:\n" +
262+
" AccountType: TestAccountType\n" +
263+
" selection2:\n" +
264+
" AccountName: TestAccountName\n" +
265+
" selection3:\n" +
266+
" EventID: 22\n" +
267+
" condition: (not selection1 and not selection2) and selection3\n" +
268+
"falsepositives:\n" +
269+
" - Legitimate usage of remote file encryption\n" +
270+
"level: high";
271+
}
272+
273+
public static String randomRuleWithNotConditionBoolAndNum() {
274+
return "title: Remote Encrypting File System Abuse\n" +
275+
"id: 5f92fff9-82e2-48eb-8fc1-8b133556a551\n" +
276+
"description: Detects remote RPC calls to possibly abuse remote encryption service via MS-EFSR\n" +
277+
"references:\n" +
278+
" - https://attack.mitre.org/tactics/TA0008/\n" +
279+
" - https://msrc.microsoft.com/update-guide/vulnerability/CVE-2021-36942\n" +
280+
" - https://github.com/jsecurity101/MSRPC-to-ATTACK/blob/main/documents/MS-EFSR.md\n" +
281+
" - https://github.com/zeronetworks/rpcfirewall\n" +
282+
" - https://zeronetworks.com/blog/stopping_lateral_movement_via_the_rpc_firewall/\n" +
283+
"tags:\n" +
284+
" - attack.defense_evasion\n" +
285+
"status: experimental\n" +
286+
"author: Sagie Dulce, Dekel Paz\n" +
287+
"date: 2022/01/01\n" +
288+
"modified: 2022/01/01\n" +
289+
"logsource:\n" +
290+
" product: rpc_firewall\n" +
291+
" category: application\n" +
292+
" 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" +
293+
"detection:\n" +
294+
" selection1:\n" +
295+
" Initiated: \"false\"\n" +
296+
" selection2:\n" +
297+
" AccountName: TestAccountName\n" +
298+
" selection3:\n" +
299+
" EventID: 21\n" +
300+
" condition: not selection1 and not selection3\n" +
301+
"falsepositives:\n" +
302+
" - Legitimate usage of remote file encryption\n" +
303+
"level: high";
304+
}
211305

212306
public static String randomNullRule() {
213307
return "title: null field\n" +

‎src/test/java/org/opensearch/securityanalytics/findings/FindingIT.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.stream.Collectors;
1414
import org.apache.http.HttpStatus;
1515
import org.junit.Assert;
16+
import org.junit.Ignore;
1617
import org.opensearch.client.Request;
1718
import org.opensearch.client.Response;
1819
import org.opensearch.client.ResponseException;
@@ -262,7 +263,7 @@ public void testGetFindings_byDetectorType_success() throws IOException {
262263
getFindingsBody = entityAsMap(getFindingsResponse);
263264
Assert.assertEquals(1, getFindingsBody.get("total_findings"));
264265
}
265-
266+
@Ignore("Test is currently ignored because of flakiness, need to fix it")
266267
public void testGetFindings_rolloverByMaxAge_success() throws IOException, InterruptedException {
267268

268269
updateClusterSetting(FINDING_HISTORY_ROLLOVER_PERIOD.getKey(), "1s");

‎src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,112 @@ public void testGetMappingsViewLinuxSuccess() throws IOException {
389389
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
390390
}
391391

392+
// Tests mappings where multiple raw fields correspond to one ecs value
393+
public void testGetMappingsViewWindowsSuccess() throws IOException {
394+
395+
String testIndexName = "get_mappings_view_index";
396+
397+
createSampleWindex(testIndexName);
398+
399+
// Execute GetMappingsViewAction to add alias mapping for index
400+
Request request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
401+
// both req params and req body are supported
402+
request.addParameter("index_name", testIndexName);
403+
request.addParameter("rule_topic", "windows");
404+
Response response = client().performRequest(request);
405+
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
406+
Map<String, Object> respMap = responseAsMap(response);
407+
408+
// Verify alias mappings
409+
Map<String, Object> props = (Map<String, Object>) respMap.get("properties");
410+
assertEquals(3, props.size());
411+
assertTrue(props.containsKey("winlog.event_data.LogonType"));
412+
assertTrue(props.containsKey("winlog.provider_name"));
413+
assertTrue(props.containsKey("host.hostname"));
414+
415+
// Verify unmapped index fields
416+
List<String> unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
417+
assertEquals(3, unmappedIndexFields.size());
418+
assert(unmappedIndexFields.contains("plain1"));
419+
assert(unmappedIndexFields.contains("ParentUser.first"));
420+
assert(unmappedIndexFields.contains("ParentUser.last"));
421+
422+
// Verify unmapped field aliases
423+
List<String> filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
424+
assertEquals(191, filteredUnmappedFieldAliases.size());
425+
assert(!filteredUnmappedFieldAliases.contains("winlog.event_data.LogonType"));
426+
assert(!filteredUnmappedFieldAliases.contains("winlog.provider_name"));
427+
assert(!filteredUnmappedFieldAliases.contains("host.hostname"));
428+
429+
// Index a doc for a field with multiple raw fields corresponding to one ecs field
430+
indexDoc(testIndexName, "1", "{ \"EventID\": 1 }");
431+
// Execute GetMappingsViewAction to add alias mapping for index
432+
request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
433+
// both req params and req body are supported
434+
request.addParameter("index_name", testIndexName);
435+
request.addParameter("rule_topic", "windows");
436+
response = client().performRequest(request);
437+
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
438+
respMap = responseAsMap(response);
439+
440+
// Verify alias mappings
441+
props = (Map<String, Object>) respMap.get("properties");
442+
assertEquals(4, props.size());
443+
assertTrue(props.containsKey("winlog.event_id"));
444+
445+
// verify unmapped index fields
446+
unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
447+
assertEquals(3, unmappedIndexFields.size());
448+
449+
// verify unmapped field aliases
450+
filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
451+
assertEquals(190, filteredUnmappedFieldAliases.size());
452+
assert(!filteredUnmappedFieldAliases.contains("winlog.event_id"));
453+
}
454+
455+
// Tests mappings where multiple raw fields correspond to one ecs value and all fields are present in the index
456+
public void testGetMappingsViewMulitpleRawFieldsSuccess() throws IOException {
457+
458+
String testIndexName = "get_mappings_view_index";
459+
460+
createSampleWindex(testIndexName);
461+
String sampleDoc = "{" +
462+
" \"EventID\": 1," +
463+
" \"EventId\": 2," +
464+
" \"event_uid\": 3" +
465+
"}";
466+
indexDoc(testIndexName, "1", sampleDoc);
467+
468+
// Execute GetMappingsViewAction to add alias mapping for index
469+
Request request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
470+
// both req params and req body are supported
471+
request.addParameter("index_name", testIndexName);
472+
request.addParameter("rule_topic", "windows");
473+
Response response = client().performRequest(request);
474+
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
475+
Map<String, Object> respMap = responseAsMap(response);
476+
477+
// Verify alias mappings
478+
Map<String, Object> props = (Map<String, Object>) respMap.get("properties");
479+
assertEquals(4, props.size());
480+
assertTrue(props.containsKey("winlog.event_data.LogonType"));
481+
assertTrue(props.containsKey("winlog.provider_name"));
482+
assertTrue(props.containsKey("host.hostname"));
483+
assertTrue(props.containsKey("winlog.event_id"));
484+
485+
// Verify unmapped index fields
486+
List<String> unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
487+
assertEquals(5, unmappedIndexFields.size());
488+
489+
// Verify unmapped field aliases
490+
List<String> filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
491+
assertEquals(190, filteredUnmappedFieldAliases.size());
492+
assert(!filteredUnmappedFieldAliases.contains("winlog.event_data.LogonType"));
493+
assert(!filteredUnmappedFieldAliases.contains("winlog.provider_name"));
494+
assert(!filteredUnmappedFieldAliases.contains("host.hostname"));
495+
assert(!filteredUnmappedFieldAliases.contains("winlog.event_id"));
496+
}
497+
392498
public void testCreateMappings_withDatastream_success() throws IOException {
393499
String datastream = "test_datastream";
394500

@@ -1272,6 +1378,69 @@ private void createSampleIndex(String indexName, Settings settings, String alias
12721378
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
12731379
}
12741380

1381+
private void createSampleWindex(String indexName) throws IOException {
1382+
createSampleWindex(indexName, Settings.EMPTY, null);
1383+
}
1384+
1385+
private void createSampleWindex(String indexName, Settings settings, String aliases) throws IOException {
1386+
String indexMapping =
1387+
" \"properties\": {" +
1388+
" \"LogonType\": {" +
1389+
" \"type\": \"integer\"" +
1390+
" }," +
1391+
" \"Provider\": {" +
1392+
" \"type\": \"text\"" +
1393+
" }," +
1394+
" \"hostname\": {" +
1395+
" \"type\": \"text\"" +
1396+
" }," +
1397+
" \"plain1\": {" +
1398+
" \"type\": \"integer\"" +
1399+
" }," +
1400+
" \"ParentUser\":{" +
1401+
" \"type\":\"nested\"," +
1402+
" \"properties\":{" +
1403+
" \"first\":{" +
1404+
" \"type\":\"text\"," +
1405+
" \"fields\":{" +
1406+
" \"keyword\":{" +
1407+
" \"type\":\"keyword\"," +
1408+
" \"ignore_above\":256" +
1409+
"}" +
1410+
"}" +
1411+
"}," +
1412+
" \"last\":{" +
1413+
"\"type\":\"text\"," +
1414+
"\"fields\":{" +
1415+
" \"keyword\":{" +
1416+
" \"type\":\"keyword\"," +
1417+
" \"ignore_above\":256" +
1418+
"}" +
1419+
"}" +
1420+
"}" +
1421+
"}" +
1422+
"}" +
1423+
" }";
1424+
1425+
createIndex(indexName, settings, indexMapping, aliases);
1426+
1427+
// Insert sample doc with event_uid not explicitly mapped
1428+
String sampleDoc = "{" +
1429+
" \"LogonType\":1," +
1430+
" \"Provider\":\"Microsoft-Windows-Security-Auditing\"," +
1431+
" \"hostname\":\"FLUXCAPACITOR\"" +
1432+
"}";
1433+
1434+
// Index doc
1435+
Request indexRequest = new Request("POST", indexName + "/_doc?refresh=wait_for");
1436+
indexRequest.setJsonEntity(sampleDoc);
1437+
Response response = client().performRequest(indexRequest);
1438+
assertEquals(HttpStatus.SC_CREATED, response.getStatusLine().getStatusCode());
1439+
// Refresh everything
1440+
response = client().performRequest(new Request("POST", "_refresh"));
1441+
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
1442+
}
1443+
12751444
private void createSampleDatastream(String datastreamName) throws IOException {
12761445
String indexMapping =
12771446
" \"properties\": {" +

0 commit comments

Comments
 (0)